From bb179b889a1cbd986cc7c9990d9621aa0eb3875a Mon Sep 17 00:00:00 2001 From: Dave Mihalcik Date: Mon, 31 Aug 2026 22:41:00 -0400 Subject: [PATCH] feat(sdk): graduate the chunked writer to stable API The chunked writer has been the sole implementation of TDF construction since CreateTDF was rewritten on top of it. Every TDF the SDK produces already goes through this code; the "Experimental: not part of the stable SDK API; may change or be removed" markers on it stopped being true at that point. Drop them, and give the writer the constructor it was missing. SDK.NewChunkedWriter resolves key access against the platform the SDK is connected to, running attributes through the same autoconfigure path CreateTDF uses. Until now a chunked caller who wanted multi-KAS attribute grants had to implement a KeySplitter to get them, reimplementing autoconfigure by hand; DefaultKeySplitter is single-KAS and attribute-blind. The TDFOptions that shape key access -- WithDataAttributes, WithKasInformation, WithWrappingKeyAlg -- go in through WithChunkedTDFOptions. Resolution happens at Finalize, not at construction. That is the difference from CreateTDF and the reason this is a distinct resolver rather than a shared one: a chunked caller may still be adding attributes while segments are in flight, so the attribute set is not known until Finalize is called. The cost is that an unreachable KAS surfaces at Finalize, after segments have already been handed back. Passing WithChunkedKeySplitter opts out entirely and the given splitter is used as-is -- the config tracks whether the option was supplied so that "left at the default" and "deliberately overridden" are distinguishable. The test seams -- archiveFactory, cipherFactory, clock -- stay unexported. archiveFactory returns a zipstream.SegmentWriter, and zipstream lives under internal/, so there is no form of these that could be exported and supported. Graduating the package does not graduate them. sdk/experimental/tdf is deprecated at the package level rather than deleted. It has forwarded to this implementation since the Writer delegation commit and its manifest and assertion types are already aliases of the sdk types, so values move across the boundary without conversion and callers can migrate a call site at a time. The package doc carries the full option-by-option mapping. The experimental-writer benchmark example moves with it: benchmark-experimental-writer becomes benchmark-chunked-writer and builds on the package-level NewChunkedWriter, since it talks to one KAS whose key it has already fetched and has no reason to pay for platform resolution inside the timed section. Two bugs in it are fixed in passing: the segment goroutines read the loop variable to compute their own offsets, and shared a single err. Testing: the chunked suite and the classic TDFSuite round trips both pass unchanged. Two new cases cover the constructor -- one asserting that attributes resolve through the platform to the right KAS URL and KID and that the result round-trips through LoadTDF, one asserting that an explicit splitter is honored, using a KAS URL that would fail loudly if platform resolution ran anyway. The fake KAS gained a PublicKey handler so the first exercises the real key fetch rather than being handed a PEM. Also verified against cross-module builds of examples, otdfctl, service, and tests-bdd. Graduating also settles what a nil default KAS means, which the experimental options never did. WithChunkedDefaultKAS and WithChunkedDefaultKASForFinalize now reject nil, like every other option in the file that takes a pointer or an interface. Storing a nil is not recoverable later -- the field is indistinguishable from "not set" -- and for this particular field the failure is silent rather than a panic: resolution falls through to the platform base key, and the caller finds out their data went somewhere they never named only when a reader cannot unwrap it. That is the shape of the base-key contention that broke the streaming e2e file. SDK.NewChunkedWriter's doc comment now spells the fallback out, including that WithKasInformation does not suppress it, since it fills kasInfoList without clearing autoconfigure. sdk/experimental/tdf keeps taking nil as "unset" -- WithDefaultKAS and WithDefaultKASForWriter ship on main and are not this stack's to tighten -- so the delegation appends the stable option only when a KAS is actually set. A nil there still surfaces as the splitter's ErrNoDefaultKAS at Finalize, which is what callers already handle, and a test pins that so a future change cannot leak the option error through. TestChunkedOptionsRejectNil covers both new guards and TestSDKChunkedWriterFallsBackToBaseKey pins the fallback itself, so the behavior the doc comment describes is asserted rather than just written down. Both were mutation-checked: removing either guard fails them. Signed-off-by: Dave Mihalcik --- ...k_experimental.go => benchmark_chunked.go} | 58 ++++--- sdk/chunked_options.go | 67 ++++---- sdk/chunked_test.go | 150 +++++++++++++++++- sdk/chunked_writer.go | 90 +++++++---- sdk/experimental/tdf/doc.go | 42 +++-- sdk/experimental/tdf/writer.go | 23 ++- sdk/experimental/tdf/writer_test.go | 17 ++ sdk/key_splitter.go | 57 +++++-- 8 files changed, 387 insertions(+), 117 deletions(-) rename examples/cmd/{benchmark_experimental.go => benchmark_chunked.go} (64%) diff --git a/examples/cmd/benchmark_experimental.go b/examples/cmd/benchmark_chunked.go similarity index 64% rename from examples/cmd/benchmark_experimental.go rename to examples/cmd/benchmark_chunked.go index f50c5cc348..4170f43f57 100644 --- a/examples/cmd/benchmark_experimental.go +++ b/examples/cmd/benchmark_chunked.go @@ -14,7 +14,7 @@ import ( "github.com/opentdf/platform/protocol/go/kas/kasconnect" "github.com/opentdf/platform/protocol/go/policy" - "github.com/opentdf/platform/sdk/experimental/tdf" + "github.com/opentdf/platform/sdk" "github.com/opentdf/platform/sdk/httputil" "github.com/spf13/cobra" ) @@ -27,10 +27,10 @@ var ( func init() { benchmarkCmd := &cobra.Command{ - Use: "benchmark-experimental-writer", - Short: "Benchmark experimental TDF writer speed", - Long: `Benchmark the experimental TDF writer with configurable payload size.`, - RunE: runExperimentalWriterBenchmark, + Use: "benchmark-chunked-writer", + Short: "Benchmark chunked TDF writer speed", + Long: `Benchmark the chunked TDF writer with configurable payload size.`, + RunE: runChunkedWriterBenchmark, } //nolint: mnd // no magic number, this is just default value for payload size benchmarkCmd.Flags().IntVar(&payloadSize, "payload-size", 1024*1024, "Payload size in bytes") // Default 1MB @@ -39,7 +39,7 @@ func init() { ExamplesCmd.AddCommand(benchmarkCmd) } -func runExperimentalWriterBenchmark(_ *cobra.Command, _ []string) error { +func runChunkedWriterBenchmark(_ *cobra.Command, _ []string) error { payload := make([]byte, payloadSize) _, err := rand.Read(payload) if err != nil { @@ -53,7 +53,6 @@ func runExperimentalWriterBenchmark(_ *cobra.Command, _ []string) error { if err != nil { return fmt.Errorf("failed to get public key from KAS: %w", err) } - var attrs []*policy.Value simpleyKey := &policy.SimpleKasKey{ KasUri: platformEndpoint, @@ -65,31 +64,44 @@ 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)) + attrs := []*policy.Value{{ + Fqn: testAttr, + KasKeys: []*policy.SimpleKasKey{simpleyKey}, + Attribute: &policy.Attribute{Namespace: &policy.Namespace{Name: "example.com"}, Fqn: testAttr}, + }} + + // The package-level constructor rather than SDK.NewChunkedWriter: this + // benchmark talks to one KAS whose key it already fetched, so there is + // nothing for the platform to resolve and no reason to pay for a round trip + // to it inside the timed section. + writer, err := sdk.NewChunkedWriter(context.Background(), + sdk.WithChunkedDefaultKAS(simpleyKey), + sdk.WithChunkedInitialAttributes(attrs), + sdk.WithChunkedSegmentIntegrityAlgorithm(sdk.HS256), + ) if err != nil { return fmt.Errorf("failed to create writer: %w", err) } - i := 0 - wg := sync.WaitGroup{} + segs := len(payload) / segmentChunk + errs := make([]error, segs) + wg := sync.WaitGroup{} wg.Add(segs) start := time.Now() - for i < segs { - segment := i + for segment := range segs { go func() { - start := i * segmentChunk - end := min(start+segmentChunk, len(payload)) - _, err = writer.WriteSegment(context.Background(), segment, payload[start:end]) - if err != nil { - fmt.Println(err) - panic(err) - } - wg.Done() + defer wg.Done() + lo := segment * segmentChunk + hi := min(lo+segmentChunk, len(payload)) + _, errs[segment] = writer.WriteSegment(context.Background(), segment, payload[lo:hi]) }() - i++ } wg.Wait() + for i, err := range errs { + if err != nil { + return fmt.Errorf("failed to write segment %d: %w", i, err) + } + } end := time.Now() result, err := writer.Finalize(context.Background()) @@ -98,7 +110,7 @@ func runExperimentalWriterBenchmark(_ *cobra.Command, _ []string) error { } totalTime := end.Sub(start) - fmt.Printf("# Benchmark Experimental TDF Writer Results:\n") + fmt.Printf("# Benchmark Chunked TDF Writer Results:\n") fmt.Printf("| Metric | Value |\n") fmt.Printf("|--------------------|--------------|\n") fmt.Printf("| Payload Size (B) | %d |\n", payloadSize) diff --git a/sdk/chunked_options.go b/sdk/chunked_options.go index e80abbfa43..e0bd239ac2 100644 --- a/sdk/chunked_options.go +++ b/sdk/chunked_options.go @@ -8,12 +8,16 @@ import ( "github.com/opentdf/platform/protocol/go/policy" ) -// Each injection-seam option below rejects nil rather than storing it. -// A nil seam is not detectable later: the config field is -// indistinguishable from "not set", so NewChunkedWriter installs no -// default and the nil is dereferenced during writing -- for the -// splitter, not until Finalize, long after the caller has encrypted -// every segment. +// Every option below that takes a pointer or an interface rejects nil +// rather than storing it. A stored nil is not detectable later: the +// config field is indistinguishable from "not set". For an injection +// seam that means NewChunkedWriter installs no default and the nil is +// dereferenced during writing -- for the splitter, not until Finalize, +// long after the caller has encrypted every segment. For the default +// KAS it is worse than a panic, because nothing fails: key access +// silently falls back to the platform base key, and the caller learns +// their data went to a KAS they never named only when a reader cannot +// unwrap it. // withChunkedArchiveWriterFactory overrides the ZIP archive writer // factory used by the chunked Writer. The factory must not be nil. @@ -55,8 +59,6 @@ func withChunkedClock(clock clock) ChunkedWriterOption { // WithChunkedInitialAttributes sets attribute values used by Finalize // when the Finalize call does not supply its own. -// -// Experimental: not part of the stable SDK API; may change or be removed. func WithChunkedInitialAttributes(values []*policy.Value) ChunkedWriterOption { return func(c *ChunkedWriterConfig) error { c.initialAttributes = values @@ -65,11 +67,13 @@ func WithChunkedInitialAttributes(values []*policy.Value) ChunkedWriterOption { } // WithChunkedDefaultKAS sets the default KAS used by Finalize when -// the Finalize call does not supply its own. -// -// Experimental: not part of the stable SDK API; may change or be removed. +// the Finalize call does not supply its own. The KAS must not be nil: +// omit the option to leave key access to be resolved some other way. func WithChunkedDefaultKAS(kas *policy.SimpleKasKey) ChunkedWriterOption { return func(c *ChunkedWriterConfig) error { + if kas == nil { + return errors.New("chunked: default KAS must not be nil") + } c.initialDefaultKAS = kas return nil } @@ -77,8 +81,6 @@ func WithChunkedDefaultKAS(kas *policy.SimpleKasKey) ChunkedWriterOption { // WithChunkedIntegrityAlgorithm sets the algorithm used for the // manifest root signature. algo must be HS256 or GMAC. -// -// Experimental: not part of the stable SDK API; may change or be removed. func WithChunkedIntegrityAlgorithm(algo IntegrityAlgorithm) ChunkedWriterOption { return func(c *ChunkedWriterConfig) error { if algo != HS256 && algo != GMAC { @@ -93,14 +95,13 @@ func WithChunkedIntegrityAlgorithm(algo IntegrityAlgorithm) ChunkedWriterOption // chunked Writer. Callers with multi-KAS attribute grants should // inject a splitter that understands their grant model. The splitter // must not be nil. -// -// Experimental: not part of the stable SDK API; may change or be removed. func WithChunkedKeySplitter(splitter KeySplitter) ChunkedWriterOption { return func(c *ChunkedWriterConfig) error { if splitter == nil { return errors.New("chunked: key splitter must not be nil") } c.splitter = splitter + c.splitterSet = true return nil } } @@ -117,10 +118,20 @@ func withChunkedRand(r io.Reader) ChunkedWriterOption { } } +// WithChunkedTDFOptions supplies the key access options — attributes, KAS +// information, preferred wrapping algorithm — that SDK.NewChunkedWriter +// resolves against the platform at Finalize. It has no effect on the +// package-level NewChunkedWriter, which has no platform to resolve against; +// use WithChunkedKeySplitter there. +func WithChunkedTDFOptions(opts ...TDFOption) ChunkedWriterOption { + return func(c *ChunkedWriterConfig) error { + c.tdfOptions = append(c.tdfOptions, opts...) + return nil + } +} + // WithChunkedSegmentIntegrityAlgorithm sets the algorithm used for // per-segment integrity hashes. algo must be HS256 or GMAC. -// -// Experimental: not part of the stable SDK API; may change or be removed. func WithChunkedSegmentIntegrityAlgorithm(algo IntegrityAlgorithm) ChunkedWriterOption { return func(c *ChunkedWriterConfig) error { if algo != HS256 && algo != GMAC { @@ -135,8 +146,6 @@ func WithChunkedSegmentIntegrityAlgorithm(algo IntegrityAlgorithm) ChunkedWriter // TDF. Each assertion is bound to the payload's aggregate hash, so // they are signed at Finalize once every segment is in. Assertions // without their own SigningKey are signed with HS256 over the DEK. -// -// Experimental: not part of the stable SDK API; may change or be removed. func WithChunkedAssertions(assertions []AssertionConfig) ChunkedFinalizeOption { return func(c *ChunkedFinalizeConfig) error { c.assertions = assertions @@ -146,8 +155,6 @@ func WithChunkedAssertions(assertions []AssertionConfig) ChunkedFinalizeOption { // WithChunkedAttributes overrides the writer's initial attributes for // this Finalize call. -// -// Experimental: not part of the stable SDK API; may change or be removed. func WithChunkedAttributes(values []*policy.Value) ChunkedFinalizeOption { return func(c *ChunkedFinalizeConfig) error { c.attributes = values @@ -156,11 +163,13 @@ func WithChunkedAttributes(values []*policy.Value) ChunkedFinalizeOption { } // WithChunkedDefaultKASForFinalize overrides the writer's initial -// default KAS for this Finalize call. -// -// Experimental: not part of the stable SDK API; may change or be removed. +// default KAS for this Finalize call. The KAS must not be nil: omit +// the option to keep whatever WithChunkedDefaultKAS set. func WithChunkedDefaultKASForFinalize(kas *policy.SimpleKasKey) ChunkedFinalizeOption { return func(c *ChunkedFinalizeConfig) error { + if kas == nil { + return errors.New("chunked: default KAS must not be nil") + } c.defaultKAS = kas return nil } @@ -169,8 +178,6 @@ func WithChunkedDefaultKASForFinalize(kas *policy.SimpleKasKey) ChunkedFinalizeO // WithChunkedEncryptedMetadata attaches AES-GCM-encrypted metadata to // every KAO in the TDF. The metadata is keyed on the split share and // only decryptable by a reader that has been granted access. -// -// Experimental: not part of the stable SDK API; may change or be removed. func WithChunkedEncryptedMetadata(metadata string) ChunkedFinalizeOption { return func(c *ChunkedFinalizeConfig) error { c.encryptedMetadata = metadata @@ -186,8 +193,6 @@ func WithChunkedEncryptedMetadata(metadata string) ChunkedFinalizeOption { // WriteSegment, before this option is seen, so on its own this option // makes Finalize fail with [ErrChunkedVersionHexMismatch]. Pass // [WithChunkedTargetMode] at construction instead; it sets both. -// -// Experimental: not part of the stable SDK API; may change or be removed. func WithChunkedExcludeVersion() ChunkedFinalizeOption { return func(c *ChunkedFinalizeConfig) error { c.excludeVersion = true @@ -206,8 +211,6 @@ func WithChunkedExcludeVersion() ChunkedFinalizeOption { // cannot be verified by any reader. // // An empty mode selects the current format. -// -// Experimental: not part of the stable SDK API; may change or be removed. func WithChunkedTargetMode(mode string) ChunkedWriterOption { return func(c *ChunkedWriterConfig) error { if mode == "" { @@ -226,8 +229,6 @@ func WithChunkedTargetMode(mode string) ChunkedWriterOption { } // WithChunkedMimeType records the payload MIME type in the manifest. -// -// Experimental: not part of the stable SDK API; may change or be removed. func WithChunkedMimeType(mimeType string) ChunkedFinalizeOption { return func(c *ChunkedFinalizeConfig) error { c.mimeType = mimeType @@ -259,8 +260,6 @@ func WithChunkedMimeType(mimeType string) ChunkedFinalizeOption { // excludes from the manifest -- must still be appended by the caller // when assembling the final file. Skipping a dropped segment's bytes // produces an archive whose central directory offsets overshoot. -// -// Experimental: not part of the stable SDK API; may change or be removed. func WithChunkedSegments(indices []int) ChunkedFinalizeOption { return func(c *ChunkedFinalizeConfig) error { c.keepSegments = indices diff --git a/sdk/chunked_test.go b/sdk/chunked_test.go index 285431e7a8..947e723058 100644 --- a/sdk/chunked_test.go +++ b/sdk/chunked_test.go @@ -622,6 +622,15 @@ func newChunkedWriterForTest(ctx context.Context, t *testing.T, opts ...ChunkedW // Rewrap unwraps every RSA-wrapped KAO under the KAS private key and // re-wraps under the caller's session public key. +// PublicKey serves the wrapping key, so that a caller who names this KAS by URL +// alone can have its key fetched rather than having to supply the PEM. +func (k *chunkedFakeKAS) PublicKey(_ context.Context, _ *connect.Request[kaspb.PublicKeyRequest]) (*connect.Response[kaspb.PublicKeyResponse], error) { + return connect.NewResponse(&kaspb.PublicKeyResponse{ + PublicKey: k.publicPEM, + Kid: k.kid, + }), nil +} + func (k *chunkedFakeKAS) Rewrap(_ context.Context, in *connect.Request[kaspb.RewrapRequest]) (*connect.Response[kaspb.RewrapResponse], error) { tok, err := jwt.ParseInsecure([]byte(in.Msg.GetSignedRequestToken())) if err != nil { @@ -895,12 +904,14 @@ func TestChunkedTargetModeInvalid(t *testing.T) { assert.Contains(t, err.Error(), "not-a-version") } -// TestChunkedOptionsRejectNil checks that the injection-seam options -// refuse a nil value instead of storing it. A stored nil is -// indistinguishable from an unset field, so no default gets installed -// and the nil surfaces as a panic partway through writing -- for the -// key splitter, not until Finalize, after the caller has already -// encrypted and uploaded every segment. +// TestChunkedOptionsRejectNil checks that every option taking a +// pointer or an interface refuses a nil value instead of storing it. A +// stored nil is indistinguishable from an unset field, so no default +// gets installed and the nil surfaces as a panic partway through +// writing -- for the key splitter, not until Finalize, after the caller +// has already encrypted and uploaded every segment. The default KAS +// fails more quietly still: it does not panic at all, it just sends the +// data to the platform base key. func TestChunkedOptionsRejectNil(t *testing.T) { ctx := context.Background() kasBundle := newChunkedFakeKAS(t) @@ -913,6 +924,7 @@ func TestChunkedOptionsRejectNil(t *testing.T) { {"archive writer factory", withChunkedArchiveWriterFactory(nil)}, {"cipher factory", withChunkedCipherFactory(nil)}, {"clock", withChunkedClock(nil)}, + {"default KAS", WithChunkedDefaultKAS(nil)}, {"key splitter", WithChunkedKeySplitter(nil)}, {"rand", withChunkedRand(nil)}, } { @@ -926,6 +938,18 @@ func TestChunkedOptionsRejectNil(t *testing.T) { assert.Nil(t, writer) }) } + + t.Run("default KAS for finalize", func(t *testing.T) { + writer, err := NewChunkedWriter(ctx, WithChunkedDefaultKAS(kasBundle.simpleKey())) + require.NoError(t, err) + + _, err = writer.WriteSegment(ctx, 0, []byte("hello")) + require.NoError(t, err) + + _, err = writer.Finalize(ctx, WithChunkedDefaultKASForFinalize(nil)) + require.Error(t, err) + assert.Contains(t, err.Error(), "must not be nil") + }) } // TestChunkedIntegrityAlgorithmRejectsUnsupported verifies both @@ -1459,3 +1483,117 @@ func TestChunkedFinalizeRejectsUnresolvedKAS(t *testing.T) { assert.Contains(t, err.Error(), "https://unresolved.example.com") assert.Contains(t, err.Error(), "kas public key is missing") } + +// TestSDKChunkedWriterResolvesKeyAccess covers SDK.NewChunkedWriter, whose +// whole point over the package-level constructor is that key access comes from +// the platform rather than from a KeySplitter the caller had to write. The KAS +// info here carries no public key, so passing the round trip means the writer +// went out and fetched it. +func TestSDKChunkedWriterResolvesKeyAccess(t *testing.T) { + ctx := context.Background() + kasBundle := newChunkedFakeKAS(t) + defer kasBundle.server.Close() + + s := newChunkedTestSDK(t) + + writer, err := s.NewChunkedWriter(ctx, WithChunkedTDFOptions( + // Autoconfigure off: this SDK is pointed at a bare KAS, with no policy + // service to ask which KAS grants which attribute. + WithAutoconfigure(false), + WithKasInformation(KASInfo{URL: kasBundle.url}), + )) + require.NoError(t, err) + + body := writeChunkedSegments(ctx, t, writer, [][]byte{[]byte("sdk-"), []byte("chunked")}) + fin, err := writer.Finalize(ctx) + require.NoError(t, err) + require.Len(t, fin.Manifest.KeyAccessObjs, 1) + assert.Equal(t, kasBundle.url, fin.Manifest.KeyAccessObjs[0].KasURL) + assert.Equal(t, kasBundle.kid, fin.Manifest.KeyAccessObjs[0].KID) + + reader, err := s.LoadTDF(bytes.NewReader(append(body, fin.Data...)), + WithKasAllowlist([]string{kasBundle.url}), + ) + require.NoError(t, err) + plain, err := io.ReadAll(reader) + require.NoError(t, err) + assert.Equal(t, []byte("sdk-chunked"), plain) +} + +// TestSDKChunkedWriterKeepsAnExplicitSplitter checks that a caller who supplies +// their own splitter keeps it. The TDF options name a KAS that does not exist, +// so platform resolution would fail loudly rather than quietly produce the same +// answer. +func TestSDKChunkedWriterKeepsAnExplicitSplitter(t *testing.T) { + ctx := context.Background() + kasBundle := newChunkedFakeKAS(t) + defer kasBundle.server.Close() + + s := newChunkedTestSDK(t) + + writer, err := s.NewChunkedWriter(ctx, + WithChunkedKeySplitter(DefaultKeySplitter()), + WithChunkedDefaultKAS(kasBundle.simpleKey()), + WithChunkedTDFOptions( + WithAutoconfigure(false), + WithKasInformation(KASInfo{URL: "https://kas.invalid"}), + ), + ) + require.NoError(t, err) + + body := writeChunkedSegments(ctx, t, writer, [][]byte{[]byte("explicit splitter")}) + fin, err := writer.Finalize(ctx) + require.NoError(t, err) + require.Len(t, fin.Manifest.KeyAccessObjs, 1) + assert.Equal(t, kasBundle.url, fin.Manifest.KeyAccessObjs[0].KasURL) + + reader, err := s.LoadTDF(bytes.NewReader(append(body, fin.Data...)), + WithKasAllowlist([]string{kasBundle.url}), + ) + require.NoError(t, err) + plain, err := io.ReadAll(reader) + require.NoError(t, err) + assert.Equal(t, []byte("explicit splitter"), plain) +} + +// TestSDKChunkedWriterFallsBackToBaseKey pins what happens when the caller +// names no KAS at all: SDK.NewChunkedWriter leaves autoconfigure on, finds no +// attribute grants, and wraps to the platform base key. Nothing errors, so this +// is the case that silently sends data somewhere the caller did not choose -- +// which is why WithChunkedDefaultKAS rejects nil rather than treating it as +// "unset", and why the constructor's doc comment spells the fallback out. +func TestSDKChunkedWriterFallsBackToBaseKey(t *testing.T) { + ctx := context.Background() + kasBundle := newChunkedFakeKAS(t) + defer kasBundle.server.Close() + + s := newChunkedTestSDK(t) + s.wellknownConfiguration = newMockWellKnownService(map[string]interface{}{ + baseKeyWellKnown: map[string]interface{}{ + "kas_uri": kasBundle.url, + baseKeyPublicKey: map[string]interface{}{ + baseKeyAlg: "rsa:2048", + "kid": kasBundle.kid, + "pem": kasBundle.publicPEM, + }, + }, + }, nil) + + writer, err := s.NewChunkedWriter(ctx) + require.NoError(t, err) + + body := writeChunkedSegments(ctx, t, writer, [][]byte{[]byte("base key")}) + fin, err := writer.Finalize(ctx) + require.NoError(t, err) + require.Len(t, fin.Manifest.KeyAccessObjs, 1) + assert.Equal(t, kasBundle.url, fin.Manifest.KeyAccessObjs[0].KasURL) + assert.Equal(t, kasBundle.kid, fin.Manifest.KeyAccessObjs[0].KID) + + reader, err := s.LoadTDF(bytes.NewReader(append(body, fin.Data...)), + WithKasAllowlist([]string{kasBundle.url}), + ) + require.NoError(t, err) + plain, err := io.ReadAll(reader) + require.NoError(t, err) + assert.Equal(t, []byte("base key"), plain) +} diff --git a/sdk/chunked_writer.go b/sdk/chunked_writer.go index 95b81067e1..9d6557205a 100644 --- a/sdk/chunked_writer.go +++ b/sdk/chunked_writer.go @@ -86,8 +86,6 @@ func defaultArchiveWriterFactory(c clock) zipstream.SegmentWriter { } // Sentinel errors returned by [ChunkedWriter]. -// -// Experimental: not part of the stable SDK API; may change or be removed. var ( // ErrChunkedAlreadyFinalized is returned when a ChunkedWriter // method is called after Finalize has already succeeded. @@ -125,8 +123,6 @@ var ( // off-thread or in parallel — then call Finalize to close the // archive. Contrast with SDK.CreateTDF, which requires the full // plaintext up front. -// -// Experimental: not part of the stable SDK API; may change or be removed. type ChunkedWriter interface { // Finalize completes TDF creation. Every option applies only to // this Finalize call; writer-level defaults set at NewChunked* @@ -157,8 +153,6 @@ type ChunkedWriter interface { // ChunkedSegmentResult carries the ZIP bytes for one segment plus its // integrity metadata. -// -// Experimental: not part of the stable SDK API; may change or be removed. type ChunkedSegmentResult struct { // EncryptedSize is the ciphertext byte length including nonce and // GCM tag. @@ -184,8 +178,6 @@ type ChunkedSegmentResult struct { // ChunkedFinalizeResult carries the finalized TDF's closing bytes and // metadata about what was written. -// -// Experimental: not part of the stable SDK API; may change or be removed. type ChunkedFinalizeResult struct { // Data is the ZIP closing bytes, in order: the payload's data // descriptor, the embedded manifest entry (its own local file @@ -215,8 +207,6 @@ type ChunkedFinalizeResult struct { // ChunkedWriterConfig captures the settings supplied at // NewChunkedWriter time. Fields are unexported; use options. -// -// Experimental: not part of the stable SDK API; may change or be removed. type ChunkedWriterConfig struct { // archiveFactory builds the ZIP archive writer that lays out the // TDF. Defaults to defaultArchiveWriterFactory. @@ -276,6 +266,15 @@ type ChunkedWriterConfig struct { // keyAccess is set. splitter KeySplitter + // splitterSet records whether WithChunkedKeySplitter was given, so + // that SDK.NewChunkedWriter can tell "left at the default" from + // "deliberately overridden" and only replace the former. + splitterSet bool + + // tdfOptions shape key access resolved against the platform. Only + // meaningful for SDK.NewChunkedWriter; see WithChunkedTDFOptions. + tdfOptions []TDFOption + // useHex hex-encodes segment, root, and assertion signatures // before base64, producing the doubly-encoded form that readers // older than 4.3.0 require. Set by WithChunkedTargetMode. @@ -283,8 +282,6 @@ type ChunkedWriterConfig struct { } // ChunkedFinalizeConfig captures Finalize-time overrides. -// -// Experimental: not part of the stable SDK API; may change or be removed. type ChunkedFinalizeConfig struct { // assertions to sign and attach to the produced TDF. Each // AssertionConfig must carry a SigningKey (or the writer's DEK @@ -321,13 +318,9 @@ type ChunkedFinalizeConfig struct { // ChunkedWriterOption configures a ChunkedWriter at construction // time. -// -// Experimental: not part of the stable SDK API; may change or be removed. type ChunkedWriterOption func(*ChunkedWriterConfig) error // ChunkedFinalizeOption configures a single Finalize call. -// -// Experimental: not part of the stable SDK API; may change or be removed. type ChunkedFinalizeOption func(*ChunkedFinalizeConfig) error // chunkedWriter is the concrete ChunkedWriter. @@ -393,19 +386,10 @@ type chunkedWriter struct { useHex bool } -// NewChunkedWriter constructs a per-segment TDF writer. WriteSegment -// may be called from several goroutines at once so long as each -// targets a distinct segment index; two concurrent calls for the same -// index are not allowed, and one of them will fail with -// ErrChunkedSegmentAlreadyWritten rather than corrupt the archive. -// -// No SDK value is needed: everything the writer depends on — the key -// splitter, the archive and cipher factories, the entropy source — is -// supplied through options. -// -// Experimental: not part of the stable SDK API; may change or be removed. -func NewChunkedWriter(_ context.Context, opts ...ChunkedWriterOption) (ChunkedWriter, error) { - cfg := ChunkedWriterConfig{ +// defaultChunkedWriterConfig is the starting point both constructors apply +// options over. +func defaultChunkedWriterConfig() ChunkedWriterConfig { + return ChunkedWriterConfig{ archiveFactory: defaultArchiveWriterFactory, cipherFactory: defaultSegmentCipherFactory, clock: systemClock{}, @@ -414,11 +398,59 @@ func NewChunkedWriter(_ context.Context, opts ...ChunkedWriterOption) (ChunkedWr segmentIntegrityAlgorithm: HS256, splitter: DefaultKeySplitter(), } +} + +// WriteSegment may be called from several goroutines at once so long +// as each targets a distinct segment index; two concurrent calls for +// the same index are not allowed, and one of them will fail with +// ErrChunkedSegmentAlreadyWritten rather than corrupt the archive. +// +// No SDK value is needed: everything the writer depends on — the key +// splitter, the archive and cipher factories, the entropy source — is +// supplied through options. +func NewChunkedWriter(_ context.Context, opts ...ChunkedWriterOption) (ChunkedWriter, error) { + cfg := defaultChunkedWriterConfig() + for _, opt := range opts { + if err := opt(&cfg); err != nil { + return nil, err + } + } + return newChunkedWriter(cfg) +} + +// NewChunkedWriter creates a TDF from segments that may arrive in any order, +// with key access resolved against the platform this SDK is connected to. +// Attributes are run through the same autoconfigure path SDK.CreateTDF uses, so +// a caller gets multi-KAS attribute grants without implementing a KeySplitter. +// +// Options are the same as for the package-level [NewChunkedWriter]. Pass the +// TDFOptions that shape key access — [WithDataAttributes], [WithKasInformation], +// [WithWrappingKeyAlg] and so on — through [WithChunkedTDFOptions]; they are +// replayed at Finalize. Supplying [WithChunkedKeySplitter] opts out of platform +// resolution entirely and the given splitter is used as-is. +// +// Unlike SDK.CreateTDF, key access is resolved at Finalize rather than up front, +// because a chunked caller may still be adding attributes while segments are in +// flight. An unreachable KAS therefore surfaces at Finalize, after segments have +// already been handed back. +// +// Naming no KAS is not an error: with no [WithChunkedDefaultKAS] and no attribute +// that grants one, resolution falls through to the platform's base key, exactly as +// SDK.CreateTDF does. [WithKasInformation] does not change that — it fills +// kasInfoList but leaves autoconfigure on, so a platform with a base key configured +// overwrites it and logs "base key is enabled, overwriting kasInfoList with base key +// info". To pin key access to a KAS of your choosing, pass [WithChunkedDefaultKAS]; +// that is the only option here that turns autoconfigure off. +func (s SDK) NewChunkedWriter(_ context.Context, opts ...ChunkedWriterOption) (ChunkedWriter, error) { + cfg := defaultChunkedWriterConfig() for _, opt := range opts { if err := opt(&cfg); err != nil { return nil, err } } + if !cfg.splitterSet { + cfg.keyAccess = sdkKeyAccess{sdk: s, opts: cfg.tdfOptions} + } return newChunkedWriter(cfg) } diff --git a/sdk/experimental/tdf/doc.go b/sdk/experimental/tdf/doc.go index 798138dcad..d9d87b7157 100644 --- a/sdk/experimental/tdf/doc.go +++ b/sdk/experimental/tdf/doc.go @@ -1,13 +1,35 @@ -// Experimental: This package is EXPERIMENTAL and may change or be removed at any time -// Package tdf provides experimental streaming TDF (Trusted Data Format) creation capabilities. -// -// # Experimental Status -// -// This package is EXPERIMENTAL and its API is subject to change in future releases. -// It is designed for advanced use cases requiring fine-grained control over TDF creation -// with streaming support for large datasets. -// -// For most use cases, prefer the stable SDK-level TDF creation APIs. +// Package tdf provides streaming TDF (Trusted Data Format) creation capabilities. +// +// Deprecated: this package has graduated into the sdk package itself. Use +// [github.com/opentdf/platform/sdk.SDK.NewChunkedWriter], or the package-level +// [github.com/opentdf/platform/sdk.NewChunkedWriter] when there is no platform +// connection to resolve key access against. This package now forwards to that +// implementation and will be removed in a future release. +// +// # Migration +// +// Writer options: +// +// tdf.NewWriter(ctx, opts...) -> sdk.NewChunkedWriter(ctx, opts...) +// client.NewChunkedWriter(ctx, opts...) +// tdf.WithInitialAttributes(vs) -> sdk.WithChunkedInitialAttributes(vs) +// tdf.WithDefaultKASForWriter(k) -> sdk.WithChunkedDefaultKAS(k) +// tdf.WithIntegrityAlgorithm(a) -> sdk.WithChunkedIntegrityAlgorithm(a) +// tdf.WithSegmentIntegrityAlgorithm(a) -> sdk.WithChunkedSegmentIntegrityAlgorithm(a) +// tdf.WithTargetMode(m) -> sdk.WithChunkedTargetMode(m) +// +// Finalize options: +// +// tdf.WithAssertions(as) -> sdk.WithChunkedAssertions(as) +// tdf.WithAttributeValues(vs) -> sdk.WithChunkedAttributes(vs) +// tdf.WithDefaultKAS(k) -> sdk.WithChunkedDefaultKASForFinalize(k) +// tdf.WithEncryptedMetadata(m) -> sdk.WithChunkedEncryptedMetadata(m) +// tdf.WithExcludeVersionFromManifest() -> sdk.WithChunkedExcludeVersion() +// tdf.WithPayloadMimeType(m) -> sdk.WithChunkedMimeType(m) +// tdf.WithSegments(ix) -> sdk.WithChunkedSegments(ix) +// +// The manifest and assertion types this package exports are aliases of the sdk +// types, so values move across the boundary without conversion. // // # Overview // diff --git a/sdk/experimental/tdf/writer.go b/sdk/experimental/tdf/writer.go index aefa4fa9e4..5e435e3cd6 100644 --- a/sdk/experimental/tdf/writer.go +++ b/sdk/experimental/tdf/writer.go @@ -127,14 +127,22 @@ func NewWriter(ctx context.Context, opts ...Option[*WriterConfig]) (*Writer, err opt(config) } - inner, err := sdk.NewChunkedWriter(ctx, + // A nil KAS means "unset" here -- WithDefaultKASForWriter has always + // accepted one -- but the stable option rejects nil, so leave it off + // rather than forwarding. The splitter reports the missing KAS at + // Finalize as ErrNoDefaultKAS, which is what callers already handle. + chunkedOpts := []sdk.ChunkedWriterOption{ sdk.WithChunkedIntegrityAlgorithm(sdk.IntegrityAlgorithm(config.integrityAlgorithm)), sdk.WithChunkedSegmentIntegrityAlgorithm(sdk.IntegrityAlgorithm(config.segmentIntegrityAlgorithm)), sdk.WithChunkedInitialAttributes(config.initialAttributes), - sdk.WithChunkedDefaultKAS(config.initialDefaultKAS), sdk.WithChunkedKeySplitter(xorSplitter{}), sdk.WithChunkedTargetMode(config.targetMode), - ) + } + if config.initialDefaultKAS != nil { + chunkedOpts = append(chunkedOpts, sdk.WithChunkedDefaultKAS(config.initialDefaultKAS)) + } + + inner, err := sdk.NewChunkedWriter(ctx, chunkedOpts...) if err != nil { return nil, err } @@ -280,12 +288,17 @@ func finalizeOptions(opts []Option[*WriterFinalizeConfig]) []sdk.ChunkedFinalize for _, opt := range opts { opt(cfg) } - return []sdk.ChunkedFinalizeOption{ + out := []sdk.ChunkedFinalizeOption{ sdk.WithChunkedAttributes(cfg.attributes), - sdk.WithChunkedDefaultKASForFinalize(cfg.defaultKas), sdk.WithChunkedEncryptedMetadata(cfg.encryptedMetadata), sdk.WithChunkedMimeType(cfg.payloadMimeType), sdk.WithChunkedSegments(cfg.keepSegments), sdk.WithChunkedAssertions(cfg.assertions), } + // Omitted rather than forwarded as nil, for the reason given in + // NewWriter: the stable option rejects nil, this package's does not. + if cfg.defaultKas != nil { + out = append(out, sdk.WithChunkedDefaultKASForFinalize(cfg.defaultKas)) + } + return out } diff --git a/sdk/experimental/tdf/writer_test.go b/sdk/experimental/tdf/writer_test.go index ba662a5696..d81d010f45 100644 --- a/sdk/experimental/tdf/writer_test.go +++ b/sdk/experimental/tdf/writer_test.go @@ -678,6 +678,23 @@ func testErrorConditions(t *testing.T) { assert.Contains(t, err.Error(), "no default KAS", "Error should mention missing default KAS") }) + // A nil KAS means "unset" in this package, but the stable option it + // delegates to rejects nil outright. The writer has to drop the option + // rather than forward the nil, or construction and Finalize would fail + // with an option error instead of the ErrNoDefaultKAS callers handle. + t.Run("ExplicitNilKASStaysUnset", func(t *testing.T) { + writer, err := NewWriter(ctx, WithDefaultKASForWriter(nil)) + require.NoError(t, err, "a nil KAS at construction is not an error here") + + _, err = writer.WriteSegment(ctx, 0, []byte("test")) + require.NoError(t, err) + + _, err = writer.Finalize(ctx, WithDefaultKAS(nil)) + require.Error(t, err) + assert.Contains(t, err.Error(), "no default KAS", + "a nil KAS should surface as the splitter's error, not an option error") + }) + t.Run("SegmentsNamesUnwrittenIndex", func(t *testing.T) { writer, err := NewWriter(ctx) require.NoError(t, err) diff --git a/sdk/key_splitter.go b/sdk/key_splitter.go index 470fea684a..e70a00ac54 100644 --- a/sdk/key_splitter.go +++ b/sdk/key_splitter.go @@ -6,6 +6,7 @@ import ( "errors" "fmt" "io" + "slices" "github.com/opentdf/platform/lib/ocrypto" "github.com/opentdf/platform/protocol/go/policy" @@ -15,8 +16,6 @@ import ( // key splits, each addressed to one or more KAS servers. Injected on // the chunked Writer so tests can substitute an identity splitter // without touching real attribute grants. -// -// Experimental: not part of the stable SDK API; may change or be removed. type KeySplitter interface { // Split evaluates the ABAC policy expressed by attrs, produces N // splits of dek per the resulting boolean expression, and returns @@ -26,8 +25,6 @@ type KeySplitter interface { // Split is one XOR share of the DEK bound to one or more KAS // servers. -// -// Experimental: not part of the stable SDK API; may change or be removed. type Split struct { // Data is the split share (XOR of the DEK with the other shares). Data []byte @@ -44,8 +41,6 @@ type Split struct { // SplitResult is what KeySplitter.Split returns: the shares plus the // KAS wrapping keys needed to encrypt each share into a KeyAccess // object. -// -// Experimental: not part of the stable SDK API; may change or be removed. type SplitResult struct { // KASPublicKeys maps KAS URL to the wrapping key to use for that // URL. Populated for every URL referenced by any split. @@ -56,8 +51,6 @@ type SplitResult struct { } // KASPublicKey is the wrapping key resolved for one KAS URL. -// -// Experimental: not part of the stable SDK API; may change or be removed. type KASPublicKey struct { // Algorithm identifies the wrapping scheme as an exact // ocrypto.KeyType string, e.g. "rsa:2048" or "ec:secp256r1" -- use @@ -91,8 +84,6 @@ var ErrSplitterUnsupportedAlgorithm = errors.New("chunked: unsupported KAS key a // Attributes are ignored; the entire DEK is bound to the caller's // default KAS. Callers with attribute-based key splits requirements // should inject their own splitter via WithChunkedKeySplitter. -// -// Experimental: not part of the stable SDK API; may change or be removed. func DefaultKeySplitter() KeySplitter { return &singleKASSplitter{} } // singleKASSplitter binds the full DEK to a single KAS. Attributes @@ -184,6 +175,52 @@ func (r staticKeyAccess) resolve(_ context.Context, _ []byte, _ *ChunkedFinalize return r.policy, r.kaos, nil } +// sdkKeyAccess resolves key access through the platform, the way SDK.CreateTDF +// does: the attributes settled by Finalize are run through autoconfigure to +// find the KAS servers that grant them, and the DEK is split across the +// resulting plan. +// +// This is what SDK.NewChunkedWriter installs in place of DefaultKeySplitter, +// which is single-KAS and attribute-blind. Resolution is deferred to Finalize +// rather than done at construction because a chunked caller may still be adding +// attributes while segments are in flight. +type sdkKeyAccess struct { + // sdk is the platform connection used to resolve grants and fetch KAS + // public keys. + sdk SDK + + // opts are the TDFOptions given to SDK.NewChunkedWriter. They are replayed + // on each Finalize so that resolution sees the attributes as of that call. + opts []TDFOption +} + +func (r sdkKeyAccess) resolve(ctx context.Context, dek []byte, cfg *ChunkedFinalizeConfig) (string, []KeyAccess, error) { + opts := r.opts + if len(cfg.attributes) > 0 { + opts = append(slices.Clone(opts), WithDataAttributeValues(cfg.attributes...)) + } + tdfConfig, err := newTDFConfig(opts...) + if err != nil { + return "", nil, err + } + tdfConfig.metaData = cfg.encryptedMetadata + + // A caller-named KAS is a decision, not a hint: honor it instead of asking + // the platform which KAS the attributes point at. Autoconfigure has to go + // off for that, since initKAOTemplate refuses to run both. + if cfg.defaultKAS != nil { + tdfConfig.autoconfigure = false + if err := populateKasInfoFromBaseKey(cfg.defaultKAS, tdfConfig); err != nil { + return "", nil, err + } + } + + if err := tdfConfig.initKAOTemplate(ctx, r.sdk); err != nil { + return "", nil, err + } + return r.sdk.resolveKeyAccess(ctx, tdfConfig, dek) +} + // splitterKeyAccess adapts a public KeySplitter to keyAccessResolver. type splitterKeyAccess struct { // splitter maps attributes plus the DEK onto KAS-addressed shares.