diff --git a/sdk/chunked_test.go b/sdk/chunked_test.go index 76ed0118aa..5080d82621 100644 --- a/sdk/chunked_test.go +++ b/sdk/chunked_test.go @@ -6,6 +6,7 @@ import ( "context" "crypto/ecdsa" "crypto/rand" + "crypto/sha256" "crypto/subtle" "crypto/x509" "encoding/hex" @@ -650,10 +651,23 @@ func verifyChunkedPolicyBinding(binding *kaspb.PolicyBinding, share []byte, base if binding.GetAlgorithm() != hmacIntegrityAlgorithm { return fmt.Errorf("unsupported policy binding algorithm %q", binding.GetAlgorithm()) } - want := string(ocrypto.Base64Encode([]byte( - hex.EncodeToString(ocrypto.CalculateSHA256Hmac(share, []byte(base64Policy))), - ))) - if subtle.ConstantTimeCompare([]byte(want), []byte(binding.GetHash())) != 1 { + // Mirror the real KAS (service/kas/access.decodePolicyBinding): decode to raw + // HMAC bytes, accepting both the spec >= 4.3.0 base64(hmac) form and the + // pre-4.3.0 base64(hex(hmac)) form. The two are unambiguous by length, so a + // KAS dual-accepts without needing a version signal. A fake that only took + // hex would pass while every real KAS rejected the output, and vice versa. + got, err := ocrypto.Base64Decode([]byte(binding.GetHash())) + if err != nil { + return fmt.Errorf("policy binding is not base64: %w", err) + } + if len(got) == hex.EncodedLen(sha256.Size) { + dehexed := make([]byte, hex.DecodedLen(len(got))) + if _, err := hex.Decode(dehexed, got); err == nil { + got = dehexed + } + } + want := ocrypto.CalculateSHA256Hmac(share, []byte(base64Policy)) + if subtle.ConstantTimeCompare(want, got) != 1 { return errors.New("policy binding does not match the policy") } return nil @@ -746,7 +760,7 @@ func TestChunkedECKeyAccess(t *testing.T) { }}, }} - kaos, err := buildKeyAccessObjects(shares, `{"uuid":"test"}`, "") + kaos, err := buildKeyAccessObjects(shares, `{"uuid":"test"}`, "", false) require.NoError(t, err) require.Len(t, kaos, 1) @@ -812,7 +826,7 @@ func TestChunkedKeyAccessRejectsShareWithNoKAS(t *testing.T) { {id: "orphaned", data: dek}, } - _, err = buildKeyAccessObjects(shares, `{"uuid":"test"}`, "") + _, err = buildKeyAccessObjects(shares, `{"uuid":"test"}`, "", false) require.Error(t, err, "a share with no KAS to unwrap it makes the DEK unrecoverable") assert.Contains(t, err.Error(), "orphaned", "the error must name the split that cannot be recovered") } @@ -843,6 +857,20 @@ func TestChunkedFinalizeManifestIsIndependent(t *testing.T) { assert.Equal(t, wantHash, got.Segments[0].Hash) } +// chunkedPolicyBindingBytes base64-decodes the first KAO's policy binding +// hash. The length of the result is what distinguishes the two wire forms: +// sha256.Size for the 4.3.0 raw HMAC, twice that for the pre-4.3.0 hex. +func chunkedPolicyBindingBytes(t *testing.T, m *Manifest) []byte { + t.Helper() + require.NotNil(t, m) + require.NotEmpty(t, m.KeyAccessObjs) + binding, ok := m.KeyAccessObjs[0].PolicyBinding.(PolicyBinding) + require.True(t, ok, "policy binding should be a PolicyBinding, got %T", m.KeyAccessObjs[0].PolicyBinding) + decoded, err := ocrypto.Base64Decode([]byte(binding.Hash)) + require.NoError(t, err, "policy binding hash should be base64") + return decoded +} + // TestChunkedLegacyTargetMode verifies that a pre-4.3.0 target mode // produces the doubly-encoded (hex-then-base64) signatures that legacy // readers require, and that the mainline reader -- which infers legacy @@ -874,6 +902,10 @@ func TestChunkedLegacyTargetMode(t *testing.T) { assert.Lenf(t, segSig, 2*kGMACPayloadLength, "segment %d hash must be hex-encoded before base64", i) } + // The policy binding follows the same threshold as the signatures above. + assert.Len(t, chunkedPolicyBindingBytes(t, fin.Manifest), hex.EncodedLen(sha256.Size), + "legacy policy binding must be hex-encoded before base64") + tdfBytes := bytes.Join([][]byte{body, fin.Data}, nil) reader, err := s.LoadTDF(bytes.NewReader(tdfBytes), WithKasAllowlist([]string{kasBundle.url}), @@ -902,6 +934,10 @@ func TestChunkedCurrentTargetMode(t *testing.T) { require.NoError(t, err) assert.Len(t, rootSig, 32, "root signature must be the raw HMAC, not hex") + // The policy binding follows the same threshold as the signature above. + assert.Len(t, chunkedPolicyBindingBytes(t, fin.Manifest), sha256.Size, + "policy binding must be the raw HMAC, not hex") + tdfBytes := bytes.Join([][]byte{body, fin.Data}, nil) reader, err := s.LoadTDF(bytes.NewReader(tdfBytes), WithKasAllowlist([]string{kasBundle.url}), diff --git a/sdk/chunked_writer.go b/sdk/chunked_writer.go index 8599ac0486..b8009de1db 100644 --- a/sdk/chunked_writer.go +++ b/sdk/chunked_writer.go @@ -403,9 +403,10 @@ type chunkedWriterConfig struct { // keyAccess is set. splitter KeySplitter - // useHex hex-encodes segment, root, and assertion signatures - // before base64, producing the doubly-encoded form that readers - // older than 4.3.0 require. Set by WithChunkedTargetMode. + // useHex hex-encodes segment, root, and assertion signatures and + // the policy binding before base64, producing the doubly-encoded + // form that readers older than 4.3.0 require. Set by + // WithChunkedTargetMode. useHex bool } @@ -605,7 +606,7 @@ func newChunkedWriter(cfg chunkedWriterConfig) (*chunkedWriter, error) { } keyAccess := cfg.keyAccess if keyAccess == nil { - keyAccess = splitterKeyAccess{splitter: cfg.splitter} + keyAccess = splitterKeyAccess{splitter: cfg.splitter, useHex: cfg.useHex} } maxEncryptions := cfg.maxEncryptions if maxEncryptions == 0 { diff --git a/sdk/experimental/tdf/keysplit_adapter_test.go b/sdk/experimental/tdf/keysplit_adapter_test.go index 0d9a6adda3..bc5e303129 100644 --- a/sdk/experimental/tdf/keysplit_adapter_test.go +++ b/sdk/experimental/tdf/keysplit_adapter_test.go @@ -3,7 +3,6 @@ package tdf import ( - "encoding/hex" "fmt" "testing" @@ -89,8 +88,7 @@ func TestXORSplitterAdapterRoundTrip(t *testing.T) { binding, ok := kao.PolicyBinding.(PolicyBinding) require.True(t, ok, "policy binding should be a PolicyBinding, got %T", kao.PolicyBinding) - expectedHash := ocrypto.CalculateSHA256Hmac(share, policyBytes) - expected := string(ocrypto.Base64Encode([]byte(hex.EncodeToString(expectedHash)))) + expected := string(ocrypto.Base64Encode(ocrypto.CalculateSHA256Hmac(share, policyBytes))) require.Equal(t, expected, binding.Hash, "recovered share for KAS %s must reproduce its own policy binding", kao.KasURL) if dek == nil { diff --git a/sdk/experimental/tdf/writer_test.go b/sdk/experimental/tdf/writer_test.go index 23da5cfcea..7c0818dc25 100644 --- a/sdk/experimental/tdf/writer_test.go +++ b/sdk/experimental/tdf/writer_test.go @@ -5,7 +5,6 @@ package tdf import ( "bytes" "crypto/rand" - "encoding/hex" "encoding/json" "io" "os" @@ -942,8 +941,7 @@ func hybridUnwrapForTest(t *testing.T, ktype ocrypto.KeyType, privatePEM string, require.NoError(t, err, "hybrid Decrypt") require.Len(t, dek, 32, "%s recovered DEK should be 32 bytes", ktype) - hash := hex.EncodeToString(ocrypto.CalculateSHA256Hmac(dek, []byte(manifest.Policy))) - expected := string(ocrypto.Base64Encode([]byte(hash))) + expected := string(ocrypto.Base64Encode(ocrypto.CalculateSHA256Hmac(dek, []byte(manifest.Policy)))) binding, ok := keyAccess.PolicyBinding.(PolicyBinding) require.True(t, ok, "policy binding should be a PolicyBinding, got %T", keyAccess.PolicyBinding) diff --git a/sdk/key_splitter.go b/sdk/key_splitter.go index 232e4802ff..ad8bf7bed2 100644 --- a/sdk/key_splitter.go +++ b/sdk/key_splitter.go @@ -354,6 +354,9 @@ func (r staticKeyAccess) resolve(_ context.Context, _ []byte, _ *chunkedFinalize type splitterKeyAccess struct { // splitter maps attributes plus the DEK onto KAS-addressed shares. splitter KeySplitter + + // useHex selects the pre-4.3.0 Base64(hex(HMAC)) policy binding. + useHex bool } func (r splitterKeyAccess) resolve(ctx context.Context, dek []byte, cfg *chunkedFinalizeConfig) (string, []KeyAccess, error) { @@ -410,14 +413,14 @@ func (r splitterKeyAccess) resolve(ctx context.Context, dek []byte, cfg *chunked for _, v := range cfg.attributes { fqns = append(fqns, v.GetFqn()) } - return resolvePolicyAndKeyAccess(fqns, shares, cfg.encryptedMetadata) + return resolvePolicyAndKeyAccess(fqns, shares, cfg.encryptedMetadata, r.useHex) } // resolvePolicyAndKeyAccess builds the policy document the DEK is bound to and wraps // every share to its KAS targets, returning the two manifest fields that bind a DEK to // policy. Shared by SDK.CreateTDF's KAO template path and the chunked writer's // KeySplitter path, so both emit byte-identical policy for the same attributes. -func resolvePolicyAndKeyAccess(fqns []string, shares []splitShare, metadata string) (string, []KeyAccess, error) { +func resolvePolicyAndKeyAccess(fqns []string, shares []splitShare, metadata string, useHex bool) (string, []KeyAccess, error) { policyObj, err := createPolicyObjectFromFQNs(fqns) if err != nil { return "", nil, fmt.Errorf("fail to create policy object:%w", err) @@ -428,7 +431,7 @@ func resolvePolicyAndKeyAccess(fqns []string, shares []splitShare, metadata stri } base64Policy := string(ocrypto.Base64Encode(policyObjectAsStr)) - kaos, err := buildKeyAccessObjects(shares, base64Policy, metadata) + kaos, err := buildKeyAccessObjects(shares, base64Policy, metadata, useHex) if err != nil { return "", nil, err } @@ -437,7 +440,7 @@ func resolvePolicyAndKeyAccess(fqns []string, shares []splitShare, metadata stri // buildKeyAccessObjects wraps every share to each of its KAS targets, // emitting the manifest's keyAccess array in share order. -func buildKeyAccessObjects(shares []splitShare, base64Policy, metadata string) ([]KeyAccess, error) { +func buildKeyAccessObjects(shares []splitShare, base64Policy, metadata string, useHex bool) ([]KeyAccess, error) { var kaos []KeyAccess for _, share := range shares { // A share with no KAS target contributes no key access object, @@ -455,7 +458,7 @@ func buildKeyAccessObjects(shares []splitShare, base64Policy, metadata string) ( // Policy binding and metadata are keyed on the split share, not // on the KAS, so compute them once per share rather than once // per KAS URL in an OR-group. - policyBinding := createPolicyBinding(share.data, base64Policy) + policyBinding := createPolicyBinding(share.data, base64Policy, useHex) var encryptedMetadata string if metadata != "" { diff --git a/sdk/tdf.go b/sdk/tdf.go index 5d2003f585..4a1606e2fe 100644 --- a/sdk/tdf.go +++ b/sdk/tdf.go @@ -583,7 +583,7 @@ func (s SDK) resolveKeyAccess(ctx context.Context, tdfConfig *TDFConfig, dek []b for _, attribute := range tdfConfig.attributes { fqns = append(fqns, attribute.String()) } - return resolvePolicyAndKeyAccess(fqns, shares, tdfConfig.metaData) + return resolvePolicyAndKeyAccess(fqns, shares, tdfConfig.metaData, tdfConfig.useHex) } // templateSplitShares groups the KAO template by split ID and divides the DEK across @@ -654,11 +654,18 @@ func manifestSegmentIntegrityAlg(alg string) SegmentIntegrityAlg { // createPolicyBinding binds a key split to the policy it unlocks, keyed on the split // itself so that a KAS can verify the policy it is asked to enforce is the one the // creator bound. -func createPolicyBinding(symKey []byte, base64Policy string) PolicyBinding { - policyBindingHash := hex.EncodeToString(ocrypto.CalculateSHA256Hmac(symKey, []byte(base64Policy))) +// +// Spec (>= 4.3.0) requires Base64(HMAC); pre-4.3.0 TDFs used Base64(hex(HMAC)). +// useHex tracks the same threshold as segment/root signatures via WithTargetMode, +// so the binding stays consistent with the rest of the manifest's integrity values. +func createPolicyBinding(symKey []byte, base64Policy string, useHex bool) PolicyBinding { + hash := ocrypto.CalculateSHA256Hmac(symKey, []byte(base64Policy)) + if useHex { + hash = []byte(hex.EncodeToString(hash)) + } return PolicyBinding{ Alg: hmacIntegrityAlgorithm, - Hash: string(ocrypto.Base64Encode([]byte(policyBindingHash))), + Hash: string(ocrypto.Base64Encode(hash)), } } diff --git a/sdk/tdf_helpers_test.go b/sdk/tdf_helpers_test.go index cdcd090b17..eafe20cf97 100644 --- a/sdk/tdf_helpers_test.go +++ b/sdk/tdf_helpers_test.go @@ -2,6 +2,7 @@ package sdk import ( "crypto/rand" + "crypto/sha256" "encoding/hex" "testing" @@ -123,34 +124,49 @@ func TestCreatePolicyBinding(t *testing.T) { policyJSON := `{"uuid":"test","body":{"dataAttributes":[{"attribute":"test"}],"dissem":[]}}` - // The wire format is base64(hex(hmac)), and KAS decodes in that order. The - // hex layer is easy to drop in a rewrite: every property below still holds - // without it, but every KAS would reject the result. Pin it to a vector. + // Two wire formats, selected by useHex (see WithTargetMode). Spec >= 4.3.0 is + // base64(hmac); pre-4.3.0 is base64(hex(hmac)). Either layer is easy to drop or + // re-add in a rewrite -- every property below still holds, but the result would + // no longer match what KAS decodes for that target mode. Pin both to vectors. t.Run("known answer", func(t *testing.T) { fixedKey := make([]byte, kKeySize) for i := range fixedKey { fixedKey[i] = byte(i) } - binding := createPolicyBinding(fixedKey, string(ocrypto.Base64Encode([]byte(`{"uuid":"test"}`)))) - + policy := string(ocrypto.Base64Encode([]byte(`{"uuid":"test"}`))) + assert.Equal(t, + "wc43nUyrFtbactuerWTXy0ypvayrjAaF/5Lj90qiwS0=", + createPolicyBinding(fixedKey, policy, false).Hash, + "spec >= 4.3.0 binding should be base64(hmac)") assert.Equal(t, "YzFjZTM3OWQ0Y2FiMTZkNmRhNzJkYjllYWQ2NGQ3Y2I0Y2E5YmRhY2FiOGMwNjg1ZmY5MmUzZjc0YWEyYzEyZA==", - binding.Hash) + createPolicyBinding(fixedKey, policy, true).Hash, + "pre-4.3.0 binding should be base64(hex(hmac))") }) t.Run("binds with HS256 over base64 policy", func(t *testing.T) { - binding := createPolicyBinding(symKey, string(ocrypto.Base64Encode([]byte(policyJSON)))) - - assert.Equal(t, hmacIntegrityAlgorithm, binding.Alg) - require.NotEmpty(t, binding.Hash) - _, err := ocrypto.Base64Decode([]byte(binding.Hash)) - require.NoError(t, err, "hash should be base64") + for _, useHex := range []bool{false, true} { + binding := createPolicyBinding(symKey, string(ocrypto.Base64Encode([]byte(policyJSON))), useHex) + + assert.Equal(t, hmacIntegrityAlgorithm, binding.Alg) + require.NotEmpty(t, binding.Hash) + decoded, err := ocrypto.Base64Decode([]byte(binding.Hash)) + require.NoError(t, err, "hash should be base64") + + // The two encodings are distinguishable by length alone, which is how + // KAS dual-accepts them without a version signal. + if useHex { + assert.Len(t, decoded, hex.EncodedLen(sha256.Size)) + } else { + assert.Len(t, decoded, sha256.Size) + } + } }) t.Run("different policies bind differently", func(t *testing.T) { - b1 := createPolicyBinding(symKey, string(ocrypto.Base64Encode([]byte(`{"policy":"test1"}`)))) - b2 := createPolicyBinding(symKey, string(ocrypto.Base64Encode([]byte(`{"policy":"test2"}`)))) + b1 := createPolicyBinding(symKey, string(ocrypto.Base64Encode([]byte(`{"policy":"test1"}`))), false) + b2 := createPolicyBinding(symKey, string(ocrypto.Base64Encode([]byte(`{"policy":"test2"}`))), false) assert.NotEqual(t, b1.Hash, b2.Hash) }) @@ -161,8 +177,8 @@ func TestCreatePolicyBinding(t *testing.T) { policy := string(ocrypto.Base64Encode([]byte(policyJSON))) assert.NotEqual(t, - createPolicyBinding(symKey, policy).Hash, - createPolicyBinding(otherKey, policy).Hash, + createPolicyBinding(symKey, policy, false).Hash, + createPolicyBinding(otherKey, policy, false).Hash, ) }) } diff --git a/sdk/tdf_test.go b/sdk/tdf_test.go index 49ae7ad383..6cbc526517 100644 --- a/sdk/tdf_test.go +++ b/sdk/tdf_test.go @@ -534,6 +534,7 @@ func (s *TDFSuite) Test_SimpleTDF() { WithSessionKeyType(ocrypto.EC256Key), WithKasAllowlist([]string{s.kasTestURLLookup["https://d.kas/"]}), }, + expectedSize: 1970, }, { name: "target-mode-0", @@ -669,6 +670,35 @@ func (s *TDFSuite) Test_SimpleTDF() { s.Require().Error(err2) } + // check that the policy binding matches the same hex/raw scheme as the + // other signatures. Spec >= 4.3.0 emits Base64(HMAC); pre-4.3.0 emits + // Base64(hex(HMAC)). + s.Require().NotEmpty(r.Manifest().KeyAccessObjs) + pb, ok := r.Manifest().KeyAccessObjs[0].PolicyBinding.(map[string]any) + s.Require().True(ok, "expected PolicyBinding to deserialize as map") + pbHash, ok := pb["hash"].(string) + s.Require().True(ok, "expected PolicyBinding.hash to be a string") + decodedPB, err := ocrypto.Base64Decode([]byte(pbHash)) + s.Require().NoError(err) + rawPB := decodedPB + if config.useHex { + s.Len(decodedPB, hex.EncodedLen(sha256.Size), "legacy policy binding should be hex-encoded HMAC") + rawPB, err = hex.DecodeString(string(decodedPB)) + s.Require().NoError(err) + } else { + s.Len(decodedPB, sha256.Size, "spec-compliant policy binding should be raw HMAC bytes") + } + // Shape alone would pass on a binding keyed on the wrong thing, so + // pin the value too. The payload key is the KAO key only when the + // DEK was not split; with splits each KAO is keyed on its own share. + if len(r.Manifest().KeyAccessObjs) == 1 { + s.Equal( + ocrypto.CalculateSHA256Hmac(payloadKey, []byte(r.Manifest().Policy)), + rawPB, + "policy binding should be HMAC(payload key, base64 policy)", + ) + } + // check version is present if usehex is false if config.useHex { s.Empty(r.Manifest().TDFVersion) @@ -937,7 +967,7 @@ func (s *TDFSuite) Test_TDFWithAssertion() { }, verifiers: nil, disableAssertionVerification: false, - expectedSize: 2689, + expectedSize: 2656, }, { assertions: []AssertionConfig{ @@ -1000,7 +1030,7 @@ func (s *TDFSuite) Test_TDFWithAssertion() { DefaultKey: defaultKey, }, disableAssertionVerification: false, - expectedSize: 2689, + expectedSize: 2656, }, { assertions: []AssertionConfig{ @@ -1049,7 +1079,7 @@ func (s *TDFSuite) Test_TDFWithAssertion() { }, }, disableAssertionVerification: false, - expectedSize: 2988, + expectedSize: 2955, }, { assertions: []AssertionConfig{ @@ -1089,7 +1119,7 @@ func (s *TDFSuite) Test_TDFWithAssertion() { }, }, disableAssertionVerification: false, - expectedSize: 2689, + expectedSize: 2656, }, { assertions: []AssertionConfig{ @@ -1106,7 +1136,7 @@ func (s *TDFSuite) Test_TDFWithAssertion() { }, }, disableAssertionVerification: true, - expectedSize: 2180, + expectedSize: 2147, }, } { expectedTdfSize := test.expectedSize @@ -1343,7 +1373,7 @@ func (s *TDFSuite) Test_TDFWithAssertionNegativeTests() { SigningKey: defaultKey, }, }, - expectedSize: 2689, + expectedSize: 2656, }, { assertions: []AssertionConfig{ @@ -1391,7 +1421,7 @@ func (s *TDFSuite) Test_TDFWithAssertionNegativeTests() { }, }, }, - expectedSize: 2988, + expectedSize: 2955, }, { assertions: []AssertionConfig{ @@ -1425,7 +1455,7 @@ func (s *TDFSuite) Test_TDFWithAssertionNegativeTests() { verifiers: &AssertionVerificationKeys{ DefaultKey: defaultKey, }, - expectedSize: 2689, + expectedSize: 2656, }, } { expectedTdfSize := test.expectedSize @@ -2239,7 +2269,7 @@ func (s *TDFSuite) Test_KeySplit_SameKas_SameAlgorithm() { { n: "multiple-keys-same-kas-same-algorithm", fileSize: 5, - tdfFileSize: 2581, + tdfFileSize: 2445, checksum: "ed968e840d10d2d313a870bc131a4e2c311d7ad09bdf32b3418147221f51a6e2", }, } { @@ -2318,7 +2348,7 @@ func (s *TDFSuite) Test_KeySplits() { { n: "shared", fileSize: 5, - tdfFileSize: 2759, + tdfFileSize: 2635, checksum: "ed968e840d10d2d313a870bc131a4e2c311d7ad09bdf32b3418147221f51a6e2", splitPlan: []keySplitStep{ {KAS: s.kasTestURLLookup["https://a.kas/"], SplitID: "a"}, @@ -2329,7 +2359,7 @@ func (s *TDFSuite) Test_KeySplits() { { n: "split", fileSize: 5, - tdfFileSize: 2759, + tdfFileSize: 2635, checksum: "ed968e840d10d2d313a870bc131a4e2c311d7ad09bdf32b3418147221f51a6e2", splitPlan: []keySplitStep{ {KAS: s.kasTestURLLookup["https://a.kas/"], SplitID: "a"}, @@ -2340,7 +2370,7 @@ func (s *TDFSuite) Test_KeySplits() { { n: "mixture", fileSize: 5, - tdfFileSize: 3351, + tdfFileSize: 3191, checksum: "ed968e840d10d2d313a870bc131a4e2c311d7ad09bdf32b3418147221f51a6e2", splitPlan: []keySplitStep{ {KAS: s.kasTestURLLookup["https://a.kas/"], SplitID: "a"},