kuenishi commented on a change in pull request #2999:
URL: https://github.com/apache/ozone/pull/2999#discussion_r800312014
##########
File path:
hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/OzoneClientProducer.java
##########
@@ -90,7 +90,12 @@ public S3Auth getSignature() {
}
String awsAccessId = signatureInfo.getAwsAccessId();
- validateAccessId(awsAccessId);
+ // ONLY validate aws access id when needed.
+ if (awsAccessId == null || awsAccessId.equals("")) {
+ LOG.debug("Malformed s3 header. awsAccessID: ", awsAccessId);
+ throw ACCESS_DENIED;
Review comment:
That's very good question. In this case, either 403 or 400 is not
important, but returning non-500 error is important. I aligned my patch to the
case 1 below which I thought a more major case. But I can set it back to
`MALFORMED_HEADER` again.
Let's think about two cases in S3:
1. There's no 'Authorization:' header in the request from a client (or empty
"Authorization:" line)
2. There's an authorization header line but it's malformed, e.g.
"Authorization: some-bad-string"
AWS S3 handles these cases differently. In case 1, S3 handles the request as
anonymous request, thus it returns with 403 Forbidden.
```
$ curl -i https://s3.amazonaws.com/bucket/key
HTTP/1.1 403 Forbidden
x-amz-request-id: E2VWAJ9YH1JTVMRS
x-amz-id-2:
uge4iAL4wdIbjHeqGZmRzi14YH5iXVXX2uG8jBcp9z5doXujzr5H42M9k0bqgmaKMixHof1/xpw=
Content-Type: application/xml
Transfer-Encoding: chunked
Date: Mon, 07 Feb 2022 04:43:49 GMT
Server: AmazonS3
<?xml version="1.0" encoding="UTF-8"?>
<Error><Code>AccessDenied</Code><Message>Access
Denied</Message><RequestId>E2VWAJ9YH1JTVMRS</RequestId><HostId>uge4iAL4wdIbjHeqGZmRzi14YH5iXVXX2uG8jBcp9z5doXujzr5H42M9k0bqgmaKMixHof1/xpw=</HostId></Error>
$ curl -i -H "Authorization: " https://s3.amazonaws.com/bucket/key
HTTP/1.1 403 Forbidden
x-amz-request-id: TSTM8HEYXCQZW3GH
x-amz-id-2:
hbx5TvaQNHPWgq/IjP9qkFDKgsKtwXNAKrgmSd4HoYfF9OFeETGZLklMhh9XOCpdwDGEV0/BPH0=
Content-Type: application/xml
Transfer-Encoding: chunked
Date: Mon, 07 Feb 2022 04:31:47 GMT
Server: AmazonS3
<?xml version="1.0" encoding="UTF-8"?>
<Error><Code>AccessDenied</Code><Message>Access
Denied</Message><RequestId>TSTM8HEYXCQZW3GH</RequestId><HostId>hbx5TvaQNHPWgq/IjP9qkFDKgsKtwXNAKrgmSd4HoYfF9OFeETGZLklMhh9XOCpdwDGEV0/BPH0=</HostId></Error>
```
In case 2, S3 handles the request as broken one, thus returns with malformed
header.
```
$ curl -i -H "Authorization: some-bad-string"
https://s3.amazonaws.com/bucket/key
HTTP/1.1 400 Bad Request
x-amz-request-id: 8SYS9TNJ1WTE2DTF
x-amz-id-2:
frpGirFa3+etqYSwy5LnvtPLd4k4ODJCylSbKbpLGXKBJ0XUYkjpMXhxCopasq10V4RbmzW5P8M=
Content-Type: application/xml
Transfer-Encoding: chunked
Date: Mon, 07 Feb 2022 04:33:03 GMT
Server: AmazonS3
Connection: close
<?xml version="1.0" encoding="UTF-8"?>
<Error><Code>InvalidArgument</Code><Message>Authorization header is invalid
-- one and only one ' ' (space)
required</Message><ArgumentName>Authorization</ArgumentName><ArgumentValue>some-bad-string</ArgumentValue><RequestId>8SYS9TNJ1WTE2DTF</RequestId><HostId>frpGirFa3+etqYSwy5LnvtPLd4k4ODJCylSbKbpLGXKBJ0XUYkjpMXhxCopasq10V4RbmzW5P8M=</HostId></Error>
```
On the other hand, Ozone's code does not distinguish those cases but just
has `awsAccessId` empty string (AFAIK, but maybe null). Either 400 or 403 is
better than 500 from perspective of error handling for both system admin and
application developers. Administrators do not have to care about app errors
like this, and developers knows their app is wrong by returning 40x here.
IMO aligning errors of Ozone to AWS S3 for compatibility would be important
in some case, but it needs some hustle fixing authorization header parsers.
This fix is a low hanging fruit for us.
--
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]
