docs
docs copied to clipboard
Clarify if all OIDC conditions are supported on AWS (they don't appear to be)
Code of Conduct
- [X] I have read and agree to the GitHub Docs project's Code of Conduct
What article on docs.github.com is affected?
https://docs.github.com/en/actions/deployment/security-hardening-your-deployments/configuring-openid-connect-in-amazon-web-services
What part(s) of the article would you like to see updated?
From my experience and others in this ticket, it appears that AWS don't currently support checking the various assertions that are passed through by GitHub e.g. repository_owner in my case. Their docs suggest otherwise, so it is probably a bug in AWS.
It might be worth highlighting in the docs, unless you have been able to get it to work yourselves.
Filtering on sub
works fine.
Additional information
No response
[maintainer edit]
Content Plan
See this comment for additional details when working on this issue.
Thanks for opening this issue. A GitHub docs team member should be by to give feedback soon. In the meantime, please check out the contributing guidelines.
@mungojam Thanks so much for opening an issue! I'll triage this for the team to take a look :eyes:
There is a little more evidence that custom claims aren't supported through AssumeRoleWithWebIdentity
here:
https://gist.github.com/gene1wood/54611c45a3298504f8e33d95e317ae5c?permalink_comment_id=2973957#gistcomment-2973957
At least they weren't in 2019
And here are the official docs on it that only list a limited number of fields:
https://docs.aws.amazon.com/en_en/IAM/latest/UserGuide/reference_policies_iam-condition-keys.html#condition-keys-wif
In the example given in the Github Actions docs:
"Condition": {
"ForAllValues:StringEquals": {
"token.actions.githubusercontent.com:aud": "sts.amazonaws.com",
"token.actions.githubusercontent.com:sub": "repo:octo-org/octo-repo:ref:refs/heads/octo-branch"
}
}
Notice that "StringEquals"
has changed to "ForAllValues:StringEquals
. This fixed it for me. I can use the custom claims now.
This should probably be clarified or called out specifically in this documentation, because it's not super obvious.
aud
and sub
are both listed as supported in the AWS docs, so that explains why it works. Unfortunately none of the custom claims like actor
or repository_owner
are supported. For repository_owner
we can use a StringLike
with sub
to achieve the same effect, but not for actor
which doesn't appear in any of the standard claims.
👋 Checking with engineering team
Thanks @mungojam for raising this issue. As you have rightly figured out above, AWS currently supports only aud
and sub
along with a few others but dont have support for all the custom claims that are part of GitHub OIDC token.
Can you please provide more details around your specific requirement to set condition on actor
or repository_owner
? Would this validation be needed across all workflows in your organization or just a subset of workflows?
We are currently evaluating to provide a programatic way to customize what custom claims can be made part of the sub
claim and any further details in to your use case will help us build more context. Thanks!
At least for me, I want to be able to allow Github Actions workflows that are run in my organization to assume roles in my AWS account using the OIDC connector. The simplest way for me to allow only repos in my organization to do that is to set a condition on the repository_owner
claim.
Thanks @mungojam for raising this issue. As you have rightly figured out above, AWS currently supports only
aud
andsub
along with a few others but dont have support for all the custom claims that are part of GitHub OIDC token.Can you please provide more details around your specific requirement to set condition on
actor
orrepository_owner
? Would this validation be needed across all workflows in your organization or just a subset of workflows?We are currently evaluating to provide a programatic way to customize what custom claims can be made part of the
sub
claim and any further details in to your use case will help us build more context. Thanks!
I wanted to be able to have some backstop that prevents any Github Actions access to our AWS accounts from repositories outside our GitHub org. We would expect all teams to have additional protections to limit access from only their repositories and may also enforce that, but the ultimate backstop would still be wanted. As it is, I have been able to use StringLike to achieve this instead so the impact on me is not big.
I raised this particular issue as I think the GitHub docs should be explicit about the AWS limitations so that people don't waste time or accidentally make dangerous security rules like using ForAllValues:StringEquals
(@ianling, this is doing the opposite of what you think and potentially opening up your accounts to all repos).
👋 @mungojam - Just checking in on this one. Are you able to make a PR based on your latest comment above?
👋 @mungojam - Just checking in on this one. Are you able to make a PR based on your latest comment above?
Sorry, I only really have time for doing small documentation tweaks in PRs, or code changes. My skills at writing clear English docs aren't great so it takes me ages (despite being English!).
Though on this topic, I note the new additions that allow for the org. name to be included in the audience and also the ability to set custom claims in the sub
field. Both are steps in the right direction and I hope more customisability will come and also better documentation for them (for example what will the format of the audience url be if the org. is added?)
That's a shame, closing with no explanation when it is clear that plenty of people are confused and potentially use dangerous settings because of the lack of clarity
@mungojam It looks like stalebot closed this in error, so I'm reopening it. It's currently on the help wanted
board for anyone to pick up and make this contribution 💖
👋🏻 @mungojam
We're reviewing all open help wanted
issues as part of our preparation for Hacktoberfest, which is coming up shortly.
I've looked at the changes to the article since this issue was originally opened and can see that the following change has been made:
-
Configuring the role and trust policy explains how to define a role and trust policy and no longer suggests
ForAllValues
.
I think that the only additional change we might want to make for this issue is to expand the sentence: "The token also includes custom claims provided by GitHub." to explain that some cloud providers do not recognize GitHub's custom claims and that you should check the provider's own documentation before configuring your connection.
I think that the only additional change we might want to make for this issue is to expand the sentence: "The token also includes custom claims provided by GitHub." to explain that some cloud providers do not recognize GitHub's custom claims and that you should check the provider's own documentation before configuring your connection.
@felicitymay Thanks for looking into this. A few thoughts on what could still be improved. The page you link has a pre-requisite section at the top. I'm pretty methodical so thought I should read all that it said:
Specifically it says Before proceeding, you must plan your security strategy to ensure that access tokens are only allocated in a predictable way.
and then links to "About security hardening with OpenID Connect." for more information.
It is that page which doesn't seem to clearly state that not all token attributes will be supported by all cloud providers. So somebody could follow the advice of the pre-req and plan their enforcement based on that page, assuming that it will be possible to achieve it (I think that was the cause for me).
Similarly on the AWS page, there is a clear note recommending the use of sub
but nothing to suggest that other claims may not be supported
Thank you for getting back to us so quickly with such clear feedback 💖
I'll update the issue summary based on your feedback and this will be ready for someone to work on. We usually get a lot of contributions during Hacktoberfest.
Based on your feedback, I've updated the issue summary to suggest a location in each file where we should note that the claims included in a GitHub token may not be supported by your cloud provider and that you should verify which claims are supported.
👋 Hey @mungojam. Thanks for the suggestion here and for your patience while we worked through this. I've been communicating with some folks internally on this issue and we agree that the docs could use a couple changes to specify the OIDC conditions for AWS.
Because these files don't accept community contributions, I'll close this and open an internal issue to fix this.
That's cool thanks.
One side note is that here and on the GitHub roadmap I've noticed githubbers not making use of the close as dupe/not-planned GitHub feature. It really helps to communicate when people are searching for closed issues or looking at notifications