HDDS-7035. Generate strToSign before applying virtual host style filter.

[SaketaChalamchala]

What changes were proposed in this pull request?

When using Virtual-Host Style addressing using AWS CLI the client uses / as the canonical URI when generating the S3 request. At the server side, VirtualHostStyleFilter.java extracts bucket name from the host value and appends it to the URI. This causes the signature validation to fail because the canonical URI used to generating string to sign is /bucketname. But, this is also necessary to resource match the request to the correct endpoint.

Proposed change introduces an

• AuthorizationFilter.java and moves the string to sign generation function from OzoneClientProducer.java to AuthorizationFilter.java. The filter runs before the VirtualHostStyleFilter.java where the URI is changed.
• Repurposes SignatureInfo object to store String to Sign information.
• Fixed a bug in VirtualHostStyleFilter.java: URIs with keynames were generated as /bucketnamekeyname instead of /bucketname/keyname
• Minor changes: Used a standard approach to assign prioritues to filters.

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/HDDS-7035

How was this patch tested?

• Unit Tests
• Acceptance Tests

[errose28]

cc @tanvipenumudy for review

[tanvipenumudy]

Thanks @SaketaChalamchala for the patch!

Minor NIT: Would it be helpful to have a test where virtual-host style addressing is not used to verify if the canonical URI remains unchanged?

[SaketaChalamchala]

Thanks @SaketaChalamchala for the patch!

Minor NIT: Would it be helpful to have a test where virtual-host style addressing is not used to verify if the canonical URI remains unchanged?

Thanks for the review @tanvipenumudy, there are existing tests in TestStringToSignProducer.java#test and TestVirtualHostStyleFilter.java#testPathStyle that cover path style addressing.

[errose28]

Thanks for working on this @SaketaChalamchala. I'm not very familiar with this area of the code or the AWS virtual/path style specs, but I did a pass cross-referencing with the AWS documentation.

[errose28]

Can we rename the other test in this class to testPathStyleAddressURI to clarify the difference?

[errose28]

Where did these values come from? I can't find them documented by AWS anywhere, for example at https://docs.aws.amazon.com/AmazonS3/latest/userguide/VirtualHosting.html

[SaketaChalamchala]

I added these internally to identify when virtual host style addressing is used.

[errose28]

According to examples in s3 docs, no extra query parameters are required. The tests should run the same way.

[SaketaChalamchala]

Thanks for the review @errose28 I added the extra parameters are added internally in VirtualHostStyleFilter.java so that when we do signature validation late we know to use the original request URI sent by the client and not the URI with bucket name that is appended in VirtualHostStyleFilter.java.

[errose28]

Also, why did our existing secure S3 acceptance tests not catch this? Since path style is deprecated, I would assume the s3 client we are using in those tests is calling s3 gateway with virtual style. Can you provide an acceptance test that passes only after the fix is applied?

[kerneltime]

Also, why did our existing secure S3 acceptance tests not catch this? Since path style is deprecated, I would assume the s3 client we are using in those tests is calling s3 gateway with virtual style. Can you provide an acceptance test that passes only after the fix is applied?

Even though path style is deprecated, most on prem deployments still use it. It is rare to find clients using virtual host style bucket addressing due to the complexity of bucket creation and DNS updates needed.

[SaketaChalamchala]

@errose28 & @kerneltime,
I modified the patch to include an AuthorizationFilter before all other filters and added an acceptance test.
Updated the PR description accordingly.

[SaketaChalamchala]

Looks like there are some acceptance test failures. Working on them.