distribution
[release/2.8] vendor: github.com/aws/aws-sdk-go v1.43.16
This updates the vendored version of the AWS SDK to v1.43.7, which is currently the latest version available. This should resolve #3437 and make it possible to resolve #3275 in a subsequent PR.
Signed-off-by: Trevor Wood [email protected]
Codecov Report
Merging #3599 (2e016d0) into release/2.8 (e4a447d) will not change coverage. The diff coverage is
n/a.
@@ Coverage Diff @@
## release/2.8 #3599 +/- ##
============================================
Coverage 58.72% 58.72%
============================================
Files 102 102
Lines 7104 7104
============================================
Hits 4172 4172
Misses 2286 2286
Partials 646 646
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact),ø = not affected,? = missing dataPowered by Codecov. Last update e4a447d...2e016d0. Read the comment docs.
Updated this to the latest SDK version. Any chance you can take a look at this @milosgajdos?
since we want to maintain 2.8 for security fix only, so Im not sure whether we still accept other PRs. @milosgajdos
Yeah this is very risky to accept; if we do decide to do it it'd have to be for 2.9 release and I'm quite skeptical, if not convinced, that will happen I'm afraid.
If I understand correctly, this PR fixes a rather major bug in AWS authentication (the default credential chain doesn't work with web identites without this).
If it can't be part of 2.8 when can it be released in a stable version. If the next release is months away, I'd say merging it and releasing it as a 2.8.x patch would greatly benefit the community.
I could argue that this is in fact a security problem: without support for web identities we can't use standard tools intended for granting access to workloads within certain environments. The alternatives are:
- grant access to resources through node role which would let other workloads also access those resources
- manually configure IAM accounts
Both of these alternatives are less secure compared to using workload identities.
Again, it is my understanding that this is still an issue with the current release. If it's not, feel free to disregard my comment.
If I understand correctly, this PR fixes a rather major bug in AWS authentication (the default credential chain doesn't work with web identites without this).
If it can't be part of 2.8 when can it be released in a stable version. If the next release is months away, I'd say merging it and releasing it as a 2.8.x patch would greatly benefit the community.
I could argue that this is in fact a security problem: without support for web identities we can't use standard tools intended for granting access to workloads within certain environments. The alternatives are:
- grant access to resources through node role which would let other workloads also access those resources
- manually configure IAM accounts
Both of these alternatives are less secure compared to using workload identities.
Again, it is my understanding that this is still an issue with the current release. If it's not, feel free to disregard my comment.
This will not fix the credential chain since it is currently set to something other than the default (https://github.com/distribution/distribution/blob/v2.8.1/registry/storage/driver/s3-aws/s3.go#L412-L423); however, the credential chain cannot be updated to enable things like IRSA without this being merged first. Moreover, this also enables the use of IMDSv2, which is another security fix.
I understand the points made in these comments, but I would like to avoid a discussion about what constitutes a code related security issue vs what is an operator hurdle causing security headaches. I also understand as soon as I press comment button this will totally backfire on me, but I'm still reluctant to consider adding this change to 2.8
I understand the points made in these comments, but I would like to avoid a discussion about what constitutes a code related security issue vs what is an operator hurdle causing security headaches. I also understand as soon as I press comment button this will totally backfire on me, but I'm still reluctant to consider adding this change to 2.8
That's completely understandable. If there is a 2.9 release branch, then I would be happy to point this PR to that instead and get the rest of the necessary changes staged up as well.
@milosgajdos I think that's understandable. What are the alternatives then? (eg. a 2.9 version?)
can we merge this and add subsequent work to support IRSA as part of https://github.com/distribution/distribution/issues/3275
@milosgajdos what's the state of this PR? we need IAM roles support for accessing s3, and it's blocked by this PR. Would you recommend us to build a binary from this branch?
@pchico83 For IAM role support https://github.com/distribution/distribution/pull/3921 is also needed.
It's been 2 years since @mwek mentioned this issue was fixed on main in https://github.com/distribution/distribution/issues/3437#issuecomment-897780206. As far as I can tell using the latest release (2.8.2), nothing has changed.
Does this project have a release schedule? Is there a date for the next release that can support IMDSv2?
@milosgajdos Do you think we could open the discussion to having this target a 2.9 release branch? I think it’d be nice for everyone to get this issue that’s been going on for years cleared out.
@raxod502-plaid The reason this is not fixed in release 2.8.2 is because the 2.8 release branch is very different from the main branch.
@milosgajdos Do you think we could open the discussion to having this target a 2.9 release branch?
There will be no 2.9 release. All the effort should go into getting v3 out.
Its a bit sad, that the choice as an operator of such a registry i have, is to either run an outdated version where AWS IAM auth works properly (sha256:3c1586b8ac273a1c72303dae137190bfca983cbe0f6e752e0456a9d8a232e736) or have a more recent version with security issues fixed, but only can use considered insecure ways to pass in credentials.
As all is aimed for v3 is there any rough indication on when this could be expected to be available for use, or as a beta for testing?
FWIW, we forked the registry codebase, merged this PR, built a custom image, and that's been running in production just fine for the last few weeks. I don't think we have any other realistic alternatives given the security requirements.
The changes in this PR got merged here https://github.com/distribution/distribution/pull/3606
@mikecook That is for the main branch. This PR targets the current release branch.
@mikecook That is for the main branch. This PR targets the current release branch.
Ah! I see. Thanks, I missed that.
With https://github.com/distribution/distribution/releases/tag/v3.0.0-alpha.1 out now, I'm closing this PR as it won't be merged into 2.x branch. I'd strongly recommend using the new release as all the further development is going into getting a stable v3 release out early next year.