chihsuan commented on code in PR #11222:
URL: https://github.com/apache/ozone/pull/11222#discussion_r3995026227
##########
hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/signature/TestChunksValidator.java:
##########
@@ -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"));
Review Comment:
I wonder if this test could change only the signature. Right now, the chain
has not been advanced, and the hash is fake too, so a broken signature check
might still pass.
##########
hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestObjectPut.java:
##########
@@ -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 {
Review Comment:
Would it be worth adding a test that uploads an empty object with a trailer?
##########
hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/SignedChunksInputStream.java:
##########
@@ -304,6 +320,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"));
Review Comment:
nit: Could we reuse `DigestUtils.sha256Hex` here? Then the local `sha256Hex`
helper can go.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
