diff --git a/s3api/utils/unsigned-chunk-reader.go b/s3api/utils/unsigned-chunk-reader.go index d7067f1..24e26d1 100644 --- a/s3api/utils/unsigned-chunk-reader.go +++ b/s3api/utils/unsigned-chunk-reader.go @@ -35,8 +35,9 @@ import ( ) var ( - trailerDelim = []byte{'\n', '\r', '\n'} - errMalformedEncoding = errors.New("malformed chunk encoding") + trailerDelim = []byte{'\n', '\r', '\n'} + minChunkSize int64 = 8192 + errMalformedEncoding = errors.New("malformed chunk encoding") ) type UnsignedChunkReader struct { @@ -46,6 +47,9 @@ type UnsignedChunkReader struct { hasher hash.Hash stash []byte offset int + // this data is necessary for 'InvalidChunkSizeError' error + // TODO: add 'Chunk' and 'BadChunkSize' in the error + chunkSizes []int64 } func NewUnsignedChunkReader(r io.Reader, ct checksumType) (*UnsignedChunkReader, error) { @@ -65,6 +69,7 @@ func NewUnsignedChunkReader(r io.Reader, ct checksumType) (*UnsignedChunkReader, checksumType: ct, stash: make([]byte, 0), hasher: hasher, + chunkSizes: []int64{}, }, nil } @@ -188,11 +193,36 @@ func (ucr *UnsignedChunkReader) extractChunkSize() (int64, error) { return 0, s3err.GetAPIError(s3err.ErrIncompleteBody) } + if !ucr.isValidChunkSize(chunkSize) { + return chunkSize, s3err.GetAPIError(s3err.ErrInvalidChunkSize) + } + + ucr.chunkSizes = append(ucr.chunkSizes, chunkSize) + debuglogger.Infof("chunk size extracted: %v", chunkSize) return chunkSize, nil } +// isValidChunkSize checks if the parsed chunk size is valid +// they follow one rule: all chunk sizes except for the last one +// should be greater than 8192 +func (ucr *UnsignedChunkReader) isValidChunkSize(size int64) bool { + if len(ucr.chunkSizes) == 0 { + // any valid number is valid as a first chunk size + return true + } + + lastChunkSize := ucr.chunkSizes[len(ucr.chunkSizes)-1] + // any chunk size, except the last one should be greater than 8192 + if size != 0 && lastChunkSize < minChunkSize { + debuglogger.Logf("invalid chunk size %v", lastChunkSize) + return false + } + + return true +} + // Reads and validates the trailer at the end func (ucr *UnsignedChunkReader) readTrailer() error { var trailerBuffer bytes.Buffer diff --git a/s3err/s3err.go b/s3err/s3err.go index 6936a48..70ee11e 100644 --- a/s3err/s3err.go +++ b/s3err/s3err.go @@ -180,6 +180,7 @@ const ( ErrInvalidLocationConstraint ErrInvalidArgument ErrMalformedTrailer + ErrInvalidChunkSize // Non-AWS errors ErrExistingObjectIsDirectory @@ -804,6 +805,11 @@ var errorCodeResponse = map[ErrorCode]APIError{ Description: "The request contained trailing data that was not well-formed or did not conform to our published schema.", HTTPStatusCode: http.StatusBadRequest, }, + ErrInvalidChunkSize: { + Code: "InvalidChunkSizeError", + Description: "Only the last chunk is allowed to have a size less than 8192 bytes", + HTTPStatusCode: http.StatusBadRequest, + }, // non aws errors ErrExistingObjectIsDirectory: { diff --git a/tests/integration/group-tests.go b/tests/integration/group-tests.go index 9e3976d..4ee9bff 100644 --- a/tests/integration/group-tests.go +++ b/tests/integration/group-tests.go @@ -1101,6 +1101,7 @@ func TestUnsignedStreaminPayloadTrailer(ts *TestState) { ts.Run(UnsignedStreamingPayloadTrailer_multiple_checksum_headers) ts.Run(UnsignedStreamingPayloadTrailer_sdk_algo_and_trailer_mismatch) ts.Run(UnsignedStreamingPayloadTrailer_incomplete_body) + ts.Run(UnsignedStreamingPayloadTrailer_invalid_chunk_size) ts.Run(UnsignedStreamingPayloadTrailer_no_trailer_should_calculate_crc64nvme) ts.Run(UnsignedStreamingPayloadTrailer_no_payload_trailer_only_headers) ts.Run(UnsignedStreamingPayloadTrailer_success_both_sdk_algo_and_trailer) @@ -1754,6 +1755,7 @@ func GetIntTests() IntTests { "UnsignedStreamingPayloadTrailer_multiple_checksum_headers": UnsignedStreamingPayloadTrailer_multiple_checksum_headers, "UnsignedStreamingPayloadTrailer_sdk_algo_and_trailer_mismatch": UnsignedStreamingPayloadTrailer_sdk_algo_and_trailer_mismatch, "UnsignedStreamingPayloadTrailer_incomplete_body": UnsignedStreamingPayloadTrailer_incomplete_body, + "UnsignedStreamingPayloadTrailer_invalid_chunk_size": UnsignedStreamingPayloadTrailer_invalid_chunk_size, "UnsignedStreamingPayloadTrailer_no_trailer_should_calculate_crc64nvme": UnsignedStreamingPayloadTrailer_no_trailer_should_calculate_crc64nvme, "UnsignedStreamingPayloadTrailer_no_payload_trailer_only_headers": UnsignedStreamingPayloadTrailer_no_payload_trailer_only_headers, "UnsignedStreamingPayloadTrailer_success_both_sdk_algo_and_trailer": UnsignedStreamingPayloadTrailer_success_both_sdk_algo_and_trailer, diff --git a/tests/integration/unsigned_streaming_payload_trailer.go b/tests/integration/unsigned_streaming_payload_trailer.go index 39741a2..26a14e5 100644 --- a/tests/integration/unsigned_streaming_payload_trailer.go +++ b/tests/integration/unsigned_streaming_payload_trailer.go @@ -211,6 +211,63 @@ func UnsignedStreamingPayloadTrailer_incomplete_body(s *S3Conf) error { }) } +func UnsignedStreamingPayloadTrailer_invalid_chunk_size(s *S3Conf) error { + testName := "UnsignedStreamingPayloadTrailer_invalid_chunk_size" + return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { + object := "my-object" + chSizeErr := s3err.GetAPIError(s3err.ErrInvalidChunkSize) + for i, test := range []struct { + chunkSizes []int64 + expectedErr error + }{ + {[]int64{8192, 8192, 100, 0}, nil}, + {[]int64{100, 0}, nil}, + {[]int64{10000, 9500, 20, 0}, nil}, + {[]int64{8200, 8201, 10000, 10, 0}, nil}, + {[]int64{100, 8192, 100, 0}, chSizeErr}, + {[]int64{8192, 10, 100, 0}, chSizeErr}, + {[]int64{10, 10, 10, 0}, chSizeErr}, + {[]int64{8192, 10, 100, 0}, chSizeErr}, + {[]int64{10000, 10, 8192, 8192, 0}, chSizeErr}, + {[]int64{8192, 10, 10, 8192, 20, 20000, 0}, chSizeErr}, + } { + cLength, payload, err := constructUnsignedPaylod(test.chunkSizes...) + if err != nil { + return fmt.Errorf("test %v failed: %w", i+1, err) + } + + reqHeaders := map[string]string{ + "x-amz-decoded-content-length": fmt.Sprint(cLength), + } + + _, apiErr, err := testUnsignedStreamingPayloadTrailerObjectPut(s, bucket, object, payload, reqHeaders) + if err != nil { + return fmt.Errorf("test %v failed: %w", i+1, err) + } + + if test.expectedErr == nil && apiErr != nil { + return fmt.Errorf("test %v failed: (%s) %s", i+1, apiErr.Code, apiErr.Message) + } + + if test.expectedErr != nil { + if apiErr == nil { + return fmt.Errorf("test %v failed: expected %w, instead got nil", i+1, test.expectedErr) + } + + expErr, ok := test.expectedErr.(s3err.APIError) + if !ok { + return fmt.Errorf("test %v failed: invalid expected error type", i+1) + } + + if err := compareS3ApiError(expErr, apiErr); err != nil { + return fmt.Errorf("test %v failed: %w", i+1, err) + } + } + } + return nil + }) +} + func UnsignedStreamingPayloadTrailer_no_trailer_should_calculate_crc64nvme(s *S3Conf) error { testName := "UnsignedStreamingPayloadTrailer_no_trailer_should_calculate_crc64nvme" return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { diff --git a/tests/integration/utils.go b/tests/integration/utils.go index 7c2ee44..c6a710e 100644 --- a/tests/integration/utils.go +++ b/tests/integration/utils.go @@ -2093,3 +2093,28 @@ func testUnsignedStreamingPayloadTrailerUploadPart(s *S3Conf, bucket, object str return sendSignedRequest(s, req, cancel) } + +// constructUnsignedPaylod constructs an unsigned streaming upload payload +// and returns the decoded content length and the payload +func constructUnsignedPaylod(chunkSizes ...int64) (int64, []byte, error) { + var cLength int64 + buffer := bytes.NewBuffer([]byte{}) + + for _, chunkSize := range chunkSizes { + cLength += chunkSize + _, err := buffer.WriteString(fmt.Sprintf("%x\r\n", chunkSize)) + if err != nil { + return 0, nil, err + } + _, err = buffer.WriteString(strings.Repeat("a", int(chunkSize))) + if err != nil { + return 0, nil, err + } + _, err = buffer.WriteString("\r\n") + if err != nil { + return 0, nil, err + } + } + + return cLength, buffer.Bytes(), nil +}