Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow audience to be explicitly specified #285

Closed
wants to merge 0 commits into from

Conversation

alblue
Copy link
Contributor

@alblue alblue commented Oct 9, 2021

Description of changes:

Allow the audience to be configured instead of defaulting to 'sigstore'.

It's desirable to be able to use different audiences to ensure that changes aren't enabled on the wrong location -- for example, you can use an audience of 'prod' in the production accounts and 'dev' in the development accounts. This is a parameter that's passed to the ACTIONS_ID_TOKEN_REQUEST_URL already, but it is hard-coded.

By adding audience as an argument to the job, you can set a specific parameter in the with block, and the example has been updated to reflect this.

(You could, for example, us sts.amazonaws.com as the default argument in the examples shown while still allowing the sigstore default to be used if people desire.)

This approach should also be usable with the (recently backed-out #280) approach, except by decoupling what the audience is with what the hard-coded default is, you won't break other usages of it.

By the way, it's possible to configure both the role and the OICD provider with an 'either' clause, so you can use sigstore or sts.amazonaws.com -- but that somewhat defeats the point of using a non-default value :)

  "Action": "sts:AssumeRoleWithWebIdentity",
  "Condition": {
    "StringLike": {
      "token.actions.githubusercontent.com:sub": "repo:alblue/*",
      "token.actions.githubusercontent.com:aud": ["sigstore","alblue"]
    }

Furthermore this approach allows you to use the audience to allow for different sets roles to be enabled; you could have a dev audience that is only trusted by the GitHubDev role, and a prod audience that is only trusted by the GitHubProd role. The OICD audience could have both, but you'd then guarantee that the dev audience couldn't assume the GitHubProd role (and vice versa).

The equivalent post a78fcb0 change would be:

 core.getIDToken(audience);

By the way, It think the reason that a78fcb0 had to be rolled back wasn't because the new token was the wrong way of doing it, but because the default audience changed from sigstore to sts.amazonaws.com without letting others update their OIDC providers. I think the new code would work as is if the 'sigstore' was still the default -- but with this PR, you could allow people to specify what they want.

I'd personally suggest, after merging this PR, you could look at re-reverting a78fcb0 (except change the line as above) and update the documentation/README to say 'audience: sts.amazonaws.com' but have the code default to 'sigstore' if the audience: field is not present. Then you wouldn't break existing users while encouraging them to use the new audience instead.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

index.js Outdated
@@ -14,12 +14,14 @@ const MAX_TAG_VALUE_LENGTH = 256;
const SANITIZATION_CHARACTER = '_';
const ROLE_SESSION_NAME = 'GitHubActions';
const REGION_REGEX = /^[a-z0-9-]+$/g;
const DEFAULT_AUDIENCE = 'sigstore';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we will want the default audience to be sigstore. That was an example value that was used while the provider was under development, it's not a good default value for production.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree. However, if you change this now, and people have it embedded in builds (as master), you'll get issues like #280 being raised because you'll immediately break them. In any case, it's the default audience if not specified by GitHub, so I disagree it's not the correct default, even if the documentation explicitly shows how to change it to sts.amazonaws.com in your repository.

@richardhboyd
Copy link
Contributor

The reason the commit was rolled back was because the ability to set an arbitrary audience wasn't completely rolled out yet by GH so we reverted it until the feature was officially released

@alblue
Copy link
Contributor Author

alblue commented Oct 9, 2021

Just FYI we've been using custom audiences for a few weeks now successfully. You've got 'sigstore' encoded as the URL at the moment, which is why it's there -- but you can swap it for something else fine. The issue is you need to ensure that all of the request/oicd/role support the correct audience name, which is often a challenge ...

It's possible to configure an OpenID provider to allow more than one audience (up to 5 I think) and similarly for a role, so you can grandfather in 'sigstore' if you need to. Personally though I would like to avoid using 'sigstore' too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants