Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[1/3] Keydata v3 platform API changes #265

Merged
merged 23 commits into from
Feb 19, 2024
Merged

[1/3] Keydata v3 platform API changes #265

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
0dc8c1b
keydata.go: introduce MakeDiskUnlockKey() API and new key format
sespiros Oct 18, 2023
f6cba7e
keydata.go: allow marshalling hashAlg to ASN1
sespiros Oct 16, 2023
cfcca36
platform.go: add new API for PlatformKeyData
sespiros Oct 13, 2023
0712e4b
keydata.go: initial changes to passphrase-backed keys
sespiros Oct 16, 2023
7fe9a98
keydata.go: add NewKeyDataWithPassphrase API
sespiros Oct 16, 2023
98db494
keydata.go: remove old API around passphrase protected payloads
sespiros Oct 16, 2023
fdfa79e
keydata*test.go: fix/add tests
sespiros Oct 19, 2023
6452fb6
keydata*test.go: add API tests for keydata v3
sespiros Oct 19, 2023
977d320
crypt_test.go: add API tests for keydata v3
sespiros Oct 20, 2023
5af09ed
keydata*.go: refactored the legacy keyData test suite
sespiros Jan 9, 2024
1a9de9b
keydata*.go: modify version behavior and fix legacy test cases
sespiros Jan 10, 2024
08c2d56
keydata_test.go: make use of helper in tests
sespiros Jan 15, 2024
c8cb933
keydata*test.go: validate mock platform data fields
sespiros Jan 15, 2024
3cf4c26
keydata.go,platform.go: add doc comments
sespiros Jan 15, 2024
759419a
keydata.go: add doc comments
sespiros Jan 22, 2024
ce63a2f
keydata_test.go: add tests for derivePassphraseKeys error handling
sespiros Jan 22, 2024
25099ff
keydata_test.go: test cleanup
sespiros Jan 22, 2024
c7d0f3b
fixup! keydata*test.go: validate mock platform data fields
sespiros Jan 29, 2024
7f9a3b7
fixup! keydata*test.go: validate mock platform data fields
sespiros Jan 29, 2024
8c92177
keydata*test.go: add test case for unavailable KDF
sespiros Feb 7, 2024
18b89e5
keydata.go: add doc comment for KDFAlg field
sespiros Feb 7, 2024
23c36bc
keydata_test.go: add test case for derivation info fields
sespiros Feb 7, 2024
ce88f13
multiple: rename version field to generation
sespiros Feb 13, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions export_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ import (
var (
UnmarshalV1KeyPayload = unmarshalV1KeyPayload
UnmarshalProtectedKeys = unmarshalProtectedKeys
KeyDataVersion = keyDataVersion
KeyDataGeneration = keyDataGeneration
)

type ProtectedKeys = protectedKeys
Expand Down Expand Up @@ -128,11 +128,11 @@ func MockStderr(w io.Writer) (restore func()) {
}
}

func MockKeyDataVersion(n int) (restore func()) {
orig := keyDataVersion
keyDataVersion = n
func MockKeyDataGeneration(n int) (restore func()) {
orig := keyDataGeneration
keyDataGeneration = n
return func() {
keyDataVersion = orig
keyDataGeneration = orig
}
}

Expand Down
28 changes: 16 additions & 12 deletions keydata.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ const (
)

var (
keyDataVersion int = 2
keyDataGeneration int = 2
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a short doc comment describing this even it's private might not hurt

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've actually got a follow-up PR that makes this public (so that the version can be bound into the AAD)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh sorry about that, I thought I had that change in here already, I also make it public in the final PR because I needed it for a test for AAD 33445cf

snapModelHMACKDFLabel = []byte("SNAP-MODEL-HMAC")
sha1Oid = asn1.ObjectIdentifier{1, 3, 14, 3, 2, 26}
sha224Oid = asn1.ObjectIdentifier{2, 16, 840, 1, 101, 3, 4, 2, 4}
Expand Down Expand Up @@ -415,7 +415,11 @@ type passphraseParams struct {
}

type keyData struct {
Version int `json:"version,omitempty"`
// Generation is a number used to differentiate between different key formats.
// i.e Gen1 keys are binary serialized and include a primary and an unlock key while
// Gen2 keys are ASN1 serialized and include a primary key and a unique key which is
// used to derive the unlock key.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The generations might also support different set of platform data fields?

Generation int `json:"generation,omitempty"`

PlatformName string `json:"platform_name"` // used to identify a PlatformKeyDataHandler

Expand Down Expand Up @@ -644,15 +648,15 @@ func (d *KeyData) openWithPassphrase(passphrase string, kdf KDF) (payload []byte

func (d *KeyData) platformKeyData() *PlatformKeyData {
return &PlatformKeyData{
Version: d.Version(),
Generation: d.Generation(),
EncodedHandle: d.data.PlatformHandle,
KDFAlg: crypto.Hash(d.data.KDFAlg),
AuthMode: d.AuthMode(),
}
}

func (d *KeyData) recoverKeysCommon(data []byte) (DiskUnlockKey, PrimaryKey, error) {
switch d.Version() {
switch d.Generation() {
case 1:
unlockKey, primaryKey, err := unmarshalV1KeyPayload(data)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why V1? as Version here is 0? also we have already key own versions, maybe we should use a different naming/termilogy than just version, internal for this level?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is related to #265 (comment) as well.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So I already changed it and replied to the comment above but summarizing it again to make sure I don't miss something. The following versions are possible:

  • version 0. Key data already used are missing a version field. Version() called in that keydata will return 1 and unmarshalV1KeyPayload will be called.
  • version 1 shouldn't exist (?), but even then the behavior would be the same as version 0 keys.
  • version 2 is the currently introduced format.

When I first saw V1, I assumed there was another V0. Isn't that the case? If not, maybe we could simply call old keys legacy keys?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe "generation"? So that we can refer to cross-platform keydata versions as generations and then each platform can refer to its internal version simply as "version".

i.e TPM platform's key versions 0-2 use cross-platform keys of generation 1 (or 0, this is still a bit confusing) while TPM platform's version 3 keys use generation 2 cross-platform keys.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"generation" would work for me

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, generation is ok with me as well.

if err != nil {
Expand All @@ -669,19 +673,19 @@ func (d *KeyData) recoverKeysCommon(data []byte) (DiskUnlockKey, PrimaryKey, err
}
return pk.unlockKey(crypto.Hash(d.data.KDFAlg)), pk.Primary, nil
default:
return nil, nil, fmt.Errorf("invalid keydata version %d", d.Version())
return nil, nil, fmt.Errorf("invalid keydata generation %d", d.Generation())
}
}

// Version returns this key data's version. Since the version field didn't exist
// for key data versions < 2, we fake the version returned to 1.
func (d *KeyData) Version() int {
switch d.data.Version {
// Generation returns this keydata's generation. Since the generation field didn't exist
// for older keydata with generation < 2, we fake the generation returned to 1.
func (d *KeyData) Generation() int {
switch d.data.Generation {
case 0:
// This field was missing in v1
// This field was missing in gen1
return 1
default:
return d.data.Version
return d.data.Generation
}
}

Expand Down Expand Up @@ -931,7 +935,7 @@ func NewKeyData(params *KeyParams) (*KeyData, error) {

kd := &KeyData{
data: keyData{
Version: keyDataVersion,
Generation: keyDataGeneration,
PlatformName: params.PlatformName,
PlatformHandle: json.RawMessage(encodedHandle),
KDFAlg: hashAlg(params.KDFAlg),
Expand Down
16 changes: 8 additions & 8 deletions keydata_legacy_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,11 +49,11 @@ func (s *keyDataLegacyTestBase) mockProtectKeys(c *C, key DiskUnlockKey, auxKey
c.Assert(err, IsNil)

handle := mockPlatformKeyDataHandle{
Key: k[:32],
IV: k[32:],
ExpectedVersion: 1,
ExpectedKDFAlg: kdfAlg,
ExpectedAuthMode: AuthModeNone,
Key: k[:32],
IV: k[32:],
ExpectedGeneration: 1,
ExpectedKDFAlg: kdfAlg,
ExpectedAuthMode: AuthModeNone,
}

h := hmac.New(func() hash.Hash { return kdfAlg.New() }, handle.Key)
Expand Down Expand Up @@ -142,7 +142,7 @@ func (s *keyDataLegacySuite) TestRecoverKeys(c *C) {
key, auxKey := s.newKeyDataKeys(c, 32, 32)
protected := s.mockProtectKeys(c, key, auxKey, crypto.SHA256, crypto.SHA256)

restore := MockKeyDataVersion(0)
restore := MockKeyDataGeneration(0)
defer restore()
keyData, err := NewKeyData(protected)

Expand All @@ -159,7 +159,7 @@ func (s *keyDataLegacySuite) TestRecoverKeysUnrecognizedPlatform(c *C) {

protected.PlatformName = "foo"

restore := MockKeyDataVersion(0)
restore := MockKeyDataGeneration(0)
defer restore()
keyData, err := NewKeyData(protected)

Expand All @@ -176,7 +176,7 @@ func (s *keyDataLegacySuite) TestRecoverKeysInvalidData(c *C) {

protected.Handle = []byte("\"\"")

restore := MockKeyDataVersion(0)
restore := MockKeyDataGeneration(0)
defer restore()
keyData, err := NewKeyData(protected)

Expand Down
50 changes: 25 additions & 25 deletions keydata_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,12 +50,12 @@
)

type mockPlatformKeyDataHandle struct {
Key []byte `json:"key"`
IV []byte `json:"iv"`
AuthKeyHMAC []byte `json:"auth-key-hmac"`
ExpectedVersion int `json:"exp-version"`
ExpectedKDFAlg crypto.Hash `json:"exp-kdf_alg"`
ExpectedAuthMode AuthMode `json:"exp-auth-mode"`
Key []byte `json:"key"`
IV []byte `json:"iv"`
AuthKeyHMAC []byte `json:"auth-key-hmac"`
ExpectedGeneration int `json:"exp-generation"`
ExpectedKDFAlg crypto.Hash `json:"exp-kdf_alg"`
ExpectedAuthMode AuthMode `json:"exp-auth-mode"`
}

const (
Expand Down Expand Up @@ -86,11 +86,11 @@
return nil, &PlatformHandlerError{Type: PlatformHandlerErrorInvalidData, Err: xerrors.Errorf("JSON decode error: %w", err)}
}

if data.Version != handle.ExpectedVersion {
return nil, &PlatformHandlerError{Type: PlatformHandlerErrorInvalidData, Err: errors.New("unexpected version")}
if data.Generation != handle.ExpectedGeneration {
return nil, &PlatformHandlerError{Type: PlatformHandlerErrorInvalidData, Err: errors.New("unexpected generation")}
}

if data.Version > 1 {
if data.Generation > 1 {
if data.KDFAlg != handle.ExpectedKDFAlg {
return nil, &PlatformHandlerError{Type: PlatformHandlerErrorInvalidData, Err: errors.New("unexpected KDFAlg")}
}
Expand Down Expand Up @@ -288,11 +288,11 @@
c.Assert(err, IsNil)

handle := mockPlatformKeyDataHandle{
Key: k[:32],
IV: k[32:],
ExpectedVersion: KeyDataVersion,
ExpectedKDFAlg: kdfAlg,
ExpectedAuthMode: AuthModeNone,
Key: k[:32],
IV: k[32:],
ExpectedGeneration: KeyDataGeneration,
ExpectedKDFAlg: kdfAlg,
ExpectedAuthMode: AuthModeNone,
}

h := hmac.New(func() hash.Hash { return crypto.SHA256.New() }, handle.Key)
Expand Down Expand Up @@ -322,7 +322,7 @@
c.Assert(ok, testutil.IsTrue)

expectedHandle.ExpectedAuthMode = AuthModePassphrase
expectedHandle.ExpectedVersion = KeyDataVersion
expectedHandle.ExpectedGeneration = KeyDataGeneration
expectedHandle.ExpectedKDFAlg = KDFAlg

if kdfOptions == nil {
Expand Down Expand Up @@ -357,9 +357,9 @@
_, ok = j["kdf_alg"].(string)
c.Check(ok, testutil.IsTrue)

version, ok := j["version"].(float64)
generation, ok := j["generation"].(float64)
c.Check(ok, testutil.IsTrue)
c.Check(version, Equals, float64(2))
c.Check(generation, Equals, float64(2))

m, ok := j["authorized_snap_models"].(map[string]interface{})
c.Assert(ok, testutil.IsTrue)
Expand Down Expand Up @@ -687,11 +687,11 @@
}

func (s *keyDataSuite) TestUnmarshalPlatformHandle(c *C) {
primaryKey := s.newPrimaryKey(c, 32)

Check failure on line 690 in keydata_test.go

View workflow job for this annotation

GitHub Actions / tests (1.18) 

s.newKeyDataKeys undefined (type *keyDataSuite has no field or method newKeyDataKeys)

Check failure on line 690 in keydata_test.go

View workflow job for this annotation

GitHub Actions / tests (stable) 

s.newKeyDataKeys undefined (type *keyDataSuite has no field or method newKeyDataKeys)
protected, _ := s.mockProtectKeys(c, primaryKey, crypto.SHA256, crypto.SHA256)

Check failure on line 691 in keydata_test.go

View workflow job for this annotation

GitHub Actions / tests (1.18) 

assignment mismatch: 1 variable but s.mockProtectKeys returns 2 values

Check failure on line 691 in keydata_test.go

View workflow job for this annotation

GitHub Actions / tests (stable) 

assignment mismatch: 1 variable but s.mockProtectKeys returns 2 values
keyData, err := NewKeyData(protected)
c.Assert(err, IsNil)

Check failure on line 694 in keydata_test.go

View workflow job for this annotation

GitHub Actions / tests (1.18) 

undefined: mockPlatformName

Check failure on line 694 in keydata_test.go

View workflow job for this annotation

GitHub Actions / tests (stable) 

undefined: mockPlatformName
var handle *mockPlatformKeyDataHandle
c.Check(keyData.UnmarshalPlatformHandle(&handle), IsNil)

Expand Down Expand Up @@ -807,14 +807,14 @@

j := []byte(
`{` +
`"version":2,` +
`"generation":2,` +
`"platform_name":"mock",` +
`"platform_handle":` +
`{` +
`"key":"GtaI3cZX9H3Ig1YxSCPTxLshteV0AXK2pFgQuE5NRIQ=",` +
`"iv":"0VUZD/yYi6PfRzdPB0a1GA==",` +
`"auth-key-hmac":"7/AmPJvhwHNY/E1a3oEoqF5xjmt5FBr9YTppQvESUSY=",` +
`"exp-version":2,` +
`"exp-generation":2,` +
`"exp-kdf_alg":5,` +
`"exp-auth-mode":1},` +
`"kdf_alg":"sha256",` +
Expand Down Expand Up @@ -910,7 +910,7 @@
// fails when the platform handler doesn't have passphrase support.
j := []byte(
`{` +
`"version":2,` +
`"generation":2,` +
`"platform_name":"mock",` +
`"platform_handle":` +
`{` +
Expand Down Expand Up @@ -954,7 +954,7 @@
// Test that changing passphrase on a key data without a passphrase set fails.
j := []byte(
`{` +
`"version":2,` +
`"generation":2,` +
`"platform_name":"mock",` +
`"platform_handle":` +
`{` +
Expand Down Expand Up @@ -1435,14 +1435,14 @@
// Valid KeyData with passphrase "passphrase"
j := []byte(
`{` +
`"version":2,` +
`"generation":2,` +
`"platform_name":"mock",` +
`"platform_handle":` +
`{` +
`"key":"PNmzLCfVurOXSYAFaLAOdHuhBMo7fmrFS2RtNooe3fw=",` +
`"iv":"D84HW2UYyF6nOMyfEPMtiQ==",` +
`"auth-key-hmac":"EAMRNlNzn3Tz47uM9kLTgXBaM341G4D6W3f57PDc8xs=",` +
`"exp-version":2,` +
`"exp-generation":2,` +
`"exp-kdf_alg":5,` +
`"exp-auth-mode":1},` +
`"kdf_alg":"sha256",` +
Expand Down Expand Up @@ -1616,7 +1616,7 @@
`"key":"7AQQmeIwl5iv3V+yTszelcdF6MkJpKz+7EA0kKUJNEo=",` +
`"iv":"i88WWEI7WyJ1gXX5LGhRSg==",` +
`"auth-key-hmac":"WybrzR13ozdYwzyt4oyihIHSABZozpHyQSAn+NtQSkA=",` +
`"exp-version":1,` +
`"exp-generation":1,` +
`"exp-kdf_alg":0,` +
`"exp-auth-mode":0},` +
`"encrypted_payload":"eMeLrknRAi/dFBM607WPxFOCE1L9RZ4xxUs+Leodz78s/id7Eq+IHhZdOC/stXSNe+Gn/PWgPxcd0TfEPUs5TA350lo=",` +
Expand Down Expand Up @@ -1681,7 +1681,7 @@
`"key":"7AQQmeIwl5iv3V+yTszelcdF6MkJpKz+7EA0kKUJNEo=",`+
`"iv":"i88WWEI7WyJ1gXX5LGhRSg==",`+
`"auth-key-hmac":"WybrzR13ozdYwzyt4oyihIHSABZozpHyQSAn+NtQSkA=",`+
`"exp-version":1,`+
`"exp-generation":1,`+
`"exp-kdf_alg":0,`+
`"exp-auth-mode":0},`+
`"encrypted_payload":"eMeLrknRAi/dFBM607WPxFOCE1L9RZ4xxUs+Leodz78s/id7Eq+IHhZdOC/stXSNe+Gn/PWgPxcd0TfEPUs5TA350lo=",`+
Expand Down
2 changes: 1 addition & 1 deletion platform.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ func (e *PlatformHandlerError) Unwrap() error {
// PlatformKeyData represents the data exchanged between this package and
// platform implementations via the PlatformKeyDataHandler.
type PlatformKeyData struct {
Version int
Generation int
EncodedHandle []byte // The JSON encoded platform handle
KDFAlg crypto.Hash

Expand Down
Loading