From 9265e81fe9b600f8503cf86645d469d04ccf4685 Mon Sep 17 00:00:00 2001 From: Chris Lu Date: Mon, 11 Aug 2025 10:31:01 -0700 Subject: [PATCH] S3 API: unsigned streaming (no cred) but chunks contain signatures (#7118) * This handles the case where we have unsigned streaming (no cred) but chunks contain signatures * Update chunked_reader_v4.go * address comments --- weed/s3api/chunked_bug_reproduction_test.go | 55 +++++++++++++++++++++ weed/s3api/chunked_reader_v4.go | 44 ++++++++++++----- 2 files changed, 86 insertions(+), 13 deletions(-) create mode 100644 weed/s3api/chunked_bug_reproduction_test.go diff --git a/weed/s3api/chunked_bug_reproduction_test.go b/weed/s3api/chunked_bug_reproduction_test.go new file mode 100644 index 000000000..dc02bc282 --- /dev/null +++ b/weed/s3api/chunked_bug_reproduction_test.go @@ -0,0 +1,55 @@ +package s3api + +import ( + "bytes" + "io" + "net/http" + "testing" + + "github.com/seaweedfs/seaweedfs/weed/s3api/s3err" +) + +// TestChunkedEncodingMixedFormat tests the fix for GitHub issue #6847 +// where AWS SDKs send mixed format: unsigned streaming headers but signed chunk data +func TestChunkedEncodingMixedFormat(t *testing.T) { + expectedContent := "hello world\n" + + // Create the problematic mixed format payload: + // - Unsigned streaming headers (STREAMING-UNSIGNED-PAYLOAD-TRAILER) + // - But chunk data contains chunk-signature headers + mixedFormatPayload := "c;chunk-signature=347f6c62acd95b7c6ae18648776024a9e8cd6151184a5e777ea8e1d9b4e45b3c\r\n" + + "hello world\n\r\n" + + "0;chunk-signature=1a99b7790b8db0f4bfc048c8802056c3179d561e40c073167e79db5f1a6af4b2\r\n" + + "x-amz-checksum-crc32:rwg7LQ==\r\n" + + "\r\n" + + // Create HTTP request with unsigned streaming headers + req, _ := http.NewRequest("PUT", "/test-bucket/test-object", bytes.NewReader([]byte(mixedFormatPayload))) + req.Header.Set("x-amz-content-sha256", "STREAMING-UNSIGNED-PAYLOAD-TRAILER") + req.Header.Set("x-amz-trailer", "x-amz-checksum-crc32") + + // Process through SeaweedFS chunked reader + iam := setupTestIAM() + reader, errCode := iam.newChunkedReader(req) + + if errCode != s3err.ErrNone { + t.Fatalf("Failed to create chunked reader: %v", errCode) + } + + // Read the content + actualContent, err := io.ReadAll(reader) + if err != nil { + t.Fatalf("Failed to read content: %v", err) + } + + // Should correctly extract just the content, ignoring chunk signatures + if string(actualContent) != expectedContent { + t.Errorf("Mixed format handling failed. Expected: %q, Got: %q", expectedContent, string(actualContent)) + } +} + +// setupTestIAM creates a test IAM instance using the same pattern as existing tests +func setupTestIAM() *IdentityAccessManagement { + iam := &IdentityAccessManagement{} + return iam +} diff --git a/weed/s3api/chunked_reader_v4.go b/weed/s3api/chunked_reader_v4.go index 5973cbcc2..ca35fe3cd 100644 --- a/weed/s3api/chunked_reader_v4.go +++ b/weed/s3api/chunked_reader_v4.go @@ -440,18 +440,35 @@ func (cr *s3ChunkedReader) Read(buf []byte) (n int, err error) { continue } case verifyChunk: - // Calculate the hashed chunk. - hashedChunk := hex.EncodeToString(cr.chunkSHA256Writer.Sum(nil)) - // Calculate the chunk signature. - newSignature := cr.getChunkSignature(hashedChunk) - if !compareSignatureV4(cr.chunkSignature, newSignature) { - // Chunk signature doesn't match we return signature does not match. - cr.err = errors.New(s3err.ErrMsgChunkSignatureMismatch) - return 0, cr.err + // Check if we have credentials for signature verification + // This handles the case where we have unsigned streaming (no cred) but chunks contain signatures + // + // BUG FIX for GitHub issue #6847: + // Some AWS SDK versions (Java 3.7.412+, .NET 4.0.0-preview.6+) send mixed format: + // - HTTP headers indicate unsigned streaming (STREAMING-UNSIGNED-PAYLOAD-TRAILER) + // - But chunk data contains chunk-signature headers (normally only for signed streaming) + // This causes a nil pointer dereference when trying to verify signatures without credentials + if cr.cred != nil { + // Normal signed streaming - verify the chunk signature + // Calculate the hashed chunk. + hashedChunk := hex.EncodeToString(cr.chunkSHA256Writer.Sum(nil)) + // Calculate the chunk signature. + newSignature := cr.getChunkSignature(hashedChunk) + if !compareSignatureV4(cr.chunkSignature, newSignature) { + // Chunk signature doesn't match we return signature does not match. + cr.err = errors.New(s3err.ErrMsgChunkSignatureMismatch) + return 0, cr.err + } + // Newly calculated signature becomes the seed for the next chunk + // this follows the chaining. + cr.seedSignature = newSignature + } else { + // For unsigned streaming, we should not verify chunk signatures even if they are present + // This fixes the bug where AWS SDKs send chunk signatures with unsigned streaming headers + glog.V(3).Infof("Skipping chunk signature verification for unsigned streaming") } - // Newly calculated signature becomes the seed for the next chunk - // this follows the chaining. - cr.seedSignature = newSignature + + // Common cleanup and state transition for both signed and unsigned streaming cr.chunkSHA256Writer.Reset() if cr.lastChunk { cr.state = eofChunk @@ -513,9 +530,10 @@ func readChunkLine(b *bufio.Reader) ([]byte, error) { if err != nil { // We always know when EOF is coming. // If the caller asked for a line, there should be a line. - if err == io.EOF { + switch err { + case io.EOF: err = io.ErrUnexpectedEOF - } else if err == bufio.ErrBufferFull { + case bufio.ErrBufferFull: err = errLineTooLong } return nil, err