smengcl commented on code in PR #4473:
URL: https://github.com/apache/ozone/pull/4473#discussion_r1167042728
##########
hadoop-ozone/dist/src/main/smoketest/s3/objectputget.robot:
##########
@@ -49,7 +49,7 @@ Get object from s3
Get object with wrong signature
Pass Execution If '${SECURITY_ENABLED}' == 'false' Skip in
unsecure cluster
${result} = Execute and Ignore Error curl -i -H
'Authorization: AWS scm/[email protected]:asdfqwerty'
${ENDPOINT_URL}/${BUCKET}/${PREFIX}/putobject/key=value/f1
- Should contain ${result}
403 Forbidden
+ Should contain ${result}
400 Bad Request
Review Comment:
Looks like `403` is the expected status code for `SignatureDoesNotMatch`?
ref:
https://docs.aws.amazon.com/IAM/latest/UserGuide/signature-v4-troubleshooting.html
##########
hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/signature/AuthorizationV4QueryParser.java:
##########
@@ -82,17 +104,116 @@ public SignatureInfo parseSignature() throws
MalformedResourceException {
);
}
+ /**
+ * Validate if algorithm is in expected format.
+ */
+ private void validateAlgorithm()
+ throws MalformedResourceException {
+ String algorithm = queryParameters.get("X-Amz-Algorithm");
+ if (algorithm == null) {
+ throw new MalformedResourceException("Missing hash algorithm.");
+ }
+ if (isEmpty(algorithm) || !algorithm.equals(AWS4_SIGNING_ALGORITHM)) {
+ throw new MalformedResourceException("Unexpected hash algorithm. Algo:"
+ + algorithm);
+ }
Review Comment:
```suggestion
if (algorithm == null) {
throw new MalformedResourceException("Unspecified signature
algorithm.");
}
if (isEmpty(algorithm) || !algorithm.equals(AWS4_SIGNING_ALGORITHM)) {
throw new MalformedResourceException("Unsupported signature algorithm:
" +
algorithm);
}
```
##########
hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/OzoneClientProducer.java:
##########
@@ -83,6 +84,10 @@ public S3Auth getSignature() {
if (signatureInfo.getVersion() == Version.V4) {
stringToSign =
StringToSignProducer.createSignatureBase(signatureInfo, context);
+ } else {
+ LOG.debug("AWS signature version not supported. version: {}",
+ signatureInfo.getVersion());
+ throw S3_AUTHINFO_CREATION_ERROR;
Review Comment:
nit
```suggestion
LOG.debug("Unsupported AWS signature version: {}",
signatureInfo.getVersion());
throw S3_AUTHINFO_CREATION_ERROR;
```
##########
hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/signature/AuthorizationV4QueryParser.java:
##########
@@ -82,17 +104,116 @@ public SignatureInfo parseSignature() throws
MalformedResourceException {
);
}
+ /**
+ * Validate if algorithm is in expected format.
+ */
+ private void validateAlgorithm()
+ throws MalformedResourceException {
+ String algorithm = queryParameters.get("X-Amz-Algorithm");
+ if (algorithm == null) {
+ throw new MalformedResourceException("Missing hash algorithm.");
+ }
+ if (isEmpty(algorithm) || !algorithm.equals(AWS4_SIGNING_ALGORITHM)) {
+ throw new MalformedResourceException("Unexpected hash algorithm. Algo:"
+ + algorithm);
+ }
+ }
+
+ /** According to AWS documentation.
+ * https://docs.aws.amazon.com/AmazonS3/latest/
+ * API/sigv4-query-string-auth.html
+ * X-Amz-Date: The date and time format must follow the
+ * ISO 8601 standard, and must be formatted with the
+ * "yyyyMMddTHHmmssZ" format
+ * X-Amz-Expires: The minimum value you can set is 1,
+ * and the maximum is 604800
+ */
@VisibleForTesting
- protected void validateDateAndExpires() {
+ protected void validateDateAndExpires()
+ throws MalformedResourceException, DateTimeParseException {
final String dateString = queryParameters.get("X-Amz-Date");
final String expiresString = queryParameters.get("X-Amz-Expires");
- if (expiresString != null && expiresString.length() > 0) {
- final Long expires = Long.valueOf(expiresString);
-
+ if (dateString == null || expiresString == null ||
+ dateString.length() == 0 || expiresString.length() == 0) {
+ throw new MalformedResourceException(
+ "dateString or expiresString are missing or empty.");
+ }
+ final Long expires = Long.valueOf(expiresString);
+ if (expires >= X_AMZ_EXPIRES_MIN && expires <= X_AMZ_EXPIRES_MAX) {
if (ZonedDateTime.parse(dateString, StringToSignProducer.TIME_FORMATTER)
.plus(expires, SECONDS).isBefore(ZonedDateTime.now())) {
- throw new IllegalArgumentException("Pre-signed S3 url is expired");
+ throw new MalformedResourceException("Pre-signed S3 url is expired. "
+ + "dateString:" + dateString
+ + " expiresString:" + expiresString);
+ }
+ } else {
+ throw new MalformedResourceException("Invalid expiry duration. "
+ + "X-Amz-Expires should be between " + X_AMZ_EXPIRES_MIN
+ + "and" + X_AMZ_EXPIRES_MAX + " expiresString:" + expiresString);
+ }
+ }
+
+ private void validateCredential(Credential credential)
+ throws MalformedResourceException {
+ if (credential.getAccessKeyID().isEmpty()) {
+ throw new MalformedResourceException(
+ "AWS access id shouldn't be empty. credential: " + credential);
+ }
+ if (credential.getAwsRegion().isEmpty()) {
+ throw new MalformedResourceException(
+ "AWS region shouldn't be empty. credential: " + credential);
+ }
+ if (credential.getAwsRequest().isEmpty() ||
+ !(credential.getAwsRequest().equals("aws4_request"))) {
+ throw new MalformedResourceException(
+ "AWS request shouldn't be empty or invalid. credential:"
+ + credential);
+ }
+ if (credential.getAwsService().isEmpty()) {
+ throw new MalformedResourceException(
+ "AWS service shouldn't be empty. credential:" + credential);
+ }
+ // Date should not be empty and should be properly formatted.
+ if (!credential.getDate().isEmpty()) {
+ try {
+ LocalDate.parse(credential.getDate(), DATE_FORMATTER);
+ } catch (DateTimeParseException ex) {
+ throw new MalformedResourceException(
+ "AWS date format is invalid. credential:" + credential);
}
+ } else {
+ throw new MalformedResourceException(
+ "AWS date shouldn't be empty. credential:{}" + credential);
+ }
+ }
+
+ /**
+ * Validate Signed headers.
+ */
+ private void validateSignedHeaders()
+ throws MalformedResourceException {
+ String signedHeadersStr = queryParameters.get("X-Amz-SignedHeaders");
+ if (signedHeadersStr == null || isEmpty(signedHeadersStr)
+ || signedHeadersStr.split(";").length == 0) {
Review Comment:
indentation
##########
hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/signature/AuthorizationV4QueryParser.java:
##########
@@ -82,17 +104,116 @@ public SignatureInfo parseSignature() throws
MalformedResourceException {
);
}
+ /**
+ * Validate if algorithm is in expected format.
+ */
+ private void validateAlgorithm()
+ throws MalformedResourceException {
+ String algorithm = queryParameters.get("X-Amz-Algorithm");
+ if (algorithm == null) {
+ throw new MalformedResourceException("Missing hash algorithm.");
+ }
+ if (isEmpty(algorithm) || !algorithm.equals(AWS4_SIGNING_ALGORITHM)) {
+ throw new MalformedResourceException("Unexpected hash algorithm. Algo:"
+ + algorithm);
+ }
+ }
+
+ /** According to AWS documentation.
+ * https://docs.aws.amazon.com/AmazonS3/latest/
+ * API/sigv4-query-string-auth.html
+ * X-Amz-Date: The date and time format must follow the
+ * ISO 8601 standard, and must be formatted with the
+ * "yyyyMMddTHHmmssZ" format
+ * X-Amz-Expires: The minimum value you can set is 1,
+ * and the maximum is 604800
+ */
@VisibleForTesting
- protected void validateDateAndExpires() {
+ protected void validateDateAndExpires()
+ throws MalformedResourceException, DateTimeParseException {
final String dateString = queryParameters.get("X-Amz-Date");
final String expiresString = queryParameters.get("X-Amz-Expires");
- if (expiresString != null && expiresString.length() > 0) {
- final Long expires = Long.valueOf(expiresString);
-
+ if (dateString == null || expiresString == null ||
+ dateString.length() == 0 || expiresString.length() == 0) {
+ throw new MalformedResourceException(
+ "dateString or expiresString are missing or empty.");
+ }
+ final Long expires = Long.valueOf(expiresString);
+ if (expires >= X_AMZ_EXPIRES_MIN && expires <= X_AMZ_EXPIRES_MAX) {
if (ZonedDateTime.parse(dateString, StringToSignProducer.TIME_FORMATTER)
.plus(expires, SECONDS).isBefore(ZonedDateTime.now())) {
- throw new IllegalArgumentException("Pre-signed S3 url is expired");
+ throw new MalformedResourceException("Pre-signed S3 url is expired. "
+ + "dateString:" + dateString
+ + " expiresString:" + expiresString);
+ }
+ } else {
+ throw new MalformedResourceException("Invalid expiry duration. "
+ + "X-Amz-Expires should be between " + X_AMZ_EXPIRES_MIN
+ + "and" + X_AMZ_EXPIRES_MAX + " expiresString:" + expiresString);
+ }
+ }
+
+ private void validateCredential(Credential credential)
+ throws MalformedResourceException {
+ if (credential.getAccessKeyID().isEmpty()) {
+ throw new MalformedResourceException(
+ "AWS access id shouldn't be empty. credential: " + credential);
+ }
+ if (credential.getAwsRegion().isEmpty()) {
+ throw new MalformedResourceException(
+ "AWS region shouldn't be empty. credential: " + credential);
+ }
+ if (credential.getAwsRequest().isEmpty() ||
+ !(credential.getAwsRequest().equals("aws4_request"))) {
+ throw new MalformedResourceException(
+ "AWS request shouldn't be empty or invalid. credential:"
+ + credential);
+ }
+ if (credential.getAwsService().isEmpty()) {
+ throw new MalformedResourceException(
+ "AWS service shouldn't be empty. credential:" + credential);
+ }
+ // Date should not be empty and should be properly formatted.
+ if (!credential.getDate().isEmpty()) {
+ try {
+ LocalDate.parse(credential.getDate(), DATE_FORMATTER);
+ } catch (DateTimeParseException ex) {
+ throw new MalformedResourceException(
+ "AWS date format is invalid. credential:" + credential);
}
+ } else {
+ throw new MalformedResourceException(
+ "AWS date shouldn't be empty. credential:{}" + credential);
+ }
+ }
Review Comment:
indentation
##########
hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/signature/AuthorizationV4QueryParser.java:
##########
@@ -82,17 +104,116 @@ public SignatureInfo parseSignature() throws
MalformedResourceException {
);
}
+ /**
+ * Validate if algorithm is in expected format.
+ */
+ private void validateAlgorithm()
+ throws MalformedResourceException {
+ String algorithm = queryParameters.get("X-Amz-Algorithm");
+ if (algorithm == null) {
+ throw new MalformedResourceException("Missing hash algorithm.");
+ }
+ if (isEmpty(algorithm) || !algorithm.equals(AWS4_SIGNING_ALGORITHM)) {
+ throw new MalformedResourceException("Unexpected hash algorithm. Algo:"
+ + algorithm);
+ }
+ }
+
+ /** According to AWS documentation.
+ * https://docs.aws.amazon.com/AmazonS3/latest/
+ * API/sigv4-query-string-auth.html
+ * X-Amz-Date: The date and time format must follow the
+ * ISO 8601 standard, and must be formatted with the
+ * "yyyyMMddTHHmmssZ" format
+ * X-Amz-Expires: The minimum value you can set is 1,
+ * and the maximum is 604800
+ */
@VisibleForTesting
- protected void validateDateAndExpires() {
+ protected void validateDateAndExpires()
+ throws MalformedResourceException, DateTimeParseException {
final String dateString = queryParameters.get("X-Amz-Date");
final String expiresString = queryParameters.get("X-Amz-Expires");
- if (expiresString != null && expiresString.length() > 0) {
- final Long expires = Long.valueOf(expiresString);
-
+ if (dateString == null || expiresString == null ||
+ dateString.length() == 0 || expiresString.length() == 0) {
+ throw new MalformedResourceException(
+ "dateString or expiresString are missing or empty.");
+ }
+ final Long expires = Long.valueOf(expiresString);
+ if (expires >= X_AMZ_EXPIRES_MIN && expires <= X_AMZ_EXPIRES_MAX) {
if (ZonedDateTime.parse(dateString, StringToSignProducer.TIME_FORMATTER)
.plus(expires, SECONDS).isBefore(ZonedDateTime.now())) {
- throw new IllegalArgumentException("Pre-signed S3 url is expired");
+ throw new MalformedResourceException("Pre-signed S3 url is expired. "
+ + "dateString:" + dateString
+ + " expiresString:" + expiresString);
+ }
+ } else {
+ throw new MalformedResourceException("Invalid expiry duration. "
+ + "X-Amz-Expires should be between " + X_AMZ_EXPIRES_MIN
+ + "and" + X_AMZ_EXPIRES_MAX + " expiresString:" + expiresString);
+ }
+ }
+
+ private void validateCredential(Credential credential)
+ throws MalformedResourceException {
+ if (credential.getAccessKeyID().isEmpty()) {
+ throw new MalformedResourceException(
+ "AWS access id shouldn't be empty. credential: " + credential);
+ }
+ if (credential.getAwsRegion().isEmpty()) {
+ throw new MalformedResourceException(
+ "AWS region shouldn't be empty. credential: " + credential);
+ }
+ if (credential.getAwsRequest().isEmpty() ||
+ !(credential.getAwsRequest().equals("aws4_request"))) {
+ throw new MalformedResourceException(
+ "AWS request shouldn't be empty or invalid. credential:"
+ + credential);
+ }
+ if (credential.getAwsService().isEmpty()) {
+ throw new MalformedResourceException(
+ "AWS service shouldn't be empty. credential:" + credential);
+ }
+ // Date should not be empty and should be properly formatted.
+ if (!credential.getDate().isEmpty()) {
+ try {
+ LocalDate.parse(credential.getDate(), DATE_FORMATTER);
+ } catch (DateTimeParseException ex) {
+ throw new MalformedResourceException(
+ "AWS date format is invalid. credential:" + credential);
}
+ } else {
+ throw new MalformedResourceException(
+ "AWS date shouldn't be empty. credential:{}" + credential);
+ }
+ }
+
+ /**
+ * Validate Signed headers.
+ */
+ private void validateSignedHeaders()
+ throws MalformedResourceException {
+ String signedHeadersStr = queryParameters.get("X-Amz-SignedHeaders");
+ if (signedHeadersStr == null || isEmpty(signedHeadersStr)
+ || signedHeadersStr.split(";").length == 0) {
+ throw new MalformedResourceException("No signed headers found.");
+ }
+ }
+
+ /**
+ * Validate signature.
+ */
+ private void validateSignature()
+ throws MalformedResourceException {
Review Comment:
indentation
##########
hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/signature/StringToSignProducer.java:
##########
@@ -183,19 +185,38 @@ public static String buildCanonicalRequest(
canonicalHeaders.append(NEWLINE);
// Set for testing purpose only to skip date and host validation.
- validateSignedHeader(schema, header, headerValue);
+ try {
+ validateSignedHeader(schema, header, headerValue);
+ } catch (DateTimeParseException ex) {
+ LOG.error("DateTime format invalid.", ex);
+ throw S3_AUTHINFO_CREATION_ERROR;
+ }
} else {
- throw new RuntimeException("Header " + header + " not present in " +
- "request but requested to be signed.");
+ LOG.error("Header " + header + " not present in " +
+ "request but requested to be signed.");
Review Comment:
indentation
##########
hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/signature/AuthorizationV4QueryParser.java:
##########
@@ -56,7 +67,14 @@ public SignatureInfo parseSignature() throws
MalformedResourceException {
return null;
}
- validateDateAndExpires();
+ validateAlgorithm();
+ try {
+ validateDateAndExpires();
+ } catch (DateTimeParseException ex) {
+ throw new MalformedResourceException(
+ "Invalid X-Amz-Date format: "
+ + queryParameters.get("X-Amz-Date"));
+ }
Review Comment:
indentation
##########
hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/signature/AuthorizationV4QueryParser.java:
##########
@@ -82,17 +104,116 @@ public SignatureInfo parseSignature() throws
MalformedResourceException {
);
}
+ /**
+ * Validate if algorithm is in expected format.
+ */
+ private void validateAlgorithm()
+ throws MalformedResourceException {
+ String algorithm = queryParameters.get("X-Amz-Algorithm");
+ if (algorithm == null) {
+ throw new MalformedResourceException("Missing hash algorithm.");
+ }
+ if (isEmpty(algorithm) || !algorithm.equals(AWS4_SIGNING_ALGORITHM)) {
+ throw new MalformedResourceException("Unexpected hash algorithm. Algo:"
+ + algorithm);
+ }
+ }
+
+ /** According to AWS documentation.
Review Comment:
```suggestion
/**
* According to AWS documentation:
```
##########
hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/TestOzoneClientProducer.java:
##########
@@ -154,18 +164,21 @@ public void testGetSignature() {
fail("Empty AuthHeader must fail");
}
} catch (WebApplicationException ex) {
- if (authHeader == null || authHeader.equals("")) {
- // Empty auth header should be 403
- Assert.assertEquals(HTTP_FORBIDDEN, ex.getResponse().getStatus());
- // TODO: Should return XML in body like this (bot not for now):
- // <Error>
- // <Code>AccessDenied</Code><Message>Access Denied</Message>
- // <RequestId>...</RequestId><HostId>...</HostId>
- // </Error>
+ Assert.assertEquals(HTTP_BAD_REQUEST, ex.getResponse().getStatus());
+ if (authHeader == null || authHeader.equals("") ||
+ authHeader.startsWith("AWS ")) {
Review Comment:
```suggestion
if (authHeader == null || authHeader.isEmpty() ||
authHeader.startsWith("AWS ")) {
```
##########
hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/signature/StringToSignProducer.java:
##########
@@ -183,19 +185,38 @@ public static String buildCanonicalRequest(
canonicalHeaders.append(NEWLINE);
// Set for testing purpose only to skip date and host validation.
- validateSignedHeader(schema, header, headerValue);
+ try {
+ validateSignedHeader(schema, header, headerValue);
+ } catch (DateTimeParseException ex) {
+ LOG.error("DateTime format invalid.", ex);
+ throw S3_AUTHINFO_CREATION_ERROR;
+ }
} else {
- throw new RuntimeException("Header " + header + " not present in " +
- "request but requested to be signed.");
+ LOG.error("Header " + header + " not present in " +
+ "request but requested to be signed.");
+ throw S3_AUTHINFO_CREATION_ERROR;
}
}
+ validateCanonicalHeaders(canonicalHeaders.toString(), headers,
+ unsignedPayload);
+
String payloadHash;
if (UNSIGNED_PAYLOAD.equals(
headers.get(X_AMZ_CONTENT_SHA256)) || unsignedPayload) {
payloadHash = UNSIGNED_PAYLOAD;
} else {
+ // According to AWS Sig V4 documentation
+ // https://docs.aws.amazon.com/AmazonS3/latest/API/
+ // sig-v4-header-based-auth.html
+ // Note: The x-amz-content-sha256 header is required
+ // for all AWS Signature Version 4 requests.(using Authorization header)
+ if (!headers.containsKey(X_AMZ_CONTENT_SHA256)) {
+ LOG.error("The request must include x-amz-content-sha256 header "
+ + "for signed paylods");
Review Comment:
```suggestion
LOG.error("The request must include " + X_AMZ_CONTENT_SHA256
+ " header for signed payload");
```
##########
hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/TestOzoneClientProducer.java:
##########
@@ -154,18 +164,21 @@ public void testGetSignature() {
fail("Empty AuthHeader must fail");
}
} catch (WebApplicationException ex) {
- if (authHeader == null || authHeader.equals("")) {
- // Empty auth header should be 403
- Assert.assertEquals(HTTP_FORBIDDEN, ex.getResponse().getStatus());
- // TODO: Should return XML in body like this (bot not for now):
- // <Error>
- // <Code>AccessDenied</Code><Message>Access Denied</Message>
- // <RequestId>...</RequestId><HostId>...</HostId>
- // </Error>
+ Assert.assertEquals(HTTP_BAD_REQUEST, ex.getResponse().getStatus());
+ if (authHeader == null || authHeader.equals("") ||
+ authHeader.startsWith("AWS ")) {
+ // Empty auth header and unsupported AWS signature
+ // should fail with Invalid Request.
+ Assert.assertEquals(S3_AUTHINFO_CREATION_ERROR.getErrorMessage(),
+ ex.getMessage());
} else {
- // Other requests have stale timestamp and thus should fail
- Assert.assertEquals(HTTP_BAD_REQUEST, ex.getResponse().getStatus());
+ // Other requests have stale timestamp and
+ // should fail with Malformed Authorization Header.
+ Assert.assertEquals(MALFORMED_HEADER.getErrorMessage(),
+ ex.getMessage());
Review Comment:
```suggestion
ex.getMessage());
```
##########
hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/signature/AuthorizationV4QueryParser.java:
##########
@@ -82,17 +104,116 @@ public SignatureInfo parseSignature() throws
MalformedResourceException {
);
}
+ /**
+ * Validate if algorithm is in expected format.
+ */
+ private void validateAlgorithm()
+ throws MalformedResourceException {
+ String algorithm = queryParameters.get("X-Amz-Algorithm");
+ if (algorithm == null) {
+ throw new MalformedResourceException("Missing hash algorithm.");
+ }
+ if (isEmpty(algorithm) || !algorithm.equals(AWS4_SIGNING_ALGORITHM)) {
+ throw new MalformedResourceException("Unexpected hash algorithm. Algo:"
+ + algorithm);
+ }
+ }
+
+ /** According to AWS documentation.
+ * https://docs.aws.amazon.com/AmazonS3/latest/
+ * API/sigv4-query-string-auth.html
+ * X-Amz-Date: The date and time format must follow the
+ * ISO 8601 standard, and must be formatted with the
+ * "yyyyMMddTHHmmssZ" format
+ * X-Amz-Expires: The minimum value you can set is 1,
+ * and the maximum is 604800
+ */
@VisibleForTesting
- protected void validateDateAndExpires() {
+ protected void validateDateAndExpires()
+ throws MalformedResourceException, DateTimeParseException {
final String dateString = queryParameters.get("X-Amz-Date");
final String expiresString = queryParameters.get("X-Amz-Expires");
- if (expiresString != null && expiresString.length() > 0) {
- final Long expires = Long.valueOf(expiresString);
-
+ if (dateString == null || expiresString == null ||
+ dateString.length() == 0 || expiresString.length() == 0) {
Review Comment:
```suggestion
if (dateString == null ||
expiresString == null ||
dateString.length() == 0 ||
expiresString.length() == 0) {
```
##########
hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/signature/AuthorizationV4HeaderParser.java:
##########
@@ -189,7 +191,14 @@ private Credential parseCredentials(String credential)
// Date should not be empty and within valid range.
if (!credentialObj.getDate().isEmpty()) {
- validateDateRange(credentialObj);
+ try {
+ validateDateRange(credentialObj);
+ } catch (DateTimeParseException ex) {
+ throw new MalformedResourceException(
+ "AWS date format is invalid. credential:" + credential,
+ authHeader);
Review Comment:
indentation
```suggestion
throw new MalformedResourceException(
"AWS date format is invalid. credential:" + credential,
authHeader);
```
You can avoid this in the future by importing ozone code style:
https://github.com/apache/ozone/blob/dd003040a41def491e8de003ef8539ce40854972/CONTRIBUTING.md#L115-L121
##########
hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/signature/AuthorizationV4QueryParser.java:
##########
@@ -82,17 +104,116 @@ public SignatureInfo parseSignature() throws
MalformedResourceException {
);
}
+ /**
+ * Validate if algorithm is in expected format.
+ */
+ private void validateAlgorithm()
+ throws MalformedResourceException {
+ String algorithm = queryParameters.get("X-Amz-Algorithm");
+ if (algorithm == null) {
+ throw new MalformedResourceException("Missing hash algorithm.");
+ }
+ if (isEmpty(algorithm) || !algorithm.equals(AWS4_SIGNING_ALGORITHM)) {
+ throw new MalformedResourceException("Unexpected hash algorithm. Algo:"
+ + algorithm);
+ }
+ }
+
+ /** According to AWS documentation.
+ * https://docs.aws.amazon.com/AmazonS3/latest/
+ * API/sigv4-query-string-auth.html
+ * X-Amz-Date: The date and time format must follow the
+ * ISO 8601 standard, and must be formatted with the
+ * "yyyyMMddTHHmmssZ" format
+ * X-Amz-Expires: The minimum value you can set is 1,
+ * and the maximum is 604800
+ */
@VisibleForTesting
- protected void validateDateAndExpires() {
+ protected void validateDateAndExpires()
+ throws MalformedResourceException, DateTimeParseException {
final String dateString = queryParameters.get("X-Amz-Date");
final String expiresString = queryParameters.get("X-Amz-Expires");
- if (expiresString != null && expiresString.length() > 0) {
- final Long expires = Long.valueOf(expiresString);
-
+ if (dateString == null || expiresString == null ||
+ dateString.length() == 0 || expiresString.length() == 0) {
+ throw new MalformedResourceException(
+ "dateString or expiresString are missing or empty.");
+ }
+ final Long expires = Long.valueOf(expiresString);
Review Comment:
nit
```suggestion
final long expires = Long.parseLong(expiresString);
```
##########
hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/signature/AuthorizationV4QueryParser.java:
##########
@@ -82,17 +104,116 @@ public SignatureInfo parseSignature() throws
MalformedResourceException {
);
}
+ /**
+ * Validate if algorithm is in expected format.
+ */
+ private void validateAlgorithm()
+ throws MalformedResourceException {
+ String algorithm = queryParameters.get("X-Amz-Algorithm");
+ if (algorithm == null) {
+ throw new MalformedResourceException("Missing hash algorithm.");
+ }
+ if (isEmpty(algorithm) || !algorithm.equals(AWS4_SIGNING_ALGORITHM)) {
+ throw new MalformedResourceException("Unexpected hash algorithm. Algo:"
+ + algorithm);
+ }
+ }
+
+ /** According to AWS documentation.
+ * https://docs.aws.amazon.com/AmazonS3/latest/
+ * API/sigv4-query-string-auth.html
+ * X-Amz-Date: The date and time format must follow the
+ * ISO 8601 standard, and must be formatted with the
+ * "yyyyMMddTHHmmssZ" format
+ * X-Amz-Expires: The minimum value you can set is 1,
+ * and the maximum is 604800
+ */
@VisibleForTesting
- protected void validateDateAndExpires() {
+ protected void validateDateAndExpires()
+ throws MalformedResourceException, DateTimeParseException {
final String dateString = queryParameters.get("X-Amz-Date");
final String expiresString = queryParameters.get("X-Amz-Expires");
- if (expiresString != null && expiresString.length() > 0) {
- final Long expires = Long.valueOf(expiresString);
-
+ if (dateString == null || expiresString == null ||
+ dateString.length() == 0 || expiresString.length() == 0) {
+ throw new MalformedResourceException(
+ "dateString or expiresString are missing or empty.");
Review Comment:
```suggestion
throw new MalformedResourceException(
"dateString or expiresString are missing or empty.");
```
##########
hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/signature/AuthorizationV4HeaderParser.java:
##########
@@ -176,9 +177,10 @@ private Credential parseCredentials(String credential)
"AWS region shouldn't be empty. credential: " + credential,
authHeader);
}
- if (credentialObj.getAwsRequest().isEmpty()) {
+ if (credentialObj.getAwsRequest().isEmpty() ||
+ !(credentialObj.getAwsRequest().equals("aws4_request"))) {
throw new MalformedResourceException(
- "AWS request shouldn't be empty. credential:" + credential,
+ "AWS request shouldn't be empty or invalid. credential:" +
credential,
Review Comment:
```suggestion
"AWS request is empty or invalid. credential: " + credential,
```
##########
hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/signature/AuthorizationV4QueryParser.java:
##########
@@ -82,17 +104,116 @@ public SignatureInfo parseSignature() throws
MalformedResourceException {
);
}
+ /**
+ * Validate if algorithm is in expected format.
+ */
+ private void validateAlgorithm()
+ throws MalformedResourceException {
+ String algorithm = queryParameters.get("X-Amz-Algorithm");
+ if (algorithm == null) {
+ throw new MalformedResourceException("Missing hash algorithm.");
+ }
+ if (isEmpty(algorithm) || !algorithm.equals(AWS4_SIGNING_ALGORITHM)) {
+ throw new MalformedResourceException("Unexpected hash algorithm. Algo:"
+ + algorithm);
+ }
+ }
+
+ /** According to AWS documentation.
+ * https://docs.aws.amazon.com/AmazonS3/latest/
+ * API/sigv4-query-string-auth.html
+ * X-Amz-Date: The date and time format must follow the
+ * ISO 8601 standard, and must be formatted with the
+ * "yyyyMMddTHHmmssZ" format
+ * X-Amz-Expires: The minimum value you can set is 1,
+ * and the maximum is 604800
+ */
@VisibleForTesting
- protected void validateDateAndExpires() {
+ protected void validateDateAndExpires()
+ throws MalformedResourceException, DateTimeParseException {
final String dateString = queryParameters.get("X-Amz-Date");
final String expiresString = queryParameters.get("X-Amz-Expires");
- if (expiresString != null && expiresString.length() > 0) {
- final Long expires = Long.valueOf(expiresString);
-
+ if (dateString == null || expiresString == null ||
+ dateString.length() == 0 || expiresString.length() == 0) {
+ throw new MalformedResourceException(
+ "dateString or expiresString are missing or empty.");
+ }
+ final Long expires = Long.valueOf(expiresString);
+ if (expires >= X_AMZ_EXPIRES_MIN && expires <= X_AMZ_EXPIRES_MAX) {
if (ZonedDateTime.parse(dateString, StringToSignProducer.TIME_FORMATTER)
.plus(expires, SECONDS).isBefore(ZonedDateTime.now())) {
- throw new IllegalArgumentException("Pre-signed S3 url is expired");
+ throw new MalformedResourceException("Pre-signed S3 url is expired. "
+ + "dateString:" + dateString
+ + " expiresString:" + expiresString);
Review Comment:
indentation
##########
hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/signature/AuthorizationV4QueryParser.java:
##########
@@ -65,9 +83,13 @@ public SignatureInfo parseSignature() throws
MalformedResourceException {
try {
credential = new Credential(urlDecode(rawCredential));
} catch (UnsupportedEncodingException e) {
- throw new IllegalArgumentException(
- "X-Amz-Credential is not proper URL encoded");
+ throw new MalformedResourceException(
+ "X-Amz-Credential is not proper URL encoded rawCredential:"
+ + rawCredential);
}
Review Comment:
indentation
##########
hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/signature/TestAuthorizationV4HeaderParser.java:
##########
@@ -347,6 +362,15 @@ public void testV4HeaderHashAlgoValidationFailure() throws
Exception {
+ "=fe5f80f77d5fa3beca038a248ff027";
Assert.assertNull(new AuthorizationV4HeaderParser(auth3, SAMPLE_DATE)
.parseSignature());
+
+ // Invalid algorithm
+ String auth4 = "AWS4-ZAVC-HJUA123 " +
+ "Credential=" + curDate + "/us-east-1/s3/aws4_request, " +
+ "SignedHeaders=host;range;x-amz-date, " +
+ "Signature=fe5f80f77d5fa3beca038a248ff027";
+ LambdaTestUtils.intercept(MalformedResourceException.class, "",
+ () -> new AuthorizationV4HeaderParser(auth4, SAMPLE_DATE)
+ .parseSignature());
Review Comment:
```suggestion
// Invalid algorithm
String auth4 = "AWS4-ZAVC-HJUA123 " +
"Credential=" + curDate + "/us-east-1/s3/aws4_request, " +
"SignedHeaders=host;range;x-amz-date, " +
"Signature=fe5f80f77d5fa3beca038a248ff027";
LambdaTestUtils.intercept(MalformedResourceException.class, "",
() -> new AuthorizationV4HeaderParser(auth4, SAMPLE_DATE)
.parseSignature());
```
##########
hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/signature/AuthorizationV4HeaderParser.java:
##########
@@ -176,9 +177,10 @@ private Credential parseCredentials(String credential)
"AWS region shouldn't be empty. credential: " + credential,
authHeader);
}
- if (credentialObj.getAwsRequest().isEmpty()) {
+ if (credentialObj.getAwsRequest().isEmpty() ||
+ !(credentialObj.getAwsRequest().equals("aws4_request"))) {
Review Comment:
Wrap `"aws4_request"` in a constant?
##########
hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/signature/TestStringToSignProducer.java:
##########
Review Comment:
fix indentation
##########
hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/signature/AuthorizationV4QueryParser.java:
##########
@@ -82,17 +104,116 @@ public SignatureInfo parseSignature() throws
MalformedResourceException {
);
}
+ /**
+ * Validate if algorithm is in expected format.
+ */
+ private void validateAlgorithm()
+ throws MalformedResourceException {
+ String algorithm = queryParameters.get("X-Amz-Algorithm");
+ if (algorithm == null) {
+ throw new MalformedResourceException("Missing hash algorithm.");
+ }
+ if (isEmpty(algorithm) || !algorithm.equals(AWS4_SIGNING_ALGORITHM)) {
+ throw new MalformedResourceException("Unexpected hash algorithm. Algo:"
+ + algorithm);
+ }
+ }
+
+ /** According to AWS documentation.
+ * https://docs.aws.amazon.com/AmazonS3/latest/
+ * API/sigv4-query-string-auth.html
+ * X-Amz-Date: The date and time format must follow the
+ * ISO 8601 standard, and must be formatted with the
+ * "yyyyMMddTHHmmssZ" format
+ * X-Amz-Expires: The minimum value you can set is 1,
+ * and the maximum is 604800
+ */
@VisibleForTesting
- protected void validateDateAndExpires() {
+ protected void validateDateAndExpires()
+ throws MalformedResourceException, DateTimeParseException {
final String dateString = queryParameters.get("X-Amz-Date");
final String expiresString = queryParameters.get("X-Amz-Expires");
- if (expiresString != null && expiresString.length() > 0) {
- final Long expires = Long.valueOf(expiresString);
-
+ if (dateString == null || expiresString == null ||
+ dateString.length() == 0 || expiresString.length() == 0) {
+ throw new MalformedResourceException(
+ "dateString or expiresString are missing or empty.");
+ }
+ final Long expires = Long.valueOf(expiresString);
+ if (expires >= X_AMZ_EXPIRES_MIN && expires <= X_AMZ_EXPIRES_MAX) {
if (ZonedDateTime.parse(dateString, StringToSignProducer.TIME_FORMATTER)
.plus(expires, SECONDS).isBefore(ZonedDateTime.now())) {
- throw new IllegalArgumentException("Pre-signed S3 url is expired");
+ throw new MalformedResourceException("Pre-signed S3 url is expired. "
+ + "dateString:" + dateString
+ + " expiresString:" + expiresString);
+ }
+ } else {
+ throw new MalformedResourceException("Invalid expiry duration. "
+ + "X-Amz-Expires should be between " + X_AMZ_EXPIRES_MIN
+ + "and" + X_AMZ_EXPIRES_MAX + " expiresString:" + expiresString);
Review Comment:
indentation
##########
hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/signature/AuthorizationV4QueryParser.java:
##########
@@ -82,17 +104,116 @@ public SignatureInfo parseSignature() throws
MalformedResourceException {
);
}
+ /**
+ * Validate if algorithm is in expected format.
+ */
+ private void validateAlgorithm()
+ throws MalformedResourceException {
+ String algorithm = queryParameters.get("X-Amz-Algorithm");
+ if (algorithm == null) {
+ throw new MalformedResourceException("Missing hash algorithm.");
+ }
+ if (isEmpty(algorithm) || !algorithm.equals(AWS4_SIGNING_ALGORITHM)) {
+ throw new MalformedResourceException("Unexpected hash algorithm. Algo:"
+ + algorithm);
+ }
+ }
+
+ /** According to AWS documentation.
+ * https://docs.aws.amazon.com/AmazonS3/latest/
+ * API/sigv4-query-string-auth.html
+ * X-Amz-Date: The date and time format must follow the
+ * ISO 8601 standard, and must be formatted with the
+ * "yyyyMMddTHHmmssZ" format
+ * X-Amz-Expires: The minimum value you can set is 1,
+ * and the maximum is 604800
+ */
@VisibleForTesting
- protected void validateDateAndExpires() {
+ protected void validateDateAndExpires()
+ throws MalformedResourceException, DateTimeParseException {
final String dateString = queryParameters.get("X-Amz-Date");
final String expiresString = queryParameters.get("X-Amz-Expires");
- if (expiresString != null && expiresString.length() > 0) {
- final Long expires = Long.valueOf(expiresString);
-
+ if (dateString == null || expiresString == null ||
+ dateString.length() == 0 || expiresString.length() == 0) {
+ throw new MalformedResourceException(
+ "dateString or expiresString are missing or empty.");
+ }
+ final Long expires = Long.valueOf(expiresString);
+ if (expires >= X_AMZ_EXPIRES_MIN && expires <= X_AMZ_EXPIRES_MAX) {
if (ZonedDateTime.parse(dateString, StringToSignProducer.TIME_FORMATTER)
.plus(expires, SECONDS).isBefore(ZonedDateTime.now())) {
- throw new IllegalArgumentException("Pre-signed S3 url is expired");
+ throw new MalformedResourceException("Pre-signed S3 url is expired. "
+ + "dateString:" + dateString
+ + " expiresString:" + expiresString);
+ }
+ } else {
+ throw new MalformedResourceException("Invalid expiry duration. "
+ + "X-Amz-Expires should be between " + X_AMZ_EXPIRES_MIN
+ + "and" + X_AMZ_EXPIRES_MAX + " expiresString:" + expiresString);
+ }
+ }
+
+ private void validateCredential(Credential credential)
+ throws MalformedResourceException {
+ if (credential.getAccessKeyID().isEmpty()) {
+ throw new MalformedResourceException(
+ "AWS access id shouldn't be empty. credential: " + credential);
+ }
+ if (credential.getAwsRegion().isEmpty()) {
+ throw new MalformedResourceException(
+ "AWS region shouldn't be empty. credential: " + credential);
+ }
+ if (credential.getAwsRequest().isEmpty() ||
+ !(credential.getAwsRequest().equals("aws4_request"))) {
+ throw new MalformedResourceException(
+ "AWS request shouldn't be empty or invalid. credential:"
+ + credential);
+ }
+ if (credential.getAwsService().isEmpty()) {
+ throw new MalformedResourceException(
+ "AWS service shouldn't be empty. credential:" + credential);
+ }
+ // Date should not be empty and should be properly formatted.
+ if (!credential.getDate().isEmpty()) {
+ try {
+ LocalDate.parse(credential.getDate(), DATE_FORMATTER);
+ } catch (DateTimeParseException ex) {
+ throw new MalformedResourceException(
+ "AWS date format is invalid. credential:" + credential);
}
+ } else {
+ throw new MalformedResourceException(
+ "AWS date shouldn't be empty. credential:{}" + credential);
+ }
+ }
+
+ /**
+ * Validate Signed headers.
+ */
+ private void validateSignedHeaders()
+ throws MalformedResourceException {
Review Comment:
indentation
##########
hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/signature/StringToSignProducer.java:
##########
@@ -321,4 +334,34 @@ static void validateSignedHeader(
}
}
+ /** According to AWS Sig V4 documentation
Review Comment:
```suggestion
/**
* According to AWS Sig V4 documentation:
```
##########
hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/signature/TestAuthorizationV4QueryParser.java:
##########
@@ -32,27 +36,85 @@
*/
public class TestAuthorizationV4QueryParser {
- @Test(expected = IllegalArgumentException.class)
- public void testExpiredHeaders() throws Exception {
+ private static final String DATETIME = ZonedDateTime.now().format(
+ StringToSignProducer.TIME_FORMATTER);
Review Comment:
```suggestion
StringToSignProducer.TIME_FORMATTER);
```
##########
hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/signature/StringToSignProducer.java:
##########
@@ -183,19 +185,38 @@ public static String buildCanonicalRequest(
canonicalHeaders.append(NEWLINE);
// Set for testing purpose only to skip date and host validation.
- validateSignedHeader(schema, header, headerValue);
+ try {
+ validateSignedHeader(schema, header, headerValue);
+ } catch (DateTimeParseException ex) {
+ LOG.error("DateTime format invalid.", ex);
+ throw S3_AUTHINFO_CREATION_ERROR;
+ }
} else {
- throw new RuntimeException("Header " + header + " not present in " +
- "request but requested to be signed.");
+ LOG.error("Header " + header + " not present in " +
+ "request but requested to be signed.");
+ throw S3_AUTHINFO_CREATION_ERROR;
}
}
+ validateCanonicalHeaders(canonicalHeaders.toString(), headers,
+ unsignedPayload);
Review Comment:
indentation
##########
hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/signature/StringToSignProducer.java:
##########
@@ -288,22 +309,14 @@ static void validateSignedHeader(
String header,
String headerValue
)
- throws OS3Exception {
+ throws OS3Exception, DateTimeParseException {
switch (header) {
case HOST:
- try {
- URI hostUri = new URI(schema + "://" + headerValue);
- InetAddress.getByName(hostUri.getHost());
- // TODO: Validate if current request is coming from same host.
- } catch (UnknownHostException | URISyntaxException e) {
- LOG.error("Host value mentioned in signed header is not valid. " +
- "Host:{}", headerValue);
- throw S3_AUTHINFO_CREATION_ERROR;
- }
+ // TODO: Placeholder for any host validations.
Review Comment:
What type of host validatation are we going to add here now that the old
logic is removed?
##########
hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/signature/AuthorizationV4QueryParser.java:
##########
@@ -82,17 +104,116 @@ public SignatureInfo parseSignature() throws
MalformedResourceException {
);
}
+ /**
+ * Validate if algorithm is in expected format.
+ */
+ private void validateAlgorithm()
+ throws MalformedResourceException {
+ String algorithm = queryParameters.get("X-Amz-Algorithm");
+ if (algorithm == null) {
+ throw new MalformedResourceException("Missing hash algorithm.");
+ }
+ if (isEmpty(algorithm) || !algorithm.equals(AWS4_SIGNING_ALGORITHM)) {
+ throw new MalformedResourceException("Unexpected hash algorithm. Algo:"
+ + algorithm);
+ }
+ }
+
+ /** According to AWS documentation.
+ * https://docs.aws.amazon.com/AmazonS3/latest/
+ * API/sigv4-query-string-auth.html
+ * X-Amz-Date: The date and time format must follow the
+ * ISO 8601 standard, and must be formatted with the
+ * "yyyyMMddTHHmmssZ" format
+ * X-Amz-Expires: The minimum value you can set is 1,
+ * and the maximum is 604800
+ */
@VisibleForTesting
- protected void validateDateAndExpires() {
+ protected void validateDateAndExpires()
+ throws MalformedResourceException, DateTimeParseException {
final String dateString = queryParameters.get("X-Amz-Date");
final String expiresString = queryParameters.get("X-Amz-Expires");
- if (expiresString != null && expiresString.length() > 0) {
- final Long expires = Long.valueOf(expiresString);
-
+ if (dateString == null || expiresString == null ||
+ dateString.length() == 0 || expiresString.length() == 0) {
+ throw new MalformedResourceException(
+ "dateString or expiresString are missing or empty.");
+ }
+ final Long expires = Long.valueOf(expiresString);
+ if (expires >= X_AMZ_EXPIRES_MIN && expires <= X_AMZ_EXPIRES_MAX) {
if (ZonedDateTime.parse(dateString, StringToSignProducer.TIME_FORMATTER)
.plus(expires, SECONDS).isBefore(ZonedDateTime.now())) {
- throw new IllegalArgumentException("Pre-signed S3 url is expired");
+ throw new MalformedResourceException("Pre-signed S3 url is expired. "
+ + "dateString:" + dateString
+ + " expiresString:" + expiresString);
+ }
+ } else {
+ throw new MalformedResourceException("Invalid expiry duration. "
+ + "X-Amz-Expires should be between " + X_AMZ_EXPIRES_MIN
+ + "and" + X_AMZ_EXPIRES_MAX + " expiresString:" + expiresString);
+ }
+ }
+
+ private void validateCredential(Credential credential)
+ throws MalformedResourceException {
+ if (credential.getAccessKeyID().isEmpty()) {
+ throw new MalformedResourceException(
+ "AWS access id shouldn't be empty. credential: " + credential);
+ }
+ if (credential.getAwsRegion().isEmpty()) {
+ throw new MalformedResourceException(
+ "AWS region shouldn't be empty. credential: " + credential);
+ }
+ if (credential.getAwsRequest().isEmpty() ||
+ !(credential.getAwsRequest().equals("aws4_request"))) {
+ throw new MalformedResourceException(
+ "AWS request shouldn't be empty or invalid. credential:"
+ + credential);
+ }
+ if (credential.getAwsService().isEmpty()) {
+ throw new MalformedResourceException(
+ "AWS service shouldn't be empty. credential:" + credential);
+ }
+ // Date should not be empty and should be properly formatted.
+ if (!credential.getDate().isEmpty()) {
+ try {
+ LocalDate.parse(credential.getDate(), DATE_FORMATTER);
+ } catch (DateTimeParseException ex) {
+ throw new MalformedResourceException(
+ "AWS date format is invalid. credential:" + credential);
}
+ } else {
+ throw new MalformedResourceException(
+ "AWS date shouldn't be empty. credential:{}" + credential);
+ }
+ }
+
+ /**
+ * Validate Signed headers.
+ */
+ private void validateSignedHeaders()
+ throws MalformedResourceException {
+ String signedHeadersStr = queryParameters.get("X-Amz-SignedHeaders");
+ if (signedHeadersStr == null || isEmpty(signedHeadersStr)
+ || signedHeadersStr.split(";").length == 0) {
+ throw new MalformedResourceException("No signed headers found.");
+ }
+ }
+
+ /**
+ * Validate signature.
+ */
+ private void validateSignature()
+ throws MalformedResourceException {
+ String signature = queryParameters.get("X-Amz-Signature");
+ if (isEmpty(signature)) {
+ throw new MalformedResourceException("Signature can't be empty.");
+ }
+ try {
+ Hex.decodeHex(signature);
+ } catch (DecoderException e) {
+ throw new MalformedResourceException(
+ "Signature:" + signature +
+ " should be in hexa-decimal encoding.");
Review Comment:
indentation
##########
hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/TestOzoneClientProducer.java:
##########
@@ -154,18 +164,21 @@ public void testGetSignature() {
fail("Empty AuthHeader must fail");
}
} catch (WebApplicationException ex) {
- if (authHeader == null || authHeader.equals("")) {
- // Empty auth header should be 403
- Assert.assertEquals(HTTP_FORBIDDEN, ex.getResponse().getStatus());
- // TODO: Should return XML in body like this (bot not for now):
- // <Error>
- // <Code>AccessDenied</Code><Message>Access Denied</Message>
- // <RequestId>...</RequestId><HostId>...</HostId>
- // </Error>
+ Assert.assertEquals(HTTP_BAD_REQUEST, ex.getResponse().getStatus());
+ if (authHeader == null || authHeader.equals("") ||
+ authHeader.startsWith("AWS ")) {
+ // Empty auth header and unsupported AWS signature
+ // should fail with Invalid Request.
+ Assert.assertEquals(S3_AUTHINFO_CREATION_ERROR.getErrorMessage(),
+ ex.getMessage());
Review Comment:
```suggestion
ex.getMessage());
```
##########
hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/signature/StringToSignProducer.java:
##########
@@ -321,4 +334,34 @@ static void validateSignedHeader(
}
}
+ /** According to AWS Sig V4 documentation
+ *
https://docs.aws.amazon.com/IAM/latest/UserGuide/create-signed-request.html
+ * The CanonicalHeaders list must include the following:
+ * HTTP host header..
+ * Any x-amz-* headers that you plan to include
+ * in your request must also be added.
+ *
+ * @param canonicalHeaders
+ * @param headers
+ */
+ private static void validateCanonicalHeaders(
+ String canonicalHeaders,
+ Map<String, String> headers,
+ Boolean unsignedPaylod
Review Comment:
indentation
##########
hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/signature/TestAuthorizationV4HeaderParser.java:
##########
@@ -143,6 +143,11 @@ public void testV4HeaderDateValidationFailure() throws
Exception {
String dateStr3 = DATE_FORMATTER.format(now.minus(2, DAYS));
LambdaTestUtils.intercept(MalformedResourceException.class, "",
() -> testRequestWithSpecificDate(dateStr3));
+
+ // Case 4: Invalid date format
+ String dateStr4 = now.toString();
+ LambdaTestUtils.intercept(MalformedResourceException.class, "",
+ () -> testRequestWithSpecificDate(dateStr4));
Review Comment:
```suggestion
() -> testRequestWithSpecificDate(dateStr4));
```
##########
hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/signature/TestAuthorizationV4HeaderParser.java:
##########
@@ -240,6 +245,16 @@ public void testV4HeaderRequestValidationFailure() throws
Exception {
LambdaTestUtils.intercept(MalformedResourceException.class, "",
() -> new AuthorizationV4HeaderParser(auth3, SAMPLE_DATE)
.parseSignature());
+
+ String auth4 =
+ "AWS4-HMAC-SHA256 Credential=ozone/" + curDate + "/us-east-1/s3" +
+ "/invalid_request,"
+ + "SignedHeaders=host;x-amz-content-sha256;x-amz-date,"
+ + "Signature"
+ + "=fe5f80f77d5fa3beca038a248ff027";
+ LambdaTestUtils.intercept(MalformedResourceException.class, "",
+ () -> new AuthorizationV4HeaderParser(auth4, SAMPLE_DATE)
+ .parseSignature());
Review Comment:
```suggestion
String auth4 =
"AWS4-HMAC-SHA256 Credential=ozone/" + curDate + "/us-east-1/s3" +
"/invalid_request,"
+ "SignedHeaders=host;x-amz-content-sha256;x-amz-date,"
+ "Signature"
+ "=fe5f80f77d5fa3beca038a248ff027";
LambdaTestUtils.intercept(MalformedResourceException.class, "",
() -> new AuthorizationV4HeaderParser(auth4, SAMPLE_DATE)
.parseSignature());
```
##########
hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/signature/StringToSignProducer.java:
##########
@@ -321,4 +334,34 @@ static void validateSignedHeader(
}
}
+ /** According to AWS Sig V4 documentation
+ *
https://docs.aws.amazon.com/IAM/latest/UserGuide/create-signed-request.html
+ * The CanonicalHeaders list must include the following:
+ * HTTP host header..
+ * Any x-amz-* headers that you plan to include
+ * in your request must also be added.
+ *
+ * @param canonicalHeaders
+ * @param headers
+ */
+ private static void validateCanonicalHeaders(
+ String canonicalHeaders,
+ Map<String, String> headers,
+ Boolean unsignedPaylod
+ ) throws OS3Exception {
+ if (!canonicalHeaders.contains(HOST + ":")) {
+ LOG.error("The SignedHeaders list must include HTTP Host header");
+ throw S3_AUTHINFO_CREATION_ERROR;
+ }
+ for (String header : headers.keySet().stream()
+ .filter(s -> s.startsWith("x-amz-"))
+ .collect(Collectors.toSet())) {
+ if (!(canonicalHeaders.contains(header + ":"))) {
+ LOG.error("The SignedHeaders list must include all " +
+ "x-amz-* headers in the request");
Review Comment:
indentation
##########
hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/signature/TestAuthorizationV4QueryParser.java:
##########
Review Comment:
need to fix indentation
--
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]
