From dd33e5ee44354c27a86aa228ac94651c563c42c1 Mon Sep 17 00:00:00 2001 From: KUAN-HAO HUANG <101171023+rich7420@users.noreply.github.com> Date: Wed, 9 Sep 2026 12:43:51 +0800 Subject: [PATCH 1/4] HDDS-15142. Support HMAC-SHA256 trailer signature verification for S3 chunked uploads --- .../ozone/s3/SignedChunksInputStream.java | 118 +++++++++++++++--- .../ozone/s3/endpoint/EndpointBase.java | 15 ++- .../ozone/s3/signature/ChunksValidator.java | 33 ++++- .../apache/hadoop/ozone/s3/util/S3Consts.java | 1 + .../ozone/s3/TestSignedChunksInputStream.java | 95 ++++++++++++++ .../ozone/s3/endpoint/TestObjectPut.java | 66 +++++++--- .../ozone/s3/endpoint/TestPartUpload.java | 39 ++++++ .../s3/endpoint/TestUploadWithStream.java | 34 +++++ .../s3/signature/SignatureTestUtils.java | 34 +++++ .../s3/signature/TestChunksValidator.java | 42 +++++++ 10 files changed, 436 insertions(+), 41 deletions(-) diff --git a/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/SignedChunksInputStream.java b/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/SignedChunksInputStream.java index 098111222d5e..863bf4dd3bfb 100644 --- a/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/SignedChunksInputStream.java +++ b/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/SignedChunksInputStream.java @@ -21,6 +21,7 @@ import java.io.IOException; import java.io.InputStream; +import java.nio.charset.StandardCharsets; import java.security.MessageDigest; import java.security.NoSuchAlgorithmException; import java.util.Locale; @@ -85,6 +86,11 @@ public class SignedChunksInputStream extends InputStream { */ private static final Pattern SIGNATURE_LINE_PATTERN = Pattern.compile("([0-9A-Fa-f]+);chunk-signature=([0-9A-Fa-f]{64})"); + private static final Pattern TRAILER_SIGNATURE_PATTERN = + Pattern.compile("x-amz-trailer-signature:([0-9A-Fa-f]{64})", Pattern.CASE_INSENSITIVE); + private static final Pattern CHECKSUM_TRAILER_PATTERN = + Pattern.compile("x-amz-checksum-(crc32|crc32c|crc64nvme|sha1|sha256)"); + private static final int MAX_LINE_LENGTH = 8 * 1024; private final InputStream originalStream; @@ -100,6 +106,9 @@ public class SignedChunksInputStream extends InputStream { /** Signature parsed from the current chunk header line. */ private String chunkSignature; + /** Checksum header declared by x-amz-trailer, or null for a regular signed stream. */ + private final String trailerHeader; + /** * Size of the chunk payload. If zero, the signature line should be parsed to * retrieve the subsequent chunk payload size. @@ -113,8 +122,23 @@ public class SignedChunksInputStream extends InputStream { private boolean isFinalChunkEncountered = false; public SignedChunksInputStream(InputStream inputStream, String keyPath) { + this(inputStream, keyPath, null); + } + + /** + * Creates a signed chunk stream. + * + * @param inputStream the encoded request body + * @param keyPath resource used in S3 errors + * @param trailerHeader the value of x-amz-trailer, or null when no trailer is expected + */ + public SignedChunksInputStream(InputStream inputStream, String keyPath, String trailerHeader) { originalStream = inputStream; this.keyPath = keyPath; + this.trailerHeader = trailerHeader == null ? null : trailerHeader.trim().toLowerCase(Locale.ROOT); + if (this.trailerHeader != null && !CHECKSUM_TRAILER_PATTERN.matcher(this.trailerHeader).matches()) { + throw invalidBody("Invalid x-amz-trailer header"); + } } /** @@ -200,11 +224,14 @@ private boolean ensureChunkPayload() throws IOException { return true; } if (remainingData == 0) { - // final zero-byte chunk: verify it (empty payload) and stop reading - if (validator != null) { + // The final zero-byte chunk has no payload terminator when trailing headers follow it. + if (validator != null && trailerHeader == null) { readChunkTerminator(); } validateChunk(); + if (trailerHeader != null) { + validateTrailer(); + } isFinalChunkEncountered = true; } else { stopAtUnexpectedEof(); @@ -268,21 +295,9 @@ private void readChunkTerminator() throws IOException { } private int readContentLengthFromHeader() throws IOException { - int prev = -1; - int curr = 0; - StringBuilder buf = new StringBuilder(); - - //read everything until the next \r\n - while (!eol(prev, curr) && curr != -1) { - int next = originalStream.read(); - if (next != -1) { - buf.append((char) next); - } - prev = curr; - curr = next; - } - if (!eol(prev, curr)) { - checkNotTruncated(); + String signatureLine = readLine(false); + if (signatureLine == null) { + return -1; } // Example of a single chunk data: // 10000;chunk-signature=b474d8862b1487a5145d686f57f013e54db672cee1c953b3010fb58501ef5aa2\r\n @@ -290,7 +305,7 @@ private int readContentLengthFromHeader() throws IOException { // // 10000 will be read and decoded from base-16 representation to 65536, which is the size of // the subsequent chunk payload. - String signatureLine = buf.toString().trim(); + signatureLine = signatureLine.trim(); if (signatureLine.isEmpty()) { return -1; } @@ -304,6 +319,73 @@ private int readContentLengthFromHeader() throws IOException { return Integer.parseInt(matcher.group(1), 16); } + private void validateTrailer() throws IOException { + String trailerLine = readLine(true); + if (trailerLine == null || trailerLine.isEmpty()) { + throw invalidBody("Missing trailing checksum header"); + } + int separator = trailerLine.indexOf(':'); + if (separator <= 0) { + throw invalidBody("Invalid trailing header: " + trailerLine); + } + String name = trailerLine.substring(0, separator).trim().toLowerCase(Locale.ROOT); + String value = trailerLine.substring(separator + 1).trim(); + if (!trailerHeader.equals(name)) { + throw invalidBody("Unexpected trailing header: " + name); + } + + String signatureLine = readLine(true); + if (signatureLine == null) { + throw invalidBody("Missing trailing signature"); + } + Matcher matcher = TRAILER_SIGNATURE_PATTERN.matcher(signatureLine.trim()); + if (!matcher.matches()) { + throw invalidBody("Invalid trailing signature"); + } + if (validator != null) { + validator.validateTrailer(matcher.group(1), sha256Hex(name + ":" + value + "\n")); + } + // AWS SDKs terminate the trailer section with a blank line after the signature. + if (!"".equals(readLine(true))) { + throw invalidBody("Invalid trailer terminator"); + } + if (originalStream.read() != -1) { + throw invalidBody("Unexpected data after trailing signature"); + } + } + + private String readLine(boolean trailingHeader) throws IOException { + StringBuilder line = new StringBuilder(); + int previous = -1; + while (true) { + int current = originalStream.read(); + if (current == -1) { + if (trailingHeader && line.length() > 0) { + throw invalidBody("Truncated trailing header"); + } + if (!trailingHeader) { + checkNotTruncated(); + } + return line.length() == 0 ? null : line.toString(); + } + line.append((char) current); + if (line.length() > MAX_LINE_LENGTH) { + throw invalidBody("Chunk or trailing header line is too long"); + } + if (eol(previous, current)) { + line.setLength(line.length() - 2); + return line.toString(); + } + previous = current; + } + } + + private static String sha256Hex(String value) { + MessageDigest digest = newSha256(); + byte[] hash = digest.digest(value.getBytes(StandardCharsets.UTF_8)); + return DatatypeConverter.printHexBinary(hash).toLowerCase(Locale.ROOT); + } + private void updateDigest(byte b) { if (chunkDigest != null) { chunkDigest.update(b); diff --git a/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/EndpointBase.java b/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/EndpointBase.java index 8315b337bb50..4c66f2acf9c4 100644 --- a/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/EndpointBase.java +++ b/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/EndpointBase.java @@ -43,11 +43,13 @@ import static org.apache.hadoop.ozone.s3.util.S3Consts.STORAGE_CLASS_HEADER; import static org.apache.hadoop.ozone.s3.util.S3Consts.STORAGE_CONFIG_HEADER; import static org.apache.hadoop.ozone.s3.util.S3Consts.STREAMING_AWS4_HMAC_SHA256_PAYLOAD; +import static org.apache.hadoop.ozone.s3.util.S3Consts.STREAMING_AWS4_HMAC_SHA256_PAYLOAD_TRAILER; import static org.apache.hadoop.ozone.s3.util.S3Consts.TAG_HEADER; import static org.apache.hadoop.ozone.s3.util.S3Consts.TAG_KEY_LENGTH_LIMIT; import static org.apache.hadoop.ozone.s3.util.S3Consts.TAG_NUM_LIMIT; import static org.apache.hadoop.ozone.s3.util.S3Consts.TAG_REGEX_PATTERN; import static org.apache.hadoop.ozone.s3.util.S3Consts.TAG_VALUE_LENGTH_LIMIT; +import static org.apache.hadoop.ozone.s3.util.S3Consts.X_AMZ_TRAILER; import static org.apache.hadoop.ozone.s3.util.S3Utils.hasMultiChunksPayload; import static org.apache.hadoop.ozone.s3.util.S3Utils.hasUnsignedPayload; import static org.apache.hadoop.ozone.s3.util.S3Utils.urlDecode; @@ -759,7 +761,15 @@ protected S3ChunkInputStreamInfo getS3ChunkInputStreamInfo( if (hasUnsignedPayload(amzContentSha256Header)) { chunkInputStream = new UnsignedChunksInputStream(body); } else { - chunkInputStream = new SignedChunksInputStream(body, keyPath); + String trailerHeader = STREAMING_AWS4_HMAC_SHA256_PAYLOAD_TRAILER.equals(amzContentSha256Header) + ? getHeaders().getHeaderString(X_AMZ_TRAILER) : null; + if (STREAMING_AWS4_HMAC_SHA256_PAYLOAD_TRAILER.equals(amzContentSha256Header) + && StringUtils.isBlank(trailerHeader)) { + OS3Exception ex = newError(INVALID_ARGUMENT, keyPath); + ex.setErrorMessage("The " + X_AMZ_TRAILER + " header is required for signed trailing headers"); + throw ex; + } + chunkInputStream = new SignedChunksInputStream(body, keyPath, trailerHeader); } effectiveLength = Long.parseLong(amzDecodedLength); } else { @@ -783,7 +793,8 @@ protected S3ChunkInputStreamInfo getS3ChunkInputStreamInfo( // Header auth only: for a presigned (query) request the payload hash is not part of the signed // canonical request, so its seed signature cannot start the chunk signature chain. boolean verifyChunkSignature = signatureInfo.isSignPayload() - && STREAMING_AWS4_HMAC_SHA256_PAYLOAD.equals(amzContentSha256Header); + && (STREAMING_AWS4_HMAC_SHA256_PAYLOAD.equals(amzContentSha256Header) + || STREAMING_AWS4_HMAC_SHA256_PAYLOAD_TRAILER.equals(amzContentSha256Header)); return new S3ChunkInputStreamInfo(multiDigestInputStream, effectiveLength, verifyChunkSignature); } diff --git a/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/signature/ChunksValidator.java b/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/signature/ChunksValidator.java index 70e731fd42ba..b75b225b1997 100644 --- a/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/signature/ChunksValidator.java +++ b/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/signature/ChunksValidator.java @@ -31,8 +31,8 @@ import org.apache.hadoop.ozone.s3.exception.OS3Exception; /** - * Verifies the per-chunk signatures of a SigV4 chunked upload - * ({@code STREAMING-AWS4-HMAC-SHA256-PAYLOAD}). + * Verifies the per-chunk and trailing-header signatures of a SigV4 chunked upload + * ({@code STREAMING-AWS4-HMAC-SHA256-PAYLOAD} and its trailer variant). *

* Each chunk signature is {@code hex(HMAC-SHA256(signingKey, stringToSign))}, * where the string-to-sign is: @@ -57,6 +57,8 @@ public class ChunksValidator { private static final String CHUNK_STRING_TO_SIGN_ALGORITHM = "AWS4-HMAC-SHA256-PAYLOAD"; + private static final String TRAILER_STRING_TO_SIGN_ALGORITHM = + "AWS4-HMAC-SHA256-TRAILER"; private static final String HMAC_SHA256 = "HmacSHA256"; private static final String NEWLINE = "\n"; @@ -101,15 +103,34 @@ public void validateChunk(String chunkSignature, String payloadSha256Hex) String stringToSign = String.join(NEWLINE, CHUNK_STRING_TO_SIGN_ALGORITHM, dateTime, credentialScope, previousSignature, EMPTY_STRING_SHA256, payloadSha256Hex); + validateSignature(chunkSignature, stringToSign); + // The chain feeds this chunk's signature, in hex, into the next chunk's string-to-sign. + // chunkSignature equals expected (verified above), so reuse it normalized to lower-case. + previousSignature = chunkSignature.toLowerCase(Locale.ROOT); + } + + /** + * Verify the signature of the trailing headers and complete the signature chain. + * + * @param trailerSignature the signature from the {@code x-amz-trailer-signature} header + * @param trailingHeadersSha256Hex SHA-256 of the canonical trailing headers + * @throws OS3Exception if the computed signature does not match + */ + public void validateTrailer(String trailerSignature, String trailingHeadersSha256Hex) + throws OS3Exception { + String stringToSign = String.join(NEWLINE, + TRAILER_STRING_TO_SIGN_ALGORITHM, dateTime, credentialScope, + previousSignature, trailingHeadersSha256Hex); + validateSignature(trailerSignature, stringToSign); + } + + private void validateSignature(String signature, String stringToSign) { byte[] expected = hmacSha256(stringToSign); // Constant-time comparison to avoid leaking the signature via timing. Decoding the hex also // makes the comparison case-insensitive, as the signature may be sent in either case. - if (!MessageDigest.isEqual(expected, DatatypeConverter.parseHexBinary(chunkSignature))) { + if (!MessageDigest.isEqual(expected, DatatypeConverter.parseHexBinary(signature))) { throw newError(SIGNATURE_DOES_NOT_MATCH, resource); } - // The chain feeds this chunk's signature, in hex, into the next chunk's string-to-sign. - // chunkSignature equals expected (verified above), so reuse it normalized to lower-case. - previousSignature = chunkSignature.toLowerCase(Locale.ROOT); } private byte[] hmacSha256(String msg) { diff --git a/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/util/S3Consts.java b/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/util/S3Consts.java index f75653ad098b..620d554aa5cb 100644 --- a/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/util/S3Consts.java +++ b/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/util/S3Consts.java @@ -35,6 +35,7 @@ public final class S3Consts { // Constants related to AWS Signature Version V4 calculation // https://docs.aws.amazon.com/AmazonS3/latest/API/sig-v4-authenticating-requests.html public static final String X_AMZ_CONTENT_SHA256 = "x-amz-content-sha256"; + public static final String X_AMZ_TRAILER = "x-amz-trailer"; public static final String UNSIGNED_PAYLOAD = "UNSIGNED-PAYLOAD"; public static final String STREAMING_UNSIGNED_PAYLOAD_TRAILER = "STREAMING-UNSIGNED-PAYLOAD-TRAILER"; diff --git a/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/TestSignedChunksInputStream.java b/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/TestSignedChunksInputStream.java index f48a1b2a7b05..b1bca25bbadb 100644 --- a/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/TestSignedChunksInputStream.java +++ b/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/TestSignedChunksInputStream.java @@ -19,6 +19,7 @@ import static java.nio.charset.StandardCharsets.UTF_8; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertThrows; @@ -55,6 +56,16 @@ public class TestSignedChunksInputStream { private static final String KEY_PATH = "key1"; private static final String FINAL_CHUNK_SIGNATURE = "b6c6ea8a5354eaf15b3cb7646744f4275b71ea724fed81ceb9323e279d449df9"; + private static final String TRAILER_SEED_SIGNATURE = + "106e2a8a18243abcf37539882f36619c00e2dfc72633413f02d3b74544bfeb8e"; + private static final String TRAILER_CHUNK1_SIGNATURE = + "b474d8862b1487a5145d686f57f013e54db672cee1c953b3010fb58501ef5aa2"; + private static final String TRAILER_CHUNK2_SIGNATURE = + "1c1344b170168f8e65b41376b44b20fe354e373826ccbbe2c1d40a8cae51e5c7"; + private static final String TRAILER_FINAL_CHUNK_SIGNATURE = + "2ca2aba2005185cf7159c6277faf83795951dd77a3a99e6e65d5c9f85863f992"; + private static final String TRAILER_SIGNATURE = + "d81f82fc3505edab99d459891051a732e8730629a2e4a59689829ca17fe2e435"; /** Well-formed but meaningless signature, for the tests that only strip the chunk format. */ private static final String FAKE_SIGNATURE = repeat('0', 64); @@ -257,6 +268,74 @@ void testMultiChunksWithTrailer() throws Exception { } } + @Test + void verifiesTrailerSignature() throws Exception { + String body = trailerBody(); + try (SignedChunksInputStream is = new SignedChunksInputStream( + new ByteArrayInputStream(body.getBytes(UTF_8)), KEY_PATH, "x-amz-checksum-crc32c")) { + is.attachValidator(newTrailerValidator()); + assertThat(IOUtils.toString(is, UTF_8)).isEqualTo(repeat('a', 66560)); + } + } + + @Test + void rejectsTamperedTrailerSignature() { + String body = trailerBody().replace(TRAILER_SIGNATURE, + TRAILER_SIGNATURE.substring(0, TRAILER_SIGNATURE.length() - 1) + "0"); + SignedChunksInputStream is = new SignedChunksInputStream( + new ByteArrayInputStream(body.getBytes(UTF_8)), KEY_PATH, "x-amz-checksum-crc32c"); + is.attachValidator(newTrailerValidator()); + assertSignatureMismatch(is); + } + + @Test + void rejectsMissingTrailerSignature() { + String body = trailerBody().replace("x-amz-trailer-signature:" + TRAILER_SIGNATURE + "\r\n\r\n", ""); + SignedChunksInputStream is = new SignedChunksInputStream( + new ByteArrayInputStream(body.getBytes(UTF_8)), KEY_PATH, "x-amz-checksum-crc32c"); + is.attachValidator(newTrailerValidator()); + assertInvalidBody(is, "Missing trailing signature"); + } + + @Test + void rejectsUnterminatedTrailerSignature() { + String body = trailerBody(); + body = body.substring(0, body.length() - 4); + SignedChunksInputStream is = new SignedChunksInputStream( + new ByteArrayInputStream(body.getBytes(UTF_8)), KEY_PATH, "x-amz-checksum-crc32c"); + is.attachValidator(newTrailerValidator()); + assertInvalidBody(is, "Truncated trailing header"); + } + + @Test + void rejectsDataAfterTrailerSignature() { + SignedChunksInputStream is = new SignedChunksInputStream( + new ByteArrayInputStream((trailerBody() + "extra").getBytes(UTF_8)), KEY_PATH, + "x-amz-checksum-crc32c"); + is.attachValidator(newTrailerValidator()); + assertInvalidBody(is, "Unexpected data after trailing signature"); + } + + @Test + void rejectsMissingTrailerTerminator() { + String body = trailerBody(); + SignedChunksInputStream is = new SignedChunksInputStream( + new ByteArrayInputStream(body.substring(0, body.length() - 2).getBytes(UTF_8)), KEY_PATH, + "x-amz-checksum-crc32c"); + is.attachValidator(newTrailerValidator()); + assertInvalidBody(is, "Invalid trailer terminator"); + } + + @ParameterizedTest + @ValueSource(strings = {"", " ", ",", "x-amz-meta-test", "x-amz-trailer-signature", "x-amz-checksum-crc32c,", + "x-amz-checksum-crc32c,x-amz-checksum-crc32c", "x-amz-checksum-crc32,x-amz-checksum-sha256"}) + void rejectsInvalidTrailerHeader(String header) { + assertThatThrownBy(() -> new SignedChunksInputStream( + new ByteArrayInputStream(new byte[0]), KEY_PATH, header)) + .isInstanceOfSatisfying(OS3Exception.class, + ex -> assertThat(ex.getCode()).isEqualTo(S3ErrorTable.INVALID_REQUEST.getCode())); + } + @Test void attachValidatorEnablesVerification() throws IOException { // The signing key is only known after the key is opened, so the validator @@ -353,6 +432,22 @@ private static ChunksValidator newValidator() { DATE_TIME, SCOPE, SEED_SIGNATURE, KEY_PATH); } + private static ChunksValidator newTrailerValidator() { + return new ChunksValidator( + SignatureTestUtils.signingKey(SECRET_KEY, "20130524", "us-east-1", "s3"), + DATE_TIME, SCOPE, TRAILER_SEED_SIGNATURE, KEY_PATH); + } + + private static String trailerBody() { + return "10000;chunk-signature=" + TRAILER_CHUNK1_SIGNATURE + "\r\n" + + repeat('a', 65536) + "\r\n" + + "400;chunk-signature=" + TRAILER_CHUNK2_SIGNATURE + "\r\n" + + repeat('a', 1024) + "\r\n" + + "0;chunk-signature=" + TRAILER_FINAL_CHUNK_SIGNATURE + "\r\n" + + "x-amz-checksum-crc32c:sOO8/Q==\r\n" + + "x-amz-trailer-signature:" + TRAILER_SIGNATURE + "\r\n\r\n"; + } + private static SignedChunksInputStream verifiedStream(String body) { SignedChunksInputStream stream = new SignedChunksInputStream(new ByteArrayInputStream(body.getBytes(UTF_8)), KEY_PATH); diff --git a/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestObjectPut.java b/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestObjectPut.java index 9a51f4a3e066..d14b29847dd4 100644 --- a/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestObjectPut.java +++ b/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestObjectPut.java @@ -30,6 +30,7 @@ import static org.apache.hadoop.ozone.s3.exception.S3ErrorTable.SIGNATURE_DOES_NOT_MATCH; import static org.apache.hadoop.ozone.s3.signature.SignatureTestUtils.signatureInfo; import static org.apache.hadoop.ozone.s3.signature.SignatureTestUtils.signedChunkedBody; +import static org.apache.hadoop.ozone.s3.signature.SignatureTestUtils.signedChunkedBodyWithTrailer; import static org.apache.hadoop.ozone.s3.signature.SignatureTestUtils.signingKey; import static org.apache.hadoop.ozone.s3.util.S3Consts.COPY_SOURCE_HEADER; import static org.apache.hadoop.ozone.s3.util.S3Consts.CUSTOM_METADATA_COPY_DIRECTIVE_HEADER; @@ -47,6 +48,7 @@ import static org.apache.hadoop.ozone.s3.util.S3Consts.TAG_NUM_LIMIT; import static org.apache.hadoop.ozone.s3.util.S3Consts.TAG_VALUE_LENGTH_LIMIT; import static org.apache.hadoop.ozone.s3.util.S3Consts.X_AMZ_CONTENT_SHA256; +import static org.apache.hadoop.ozone.s3.util.S3Consts.X_AMZ_TRAILER; import static org.apache.hadoop.ozone.s3.util.S3Utils.parseETag; import static org.apache.hadoop.ozone.s3.util.S3Utils.urlEncode; import static org.assertj.core.api.Assertions.assertThat; @@ -116,8 +118,6 @@ class TestObjectPut { private static final String DEST_BUCKET_NAME = "b2"; private static final String DEST_KEY = "key=value/2"; private static final String NONEXISTENT_BUCKET = "nonexist"; - /** Well-formed but meaningless signature, for the path that only strips the chunk framing. */ - private static final String FAKE_SIGNATURE = StringUtils.repeat('0', 64); private ObjectEndpoint objectEndpoint; private HttpHeaders headers; @@ -269,21 +269,47 @@ void testPutObjectWithValidSignedChunks() throws Exception { } @Test - void testPutObjectWithUnverifiedSignedChunks() throws Exception { - // Only STREAMING-AWS4-HMAC-SHA256-PAYLOAD opts into verification; the -TRAILER variant is - // HDDS-15142. Here the stream just strips the chunk framing, so the signatures are not checked - // and a body without the terminating zero-byte chunk is still accepted, as before this change. - String chunkedContent = "0a;chunk-signature=" + FAKE_SIGNATURE + "\r\n" - + "1234567890\r\n" - + "05;chunk-signature=" + FAKE_SIGNATURE + "\r\n" - + "abcde\r\n"; + void testPutObjectWithValidSignedChunksAndTrailer() throws Exception { + configureSignedChunksWithTrailer(CONTENT.length()); + + // The trailer value is covered by the HMAC; checksum calculation is outside this test. + assertSucceeds(() -> putObject(signedChunkedBodyWithTrailer( + CONTENT, "x-amz-checksum-crc32c", "sOO8/Q=="))); + + assertKeyContent(bucket, KEY_NAME, CONTENT); + } + + @Test + void testPutObjectRejectsTamperedTrailer() { + configureSignedChunksWithTrailer(CONTENT.length()); + String body = signedChunkedBodyWithTrailer(CONTENT, "x-amz-checksum-crc32c", "sOO8/Q==") + .replace("sOO8/Q==", "tampered"); + + assertErrorResponse(SIGNATURE_DOES_NOT_MATCH, () -> putObject(body)); + assertKeyWasNotCommitted(); + } + + @Test + void testPutObjectRejectsMissingTrailerFinalChunk() { + configureSignedChunksWithTrailer(CONTENT.length()); + String body = withoutFinalChunk(signedChunkedBody(CONTENT)); + + OS3Exception ex = assertErrorResponse(S3ErrorTable.INVALID_REQUEST, + () -> putObject(body)); + assertThat(ex.getErrorMessage()).contains("terminating 0-byte chunk"); + assertKeyWasNotCommitted(); + } + + @Test + void testPutObjectRejectsMissingTrailerHeader() { + ((OzoneBucketStub) bucket).setDerivedKey(signingKey()); when(headers.getHeaderString(X_AMZ_CONTENT_SHA256)) .thenReturn(STREAMING_AWS4_HMAC_SHA256_PAYLOAD_TRAILER); - when(headers.getHeaderString(DECODED_CONTENT_LENGTH_HEADER)).thenReturn("15"); - - assertSucceeds(() -> putObject(chunkedContent)); + when(headers.getHeaderString(DECODED_CONTENT_LENGTH_HEADER)).thenReturn("0"); - assertKeyContent(bucket, KEY_NAME, "1234567890abcde"); + assertErrorResponse(S3ErrorTable.INVALID_ARGUMENT, + () -> putObject(signedChunkedBodyWithTrailer("", "x-amz-checksum-crc32c", "sOO8/Q=="))); + assertKeyWasNotCommitted(); } @Test @@ -380,7 +406,7 @@ void testPutObjectRejectsDecodedLengthEndingMidChunk() { static Stream chunkSignatureVerificationCases() { return Stream.of( Arguments.of(STREAMING_AWS4_HMAC_SHA256_PAYLOAD, true), - Arguments.of(STREAMING_AWS4_HMAC_SHA256_PAYLOAD_TRAILER, false), + Arguments.of(STREAMING_AWS4_HMAC_SHA256_PAYLOAD_TRAILER, true), Arguments.of(STREAMING_AWS4_ECDSA_P256_SHA256_PAYLOAD, false), Arguments.of(STREAMING_AWS4_ECDSA_P256_SHA256_PAYLOAD_TRAILER, false), Arguments.of(STREAMING_UNSIGNED_PAYLOAD_TRAILER, false)); @@ -390,6 +416,9 @@ static Stream chunkSignatureVerificationCases() { @MethodSource("chunkSignatureVerificationCases") void testChunkSignatureVerificationSelection(String algorithm, boolean expected) throws Exception { when(headers.getHeaderString(X_AMZ_CONTENT_SHA256)).thenReturn(algorithm); + if (STREAMING_AWS4_HMAC_SHA256_PAYLOAD_TRAILER.equals(algorithm)) { + when(headers.getHeaderString(X_AMZ_TRAILER)).thenReturn("x-amz-checksum-crc32c"); + } EndpointBase.S3ChunkInputStreamInfo info = objectEndpoint.getS3ChunkInputStreamInfo( new ByteArrayInputStream(new byte[0]), 0, "0", KEY_NAME); @@ -981,6 +1010,13 @@ private void configureSignedChunks(long decodedLength) { configureSignedChunkHeaders(decodedLength); } + private void configureSignedChunksWithTrailer(long decodedLength) { + configureSignedChunks(decodedLength); + when(headers.getHeaderString(X_AMZ_CONTENT_SHA256)) + .thenReturn(STREAMING_AWS4_HMAC_SHA256_PAYLOAD_TRAILER); + when(headers.getHeaderString(X_AMZ_TRAILER)).thenReturn("x-amz-checksum-crc32c"); + } + private void configureSignedChunkHeaders(long decodedLength) { when(headers.getHeaderString(X_AMZ_CONTENT_SHA256)).thenReturn(STREAMING_AWS4_HMAC_SHA256_PAYLOAD); when(headers.getHeaderString(DECODED_CONTENT_LENGTH_HEADER)).thenReturn(String.valueOf(decodedLength)); diff --git a/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestPartUpload.java b/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestPartUpload.java index a68a02f0c087..f8109676e148 100644 --- a/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestPartUpload.java +++ b/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestPartUpload.java @@ -24,11 +24,14 @@ import static org.apache.hadoop.ozone.s3.endpoint.EndpointTestUtils.put; import static org.apache.hadoop.ozone.s3.signature.SignatureTestUtils.signatureInfo; import static org.apache.hadoop.ozone.s3.signature.SignatureTestUtils.signedChunkedBody; +import static org.apache.hadoop.ozone.s3.signature.SignatureTestUtils.signedChunkedBodyWithTrailer; import static org.apache.hadoop.ozone.s3.signature.SignatureTestUtils.signingKey; import static org.apache.hadoop.ozone.s3.util.S3Consts.DECODED_CONTENT_LENGTH_HEADER; import static org.apache.hadoop.ozone.s3.util.S3Consts.STORAGE_CLASS_HEADER; import static org.apache.hadoop.ozone.s3.util.S3Consts.STREAMING_AWS4_HMAC_SHA256_PAYLOAD; +import static org.apache.hadoop.ozone.s3.util.S3Consts.STREAMING_AWS4_HMAC_SHA256_PAYLOAD_TRAILER; import static org.apache.hadoop.ozone.s3.util.S3Consts.X_AMZ_CONTENT_SHA256; +import static org.apache.hadoop.ozone.s3.util.S3Consts.X_AMZ_TRAILER; import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotEquals; @@ -198,6 +201,35 @@ public void testPartUploadRejectsTamperedSignedChunk() throws Exception { assertNoParts(uploadID, keyName); } + @Test + public void testPartUploadWithValidSignedTrailer() throws Exception { + String keyName = UUID.randomUUID().toString(); + String content = "1234567890abcde"; + String chunkedContent = signedChunkedBodyWithTrailer( + content, "x-amz-checksum-crc32c", "sOO8/Q=="); + configureSignedChunksWithTrailer(content.length()); + + String uploadID = initiateMultipartUpload(rest, OzoneConsts.S3_BUCKET, keyName); + + assertSucceeds(() -> put(rest, OzoneConsts.S3_BUCKET, keyName, 1, uploadID, chunkedContent)); + assertContentLength(uploadID, keyName, content.length()); + } + + @Test + public void testPartUploadRejectsTamperedTrailer() throws Exception { + String keyName = UUID.randomUUID().toString(); + String content = "1234567890abcde"; + String chunkedContent = signedChunkedBodyWithTrailer( + content, "x-amz-checksum-crc32c", "sOO8/Q==").replace("sOO8/Q==", "tampered"); + configureSignedChunksWithTrailer(content.length()); + + String uploadID = initiateMultipartUpload(rest, OzoneConsts.S3_BUCKET, keyName); + + assertErrorResponse(S3ErrorTable.SIGNATURE_DOES_NOT_MATCH, + () -> put(rest, OzoneConsts.S3_BUCKET, keyName, 1, uploadID, chunkedContent)); + assertNoParts(uploadID, keyName); + } + @Test public void testPartUploadRejectsMissingDerivedKeyInSecureMode() throws Exception { String keyName = UUID.randomUUID().toString(); @@ -218,6 +250,13 @@ private void configureSignedChunks(int contentLength) throws IOException { configureSignedChunkHeaders(contentLength); } + private void configureSignedChunksWithTrailer(int contentLength) throws IOException { + configureSignedChunks(contentLength); + when(headers.getHeaderString(X_AMZ_CONTENT_SHA256)) + .thenReturn(STREAMING_AWS4_HMAC_SHA256_PAYLOAD_TRAILER); + when(headers.getHeaderString(X_AMZ_TRAILER)).thenReturn("x-amz-checksum-crc32c"); + } + private void configureSignedChunkHeaders(int contentLength) { when(headers.getHeaderString(X_AMZ_CONTENT_SHA256)) .thenReturn(STREAMING_AWS4_HMAC_SHA256_PAYLOAD); diff --git a/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestUploadWithStream.java b/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestUploadWithStream.java index 27826401b7c7..736b5dc223a8 100644 --- a/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestUploadWithStream.java +++ b/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestUploadWithStream.java @@ -26,12 +26,15 @@ import static org.apache.hadoop.ozone.s3.endpoint.EndpointTestUtils.put; import static org.apache.hadoop.ozone.s3.signature.SignatureTestUtils.signatureInfo; import static org.apache.hadoop.ozone.s3.signature.SignatureTestUtils.signedChunkedBody; +import static org.apache.hadoop.ozone.s3.signature.SignatureTestUtils.signedChunkedBodyWithTrailer; import static org.apache.hadoop.ozone.s3.signature.SignatureTestUtils.signingKey; import static org.apache.hadoop.ozone.s3.util.S3Consts.COPY_SOURCE_HEADER; import static org.apache.hadoop.ozone.s3.util.S3Consts.DECODED_CONTENT_LENGTH_HEADER; import static org.apache.hadoop.ozone.s3.util.S3Consts.STORAGE_CLASS_HEADER; import static org.apache.hadoop.ozone.s3.util.S3Consts.STREAMING_AWS4_HMAC_SHA256_PAYLOAD; +import static org.apache.hadoop.ozone.s3.util.S3Consts.STREAMING_AWS4_HMAC_SHA256_PAYLOAD_TRAILER; import static org.apache.hadoop.ozone.s3.util.S3Consts.X_AMZ_CONTENT_SHA256; +import static org.apache.hadoop.ozone.s3.util.S3Consts.X_AMZ_TRAILER; import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertThrows; @@ -132,6 +135,29 @@ public void testUploadRejectsTamperedSignedChunk() throws Exception { assertThatThrownBy(() -> bucket.getKey(S3KEY)).isInstanceOf(IOException.class); } + @Test + public void testUploadWithValidSignedTrailer() throws Exception { + OzoneBucket bucket = configureSignedChunksWithTrailer(S3_COPY_EXISTING_KEY_CONTENT.length()); + String body = signedChunkedBodyWithTrailer( + S3_COPY_EXISTING_KEY_CONTENT, "x-amz-checksum-crc32c", "sOO8/Q=="); + + assertSucceeds(() -> put(rest, S3BUCKET, S3KEY, body)); + + assertKeyContent(bucket, S3KEY, S3_COPY_EXISTING_KEY_CONTENT); + } + + @Test + public void testUploadRejectsTamperedTrailer() throws Exception { + OzoneBucket bucket = configureSignedChunksWithTrailer(S3_COPY_EXISTING_KEY_CONTENT.length()); + String body = signedChunkedBodyWithTrailer( + S3_COPY_EXISTING_KEY_CONTENT, "x-amz-checksum-crc32c", "sOO8/Q==") + .replace("sOO8/Q==", "tampered"); + + assertErrorResponse(S3ErrorTable.SIGNATURE_DOES_NOT_MATCH, + () -> put(rest, S3BUCKET, S3KEY, body)); + assertThatThrownBy(() -> bucket.getKey(S3KEY)).isInstanceOf(IOException.class); + } + @Test public void testUploadDoesNotCommitWhenBodyReadFails() throws Exception { OzoneBucket bucket = client.getObjectStore().getS3Bucket(S3BUCKET); @@ -197,4 +223,12 @@ private OzoneBucket configureSignedChunks(int decodedLength) throws IOException when(headers.getHeaderString(DECODED_CONTENT_LENGTH_HEADER)).thenReturn(String.valueOf(decodedLength)); return bucket; } + + private OzoneBucket configureSignedChunksWithTrailer(int decodedLength) throws IOException { + OzoneBucket bucket = configureSignedChunks(decodedLength); + when(headers.getHeaderString(X_AMZ_CONTENT_SHA256)) + .thenReturn(STREAMING_AWS4_HMAC_SHA256_PAYLOAD_TRAILER); + when(headers.getHeaderString(X_AMZ_TRAILER)).thenReturn("x-amz-checksum-crc32c"); + return bucket; + } } diff --git a/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/signature/SignatureTestUtils.java b/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/signature/SignatureTestUtils.java index 3a276c3a3d19..edb75cea592d 100644 --- a/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/signature/SignatureTestUtils.java +++ b/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/signature/SignatureTestUtils.java @@ -104,6 +104,16 @@ public static String chunkSignature(byte[] signingKey, String dateTime, String c return DatatypeConverter.printHexBinary(hmac(signingKey, stringToSign)).toLowerCase(Locale.ROOT); } + /** Compute one SigV4 streaming trailer signature. */ + public static String trailerSignature(byte[] signingKey, String dateTime, String credentialScope, + String previousSignature, String trailingHeaders) { + byte[] canonicalHeaders = (trailingHeaders + "\n").getBytes(UTF_8); + String trailingHeadersHash = sha256Hex(canonicalHeaders, 0, canonicalHeaders.length); + String stringToSign = String.join("\n", "AWS4-HMAC-SHA256-TRAILER", dateTime, + credentialScope, previousSignature, trailingHeadersHash); + return DatatypeConverter.printHexBinary(hmac(signingKey, stringToSign)).toLowerCase(Locale.ROOT); + } + /** Build a one-data-chunk SigV4 streaming body, including the terminating zero-byte chunk. */ public static String signedChunkedBody(byte[] signingKey, String dateTime, String credentialScope, String seedSignature, String content) { @@ -125,4 +135,28 @@ public static String signedChunkedBody(byte[] signingKey, String dateTime, Strin public static String signedChunkedBody(String content) { return signedChunkedBody(SIGNING_KEY, DATE_TIME, CREDENTIAL_SCOPE, SEED_SIGNATURE, content); } + + /** Build a one-data-chunk SigV4 body with one trailing header and its signature. */ + public static String signedChunkedBodyWithTrailer(String content, String trailerName, + String trailerValue) { + byte[] payload = content.getBytes(UTF_8); + String previousSignature = SEED_SIGNATURE; + StringBuilder body = new StringBuilder(); + if (payload.length > 0) { + previousSignature = chunkSignature(SIGNING_KEY, DATE_TIME, CREDENTIAL_SCOPE, + SEED_SIGNATURE, payload); + body.append(Integer.toHexString(payload.length)) + .append(";chunk-signature=").append(previousSignature).append("\r\n") + .append(content).append("\r\n"); + } + String finalSignature = chunkSignature(SIGNING_KEY, DATE_TIME, CREDENTIAL_SCOPE, + previousSignature, new byte[0]); + body.append("0;chunk-signature=").append(finalSignature).append("\r\n") + .append(trailerName).append(':').append(trailerValue).append("\r\n") + .append("x-amz-trailer-signature:") + .append(trailerSignature(SIGNING_KEY, DATE_TIME, CREDENTIAL_SCOPE, finalSignature, + trailerName + ":" + trailerValue)) + .append("\r\n\r\n"); + return body.toString(); + } } diff --git a/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/signature/TestChunksValidator.java b/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/signature/TestChunksValidator.java index 2b28c6f45a40..6974691d92fc 100644 --- a/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/signature/TestChunksValidator.java +++ b/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/signature/TestChunksValidator.java @@ -53,6 +53,16 @@ class TestChunksValidator { "0055627c9e194cb4542bae2aa5492e3c1575bbb81b612b7d234b86a503ef5497"; private static final String FINAL_CHUNK_SIGNATURE = "b6c6ea8a5354eaf15b3cb7646744f4275b71ea724fed81ceb9323e279d449df9"; + private static final String TRAILER_SEED_SIGNATURE = + "106e2a8a18243abcf37539882f36619c00e2dfc72633413f02d3b74544bfeb8e"; + private static final String TRAILER_CHUNK1_SIGNATURE = + "b474d8862b1487a5145d686f57f013e54db672cee1c953b3010fb58501ef5aa2"; + private static final String TRAILER_CHUNK2_SIGNATURE = + "1c1344b170168f8e65b41376b44b20fe354e373826ccbbe2c1d40a8cae51e5c7"; + private static final String TRAILER_FINAL_CHUNK_SIGNATURE = + "2ca2aba2005185cf7159c6277faf83795951dd77a3a99e6e65d5c9f85863f992"; + private static final String TRAILER_SIGNATURE = + "d81f82fc3505edab99d459891051a732e8730629a2e4a59689829ca17fe2e435"; /** A chunk that fails verification must surface as SignatureDoesNotMatch (HTTP 403), not any other error. */ private static void assertSignatureMismatch(Executable call) { @@ -67,6 +77,12 @@ private ChunksValidator newValidator() { DATE_TIME, SCOPE, SEED_SIGNATURE, KEY_PATH); } + private ChunksValidator newTrailerValidator() { + return new ChunksValidator( + SignatureTestUtils.signingKey(SECRET_KEY, "20130524", "us-east-1", "s3"), + DATE_TIME, SCOPE, TRAILER_SEED_SIGNATURE, KEY_PATH); + } + @Test void acceptsMatchingChunkSignatures() { ChunksValidator validator = newValidator(); @@ -91,6 +107,32 @@ void acceptsUppercaseChunkSignature() { SignatureTestUtils.sha256Hex(chunk, 0, chunk.length))).doesNotThrowAnyException(); } + @Test + void acceptsMatchingTrailerSignature() { + ChunksValidator validator = newTrailerValidator(); + byte[] chunk1 = repeat('a', 65536); + byte[] chunk2 = repeat('a', 1024); + String trailer = "x-amz-checksum-crc32c:sOO8/Q=="; + + assertDoesNotThrow(() -> validator.validateChunk(TRAILER_CHUNK1_SIGNATURE, + SignatureTestUtils.sha256Hex(chunk1, 0, chunk1.length))); + assertDoesNotThrow(() -> validator.validateChunk(TRAILER_CHUNK2_SIGNATURE, + SignatureTestUtils.sha256Hex(chunk2, 0, chunk2.length))); + assertDoesNotThrow(() -> validator.validateChunk(TRAILER_FINAL_CHUNK_SIGNATURE, + SignatureTestUtils.sha256Hex(new byte[0], 0, 0))); + assertDoesNotThrow(() -> validator.validateTrailer(TRAILER_SIGNATURE, + SignatureTestUtils.sha256Hex((trailer + "\n").getBytes(java.nio.charset.StandardCharsets.UTF_8), + 0, trailer.length() + 1))); + } + + @Test + void rejectsTamperedTrailerSignature() { + ChunksValidator validator = newTrailerValidator(); + assertSignatureMismatch(() -> validator.validateTrailer( + TRAILER_SIGNATURE.substring(0, TRAILER_SIGNATURE.length() - 1) + "0", + "invalid-trailer-hash")); + } + @Test void rejectsTamperedChunkSignature() { ChunksValidator validator = newValidator(); From 2fb7275cad763ed5778c0709d92eaaa34193eaf2 Mon Sep 17 00:00:00 2001 From: KUAN-HAO HUANG <101171023+rich7420@users.noreply.github.com> Date: Wed, 9 Sep 2026 14:10:13 +0800 Subject: [PATCH 2/4] HDDS-15142. Document the final trailer CRLF --- .../java/org/apache/hadoop/ozone/s3/SignedChunksInputStream.java | 1 + 1 file changed, 1 insertion(+) diff --git a/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/SignedChunksInputStream.java b/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/SignedChunksInputStream.java index 863bf4dd3bfb..c494e7aa1e00 100644 --- a/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/SignedChunksInputStream.java +++ b/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/SignedChunksInputStream.java @@ -47,6 +47,7 @@ * 0;chunk-signature=b6c6ea8a5354eaf15b3cb7646744f4275b71ea724fed81ceb9323e279d449df9\r\n * x-amz-checksum-crc32c:sOO8/Q==\r\n * x-amz-trailer-signature:63bddb248ad2590c92712055f51b8e78ab024eead08276b24f010b0efd74843f\r\n + * \r\n * *

* For the first chunk 10000 will be read and decoded from base-16 representation to 65536, which is the size of From d447c403795080decdefb7b3404687f3c1f631a6 Mon Sep 17 00:00:00 2001 From: KUAN-HAO HUANG <101171023+rich7420@users.noreply.github.com> Date: Sat, 12 Sep 2026 21:25:59 +0800 Subject: [PATCH 3/4] HDDS-15142. Derive S3 signing keys before Ratis submission --- .../apache/hadoop/ozone/om/OzoneManager.java | 18 +- .../om/request/key/OMKeyCreateRequest.java | 30 +-- .../key/OMKeyCreateRequestWithFSO.java | 3 +- ...ManagerProtocolServerSideTranslatorPB.java | 24 +- .../request/key/TestOMKeyCreateRequest.java | 9 +- .../ozone/om/response/TestS3DerivedKey.java | 239 ++++++++++++++++++ .../ozone/s3/SignedChunksInputStream.java | 10 +- .../ozone/s3/endpoint/TestObjectPut.java | 24 +- .../s3/signature/TestChunksValidator.java | 26 +- 9 files changed, 308 insertions(+), 75 deletions(-) create mode 100644 hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/response/TestS3DerivedKey.java diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java index 1960e1fc3bdc..856e1c25814d 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java @@ -5535,9 +5535,21 @@ public void startQuotaRepair(List buckets) throws IOException { new QuotaRepairTask(this).repair(buckets); } - public byte[] getS3DerivedKey(String accessId, String signingKey) throws IOException { - String awsSecretKey = s3SecretManager.getSecretString(accessId); - return AWSV4AuthValidator.getSigningKey(awsSecretKey, signingKey); + /** Derives the signing key on the RPC thread after S3 authentication has succeeded. */ + public byte[] getS3DerivedKey(S3Authentication s3Auth) throws IOException { + final String awsSecretKey; + if (StringUtils.isNotEmpty(s3Auth.getSessionToken())) { + STSTokenIdentifier stsToken = getStsTokenIdentifier(); + if (stsToken == null || !s3Auth.getAccessId().equals(stsToken.getTempAccessKeyId()) + || StringUtils.isEmpty(stsToken.getSecretAccessKey())) { + throw new OMException("Missing authenticated STS credentials for signing key derivation", + OMException.ResultCodes.INVALID_TOKEN); + } + awsSecretKey = stsToken.getSecretAccessKey(); + } else { + awsSecretKey = getS3SecretManager().getSecretString(s3Auth.getAccessId()); + } + return AWSV4AuthValidator.getSigningKey(awsSecretKey, s3Auth.getStringToSign()); } @Override diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyCreateRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyCreateRequest.java index 71d49df7bfcd..24bd3116b7fd 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyCreateRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyCreateRequest.java @@ -22,7 +22,6 @@ import static org.apache.hadoop.ozone.om.request.file.OMFileRequest.OMDirectoryResult.FILE_EXISTS_IN_GIVENPATH; import static org.apache.hadoop.ozone.util.MetricUtil.captureLatencyNs; -import com.google.protobuf.ByteString; import java.io.IOException; import java.nio.file.InvalidPathException; import java.nio.file.Paths; @@ -65,12 +64,9 @@ import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.KeyArgs; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMRequest; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMResponse; -import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMTokenProto; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.Type; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.UserInfo; import org.apache.hadoop.ozone.request.validation.RequestProcessingPhase; -import org.apache.hadoop.ozone.security.OzoneTokenIdentifier; -import org.apache.hadoop.ozone.security.S3SecurityUtil; import org.apache.hadoop.ozone.security.acl.IAccessAuthorizer; import org.apache.hadoop.util.Time; import org.slf4j.Logger; @@ -322,8 +318,7 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, Execut checkBucketQuotaInBytes(omMetadataManager, bucketInfo, preAllocatedSpace); checkBucketQuotaInNamespace(bucketInfo, numMissingParents + 1L); - CreateKeyResponse.Builder builder = - getResponseBuilderWithDerivedKey(getOmRequest(), ozoneManager, createKeyRequest); + CreateKeyResponse.Builder builder = CreateKeyResponse.newBuilder(); perfMetrics.addCreateKeyQuotaCheckLatencyNs(Time.monotonicNowNanos() - quotaCheckStartTime); bucketInfo.incrUsedNamespace(numMissingParents); @@ -465,27 +460,4 @@ public static OMRequest blockCreateKeyWithBucketLayoutFromOldClient( } return req; } - - protected CreateKeyResponse.Builder getResponseBuilderWithDerivedKey( - OMRequest omRequest, OzoneManager ozoneManager, - CreateKeyRequest createKeyRequest) throws IOException { - CreateKeyResponse.Builder builder = CreateKeyResponse.newBuilder(); - if (omRequest.hasS3Authentication() && ozoneManager.isSecurityEnabled() - && createKeyRequest.hasDerivedKeyPiggyBacking() - && createKeyRequest.getDerivedKeyPiggyBacking() - ) { - OzoneTokenIdentifier s3Token = S3SecurityUtil.constructS3Token(omRequest); - if (!s3Token.getTokenType().equals(OMTokenProto.Type.S3AUTHINFO)) { - // Piggyback was requested but this token type cannot produce a derived key. - // S3 Gateway should only set this flag for S3AUTHINFO tokens. - LOG.warn("Derived key piggyback requested but token type is {}, " + - "not S3AUTHINFO. Derived key will not be returned.", - s3Token.getTokenType()); - return builder; - } - byte[] derivedKey = ozoneManager.getS3DerivedKey(s3Token.getAwsAccessId(), s3Token.getStrToSign()); - builder.setDerivedKey(ByteString.copyFrom(derivedKey)); - } - return builder; - } } diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyCreateRequestWithFSO.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyCreateRequestWithFSO.java index 3642f96c7fcf..d59b2256fdff 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyCreateRequestWithFSO.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyCreateRequestWithFSO.java @@ -184,8 +184,7 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, Execut checkBucketQuotaInBytes(omMetadataManager, omBucketInfo, preAllocatedSpace); checkBucketQuotaInNamespace(omBucketInfo, numKeysCreated + 1L); - CreateKeyResponse.Builder createKeyResponseBuilder = - getResponseBuilderWithDerivedKey(getOmRequest(), ozoneManager, createKeyRequest); + CreateKeyResponse.Builder createKeyResponseBuilder = CreateKeyResponse.newBuilder(); perfMetrics.addCreateKeyQuotaCheckLatencyNs(Time.monotonicNowNanos() - quotaCheckStartTime); omBucketInfo.incrUsedNamespace(numKeysCreated); diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/protocolPB/OzoneManagerProtocolServerSideTranslatorPB.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/protocolPB/OzoneManagerProtocolServerSideTranslatorPB.java index b5d46fcdb172..4693adbdcf88 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/protocolPB/OzoneManagerProtocolServerSideTranslatorPB.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/protocolPB/OzoneManagerProtocolServerSideTranslatorPB.java @@ -24,6 +24,7 @@ import static org.apache.hadoop.ozone.util.MetricUtil.captureLatencyNs; import com.google.common.annotations.VisibleForTesting; +import com.google.protobuf.ByteString; import com.google.protobuf.RpcController; import com.google.protobuf.ServiceException; import java.io.IOException; @@ -176,6 +177,7 @@ public void logLargeResponseIfNeeded(OMResponse response) { private OMResponse internalProcessRequest(OMRequest request) throws ServiceException { boolean s3Auth = false; + ByteString derivedKey = null; try { if (request.hasS3Authentication()) { @@ -185,6 +187,13 @@ private OMResponse internalProcessRequest(OMRequest request) throws ServiceExcep // If request has S3Authentication, validate S3 credentials. // If current OM is leader and then proceed with the request. S3SecurityUtil.validateS3Credential(request, ozoneManager); + if (ozoneManager.isSecurityEnabled() && request.getCmdType() == OzoneManagerProtocolProtos.Type.CreateKey + && request.getCreateKeyRequest().getDerivedKeyPiggyBacking()) { + derivedKey = ByteString.copyFrom(ozoneManager.getS3DerivedKey(request.getS3Authentication())); + // Only the RPC response needs this key. Older followers must not derive it during apply either. + request = request.toBuilder().setCreateKeyRequest(request.getCreateKeyRequest().toBuilder() + .clearDerivedKeyPiggyBacking()).build(); + } } catch (IOException ex) { return createErrorResponse(request, ex); } @@ -201,13 +210,16 @@ private OMResponse internalProcessRequest(OMRequest request) throws ServiceExcep } // check retry cache - final OMResponse cached = omRatisServer.checkRetryCache(); - if (cached != null) { - return cached; + OMResponse response = omRatisServer.checkRetryCache(); + if (response == null) { + this.lastRequestToSubmit = request; + response = ozoneManager.getOmExecutionFlow().submit(request, true); } - - this.lastRequestToSubmit = request; - return ozoneManager.getOmExecutionFlow().submit(request, true); + if (derivedKey != null && response.getSuccess() && response.hasCreateKeyResponse()) { + return response.toBuilder().setCreateKeyResponse(response.getCreateKeyResponse().toBuilder() + .setDerivedKey(derivedKey)).build(); + } + return response; } finally { OzoneManager.setS3Auth(null); OzoneManager.setStsTokenIdentifier(null); diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeyCreateRequest.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeyCreateRequest.java index f00276040dfa..c317ec622aa7 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeyCreateRequest.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeyCreateRequest.java @@ -1519,12 +1519,11 @@ protected OmKeyInfo checkCreatedPaths( } @Test - public void testCreateKeyWithS3DerivedKey() throws Exception { + public void testCreateKeyDoesNotDeriveSigningKeyDuringApply() throws Exception { when(ozoneManager.getOzoneLockProvider()).thenReturn( new OzoneLockProvider(true, true)); when(ozoneManager.isSecurityEnabled()).thenReturn(true); - byte[] expectedDerivedKey = new byte[] {9, 8, 7, 6}; - when(ozoneManager.getS3DerivedKey(anyString(), anyString())).thenReturn(expectedDerivedKey); + when(ozoneManager.getS3DerivedKey(any())).thenThrow(new IOException("Credentials unavailable during apply")); KeyArgs.Builder keyArgs = KeyArgs.newBuilder() .setVolumeName(volumeName) @@ -1568,8 +1567,8 @@ public void testCreateKeyWithS3DerivedKey() throws Exception { OzoneManagerProtocolProtos.CreateKeyResponse createKeyResponse = response.getOMResponse().getCreateKeyResponse(); assertNotNull(createKeyResponse); - assertTrue(createKeyResponse.hasDerivedKey()); - assertEquals(com.google.protobuf.ByteString.copyFrom(expectedDerivedKey), createKeyResponse.getDerivedKey()); + assertThat(createKeyResponse.hasDerivedKey()).isFalse(); + verify(ozoneManager, never()).getS3DerivedKey(any()); } @Test diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/response/TestS3DerivedKey.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/response/TestS3DerivedKey.java new file mode 100644 index 000000000000..25ed1585d1e8 --- /dev/null +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/response/TestS3DerivedKey.java @@ -0,0 +1,239 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.hadoop.ozone.om.response; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyBoolean; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.CALLS_REAL_METHODS; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import java.io.IOException; +import java.time.Instant; +import org.apache.commons.codec.digest.HmacUtils; +import org.apache.hadoop.hdds.conf.OzoneConfiguration; +import org.apache.hadoop.hdds.utils.ProtocolMessageMetrics; +import org.apache.hadoop.hdds.utils.db.InMemoryTestTable; +import org.apache.hadoop.ozone.om.AWSV4AuthValidator; +import org.apache.hadoop.ozone.om.OMMetadataManager; +import org.apache.hadoop.ozone.om.OzoneManager; +import org.apache.hadoop.ozone.om.S3SecretManager; +import org.apache.hadoop.ozone.om.execution.OMExecutionFlow; +import org.apache.hadoop.ozone.om.ratis.OzoneManagerRatisServer; +import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.CreateKeyRequest; +import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.CreateKeyResponse; +import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.KeyArgs; +import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMRequest; +import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMResponse; +import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.S3Authentication; +import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.Status; +import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.Type; +import org.apache.hadoop.ozone.protocolPB.OzoneManagerProtocolServerSideTranslatorPB; +import org.apache.hadoop.ozone.security.OzoneDelegationTokenSecretManager; +import org.apache.hadoop.ozone.security.STSTokenSecretManager; +import org.apache.hadoop.ozone.security.SecretKeyTestClient; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.CsvSource; +import org.junit.jupiter.params.provider.ValueSource; +import org.mockito.ArgumentCaptor; + +/** Tests derived-key responses after S3 authentication, outside Ratis apply. */ +class TestS3DerivedKey { + private static final String ACCESS_ID = "permanent-access-id"; + private static final String TEMP_ACCESS_ID = "temporary-access-id"; + private static final String SECRET = "permanent-secret"; + private static final String TEMP_SECRET = "temporary-secret"; + private static final String STRING_TO_SIGN = "AWS4-HMAC-SHA256\n20260912T010203Z\n" + + "20260912/us-east-1/s3/aws4_request\n" + + "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855"; + + private final OzoneManager om = mock(OzoneManager.class, CALLS_REAL_METHODS); + private final S3SecretManager secrets = mock(S3SecretManager.class); + private final OMExecutionFlow execution = mock(OMExecutionFlow.class); + private final OzoneManagerRatisServer ratis = mock(OzoneManagerRatisServer.class); + private final SecretKeyTestClient secretKeyClient = new SecretKeyTestClient(); + private final InMemoryTestTable revocations = new InMemoryTestTable<>(); + private OzoneManagerProtocolServerSideTranslatorPB translator; + + @BeforeEach + void setup() throws Exception { + when(om.getConfiguration()).thenReturn(new OzoneConfiguration()); + when(om.isSecurityEnabled()).thenReturn(true); + when(om.getS3SecretManager()).thenReturn(secrets); + when(om.getSecretKeyClient()).thenReturn(secretKeyClient); + when(om.getDelegationTokenMgr()).thenReturn(mock(OzoneDelegationTokenSecretManager.class)); + when(om.getOmExecutionFlow()).thenReturn(execution); + OMMetadataManager metadata = mock(OMMetadataManager.class); + when(om.getMetadataManager()).thenReturn(metadata); + when(metadata.getS3RevokedStsTokenTable()).thenReturn(revocations); + when(secrets.hasS3Secret(ACCESS_ID)).thenReturn(true); + when(secrets.getSecretString(ACCESS_ID)).thenReturn(SECRET); + translator = new OzoneManagerProtocolServerSideTranslatorPB(om, ratis, mock(ProtocolMessageMetrics.class)); + when(execution.submit(any(), anyBoolean())).thenReturn(response(Status.OK)); + } + + @AfterEach + void cleanup() { + assertThat(OzoneManager.getS3Auth()).isNull(); + assertThat(OzoneManager.getStsTokenIdentifier()).isNull(); + } + + @ParameterizedTest + @CsvSource({"false,false", "false,true", "true,false", "true,true"}) + void returnsDerivedKey(boolean sts, boolean cacheHit) throws Exception { + OMRequest request = request(sts, 3600); + OMResponse original = response(Status.OK); + when(ratis.checkRetryCache()).thenReturn(cacheHit ? original : null); + when(execution.submit(any(), anyBoolean())).thenReturn(original); + + OMResponse result = translator.processRequest(request); + + assertThat(result.getStatus()).isEqualTo(Status.OK); + assertThat(result.getCreateKeyResponse().getDerivedKey().toByteArray()) + .isEqualTo(AWSV4AuthValidator.getSigningKey(sts ? TEMP_SECRET : SECRET, STRING_TO_SIGN)); + assertThat(original.getCreateKeyResponse().hasDerivedKey()).isFalse(); + if (cacheHit) { + verify(execution, never()).submit(any(), anyBoolean()); + } else { + ArgumentCaptor submitted = ArgumentCaptor.forClass(OMRequest.class); + verify(execution).submit(submitted.capture(), anyBoolean()); + // The response-only hint must not make older followers resolve credentials during apply. + assertThat(submitted.getValue().getCreateKeyRequest().getDerivedKeyPiggyBacking()).isFalse(); + assertThat(request.getCreateKeyRequest().getDerivedKeyPiggyBacking()).isTrue(); + } + if (sts) { + verify(secrets, never()).getSecretString(anyString()); + } + } + + @Test + void doesNotAttachKeyToFailedResponse() throws Exception { + when(execution.submit(any(), anyBoolean())).thenReturn(response(Status.ACCESS_DENIED)); + + OMResponse result = translator.processRequest(request(true, 3600)); + + assertThat(result.getStatus()).isEqualTo(Status.ACCESS_DENIED); + assertThat(result.getCreateKeyResponse().hasDerivedKey()).isFalse(); + } + + @Test + void doesNotDeriveKeyUnlessRequested() throws Exception { + OMRequest request = request(true, 3600); + request = request.toBuilder().setCreateKeyRequest(request.getCreateKeyRequest().toBuilder() + .clearDerivedKeyPiggyBacking()).build(); + + OMResponse result = translator.processRequest(request); + + assertThat(result.getStatus()).isEqualTo(Status.OK); + assertThat(result.getCreateKeyResponse().hasDerivedKey()).isFalse(); + verify(secrets, never()).getSecretString(anyString()); + } + + @Test + void doesNotDeriveKeyInInsecureMode() throws Exception { + when(om.isSecurityEnabled()).thenReturn(false); + + OMResponse result = translator.processRequest(request(false, 3600)); + + assertThat(result.getStatus()).isEqualTo(Status.OK); + assertThat(result.getCreateKeyResponse().hasDerivedKey()).isFalse(); + verify(secrets, never()).getSecretString(anyString()); + } + + @ParameterizedTest + @ValueSource(booleans = {false, true}) + void rejectsExpiredTokenBeforeSubmissionOrCacheLookup(boolean cacheHit) throws Exception { + when(ratis.checkRetryCache()).thenReturn(cacheHit ? response(Status.OK) : null); + + OMResponse result = translator.processRequest(request(true, -1)); + + assertThat(result.getStatus()).isEqualTo(Status.TOKEN_EXPIRED); + verify(execution, never()).submit(any(), anyBoolean()); + verify(ratis, never()).checkRetryCache(); + } + + @Test + void rejectsRevokedTokenBeforeSubmission() throws Exception { + revocations.put(ACCESS_ID, Instant.now().plusSeconds(60).toEpochMilli()); + + OMResponse result = translator.processRequest(request(true, 3600)); + + assertThat(result.getStatus()).isEqualTo(Status.REVOKED_TOKEN); + verify(execution, never()).submit(any(), anyBoolean()); + } + + @Test + void rejectsInvalidSignatureBeforeSubmission() throws Exception { + OMRequest request = request(true, 3600); + request = request.toBuilder().setS3Authentication(request.getS3Authentication().toBuilder() + .setSignature("invalid")).build(); + + OMResponse result = translator.processRequest(request); + + assertThat(result.getStatus()).isEqualTo(Status.INVALID_TOKEN); + verify(execution, never()).submit(any(), anyBoolean()); + } + + @Test + void handlesSecretLookupFailureBeforeSubmission() throws Exception { + when(secrets.getSecretString(ACCESS_ID)).thenThrow(new IOException("secret unavailable")); + + OMResponse result = translator.processRequest(request(false, 3600)); + + assertThat(result.getSuccess()).isFalse(); + assertThat(result.getCreateKeyResponse().hasDerivedKey()).isFalse(); + verify(execution, never()).submit(any(), anyBoolean()); + } + + private OMRequest request(boolean sts, int durationSeconds) throws Exception { + byte[] signingKey = AWSV4AuthValidator.getSigningKey(sts ? TEMP_SECRET : SECRET, STRING_TO_SIGN); + S3Authentication.Builder auth = S3Authentication.newBuilder() + .setAccessId(sts ? TEMP_ACCESS_ID : ACCESS_ID) + .setStringToSign(STRING_TO_SIGN) + .setSignature(new HmacUtils("HmacSHA256", signingKey).hmacHex(STRING_TO_SIGN)); + if (sts) { + auth.setSessionToken(new STSTokenSecretManager(secretKeyClient).createSTSTokenString( + STSTokenSecretManager.CreateSTSTokenParams.newBuilder() + .setTempAccessKeyId(TEMP_ACCESS_ID) + .setOriginalAccessKeyId(ACCESS_ID) + .setSecretAccessKey(TEMP_SECRET) + .setRoleArn("arn:aws:iam::123456789012:role/test") + .setCreationTime(Instant.now()) + .setDurationSeconds(durationSeconds) + .setSessionPolicy("") + .build())); + } + return OMRequest.newBuilder().setCmdType(Type.CreateKey).setClientId("client") + .setS3Authentication(auth) + .setCreateKeyRequest(CreateKeyRequest.newBuilder().setDerivedKeyPiggyBacking(true) + .setKeyArgs(KeyArgs.newBuilder().setVolumeName("volume").setBucketName("bucket").setKeyName("key"))) + .build(); + } + + private OMResponse response(Status status) { + return OMResponse.newBuilder().setCmdType(Type.CreateKey).setStatus(status) + .setSuccess(status == Status.OK).setCreateKeyResponse(CreateKeyResponse.newBuilder()).build(); + } +} diff --git a/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/SignedChunksInputStream.java b/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/SignedChunksInputStream.java index c494e7aa1e00..77675b801b94 100644 --- a/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/SignedChunksInputStream.java +++ b/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/SignedChunksInputStream.java @@ -21,7 +21,6 @@ import java.io.IOException; import java.io.InputStream; -import java.nio.charset.StandardCharsets; import java.security.MessageDigest; import java.security.NoSuchAlgorithmException; import java.util.Locale; @@ -29,6 +28,7 @@ import java.util.regex.Matcher; import java.util.regex.Pattern; import javax.xml.bind.DatatypeConverter; +import org.apache.commons.codec.digest.DigestUtils; import org.apache.hadoop.ozone.s3.exception.OS3Exception; import org.apache.hadoop.ozone.s3.exception.S3ErrorTable; import org.apache.hadoop.ozone.s3.signature.ChunksValidator; @@ -344,7 +344,7 @@ private void validateTrailer() throws IOException { throw invalidBody("Invalid trailing signature"); } if (validator != null) { - validator.validateTrailer(matcher.group(1), sha256Hex(name + ":" + value + "\n")); + validator.validateTrailer(matcher.group(1), DigestUtils.sha256Hex(name + ":" + value + "\n")); } // AWS SDKs terminate the trailer section with a blank line after the signature. if (!"".equals(readLine(true))) { @@ -381,12 +381,6 @@ private String readLine(boolean trailingHeader) throws IOException { } } - private static String sha256Hex(String value) { - MessageDigest digest = newSha256(); - byte[] hash = digest.digest(value.getBytes(StandardCharsets.UTF_8)); - return DatatypeConverter.printHexBinary(hash).toLowerCase(Locale.ROOT); - } - private void updateDigest(byte b) { if (chunkDigest != null) { chunkDigest.update(b); diff --git a/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestObjectPut.java b/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestObjectPut.java index d14b29847dd4..26bb431c221f 100644 --- a/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestObjectPut.java +++ b/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestObjectPut.java @@ -104,7 +104,9 @@ import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.EmptySource; import org.junit.jupiter.params.provider.MethodSource; +import org.junit.jupiter.params.provider.ValueSource; import org.mockito.MockedStatic; /** @@ -268,21 +270,25 @@ void testPutObjectWithValidSignedChunks() throws Exception { assertThat(keyDetails.getDataSize()).isEqualTo(CONTENT.length()); } - @Test - void testPutObjectWithValidSignedChunksAndTrailer() throws Exception { - configureSignedChunksWithTrailer(CONTENT.length()); + @ParameterizedTest + @EmptySource + @ValueSource(strings = CONTENT) + void testPutObjectWithValidSignedChunksAndTrailer(String content) throws Exception { + configureSignedChunksWithTrailer(content.length()); // The trailer value is covered by the HMAC; checksum calculation is outside this test. assertSucceeds(() -> putObject(signedChunkedBodyWithTrailer( - CONTENT, "x-amz-checksum-crc32c", "sOO8/Q=="))); + content, "x-amz-checksum-crc32c", "sOO8/Q=="))); - assertKeyContent(bucket, KEY_NAME, CONTENT); + assertThat(assertKeyContent(bucket, KEY_NAME, content).getDataSize()).isEqualTo(content.length()); } - @Test - void testPutObjectRejectsTamperedTrailer() { - configureSignedChunksWithTrailer(CONTENT.length()); - String body = signedChunkedBodyWithTrailer(CONTENT, "x-amz-checksum-crc32c", "sOO8/Q==") + @ParameterizedTest + @EmptySource + @ValueSource(strings = CONTENT) + void testPutObjectRejectsTamperedTrailer(String content) { + configureSignedChunksWithTrailer(content.length()); + String body = signedChunkedBodyWithTrailer(content, "x-amz-checksum-crc32c", "sOO8/Q==") .replace("sOO8/Q==", "tampered"); assertErrorResponse(SIGNATURE_DOES_NOT_MATCH, () -> putObject(body)); diff --git a/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/signature/TestChunksValidator.java b/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/signature/TestChunksValidator.java index 6974691d92fc..c41090ab9b47 100644 --- a/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/signature/TestChunksValidator.java +++ b/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/signature/TestChunksValidator.java @@ -28,6 +28,8 @@ import org.apache.hadoop.ozone.s3.exception.OS3Exception; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.function.Executable; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; /** * Verifies {@link ChunksValidator} against the canonical AWS SigV4 streaming @@ -107,8 +109,9 @@ void acceptsUppercaseChunkSignature() { SignatureTestUtils.sha256Hex(chunk, 0, chunk.length))).doesNotThrowAnyException(); } - @Test - void acceptsMatchingTrailerSignature() { + @ParameterizedTest + @ValueSource(booleans = {false, true}) + void validatesTrailerSignature(boolean tampered) { ChunksValidator validator = newTrailerValidator(); byte[] chunk1 = repeat('a', 65536); byte[] chunk2 = repeat('a', 1024); @@ -120,17 +123,14 @@ void acceptsMatchingTrailerSignature() { SignatureTestUtils.sha256Hex(chunk2, 0, chunk2.length))); assertDoesNotThrow(() -> validator.validateChunk(TRAILER_FINAL_CHUNK_SIGNATURE, SignatureTestUtils.sha256Hex(new byte[0], 0, 0))); - assertDoesNotThrow(() -> validator.validateTrailer(TRAILER_SIGNATURE, - SignatureTestUtils.sha256Hex((trailer + "\n").getBytes(java.nio.charset.StandardCharsets.UTF_8), - 0, trailer.length() + 1))); - } - - @Test - void rejectsTamperedTrailerSignature() { - ChunksValidator validator = newTrailerValidator(); - assertSignatureMismatch(() -> validator.validateTrailer( - TRAILER_SIGNATURE.substring(0, TRAILER_SIGNATURE.length() - 1) + "0", - "invalid-trailer-hash")); + String hash = SignatureTestUtils.sha256Hex((trailer + "\n").getBytes(java.nio.charset.StandardCharsets.UTF_8), + 0, trailer.length() + 1); + if (tampered) { + assertSignatureMismatch(() -> validator.validateTrailer( + TRAILER_SIGNATURE.substring(0, TRAILER_SIGNATURE.length() - 1) + "0", hash)); + } else { + assertDoesNotThrow(() -> validator.validateTrailer(TRAILER_SIGNATURE, hash)); + } } @Test From d29a1942ac31746cac28e0d37c7b57e22c7185dc Mon Sep 17 00:00:00 2001 From: KUAN-HAO HUANG <101171023+rich7420@users.noreply.github.com> Date: Tue, 15 Sep 2026 23:40:20 +0800 Subject: [PATCH 4/4] HDDS-15142. Consume final chunk terminator without signature validation --- .../ozone/s3/SignedChunksInputStream.java | 6 +++--- .../ozone/s3/TestSignedChunksInputStream.java | 21 +++++++++++++++++++ 2 files changed, 24 insertions(+), 3 deletions(-) diff --git a/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/SignedChunksInputStream.java b/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/SignedChunksInputStream.java index 77675b801b94..690cee0a1bf1 100644 --- a/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/SignedChunksInputStream.java +++ b/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/SignedChunksInputStream.java @@ -127,11 +127,11 @@ public SignedChunksInputStream(InputStream inputStream, String keyPath) { } /** - * Creates a signed chunk stream. + * Creates a signed chunk stream. Supports one checksum trailer; comma-separated trailer names are not supported. * * @param inputStream the encoded request body * @param keyPath resource used in S3 errors - * @param trailerHeader the value of x-amz-trailer, or null when no trailer is expected + * @param trailerHeader a single supported x-amz-checksum-* header name, or null when no trailer is expected */ public SignedChunksInputStream(InputStream inputStream, String keyPath, String trailerHeader) { originalStream = inputStream; @@ -226,7 +226,7 @@ private boolean ensureChunkPayload() throws IOException { } if (remainingData == 0) { // The final zero-byte chunk has no payload terminator when trailing headers follow it. - if (validator != null && trailerHeader == null) { + if (trailerHeader == null) { readChunkTerminator(); } validateChunk(); diff --git a/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/TestSignedChunksInputStream.java b/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/TestSignedChunksInputStream.java index b1bca25bbadb..866455f8893b 100644 --- a/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/TestSignedChunksInputStream.java +++ b/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/TestSignedChunksInputStream.java @@ -34,6 +34,7 @@ import org.apache.hadoop.ozone.s3.signature.SignatureTestUtils; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.CsvSource; import org.junit.jupiter.params.provider.ValueSource; /** @@ -78,6 +79,26 @@ void testEmptyFile() throws IOException { } } + @ParameterizedTest + @CsvSource({"false, false", "false, true", "true, false", "true, true"}) + void consumesFinalChunkTerminatorWithoutValidator(boolean buffered, boolean empty) throws IOException { + String payload = empty ? "" : "data"; + String body = (empty ? "" : "4;chunk-signature=" + FAKE_SIGNATURE + "\r\ndata\r\n") + + "0;chunk-signature=" + FAKE_SIGNATURE + "\r\n\r\n"; + ByteArrayInputStream original = new ByteArrayInputStream(body.getBytes(UTF_8)); + try (SignedChunksInputStream stream = new SignedChunksInputStream(original, KEY_PATH)) { + if (buffered) { + assertThat(IOUtils.toString(stream, UTF_8)).isEqualTo(payload); + } else { + for (char expected : payload.toCharArray()) { + assertThat(stream.read()).isEqualTo(expected); + } + } + assertThat(stream.read()).isEqualTo(-1); + assertThat(original.available()).isZero(); + } + } + @Test void testEmptyFileWithTrailer() throws IOException { try (InputStream is = wrapContent("0;chunk-signature"