You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by GitBox <gi...@apache.org> on 2022/01/19 10:09:31 UTC
[GitHub] [ozone] kuenishi opened a new pull request #2999: HDDS-6204. Cleanup handling malformed authorization header
kuenishi opened a new pull request #2999:
URL: https://github.com/apache/ozone/pull/2999
## What changes were proposed in this pull request?
Currently in some cases where the authorization header is wrong, S3Gateway returns wrong status code. For example,
1. 500 for no such bucket, or
2. 404 for malformed request.
Some cases are not reproducible for me, but it's apparently wrong in code. 1. is because OS3Exception is doubly wrapped by WebApplicationError and thus failing to catch in the `catch (OS3Exception ex)` clause and goes through to `catch (Exception ex)` clause. 2. just comes from wrong definition in S3ErrorTable.
Here are some examples against our production cluster for case 1:
```sh
$ curl -H "Authorization: invalid" https://ozone.example.com/no-such-bucket
<html>
<head>
<meta http-equiv="Content-Type" content="text/html;charset=utf-8"/>
<title>Error 500 Internal Server Error</title>
</head>
<body><h2>HTTP ERROR 500 Internal Server Error</h2>
<table>
<tr><th>URI:</th><td>/no-such-bucket</td></tr>
<tr><th>STATUS:</th><td>500</td></tr>
<tr><th>MESSAGE:</th><td>Internal Server Error</td></tr>
<tr><th>SERVLET:</th><td>jaxrs</td></tr>
</table>
</body>
</html>
$ curl -H "Authorization: " https://ozoene.example.com/no-such-bucket
<html>
<head>
<meta http-equiv="Content-Type" content="text/html;charset=utf-8"/>
<title>Error 500 Internal Server Error</title>
</head>
<body><h2>HTTP ERROR 500 Internal Server Error</h2>
<table>
<tr><th>URI:</th><td>/no-such-bucket</td></tr>
<tr><th>STATUS:</th><td>500</td></tr>
<tr><th>MESSAGE:</th><td>Internal Server Error</td></tr>
<tr><th>SERVLET:</th><td>jaxrs</td></tr>
</table>
</body>
</html>
[kuenishi@nausicaa ozone-master]$ curl https://ozone.example.com/no-such-bucket
<html>
<head>
<meta http-equiv="Content-Type" content="text/html;charset=utf-8"/>
<title>Error 500 Internal Server Error</title>
</head>
<body><h2>HTTP ERROR 500 Internal Server Error</h2>
<table>
<tr><th>URI:</th><td>/no-such-bucket</td></tr>
<tr><th>STATUS:</th><td>500</td></tr>
<tr><th>MESSAGE:</th><td>Internal Server Error</td></tr>
<tr><th>SERVLET:</th><td>jaxrs</td></tr>
</table>
</body>
</html>
```
## What is the link to the Apache JIRA
HDDS-6204
## How was this patch tested?
- Unit test added
- Manual tests with IntelliJ
- Production environment
--
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 a change in pull request #2999: HDDS-6204. Cleanup handling malformed authorization header
Posted by GitBox <gi...@apache.org>.
adoroszlai commented on a change in pull request #2999:
URL: https://github.com/apache/ozone/pull/2999#discussion_r800092001
##########
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:
Shouldn't we throw `MALFORMED_HEADER` in this case? If I understand correctly, the problem was with the extra `wrapOS3Exception`, not the specific error code.
--
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] kuenishi commented on a change in pull request #2999: HDDS-6204. Cleanup handling malformed authorization header
Posted by GitBox <gi...@apache.org>.
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: " or "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: 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] kuenishi commented on a change in pull request #2999: HDDS-6204. Cleanup handling malformed authorization header
Posted by GitBox <gi...@apache.org>.
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: 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 merged pull request #2999: HDDS-6204. Cleanup handling malformed authorization header
Posted by GitBox <gi...@apache.org>.
adoroszlai merged pull request #2999:
URL: https://github.com/apache/ozone/pull/2999
--
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 a change in pull request #2999: HDDS-6204. Cleanup handling malformed authorization header
Posted by GitBox <gi...@apache.org>.
adoroszlai commented on a change in pull request #2999:
URL: https://github.com/apache/ozone/pull/2999#discussion_r800604760
##########
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:
Thanks @kuenishi for the examples. So for null or empty access ID `ACCESS_DENIED` matches Amazon's response.
--
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