From 0f191859593260f75376fd0296060c0e9ba5ac90 Mon Sep 17 00:00:00 2001 From: Dave Mihalcik Date: Thu, 10 Sep 2026 10:18:00 -0400 Subject: [PATCH] fix(sdk): reject GMAC root signatures (DSPX-4703) A ZTDF's root signature is the only thing that authenticates the manifest's ordered list of segment hashes. AES-GCM tags bind a segment's own bytes and nothing about its index, its neighbours, or how many segments there are, so segment-level integrity cannot notice a truncated, reordered, or duplicated segment list. `calculateSignature` applied one dispatch to both jobs. For a segment that is correct: "GMAC" means read back the AES-GCM tag the cipher just computed over that segment's ciphertext. For the root it is not, because the aggregate hash never passes through AES-GCM -- there is no tag to read out, so the function returned a copy of the trailing bytes of its own input, i.e. the last segment hash. Manifest data compared against manifest data, with the payload key never used. `validateRootSignature` took the algorithm from `manifest.rootSignature.alg`, which is unauthenticated, and coerced anything unrecognised to HS256 while honouring "GMAC" in any casing. An attacker with no key could therefore take an HS256-rooted TDF, rewrite the root to `alg: "GMAC"` with a signature copied from the last segment hash, and then truncate, reorder, or duplicate segments with the whole thing still verifying. Read path (sdk): - Retire `calculateSignature` in favour of four functions that cannot be misapplied: `hmacIntegrity`, `readAEADTag` (unexported, reachable only through `segmentIntegrity`), `segmentIntegrity` (HS256 or GMAC), and `rootIntegrity` (HS256 only). - `validateRootSignature` now fails closed on a case-insensitive allowlist: anything other than HS256 returns ErrUnsupportedRootIntegrityAlgorithm, surfaced to callers as ErrRootSignatureFailure (which wraps ErrTampered). An absent `alg` still means HS256, as before; the HMAC must verify either way. - `manifestSegmentIntegrityAlg` keeps the segment path permissive, since both algorithms are meaningful over ciphertext. - manifest.schema.json constrains `rootSignature.alg` to HS256. manifest-lax.schema.json deliberately stays permissive so it can still parse hostile input for testing. Write path, both `sdk` and `sdk/experimental/tdf`: - Split `IntegrityAlgorithm` into `RootIntegrityAlg` (RootHS256 only) and `SegmentIntegrityAlg` (SegmentHS256, SegmentGMAC). One type could not express that the two positions accept different sets, which is what let a GMAC root be configured in the first place. Each type's String() returns the manifest spelling, and a non-spelling fallback out of range, so a value with no legal name cannot reach a manifest. - `IntegrityAlgorithm`, `HS256` and `GMAC` remain as deprecated untyped constants with their original numeric values, so existing callers still compile and convert to the matching new constant. - The experimental writer previously accepted `WithIntegrityAlgorithm(GMAC)` and emitted the forged-by-construction root described above; Finalize now returns ErrUnsupportedRootIntegrityAlgorithm. `Option` cannot return an error, so Finalize is the only place to catch it. Unit tests in both packages pin the boundary each helper enforces, and the experimental writer's Finalize tests pin the write-side refusal. End-to-end coverage of the attack itself -- truncation, reordering, GMAC in four casings, and an unknown algorithm that must not be coerced -- lives in the cross-SDK corpus, so every SDK is held to the same behaviour rather than Go alone. Every golden TDF in the cross-SDK corpus is already `alg: HS256`, so no existing well-formed file is affected. Refs: DSPX-4703, and the write-side controls in DSPX-4736. Signed-off-by: Dave Mihalcik --- examples/cmd/benchmark_experimental.go | 2 +- sdk/experimental/tdf/doc.go | 3 +- sdk/experimental/tdf/example_test.go | 8 +- sdk/experimental/tdf/integrity_test.go | 140 +++++++++++++++++++++++ sdk/experimental/tdf/manifest.go | 67 ++++++++--- sdk/experimental/tdf/options.go | 137 +++++++++++++++++------ sdk/experimental/tdf/writer.go | 24 ++-- sdk/experimental/tdf/writer_test.go | 4 +- sdk/schema/manifest-lax.schema.json | 2 +- sdk/schema/manifest.schema.json | 5 +- sdk/tdf.go | 148 +++++++++++++++++-------- sdk/tdf_config.go | 88 +++++++++++++-- sdk/tdf_helpers_test.go | 95 +++++++++++++--- sdk/tdf_readat_test.go | 2 +- sdk/tdferrors.go | 9 ++ 15 files changed, 593 insertions(+), 141 deletions(-) create mode 100644 sdk/experimental/tdf/integrity_test.go diff --git a/examples/cmd/benchmark_experimental.go b/examples/cmd/benchmark_experimental.go index f50c5cc348..31877fa3e3 100644 --- a/examples/cmd/benchmark_experimental.go +++ b/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) } diff --git a/sdk/experimental/tdf/doc.go b/sdk/experimental/tdf/doc.go index 8e80e36173..b149c0212d 100644 --- a/sdk/experimental/tdf/doc.go +++ b/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 // diff --git a/sdk/experimental/tdf/example_test.go b/sdk/experimental/tdf/example_test.go index ec0fe4fc79..6e36939e5e 100644 --- a/sdk/experimental/tdf/example_test.go +++ b/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) diff --git a/sdk/experimental/tdf/integrity_test.go b/sdk/experimental/tdf/integrity_test.go new file mode 100644 index 0000000000..643a403ce8 --- /dev/null +++ b/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") + }) + } +} diff --git a/sdk/experimental/tdf/manifest.go b/sdk/experimental/tdf/manifest.go index 10f5eceb0f..4171368f89 100644 --- a/sdk/experimental/tdf/manifest.go +++ b/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 } diff --git a/sdk/experimental/tdf/options.go b/sdk/experimental/tdf/options.go index e2c7e70d31..9de6b4d7ae 100644 --- a/sdk/experimental/tdf/options.go +++ b/sdk/experimental/tdf/options.go @@ -2,25 +2,29 @@ package tdf -import "github.com/opentdf/platform/protocol/go/policy" +import ( + "fmt" -// IntegrityAlgorithm specifies the cryptographic algorithm used for integrity verification. -// -// Different algorithms provide different security and performance characteristics: -// - HS256: HMAC-SHA256, widely supported, good balance of security and performance -// - GMAC: Galois Message Authentication Code, faster but requires AES-GCM support + "github.com/opentdf/platform/protocol/go/policy" +) + +// IntegrityAlgorithm specified an integrity algorithm without saying what it +// was allowed to protect. // -// The algorithm choice affects both segment-level and root-level integrity verification. +// Deprecated: the root signature and the segment hashes accept different sets +// of algorithms, which one type cannot express. Use [RootIntegrityAlg] or +// [SegmentIntegrityAlg]. type IntegrityAlgorithm int // String returns the string representation of the integrity algorithm. -// Used for manifest generation and protocol compatibility. +// +// Deprecated: use [RootIntegrityAlg.String] or [SegmentIntegrityAlg.String]. func (i IntegrityAlgorithm) String() string { switch i { case HS256: - return "HS256" + return algHS256 case GMAC: - return "GMAC" + return algGMAC default: return "unknown" } @@ -28,13 +32,76 @@ func (i IntegrityAlgorithm) String() string { const ( // HS256 uses HMAC-SHA256 for integrity verification. - // This is the default and most widely supported algorithm. + // + // Deprecated: use [RootHS256] or [SegmentHS256]. HS256 = iota // GMAC uses Galois Message Authentication Code for integrity verification. - // Provides better performance with AES-GCM but requires hardware support for optimal speed. + // + // Deprecated: use [SegmentGMAC]. GMAC is not a legal root algorithm. GMAC ) +// Manifest spellings of the two algorithms. +const ( + algHS256 = "HS256" + algGMAC = "GMAC" +) + +// RootIntegrityAlg is the algorithm that signs the aggregate hash -- the +// concatenation of every segment hash, in manifest order. The root signature is +// the only thing that authenticates the manifest's description of the payload, +// so a segment list that has been truncated, reordered, or duplicated is caught +// here or not at all. +// +// HS256 is the only value, and this type exists to say so at compile time. The +// aggregate hash is manifest data that never passed through the AEAD, so tag +// extraction has nothing to extract: a "GMAC" root signature is just a copy of +// the last segment hash, producible by an attacker with no key. +type RootIntegrityAlg int + +// RootHS256 is an HMAC-SHA256 over the aggregate hash, keyed by the DEK. It is +// the default and the only supported value. +const RootHS256 RootIntegrityAlg = iota + +// SegmentIntegrityAlg is the algorithm that authenticates a single segment's +// bytes. Unlike the root, both values are genuine authenticators, because the +// input is data the cipher itself produced. +type SegmentIntegrityAlg int + +const ( + // SegmentHS256 is an HMAC-SHA256 over the segment's bytes, keyed by the + // DEK. It is the only meaningful choice when those bytes are not AEAD + // output -- a plaintext segment has no tag to read out -- and it is this + // writer's default. + SegmentHS256 SegmentIntegrityAlg = iota + // SegmentGMAC reads out the AES-GCM tag the cipher already computed over + // exactly this segment's ciphertext. + SegmentGMAC +) + +// String returns the manifest spelling of the algorithm. Out-of-range values +// have no spelling: the type is int-backed, so they are representable, and +// naming one "HS256" in a manifest would claim a signature that was never +// computed. +func (a RootIntegrityAlg) String() string { + if a == RootHS256 { + return algHS256 + } + return fmt.Sprintf("RootIntegrityAlg(%d)", int(a)) +} + +// String returns the manifest spelling of the algorithm. See +// [RootIntegrityAlg.String] on out-of-range values. +func (a SegmentIntegrityAlg) String() string { + switch a { + case SegmentHS256: + return algHS256 + case SegmentGMAC: + return algGMAC + } + return fmt.Sprintf("SegmentIntegrityAlg(%d)", int(a)) +} + // BaseConfig provides common configuration foundation for TDF operations. // Currently empty but reserved for future common configuration options. type BaseConfig struct{} @@ -42,16 +109,17 @@ type BaseConfig struct{} // WriterConfig contains configuration options for TDF Writer creation. // // The configuration controls cryptographic algorithms and processing behavior: -// - integrityAlgorithm: Algorithm for root integrity signature calculation -// - segmentIntegrityAlgorithm: Algorithm for individual segment hash calculation +// - rootIntegrityAlg: Algorithm for root integrity signature calculation +// - segmentIntegrityAlg: Algorithm for individual segment hash calculation // -// These can be set independently to optimize for different security/performance requirements. +// These are set independently, and their legal values differ: see +// [RootIntegrityAlg] and [SegmentIntegrityAlg]. type WriterConfig struct { BaseConfig - // integrityAlgorithm specifies the algorithm for root integrity verification - integrityAlgorithm IntegrityAlgorithm - // segmentIntegrityAlgorithm specifies the algorithm for segment-level integrity - segmentIntegrityAlgorithm IntegrityAlgorithm + // rootIntegrityAlg specifies the algorithm for root integrity verification + rootIntegrityAlg RootIntegrityAlg + // segmentIntegrityAlg specifies the algorithm for segment-level integrity + segmentIntegrityAlg SegmentIntegrityAlg // initialAttributes allows callers to provide attribute values at writer creation time. // These will be used during Finalize() if no attributes are provided there. @@ -77,7 +145,7 @@ type ReaderConfig struct { // // Example usage: // -// writer, err := NewWriter(ctx, WithIntegrityAlgorithm(GMAC)) +// writer, err := NewWriter(ctx, WithSegmentIntegrityAlgorithm(SegmentGMAC)) // finalBytes, manifest, err := writer.Finalize(ctx, WithPayloadMimeType("text/plain")) type Option[T any] func(T) @@ -86,16 +154,15 @@ type Option[T any] func(T) // The root integrity algorithm is used to generate a signature over all segment hashes, // providing verification that the complete TDF has not been tampered with. // -// Algorithm options: -// - HS256: HMAC-SHA256 (default) - widely supported, secure -// - GMAC: Galois Message Authentication Code - faster with hardware acceleration +// [RootHS256] is the only supported value, and the default. Options cannot +// return an error, so anything else is refused by Finalize. // // Example: // -// writer, err := NewWriter(ctx, WithIntegrityAlgorithm(GMAC)) -func WithIntegrityAlgorithm(algo IntegrityAlgorithm) Option[*WriterConfig] { +// writer, err := NewWriter(ctx, WithIntegrityAlgorithm(RootHS256)) +func WithIntegrityAlgorithm(algo RootIntegrityAlg) Option[*WriterConfig] { return func(c *WriterConfig) { - c.integrityAlgorithm = algo + c.rootIntegrityAlg = algo } } @@ -106,21 +173,23 @@ func WithIntegrityAlgorithm(algo IntegrityAlgorithm) Option[*WriterConfig] { // complete file. This is particularly useful for streaming scenarios where // segments may be processed independently. // -// The segment algorithm can differ from the root algorithm to optimize for -// different processing patterns: -// - Use GMAC for segments if processing many small segments (better performance) -// - Use HS256 for root signature for broader compatibility +// Both [SegmentHS256] and [SegmentGMAC] are supported, and the choice is +// independent of the root: +// - SegmentGMAC reads out the AES-GCM tag the cipher already computed, so it +// costs nothing extra per segment +// - SegmentHS256 is the default, and the only option for bytes the AEAD did +// not produce // // Example: // // // Fast segment processing with compatible root signature // writer, err := NewWriter(ctx, -// WithSegmentIntegrityAlgorithm(GMAC), // Fast segment hashing -// WithIntegrityAlgorithm(HS256), // Compatible root signature +// WithSegmentIntegrityAlgorithm(SegmentGMAC), // Fast segment hashing +// WithIntegrityAlgorithm(RootHS256), // Compatible root signature // ) -func WithSegmentIntegrityAlgorithm(algo IntegrityAlgorithm) Option[*WriterConfig] { +func WithSegmentIntegrityAlgorithm(algo SegmentIntegrityAlg) Option[*WriterConfig] { return func(c *WriterConfig) { - c.segmentIntegrityAlgorithm = algo + c.segmentIntegrityAlg = algo } } diff --git a/sdk/experimental/tdf/writer.go b/sdk/experimental/tdf/writer.go index 2a58af6c87..28ce926543 100644 --- a/sdk/experimental/tdf/writer.go +++ b/sdk/experimental/tdf/writer.go @@ -79,7 +79,7 @@ var ( // // Example usage: // -// writer, err := NewWriter(ctx, WithIntegrityAlgorithm(HS256)) +// writer, err := NewWriter(ctx, WithIntegrityAlgorithm(RootHS256)) // if err != nil { // return err // } @@ -132,8 +132,8 @@ type Writer struct { // - Memory-efficient segment processing // // Configuration options can be provided to customize: -// - Integrity algorithm selection (HS256, GMAC) -// - Segment integrity algorithm (independent of root algorithm) +// - Segment integrity algorithm (SegmentHS256, SegmentGMAC) +// - Root integrity algorithm, which accepts RootHS256 and nothing else // // The writer generates a unique Data Encryption Key (DEK) and initializes // the underlying archive writer for ZIP structure management. @@ -150,14 +150,14 @@ type Writer struct { // // // Custom integrity algorithms // writer, err := NewWriter(ctx, -// WithIntegrityAlgorithm(GMAC), -// WithSegmentIntegrityAlgorithm(HS256), +// WithIntegrityAlgorithm(RootHS256), +// WithSegmentIntegrityAlgorithm(SegmentGMAC), // ) func NewWriter(_ context.Context, opts ...Option[*WriterConfig]) (*Writer, error) { // Initialize Config config := &WriterConfig{ - integrityAlgorithm: HS256, - segmentIntegrityAlgorithm: HS256, + rootIntegrityAlg: RootHS256, + segmentIntegrityAlg: SegmentHS256, } for _, opt := range opts { @@ -264,7 +264,7 @@ func (w *Writer) WriteSegment(ctx context.Context, index int, data []byte) (*Seg if err != nil { return nil, err } - segmentSig, err := calculateSignature(segmentCipher, w.dek, w.segmentIntegrityAlgorithm, false) // Don't ever hex encode new tdf's + segmentSig, err := segmentIntegrity(segmentCipher, w.dek, w.segmentIntegrityAlg) if err != nil { return nil, err } @@ -538,7 +538,9 @@ func (w *Writer) getManifest(ctx context.Context, cfg *WriterFinalizeConfig) (*M } // Set segment hash algorithm - encryptInfo.SegmentHashAlgorithm = w.segmentIntegrityAlgorithm.String() + // Safe to take the name from the config: an algorithm with no manifest + // spelling would already have failed every segmentIntegrity call above. + encryptInfo.SegmentHashAlgorithm = w.segmentIntegrityAlg.String() var aggregateHash bytes.Buffer // Calculate totals and iterate through segments in finalize order @@ -565,12 +567,12 @@ func (w *Writer) getManifest(ctx context.Context, cfg *WriterFinalizeConfig) (*M return nil, 0, 0, errors.New("empty segment hash") } - rootSignature, err := calculateSignature(aggregateHash.Bytes(), w.dek, w.integrityAlgorithm, false) + rootSignature, err := rootIntegrity(aggregateHash.Bytes(), w.dek, w.rootIntegrityAlg) if err != nil { return nil, 0, 0, err } encryptInfo.RootSignature = RootSignature{ - Algorithm: w.integrityAlgorithm.String(), + Algorithm: w.rootIntegrityAlg.String(), Signature: string(ocrypto.Base64Encode([]byte(rootSignature))), } diff --git a/sdk/experimental/tdf/writer_test.go b/sdk/experimental/tdf/writer_test.go index 300b5746be..038ccd63e2 100644 --- a/sdk/experimental/tdf/writer_test.go +++ b/sdk/experimental/tdf/writer_test.go @@ -472,7 +472,7 @@ func testKeySplittingWithMultipleAttributes(t *testing.T) { func testManifestGeneration(t *testing.T) { ctx := t.Context() - writer, err := NewWriter(ctx, WithIntegrityAlgorithm(HS256)) + writer, err := NewWriter(ctx, WithIntegrityAlgorithm(RootHS256)) require.NoError(t, err, "Failed to create TDF writer") // Write test data @@ -1179,7 +1179,7 @@ func testGetManifestBeforeAndAfterFinalize(t *testing.T) { assert.Len(t, m1.Segments, 1) assert.Equal(t, int64(len(data)), m1.DefaultSegmentSize) assert.Greater(t, m1.DefaultEncryptedSegSize, int64(len(data))) - assert.Equal(t, writer.segmentIntegrityAlgorithm.String(), m1.SegmentHashAlgorithm) + assert.Equal(t, writer.segmentIntegrityAlg.String(), m1.SegmentHashAlgorithm) // Finalize and GetManifest should return the final one (with key access, root signature) attrs := []*policy.Value{createTestAttribute("https://example.com/attr/Test/value/Basic", testKAS1, "kid1")} diff --git a/sdk/schema/manifest-lax.schema.json b/sdk/schema/manifest-lax.schema.json index 2d9c730279..9353fc3c09 100644 --- a/sdk/schema/manifest-lax.schema.json +++ b/sdk/schema/manifest-lax.schema.json @@ -129,7 +129,7 @@ "type": "object", "properties": { "alg": { - "description": "Algorithm used to generate the root signature of the payload", + "description": "Algorithm used to generate the root signature of the payload. Deliberately unconstrained here, unlike the strict schema: Lax backs IsValidTdf, which answers 'is this a TDF at all', and the root algorithm is enforced by validateRootSignature at every validation intensity. Rejecting it here would report ErrInvalidPerSchema instead of ErrRootSignatureFailure and would let a schema setting mask the integrity check.", "type": "string" }, "sig": { diff --git a/sdk/schema/manifest.schema.json b/sdk/schema/manifest.schema.json index 8a0e47ac94..4accd26388 100644 --- a/sdk/schema/manifest.schema.json +++ b/sdk/schema/manifest.schema.json @@ -129,8 +129,9 @@ "type": "object", "properties": { "alg": { - "description": "Algorithm used to generate the root signature of the payload", - "type": "string" + "description": "Algorithm used to generate the root signature of the payload. HS256 only: the root signature covers the aggregate hash, which AES-GCM never processes, so a GMAC root has no authentication tag to read back out. An absent alg means HS256.", + "type": "string", + "enum": ["HS256"] }, "sig": { "description": "The payload signature", diff --git a/sdk/tdf.go b/sdk/tdf.go index f3f8086174..5ccd91b62c 100644 --- a/sdk/tdf.go +++ b/sdk/tdf.go @@ -274,8 +274,8 @@ func (s SDK) CreateTDFContext(ctx context.Context, writer io.Writer, reader io.R return nil, fmt.Errorf("io.writer.Write failed: %w", err) } - segmentSig, err := calculateSignature(cipherData, tdfObject.payloadKey[:], - tdfConfig.segmentIntegrityAlgorithm, tdfConfig.useHex) + segmentSig, err := segmentIntegrity(cipherData, tdfObject.payloadKey[:], + tdfConfig.segmentIntegrityAlg, tdfConfig.useHex) if err != nil { return nil, fmt.Errorf("splitKey.GetSignaturefailed: %w", err) } @@ -294,8 +294,8 @@ func (s SDK) CreateTDFContext(ctx context.Context, writer io.Writer, reader io.R segmentIndex++ } - rootSignature, err := calculateSignature([]byte(aggregateHashBuilder.String()), tdfObject.payloadKey[:], - tdfConfig.integrityAlgorithm, tdfConfig.useHex) + rootSignature, err := rootIntegrity([]byte(aggregateHashBuilder.String()), tdfObject.payloadKey[:], + tdfConfig.rootIntegrityAlg, tdfConfig.useHex) if err != nil { return nil, fmt.Errorf("splitKey.GetSignaturefailed: %w", err) } @@ -303,12 +303,15 @@ func (s SDK) CreateTDFContext(ctx context.Context, writer io.Writer, reader io.R sig := string(ocrypto.Base64Encode([]byte(rootSignature))) tdfObject.manifest.Signature = sig - tdfObject.manifest.Algorithm = integrityAlgorithmString(tdfConfig.integrityAlgorithm) + // Both names are safe to take from the config only because the two + // signature helpers above have already refused every value that has no + // manifest spelling. + tdfObject.manifest.Algorithm = tdfConfig.rootIntegrityAlg.String() tdfObject.manifest.DefaultSegmentSize = segmentSize tdfObject.manifest.DefaultEncryptedSegSize = encryptedSegmentSize - tdfObject.manifest.SegmentHashAlgorithm = integrityAlgorithmString(tdfConfig.segmentIntegrityAlgorithm) + tdfObject.manifest.SegmentHashAlgorithm = tdfConfig.segmentIntegrityAlg.String() tdfObject.manifest.Method.IsStreamable = true // add payload info @@ -587,19 +590,20 @@ func (s SDK) prepareManifest(ctx context.Context, t *TDFObject, tdfConfig TDFCon return nil } -// integrityAlgorithmString maps an IntegrityAlgorithm to its manifest -// string form. +// manifestSegmentIntegrityAlg maps the manifest's segmentHashAlg string onto +// the algorithm used to verify each segment. // -// 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 +// Unlike the root signature this stays permissive: a segment hash is computed +// over ciphertext the AEAD produced, so both algorithms are real +// authenticators, and every TDF in the wild declares GMAC here. A manifest that +// lies about segmentHashAlg gains nothing -- whichever value it names, the +// resulting segment hashes still have to reproduce the aggregate hash covered +// by the HS256 root signature. +func manifestSegmentIntegrityAlg(alg string) SegmentIntegrityAlg { + if strings.EqualFold(gmacIntegrityAlgorithm, alg) { + return SegmentGMAC + } + return SegmentHS256 } // createPolicyBinding produces an HMAC-SHA256 binding value keyed on the @@ -989,13 +993,8 @@ func (r *Reader) WriteTo(writer io.Writer) (int64, error) { return totalBytes, ErrSegSizeMismatch } - segHashAlg := r.manifest.SegmentHashAlgorithm - sigAlg := HS256 - if strings.EqualFold(gmacIntegrityAlgorithm, segHashAlg) { - sigAlg = GMAC - } - - payloadSig, err := calculateSignature(readBuf, r.payloadKey, sigAlg, isLegacyTDF) + sigAlg := manifestSegmentIntegrityAlg(r.manifest.SegmentHashAlgorithm) + payloadSig, err := segmentIntegrity(readBuf, r.payloadKey, sigAlg, isLegacyTDF) if err != nil { return totalBytes, fmt.Errorf("splitKey.GetSignaturefailed: %w", err) } @@ -1116,13 +1115,8 @@ func (r *Reader) ReadAt(buf []byte, offset int64) (int, error) { //nolint:funlen return 0, ErrSegSizeMismatch } - segHashAlg := r.manifest.SegmentHashAlgorithm - sigAlg := HS256 - if strings.EqualFold(gmacIntegrityAlgorithm, segHashAlg) { - sigAlg = GMAC - } - - payloadSig, err := calculateSignature(readBuf, r.payloadKey, sigAlg, isLegacyTDF) + sigAlg := manifestSegmentIntegrityAlg(r.manifest.SegmentHashAlgorithm) + payloadSig, err := segmentIntegrity(readBuf, r.payloadKey, sigAlg, isLegacyTDF) if err != nil { return 0, fmt.Errorf("splitKey.GetSignaturefailed: %w", err) } @@ -1560,23 +1554,76 @@ func (r *Reader) doPayloadKeyUnwrap(ctx context.Context) error { //nolint:gocogn return r.buildKey(ctx, kaoResults) } -// calculateSignature calculate signature of data of the given algorithm. -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 +// hmacIntegrity computes an HMAC-SHA256 over data, keyed by the payload key. +// +// Legacy (pre-4.3.0) TDFs hex-encode the digest before the caller base64s it; +// isLegacyTDF preserves that wire format. +func hmacIntegrity(data, key []byte, isLegacyTDF bool) string { + hmac := ocrypto.CalculateSHA256Hmac(key, data) + if isLegacyTDF { + return hex.EncodeToString(hmac) } - if kGMACPayloadLength > len(data) { - return "", fmt.Errorf("%w: ciphertext length=%d", ErrGMACSignatureFailed, len(data)) + return string(hmac) +} + +// readAEADTag returns the trailing authentication tag of an AES-GCM ciphertext. +// +// PRECONDITION: ciphertext must be exactly the bytes AES-GCM sealed under the +// payload key. Under that precondition the trailing kGMACPayloadLength bytes +// are the GHASH-derived tag the cipher already computed over those bytes, so +// reading them back out is a genuine MAC obtained for free -- the key was used, +// a moment earlier, by the AEAD. +// +// Applied to anything the AEAD never processed the same rule authenticates +// nothing: it just returns a copy of the input's own last 16 bytes. That is why +// this helper is unexported and reachable only through segmentIntegrity, whose +// argument is by construction a segment's ciphertext. rootIntegrity, whose +// input is the aggregate hash, has no way to call it. +func readAEADTag(ciphertext []byte, isLegacyTDF bool) (string, error) { + if kGMACPayloadLength > len(ciphertext) { + return "", fmt.Errorf("%w: ciphertext length=%d", ErrGMACSignatureFailed, len(ciphertext)) } + tag := ciphertext[len(ciphertext)-kGMACPayloadLength:] if isLegacyTDF { - return hex.EncodeToString(data[len(data)-kGMACPayloadLength:]), nil + return hex.EncodeToString(tag), nil } - return string(data[len(data)-kGMACPayloadLength:]), nil + return string(tag), nil +} + +// segmentIntegrity computes the integrity value recorded in (and checked +// against) a segment's manifest entry, over that segment's AES-GCM ciphertext. +// +// Both algorithms are legitimate here. GMAC reads out the AEAD tag already +// computed over exactly these bytes; HS256 recomputes an HMAC over them. GMAC +// is the default and is what essentially every existing TDF uses. +// +// The switch is closed rather than "anything that is not HS256 is GMAC": +// SegmentIntegrityAlg is int-backed, and silently treating an out-of-range +// value as GMAC would write a manifest naming an algorithm the signature was +// not computed with. +func segmentIntegrity(ciphertext, key []byte, alg SegmentIntegrityAlg, isLegacyTDF bool) (string, error) { + switch alg { + case SegmentHS256: + return hmacIntegrity(ciphertext, key, isLegacyTDF), nil + case SegmentGMAC: + return readAEADTag(ciphertext, isLegacyTDF) + } + return "", fmt.Errorf("%w: %s", ErrUnsupportedSegmentIntegrityAlgorithm, alg) +} + +// rootIntegrity computes the root signature over the aggregate hash: the +// concatenation of every segment's decoded 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 -- see +// ErrUnsupportedRootIntegrityAlgorithm. 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, isLegacyTDF bool) (string, error) { + if alg != RootHS256 { + return "", fmt.Errorf("%w: %s", ErrUnsupportedRootIntegrityAlgorithm, alg) + } + return hmacIntegrity(aggregateHash, key, isLegacyTDF), nil } // validate the root signature @@ -1585,12 +1632,17 @@ func validateRootSignature(manifest Manifest, aggregateHash, secret []byte) (boo rootSigValue := manifest.Signature isLegacyTDF := manifest.TDFVersion == "" - sigAlg := HS256 - if strings.EqualFold(gmacIntegrityAlgorithm, rootSigAlg) { - sigAlg = GMAC + // Allowlist, not "anything that is not GMAC means HS256". The algorithm + // name arrives from the unauthenticated manifest, so an unrecognised value + // has to be refused rather than quietly verified as something else -- + // coercing to HS256 would validate a downgraded file against the wrong + // algorithm and hide the substitution. An absent or empty alg keeps its + // historical meaning of HS256. + if rootSigAlg != "" && !strings.EqualFold(hmacIntegrityAlgorithm, rootSigAlg) { + return false, fmt.Errorf("%w: %q", ErrUnsupportedRootIntegrityAlgorithm, rootSigAlg) } - sig, err := calculateSignature(aggregateHash, secret, sigAlg, isLegacyTDF) + sig, err := rootIntegrity(aggregateHash, secret, RootHS256, isLegacyTDF) if err != nil { return false, fmt.Errorf("splitkey.getSignature failed:%w", err) } diff --git a/sdk/tdf_config.go b/sdk/tdf_config.go index b77e85030c..f9d6aba0a2 100644 --- a/sdk/tdf_config.go +++ b/sdk/tdf_config.go @@ -35,13 +35,83 @@ const ( schemeSeperator = "://" ) +// IntegrityAlgorithm named an integrity algorithm without saying what it was +// allowed to protect. +// +// Deprecated: the root signature and the segment hashes accept different sets +// of algorithms, which one type cannot express. Use [RootIntegrityAlg] or +// [SegmentIntegrityAlg]. type IntegrityAlgorithm = int const ( + // HS256 selects HMAC-SHA256. + // + // Deprecated: use [RootHS256] or [SegmentHS256]. HS256 = iota + // GMAC selects AES-GCM tag extraction. + // + // Deprecated: use [SegmentGMAC]. GMAC is not a legal root algorithm. GMAC ) +// RootIntegrityAlg is the algorithm that signs a TDF's aggregate hash -- the +// concatenation of every segment hash, in manifest order. The root signature is +// the only thing that authenticates the manifest's description of the payload, +// so a segment list that has been truncated, reordered, or duplicated is caught +// here or not at all. +// +// HS256 is the only value, and this type exists to say so at compile time. The +// aggregate hash is manifest data that never passed through the AEAD, so +// tag extraction has nothing to extract: a "GMAC" root signature is just a copy +// of the last segment hash, producible by an attacker with no key. +type RootIntegrityAlg int + +// RootHS256 is an HMAC-SHA256 over the aggregate hash, keyed by the payload +// key. It is the default, the only supported value, and what every TDF in the +// wild already declares. +const RootHS256 RootIntegrityAlg = iota + +// SegmentIntegrityAlg is the algorithm that authenticates a single segment's +// bytes. Unlike the root, both values are genuine authenticators, because the +// input is data the cipher itself produced. +type SegmentIntegrityAlg int + +const ( + // SegmentHS256 is an HMAC-SHA256 over the segment's bytes, keyed by the + // payload key. It is the only meaningful choice when those bytes are not + // AEAD output -- a plaintext segment has no tag to read out -- which the + // experimental writer can produce and the stable one cannot. Readers must + // keep accepting it either way. + SegmentHS256 SegmentIntegrityAlg = iota + // SegmentGMAC reads out the AES-GCM tag the cipher already computed over + // exactly this segment's ciphertext. It is the default, and what + // essentially every existing TDF declares. + SegmentGMAC +) + +// String returns the manifest spelling of the algorithm. Out-of-range values +// have no spelling: the type is int-backed, so they are representable, and +// naming one "HS256" in a manifest would claim a signature that was never +// computed. +func (a RootIntegrityAlg) String() string { + if a == RootHS256 { + return hmacIntegrityAlgorithm + } + return fmt.Sprintf("RootIntegrityAlg(%d)", int(a)) +} + +// String returns the manifest spelling of the algorithm. See +// [RootIntegrityAlg.String] on out-of-range values. +func (a SegmentIntegrityAlg) String() string { + switch a { + case SegmentHS256: + return hmacIntegrityAlgorithm + case SegmentGMAC: + return gmacIntegrityAlgorithm + } + return fmt.Sprintf("SegmentIntegrityAlg(%d)", int(a)) +} + // KASInfo contains Key Access Server information. type KASInfo struct { // URL of the KAS server @@ -67,8 +137,8 @@ type TDFConfig struct { tdfFormat TDFFormat metaData string mimeType string - integrityAlgorithm IntegrityAlgorithm - segmentIntegrityAlgorithm IntegrityAlgorithm + rootIntegrityAlg RootIntegrityAlg + segmentIntegrityAlg SegmentIntegrityAlg assertions []AssertionConfig attributes []AttributeValueFQN attributeValues []*policy.Value @@ -83,13 +153,13 @@ type TDFConfig struct { func newTDFConfig(opt ...TDFOption) (*TDFConfig, error) { c := &TDFConfig{ - autoconfigure: true, - defaultSegmentSize: defaultSegmentSize, - enableEncryption: true, - tdfFormat: JSONFormat, - integrityAlgorithm: HS256, - segmentIntegrityAlgorithm: GMAC, - addDefaultAssertion: false, + autoconfigure: true, + defaultSegmentSize: defaultSegmentSize, + enableEncryption: true, + tdfFormat: JSONFormat, + rootIntegrityAlg: RootHS256, + segmentIntegrityAlg: SegmentGMAC, + addDefaultAssertion: false, } for _, o := range opt { diff --git a/sdk/tdf_helpers_test.go b/sdk/tdf_helpers_test.go index da0e43f064..9cb1d50011 100644 --- a/sdk/tdf_helpers_test.go +++ b/sdk/tdf_helpers_test.go @@ -2,6 +2,7 @@ package sdk import ( "crypto/rand" + "encoding/hex" "testing" "github.com/opentdf/platform/lib/ocrypto" @@ -9,18 +10,32 @@ import ( "github.com/stretchr/testify/require" ) -func TestIntegrityAlgorithmString(t *testing.T) { - assert.Equal(t, hmacIntegrityAlgorithm, integrityAlgorithmString(HS256)) - assert.Equal(t, gmacIntegrityAlgorithm, integrityAlgorithmString(GMAC)) +// The two types spell themselves for the manifest. The root has one legal +// value, the segment has two, and neither may invent a spelling for anything +// else -- both are int-backed, so out-of-range values are representable. +func TestIntegrityAlgStrings(t *testing.T) { + assert.Equal(t, hmacIntegrityAlgorithm, RootHS256.String()) + assert.Equal(t, hmacIntegrityAlgorithm, SegmentHS256.String()) + assert.Equal(t, gmacIntegrityAlgorithm, SegmentGMAC.String()) + + for _, s := range []string{ + RootIntegrityAlg(1).String(), + RootIntegrityAlg(-1).String(), + SegmentIntegrityAlg(99).String(), + SegmentIntegrityAlg(-1).String(), + } { + assert.NotEqual(t, hmacIntegrityAlgorithm, s) + assert.NotEqual(t, gmacIntegrityAlgorithm, s) + } } -// The manifest string has to name the algorithm calculateSignature actually +// The manifest string has to name the algorithm segmentIntegrity 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) { +// integrity failure. Anything the String method cannot spell, segmentIntegrity +// must refuse to sign -- that pairing is what keeps the two in step for values +// neither one has a case for. The java and web SDKs use an enum and a string +// union respectively, so neither can express the out-of-range case at all. +func TestSegmentIntegrityMatchesItsManifestString(t *testing.T) { key := make([]byte, kKeySize) _, err := rand.Read(key) require.NoError(t, err) @@ -29,24 +44,29 @@ func TestIntegrityAlgorithmStringMatchesCalculateSignature(t *testing.T) { _, 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) + for _, alg := range []SegmentIntegrityAlg{SegmentHS256, SegmentGMAC} { + sig, err := segmentIntegrity(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, + assert.Equal(t, usedGMAC, alg.String() == gmacIntegrityAlgorithm, "manifest string %q disagrees with the signature computed for alg %d", - integrityAlgorithmString(alg), alg) + alg, alg) + } + + for _, alg := range []SegmentIntegrityAlg{SegmentIntegrityAlg(99), SegmentIntegrityAlg(-1)} { + _, err := segmentIntegrity(data, key, alg, false) + require.ErrorIs(t, err, ErrUnsupportedSegmentIntegrityAlgorithm, "alg %d", alg) } } // GMAC signs by returning the ciphertext's trailing kGMACPayloadLength bytes // verbatim; a ciphertext shorter than that has no tag to return and must be // rejected rather than silently truncated or padded. -func TestCalculateSignatureGMACShortCiphertext(t *testing.T) { +func TestSegmentIntegrityGMACShortCiphertext(t *testing.T) { key := make([]byte, kKeySize) _, err := rand.Read(key) require.NoError(t, err) @@ -55,11 +75,56 @@ func TestCalculateSignatureGMACShortCiphertext(t *testing.T) { _, err = rand.Read(data) require.NoError(t, err) - _, err = calculateSignature(data, key, GMAC, false) + _, err = segmentIntegrity(data, key, SegmentGMAC, false) require.ErrorIs(t, err, ErrGMACSignatureFailed) require.ErrorIs(t, err, ErrTampered) } +// rootIntegrity is the half of the old calculateSignature whose input never +// went through the AEAD, so it must refuse every algorithm but HS256. +// RootIntegrityAlg names no other value, but it is int-backed: a conversion +// from the deprecated GMAC constant, or from any other int, still compiles. +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, false) + require.ErrorIs(t, err, ErrUnsupportedRootIntegrityAlgorithm, "alg %d", alg) + } + + // HS256 keeps both encodings: raw HMAC for 4.3.0+, hex-encoded for 4.2.2. + sig, err := rootIntegrity(aggregate, key, RootHS256, false) + require.NoError(t, err) + assert.Equal(t, string(ocrypto.CalculateSHA256Hmac(key, aggregate)), sig) + + legacySig, err := rootIntegrity(aggregate, key, RootHS256, true) + require.NoError(t, err) + assert.Equal(t, hex.EncodeToString(ocrypto.CalculateSHA256Hmac(key, aggregate)), legacySig) +} + +// Closing the root off to GMAC must not move the defaults: HS256 root, GMAC +// segments, which is what every writer has been emitting all along. +func TestIntegrityAlgDefaults(t *testing.T) { + def, err := newTDFConfig() + require.NoError(t, err) + assert.Equal(t, RootHS256, def.rootIntegrityAlg) + assert.Equal(t, SegmentGMAC, def.segmentIntegrityAlg) +} + +// 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)) +} + func TestCreatePolicyBinding(t *testing.T) { symKey := make([]byte, kKeySize) _, err := rand.Read(symKey) diff --git a/sdk/tdf_readat_test.go b/sdk/tdf_readat_test.go index 4d44022839..7d6585fd8a 100644 --- a/sdk/tdf_readat_test.go +++ b/sdk/tdf_readat_test.go @@ -88,7 +88,7 @@ func newNonUniformReader(t *testing.T, sizes []int) (*Reader, []byte) { body.Write(hdr) body.Write(cipherText) - sig, err := calculateSignature(cipherText, key, HS256, false) + sig, err := segmentIntegrity(cipherText, key, HS256, false) require.NoError(t, err) segments = append(segments, Segment{ diff --git a/sdk/tdferrors.go b/sdk/tdferrors.go index 428b6dbbb2..0a9299fb34 100644 --- a/sdk/tdferrors.go +++ b/sdk/tdferrors.go @@ -23,6 +23,15 @@ var ( ErrRootSignatureFailure = fmt.Errorf("[%w] tdf: issue verifying root signature", ErrTampered) ErrRewrapBadRequest = fmt.Errorf("[%w] tdf: rewrap request 400", ErrTampered) + // ErrUnsupportedRootIntegrityAlgorithm rejects any root signature algorithm + // other than HS256, on both the write and the read path. + 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") + // kasGenericBadRequest is the substring the SDK looks for in serialized // KAS 400 errors to identify potential tamper. KAS uses the generic message // "bad request" for errors involving secret key material (policy binding,