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/08/29 08:06:25 UTC
[GitHub] [ozone] symious opened a new pull request, #3726: HDDS-7185. Encode asterisk for parsing signature
symious opened a new pull request, #3726:
URL: https://github.com/apache/ozone/pull/3726
## What changes were proposed in this pull request?
The asterisk not encoded when building CanonicalRequest, which will cause the error when validating signatures.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-7185
## How was this patch tested?
Test using the following commands:
aws s3api --endpoint http://s3g:9878 list-objects --bucket bucket --prefix "dir1/*"
--
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] neils-dev commented on a diff in pull request #3726: HDDS-7185. Encode asterisk for parsing signature
Posted by GitBox <gi...@apache.org>.
neils-dev commented on code in PR #3726:
URL: https://github.com/apache/ozone/pull/3726#discussion_r957876606
##########
hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/signature/StringToSignProducer.java:
##########
@@ -249,6 +249,7 @@ private static String urlEncode(String str) {
try {
return S3Utils.urlEncode(str)
.replaceAll("\\+", "%20")
+ .replaceAll("\\*", "%2A")
.replaceAll("%7E", "~");
} catch (UnsupportedEncodingException e) {
Review Comment:
Also, is there an alternative encoder that removes the need for correcting a few characters after encoding?
--
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] neils-dev commented on a diff in pull request #3726: HDDS-7185. Encode asterisk for parsing signature
Posted by GitBox <gi...@apache.org>.
neils-dev commented on code in PR #3726:
URL: https://github.com/apache/ozone/pull/3726#discussion_r957872302
##########
hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/signature/StringToSignProducer.java:
##########
@@ -249,6 +249,7 @@ private static String urlEncode(String str) {
try {
return S3Utils.urlEncode(str)
.replaceAll("\\+", "%20")
+ .replaceAll("\\*", "%2A")
.replaceAll("%7E", "~");
} catch (UnsupportedEncodingException e) {
Review Comment:
Thanks @symious for catching this problem and submitting this fix. As the uri is included in the string2sign and signature validation do we need to extend this encoding to support other special characters that need to be in ASCII hex or vice-versa?
For the URLEncoder class, all alphanumeric characters remain the same after encoding. However the spaces are converted to '+' that we correct for in line 251. All special characters ".", "-", "\*" , and remain the"_" same. With all others converted to hex. Are there any other cases of special characters that we need to convert to hex similar to the '*"?
--
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] neils-dev commented on a diff in pull request #3726: HDDS-7185. Encode asterisk for parsing signature
Posted by GitBox <gi...@apache.org>.
neils-dev commented on code in PR #3726:
URL: https://github.com/apache/ozone/pull/3726#discussion_r957872302
##########
hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/signature/StringToSignProducer.java:
##########
@@ -249,6 +249,7 @@ private static String urlEncode(String str) {
try {
return S3Utils.urlEncode(str)
.replaceAll("\\+", "%20")
+ .replaceAll("\\*", "%2A")
.replaceAll("%7E", "~");
} catch (UnsupportedEncodingException e) {
Review Comment:
Thanks @symious for catching is problem and submitting this fix. As the uri is included in the string2sign and signature validation do we need to extend this encoding to support other special characters that need to be in ASCII hex or vice-versa?
For the URLEncoder class, all alphanumeric characters remain the same after encoding. However the spaces are converted to '+' that we correct for in line 251. All special characters ".", "-", "\*" , and remain the"_" same. With all others converted to hex. Are there any other cases o f special characters that we need to convert to hex similar to the '*"?
--
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 #3726: HDDS-7185. Encode asterisk for parsing signature
Posted by GitBox <gi...@apache.org>.
kerneltime commented on PR #3726:
URL: https://github.com/apache/ozone/pull/3726#issuecomment-1230519610
@duongnguyen0
--
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] neils-dev commented on a diff in pull request #3726: HDDS-7185. Encode asterisk for parsing signature
Posted by GitBox <gi...@apache.org>.
neils-dev commented on code in PR #3726:
URL: https://github.com/apache/ozone/pull/3726#discussion_r957872302
##########
hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/signature/StringToSignProducer.java:
##########
@@ -249,6 +249,7 @@ private static String urlEncode(String str) {
try {
return S3Utils.urlEncode(str)
.replaceAll("\\+", "%20")
+ .replaceAll("\\*", "%2A")
.replaceAll("%7E", "~");
} catch (UnsupportedEncodingException e) {
Review Comment:
Thanks @symious for catching is problem and submitting this fix. As the uri is included in the string2sign and signature validation do we need to extend this encoding to support other special characters that need to be in ASCII hex or vice-versa?
For the URLEncoder class, all alphanumeric characters remain the same after encoding. However the spaces are converted to '+' that we correct for in line 251. All special characters ".", "-", "*", and remain the"_" same. With all others converted to hex. Are there any other cases of special characters that we need to convert to hex similar to the '*"?
--
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] neils-dev commented on a diff in pull request #3726: HDDS-7185. Encode asterisk for parsing signature
Posted by GitBox <gi...@apache.org>.
neils-dev commented on code in PR #3726:
URL: https://github.com/apache/ozone/pull/3726#discussion_r958980257
##########
hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/signature/StringToSignProducer.java:
##########
@@ -249,6 +249,7 @@ private static String urlEncode(String str) {
try {
return S3Utils.urlEncode(str)
.replaceAll("\\+", "%20")
+ .replaceAll("\\*", "%2A")
.replaceAll("%7E", "~");
} catch (UnsupportedEncodingException e) {
Review Comment:
@symious thanks. I checked with aws `botocore` for encoding the URI for the signature. It looks like the `StringToSignProducer` encoder is equivalent now to aws `botocore`. Which should be fine. It is a python implementation that uses `urllib`.`quote`.
https://github.com/boto/botocore/blob/f2b0dbb800b8dc2a3541334d5ca1190faf900150/botocore/auth.py#L362
Within quote its safe characters are : Letters, digits, and the characters '_.-~' as well as user added '/\~' .
https://docs.python.org/3/library/urllib.parse.html
If we are compatible with this then we should not have any unexpected problems.
With the java class `java.net.URLEncoder,` the safe characters include in addition to the urllib.parse.quote safe characters just "*". Please check this as well.
https://docs.oracle.com/javase/1.5.0/docs/api/java/net/URLEncoder.html
--
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] symious commented on a diff in pull request #3726: HDDS-7185. Encode asterisk for parsing signature
Posted by GitBox <gi...@apache.org>.
symious commented on code in PR #3726:
URL: https://github.com/apache/ozone/pull/3726#discussion_r957882037
##########
hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/signature/StringToSignProducer.java:
##########
@@ -249,6 +249,7 @@ private static String urlEncode(String str) {
try {
return S3Utils.urlEncode(str)
.replaceAll("\\+", "%20")
+ .replaceAll("\\*", "%2A")
.replaceAll("%7E", "~");
} catch (UnsupportedEncodingException e) {
Review Comment:
There are some SAFE Characters mentioned here: https://docs.aws.amazon.com/AmazonS3/latest/userguide/object-keys.html.
Maybe we need also to check the availabilities for them?
--
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] neils-dev merged pull request #3726: HDDS-7185. Encode asterisk for parsing signature
Posted by GitBox <gi...@apache.org>.
neils-dev merged PR #3726:
URL: https://github.com/apache/ozone/pull/3726
--
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] symious commented on pull request #3726: HDDS-7185. Encode asterisk for parsing signature
Posted by GitBox <gi...@apache.org>.
symious commented on PR #3726:
URL: https://github.com/apache/ozone/pull/3726#issuecomment-1253283245
@adoroszlai Thank you for the notice.
--
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 #3726: HDDS-7185. Encode asterisk for parsing signature
Posted by GitBox <gi...@apache.org>.
adoroszlai commented on PR #3726:
URL: https://github.com/apache/ozone/pull/3726#issuecomment-1253281153
Thanks @symious for the patch, @duongkame, @neils-dev for the review.
@neils-dev Github adds Co-Author in the commit message if the PR has commits from multiple email addresses. If commits are really only from one person, please feel free to remove the Co-Author part when merging. Also, please don't forget to resolve the Jira issue as well.
--
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] neils-dev commented on a diff in pull request #3726: HDDS-7185. Encode asterisk for parsing signature
Posted by GitBox <gi...@apache.org>.
neils-dev commented on code in PR #3726:
URL: https://github.com/apache/ozone/pull/3726#discussion_r958980787
##########
hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/signature/StringToSignProducer.java:
##########
@@ -249,6 +249,7 @@ private static String urlEncode(String str) {
try {
return S3Utils.urlEncode(str)
.replaceAll("\\+", "%20")
+ .replaceAll("\\*", "%2A")
.replaceAll("%7E", "~");
} catch (UnsupportedEncodingException e) {
Review Comment:
LGTM
--
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] symious commented on a diff in pull request #3726: HDDS-7185. Encode asterisk for parsing signature
Posted by GitBox <gi...@apache.org>.
symious commented on code in PR #3726:
URL: https://github.com/apache/ozone/pull/3726#discussion_r959069041
##########
hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/signature/StringToSignProducer.java:
##########
@@ -249,6 +249,7 @@ private static String urlEncode(String str) {
try {
return S3Utils.urlEncode(str)
.replaceAll("\\+", "%20")
+ .replaceAll("\\*", "%2A")
.replaceAll("%7E", "~");
} catch (UnsupportedEncodingException e) {
Review Comment:
@neils-dev Thanks for the check. Agreed that we only need to add an aditional check for "*".
FYI: I also find the safe characters here based on your information. https://github.com/boto/botocore/blob/f2b0dbb800b8dc2a3541334d5ca1190faf900150/botocore/auth.py#L251
--
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