Skip to content

Commit

Permalink
Merge pull request #366 from chrisccoulson/fix-role-plumbing
Browse files Browse the repository at this point in the history 
Fix plumbing of the role via the PlatformKeyDataHandler interface.

PR #361 addressed the issue that most tpm2 tests were being performed with an empty role, which isn't realistic. It turns out that I missed a few places, particularly in some of the platform tests, that if I had fixed in the previous PR, would have noticed that the role parameter isn't being passed through the platform interface (this would get picked up when changing a password because key validation would fail).

This addresses this, and fixes tests in the top-level package to catch it as well.

Note that I'm using the same value in most tests - my normal approach to unit testing is to test each input with at least more than one value, but the tests in the top-level secboot package and the tpm2 sub-package are getting a bit complicated with a lot of inconsistencies in their approach to testing. I have issues #329 and #330 open to refactor the tests so they are structured in a more consistent way, like the tests for the recently added plainkey platform are.
  • Loading branch information
@chrisccoulson
chrisccoulson authored Jan 27, 2025
2 parents 4525521 + c4f6057 commit e908152
Show file tree
Hide file tree
Showing 7 changed files with 111 additions and 77 deletions.
19 changes: 9 additions & 10 deletions crypt_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -421,7 +421,6 @@ func (s *cryptSuite) TearDownSuite(c *C) {

func (s *cryptSuite) SetUpTest(c *C) {
s.AddCleanup(MockUnixStat(func(devicePath string, st *unix.Stat_t) error {
fmt.Printf("Called mock for %s\n", devicePath)
s.requestedStats = append(s.requestedStats, devicePath)
foundSt, hasSt := s.deviceStats[devicePath]
if !hasSt {
Expand All @@ -433,11 +432,11 @@ func (s *cryptSuite) SetUpTest(c *C) {

s.deviceStats = map[string]unix.Stat_t{
"/dev/sda1": unix.Stat_t{
Mode: 0600|unix.S_IFBLK,
Mode: 0600 | unix.S_IFBLK,
Rdev: unix.Mkdev(8, 1),
},
"/dev/vda2": unix.Stat_t{
Mode: 0600|unix.S_IFBLK,
Mode: 0600 | unix.S_IFBLK,
Rdev: unix.Mkdev(9, 2),
},
}
Expand Down Expand Up @@ -520,7 +519,7 @@ func (s *cryptSuite) checkKeyDataKeysInKeyring(c *C, prefix, path string, expect
func (s *cryptSuite) newMultipleNamedKeyData(c *C, names ...string) (keyData []*KeyData, keys []DiskUnlockKey, primaryKeys []PrimaryKey) {
for _, name := range names {
primaryKey := s.newPrimaryKey(c, 32)
protected, unlockKey := s.mockProtectKeys(c, primaryKey, crypto.SHA256, crypto.SHA256)
protected, unlockKey := s.mockProtectKeys(c, primaryKey, "foo", crypto.SHA256)

kd, err := NewKeyData(protected)
c.Assert(err, IsNil)
Expand Down Expand Up @@ -548,7 +547,7 @@ func (s *cryptSuite) newNamedKeyData(c *C, name string) (*KeyData, DiskUnlockKey
func (s *cryptSuite) newMultipleNamedKeyDataWithPassphrases(c *C, passphrases []string, names ...string) (keyData []*KeyData, keys []DiskUnlockKey, primaryKeys []PrimaryKey) {
for i, name := range names {
primaryKey := s.newPrimaryKey(c, 32)
protected, unlockKey := s.mockProtectKeysWithPassphrase(c, primaryKey, nil, 32, crypto.SHA256, crypto.SHA256)
protected, unlockKey := s.mockProtectKeysWithPassphrase(c, primaryKey, "foo", nil, 32, crypto.SHA256)

kd, err := NewKeyDataWithPassphrase(protected, passphrases[i])
c.Assert(err, IsNil)
Expand Down Expand Up @@ -791,7 +790,7 @@ func (s *cryptSuite) testActivateVolumeWithRecoveryKeyErrorHandling(c *C, data *
defer MockUnixStat(func(devicePath string, st *unix.Stat_t) error {
c.Check(devicePath, Equals, "/dev/sda1")
*st = unix.Stat_t{
Mode: 0600|unix.S_IFBLK,
Mode: 0600 | unix.S_IFBLK,
Rdev: unix.Mkdev(8, 1),
}
return nil
Expand Down Expand Up @@ -4011,11 +4010,11 @@ func (s *cryptSuite) TestActivateVolumeWithMultipleLegacyKeyDataErrorHandling15(
func (s *cryptSuite) TestActivateVolumeWithLegacyPaths(c *C) {
s.deviceStats = map[string]unix.Stat_t{
"/dev/some/path": unix.Stat_t{
Mode: 0600|unix.S_IFBLK,
Mode: 0600 | unix.S_IFBLK,
Rdev: unix.Mkdev(8, 1),
},
"/dev/some/legacy/path": unix.Stat_t{
Mode: 0600|unix.S_IFBLK,
Mode: 0600 | unix.S_IFBLK,
Rdev: unix.Mkdev(8, 1),
},
}
Expand All @@ -4033,11 +4032,11 @@ func (s *cryptSuite) TestActivateVolumeWithLegacyPaths(c *C) {
func (s *cryptSuite) TestActivateVolumeWithLegacyPathsError(c *C) {
s.deviceStats = map[string]unix.Stat_t{
"/dev/some/path": unix.Stat_t{
Mode: 0600|unix.S_IFBLK,
Mode: 0600 | unix.S_IFBLK,
Rdev: unix.Mkdev(8, 1),
},
"/dev/some/legacy/path": unix.Stat_t{
Mode: 0600|unix.S_IFBLK,
Mode: 0600 | unix.S_IFBLK,
// different node
Rdev: unix.Mkdev(8, 2),
},
Expand Down
1 change: 1 addition & 0 deletions keydata.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -533,6 +533,7 @@ func (d *KeyData) platformKeyData() *PlatformKeyData {
return &PlatformKeyData{
Generation: d.Generation(),
EncodedHandle: d.data.PlatformHandle,
Role: d.data.Role,
KDFAlg: crypto.Hash(d.data.KDFAlg),
AuthMode: d.AuthMode(),
}
Expand Down
10 changes: 6 additions & 4 deletions keydata_file_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ var _ = Suite(&keyDataFileSuite{})

func (s *keyDataFileSuite) TestWriter(c *C) {
primaryKey := s.newPrimaryKey(c, 32)
protected, _ := s.mockProtectKeys(c, primaryKey, crypto.SHA256, crypto.SHA256)
protected, _ := s.mockProtectKeys(c, primaryKey, "foo", crypto.SHA256)

keyData, err := NewKeyData(protected)
c.Assert(err, IsNil)
Expand All @@ -64,12 +64,12 @@ func (s *keyDataFileSuite) TestWriter(c *C) {
d := json.NewDecoder(f)
c.Check(d.Decode(&j), IsNil)

s.checkKeyDataJSONDecodedAuthModeNone(c, j, protected, 0)
s.checkKeyDataJSONDecodedAuthModeNone(c, j, protected)
}

func (s *keyDataFileSuite) TestWriterIsAtomic(c *C) {
primaryKey := s.newPrimaryKey(c, 32)
protected, _ := s.mockProtectKeys(c, primaryKey, crypto.SHA256, crypto.SHA256)
protected, _ := s.mockProtectKeys(c, primaryKey, "foo", crypto.SHA256)

keyData, err := NewKeyData(protected)
c.Assert(err, IsNil)
Expand All @@ -93,7 +93,7 @@ func (s *keyDataFileSuite) TestWriterIsAtomic(c *C) {

func (s *keyDataFileSuite) TestReader(c *C) {
primaryKey := s.newPrimaryKey(c, 32)
protected, unlockKey := s.mockProtectKeys(c, primaryKey, crypto.SHA256, crypto.SHA256)
protected, unlockKey := s.mockProtectKeys(c, primaryKey, "foo", crypto.SHA256)

keyData, err := NewKeyData(protected)
c.Assert(err, IsNil)
Expand All @@ -118,6 +118,8 @@ func (s *keyDataFileSuite) TestReader(c *C) {
c.Check(err, IsNil)
c.Check(id, DeepEquals, expectedId)

c.Check(keyData.Role(), Equals, "foo")

recoveredUnlockKey, recoveredPrimaryKey, err := keyData.RecoverKeys()
c.Check(err, IsNil)
c.Check(recoveredUnlockKey, DeepEquals, unlockKey)
Expand Down
13 changes: 7 additions & 6 deletions keydata_luks_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ func (s *keyDataLuksSuite) TearDownTest(c *C) {

var _ = Suite(&keyDataLuksSuite{})

func (s *keyDataLuksSuite) checkKeyDataJSONFromLUKSToken(c *C, path string, id int, keyslot int, name string, priority int, creationParams *KeyParams, nmodels int) {
func (s *keyDataLuksSuite) checkKeyDataJSONFromLUKSToken(c *C, path string, id int, keyslot int, name string, priority int, creationParams *KeyParams) {
t, exists := s.luks2.devices[path].tokens[id]
c.Assert(exists, testutil.IsTrue)

Expand All @@ -83,7 +83,7 @@ func (s *keyDataLuksSuite) checkKeyDataJSONFromLUKSToken(c *C, path string, id i
keyData, ok := token.Params["ubuntu_fde_data"].(map[string]interface{})
c.Assert(ok, testutil.IsTrue)

s.checkKeyDataJSONDecodedAuthModeNone(c, keyData, creationParams, nmodels)
s.checkKeyDataJSONDecodedAuthModeNone(c, keyData, creationParams)
}

type testKeyDataLuksWriterData struct {
Expand All @@ -97,7 +97,7 @@ type testKeyDataLuksWriterData struct {

func (s *keyDataLuksSuite) testWriter(c *C, data *testKeyDataLuksWriterData) {
primaryKey := s.newPrimaryKey(c, 32)
protected, unlockKey := s.mockProtectKeys(c, primaryKey, crypto.SHA256, crypto.SHA256)
protected, unlockKey := s.mockProtectKeys(c, primaryKey, "foo", crypto.SHA256)

s.luks2.devices[data.path] = &mockLUKS2Container{
tokens: map[int]luks2.Token{
Expand Down Expand Up @@ -125,7 +125,7 @@ func (s *keyDataLuksSuite) testWriter(c *C, data *testKeyDataLuksWriterData) {
fmt.Sprint("ImportToken(", data.path, ",", &luks2.ImportTokenOptions{Id: data.id, Replace: true}, ")"),
})

s.checkKeyDataJSONFromLUKSToken(c, data.path, data.id, data.slot, data.name, data.setPriority, protected, 0)
s.checkKeyDataJSONFromLUKSToken(c, data.path, data.id, data.slot, data.name, data.setPriority, protected)
}

func (s *keyDataLuksSuite) TestWriter(c *C) {
Expand Down Expand Up @@ -238,7 +238,7 @@ type testKeyDataLuksReaderData struct {

func (s *keyDataLuksSuite) testReader(c *C, data *testKeyDataLuksReaderData) {
primaryKey := s.newPrimaryKey(c, 32)
protected, unlockKey := s.mockProtectKeys(c, primaryKey, crypto.SHA256, crypto.SHA256)
protected, unlockKey := s.mockProtectKeys(c, primaryKey, "foo", crypto.SHA256)

s.luks2.devices[data.path] = &mockLUKS2Container{
tokens: map[int]luks2.Token{
Expand Down Expand Up @@ -361,7 +361,7 @@ func (s *keyDataLuksUnmockedSuite) TestReaderAndWriter(c *C) {
path := luks2test.CreateEmptyDiskImage(c, 20)

primaryKey := s.newPrimaryKey(c, 32)
protected, unlockKey := s.mockProtectKeys(c, primaryKey, crypto.SHA256, crypto.SHA256)
protected, unlockKey := s.mockProtectKeys(c, primaryKey, "foo", crypto.SHA256)
c.Check(InitializeLUKS2Container(path, "", unlockKey, nil), IsNil)

keyData, err := NewKeyData(protected)
Expand All @@ -385,6 +385,7 @@ func (s *keyDataLuksUnmockedSuite) TestReaderAndWriter(c *C) {
keyData, err = ReadKeyData(r)
c.Assert(err, IsNil)
c.Check(keyData.ReadableName(), Equals, path+":default")
c.Check(keyData.Role(), Equals, "foo")

id, err := keyData.UniqueID()
c.Check(err, IsNil)
Expand Down
Loading

0 comments on commit e908152

Please sign in to comment.