From b372aa1a2a2f240a34760e6209c0ee6f23c70180 Mon Sep 17 00:00:00 2001 From: Piotr Szafarczyk Date: Thu, 25 Jun 2026 16:02:29 +0200 Subject: [PATCH] feat(router): report decrypted plaintext size in ListObjects responses MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ListObjects/ListObjectsV2 previously reported the at-rest ciphertext size (plaintext + 28-byte AES-GCM-SIV envelope). Intercept bucket-level list requests, subtract the fixed encryption overhead from every , and clamp at 0. Bucket sub-resource GETs (acl, versioning, multipart listings, ?versions, …) are still forwarded unchanged. - cryptoutil: export EncryptionOverhead constant + invariant test - router: bucketOnlyPattern + isListObjects, wired into getHandler - handler: extract forwardUpstream; add handleListObjects (buffer, rewrite on 2xx, fix Content-Length) - listsize: byte-preserving regex rewrite + unit tests - e2e: assert v1 and v2 listings report plaintext size Known limitation: objects not written through the proxy (legacy plaintext, server-side copies, multipart) are reported 28 bytes short / 0 after clamp, since list responses carry no per-object encryption metadata. --- CHANGELOG.md | 12 +++ s3proxy/internal/cryptoutil/cryptoutil.go | 5 + .../internal/cryptoutil/cryptoutil_test.go | 22 ++++ s3proxy/internal/e2e/proxy_e2e_test.go | 16 +++ s3proxy/internal/router/handler.go | 101 +++++++++++++----- s3proxy/internal/router/listsize.go | 39 +++++++ s3proxy/internal/router/listsize_test.go | 62 +++++++++++ s3proxy/internal/router/router.go | 39 ++++++- 8 files changed, 271 insertions(+), 25 deletions(-) create mode 100644 s3proxy/internal/router/listsize.go create mode 100644 s3proxy/internal/router/listsize_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index e779d8f..dd0a74c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,18 @@ Two version streams move independently: ## [Unreleased] +### Added + +- `ListObjects`/`ListObjectsV2` responses now report the **decrypted plaintext + size** of each object instead of the larger at-rest ciphertext size. The proxy + intercepts bucket-level list requests, subtracts the fixed 28-byte encryption + overhead (12-byte nonce + 16-byte GCM tag) from every ``, and clamps at 0. + Bucket sub-resource GETs (acl, versioning, multipart listings, `?versions`, …) + are still forwarded unchanged. + - *Known limitation:* objects **not** written through the proxy (legacy + plaintext, server-side copies, multipart) are reported 28 bytes short (or 0 + after clamp), since a list response carries no per-object encryption metadata. + ### Chart - **`chart/1.9.3`** — Dashboard usability + Go runtime panels. diff --git a/s3proxy/internal/cryptoutil/cryptoutil.go b/s3proxy/internal/cryptoutil/cryptoutil.go index 1a0be9b..2e5aabe 100644 --- a/s3proxy/internal/cryptoutil/cryptoutil.go +++ b/s3proxy/internal/cryptoutil/cryptoutil.go @@ -18,6 +18,11 @@ import ( "github.com/tink-crypto/tink-go/v2/subtle/random" ) +// EncryptionOverhead is the fixed number of bytes Encrypt adds to the plaintext: +// a 12-byte AES-GCM-SIV nonce prefix plus a 16-byte authentication tag suffix. +// Ciphertext length is always plaintext length + EncryptionOverhead. +const EncryptionOverhead = 28 + // Encrypt generates a random key to encrypt a plaintext using AES-256-GCM. // The generated key is encrypted using the supplied key encryption key (KEK). // The ciphertext and encrypted data encryption key (DEK) are returned. diff --git a/s3proxy/internal/cryptoutil/cryptoutil_test.go b/s3proxy/internal/cryptoutil/cryptoutil_test.go index 6412452..17ec09d 100644 --- a/s3proxy/internal/cryptoutil/cryptoutil_test.go +++ b/s3proxy/internal/cryptoutil/cryptoutil_test.go @@ -46,3 +46,25 @@ func TestEncryptDecrypt(t *testing.T) { }) } } + +// TestEncryptionOverhead asserts that Encrypt adds exactly EncryptionOverhead bytes to the +// plaintext, regardless of plaintext size. This invariant is what lets the router subtract a +// constant to report decrypted sizes in LIST responses; it fails loudly if the envelope changes. +func TestEncryptionOverhead(t *testing.T) { + for _, size := range []int{0, 1, 28, 29, 1024, 1610862} { + t.Run(fmt.Sprintf("size_%d", size), func(t *testing.T) { + kek := [32]byte{} + _, err := rand.Read(kek[:]) + require.NoError(t, err) + + plaintext := make([]byte, size) + _, err = rand.Read(plaintext) + require.NoError(t, err) + + ciphertext, _, err := Encrypt(plaintext, kek) + require.NoError(t, err) + + assert.Equal(t, size+EncryptionOverhead, len(ciphertext)) + }) + } +} diff --git a/s3proxy/internal/e2e/proxy_e2e_test.go b/s3proxy/internal/e2e/proxy_e2e_test.go index f3fa1db..0af7bc4 100644 --- a/s3proxy/internal/e2e/proxy_e2e_test.go +++ b/s3proxy/internal/e2e/proxy_e2e_test.go @@ -167,6 +167,22 @@ func TestProxyRoundtripAgainstMinio(t *testing.T) { require.NoError(t, got.Body.Close()) assert.Equal(t, plaintext, gotBody) + // Listing through the proxy must report the decrypted plaintext size, not the larger + // ciphertext size that actually sits at rest. Cover both ListObjectsV2 and ListObjects v1. + listV2, err := proxyClient.ListObjectsV2(ctx, &awss3.ListObjectsV2Input{ + Bucket: aws.String(bucket), + }) + require.NoError(t, err) + require.Len(t, listV2.Contents, 1) + assert.EqualValues(t, len(plaintext), *listV2.Contents[0].Size, "ListObjectsV2 must report plaintext size") + + listV1, err := proxyClient.ListObjects(ctx, &awss3.ListObjectsInput{ + Bucket: aws.String(bucket), + }) + require.NoError(t, err) + require.Len(t, listV1.Contents, 1) + assert.EqualValues(t, len(plaintext), *listV1.Contents[0].Size, "ListObjects v1 must report plaintext size") + // Housekeeping endpoints stay reachable while traffic is flowing. healthResp, err := http.Get(proxyURL.String() + "/healthz") require.NoError(t, err) diff --git a/s3proxy/internal/router/handler.go b/s3proxy/internal/router/handler.go index 5871879..0f8a4de 100644 --- a/s3proxy/internal/router/handler.go +++ b/s3proxy/internal/router/handler.go @@ -15,6 +15,7 @@ import ( "io" "log/slog" "net/http" + "strconv" "time" v4 "github.com/aws/aws-sdk-go-v2/aws/signer/v4" @@ -138,59 +139,111 @@ func handlePutObject(client s3Client, key string, bucket string, keks cryptoutil } } +// forwardUpstream repackages, signs and sends req to the upstream S3 API, returning the +// upstream response. The caller owns resp.Body and must close it. metrics may be nil. +func forwardUpstream(client *s3.Client, req *http.Request, metrics *monitoring.Metrics) (*http.Response, error) { + newReq, err := repackage(req) + if err != nil { + return nil, fmt.Errorf("repackaging request: %w", err) + } + + cfg := client.GetConfig() + + creds, err := cfg.Credentials.Retrieve(context.TODO()) + if err != nil { + return nil, fmt.Errorf("retrieving aws creds: %w", err) + } + + signer := v4.NewSigner() + + err = signer.SignHTTP(context.TODO(), creds, newReq, newReq.Header.Get("X-Amz-Content-Sha256"), "s3", cfg.Region, time.Now()) + if err != nil { + return nil, fmt.Errorf("signing request: %w", err) + } + + resp, err := forwardHTTPClient.Do(newReq) + if err != nil { + if metrics != nil { + metrics.UpstreamErrors.Inc() + } + return nil, fmt.Errorf("do request: %w", err) + } + + return resp, nil +} + func handleForwards(client *s3.Client, log *slog.Logger, metrics *monitoring.Metrics) http.HandlerFunc { return func(w http.ResponseWriter, req *http.Request) { log.Debug("forwarding", "path", req.URL.Path, "method", req.Method, "host", req.Host) - newReq, err := repackage(req) + resp, err := forwardUpstream(client, req, metrics) if err != nil { - log.Error("failed to repackage request", "error", err) + log.Error("forwarding request", "error", err) http.Error(w, "internal server error", http.StatusInternalServerError) return } + defer func() { + if cerr := resp.Body.Close(); cerr != nil { + log.Error("failed to close upstream response body", "error", cerr) + } + }() - cfg := client.GetConfig() + // Preserve multi-value headers (Set-Cookie, Vary, etc.). + for key, values := range resp.Header { + w.Header()[key] = values + } + w.WriteHeader(resp.StatusCode) - creds, err := cfg.Credentials.Retrieve(context.TODO()) - if err != nil { - log.Error("unable to retrieve aws creds", "error", err) - http.Error(w, "internal server error", http.StatusInternalServerError) - return + if _, err := io.Copy(w, resp.Body); err != nil { + log.Error("failed to stream response", "error", err) + // Headers already written; cannot send error response. } + } +} - signer := v4.NewSigner() +// handleListObjects forwards a ListObjects/ListObjectsV2 request upstream, then rewrites the +// reported object sizes in the XML response to the decrypted plaintext size (ciphertext size +// minus the fixed encryption overhead). List pages are bounded (≤1000 keys), so buffering the +// body is safe. Only successful (2xx) responses are rewritten; everything else passes through. +func handleListObjects(client *s3.Client, log *slog.Logger, metrics *monitoring.Metrics) http.HandlerFunc { + return func(w http.ResponseWriter, req *http.Request) { + log.Debug("intercepting ListObjects", "path", req.URL.Path, "method", req.Method, "host", req.Host) - err = signer.SignHTTP(context.TODO(), creds, newReq, newReq.Header.Get("X-Amz-Content-Sha256"), "s3", cfg.Region, time.Now()) + resp, err := forwardUpstream(client, req, metrics) if err != nil { - log.Error("failed to sign request", "error", err) + log.Error("forwarding ListObjects request", "error", err) http.Error(w, "internal server error", http.StatusInternalServerError) return } - - resp, err := forwardHTTPClient.Do(newReq) - if err != nil { - log.Error("do request", "error", err) - if metrics != nil { - metrics.UpstreamErrors.Inc() - } - http.Error(w, "do request: "+err.Error(), http.StatusInternalServerError) - return - } defer func() { if cerr := resp.Body.Close(); cerr != nil { log.Error("failed to close upstream response body", "error", cerr) } }() + body, err := io.ReadAll(resp.Body) + if err != nil { + log.Error("reading ListObjects response body", "error", err) + http.Error(w, "internal server error", http.StatusInternalServerError) + return + } + + out := body + if resp.StatusCode >= 200 && resp.StatusCode < 300 { + out = rewriteListSizes(body) + } + // Preserve multi-value headers (Set-Cookie, Vary, etc.). for key, values := range resp.Header { w.Header()[key] = values } + // The rewritten body is shorter than the upstream ciphertext sizes, so the upstream + // Content-Length no longer applies — set it to the actual body length. + w.Header().Set("Content-Length", strconv.Itoa(len(out))) w.WriteHeader(resp.StatusCode) - if _, err := io.Copy(w, resp.Body); err != nil { - log.Error("failed to stream response", "error", err) - // Headers already written; cannot send error response. + if _, err := w.Write(out); err != nil { + log.Error("failed to write ListObjects response", "error", err) } } } diff --git a/s3proxy/internal/router/listsize.go b/s3proxy/internal/router/listsize.go new file mode 100644 index 0000000..91044094 --- /dev/null +++ b/s3proxy/internal/router/listsize.go @@ -0,0 +1,39 @@ +/* +Copyright (c) Intrinsec 2024 + +SPDX-License-Identifier: AGPL-3.0-only +*/ + +package router + +import ( + "fmt" + "regexp" + "strconv" + + "github.com/intrinsec/s3proxy/internal/cryptoutil" +) + +// sizeElementPattern matches a N element. In ListObjects v1/v2 responses this +// element only appears inside , so a targeted byte-preserving substitution avoids +// the namespace/formatting loss of an encoding/xml round-trip that could break S3 clients. +var sizeElementPattern = regexp.MustCompile(`(\d+)`) + +// rewriteListSizes replaces each N in a ListObjects response with +// max(0, N-EncryptionOverhead), reporting the decrypted plaintext size. The clamp at 0 covers +// 0-byte folder markers and objects not written through the proxy (smaller than the overhead). +// Everything outside the matched elements is preserved byte-for-byte. +func rewriteListSizes(body []byte) []byte { + return sizeElementPattern.ReplaceAllFunc(body, func(m []byte) []byte { + n, err := strconv.Atoi(string(sizeElementPattern.FindSubmatch(m)[1])) + if err != nil { + // Unparseable (e.g. overflows int) — leave the element untouched. + return m + } + dec := n - cryptoutil.EncryptionOverhead + if dec < 0 { + dec = 0 + } + return []byte(fmt.Sprintf("%d", dec)) + }) +} diff --git a/s3proxy/internal/router/listsize_test.go b/s3proxy/internal/router/listsize_test.go new file mode 100644 index 0000000..c5922de --- /dev/null +++ b/s3proxy/internal/router/listsize_test.go @@ -0,0 +1,62 @@ +/* +Copyright (c) Intrinsec 2024 + +SPDX-License-Identifier: AGPL-3.0-only +*/ + +package router + +import ( + "fmt" + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestRewriteListSizes(t *testing.T) { + tests := map[string]struct { + in int + want int + }{ + "zero clamps to zero": {in: 0, want: 0}, + "below overhead clamps to zero": {in: 5, want: 0}, + "exactly overhead clamps to zero": {in: 28, want: 0}, + "one byte plaintext": {in: 29, want: 1}, + "large object": {in: 1610890, want: 1610862}, + } + + for name, tt := range tests { + t.Run(name, func(t *testing.T) { + body := []byte(fmt.Sprintf("%d", tt.in)) + got := rewriteListSizes(body) + assert.Equal(t, fmt.Sprintf("%d", tt.want), string(got)) + }) + } +} + +// TestRewriteListSizesPreservesStructure asserts only values change; surrounding XML — +// including CommonPrefixes (directories, which carry no ) — is byte-for-byte preserved. +func TestRewriteListSizesPreservesStructure(t *testing.T) { + body := []byte(`` + + `` + + `bucket` + + `a.txt1610890"abc"` + + `b.txt28` + + `dir/` + + ``) + + want := []byte(`` + + `` + + `bucket` + + `a.txt1610862"abc"` + + `b.txt0` + + `dir/` + + ``) + + assert.Equal(t, string(want), string(rewriteListSizes(body))) +} + +func TestRewriteListSizesNoSizeElement(t *testing.T) { + body := []byte(`dir/`) + assert.Equal(t, string(body), string(rewriteListSizes(body))) +} diff --git a/s3proxy/internal/router/router.go b/s3proxy/internal/router/router.go index 9815f86..f4e52ca 100644 --- a/s3proxy/internal/router/router.go +++ b/s3proxy/internal/router/router.go @@ -43,8 +43,41 @@ import ( var ( bucketAndKeyPattern = regexp.MustCompile("/([^/?]+)/(.+)") + // bucketOnlyPattern matches a bucket-level path (no object key), with an optional + // trailing slash. Used to detect ListObjects/ListObjectsV2 requests. + bucketOnlyPattern = regexp.MustCompile("^/([^/?]+)/?$") ) +// listSubResourceMarkers are query parameters that turn a bucket-level GET into a +// sub-resource request (bucket ACL, versioning, multipart listing, …). Those return +// XML that has no , or is out of scope (versions), so they must not be +// rewritten — only a "clean" ListObjects v1/v2 carries object sizes. +var listSubResourceMarkers = []string{ + "acl", "policy", "versioning", "location", "tagging", "lifecycle", "cors", + "website", "replication", "encryption", "object-lock", "accelerate", "analytics", + "intelligent-tiering", "inventory", "metrics", "logging", "notification", + "ownershipControls", "publicAccessBlock", "requestPayment", "uploads", "versions", +} + +// isListObjects reports whether req is a clean ListObjects (v1) or ListObjectsV2 request, +// i.e. a bucket-level GET with no sub-resource marker query parameters. Both v1 (no +// list-type) and v2 (list-type=2) match. +func isListObjects(req *http.Request) bool { + if req.Method != http.MethodGet { + return false + } + if !bucketOnlyPattern.MatchString(req.URL.Path) { + return false + } + query := req.URL.Query() + for _, marker := range listSubResourceMarkers { + if _, ok := query[marker]; ok { + return false + } + } + return true +} + // Router implements the interception logic for the s3proxy. type Router struct { region string @@ -374,8 +407,12 @@ func (r Router) getHandler(req *http.Request, client s3Client, matchingPath bool }) } - // Forward if path doesn't match + // Forward if path doesn't match. Bucket-level ListObjects requests land here (they have + // no object key); intercept them to report decrypted plaintext sizes. if !matchingPath { + if s3Client, ok := client.(*s3.Client); ok && isListObjects(req) { + return handleListObjects(s3Client, r.log, r.metrics) + } return forwardHandler() }