From 6388dfca9b611ea884c9d977614e1e7085d3b97f Mon Sep 17 00:00:00 2001 From: Chris Lu Date: Thu, 12 Nov 2026 10:17:09 -0800 Subject: [PATCH] s3api: verify signed request bodies against the signed payload hash (#12577) * s3api: verify signed request bodies against the signed payload hash A SigV4 signature covers the declared x-amz-content-sha256, but only getRequestDataReader (PutObject, UploadPart) compared the body with it. Every other handler parsed the body as received, so a signed request, DeleteObjects for example, could be replayed with a different body. After a V4 signature verifies, wrap the body so it is handed out only once all of it has been read and matched against the hash the signature covered; a decoder that stops reading early never sees unverified bytes. getRequestDataReader unwraps it and keeps verifying uploads as they stream. Co-Authored-By: Claude Opus 5.5 * s3api: answer XAmzContentSHA256Mismatch for refused DeleteObjects bodies DeleteObjects answered a body that failed its payload-hash check with InternalError and CompleteMultipartUpload with MalformedXML. Name the check that failed instead, as PutObject does. Co-Authored-By: Claude Opus 5.5 * s3api: check Content-MD5 on signed request bodies Signature V2 signs Content-MD5 rather than a payload hash, and a V4 request sent with UNSIGNED-PAYLOAD may still sign it, but bodies read outside the upload path never compared it with the body. Check it with the payload hash once a V2 or V4 signature verifies: BadDigest on a mismatch, InvalidDigest when it is malformed. Co-Authored-By: Claude Opus 5.5 * s3api: verify presigned uploads against a signed payload hash A presigned V4 URL can sign X-Amz-Content-Sha256 as a query parameter, but getRequestDataReader compared upload bodies only with the header of that name, so the signed hash went unchecked. Verify uploads against the hash the signature covered. Co-Authored-By: Claude Opus 5.5 * s3api: check Content-MD5 on bodies buffered for their payload hash IAM and other non-S3 requests signed without x-amz-content-sha256 have their body buffered and hashed during signature verification. That buffer counted as already bound, so their Content-MD5 was never checked. Co-Authored-By: Claude Opus 5.5 * s3api: check Content-MD5 on directory marker bodies A PUT of a key ending in "/" stores its body on the directory entry without going through putToFiler, so its Content-MD5 was never compared with the body. Answer BadDigest on a mismatch, as putToFiler does. Co-Authored-By: Claude Opus 5.5 * s3api: wrap and unwrap signed bodies through helpers Co-Authored-By: Claude Opus 5.5 --------- Co-authored-by: Claude Opus 5.5 --- weed/s3api/auth_signature_v2.go | 11 +- weed/s3api/auth_signature_v4.go | 16 +- weed/s3api/request_body.go | 149 ++++++++++- weed/s3api/request_body_test.go | 241 ++++++++++++++++++ weed/s3api/s3api_object_handlers_delete.go | 2 +- weed/s3api/s3api_object_handlers_multipart.go | 2 +- weed/s3api/s3api_object_handlers_put.go | 4 + weed/s3api/s3api_put_object_helper.go | 9 +- .../s3api_storage_class_validation_test.go | 17 ++ 9 files changed, 436 insertions(+), 15 deletions(-) diff --git a/weed/s3api/auth_signature_v2.go b/weed/s3api/auth_signature_v2.go index 5148be2218..743c587a4a 100644 --- a/weed/s3api/auth_signature_v2.go +++ b/weed/s3api/auth_signature_v2.go @@ -63,10 +63,17 @@ var resourceList = []string{ // Verify if request has AWS Signature Version '2'. func (iam *IdentityAccessManagement) isReqAuthenticatedV2(r *http.Request) (*Identity, s3err.ErrorCode) { + var identity *Identity + var errCode s3err.ErrorCode if isRequestSignatureV2(r) { - return iam.doesSignV2Match(r) + identity, errCode = iam.doesSignV2Match(r) + } else { + identity, errCode = iam.doesPresignV2SignatureMatch(r) } - return iam.doesPresignV2SignatureMatch(r) + if errCode != s3err.ErrNone { + return identity, errCode + } + return identity, bindSignedPayload(r, unsignedPayload) } func (iam *IdentityAccessManagement) doesPolicySignatureV2Match(ctx context.Context, formValues http.Header) (*Identity, s3err.ErrorCode) { diff --git a/weed/s3api/auth_signature_v4.go b/weed/s3api/auth_signature_v4.go index f18f12f4eb..2bd018bdd1 100644 --- a/weed/s3api/auth_signature_v4.go +++ b/weed/s3api/auth_signature_v4.go @@ -26,7 +26,6 @@ import ( "encoding/base64" "encoding/hex" "errors" - "io" "net" "net/http" "net/url" @@ -47,15 +46,14 @@ import ( ) func (iam *IdentityAccessManagement) reqSignatureV4Verify(r *http.Request) (*Identity, s3err.ErrorCode) { - switch { - case isRequestSignatureV4(r): - identity, _, errCode := iam.doesSignatureMatch(r) - return identity, errCode - case isRequestPresignedSignatureV4(r): - identity, _, errCode := iam.doesPresignedSignatureMatch(r) + if !isRequestSignatureV4(r) && !isRequestPresignedSignatureV4(r) { + return nil, s3err.ErrAccessDenied + } + identity, _, _, authInfo, errCode := iam.verifyV4Signature(r, false) + if errCode != s3err.ErrNone { return identity, errCode } - return nil, s3err.ErrAccessDenied + return identity, bindSignedPayload(r, authInfo.HashedPayload) } // Constants specific to this file @@ -80,7 +78,7 @@ func streamHashRequestBody(r *http.Request, sizeLimit int64) (string, error) { return "", err } - r.Body = io.NopCloser(bytes.NewReader(body)) + (&signedPayloadBody{data: bytes.NewReader(body)}).wrap(&r.Body) if len(body) == 0 { return emptySHA256, nil diff --git a/weed/s3api/request_body.go b/weed/s3api/request_body.go index f55fc5c3e3..462a218e5b 100644 --- a/weed/s3api/request_body.go +++ b/weed/s3api/request_body.go @@ -1,12 +1,22 @@ package s3api import ( + "bytes" + "crypto/md5" + "crypto/sha256" "errors" "io" "net/http" + + "github.com/seaweedfs/seaweedfs/weed/s3api/s3_constants" + "github.com/seaweedfs/seaweedfs/weed/s3api/s3err" ) -var errRequestBodyTooLarge = errors.New("request body too large") +var ( + errRequestBodyTooLarge = errors.New("request body too large") + errContentSha256Mismatch = errors.New(s3err.ErrMsgContentSha256Mismatch) + errContentMd5Mismatch = errors.New("the Content-MD5 you specified did not match what was received") +) func readRequestBody(r *http.Request, limit int64) ([]byte, error) { if r.Body == nil { @@ -21,3 +31,140 @@ func readRequestBody(r *http.Request, limit int64) ([]byte, error) { } return body, nil } + +// signedPayloadBody hands out a signed request body only after all of it has +// been read and matched against the payload hash the signature covered and +// any Content-MD5, so a handler that stops reading early never acts on +// unverified bytes. +type signedPayloadBody struct { + body io.ReadCloser + size int64 + limit int64 + sha256 []byte + md5 []byte + data *bytes.Reader + err error +} + +func (b *signedPayloadBody) Read(p []byte) (int, error) { + if b.data == nil && b.err == nil { + b.data, b.err = b.load() + } + if b.err != nil { + return 0, b.err + } + return b.data.Read(p) +} + +func (b *signedPayloadBody) load() (*bytes.Reader, error) { + if b.size > b.limit { + return nil, errRequestBodyTooLarge + } + data, err := io.ReadAll(io.LimitReader(b.body, b.limit+1)) + if err != nil { + return nil, err + } + if int64(len(data)) > b.limit { + return nil, errRequestBodyTooLarge + } + if b.sha256 != nil { + if sum := sha256.Sum256(data); !bytes.Equal(sum[:], b.sha256) { + return nil, errContentSha256Mismatch + } + } + if b.md5 != nil { + if sum := md5.Sum(data); !bytes.Equal(sum[:], b.md5) { + return nil, errContentMd5Mismatch + } + } + return bytes.NewReader(data), nil +} + +func (b *signedPayloadBody) Close() error { + return b.body.Close() +} + +func (b *signedPayloadBody) wrap(body *io.ReadCloser) { + b.body = *body + *body = b +} + +// unwrapSignedPayload puts back the raw body of a signedPayloadBody nothing has +// read yet, for readers that verify it as it streams, and returns the hash it +// was bound to. +func unwrapSignedPayload(body *io.ReadCloser) []byte { + if b, ok := (*body).(*signedPayloadBody); ok && b.data == nil && b.err == nil { + *body = b.body + return b.sha256 + } + return nil +} + +// bindSignedPayload ties the request body to hashedPayload, the payload hash a +// V4 signature covered (UNSIGNED-PAYLOAD for V2), and to any Content-MD5. +// Uploads read through getRequestDataReader, which verifies the body as it +// streams instead. +func bindSignedPayload(r *http.Request, hashedPayload string) s3err.ErrorCode { + if bound, ok := r.Body.(*signedPayloadBody); ok { + if bound.sha256 != nil || bound.md5 != nil { + return s3err.ErrNone + } + // streamHashRequestBody buffered it and signed its hash; only + // Content-MD5 is left to check. + hashedPayload = unsignedPayload + } + switch hashedPayload { + case streamingContentSHA256, streamingContentSHA256Trailer, streamingUnsignedPayload: + return s3err.ErrNone + } + expected, valid := decodeContentSha256(hashedPayload) + hasBody := r.Body != nil && r.Body != http.NoBody && r.ContentLength != 0 + if !valid { + if hasBody { + return s3err.ErrInvalidArgument + } + return s3err.ErrNone + } + var contentMd5 []byte + if _, present := r.Header["Content-Md5"]; present { + decoded, err := validateContentMd5(r.Header) + if err != nil || len(decoded) != md5.Size { + return s3err.ErrInvalidDigest + } + contentMd5 = decoded + } + if expected == nil && contentMd5 == nil { + return s3err.ErrNone + } + if !hasBody { + if emptySum := sha256.Sum256(nil); expected != nil && !bytes.Equal(expected, emptySum[:]) { + return s3err.ErrContentSHA256Mismatch + } + if emptySum := md5.Sum(nil); contentMd5 != nil && !bytes.Equal(contentMd5, emptySum[:]) { + return s3err.ErrBadDigest + } + return s3err.ErrNone + } + signed := &signedPayloadBody{ + size: r.ContentLength, + limit: s3_constants.GetRequestBodyLimitFromContext(r, iamRequestBodyLimit), + sha256: expected, + md5: contentMd5, + } + signed.wrap(&r.Body) + return s3err.ErrNone +} + +// requestBodyErrorCode names the check a failed body read tripped, or returns +// fallback for any other read error. +func requestBodyErrorCode(err error, fallback s3err.ErrorCode) s3err.ErrorCode { + switch { + case errors.Is(err, errContentSha256Mismatch): + return s3err.ErrContentSHA256Mismatch + case errors.Is(err, errContentMd5Mismatch): + return s3err.ErrBadDigest + case errors.Is(err, errRequestBodyTooLarge): + return s3err.ErrEntityTooLarge + } + return fallback +} diff --git a/weed/s3api/request_body_test.go b/weed/s3api/request_body_test.go index 0fc05b61ef..c4c092ce24 100644 --- a/weed/s3api/request_body_test.go +++ b/weed/s3api/request_body_test.go @@ -1,13 +1,22 @@ package s3api import ( + "bytes" + "crypto/sha256" + "encoding/hex" + "encoding/xml" "io" "net/http" "net/http/httptest" "strings" + "sync" "testing" + "time" + "github.com/seaweedfs/seaweedfs/weed/pb/iam_pb" + "github.com/seaweedfs/seaweedfs/weed/s3api/s3err" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestReadRequestBodyLeavesOversizedBodyDrainable(t *testing.T) { @@ -22,3 +31,235 @@ func TestReadRequestBodyLeavesOversizedBodyDrainable(t *testing.T) { assert.NoError(t, readErr) assert.Equal(t, "ef", string(remaining)) } + +func newSignedPayloadTestIAM(t *testing.T) *IdentityAccessManagement { + iam := &IdentityAccessManagement{ + hashes: make(map[string]*sync.Pool), + hashCounters: make(map[string]*int32), + } + err := iam.loadS3ApiConfiguration(&iam_pb.S3ApiConfiguration{ + Identities: []*iam_pb.Identity{{ + Name: "signer", + Credentials: []*iam_pb.Credential{{AccessKey: "access_key_1", SecretKey: "secret_key_1"}}, + Actions: []string{"Read", "Write"}, + }}, + }) + if err != nil { + t.Fatal(err) + } + return iam +} + +func deleteObjectsBody(keys ...string) []byte { + var b strings.Builder + b.WriteString("") + for _, k := range keys { + b.WriteString("" + k + "") + } + b.WriteString("") + return []byte(b.String()) +} + +func TestSignedRequestBodyIsBoundToPayloadHash(t *testing.T) { + iam := newSignedPayloadTestIAM(t) + approved := deleteObjectsBody("tables/a/own.txt") + swapped := deleteObjectsBody("tables/b/victim.txt") + + newSwapped := func(body []byte) *http.Request { + req := mustNewSignedRequest(http.MethodPost, "http://127.0.0.1:9000/warehouse?delete", int64(len(approved)), bytes.NewReader(approved), t) + req.Body = io.NopCloser(bytes.NewReader(body)) + req.ContentLength = int64(len(body)) + return req + } + + t.Run("signed body is readable", func(t *testing.T) { + req := mustNewSignedRequest(http.MethodPost, "http://127.0.0.1:9000/warehouse?delete", int64(len(approved)), bytes.NewReader(approved), t) + _, errCode := iam.reqSignatureV4Verify(req) + require.Equal(t, s3err.ErrNone, errCode) + body, err := io.ReadAll(req.Body) + require.NoError(t, err) + assert.Equal(t, approved, body) + }) + + t.Run("replaced body is refused", func(t *testing.T) { + req := newSwapped(swapped) + _, errCode := iam.reqSignatureV4Verify(req) + require.Equal(t, s3err.ErrNone, errCode) + body, err := io.ReadAll(req.Body) + assert.ErrorIs(t, err, errContentSha256Mismatch) + assert.Empty(t, body) + }) + + t.Run("decoder that stops early sees no replaced bytes", func(t *testing.T) { + padded := append(append([]byte{}, swapped...), bytes.Repeat([]byte(" "), 64<<10)...) + req := newSwapped(padded) + _, errCode := iam.reqSignatureV4Verify(req) + require.Equal(t, s3err.ErrNone, errCode) + var parsed DeleteObjectsRequest + err := xml.NewDecoder(req.Body).Decode(&parsed) + assert.ErrorIs(t, err, errContentSha256Mismatch) + assert.Empty(t, parsed.Objects) + }) + + t.Run("emptied body is refused", func(t *testing.T) { + req := newSwapped(nil) + req.Body = http.NoBody + _, errCode := iam.reqSignatureV4Verify(req) + assert.Equal(t, s3err.ErrContentSHA256Mismatch, errCode) + }) + + t.Run("oversized body is refused before it is read", func(t *testing.T) { + req := newSwapped(swapped) + req.ContentLength = iamRequestBodyLimit + 1 + _, errCode := iam.reqSignatureV4Verify(req) + require.Equal(t, s3err.ErrNone, errCode) + _, err := io.ReadAll(req.Body) + assert.ErrorIs(t, err, errRequestBodyTooLarge) + }) +} + +func TestGetRequestDataReaderStreamsSignedUploads(t *testing.T) { + iam := newSignedPayloadTestIAM(t) + s3a := &S3ApiServer{iam: iam} + signed := []byte("object data") + req := mustNewSignedRequest(http.MethodPut, "http://127.0.0.1:9000/bucket/key", int64(len(signed)), bytes.NewReader(signed), t) + raw := io.NopCloser(bytes.NewReader([]byte("other data!"))) + req.Body = raw + + _, errCode := iam.reqSignatureV4Verify(req) + require.Equal(t, s3err.ErrNone, errCode) + dataReader, errCode := getRequestDataReader(s3a, req) + require.Equal(t, s3err.ErrNone, errCode) + + assert.Equal(t, raw, req.Body) + _, err := io.ReadAll(dataReader) + assert.ErrorContains(t, err, s3err.ErrMsgContentSha256Mismatch) +} + +func TestDeleteObjectsRefusesReplacedBody(t *testing.T) { + iam := newSignedPayloadTestIAM(t) + s3a := &S3ApiServer{iam: iam} + approved := deleteObjectsBody("tables/a/own.txt") + swapped := deleteObjectsBody("tables/b/victim.txt") + req := mustNewSignedRequest(http.MethodPost, "http://127.0.0.1:9000/warehouse?delete", int64(len(approved)), bytes.NewReader(approved), t) + req.Body = io.NopCloser(bytes.NewReader(swapped)) + req.ContentLength = int64(len(swapped)) + _, errCode := iam.reqSignatureV4Verify(req) + require.Equal(t, s3err.ErrNone, errCode) + + rec := httptest.NewRecorder() + s3a.DeleteMultipleObjectsHandler(rec, req) + + assert.Equal(t, http.StatusBadRequest, rec.Code) + assert.Contains(t, rec.Body.String(), "XAmzContentSHA256Mismatch") +} + +func TestSignedRequestBodyIsBoundToContentMd5(t *testing.T) { + approved := deleteObjectsBody("tables/a/own.txt") + swapped := deleteObjectsBody("tables/b/victim.txt") + const url = "http://127.0.0.1:9000/warehouse?delete" + + t.Run("v4 unsigned payload", func(t *testing.T) { + iam := newSignedPayloadTestIAM(t) + req := mustNewRequest(http.MethodPost, url, int64(len(approved)), bytes.NewReader(approved), t) + req.Header.Set("X-Amz-Content-Sha256", unsignedPayload) + require.NoError(t, signRequestV4(req, "access_key_1", "secret_key_1")) + req.Body = io.NopCloser(bytes.NewReader(swapped)) + req.ContentLength = int64(len(swapped)) + + _, errCode := iam.reqSignatureV4Verify(req) + require.Equal(t, s3err.ErrNone, errCode) + _, err := io.ReadAll(req.Body) + assert.ErrorIs(t, err, errContentMd5Mismatch) + assert.Equal(t, s3err.ErrBadDigest, requestBodyErrorCode(err, s3err.ErrInternalError)) + }) + + t.Run("v2", func(t *testing.T) { + iam := setupTestIAMForV2Auth() + cred := iam.identities[0].Credentials[0] + newV2Request := func(body []byte) *http.Request { + req := httptest.NewRequest(http.MethodPost, url, bytes.NewReader(body)) + req.Header.Set("Date", time.Now().UTC().Format(http.TimeFormat)) + req.Header.Set("Content-Md5", getMD5HashBase64(approved)) + req.Header.Set("Authorization", signatureV2(cred, req.Method, req.URL.Path, req.URL.Query().Encode(), req.Header)) + return req + } + + req := newV2Request(approved) + _, errCode := iam.isReqAuthenticatedV2(req) + require.Equal(t, s3err.ErrNone, errCode) + body, err := io.ReadAll(req.Body) + require.NoError(t, err) + assert.Equal(t, approved, body) + + req = newV2Request(swapped) + _, errCode = iam.isReqAuthenticatedV2(req) + require.Equal(t, s3err.ErrNone, errCode) + _, err = io.ReadAll(req.Body) + assert.ErrorIs(t, err, errContentMd5Mismatch) + }) + + t.Run("malformed digest", func(t *testing.T) { + iam := newSignedPayloadTestIAM(t) + req := mustNewRequest(http.MethodPost, url, int64(len(approved)), bytes.NewReader(approved), t) + req.Header.Set("Content-Md5", "bm90LWFuLW1kNQ==") + require.NoError(t, signRequestV4(req, "access_key_1", "secret_key_1")) + _, errCode := iam.reqSignatureV4Verify(req) + assert.Equal(t, s3err.ErrInvalidDigest, errCode) + }) +} + +func TestGetRequestDataReaderVerifiesPresignedPayloadHash(t *testing.T) { + iam := newSignedPayloadTestIAM(t) + s3a := &S3ApiServer{iam: iam} + signed := []byte("object data") + sum := sha256.Sum256(signed) + req := mustNewRequest(http.MethodPut, "http://127.0.0.1:9000/bucket/key?X-Amz-Content-Sha256="+hex.EncodeToString(sum[:]), int64(len(signed)), bytes.NewReader(signed), t) + req.Header.Del("X-Amz-Content-Sha256") + req.Header.Del("Content-Md5") + require.NoError(t, preSignV4(iam, req, "access_key_1", "secret_key_1", 600)) + req.Body = io.NopCloser(bytes.NewReader([]byte("other data!"))) + + _, errCode := iam.reqSignatureV4Verify(req) + require.Equal(t, s3err.ErrNone, errCode) + dataReader, errCode := getRequestDataReader(s3a, req) + require.Equal(t, s3err.ErrNone, errCode) + _, err := io.ReadAll(dataReader) + assert.ErrorContains(t, err, s3err.ErrMsgContentSha256Mismatch) +} + +func TestPrebufferedSignedBodyChecksContentMd5(t *testing.T) { + iam := newSignedPayloadTestIAM(t) + newIAMRequest := func(contentMd5 string) *http.Request { + body := []byte("Action=ListUsers&Version=2010-05-08") + req := httptest.NewRequest(http.MethodPost, "http://127.0.0.1:9000/", bytes.NewReader(body)) + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + req.Header.Set("Content-Md5", contentMd5) + now := time.Now().UTC() + req.Header.Set("X-Amz-Date", now.Format(iso8601Format)) + scope := now.Format(yyyymmdd) + "/us-east-1/iam/aws4_request" + signed := http.Header{ + "content-md5": {contentMd5}, + "content-type": {req.Header.Get("Content-Type")}, + "host": {req.Host}, + "x-amz-date": {req.Header.Get("X-Amz-Date")}, + } + sum := sha256.Sum256(body) + canonical := getCanonicalRequest(signed, hex.EncodeToString(sum[:]), "", "/", http.MethodPost) + signature := getSignature(getSigningKey("secret_key_1", now.Format(yyyymmdd), "us-east-1", "iam"), getStringToSign(canonical, now, scope)) + req.Header.Set("Authorization", "AWS4-HMAC-SHA256 Credential=access_key_1/"+scope+", SignedHeaders=content-md5;content-type;host;x-amz-date, Signature="+signature) + return req + } + + req := newIAMRequest(getMD5HashBase64([]byte("Action=ListUsers&Version=2010-05-08"))) + _, errCode := iam.reqSignatureV4Verify(req) + require.Equal(t, s3err.ErrNone, errCode) + _, err := io.ReadAll(req.Body) + assert.NoError(t, err) + + req = newIAMRequest(getMD5HashBase64([]byte("other"))) + _, errCode = iam.reqSignatureV4Verify(req) + require.Equal(t, s3err.ErrNone, errCode) + _, err = io.ReadAll(req.Body) + assert.ErrorIs(t, err, errContentMd5Mismatch) +} diff --git a/weed/s3api/s3api_object_handlers_delete.go b/weed/s3api/s3api_object_handlers_delete.go index 147eb618a3..535ef4a884 100644 --- a/weed/s3api/s3api_object_handlers_delete.go +++ b/weed/s3api/s3api_object_handlers_delete.go @@ -440,7 +440,7 @@ func (s3a *S3ApiServer) DeleteMultipleObjectsHandler(w http.ResponseWriter, r *h deleteXMLBytes, err := io.ReadAll(r.Body) if err != nil { - s3err.WriteErrorResponse(w, r, s3err.ErrInternalError) + s3err.WriteErrorResponse(w, r, requestBodyErrorCode(err, s3err.ErrInternalError)) return } diff --git a/weed/s3api/s3api_object_handlers_multipart.go b/weed/s3api/s3api_object_handlers_multipart.go index ca39881a12..ace520829e 100644 --- a/weed/s3api/s3api_object_handlers_multipart.go +++ b/weed/s3api/s3api_object_handlers_multipart.go @@ -164,7 +164,7 @@ func (s3a *S3ApiServer) CompleteMultipartUploadHandler(w http.ResponseWriter, r parts := &CompleteMultipartUpload{} if err := xmlDecoder(r.Body, parts, r.ContentLength); err != nil { - s3err.WriteErrorResponse(w, r, s3err.ErrMalformedXML) + s3err.WriteErrorResponse(w, r, requestBodyErrorCode(err, s3err.ErrMalformedXML)) return } diff --git a/weed/s3api/s3api_object_handlers_put.go b/weed/s3api/s3api_object_handlers_put.go index 2e895d3e47..e2954e468a 100644 --- a/weed/s3api/s3api_object_handlers_put.go +++ b/weed/s3api/s3api_object_handlers_put.go @@ -231,6 +231,10 @@ func (s3a *S3ApiServer) PutObjectHandler(w http.ResponseWriter, r *http.Request) // Compute MD5 for ETag (md5.Sum of nil/empty = MD5 of empty content) dirMd5 := md5.Sum(dirContent) + if expectedMd5, _ := validateContentMd5(r.Header); len(expectedMd5) > 0 && !bytes.Equal(dirMd5[:], expectedMd5) { + s3err.WriteErrorResponse(w, r, s3err.ErrBadDigest) + return + } dirEtag := fmt.Sprintf("%x", dirMd5) // SSE applies to marker content too — it is stored in entry.Content diff --git a/weed/s3api/s3api_put_object_helper.go b/weed/s3api/s3api_put_object_helper.go index d6042aec55..579f8e704d 100644 --- a/weed/s3api/s3api_put_object_helper.go +++ b/weed/s3api/s3api_put_object_helper.go @@ -19,6 +19,7 @@ import ( // newChunkedReader. func getRequestDataReader(s3a *S3ApiServer, r *http.Request) (io.ReadCloser, s3err.ErrorCode) { var s3ErrCode s3err.ErrorCode + signedSha256 := unwrapSignedPayload(&r.Body) dataReader := r.Body rAuthType := getRequestAuthType(r) if rAuthType == authTypeStreamingSigned || rAuthType == authTypeStreamingUnsigned { @@ -32,6 +33,9 @@ func getRequestDataReader(s3a *S3ApiServer, r *http.Request) (io.ReadCloser, s3e if !valid { return nil, s3err.ErrInvalidArgument } + if signedSha256 != nil { + expected = signedSha256 + } if expected != nil && dataReader != nil { if r.ContentLength == 0 { // Handlers may never read an empty body, so check it now. @@ -51,7 +55,10 @@ func getRequestDataReader(s3a *S3ApiServer, r *http.Request) (io.ReadCloser, s3e // streams. Sentinel values (streaming, unsigned) carry no hash and are skipped; // anything that decodes to neither returns valid=false. func expectedContentSha256(r *http.Request) (expected []byte, valid bool) { - v := r.Header.Get("X-Amz-Content-Sha256") + return decodeContentSha256(r.Header.Get("X-Amz-Content-Sha256")) +} + +func decodeContentSha256(v string) (expected []byte, valid bool) { if v == "" || v == unsignedPayload || v == streamingContentSHA256 || v == streamingContentSHA256Trailer || v == streamingUnsignedPayload { return nil, true diff --git a/weed/s3api/s3api_storage_class_validation_test.go b/weed/s3api/s3api_storage_class_validation_test.go index d6d51d1693..de1e8fd343 100644 --- a/weed/s3api/s3api_storage_class_validation_test.go +++ b/weed/s3api/s3api_storage_class_validation_test.go @@ -230,6 +230,23 @@ func TestPutObjectDirectoryMarkerStoresHeaders(t *testing.T) { require.ElementsMatch(t, []Tag{{Key: "k1", Value: "v1"}, {Key: "k2", Value: "v2"}}, tagging.TagSet.Tag) } +func TestPutObjectDirectoryMarkerChecksContentMd5(t *testing.T) { + s3a, filer, _ := newHeaderTestServer(t, "") + req, probe := newHeaderTestRequest("/b/dir/", "dir/", "marker", "Content-Md5", getMD5HashBase64([]byte("other"))) + + s3a.PutObjectHandler(probe, req) + + require.Equal(t, http.StatusBadRequest, probe.Code, probe.Body.String()) + require.Contains(t, probe.Body.String(), "BadDigest") + filer.mu.Lock() + require.Nil(t, filer.entries["/buckets/b/dir"], "directory marker created") + filer.mu.Unlock() + + req, probe = newHeaderTestRequest("/b/dir/", "dir/", "marker", "Content-Md5", getMD5HashBase64([]byte("marker"))) + s3a.PutObjectHandler(probe, req) + require.Equal(t, http.StatusOK, probe.Code, probe.Body.String()) +} + // A valid storage class still stores, and the content type is still sniffed. func TestPutObjectValidStorageClassSniffsContentType(t *testing.T) { s3a, filer, volume := newHeaderTestServer(t, "")