Community
Participate
Working Groups
Presently, the AmazonS3 class is using AWS Signature Version 2. The latest version is AWS Signature Version 4. As this page shows, Version 2 is no longer supported in multiple AWS regions: https://docs.aws.amazon.com/general/latest/gr/s3.html When AmazonS3 tries to send requests to a region that only supports Version 4, JGit operations fail with the following 400 Bad Request error: "InvalidRequest: The authorization mechanism you have provided is not supported. Please use AWS4-HMAC-SHA256." In order for the AmazonS3 client to work with all AWS regions, it needs to be updated to use AWS Signature Version 4. The following links provide more information about AWS Signature Version 4 and how to upgrade to it: * https://docs.aws.amazon.com/general/latest/gr/signature-version-4.html * https://docs.aws.amazon.com/general/latest/gr/signature-v4-examples.html * https://docs.aws.amazon.com/general/latest/gr/sigv4_changes.html
Per this page, "Any new buckets created after June 24, 2020 will not support SigV2 signed requests, although existing buckets will continue to support SigV2 while we work with customers to move off this older request signing method." https://aws.amazon.com/blogs/aws/amazon-s3-update-sigv2-deprecation-period-extended-modified/
I'm not familiar with that code in JGit. But basically it doesn't seem _too_ complicated; one would just have to re-implement all the stuff they show at [1]. At least for GET. For PUT, you probably have to experiment a little; I don't see an example for that. Perhaps it would make sense to pull this Amazon S3 transport into a separate bundle and use the Amazon S3 SDK [2]? [1] https://docs.aws.amazon.com/general/latest/gr/sigv4-signed-request-examples.html [2] https://mvnrepository.com/artifact/com.amazonaws/aws-java-sdk-s3
In this page I found a ZIP file with example code for GET and PUT: https://docs.aws.amazon.com/AmazonS3/latest/API/sig-v4-examples-using-sdks.html https://docs.aws.amazon.com/AmazonS3/latest/API/samples/AWSS3SigV4JavaSamples.zip
I originally labeled this as an enhancement, but given that AmazonS3 transport doesn't work with all AWS regions, I think it should actually be considered a bug. Also, I believe that AWS will eventually deprecate V2 APIs altogether, at which point AmazonS3 transport will not work at all.
(In reply to Eric Steele from comment #3) > In this page I found a ZIP file with example code for GET and PUT: > > https://docs.aws.amazon.com/AmazonS3/latest/API/sig-v4-examples-using-sdks. > html > > https://docs.aws.amazon.com/AmazonS3/latest/API/samples/ > AWSS3SigV4JavaSamples.zip These examples look good. No external dependencies. Utilities included can be replaced by stuff already in JGit. So JGit's AmazonS3 transport could be rewritten along these lines. And if JGit didn't use the JDK HttpUrlConnection but went through the HttpConnectionFactory, that would be even better. BTW, I notice that JGit constructs http URLS, not https URLs. Looks like quite a bit of work, but not too difficult. Should be done by somebody who actually uses the AmazonS3 transport, though. I don't.
One thing to note is that AWS Signature Version 4 requires that we specify the AWS region that the request is for. To support this, I believe we will need to add a new 'region' property to the JGit config file to complement the 'domain' property.
Hi there, So I cloned JGit (5.13.0) and tried my hand at integrating the code from the AWS sample. I believe I have everything working. It doesn't look like there is a lot of code coverage for this, which I guess makes sense because it requires an S3 bucket to test with. Via manual testing, I confirmed that I have the following working: * clone * push * pull I was able to confirm via debugging that my manual tests are exercising the code paths for all of AmazonS3's public methods: * decrypt * get * list * put * beginPut All of my work was done based off of 5.13.0 because our project uses Java 8 and it seems that the latest version of JGit no longer works with Java 8. I'm 100% new to contributing. Thomas Wolf can you suggest next steps?
Forgot to mention that I also confirmed delete() functionality.
You can contribute this to 5.13 on the stable-5.13 branch. We can then merge it up to stable-6.x and master later. Find the contributor guide here https://wiki.eclipse.org/EGit/Contributor_Guide#Contributing_Patches. Short summary: - register an Eclipse account, sign Eclipse Contribution Agreement - login once to https://git.eclipse.org/r using that Eclipse account - upload your public ssh key to https://git.eclipse.org/r/settings/#SSHKeys or generate an http password here to push via ssh/https - base your patch on the stable-5.13 branch - push your patch to refs/for/stable-5.13 to create a review for the stable-5.13 branch: $ push origin HEAD:refs/for/stable-5.13 If you need more help getting started let me know. AFAIR tests for this class are in WalkEncryptionTest which require some S3 bucket for testing and some additional configuration, hence we don't run this from our CI builds. See https://git.eclipse.org/r/c/jgit/jgit/+/56391
here is the link for generating HTTP password: https://git.eclipse.org/r/settings/#HTTPCredentials
Thank you for all of the info! Regarding my previous comment: "One thing to note is that AWS Signature Version 4 requires that we specify the AWS region that the request is for. To support this, I believe we will need to add a new 'region' property to the JGit config file to complement the 'domain' property." Presently, my code removes all of the V2 signature logic, adds the V4 logic, and requires the new 'region' property. I think that this should be considered a breaking/major change. I don't think it makes sense to include a default 'region', since the region should always match the region that the 'domain' property resolves to. To maintain compatibility for existing users, I think we could: 1. Check if the 'region' property is specified. 2. If region is found, use the V4 signature logic. 3. If region is not found, use the V2 signature logic. Alternatively, there could be a property to tell JGit which version to use. It would default to V2. If V4 is specified, the 'region' property would be required. Thoughts?
If no complaints, I'll go with the version property.
WalkEncryptionTest passed with my changes
New Gerrit change created: https://git.eclipse.org/r/c/jgit/jgit/+/193599
Hi folks. I put up my change for review. Let me know if I'm missing anything. :)
New Gerrit change created: https://git.eclipse.org/r/c/jgit/jgit/+/193651
New Gerrit change created: https://git.eclipse.org/r/c/jgit/jgit/+/193652
New Gerrit change created: https://git.eclipse.org/r/c/jgit/jgit/+/193703
New Gerrit change created: https://git.eclipse.org/r/c/jgit/jgit/+/193705
Hey Thomas... I think I'm a bit confused by how Gerrit does things... I followed the contributor guide and submitted some changes, but saw that it looked like my original PR was no longer being shown/active. I thought I had done something wrong and so I marked those changes as Abandoned in Gerrit. I consolidated my changes in a single change and pushed it (one commit), but now Gerrit is showing multiple reviews. I am very confused :) Hopefully we can recover from this situation. My most recent push is here: https://git.eclipse.org/r/c/jgit/jgit/+/193705/1
New Gerrit change created: https://git.eclipse.org/r/c/jgit/jgit/+/193710
New Gerrit change created: https://git.eclipse.org/r/c/jgit/jgit/+/193712
Ran into some merge conflict issue. Tried to rebase but did it wrong, so I just restarted from scratch: https://git.eclipse.org/r/c/jgit/jgit/+/193710
Latest: https://git.eclipse.org/r/c/jgit/jgit/+/193712
New Gerrit change created: https://git.eclipse.org/r/c/jgit/jgit/+/193786
(In reply to Eric Steele from comment #20) > Hey Thomas... I think I'm a bit confused by how Gerrit does things... > > I followed the contributor guide and submitted some changes, but saw that it > looked like my original PR was no longer being shown/active. I thought I had > done something wrong and so I marked those changes as Abandoned in Gerrit. I > consolidated my changes in a single change and pushed it (one commit), but > now Gerrit is showing multiple reviews. I am very confused :) > > Hopefully we can recover from this situation. My most recent push is here: > https://git.eclipse.org/r/c/jgit/jgit/+/193705/1 Sorry, Eric, only saw this now. With Gerrit you *amend* your commit. When you push the amended commit, Gerrit produces a new patch set on the change (if it has the same change ID). If you have several commits, Gerrit produces one change per commit, and one change depends on the other. To fix https://git.eclipse.org/r/c/jgit/jgit/+/193786/1 and https://git.eclipse.org/r/c/jgit/jgit/+/193712/1 : Squash them, and make sure the change-id of the squashed commit is If289dbc6d0f57323cfeaac2624c4eb5028f78d13 . Then push again. You should get a "Path Set 2" on 193712. Then abandon 193786.
Thank you Thomas!!! I followed your instructions and see patchset 2 on my PR. Please review :)
Gerrit change https://git.eclipse.org/r/c/jgit/jgit/+/193712 was merged to [stable-5.13]. Commit: http://git.eclipse.org/c/jgit/jgit.git/commit/?id=e9a5430c2557778bc6c43986527d57023090e781
thanks Eric for contributing this improvement
No problem :) Thank you so much for helping me get it done!
