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

GitHub actions extensions #90

Merged
merged 5 commits into from
May 24, 2022

Conversation

viccuad
Copy link
Member

@viccuad viccuad commented May 24, 2022

Description

On GithubVerifier, validate using the github_workflow_repository
extension in signature certificate, instead of the subject.

The github_workflow_repository extension contains the repo of the GH
Action workflow that was triggered, not the one that was called. For
example in the case of consuming a reusable workflow
octocat/reusable-workflows-repo in octocat/example,
github_workflow_repository contains octocat/example.

This protects against incorrectly validating signatures that have been
performed in reusable workflows.

Test

Passing unit tests, also tested locally with a policy-server consuming this, and public policies.

Correctly build and consume CertificateSignature{} in all tests.

Change GithubVerifier{} tests for new behaviour: now owner and repo need
to match against github_workflow_repository instead of subject.
On GithubVerifier, validate using the `github_workflow_repository`
extension in signature certificate, instead of the `subject`.

The `github_workflow_repository` extension contains the repo of the GH
Action workflow that was triggered, not the one that was called. For
example in the case of consuming a reusable workflow
`octocat/reusable-workflows-repo` in `octocat/example`,
`github_workflow_repository` contains `octocat/example`.

This protects against incorrectly validating signatures that have been
performed in reusable workflows.
@viccuad viccuad requested a review from a team as a code owner May 24, 2022 11:08
@viccuad viccuad self-assigned this May 24, 2022
Copy link
Member

@flavio flavio left a comment

Choose a reason for hiding this comment

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

LGTM, thanks a lot for the small fixes you did inside of the tests

Comment on lines 245 to 248
let github_workflow_repository = match &certificate_signature.github_workflow_repository {
Some(workflow_repository) => workflow_repository,
None => return Err(SigstoreError::VerificationConstraintError(format!("The certificate is missing the github_workflow_repository extension despite being a GitHub Action one: {}", signature_subject))),
};
Copy link
Member

Choose a reason for hiding this comment

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

Nit, this can be simplified using something like that:

let github_workflow_repository = certificate_signature.github_workflow_repository
   .ok_or_else(|| Err(SigstoreError:VerificationConstraintError("boom"))?;

Copy link
Member Author

Choose a reason for hiding this comment

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

true, thanks! done in ba6c6ab.

Copy link
Contributor

@raulcabello raulcabello left a comment

Choose a reason for hiding this comment

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

LGTM

@viccuad viccuad merged commit 87e91bb into kubewarden:main May 24, 2022
@viccuad
Copy link
Member Author

viccuad commented May 24, 2022

tagged v0.7.4 in main.

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.

4 participants