From 9318a771eb50c9a7fe3f5080f7b679c6bd862189 Mon Sep 17 00:00:00 2001 From: Dave Mihalcik Date: Mon, 31 Aug 2026 22:23:01 -0400 Subject: [PATCH 1/2] chore(sdk): rewrite CreateTDF on top of the chunked writer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CreateTDFContext and the chunked writer had grown two full implementations of the same thing: build a policy, split the DEK, wrap each share to its KAS, encrypt segments, accumulate an aggregate hash, sign assertions, emit a manifest. Two copies of TDF construction is one too many — every spec change has to land twice, and the second copy is the one that gets forgotten. CreateTDF now delegates. It keeps the parts that are genuinely its own — the KAO template, autoconfigure, the input-size resolution and read loop from the previous commit — and hands each segment to the chunked writer, which owns manifest assembly for both paths from here on. The two paths differ in when key access is resolved, so that is what the writer is now parameterized on. The chunked writer defers to a KeySplitter at Finalize, because a caller may still be adding attributes while segments are in flight. CreateTDF cannot: it knows its attributes up front and wants an unreachable KAS to fail the call before a single payload byte reaches the output writer. Both are expressed as a keyAccessResolver, with the DEK now injectable so CreateTDF can wrap it ahead of time and hand the writer a staticKeyAccess. Everything downstream of that split is shared: resolvePolicyAndKeyAccess and buildKeyAccessObjects replace prepareManifest's inline loop and the chunked writer's buildChunkedPolicy/buildChunkedKeyAccessObjects, so both paths now emit byte-identical policy and key access for the same attributes. The splitter-contract checks move with them into splitterKeyAccess.resolve, which is now the single place a third-party KeySplitter's output is validated and verified to XOR back to the DEK before anything is wrapped. Two things fall out of the unification, both moving the chunked writer onto the shipped classic behavior: - With zero attributes the policy body's "dataAttributes" and "dissem" are now null rather than []. createPolicyObjectFromFQNs initializes them inside the attribute loop; the deleted buildChunkedPolicy did so unconditionally. The classic path has always emitted null here. - A KAS named by a split but missing a public key is still rejected rather than skipped (the check moved into buildKeyAccessObjects), but the error now names the missing PEM rather than the absent map entry — the two cases were indistinguishable in practice and only the outcome matters. The writer also gained an explicit segment size. It previously reported the first segment's actual length as defaultSegmentSize, which is only correct when every segment is full; a single-segment TDF would advertise a short default. CreateTDF knows the configured size and now says so. TDFObject loses aesGcm and payloadKey, which only ever existed to carry state between prepareManifest and the encrypt loop. Deliberately not in this commit: removing enableEncryption, tdfFormat, and readActionName, which are dead but unrelated; they are a separate cleanup. Testing: the existing TDFSuite round trips pin byte-level output across integrity algorithms, segment sizes, target modes, and multi-KAS splits, and pass unchanged. Also verified against the streaming-input and input-size coverage added in the previous commit, the experimental chunked writer suite, and cross-module builds of examples, otdfctl, service, and tests-bdd. Signed-off-by: Dave Mihalcik wip: resolve conflict --- sdk/chunked_test.go | 45 +++--- sdk/chunked_writer.go | 164 +++++++------------- sdk/key_splitter.go | 209 +++++++++++++++++++++++-- sdk/tdf.go | 332 ++++++++++++++++------------------------ sdk/tdf_helpers_test.go | 10 +- 5 files changed, 412 insertions(+), 348 deletions(-) diff --git a/sdk/chunked_test.go b/sdk/chunked_test.go index 1a5db70a75..50f1c5447f 100644 --- a/sdk/chunked_test.go +++ b/sdk/chunked_test.go @@ -736,19 +736,17 @@ func TestChunkedECKeyAccess(t *testing.T) { for i := range dek { dek[i] = byte(i) } - splits := &SplitResult{ - KASPublicKeys: map[string]KASPublicKey{ - kasURL: { - Algorithm: ocrypto.EC256Key, - KID: "ec-kid", - PEM: pubPEM, - URL: kasURL, - }, - }, - Splits: []Split{{Data: dek, KASURLs: []string{kasURL}}}, - } + shares := []splitShare{{ + data: dek, + kases: []KASInfo{{ + URL: kasURL, + PublicKey: pubPEM, + KID: "ec-kid", + Algorithm: string(ocrypto.EC256Key), + }}, + }} - kaos, err := buildChunkedKeyAccessObjects(splits, dek, []byte(`{"uuid":"test"}`), "") + kaos, err := buildKeyAccessObjects(shares, `{"uuid":"test"}`, "") require.NoError(t, err) require.Len(t, kaos, 1) @@ -800,22 +798,21 @@ func TestChunkedKeyAccessRejectsShareWithNoKAS(t *testing.T) { const kasURL = "https://kas.example.com" dek := make([]byte, kKeySize) - splits := &SplitResult{ - KASPublicKeys: map[string]KASPublicKey{ - kasURL: { - Algorithm: ocrypto.EC256Key, - KID: "ec-kid", - PEM: pubPEM, + shares := []splitShare{ + { + id: "wrapped", + data: dek, + kases: []KASInfo{{ URL: kasURL, - }, - }, - Splits: []Split{ - {ID: "wrapped", Data: dek, KASURLs: []string{kasURL}}, - {ID: "orphaned", Data: dek}, + PublicKey: pubPEM, + KID: "ec-kid", + Algorithm: string(ocrypto.EC256Key), + }}, }, + {id: "orphaned", data: dek}, } - _, err = buildChunkedKeyAccessObjects(splits, dek, []byte(`{"uuid":"test"}`), "") + _, err = buildKeyAccessObjects(shares, `{"uuid":"test"}`, "") 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") } diff --git a/sdk/chunked_writer.go b/sdk/chunked_writer.go index cdd04bbc3e..fa39b3c48f 100644 --- a/sdk/chunked_writer.go +++ b/sdk/chunked_writer.go @@ -13,7 +13,6 @@ import ( "sync" "time" - "github.com/google/uuid" "github.com/opentdf/platform/lib/ocrypto" "github.com/opentdf/platform/protocol/go/policy" "github.com/opentdf/platform/sdk/internal/zipstream" @@ -362,6 +361,11 @@ type chunkedWriterConfig struct { // fixedClock for deterministic ZIP output. clock clock + // dek is a pre-generated Data Encryption Key. When nil the writer + // draws one from rand. SDK.CreateTDF presets it so that it can + // resolve key access before emitting any payload bytes. + dek []byte + // excludeVersion omits the schemaVersion field from the manifest. // Set together with useHex by WithChunkedTargetMode; readers use // the field's absence as the pre-4.3.0 marker, so the two must @@ -376,12 +380,22 @@ type chunkedWriterConfig struct { // Finalize call does not supply its own. initialDefaultKAS *policy.SimpleKasKey + // keyAccess resolves the manifest policy and key access objects. + // Defaults to a splitterKeyAccess over splitter. + keyAccess keyAccessResolver + // rand is the entropy source used to generate the DEK. Defaults // to crypto/rand.Reader. rand io.Reader + // segmentSize is the plaintext segment size advertised in the + // manifest. Zero means "report the first segment's actual size", + // which is right when every segment is the same length. + segmentSize int64 + // splitter maps attribute values to DEK splits at Finalize time. - // Defaults to DefaultKeySplitter (single-KAS only). + // Defaults to DefaultKeySplitter (single-KAS only). Ignored when + // keyAccess is set. splitter KeySplitter // useHex hex-encodes segment, root, and assertion signatures @@ -498,6 +512,10 @@ type chunkedWriter struct { // when the caller does not override. initialDefaultKAS *policy.SimpleKasKey + // keyAccess resolves the manifest policy and key access objects + // for the DEK. + keyAccess keyAccessResolver + // manifest holds the finalized manifest for post-Finalize // GetManifest calls. manifest *Manifest @@ -508,9 +526,9 @@ type chunkedWriter struct { // segments records per-index slots, reserved and then written. segments map[int]*segmentSlot - // splitter converts attributes + DEK into key splits at - // Finalize time. - splitter KeySplitter + // segmentSize is the plaintext segment size to advertise in the + // manifest, or zero to infer it from the first segment. + segmentSize int64 // useHex selects the pre-4.3.0 doubly-encoded signature form. // Read by WriteSegment, so it is fixed at construction rather @@ -552,15 +570,30 @@ func NewChunkedWriter(_ context.Context, opts ...ChunkedWriterOption) (ChunkedWr return nil, err } } + return newChunkedWriter(cfg) +} - dek := make([]byte, kKeySize) - if _, err := io.ReadFull(cfg.rand, dek); err != nil { - return nil, fmt.Errorf("generate DEK: %w", err) +// newChunkedWriter builds the writer from a fully-populated config. +// SDK.CreateTDF calls this directly with the unexported knobs its +// classic behavior needs — a preset DEK, key access resolved before +// the first payload byte, a fixed segment size — rather than going +// through the public option set. +func newChunkedWriter(cfg chunkedWriterConfig) (*chunkedWriter, error) { + dek := cfg.dek + if dek == nil { + dek = make([]byte, kKeySize) + if _, err := io.ReadFull(cfg.rand, dek); err != nil { + return nil, fmt.Errorf("generate DEK: %w", err) + } } block, err := cfg.cipherFactory(dek) if err != nil { return nil, fmt.Errorf("build segment cipher: %w", err) } + keyAccess := cfg.keyAccess + if keyAccess == nil { + keyAccess = splitterKeyAccess{splitter: cfg.splitter} + } return &chunkedWriter{ archiveWriter: cfg.archiveFactory(cfg.clock), block: block, @@ -568,8 +601,9 @@ func NewChunkedWriter(_ context.Context, opts ...ChunkedWriterOption) (ChunkedWr excludeVersion: cfg.excludeVersion, initialAttributes: cfg.initialAttributes, initialDefaultKAS: cfg.initialDefaultKAS, + keyAccess: keyAccess, segments: make(map[int]*segmentSlot), - splitter: cfg.splitter, + segmentSize: cfg.segmentSize, useHex: cfg.useHex, }, nil } @@ -1003,36 +1037,16 @@ type chunkedTotals struct { plaintext int64 } -// buildManifest composes the manifest from a snapshot, splits the DEK, -// wraps splits into KAOs, and computes the root signature. +// buildManifest composes the manifest from a snapshot, resolves the key +// access objects, and computes the root signature. // // It reads no mutable writer state and takes no lock: every other field it -// touches (dek, splitter, useHex) is fixed at construction. Keep it that way -- +// touches (dek, keyAccess, useHex) is fixed at construction. Keep it that way -- // GetManifest calls it with the read lock released. func (w *chunkedWriter) buildManifest(ctx context.Context, cfg *chunkedFinalizeConfig, snap *chunkedSnapshot) (*Manifest, chunkedTotals, error) { var totals chunkedTotals - // Hand the splitter copies of both slices. A splitter that zeroes or - // rewrites the DEK it is given -- scrubbing what it thinks is its own - // working buffer, say -- would desynchronize the DEK from the segment - // signatures already computed against it and from the root signature - // computed below, producing a TDF that fails verification exactly as a - // tampered one would. - // - // The attributes need the same defense for a different reason: on the - // fallback path cfg.attributes *is* w.initialAttributes, the writer's own - // retained slice, so a splitter that rewrites an element changes the policy - // buildChunkedPolicy writes just below -- and every later Finalize and - // GetManifest on this writer. - splits, err := w.splitter.Split(ctx, slices.Clone(cfg.attributes), slices.Clone(w.dek), cfg.defaultKAS) - if err != nil { - return nil, totals, err - } - policyBytes, err := buildChunkedPolicy(cfg.attributes) - if err != nil { - return nil, totals, err - } - kaos, err := buildChunkedKeyAccessObjects(splits, w.dek, policyBytes, cfg.encryptedMetadata) + base64Policy, kaos, err := w.keyAccess.resolve(ctx, w.dek, cfg) if err != nil { return nil, totals, err } @@ -1040,7 +1054,7 @@ func (w *chunkedWriter) buildManifest(ctx context.Context, cfg *chunkedFinalizeC encInfo := EncryptionInformation{ KeyAccessObjs: kaos, KeyAccessType: kSplitKeyType, - Policy: string(ocrypto.Base64Encode(policyBytes)), + Policy: base64Policy, Method: Method{ Algorithm: kGCMCipherAlgorithm, IsStreamable: true, @@ -1066,7 +1080,15 @@ func (w *chunkedWriter) buildManifest(ctx context.Context, cfg *chunkedFinalizeC aggregate.Write(decoded) } - if len(snap.segments) > 0 { + // A caller that knows the segment size says so, because the first + // segment's actual length is only the right answer when every + // segment is full — and the last one usually is not, so a + // single-segment TDF would otherwise advertise a short default. + switch { + case w.segmentSize > 0: + encInfo.DefaultSegmentSize = w.segmentSize + encInfo.DefaultEncryptedSegSize = w.segmentSize + gcmIvSize + aesBlockSize + case len(snap.segments) > 0: encInfo.DefaultEncryptedSegSize = snap.segments[0].EncryptedSize encInfo.DefaultSegmentSize = snap.segments[0].Size } @@ -1165,78 +1187,6 @@ func (w *chunkedWriter) segmentOrderLocked(keep []int) ([]int, error) { return out, nil } -// buildChunkedKeyAccessObjects wraps each split share to each KAS -// listed by the splitter. -func buildChunkedKeyAccessObjects(splits *SplitResult, dek, policyBytes []byte, metadata string) ([]KeyAccess, error) { - // This is the one place caller-supplied split data is turned into - // manifest content, so it is where the splitter's contract is - // enforced -- for the default splitter and for anything injected - // through WithChunkedKeySplitter alike. Validate guarantees every - // invariant the loop below relies on: at least one split, every - // split naming at least one KAS, every named KAS resolving to a - // usable wrapping key, and split ids that the reader can group on. - if err := splits.Validate(); err != nil { - return nil, err - } - // Validate first, then this: Validate names the specific structural fault - // (no splits, empty share, duplicate id, unresolved KAS), all of which - // VerifyReconstruction can only report as a length or value mismatch. - if err := splits.VerifyReconstruction(dek); err != nil { - return nil, err - } - base64Policy := ocrypto.Base64Encode(policyBytes) - - out := make([]KeyAccess, 0, len(splits.Splits)) - for _, split := range splits.Splits { - // Policy binding and metadata are keyed on the split share, not - // on the KAS, so compute them once per split rather than once - // per KAS URL in an OR-group. - policyBinding := createPolicyBinding(split.Data, base64Policy) - var encMeta string - if metadata != "" { - m, err := encryptMetadata(split.Data, metadata) - if err != nil { - return nil, fmt.Errorf("encrypt metadata for split %s: %w", split.ID, err) - } - encMeta = m - } - for _, url := range split.KASURLs { - // Validate resolved every URL, so the lookup cannot miss. - pk := splits.KASPublicKeys[url] - kao, err := createKeyAccess(pk.toKASInfo(), split.Data, policyBinding, encMeta, split.ID) - if err != nil { - return nil, fmt.Errorf("wrap key for %s: %w", url, err) - } - out = append(out, kao) - } - } - return out, nil -} - -// buildChunkedPolicy composes the TDF Policy document from attribute -// values. -func buildChunkedPolicy(values []*policy.Value) ([]byte, error) { - p := PolicyObject{UUID: uuid.NewString()} - p.Body.DataAttributes = make([]attributeObject, 0, len(values)) - p.Body.Dissem = make([]string, 0) - for i, v := range values { - // GetFqn is nil-receiver safe, so a nil element or a value that never - // had its FQN populated yields "" rather than panicking. Emitting that - // writes {"attribute": ""} into the bound policy: the PDP denies it, so - // the TDF fails closed rather than open, but it fails at decrypt with - // nothing naming the cause -- and it does so after the caller has - // encrypted and uploaded the whole payload. - fqn := v.GetFqn() - if fqn == "" { - return nil, fmt.Errorf("chunked: attribute value at index %d has an empty FQN; the policy it produces names no attribute and no KAS could evaluate it", i) - } - p.Body.DataAttributes = append(p.Body.DataAttributes, attributeObject{ - Attribute: fqn, - }) - } - return json.Marshal(p) -} - // cloneChunkedManifest copies the manifest struct and clones its three // slices, which is what makes the result safe to hand to a caller: it // can append to or overwrite elements of Segments, KeyAccessObjs and diff --git a/sdk/key_splitter.go b/sdk/key_splitter.go index 65c7e3891d..95991f8a0b 100644 --- a/sdk/key_splitter.go +++ b/sdk/key_splitter.go @@ -3,8 +3,11 @@ package sdk import ( "context" "crypto/subtle" + "encoding/json" "errors" "fmt" + "io" + "slices" "github.com/opentdf/platform/lib/ocrypto" "github.com/opentdf/platform/protocol/go/policy" @@ -234,18 +237,6 @@ func (k KASPublicKey) validate(mapKey string) error { return nil } -// toKASInfo adapts the splitter's wrapping-key descriptor to the -// KASInfo shape consumed by createKeyAccess. Default is not carried -// over; it plays no part in building a key access object. -func (k KASPublicKey) toKASInfo() KASInfo { - return KASInfo{ - URL: k.URL, - PublicKey: k.PEM, - KID: k.KID, - Algorithm: string(k.Algorithm), - } -} - // ErrSplitterRequiresDefaultKAS is returned by the default key // splitter when no default KAS was supplied. The default splitter is // single-KAS only; multi-attribute splits require injecting a full @@ -320,3 +311,197 @@ func (s *singleKASSplitter) Split(_ context.Context, _ []*policy.Value, dek []by }}, }, nil } + +// keyAccessResolver turns a DEK into the two manifest fields that +// bind it to policy: the base64-encoded policy object and the key +// access objects wrapping the DEK to each KAS. The writer holds one; +// SDK.CreateTDF resolves its key access up front and supplies a +// staticKeyAccess, while the chunked path defers to a KeySplitter at +// Finalize time. +type keyAccessResolver interface { + resolve(ctx context.Context, dek []byte, cfg *chunkedFinalizeConfig) (string, []KeyAccess, error) +} + +// splitShare is one XOR share of the DEK together with every KAS able +// to unwrap it. Several KAS entries on one share mean any of them +// suffices (OR semantics); several shares mean all are required (AND). +type splitShare struct { + // id names the share in the manifest ("sid"). Empty when the TDF + // has a single share. + id string + + // data is the share itself. + data []byte + + // kases are the wrapping targets for this share. + kases []KASInfo +} + +// staticKeyAccess returns key access objects resolved ahead of time. +type staticKeyAccess struct { + // kaos are the pre-built key access objects. + kaos []KeyAccess + + // policy is the base64-encoded policy object the kaos are bound to. + policy string +} + +func (r staticKeyAccess) resolve(_ context.Context, _ []byte, _ *chunkedFinalizeConfig) (string, []KeyAccess, error) { + return r.policy, r.kaos, nil +} + +// splitterKeyAccess adapts a public KeySplitter to keyAccessResolver. +type splitterKeyAccess struct { + // splitter maps attributes plus the DEK onto KAS-addressed shares. + splitter KeySplitter +} + +func (r splitterKeyAccess) resolve(ctx context.Context, dek []byte, cfg *chunkedFinalizeConfig) (string, []KeyAccess, error) { + // Hand the splitter copies of both slices. A splitter that zeroes or + // rewrites the DEK it is given -- scrubbing what it thinks is its own + // working buffer, say -- would desynchronize the DEK from the segment + // signatures already computed against it and from the root signature still + // to come, producing a TDF that fails verification exactly as a tampered + // one would. + // + // The attributes need the same defense for a different reason: on the + // fallback path cfg.attributes *is* the writer's retained initialAttributes + // slice, so a splitter that rewrites an element changes the policy built + // from it just below -- and every later Finalize and GetManifest on that + // writer. + splits, err := r.splitter.Split(ctx, slices.Clone(cfg.attributes), slices.Clone(dek), cfg.defaultKAS) + if err != nil { + return "", nil, err + } + // This is the one place caller-supplied split data is turned into + // manifest content, so it is where the splitter's contract is + // enforced -- for the default splitter and for anything injected + // through WithChunkedKeySplitter alike. Validate guarantees every + // invariant the loop below relies on: at least one split, every + // split naming at least one KAS, every named KAS resolving to a + // usable wrapping key, and split ids that the reader can group on. + if err := splits.Validate(); err != nil { + return "", nil, err + } + // Validate first, then this: Validate names the specific structural fault + // (no splits, empty share, duplicate id, unresolved KAS), all of which + // VerifyReconstruction can only report as a length or value mismatch. + if err := splits.VerifyReconstruction(dek); err != nil { + return "", nil, err + } + + shares := make([]splitShare, 0, len(splits.Splits)) + for _, split := range splits.Splits { + share := splitShare{id: split.ID, data: split.Data} + for _, url := range split.KASURLs { + // Validate resolved every URL, so the lookup cannot miss. + pk := splits.KASPublicKeys[url] + share.kases = append(share.kases, KASInfo{ + URL: url, + PublicKey: pk.PEM, + KID: pk.KID, + Algorithm: string(pk.Algorithm), + }) + } + shares = append(shares, share) + } + + fqns := make([]string, 0, len(cfg.attributes)) + for _, v := range cfg.attributes { + fqns = append(fqns, v.GetFqn()) + } + return resolvePolicyAndKeyAccess(fqns, shares, cfg.encryptedMetadata) +} + +// 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) { + policyObj, err := createPolicyObjectFromFQNs(fqns) + if err != nil { + return "", nil, fmt.Errorf("fail to create policy object:%w", err) + } + policyObjectAsStr, err := json.Marshal(policyObj) + if err != nil { + return "", nil, fmt.Errorf("json.Marshal failed:%w", err) + } + base64Policy := string(ocrypto.Base64Encode(policyObjectAsStr)) + + kaos, err := buildKeyAccessObjects(shares, base64Policy, metadata) + if err != nil { + return "", nil, err + } + return base64Policy, kaos, nil +} + +// 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) { + var kaos []KeyAccess + for _, share := range shares { + // A share with no KAS target contributes no key access object, + // so nothing in the manifest ever hands its bytes back. The + // reader derives the shares it must collect from the key access + // objects present, so it never learns one is missing: it XORs + // what it has, reconstructs the wrong DEK, and reports a root + // signature failure, which reads as tampering. The aggregate + // check below cannot catch this -- it passes as soon as any + // other share produced a KAO. + if len(share.kases) == 0 { + return nil, fmt.Errorf("splitID:[%s]: share names no KAS; its bytes could never be unwrapped", share.id) + } + + // 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) + + var encryptedMetadata string + if metadata != "" { + var err error + encryptedMetadata, err = encryptMetadata(share.data, metadata) + if err != nil { + return nil, err + } + } + + for _, kasInfo := range share.kases { + if kasInfo.PublicKey == "" { + return nil, fmt.Errorf("splitID:[%s], kas:[%s]: %w", share.id, kasInfo.URL, errKasPubKeyMissing) + } + keyAccess, err := createKeyAccess(kasInfo, share.data, policyBinding, encryptedMetadata, share.id) + if err != nil { + return nil, err + } + kaos = append(kaos, keyAccess) + } + } + if len(kaos) == 0 { + return nil, errors.New("no key access objects generated") + } + return kaos, nil +} + +// splitDEK returns count XOR shares of dek. Every share but the last +// is random; the last absorbs the parity so the shares XOR back to dek. +func splitDEK(dek []byte, count int, rand io.Reader) ([][]byte, error) { + if count <= 0 { + return nil, errors.New("no key splits requested") + } + shares := make([][]byte, count) + parity := make([]byte, len(dek)) + copy(parity, dek) + for i := range count - 1 { + share := make([]byte, len(dek)) + if _, err := io.ReadFull(rand, share); err != nil { + return nil, fmt.Errorf("generate key split failed: %w", err) + } + for j, b := range share { + parity[j] ^= b + } + shares[i] = share + } + shares[count-1] = parity + return shares, nil +} diff --git a/sdk/tdf.go b/sdk/tdf.go index 44b37188ee..552605e51b 100644 --- a/sdk/tdf.go +++ b/sdk/tdf.go @@ -3,16 +3,17 @@ package sdk import ( "bytes" "context" + "crypto/rand" "crypto/sha256" "encoding/hex" "encoding/json" "errors" "fmt" - "hash/crc32" "io" "log/slog" "math" "net/http" + "slices" "strconv" "strings" @@ -89,10 +90,8 @@ type RequiredObligations struct { } type TDFObject struct { - manifest Manifest - size int64 - aesGcm ocrypto.AesGcm - payloadKey [kKeySize]byte + manifest Manifest + size int64 } type countingWriter struct { @@ -216,12 +215,6 @@ func (s SDK) CreateTDFContext(ctx context.Context, writer io.Writer, reader io.R return nil, err } - tdfObject := &TDFObject{} - err = s.prepareManifest(ctx, tdfObject, *tdfConfig) - if err != nil { - return nil, fmt.Errorf("fail to create a new split key: %w", err) - } - segmentSize := tdfConfig.defaultSegmentSize if segmentSize > maxSegmentSize { return nil, fmt.Errorf("segment size too large: %d", segmentSize) @@ -229,8 +222,6 @@ func (s SDK) CreateTDFContext(ctx context.Context, writer io.Writer, reader io.R return nil, fmt.Errorf("segment size too small: %d", segmentSize) } - encryptedSegmentSize := segmentSize + gcmIvSize + aesBlockSize - // These describe a payload whose length cannot be known before it is read: no // segment count can be declared up front, the archive has to assume it may need // 64-bit offsets, and every read asks for a whole segment. A measurable payload @@ -272,18 +263,16 @@ func (s SDK) CreateTDFContext(ctx context.Context, writer io.Writer, reader io.R readBufSize = max(1, min(segmentSize, inputSize)) } - archiveOpts := []zipstream.Option{zipstream.WithZip64Mode(zipMode)} - if declaredSegments > 0 { - archiveOpts = append(archiveOpts, zipstream.WithMaxSegments(declaredSegments)) + chunked, err := s.newTDFChunkedWriter(ctx, tdfConfig, zipMode, declaredSegments) + if err != nil { + return nil, err } - archiveWriter := zipstream.NewSegmentTDFWriter(declaredSegments, archiveOpts...) outputWriter := &countingWriter{writer: writer} - var aggregateHashBuilder strings.Builder var bytesRead int64 readBuf := make([]byte, readBufSize) - for segmentIndex := 0; ; segmentIndex++ { + for index := 0; ; index++ { // io.Reader.Read is free to return fewer bytes than asked for without // erroring, so a bare Read would cut segments short at the whim of the // reader. ReadFull retries until the segment is filled or the input runs @@ -294,50 +283,19 @@ func (s SDK) CreateTDFContext(ctx context.Context, writer io.Writer, reader io.R } // A payload whose length is an exact multiple of the segment size reports // EOF with nothing read. An empty payload still gets one empty segment. - if n == 0 && segmentIndex > 0 { + if n == 0 && index > 0 { break } - readSize := int64(n) - bytesRead += readSize + bytesRead += int64(n) - cipherData, err := tdfObject.aesGcm.Encrypt(readBuf[:readSize]) + segment, err := chunked.WriteSegment(ctx, index, readBuf[:n]) if err != nil { - return nil, fmt.Errorf("ocrypto.AesGcm.Encrypt failed: %w", err) - } - - crc := crc32.ChecksumIEEE(cipherData) - headerBytes, err := archiveWriter.WriteSegment(ctx, segmentIndex, uint64(len(cipherData)), crc) - if err != nil { - return nil, fmt.Errorf("zipstream.WriteSegment failed: %w", err) - } - - if len(headerBytes) > 0 { - _, err = outputWriter.Write(headerBytes) - if err != nil { - return nil, fmt.Errorf("io.writer.Write failed: %w", err) - } + return nil, err } - - _, err = outputWriter.Write(cipherData) - if err != nil { + if _, err := io.Copy(outputWriter, segment.TDFData); err != nil { return nil, fmt.Errorf("io.writer.Write failed: %w", err) } - segmentSig, err := segmentIntegrity(cipherData, tdfObject.payloadKey[:], - tdfConfig.segmentIntegrityAlg, tdfConfig.useHex) - if err != nil { - return nil, fmt.Errorf("splitKey.GetSignaturefailed: %w", err) - } - - aggregateHashBuilder.WriteString(segmentSig) - segmentInfo := Segment{ - Hash: string(ocrypto.Base64Encode([]byte(segmentSig))), - Size: readSize, - EncryptedSize: int64(len(cipherData)), - } - - tdfObject.manifest.Segments = append(tdfObject.manifest.Segments, segmentInfo) - if readErr != nil { break } @@ -351,79 +309,88 @@ func (s SDK) CreateTDFContext(ctx context.Context, writer io.Writer, reader io.R return nil, fmt.Errorf("%w: read %d of %d bytes", errInputShorterThanDeclared, bytesRead, declaredSize) } - rootSignature, err := rootIntegrity([]byte(aggregateHashBuilder.String()), tdfObject.payloadKey[:], - tdfConfig.rootIntegrityAlg, tdfConfig.useHex) + finalizeOpts, err := tdfFinalizeOptions(tdfConfig) if err != nil { - return nil, fmt.Errorf("splitKey.GetSignaturefailed: %w", err) + return nil, err } - sig := string(ocrypto.Base64Encode([]byte(rootSignature))) - tdfObject.manifest.Signature = sig + result, err := chunked.Finalize(ctx, finalizeOpts...) + if err != nil { + return nil, err + } - // 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() + if _, err := outputWriter.Write(result.Data); err != nil { + return nil, fmt.Errorf("io.writer.Write failed: %w", err) + } - tdfObject.manifest.DefaultSegmentSize = segmentSize - tdfObject.manifest.DefaultEncryptedSegSize = encryptedSegmentSize + return &TDFObject{ + manifest: *result.Manifest, + size: outputWriter.written, + }, nil +} - tdfObject.manifest.SegmentHashAlgorithm = tdfConfig.segmentIntegrityAlg.String() - tdfObject.manifest.Method.IsStreamable = true +// newTDFChunkedWriter builds the chunked writer backing SDK.CreateTDF. Key access is +// resolved here, before any payload byte is written, so that an unreachable KAS fails +// the call without leaving a partial TDF on the output writer. +// +// The ZIP64 mode and segment count are decided by the caller, which is the only place +// that knows whether the payload could be measured: declaredSegments is zero, and +// zipMode zipstream.Zip64Always, for a payload that can only be measured by reading it. +func (s SDK) newTDFChunkedWriter(ctx context.Context, tdfConfig *TDFConfig, zipMode zipstream.Zip64Mode, declaredSegments int) (*chunkedWriter, error) { + dek := make([]byte, kKeySize) + if _, err := io.ReadFull(rand.Reader, dek); err != nil { + return nil, fmt.Errorf("fail to create a new split key: %w", err) + } - // add payload info - mimeType := tdfConfig.mimeType - if mimeType == "" { - mimeType = defaultMimeType + base64Policy, kaos, err := s.resolveKeyAccess(ctx, tdfConfig, dek) + if err != nil { + return nil, fmt.Errorf("fail to create a new split key: %w", err) } - tdfObject.manifest.MimeType = mimeType - tdfObject.manifest.Protocol = tdfAsZip - tdfObject.manifest.Type = tdfZipReference - tdfObject.manifest.URL = zipstream.TDFPayloadFileName - tdfObject.manifest.IsEncrypted = true + return newChunkedWriter(chunkedWriterConfig{ + archiveFactory: func(c clock) zipstream.SegmentWriter { + archiveOpts := []zipstream.Option{ + zipstream.WithZip64Mode(zipMode), + zipstream.WithClock(c.Now), + } + if declaredSegments > 0 { + archiveOpts = append(archiveOpts, zipstream.WithMaxSegments(declaredSegments)) + } + return zipstream.NewSegmentTDFWriter(declaredSegments, archiveOpts...) + }, + cipherFactory: defaultSegmentCipherFactory, + clock: systemClock{}, + dek: dek, + // Set together, at construction, because they have to be: hex-then-base64 + // signatures are written during WriteSegment, long before any Finalize + // option is seen, and a manifest carrying one setting without the other + // cannot be verified by any reader. TDFConfig sets its two the same way. + excludeVersion: tdfConfig.excludeVersionFromManifest, + keyAccess: staticKeyAccess{kaos: kaos, policy: base64Policy}, + segmentSize: tdfConfig.defaultSegmentSize, + useHex: tdfConfig.useHex, + }) +} + +// tdfFinalizeOptions maps the manifest-shaping parts of a TDFConfig onto the chunked +// writer's Finalize options. Encrypted metadata is absent here on purpose: the chunked +// writer only consults it when a KeySplitter builds the key access objects at Finalize +// time, and this path resolved them up front with the metadata already applied. +func tdfFinalizeOptions(tdfConfig *TDFConfig) ([]ChunkedFinalizeOption, error) { + assertions := slices.Clone(tdfConfig.assertions) if tdfConfig.addDefaultAssertion { systemMeta, err := GetSystemMetadataAssertionConfig() if err != nil { return nil, err } - tdfConfig.assertions = append(tdfConfig.assertions, systemMeta) - } - - signedAssertion, err := signAssertions( - []byte(aggregateHashBuilder.String()), - tdfConfig.assertions, - tdfObject.payloadKey[:], - tdfConfig.useHex, - ) - if err != nil { - return nil, err - } - - tdfObject.manifest.Assertions = signedAssertion - - manifestAsStr, err := json.Marshal(tdfObject.manifest) - if err != nil { - return nil, fmt.Errorf("json.Marshal failed:%w", err) + assertions = append(assertions, systemMeta) } - finalBytes, err := archiveWriter.Finalize(ctx, manifestAsStr) - if err != nil { - return nil, fmt.Errorf("zipstream.Finalize failed: %w", err) + opts := []ChunkedFinalizeOption{WithChunkedAssertions(assertions)} + if tdfConfig.mimeType != "" { + opts = append(opts, WithChunkedMimeType(tdfConfig.mimeType)) } - - _, err = outputWriter.Write(finalBytes) - if err != nil { - return nil, fmt.Errorf("io.writer.Write failed: %w", err) - } - - if err := archiveWriter.Close(); err != nil { - return nil, fmt.Errorf("zipstream.Close failed: %w", err) - } - - tdfObject.size = outputWriter.written - - return tdfObject, nil + return opts, nil } // resolveInputSize reports the payload length in bytes, or inputSizeUnknown when it @@ -591,37 +558,34 @@ func (r *Reader) Manifest() Manifest { return r.manifest } -// prepare the manifest for TDF -func (s SDK) prepareManifest(ctx context.Context, t *TDFObject, tdfConfig TDFConfig) error { //nolint:funlen,gocognit // Better readability keeping it as is - manifest := Manifest{} - - if !tdfConfig.excludeVersionFromManifest { - manifest.TDFVersion = TDFSpecVersion - } - - if len(tdfConfig.kaoTemplate) == 0 { - return fmt.Errorf("no key access template specified or inferred in initKAOTemplate: %w", errInvalidKasInfo) - } - - manifest.KeyAccessType = kSplitKeyType - - policyObj, err := createPolicyObject(tdfConfig.attributes) +// resolveKeyAccess builds the two manifest fields that bind the DEK to policy for the +// classic CreateTDF path: the base64-encoded policy object and the key access objects. +// The KAO template says which KAS servers hold which split; the DEK is divided among +// those splits and each share wrapped to the KAS servers assigned to it. +func (s SDK) resolveKeyAccess(ctx context.Context, tdfConfig *TDFConfig, dek []byte) (string, []KeyAccess, error) { + shares, err := s.templateSplitShares(ctx, tdfConfig, dek) if err != nil { - return fmt.Errorf("fail to create policy object:%w", err) + return "", nil, err } - policyObjectAsStr, err := json.Marshal(policyObj) - if err != nil { - return fmt.Errorf("json.Marshal failed:%w", err) + fqns := make([]string, 0, len(tdfConfig.attributes)) + for _, attribute := range tdfConfig.attributes { + fqns = append(fqns, attribute.String()) } + return resolvePolicyAndKeyAccess(fqns, shares, tdfConfig.metaData) +} - base64PolicyObject := ocrypto.Base64Encode(policyObjectAsStr) - - conjunction := make(map[string][]KASInfo) - var splitIDs []string +// templateSplitShares groups the KAO template by split ID and divides the DEK across +// the resulting shares. KAS entries sharing a split ID form an OR-group: any one of +// them can unwrap that share. Public keys absent from the template are fetched here. +func (s SDK) templateSplitShares(ctx context.Context, tdfConfig *TDFConfig, dek []byte) ([]splitShare, error) { + if len(tdfConfig.kaoTemplate) == 0 { + return nil, fmt.Errorf("no key access template specified or inferred in initKAOTemplate: %w", errInvalidKasInfo) + } + var shares []splitShare + index := map[string]int{} for _, tpl := range tdfConfig.kaoTemplate { - // Public key was passed in with kasInfoList ki := KASInfo{ URL: tpl.KAS, KID: tpl.kid, @@ -635,71 +599,29 @@ func (s SDK) prepareManifest(ctx context.Context, t *TDFObject, tdfConfig TDFCon } k, err := s.getPublicKey(ctx, tpl.KAS, a, tpl.kid) if err != nil { - return fmt.Errorf("unable to retrieve public key from KAS at [%s]: %w", tpl.KAS, err) + return nil, fmt.Errorf("unable to retrieve public key from KAS at [%s]: %w", tpl.KAS, err) } ki = *k } - if _, ok := conjunction[tpl.SplitID]; ok { - conjunction[tpl.SplitID] = append(conjunction[tpl.SplitID], ki) - } else { - conjunction[tpl.SplitID] = []KASInfo{ki} - splitIDs = append(splitIDs, tpl.SplitID) - } - } - - symKeys := make([][]byte, 0, len(splitIDs)) - for _, splitID := range splitIDs { - symKey, err := ocrypto.RandomBytes(kKeySize) - if err != nil { - return fmt.Errorf("ocrypto.RandomBytes failed:%w", err) - } - symKeys = append(symKeys, symKey) - - // policy binding - policyBinding := createPolicyBinding(symKey, base64PolicyObject) - - // encrypted metadata - // add meta data - var encryptedMetadata string - if len(tdfConfig.metaData) > 0 { - encryptedMetadata, err = encryptMetadata(symKey, tdfConfig.metaData) - if err != nil { - return err - } - } - - for _, kasInfo := range conjunction[splitID] { - if len(kasInfo.PublicKey) == 0 { - return fmt.Errorf("splitID:[%s], kas:[%s]: %w", splitID, kasInfo.URL, errKasPubKeyMissing) - } - - keyAccess, err := createKeyAccess(kasInfo, symKey, policyBinding, encryptedMetadata, splitID) - if err != nil { - return err - } - - manifest.KeyAccessObjs = append(manifest.KeyAccessObjs, keyAccess) - } - } - - manifest.Policy = string(base64PolicyObject) - manifest.Method.Algorithm = kGCMCipherAlgorithm - - // create the payload key by XOR all the keys in key access object. - for _, symKey := range symKeys { - for keyByteIndex, keyByte := range symKey { - t.payloadKey[keyByteIndex] ^= keyByte + if i, ok := index[tpl.SplitID]; ok { + shares[i].kases = append(shares[i].kases, ki) + continue } + index[tpl.SplitID] = len(shares) + shares = append(shares, splitShare{id: tpl.SplitID, kases: []KASInfo{ki}}) } - gcm, err := ocrypto.NewAESGcm(t.payloadKey[:]) + // The shares XOR back to the DEK, so every one of them is needed to reconstruct + // it — that is the AND half of the split semantics, with the OR half expressed by + // the several KAS servers that may sit on a single share. + data, err := splitDEK(dek, len(shares), rand.Reader) if err != nil { - return fmt.Errorf(" ocrypto.NewAESGcm failed:%w", err) + return nil, err } - - t.manifest = manifest - t.aesGcm = gcm - return nil + for i := range shares { + shares[i].data = data[i] + } + return shares, nil } // manifestSegmentIntegrityAlg maps the manifest's segmentHashAlg string onto @@ -718,10 +640,11 @@ func manifestSegmentIntegrityAlg(alg string) SegmentIntegrityAlg { return SegmentHS256 } -// 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)) +// 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))) return PolicyBinding{ Alg: hmacIntegrityAlgorithm, Hash: string(ocrypto.Base64Encode([]byte(policyBindingHash))), @@ -875,8 +798,8 @@ func generateWrapKeyWithKEM(ktype ocrypto.KeyType, publicKeyPEM string, symKey [ return string(ocrypto.Base64Encode(wrappedDER)), scheme, nil } -// create policy object -func createPolicyObject(attributes []AttributeValueFQN) (PolicyObject, error) { +// createPolicyObjectFromFQNs builds the TDF policy document from attribute value FQNs. +func createPolicyObjectFromFQNs(fqns []string) (PolicyObject, error) { uuidObj, err := uuid.NewUUID() if err != nil { return PolicyObject{}, fmt.Errorf("uuid.NewUUID failed: %w", err) @@ -885,9 +808,18 @@ func createPolicyObject(attributes []AttributeValueFQN) (PolicyObject, error) { policyObj := PolicyObject{} policyObj.UUID = uuidObj.String() - for _, attribute := range attributes { + for i, fqn := range fqns { + // An empty FQN reaches here from any caller that forwards an + // unpopulated attribute value -- policy.Value.GetFqn is nil-receiver + // safe, so a nil element yields "" rather than panicking. Emitting it + // writes {"attribute": ""} into the bound policy: the PDP denies it, so + // the TDF fails closed rather than open, but it fails at decrypt with + // nothing naming the cause, after the payload is encrypted and stored. + if fqn == "" { + return PolicyObject{}, fmt.Errorf("attribute at index %d has an empty FQN; the policy it produces names no attribute and no KAS could evaluate it", i) + } attributeObj := attributeObject{} - attributeObj.Attribute = attribute.String() + attributeObj.Attribute = fqn policyObj.Body.DataAttributes = append(policyObj.Body.DataAttributes, attributeObj) policyObj.Body.Dissem = make([]string, 0) } diff --git a/sdk/tdf_helpers_test.go b/sdk/tdf_helpers_test.go index 9cb1d50011..a4b6d46b76 100644 --- a/sdk/tdf_helpers_test.go +++ b/sdk/tdf_helpers_test.go @@ -141,7 +141,7 @@ func TestCreatePolicyBinding(t *testing.T) { fixedKey[i] = byte(i) } - binding := createPolicyBinding(fixedKey, ocrypto.Base64Encode([]byte(`{"uuid":"test"}`))) + binding := createPolicyBinding(fixedKey, string(ocrypto.Base64Encode([]byte(`{"uuid":"test"}`)))) assert.Equal(t, "YzFjZTM3OWQ0Y2FiMTZkNmRhNzJkYjllYWQ2NGQ3Y2I0Y2E5YmRhY2FiOGMwNjg1ZmY5MmUzZjc0YWEyYzEyZA==", @@ -149,7 +149,7 @@ func TestCreatePolicyBinding(t *testing.T) { }) t.Run("binds with HS256 over base64 policy", func(t *testing.T) { - binding := createPolicyBinding(symKey, ocrypto.Base64Encode([]byte(policyJSON))) + binding := createPolicyBinding(symKey, string(ocrypto.Base64Encode([]byte(policyJSON)))) assert.Equal(t, hmacIntegrityAlgorithm, binding.Alg) require.NotEmpty(t, binding.Hash) @@ -158,8 +158,8 @@ func TestCreatePolicyBinding(t *testing.T) { }) 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"}`))) + b1 := createPolicyBinding(symKey, string(ocrypto.Base64Encode([]byte(`{"policy":"test1"}`)))) + b2 := createPolicyBinding(symKey, string(ocrypto.Base64Encode([]byte(`{"policy":"test2"}`)))) assert.NotEqual(t, b1.Hash, b2.Hash) }) @@ -168,7 +168,7 @@ func TestCreatePolicyBinding(t *testing.T) { _, err := rand.Read(otherKey) require.NoError(t, err) - policy := ocrypto.Base64Encode([]byte(policyJSON)) + policy := string(ocrypto.Base64Encode([]byte(policyJSON))) assert.NotEqual(t, createPolicyBinding(symKey, policy).Hash, createPolicyBinding(otherKey, policy).Hash, From 0a6f0c719f68fd75047500ac5ed9fc2bfd61bc2b Mon Sep 17 00:00:00 2001 From: Dave Mihalcik Date: Thu, 24 Sep 2026 11:48:42 -0400 Subject: [PATCH 2/2] Apply suggestion from @dmihalcik-virtru --- sdk/key_splitter.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/key_splitter.go b/sdk/key_splitter.go index 95991f8a0b..232e4802ff 100644 --- a/sdk/key_splitter.go +++ b/sdk/key_splitter.go @@ -337,7 +337,7 @@ type splitShare struct { kases []KASInfo } -// staticKeyAccess returns key access objects resolved ahead of time. +// staticKeyAccess returns key access objects resolved ahead of time type staticKeyAccess struct { // kaos are the pre-built key access objects. kaos []KeyAccess