You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "SaketaChalamchala (via GitHub)" <gi...@apache.org> on 2023/03/25 02:25:00 UTC
[GitHub] [ozone] SaketaChalamchala opened a new pull request, #4473: HDDS-8058. Gracefully handle invalid S3 request and authorization headers
SaketaChalamchala opened a new pull request, #4473:
URL: https://github.com/apache/ozone/pull/4473
## What changes were proposed in this pull request?
Added multiple checks and validations to OzoneClientProducer.java, AWSV4HeaderParser.java, AWSV4QueryParser.java, StringToSignProducer.java based on AWS V4 documentation. Returned appropriate error codes and logging errors.
Returning `AuthorizationHeaderMalformed` errors when `Authorization` header is invalid and `InvalidRequest` when the S3 request is invalid.
Examples :
```
// AWS V2 signature
sh-4.2$ curl -X PUT -T "${file}" -H "Host: ${host}" -H "Date: ${date}" -H "Content-Type: ${content_type}" -H "Authorization: AWS ${s3_key}:${signature}" -w "%{http_code}" http://${host}:9878${resource}
<?xml version="1.0" encoding="UTF-8"?>
<Error>
<Code>InvalidRequest</Code>
<Message>Error creating s3 auth info. The request may not be signed using AWS V4 signing algorithm, or might be invalid</Message>
<Resource/>
<RequestId/>
</Error>
400
// Invalid algorithm
HTTP_REQUEST_AUTHORIZATION_HEADER_F="AWS4-ZAVC-HJUA123 Credential=om/om@EXAMPLE.COM/20230324/us-east-1/s3/aws4_request, SignedHeaders=content-type;host;x-amz-content-sha256;x-amz-date, Signature=10b148a76ac9ea043041d12e3e2672fb5c10aaadac4dbd7fc487f763493c60ef"
curl "http://${host}:9878${HTTP_CANONICAL_REQUEST_URI}" \
-H "Authorization: ${HTTP_REQUEST_AUTHORIZATION_HEADER_F}" \
-H "content-type: ${HTTP_REQUEST_CONTENT_TYPE}" \
-H "x-amz-content-sha256: ${HTTP_REQUEST_PAYLOAD_HASH}" \
-H "x-amz-date: ${CURRENT_DATE_ISO8601}" \
-T ${IN_FILE}
<?xml version="1.0" encoding="UTF-8"?>
<Error>
<Code>AuthorizationHeaderMalformed</Code>
<Message>The authorization header you provided is invalid.</Message>
<Resource>AWS4-ZAVC-HJUA123 Credential=om/om@EXAMPLE.COM/20230324/us-east-1/s3/aws4_request, SignedHeaders=content-type;host;x-amz-content-sha256;x-amz-date, Signature=10b148a76ac9ea043041d12e3e2672fb5c10aaadac4dbd7fc487f763493c60ef</Resource>
<RequestId/>
</Error>
// invalid date format in credential
HTTP_REQUEST_AUTHORIZATION_HEADER_F="AWS4-HMAC-SHA256 Credential=om/om@EXAMPLE.COM/2023-03-24/us-east-1/s3/aws4_request, SignedHeaders=content-type;host;x-amz-content-sha256;x-amz-date, Signature=10b148a76ac9ea043041d12e3e2672fb5c10aaadac4dbd7fc487f763493c60ef"
curl "http://${host}:9878${HTTP_CANONICAL_REQUEST_URI}" \
-H "Authorization: ${HTTP_REQUEST_AUTHORIZATION_HEADER_F}" \
-H "content-type: ${HTTP_REQUEST_CONTENT_TYPE}" \
-H "x-amz-content-sha256: ${HTTP_REQUEST_PAYLOAD_HASH}" \
-H "x-amz-date: ${CURRENT_DATE_ISO8601}" \
-T ${IN_FILE}
<?xml version="1.0" encoding="UTF-8"?>
<Error>
<Code>AuthorizationHeaderMalformed</Code>
<Message>The authorization header you provided is invalid.</Message>
<Resource>AWS4-HMAC-SHA256 Credential=om/om@EXAMPLE.COM/2023-03-24/us-east-1/s3/aws4_request, SignedHeaders=content-type;host;x-amz-content-sha256;x-amz-date, Signature=10b148a76ac9ea043041d12e3e2672fb5c10aaadac4dbd7fc487f763493c60ef</Resource>
<RequestId/>
</Error>
// signed header missing in request headers
HTTP_REQUEST_AUTHORIZATION_HEADER_F="AWS4-HMAC-SHA256 Credential=om/om@EXAMPLE.COM/20230324/us-east-1/s3/aws4_request, SignedHeaders=content-type;host;x-amz-content-sha256;x-amz-date;x-amz-security-token, Signature=b4304cf0de99b243d895da68733facd4ba9a892c03d353bb2389d66982380823"
curl "http://${host}:9878${HTTP_CANONICAL_REQUEST_URI}" \
-H "Authorization: ${HTTP_REQUEST_AUTHORIZATION_HEADER_F}" \
-H "Content-Length: 123" \
-H "content-type: ${HTTP_REQUEST_CONTENT_TYPE}" \
-H "x-amz-content-sha256: ${HTTP_REQUEST_PAYLOAD_HASH}" \
-H "x-amz-date: ${CURRENT_DATE_ISO8601}" \
-T ${IN_FILE}
<?xml version="1.0" encoding="UTF-8"?>
<Error>
<Code>InvalidRequest</Code>
<Message>Error creating s3 auth info. The request may not be signed using AWS V4 signing algorithm, or might be invalid</Message>
<Resource/>
<RequestId/>
</Error>
// Invalid Signature
HTTP_REQUEST_AUTHORIZATION_HEADER_F="AWS4-HMAC-SHA256 Credential=om/om@EXAMPLE.COM/20230324/us-east-1/s3/aws4_request, SignedHeaders=content-type;host;x-amz-content-sha256;x-amz-date, Signature=10b148a76ac9ea043041d12e3e2672fb5c10aaadac4dbd7fc487f763493c60ea"
curl "http://${host}:9878${HTTP_CANONICAL_REQUEST_URI}" \
-H "Authorization: ${HTTP_REQUEST_AUTHORIZATION_HEADER_F}" \
-H "content-type: ${HTTP_REQUEST_CONTENT_TYPE}" \
-H "x-amz-content-sha256: ${HTTP_REQUEST_PAYLOAD_HASH}" \
-H "x-amz-date: ${CURRENT_DATE_ISO8601}" \
-T ${IN_FILE}
<?xml version="1.0" encoding="UTF-8"?>
<Error>
<Code>AccessDenied</Code>
<Message>User doesn't have the right to access this resource.</Message>
<Resource>1.txt</Resource>
<RequestId>57ade9fc-bcd5-4bc2-80b0-fbb60bf5be16</RequestId>
</Error>
```
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-8058
## How was this patch tested?
Unit tests were added to cover Invalid and Valid S3 requests
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] smengcl commented on pull request #4473: HDDS-8058. Gracefully handle invalid S3 request and authorization headers
Posted by "smengcl (via GitHub)" <gi...@apache.org>.
smengcl commented on PR #4473:
URL: https://github.com/apache/ozone/pull/4473#issuecomment-1535593324
Previous CI is green. Latest commit is a mere comment cleanup. Will merge in a minute.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] kerneltime commented on a diff in pull request #4473: HDDS-8058. Gracefully handle invalid S3 request and authorization headers
Posted by "kerneltime (via GitHub)" <gi...@apache.org>.
kerneltime commented on code in PR #4473:
URL: https://github.com/apache/ozone/pull/4473#discussion_r1184447127
##########
hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/signature/AuthorizationV4HeaderParser.java:
##########
@@ -168,47 +168,50 @@ private Credential parseCredentials(String credential)
if (credentialObj.getAccessKeyID().isEmpty()) {
throw new MalformedResourceException(
- "AWS access id shouldn't be empty. credential: " + credential,
- authHeader);
+ "AWS access id is empty. credential: " + credential, authHeader);
}
if (credentialObj.getAwsRegion().isEmpty()) {
throw new MalformedResourceException(
- "AWS region shouldn't be empty. credential: " + credential,
- authHeader);
+ "AWS region is empty. credential: " + credential, authHeader);
}
- if (credentialObj.getAwsRequest().isEmpty()) {
+ if (credentialObj.getAwsRequest().isEmpty() ||
Review Comment:
We can do it in a separate Jira
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] smengcl commented on pull request #4473: HDDS-8058. Gracefully handle invalid S3 request and authorization headers
Posted by "smengcl (via GitHub)" <gi...@apache.org>.
smengcl commented on PR #4473:
URL: https://github.com/apache/ozone/pull/4473#issuecomment-1508910688
> I also relaxed a host IP validation to allow S3 requests via a load balancer proxy like haproxy.
Thanks @SaketaChalamchala . Pls also state this change in the PR description at the top for future references.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] SaketaChalamchala commented on pull request #4473: HDDS-8058. Gracefully handle invalid S3 request and authorization headers
Posted by "SaketaChalamchala (via GitHub)" <gi...@apache.org>.
SaketaChalamchala commented on PR #4473:
URL: https://github.com/apache/ozone/pull/4473#issuecomment-1532399730
@smengcl Thanks for the review. I addressed your comments and fixed 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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] SaketaChalamchala commented on a diff in pull request #4473: HDDS-8058. Gracefully handle invalid S3 request and authorization headers
Posted by "SaketaChalamchala (via GitHub)" <gi...@apache.org>.
SaketaChalamchala commented on code in PR #4473:
URL: https://github.com/apache/ozone/pull/4473#discussion_r1185551559
##########
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:
Sure
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] smengcl commented on pull request #4473: HDDS-8058. Gracefully handle invalid S3 request and authorization headers
Posted by "smengcl (via GitHub)" <gi...@apache.org>.
smengcl commented on PR #4473:
URL: https://github.com/apache/ozone/pull/4473#issuecomment-1492403509
> Thanks @SaketaChalamchala for the patch. Please check S3 acceptance test failures, e.g.:
>
> [SaketaChalamchala/ozone/actions/runs/4516937717/jobs/7956106418#step:5:444](https://github.com/SaketaChalamchala/ozone/actions/runs/4516937717/jobs/7956106418#step:5:444)
I concur. The test failure in your fork's dev branch CI looks relevant:
```
test_create_bucket (__main__.TestBotoClient) ... ERROR
test_list_bucket (__main__.TestBotoClient) ... ok
test_head_bucket (__main__.TestBotoClient) ... ok
test_bucket_delete (__main__.TestBotoClient) ... ok
test_upload_file (__main__.TestBotoClient) ... ok
test_download_file (__main__.TestBotoClient) ... ok
test_delete_objects (__main__.TestBotoClient) ... ERROR:root:An error occurred (InvalidRequest) when calling the DeleteObjects operation: Error creating s3 auth info. The request may not be signed using AWS V4 signing algorithm, or might be invalid
ok
test_head_object (__main__.TestBotoClient) ... ok
test_multi_uploads (__main__.TestBotoClient) ... ERROR:root:An error occurred (InvalidRequest) when calling the CreateMultipartUpload operation: Error creating s3 auth info. The request may not be signed using AWS V4 signing algorithm, or might be invalid
ok
======================================================================
ERROR: test_create_bucket (__main__.TestBotoClient)
----------------------------------------------------------------------
Traceback (most recent call last):
File "/usr/local/lib/python3.6/site-packages/boto3/s3/transfer.py", line 288, in upload_file
future.result()
File "/usr/local/lib/python3.6/site-packages/s3transfer/futures.py", line 103, in result
return self._coordinator.result()
File "/usr/local/lib/python3.6/site-packages/s3transfer/futures.py", line 266, in result
raise self._exception
File "/usr/local/lib/python3.6/site-packages/s3transfer/tasks.py", line 139, in __call__
return self._execute_main(kwargs)
File "/usr/local/lib/python3.6/site-packages/s3transfer/tasks.py", line 162, in _execute_main
return_value = self._main(**kwargs)
File "/usr/local/lib/python3.6/site-packages/s3transfer/tasks.py", line 349, in _main
Bucket=bucket, Key=key, **extra_args
File "/usr/local/lib/python3.6/site-packages/botocore/client.py", line 401, in _api_call
return self._make_api_call(operation_name, kwargs)
File "/usr/local/lib/python3.6/site-packages/botocore/client.py", line 731, in _make_api_call
raise error_class(parsed_response, operation_name)
botocore.exceptions.ClientError: An error occurred (InvalidRequest) when calling the CreateMultipartUpload operation: Error creating s3 auth info. The request may not be signed using AWS V4 signing algorithm, or might be invalid
During handling of the above exception, another exception occurred:
Traceback (most recent call last):
File "/opt/hadoop/smoketest/s3/boto_client.py", line 78, in setUp
self.s3.Bucket(str(self.target_bucket)).upload_file('./multiUpload.gz','multiUpload.1.gz')
File "/usr/local/lib/python3.6/site-packages/boto3/s3/inject.py", line 239, in bucket_upload_file
Config=Config,
File "/usr/local/lib/python3.6/site-packages/boto3/s3/inject.py", line 148, in upload_file
callback=Callback,
File "/usr/local/lib/python3.6/site-packages/boto3/s3/transfer.py", line 296, in upload_file
filename, '/'.join([bucket, key]), e
boto3.exceptions.S3UploadFailedError: Failed to upload ./multiUpload.gz to erasure/multiUpload.1.gz: An error occurred (InvalidRequest) when calling the CreateMultipartUpload operation: Error creating s3 auth info. The request may not be signed using AWS V4 signing algorithm, or might be invalid
----------------------------------------------------------------------
Ran 9 tests in 2.098s
FAILED (errors=1)
{'ResponseMetadata': {'RequestId': 'f4f5561b-7ea5-4685-b685-bfd3aeb3ff73', 'HostId': '8Dv1t0Cb0cJe', 'HTTPStatusCode': 200, 'HTTPHeaders': {'date': 'Sat, 25 Mar 2023 04:08:28 GMT', 'cache-control': 'no-cache', 'expires': 'Sat, 25 Mar 2023 04:08:28 GMT', 'pragma': 'no-cache', 'content-type': 'text/plain;charset=utf-8', 'x-content-type-options': 'nosniff', 'x-xss-protection': '1; mode=block', 'x-frame-options': 'SAMEORIGIN', 'location': 'http://s3g:9878/ozone-test-xehtenwgjo', 'server': 'Ozone', 'x-amz-id-2': '8Dv1t0Cb0cJe', 'x-amz-request-id': 'f4f5561b-7ea5-4685-b685-bfd3aeb3ff73', 'content-length': '0'}, 'RetryAttempts': 0}, 'Location': 'http://s3g:9878/ozone-test-xehtenwgjo'}
None
None
None
{'ResponseMetadata': {'RequestId': '50c351a9-383c-4b05-8db0-56a4da87a5c4', 'HostId': '0XOKGRqMQSFv', 'HTTPStatusCode': 200, 'HTTPHeaders': {'date': 'Sat, 25 Mar 2023 04:08:29 GMT', 'cache-control': 'no-cache', 'expires': 'Sat, 25 Mar 2023 04:08:29 GMT', 'pragma': 'no-cache', 'content-type': 'application/xml', 'x-content-type-options': 'nosniff', 'x-xss-protection': '1; mode=block', 'x-frame-options': 'SAMEORIGIN', 'server': 'Ozone', 'x-amz-id-2': '0XOKGRqMQSFv', 'x-amz-request-id': '50c351a9-383c-4b05-8db0-56a4da87a5c4', 'content-length': '365'}, 'RetryAttempts': 0}, 'Buckets': [{'Name': 'erasure', 'CreationDate': datetime.datetime(2023, 3, 25, 4, 8, 15, 523000, tzinfo=tzlocal())}, {'Name': 'ozone-test-xehtenwgjo', 'CreationDate': datetime.datetime(2023, 3, 25, 4, 8, 28, 180000, tzinfo=tzlocal())}]}
{'ResponseMetadata': {'RequestId': 'a4761e99-9584-469d-bf37-eaeb7107101a', 'HostId': 'AWnIZl14x', 'HTTPStatusCode': 200, 'HTTPHeaders': {'date': 'Sat, 25 Mar 2023 04:08:29 GMT', 'cache-control': 'no-cache', 'expires': 'Sat, 25 Mar 2023 04:08:29 GMT', 'pragma': 'no-cache', 'content-type': 'application/x-www-form-urlencoded', 'x-content-type-options': 'nosniff', 'x-xss-protection': '1; mode=block', 'x-frame-options': 'SAMEORIGIN', 'server': 'Ozone', 'x-amz-id-2': 'AWnIZl14x', 'x-amz-request-id': 'a4761e99-9584-469d-bf37-eaeb7107101a', 'content-length': '0'}, 'RetryAttempts': 0}}
{'ResponseMetadata': {'RequestId': '37f53a12-86d8-48a2-864c-558d76700e97', 'HostId': '3O6rrx15oiMEgyI', 'HTTPStatusCode': 204, 'HTTPHeaders': {'date': 'Sat, 25 Mar 2023 04:08:29 GMT', 'cache-control': 'no-cache', 'expires': 'Sat, 25 Mar 2023 04:08:29 GMT', 'pragma': 'no-cache', 'content-type': 'text/plain;charset=utf-8', 'x-content-type-options': 'nosniff', 'x-xss-protection': '1; mode=block', 'x-frame-options': 'SAMEORIGIN', 'server': 'Ozone', 'x-amz-id-2': '3O6rrx15oiMEgyI', 'x-amz-request-id': '37f53a12-86d8-48a2-864c-558d76700e97'}, 'RetryAttempts': 0}}
{'ResponseMetadata': {'RequestId': 'ab097016-ba8f-43ed-8d7c-fef650ab3f0f', 'HostId': 'mZjfEMlr', 'HTTPStatusCode': 200, 'HTTPHeaders': {'date': 'Sat, 25 Mar 2023 04:08:29 GMT', 'cache-control': 'no-cache', 'expires': 'Sat, 25 Mar 2023 04:08:29 GMT', 'pragma': 'no-cache', 'content-type': 'binary/octet-stream', 'x-content-type-options': 'nosniff', 'x-xss-protection': '1; mode=block', 'x-frame-options': 'SAMEORIGIN', 'etag': '2023-03-25T04:08:29.601Z', 'last-modified': 'Sat, 25 Mar 2023 04:08:29 GMT', 'server': 'Ozone', 'x-amz-id-2': 'mZjfEMlr', 'x-amz-request-id': 'ab097016-ba8f-43ed-8d7c-fef650ab3f0f', 'content-length': '3879'}, 'RetryAttempts': 0}, 'LastModified': datetime.datetime(2023, 3, 25, 4, 8, 29, tzinfo=tzutc()), 'ContentLength': 3879, 'ETag': '2023-03-25T04:08:29.601Z', 'CacheControl': 'no-cache', 'ContentType': 'binary/octet-stream', 'Expires': datetime.datetime(2023, 3, 25, 4, 8, 29, tzinfo=tzutc()), 'Metadata': {}}
An error occurred (InvalidRequest) when calling the DeleteObjects operation: Error creating s3 auth info. The request may not be signed using AWS V4 signing algorithm, or might be invalid
{'ResponseMetadata': {'RequestId': 'd5b90ab8-62a9-44b0-9645-1c30718e826f', 'HostId': 'Vdm5bMZEt6h', 'HTTPStatusCode': 200, 'HTTPHeaders': {'date': 'Sat, 25 Mar 2023 04:08:29 GMT', 'cache-control': 'no-cache', 'expires': 'Sat, 25 Mar 2023 04:08:29 GMT', 'pragma': 'no-cache', 'content-type': 'binary/octet-stream', 'x-content-type-options': 'nosniff', 'x-xss-protection': '1; mode=block', 'x-frame-options': 'SAMEORIGIN', 'etag': '2023-03-25T04:08:28.956Z', 'last-modified': 'Sat, 25 Mar 2023 04:08:28 GMT', 'server': 'Ozone', 'x-amz-id-2': 'Vdm5bMZEt6h', 'x-amz-request-id': 'd5b90ab8-62a9-44b0-9645-1c30718e826f', 'content-length': '3879'}, 'RetryAttempts': 0}, 'LastModified': datetime.datetime(2023, 3, 25, 4, 8, 28, tzinfo=tzutc()), 'ContentLength': 3879, 'ETag': '2023-03-25T04:08:28.956Z', 'CacheControl': 'no-cache', 'ContentType': 'binary/octet-stream', 'Expires': datetime.datetime(2023, 3, 25, 4, 8, 29, tzinfo=tzutc()), 'Metadata': {}}
An error occurred (InvalidRequest) when calling the CreateMultipartUpload operation: Error creating s3 auth info. The request may not be signed using AWS V4 signing algorithm, or might be invalid
Boto3 Client Test FAILED!
```
@SaketaChalamchala Would you take a look?
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] kerneltime commented on pull request #4473: HDDS-8058. Gracefully handle invalid S3 request and authorization headers
Posted by "kerneltime (via GitHub)" <gi...@apache.org>.
kerneltime commented on PR #4473:
URL: https://github.com/apache/ozone/pull/4473#issuecomment-1502030266
cc @tanvipenumudy can you please take a look?
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] smengcl commented on a diff in pull request #4473: HDDS-8058. Gracefully handle invalid S3 request and authorization headers
Posted by "smengcl (via GitHub)" <gi...@apache.org>.
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/scm@EXAMPLE.COM: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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] smengcl commented on a diff in pull request #4473: HDDS-8058. Gracefully handle invalid S3 request and authorization headers
Posted by "smengcl (via GitHub)" <gi...@apache.org>.
smengcl commented on code in PR #4473:
URL: https://github.com/apache/ozone/pull/4473#discussion_r1184515424
##########
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:
Agreed. Shall we remove this TODO then? @SaketaChalamchala
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] SaketaChalamchala commented on a diff in pull request #4473: HDDS-8058. Gracefully handle invalid S3 request and authorization headers
Posted by "SaketaChalamchala (via GitHub)" <gi...@apache.org>.
SaketaChalamchala commented on code in PR #4473:
URL: https://github.com/apache/ozone/pull/4473#discussion_r1179427789
##########
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/scm@EXAMPLE.COM:asdfqwerty' ${ENDPOINT_URL}/${BUCKET}/${PREFIX}/putobject/key=value/f1
- Should contain ${result} 403 Forbidden
+ Should contain ${result} 400 Bad Request
Review Comment:
@smengcl Thanks for the review.
The request here is sending an AWS v2 which we do not support and that's why I was returning a "400 - Bad request". If a "403 - Forbidden" return code makes sense here then I can make that change.
However, for some of the other requests with invalid/incorrect AWS V4 auth headers/requests, I'm returning a "400 - Malformed auth header" exception . Will modify the error codes and error messages according to the AWS documentation.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] SaketaChalamchala commented on a diff in pull request #4473: HDDS-8058. Gracefully handle invalid S3 request and authorization headers
Posted by "SaketaChalamchala (via GitHub)" <gi...@apache.org>.
SaketaChalamchala commented on code in PR #4473:
URL: https://github.com/apache/ozone/pull/4473#discussion_r1184089883
##########
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/scm@EXAMPLE.COM:asdfqwerty' ${ENDPOINT_URL}/${BUCKET}/${PREFIX}/putobject/key=value/f1
- Should contain ${result} 403 Forbidden
+ Should contain ${result} 400 Bad Request
Review Comment:
Kept the error message but updated the error code to 403 since that's what is expected from AWS documentation
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] SaketaChalamchala commented on pull request #4473: HDDS-8058. Gracefully handle invalid S3 request and authorization headers
Posted by "SaketaChalamchala (via GitHub)" <gi...@apache.org>.
SaketaChalamchala commented on PR #4473:
URL: https://github.com/apache/ozone/pull/4473#issuecomment-1492768111
Thanks @adoroszlai and @smengcl for the review. I added more validations and updated the expected error codes in robot tests that was causing acceptance failures.
I also relaxed a host IP validation to allow S3 requests via a load balancer proxy like haproxy.
cc @kerneltime
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] adoroszlai commented on pull request #4473: HDDS-8058. Gracefully handle invalid S3 request and authorization headers
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on PR #4473:
URL: https://github.com/apache/ozone/pull/4473#issuecomment-1483800633
Thanks @SaketaChalamchala for the patch. Please check S3 acceptance test failures, e.g.:
https://github.com/SaketaChalamchala/ozone/actions/runs/4516937717/jobs/7956106418#step:5:444
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] kerneltime commented on a diff in pull request #4473: HDDS-8058. Gracefully handle invalid S3 request and authorization headers
Posted by "kerneltime (via GitHub)" <gi...@apache.org>.
kerneltime commented on code in PR #4473:
URL: https://github.com/apache/ozone/pull/4473#discussion_r1184386505
##########
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:
I don't think we need a host validation per se
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] SaketaChalamchala commented on pull request #4473: HDDS-8058. Gracefully handle invalid S3 request and authorization headers
Posted by "SaketaChalamchala (via GitHub)" <gi...@apache.org>.
SaketaChalamchala commented on PR #4473:
URL: https://github.com/apache/ozone/pull/4473#issuecomment-1535562845
> Changes looks good to me. The only unresolved comment left is regarding TODO:
>
> [#4473 (comment)](https://github.com/apache/ozone/pull/4473#discussion_r1184515424)
@smengcl Done.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] smengcl commented on pull request #4473: HDDS-8058. Gracefully handle invalid S3 request and authorization headers
Posted by "smengcl (via GitHub)" <gi...@apache.org>.
smengcl commented on PR #4473:
URL: https://github.com/apache/ozone/pull/4473#issuecomment-1535593511
Thanks @SaketaChalamchala for the patch. Thanks @kerneltime for reviewing this.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] smengcl commented on a diff in pull request #4473: HDDS-8058. Gracefully handle invalid S3 request and authorization headers
Posted by "smengcl (via GitHub)" <gi...@apache.org>.
smengcl commented on code in PR #4473:
URL: https://github.com/apache/ozone/pull/4473#discussion_r1179461015
##########
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/scm@EXAMPLE.COM:asdfqwerty' ${ENDPOINT_URL}/${BUCKET}/${PREFIX}/putobject/key=value/f1
- Should contain ${result} 403 Forbidden
+ Should contain ${result} 400 Bad Request
Review Comment:
Thanks Saketa. `400 - Malformed auth header` sounds good. In this case the user will be informed of the correct action to take (to fix auth header).
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] SaketaChalamchala commented on a diff in pull request #4473: HDDS-8058. Gracefully handle invalid S3 request and authorization headers
Posted by "SaketaChalamchala (via GitHub)" <gi...@apache.org>.
SaketaChalamchala commented on code in PR #4473:
URL: https://github.com/apache/ozone/pull/4473#discussion_r1184090136
##########
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:
Made "aws4_request" a constant
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] kerneltime commented on a diff in pull request #4473: HDDS-8058. Gracefully handle invalid S3 request and authorization headers
Posted by "kerneltime (via GitHub)" <gi...@apache.org>.
kerneltime commented on code in PR #4473:
URL: https://github.com/apache/ozone/pull/4473#discussion_r1184447033
##########
hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/signature/AuthorizationV4HeaderParser.java:
##########
@@ -168,47 +168,50 @@ private Credential parseCredentials(String credential)
if (credentialObj.getAccessKeyID().isEmpty()) {
throw new MalformedResourceException(
- "AWS access id shouldn't be empty. credential: " + credential,
- authHeader);
+ "AWS access id is empty. credential: " + credential, authHeader);
}
if (credentialObj.getAwsRegion().isEmpty()) {
throw new MalformedResourceException(
- "AWS region shouldn't be empty. credential: " + credential,
- authHeader);
+ "AWS region is empty. credential: " + credential, authHeader);
}
- if (credentialObj.getAwsRequest().isEmpty()) {
+ if (credentialObj.getAwsRequest().isEmpty() ||
Review Comment:
I was going over the string parsing logic for credentialObj and we can definitely do a lot better in Credential::parcelCredential() method and add tests. This can be done in a separate jira. I do not think the credential parsing logic is defensive enough. Also here, we can first check if `credentialObj.getAwsRequest() != null`
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] smengcl merged pull request #4473: HDDS-8058. Gracefully handle invalid S3 request and authorization headers
Posted by "smengcl (via GitHub)" <gi...@apache.org>.
smengcl merged PR #4473:
URL: https://github.com/apache/ozone/pull/4473
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org