From 93361f6f317f4af65e3cc90e4cd4217acffa7a0f Mon Sep 17 00:00:00 2001 From: Piotr Szafarczyk Date: Thu, 25 Jun 2026 14:56:09 +0200 Subject: [PATCH] fix(router): return plaintext ETag on intercepted GetObject MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Intercepted GETs decrypt the body but returned the upstream ETag, which S3 computes over the ciphertext at rest. Clients/SDKs that validate the body against the ETag, or cache by it, saw a mismatch. Override the response ETag with md5(plaintext) — the ETag S3 would have produced for the unencrypted object — computed on the fly from the buffer already in memory, so no stored metadata or migration is needed. Pass-through objects (no DEK tag) keep the upstream ETag. PUT-response and HEAD ETags still reflect the ciphertext and remain a known gap. --- CHANGELOG.md | 12 +++++++ s3proxy/internal/router/object.go | 9 ++++++ s3proxy/internal/router/router_test.go | 44 ++++++++++++++++++++++++++ 3 files changed, 65 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index e779d8f..88c4fc4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,18 @@ Two version streams move independently: ## [Unreleased] +### Fixed +- **`GetObject` returns the plaintext ETag.** Intercepted GETs decrypt the + body but previously returned the upstream ETag, which S3 computes over the + ciphertext at rest. Clients (or SDKs) that validate the body against the + ETag, or cache by it, saw a mismatch. The proxy now overrides the response + ETag with `md5(plaintext)` — the ETag S3 would have produced for the + unencrypted object — computed on the fly from the buffer already held in + memory, so no stored metadata or migration is needed. Pass-through objects + (no DEK tag) keep the upstream ETag, which already describes the delivered + bytes. PUT-response and HEAD ETags still reflect the ciphertext and remain + a known consistency gap. + ### Chart - **`chart/1.9.3`** — Dashboard usability + Go runtime panels. diff --git a/s3proxy/internal/router/object.go b/s3proxy/internal/router/object.go index d7be9d6..4585acd 100644 --- a/s3proxy/internal/router/object.go +++ b/s3proxy/internal/router/object.go @@ -9,6 +9,7 @@ package router import ( "context" + "crypto/md5" "encoding/hex" "errors" "fmt" @@ -149,6 +150,14 @@ func (o object) get(w http.ResponseWriter, r *http.Request) { http.Error(w, "failed to decrypt object", http.StatusInternalServerError) return } + + // The upstream ETag is S3's MD5 of the ciphertext at rest, which does not + // match the decrypted body we deliver. Override it with the plaintext MD5 + // so clients that validate or cache by ETag see a consistent value. Single + // part only (multipart is blocked/forwarded), so ETag == hex(md5(content)). + //nolint:gosec // MD5 is not used as a security primitive here; it is the S3 ETag definition. + sum := md5.Sum(plaintext) + w.Header().Set("ETag", hex.EncodeToString(sum[:])) } select { diff --git a/s3proxy/internal/router/router_test.go b/s3proxy/internal/router/router_test.go index 2eae233..adf7fa7 100644 --- a/s3proxy/internal/router/router_test.go +++ b/s3proxy/internal/router/router_test.go @@ -8,6 +8,7 @@ package router import ( "bytes" "context" + "crypto/md5" "encoding/hex" "io" "log/slog" @@ -198,6 +199,49 @@ func TestGetObjectUsesRouterKEK(t *testing.T) { assert.Equal(t, "secret payload", rec.Body.String()) } +func TestGetObjectReturnsPlaintextETag(t *testing.T) { + keks := newTestKEKs(t, "expected encryption key") + version, curKEK := keks.Current() + plaintext := []byte("secret payload") + client := newEncryptedGetObjectClient(t, curKEK, version, plaintext) + // Upstream ETag describes the ciphertext at rest, not the body we deliver. + upstreamETag := "\"deadbeefdeadbeefdeadbeefdeadbeef\"" + client.getObjectOut.ETag = &upstreamETag + router := Router{keks: keks, log: testLogger()} + req := httptest.NewRequest(http.MethodGet, "/bucket/key", nil) + rec := httptest.NewRecorder() + + router.getHandler(req, client, true, "key", "bucket").ServeHTTP(rec, req) + + require.Equal(t, http.StatusOK, rec.Code) + want := md5.Sum(plaintext) + assert.Equal(t, hex.EncodeToString(want[:]), rec.Header().Get("ETag")) + assert.NotEqual(t, strings.Trim(upstreamETag, "\""), rec.Header().Get("ETag")) +} + +func TestGetObjectPassThroughKeepsUpstreamETag(t *testing.T) { + // No DEK metadata tag: the object was not proxy-encrypted, so the upstream + // ETag already describes the bytes we deliver and must be left untouched. + body := []byte("plain passthrough") + upstreamETag := "\"deadbeefdeadbeefdeadbeefdeadbeef\"" + client := &recordingS3Client{ + getObjectOut: &s3.GetObjectOutput{ + Body: io.NopCloser(bytes.NewReader(body)), + ContentLength: awsInt64(int64(len(body))), + ETag: &upstreamETag, + }, + } + router := Router{keks: newTestKEKs(t, "expected encryption key"), log: testLogger()} + req := httptest.NewRequest(http.MethodGet, "/bucket/key", nil) + rec := httptest.NewRecorder() + + router.getHandler(req, client, true, "key", "bucket").ServeHTTP(rec, req) + + require.Equal(t, http.StatusOK, rec.Code) + assert.Equal(t, body, rec.Body.Bytes()) + assert.Equal(t, strings.Trim(upstreamETag, "\""), rec.Header().Get("ETag")) +} + func TestGetObjectFailsWithWrongRouterKEK(t *testing.T) { storedKeks := newTestKEKs(t, "old encryption key") routerKeks := newTestKEKs(t, "new encryption key")