fix(sdk): reject GMAC root signatures (DSPX-4703)

examples/cmd/benchmark_experimental.go

@@ -66,7 +66,7 @@ func runExperimentalWriterBenchmark(_ *cobra.Command, _ []string) error {
 	}
 
 	attrs = append(attrs, &policy.Value{Fqn: testAttr, KasKeys: []*policy.SimpleKasKey{simpleyKey}, Attribute: &policy.Attribute{Namespace: &policy.Namespace{Name: "example.com"}, Fqn: testAttr}})
-	writer, err := tdf.NewWriter(context.Background(), tdf.WithDefaultKASForWriter(simpleyKey), tdf.WithInitialAttributes(attrs), tdf.WithSegmentIntegrityAlgorithm(tdf.HS256))
+	writer, err := tdf.NewWriter(context.Background(), tdf.WithDefaultKASForWriter(simpleyKey), tdf.WithInitialAttributes(attrs), tdf.WithSegmentIntegrityAlgorithm(tdf.SegmentHS256))
 	if err != nil {
 		return fmt.Errorf("failed to create writer: %w", err)
 	}

sdk/experimental/tdf/doc.go

@@ -97,7 +97,8 @@
 //
 //   - Custom cryptographic assertions with JWT-based integrity
 //   - Encrypted metadata storage within key access objects
-//   - Multiple integrity algorithm support (HS256, GMAC)
+//   - Segment integrity algorithm support (HS256, GMAC); the root signature is
+//     HS256 only
 //   - ZIP64 format support for large files
 //   - Memory-optimized segment processing
 //

sdk/experimental/tdf/example_test.go

