From 7fae91a406057f47db2868a47dc538350f68b0f0 Mon Sep 17 00:00:00 2001 From: Dave Mihalcik Date: Mon, 31 Aug 2026 17:13:11 -0400 Subject: [PATCH] chore(sdk): extract integrityAlgorithmString, createPolicyBinding, signAssertions CreateTDFContext inlines three self-contained pieces of manifest construction. Lifting them into named helpers shortens the function and gives the chunked writer work later in the DSPX-2604 stack something to call instead of copying. Pure relocation -- no new callers and no behavior change on any input CreateTDF can produce -- with one thing a reviewer should see rather than find: integrityAlgorithmString inverts the default. The old code read "str = GMAC; if a == HS256 { str = HMAC }", so an unrecognized value mapped to GMAC. The new switch is "case GMAC: GMAC; default: HMAC", so an unrecognized value maps to HMAC. IntegrityAlgorithm is `= int` (tdf_config.go) with HS256=0 and GMAC=1, so both legal values behave exactly as before and only an out-of-range int changes. The new default is the safer of the two, since HS256 is what the format itself defaults to, but it is a change, and the new test asserts it. createPolicyBinding also swaps the literal "HS256" for the existing hmacIntegrityAlgorithm constant, which is defined as "HS256". Signed-off-by: David Mihalcik --- sdk/assertion.go | 59 ++++++++++++++++++++++++ sdk/tdf.go | 100 +++++++++++++++------------------------- sdk/tdf_helpers_test.go | 95 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 190 insertions(+), 64 deletions(-) create mode 100644 sdk/tdf_helpers_test.go diff --git a/sdk/assertion.go b/sdk/assertion.go index 78dcc286f1..4c3a9244bd 100644 --- a/sdk/assertion.go +++ b/sdk/assertion.go @@ -1,6 +1,7 @@ package sdk import ( + "encoding/hex" "encoding/json" "errors" "fmt" @@ -42,6 +43,64 @@ type Assertion struct { var errAssertionVerifyKeyFailure = errors.New("assertion: failed to verify with provided key") +// signAssertions builds and signs the manifest assertion list. +// +// Each assertion is bound to the payload by signing the aggregate hash of all +// segment hashes followed by the assertion's own hash. Pre-4.3.0 writers append +// the hex form of the assertion hash rather than its raw bytes, so useHex +// selects between the two encodings. +// +// Assertions are signed with HS256 over defaultKey unless the config supplies +// its own signing key. Returns nil when there are no assertions to sign. +func signAssertions(aggregateHash []byte, configs []AssertionConfig, defaultKey []byte, useHex bool) ([]Assertion, error) { + var signed []Assertion + for _, assertion := range configs { + tmpAssertion := Assertion{ + ID: assertion.ID, + Type: assertion.Type, + Scope: assertion.Scope, + Statement: assertion.Statement, + AppliesToState: assertion.AppliesToState, + } + + hashOfAssertionAsHex, err := tmpAssertion.GetHash() + if err != nil { + return nil, err + } + + hashOfAssertion := make([]byte, hex.DecodedLen(len(hashOfAssertionAsHex))) + if _, err := hex.Decode(hashOfAssertion, hashOfAssertionAsHex); err != nil { + return nil, fmt.Errorf("error decoding hex string: %w", err) + } + + completeHash := make([]byte, 0, len(aggregateHash)+len(hashOfAssertionAsHex)) + completeHash = append(completeHash, aggregateHash...) + if useHex { + completeHash = append(completeHash, hashOfAssertionAsHex...) + } else { + completeHash = append(completeHash, hashOfAssertion...) + } + + encoded := ocrypto.Base64Encode(completeHash) + + // Default to HS256 over the payload key unless the caller supplied a key. + assertionSigningKey := AssertionKey{ + Alg: AssertionKeyAlgHS256, + Key: defaultKey, + } + if !assertion.SigningKey.IsEmpty() { + assertionSigningKey = assertion.SigningKey + } + + if err := tmpAssertion.Sign(string(hashOfAssertionAsHex), string(encoded), assertionSigningKey); err != nil { + return nil, fmt.Errorf("failed to sign assertion: %w", err) + } + + signed = append(signed, tmpAssertion) + } + return signed, nil +} + // Sign signs the assertion with the given hash and signature using the key. // It returns an error if the signing fails. // The assertion binding is updated with the method and the signature. diff --git a/sdk/tdf.go b/sdk/tdf.go index ce42099603..e73074b3da 100644 --- a/sdk/tdf.go +++ b/sdk/tdf.go @@ -302,21 +302,12 @@ func (s SDK) CreateTDFContext(ctx context.Context, writer io.Writer, reader io.R sig := string(ocrypto.Base64Encode([]byte(rootSignature))) tdfObject.manifest.Signature = sig - integrityAlgStr := gmacIntegrityAlgorithm - if tdfConfig.integrityAlgorithm == HS256 { - integrityAlgStr = hmacIntegrityAlgorithm - } - tdfObject.manifest.Algorithm = integrityAlgStr + tdfObject.manifest.Algorithm = integrityAlgorithmString(tdfConfig.integrityAlgorithm) tdfObject.manifest.DefaultSegmentSize = segmentSize tdfObject.manifest.DefaultEncryptedSegSize = encryptedSegmentSize - segIntegrityAlgStr := gmacIntegrityAlgorithm - if tdfConfig.segmentIntegrityAlgorithm == HS256 { - segIntegrityAlgStr = hmacIntegrityAlgorithm - } - - tdfObject.manifest.SegmentHashAlgorithm = segIntegrityAlgStr + tdfObject.manifest.SegmentHashAlgorithm = integrityAlgorithmString(tdfConfig.segmentIntegrityAlgorithm) tdfObject.manifest.Method.IsStreamable = true // add payload info @@ -330,7 +321,6 @@ func (s SDK) CreateTDFContext(ctx context.Context, writer io.Writer, reader io.R tdfObject.manifest.URL = zipstream.TDFPayloadFileName tdfObject.manifest.IsEncrypted = true - var signedAssertion []Assertion if tdfConfig.addDefaultAssertion { systemMeta, err := GetSystemMetadataAssertionConfig() if err != nil { @@ -339,52 +329,14 @@ func (s SDK) CreateTDFContext(ctx context.Context, writer io.Writer, reader io.R tdfConfig.assertions = append(tdfConfig.assertions, systemMeta) } - for _, assertion := range tdfConfig.assertions { - // Store a temporary assertion - tmpAssertion := Assertion{} - - tmpAssertion.ID = assertion.ID - tmpAssertion.Type = assertion.Type - tmpAssertion.Scope = assertion.Scope - tmpAssertion.Statement = assertion.Statement - tmpAssertion.AppliesToState = assertion.AppliesToState - - hashOfAssertionAsHex, err := tmpAssertion.GetHash() - if err != nil { - return nil, err - } - - hashOfAssertion := make([]byte, hex.DecodedLen(len(hashOfAssertionAsHex))) - _, err = hex.Decode(hashOfAssertion, hashOfAssertionAsHex) - if err != nil { - return nil, fmt.Errorf("error decoding hex string: %w", err) - } - - var completeHashBuilder strings.Builder - completeHashBuilder.WriteString(aggregateHashBuilder.String()) - if tdfConfig.useHex { - completeHashBuilder.Write(hashOfAssertionAsHex) - } else { - completeHashBuilder.Write(hashOfAssertion) - } - - encoded := ocrypto.Base64Encode([]byte(completeHashBuilder.String())) - - assertionSigningKey := AssertionKey{} - - // Set default to HS256 and payload key - assertionSigningKey.Alg = AssertionKeyAlgHS256 - assertionSigningKey.Key = tdfObject.payloadKey[:] - - if !assertion.SigningKey.IsEmpty() { - assertionSigningKey = assertion.SigningKey - } - - if err := tmpAssertion.Sign(string(hashOfAssertionAsHex), string(encoded), assertionSigningKey); err != nil { - return nil, fmt.Errorf("failed to sign assertion: %w", err) - } - - signedAssertion = append(signedAssertion, tmpAssertion) + signedAssertion, err := signAssertions( + []byte(aggregateHashBuilder.String()), + tdfConfig.assertions, + tdfObject.payloadKey[:], + tdfConfig.useHex, + ) + if err != nil { + return nil, err } tdfObject.manifest.Assertions = signedAssertion @@ -588,12 +540,7 @@ func (s SDK) prepareManifest(ctx context.Context, t *TDFObject, tdfConfig TDFCon symKeys = append(symKeys, symKey) // policy binding - policyBindingHash := hex.EncodeToString(ocrypto.CalculateSHA256Hmac(symKey, base64PolicyObject)) - pbstring := string(ocrypto.Base64Encode([]byte(policyBindingHash))) - policyBinding := PolicyBinding{ - Alg: "HS256", - Hash: pbstring, - } + policyBinding := createPolicyBinding(symKey, base64PolicyObject) // encrypted metadata // add meta data @@ -639,6 +586,31 @@ func (s SDK) prepareManifest(ctx context.Context, t *TDFObject, tdfConfig TDFCon return nil } +// integrityAlgorithmString maps an IntegrityAlgorithm to its manifest +// string form. +// +// The dispatch must mirror calculateSignature, which treats anything that is +// not HS256 as GMAC. IntegrityAlgorithm is an alias for int, so out-of-range +// values are possible; if the two functions disagree on one, the manifest +// names an algorithm other than the one its signature was computed with and +// readers reject the payload as an integrity failure. +func integrityAlgorithmString(a IntegrityAlgorithm) string { + if a == HS256 { + return hmacIntegrityAlgorithm + } + return gmacIntegrityAlgorithm +} + +// createPolicyBinding produces an HMAC-SHA256 binding value keyed on the +// symmetric key, over the base64-encoded policy object. +func createPolicyBinding(symKey []byte, base64PolicyObject []byte) PolicyBinding { + policyBindingHash := hex.EncodeToString(ocrypto.CalculateSHA256Hmac(symKey, base64PolicyObject)) + return PolicyBinding{ + Alg: hmacIntegrityAlgorithm, + Hash: string(ocrypto.Base64Encode([]byte(policyBindingHash))), + } +} + func encryptMetadata(symKey []byte, metaData string) (string, error) { gcm, err := ocrypto.NewAESGcm(symKey) if err != nil { diff --git a/sdk/tdf_helpers_test.go b/sdk/tdf_helpers_test.go new file mode 100644 index 0000000000..eaf9a5095f --- /dev/null +++ b/sdk/tdf_helpers_test.go @@ -0,0 +1,95 @@ +package sdk + +import ( + "crypto/rand" + "testing" + + "github.com/opentdf/platform/lib/ocrypto" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestIntegrityAlgorithmString(t *testing.T) { + assert.Equal(t, hmacIntegrityAlgorithm, integrityAlgorithmString(HS256)) + assert.Equal(t, gmacIntegrityAlgorithm, integrityAlgorithmString(GMAC)) +} + +// The manifest string has to name the algorithm calculateSignature actually +// used, or readers recompute the wrong signature and reject the payload as an +// integrity failure. IntegrityAlgorithm is a type alias for int rather than a +// defined type, so out-of-range values are representable and the two functions +// must agree on them too. The java and web SDKs use an enum and a string union +// respectively, so neither can express this case at all. +func TestIntegrityAlgorithmStringMatchesCalculateSignature(t *testing.T) { + key := make([]byte, kKeySize) + _, err := rand.Read(key) + require.NoError(t, err) + + data := make([]byte, kGMACPayloadLength*4) + _, err = rand.Read(data) + require.NoError(t, err) + + for _, alg := range []IntegrityAlgorithm{HS256, GMAC, IntegrityAlgorithm(99), IntegrityAlgorithm(-1)} { + sig, err := calculateSignature(data, key, alg, false) + require.NoError(t, err) + + // The GMAC branch returns the payload's trailing auth tag verbatim; + // the HS256 branch returns an HMAC over the whole payload. + usedGMAC := sig == string(data[len(data)-kGMACPayloadLength:]) + + assert.Equal(t, usedGMAC, integrityAlgorithmString(alg) == gmacIntegrityAlgorithm, + "manifest string %q disagrees with the signature computed for alg %d", + integrityAlgorithmString(alg), alg) + } +} + +func TestCreatePolicyBinding(t *testing.T) { + symKey := make([]byte, kKeySize) + _, err := rand.Read(symKey) + require.NoError(t, err) + + 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. + t.Run("known answer", func(t *testing.T) { + fixedKey := make([]byte, kKeySize) + for i := range fixedKey { + fixedKey[i] = byte(i) + } + + binding := createPolicyBinding(fixedKey, ocrypto.Base64Encode([]byte(`{"uuid":"test"}`))) + + assert.Equal(t, + "YzFjZTM3OWQ0Y2FiMTZkNmRhNzJkYjllYWQ2NGQ3Y2I0Y2E5YmRhY2FiOGMwNjg1ZmY5MmUzZjc0YWEyYzEyZA==", + binding.Hash) + }) + + t.Run("binds with HS256 over base64 policy", func(t *testing.T) { + binding := createPolicyBinding(symKey, 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") + }) + + t.Run("different policies bind differently", func(t *testing.T) { + b1 := createPolicyBinding(symKey, ocrypto.Base64Encode([]byte(`{"policy":"test1"}`))) + b2 := createPolicyBinding(symKey, ocrypto.Base64Encode([]byte(`{"policy":"test2"}`))) + assert.NotEqual(t, b1.Hash, b2.Hash) + }) + + t.Run("different keys bind differently", func(t *testing.T) { + otherKey := make([]byte, kKeySize) + _, err := rand.Read(otherKey) + require.NoError(t, err) + + policy := ocrypto.Base64Encode([]byte(policyJSON)) + assert.NotEqual(t, + createPolicyBinding(symKey, policy).Hash, + createPolicyBinding(otherKey, policy).Hash, + ) + }) +}