@@ -60,8 +60,10 @@ func ExampleWriter() {
 func ExampleWriter_withAttributes() {
 	ctx := context.Background()
 
-	// Create writer with custom integrity algorithm
-	writer, err := tdf.NewWriter(ctx, tdf.WithIntegrityAlgorithm(tdf.GMAC))
+	// Create writer with an explicit root integrity algorithm. HS256 is the
+	// only legal value -- the root signs the aggregate hash, which never went
+	// through the AEAD, so there is no tag for GMAC to read out.
+	writer, err := tdf.NewWriter(ctx, tdf.WithIntegrityAlgorithm(tdf.RootHS256))
 	if err != nil {
 		log.Println(err)
 		return
@@ -241,7 +243,7 @@ func ExampleWriter_largeFile() {
 
 	// Configure for large file processing
 	writer, err := tdf.NewWriter(ctx,
-		tdf.WithSegmentIntegrityAlgorithm(tdf.GMAC), // Faster for many segments
+		tdf.WithSegmentIntegrityAlgorithm(tdf.SegmentGMAC), // Faster for many segments
 	)
 	if err != nil {
 		log.Println(err)

sdk/experimental/tdf/integrity_test.go

@@ -0,0 +1,140 @@
+// Experimental: This package is EXPERIMENTAL and may change or be removed at any time
+
+package tdf
+
+import (
+	"crypto/rand"
+	"testing"
+
+	"github.com/opentdf/platform/lib/ocrypto"
+	"github.com/opentdf/platform/protocol/go/policy"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+// Each type spells only the values it accepts. Both are int-backed, so
+// out-of-range values are representable, and naming one "HS256" in a manifest
+// would claim a signature that was never computed.
+func TestIntegrityAlgStrings(t *testing.T) {
+	assert.Equal(t, algHS256, RootHS256.String())
+	assert.Equal(t, algHS256, SegmentHS256.String())
+	assert.Equal(t, algGMAC, SegmentGMAC.String())
+
+	for _, s := range []string{
+		RootIntegrityAlg(1).String(),
+		RootIntegrityAlg(-1).String(),
+		SegmentIntegrityAlg(99).String(),
+		SegmentIntegrityAlg(-1).String(),
+	} {
+		assert.NotEqual(t, algHS256, s)
+		assert.NotEqual(t, algGMAC, s)
+	}
+}
+
+// The deprecated constants stay numerically where they were, so an existing
+// caller that converts one into the new types lands on the same algorithm.
+func TestDeprecatedIntegrityAlgorithmConstants(t *testing.T) {
+	assert.Equal(t, RootHS256, RootIntegrityAlg(HS256))
+	assert.Equal(t, SegmentHS256, SegmentIntegrityAlg(HS256))
+	assert.Equal(t, SegmentGMAC, SegmentIntegrityAlg(GMAC))
+}
+
+// Both segment algorithms are real authenticators, because the input is data
+// the cipher produced. An out-of-range value is not, and has to be refused
+// rather than quietly treated as GMAC.
+func TestSegmentIntegrity(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)
+
+	hs256, err := segmentIntegrity(data, key, SegmentHS256)
+	require.NoError(t, err)
+	assert.Equal(t, string(ocrypto.CalculateSHA256Hmac(key, data)), hs256)
+
+	gmac, err := segmentIntegrity(data, key, SegmentGMAC)
+	require.NoError(t, err)
+	assert.Equal(t, string(data[len(data)-kGMACPayloadLength:]), gmac)
+
+	for _, alg := range []SegmentIntegrityAlg{SegmentIntegrityAlg(99), SegmentIntegrityAlg(-1)} {
+		_, err := segmentIntegrity(data, key, alg)
+		require.ErrorIs(t, err, ErrUnsupportedSegmentIntegrityAlgorithm, "alg %d", alg)
+	}
+}
+
+// The aggregate hash never passed through the AEAD, so tag extraction over it
+// authenticates nothing -- it returns a copy of the last segment hash, which is
+// manifest data an attacker already controls.
+func TestRootIntegrityRejectsNonHS256(t *testing.T) {
+	key := make([]byte, kKeySize)
+	_, err := rand.Read(key)
+	require.NoError(t, err)
+
+	aggregate := make([]byte, kGMACPayloadLength*4)
+	_, err = rand.Read(aggregate)
+	require.NoError(t, err)
+
+	for _, alg := range []RootIntegrityAlg{RootIntegrityAlg(GMAC), RootIntegrityAlg(99), RootIntegrityAlg(-1)} {
+		_, err := rootIntegrity(aggregate, key, alg)
+		require.ErrorIs(t, err, ErrUnsupportedRootIntegrityAlgorithm, "alg %d", alg)
+	}
+
+	sig, err := rootIntegrity(aggregate, key, RootHS256)
+	require.NoError(t, err)
+	assert.Equal(t, string(ocrypto.CalculateSHA256Hmac(key, aggregate)), sig)
+}
+
+// Option cannot return an error, so Finalize is the only place an illegal root
+// algorithm can be caught -- and it must be, or the writer produces a file no
+// conforming reader will accept.
+func TestFinalizeRejectsGMACRoot(t *testing.T) {
+	ctx := t.Context()
+
+	writer, err := NewWriter(ctx, WithIntegrityAlgorithm(RootIntegrityAlg(GMAC)))
+	require.NoError(t, err)
+
+	_, err = writer.WriteSegment(ctx, 0, []byte("Confidential business information"))
+	require.NoError(t, err)
+
+	attributes := []*policy.Value{
+		createTestAttribute("https://example.com/attr/Category/value/Financial", testKAS1, "kid1"),
+	}
+	_, err = writer.Finalize(ctx, WithAttributeValues(attributes))
+	require.ErrorIs(t, err, ErrUnsupportedRootIntegrityAlgorithm)
+}
+
+// Both segment algorithms round-trip through Finalize, and neither disturbs the
+// root, which stays HS256 whatever the segments declare.
+func TestFinalizeSegmentAlgNamesWhatItSigned(t *testing.T) {
+	for _, tc := range []struct {
+		name string
+		alg  SegmentIntegrityAlg
+		want string
+	}{
+		{"hs256", SegmentHS256, algHS256},
+		{"gmac", SegmentGMAC, algGMAC},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			ctx := t.Context()
+
+			writer, err := NewWriter(ctx, WithSegmentIntegrityAlgorithm(tc.alg))
+			require.NoError(t, err)
+
+			_, err = writer.WriteSegment(ctx, 0, []byte("Confidential business information"))
+			require.NoError(t, err)
+
+			attributes := []*policy.Value{
+				createTestAttribute("https://example.com/attr/Category/value/Financial", testKAS1, "kid1"),
+			}
+			result, err := writer.Finalize(ctx, WithAttributeValues(attributes))
+			require.NoError(t, err)
+
+			intInfo := result.Manifest.IntegrityInformation
+			assert.Equal(t, tc.want, intInfo.SegmentHashAlgorithm)
+			assert.Equal(t, algHS256, intInfo.Algorithm, "root stays HS256 regardless of the segment algorithm")
+		})
+	}
+}

sdk/experimental/tdf/manifest.go

@@ -3,8 +3,8 @@
 package tdf
 
 import (
-	"encoding/hex"
 	"errors"
+	"fmt"
 
 	"github.com/opentdf/platform/lib/ocrypto"
 )
@@ -103,20 +103,61 @@ type EncryptedMetadata struct {
 	Iv     string `json:"iv"`
 }
 
-func calculateSignature(data []byte, secret []byte, alg IntegrityAlgorithm, isLegacyTDF bool) (string, error) {
-	if alg == HS256 {
-		hmac := ocrypto.CalculateSHA256Hmac(secret, data)
-		if isLegacyTDF {
-			return hex.EncodeToString(hmac), nil
-		}
-		return string(hmac), nil
-	}
-	if kGMACPayloadLength > len(data) {
+var (
+	// ErrUnsupportedRootIntegrityAlgorithm rejects any root signature
+	// algorithm other than HS256.
+	ErrUnsupportedRootIntegrityAlgorithm = errors.New("tdf: unsupported root integrity algorithm")
+	// ErrUnsupportedSegmentIntegrityAlgorithm rejects a segment algorithm that
+	// is neither HS256 nor GMAC. SegmentIntegrityAlg is int-backed, so this
+	// catches an out-of-range value before it reaches a manifest.
+	ErrUnsupportedSegmentIntegrityAlgorithm = errors.New("tdf: unsupported segment integrity algorithm")
+)
+
+// None of these helpers take the hex-encoding flag the stable SDK carries for
+// 4.2.2 files: this package only ever writes current-spec TDFs.
+
+// hmacIntegrity is the HMAC-SHA256 primitive both signature paths share. It
+// takes no algorithm argument, so it cannot be pointed at the wrong branch.
+func hmacIntegrity(data, secret []byte) string {
+	return string(ocrypto.CalculateSHA256Hmac(secret, data))
+}
+
+// readAEADTag returns the trailing AES-GCM tag of a segment's ciphertext.
+//
+// This is only an authenticator because the cipher computed that tag over
+// exactly these bytes. Applied to anything the AEAD did not produce it
+// authenticates nothing: it just returns a copy of the input's own last 16
+// bytes. Hence unexported, and reachable only through segmentIntegrity.
+func readAEADTag(ciphertext []byte) (string, error) {
+	if kGMACPayloadLength > len(ciphertext) {
 		return "", errors.New("fail to create gmac signature")
 	}
+	return string(ciphertext[len(ciphertext)-kGMACPayloadLength:]), nil
+}
+
+// segmentIntegrity computes the integrity value recorded in a segment's
+// manifest entry. Both algorithms are legitimate here: the input is data the
+// cipher produced.
+func segmentIntegrity(ciphertext, key []byte, alg SegmentIntegrityAlg) (string, error) {
+	switch alg {
+	case SegmentHS256:
+		return hmacIntegrity(ciphertext, key), nil
+	case SegmentGMAC:
+		return readAEADTag(ciphertext)
+	}
+	return "", fmt.Errorf("%w: %s", ErrUnsupportedSegmentIntegrityAlgorithm, alg)
+}
 
-	if isLegacyTDF {
-		return hex.EncodeToString(data[len(data)-kGMACPayloadLength:]), nil
+// rootIntegrity computes the root signature over the aggregate hash: the
+// concatenation of every segment's hash, in manifest order.
+//
+// HS256 only. AES-GCM never processed the aggregate hash, so there is no tag to
+// extract and no keyless construction that could authenticate it.
+// RootIntegrityAlg has no other named value, but it is int-backed, so the check
+// still has to run.
+func rootIntegrity(aggregateHash, key []byte, alg RootIntegrityAlg) (string, error) {
+	if alg != RootHS256 {
+		return "", fmt.Errorf("%w: %s", ErrUnsupportedRootIntegrityAlgorithm, alg)
 	}
-	return string(data[len(data)-kGMACPayloadLength:]), nil
+	return hmacIntegrity(aggregateHash, key), nil
 }