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
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "policy-fetcher"
version = "0.7.3"
version = "0.7.4"
authors = [
"Flavio Castelli <fcastelli@suse.com>",
"Rafael Fernández López <rfernandezlopez@suse.com>",
Expand All @@ -27,7 +27,7 @@ serde = { version = "1.0", features = ["derive"] }
serde_json = "1.0.81"
serde_yaml = "0.8.24"
sha2 = "0.10.2"
sigstore = { git = "https://github.com/sigstore/sigstore-rs", default-features = false, features = ["rustls-tls"], rev = "cb3606a9d3d05a5e318bb3837f48c560c01cc2ff" }
sigstore = { git = "https://github.com/sigstore/sigstore-rs", default-features = false, features = ["rustls-tls"], rev = "a817a40534af55eda5750a46104e6e9f40b31d7a" }
tracing = "0.1.34"
url = { version = "2.2.2", features = ["serde"] }
walkdir = "2"
Expand Down
5 changes: 5 additions & 0 deletions src/verify/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -384,6 +384,11 @@ kvUsh4eKpd1lwkDAzfFDs7yXEExsEkPPuiQJBelDT68n7PDIWB/QEY7mrA==
verification_key,
issuer,
subject,
github_workflow_trigger: None,
github_workflow_sha: None,
github_workflow_name: None,
github_workflow_repository: None,
github_workflow_ref: None,
});

SignatureLayer {
Expand Down
92 changes: 70 additions & 22 deletions src/verify/verification_constraints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,8 @@ pub struct GitHubVerifier {
}

const GITHUB_ACTION_ISSUER: &str = "https://token.actions.githubusercontent.com";
const GITHUB_ACTION_SUBJECT_EXAMPLE: &str =
"https://github.com/octocat/example/.github/workflows/ci.yml@refs/tags/v0.1.0";

impl GitHubVerifier {
pub fn new(
Expand Down Expand Up @@ -224,20 +226,30 @@ impl VerificationConstraint for GitHubVerifier {
}
};

let signature_url = match &certificate_signature.subject {
// the certificate subject must be a valid github URI and not an email
let signature_subject = match &certificate_signature.subject {
CertificateSubject::Email(email) => {
debug!(
expected_value = ?GITHUB_ACTION_ISSUER,
expected_value = ?GITHUB_ACTION_SUBJECT_EXAMPLE,
current_value = ?email,
"subject not satisfied, expected URI, got email instead"
);
return Ok(false);
}
CertificateSubject::Uri(u) => u,
};
GitHubRepo::try_from(signature_subject.as_str()).map_err(|_|
SigstoreError::VerificationConstraintError(format!("The certificate subject url doesn't seem a GitHub valid one, despite the issuer being the GitHub Action one: {}", signature_subject)))?;

let signature_repo = GitHubRepo::try_from(signature_url.as_str()).map_err(|_|
SigstoreError::VerificationConstraintError(format!("The certificate signature url doesn't seem a GitHub valid one, despite the issuer being the GitHub Action one: {}", signature_url)))?;
// the certificate github_workflow_extension must be there and correctly constructed
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.


let signature_repo = GitHubRepo::try_from(format!("https://github.com/{}", github_workflow_repository).as_str())
.map_err(|_|
SigstoreError::VerificationConstraintError(format!("The certificate doesn't have a valid github_workflow_repository extension, despite the issuer being the GitHub Action one: {}", github_workflow_repository)))?;

if signature_repo.owner != self.owner {
debug!(
Expand Down Expand Up @@ -348,6 +360,7 @@ kvUsh4eKpd1lwkDAzfFDs7yXEExsEkPPuiQJBelDT68n7PDIWB/QEY7mrA==
fn build_signature_layers_keyless(
issuer: Option<String>,
subject: CertificateSubject,
github_workflow_repository: Option<String>,
) -> SignatureLayer {
let pub_key = r#"-----BEGIN PUBLIC KEY-----
MFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAELKhD7F5OKy77Z582Y6h0u1J3GNA+
Expand All @@ -370,6 +383,11 @@ kvUsh4eKpd1lwkDAzfFDs7yXEExsEkPPuiQJBelDT68n7PDIWB/QEY7mrA==
verification_key,
issuer,
subject,
github_workflow_trigger: None,
github_workflow_sha: None,
github_workflow_name: None,
github_workflow_repository,
github_workflow_ref: None,
});

SignatureLayer {
Expand Down Expand Up @@ -432,7 +450,8 @@ kvUsh4eKpd1lwkDAzfFDs7yXEExsEkPPuiQJBelDT68n7PDIWB/QEY7mrA==

let certificate_subject = CertificateSubject::Email(subject_str.to_string());

let sl = build_signature_layers_keyless(Some(issuer.to_string()), certificate_subject);
let sl =
build_signature_layers_keyless(Some(issuer.to_string()), certificate_subject, None);

let vc = GenericIssuerSubjectVerifier::new(issuer, &subject, None);
let is_verified = vc.verify(&sl).expect("Should have been successful");
Expand All @@ -445,7 +464,8 @@ kvUsh4eKpd1lwkDAzfFDs7yXEExsEkPPuiQJBelDT68n7PDIWB/QEY7mrA==

let certificate_subject = CertificateSubject::Uri(subject_str.to_string());

let sl = build_signature_layers_keyless(Some(issuer.to_string()), certificate_subject);
let sl =
build_signature_layers_keyless(Some(issuer.to_string()), certificate_subject, None);

let vc = GenericIssuerSubjectVerifier::new(issuer, &subject, None);
let is_verified = vc.verify(&sl).expect("Should have been successful");
Expand All @@ -458,7 +478,8 @@ kvUsh4eKpd1lwkDAzfFDs7yXEExsEkPPuiQJBelDT68n7PDIWB/QEY7mrA==

let certificate_subject = CertificateSubject::Email(subject_str.to_string());

let sl = build_signature_layers_keyless(Some(issuer.to_string()), certificate_subject);
let sl =
build_signature_layers_keyless(Some(issuer.to_string()), certificate_subject, None);

let vc = GenericIssuerSubjectVerifier::new(issuer, &subject, None);
let is_verified = vc.verify(&sl).expect("Should have been successful");
Expand All @@ -473,7 +494,8 @@ kvUsh4eKpd1lwkDAzfFDs7yXEExsEkPPuiQJBelDT68n7PDIWB/QEY7mrA==

let certificate_subject = CertificateSubject::Email(subject_str.to_string());

let sl = build_signature_layers_keyless(Some(issuer.to_string()), certificate_subject);
let sl =
build_signature_layers_keyless(Some(issuer.to_string()), certificate_subject, None);

let mut annotations: HashMap<String, String> = HashMap::new();
annotations.insert("key1".into(), "value1".into());
Expand All @@ -494,13 +516,16 @@ kvUsh4eKpd1lwkDAzfFDs7yXEExsEkPPuiQJBelDT68n7PDIWB/QEY7mrA==
let vc = GenericIssuerSubjectVerifier::new(issuer, &subject, None);

// a signature without issuer - could happen with early signatures of cosign
let sl = build_signature_layers_keyless(None, certificate_subject.clone());
let sl = build_signature_layers_keyless(None, certificate_subject.clone(), None);
let is_verified = vc.verify(&sl).expect("Should have been successful");
assert!(!is_verified);

// a signature with a different issuer
let sl =
build_signature_layers_keyless(Some("another issuer".to_string()), certificate_subject);
let sl = build_signature_layers_keyless(
Some("another issuer".to_string()),
certificate_subject,
None,
);
let is_verified = vc.verify(&sl).expect("Should have been successful");
assert!(!is_verified);
}
Expand All @@ -514,7 +539,7 @@ kvUsh4eKpd1lwkDAzfFDs7yXEExsEkPPuiQJBelDT68n7PDIWB/QEY7mrA==
let vc = GenericIssuerSubjectVerifier::new(issuer, &subject, None);

let another_subject = CertificateSubject::Email("alice@provider.com".to_string());
let sl = build_signature_layers_keyless(Some(issuer.to_string()), another_subject);
let sl = build_signature_layers_keyless(Some(issuer.to_string()), another_subject, None);
let is_verified = vc.verify(&sl).expect("Should have been successful");
assert!(!is_verified);
}
Expand All @@ -529,7 +554,8 @@ kvUsh4eKpd1lwkDAzfFDs7yXEExsEkPPuiQJBelDT68n7PDIWB/QEY7mrA==

let certificate_subject = CertificateSubject::Uri(subject_str.to_string());

let sl = build_signature_layers_keyless(Some(issuer.to_string()), certificate_subject);
let sl =
build_signature_layers_keyless(Some(issuer.to_string()), certificate_subject, None);

let vc = GenericIssuerSubjectVerifier::new(issuer, &subject, None);
let is_verified = vc.verify(&sl).expect("Should have been successful");
Expand All @@ -538,13 +564,15 @@ kvUsh4eKpd1lwkDAzfFDs7yXEExsEkPPuiQJBelDT68n7PDIWB/QEY7mrA==

#[test]
fn test_generic_issuer_subject_url_prefix_prevent_abuses() {
// signature done inside of `kubewarde-hacker` organization, but we trust only `kubewarden`
// signature done inside of `kubewarden-hacker` organization, but we trust only `kubewarden`
// org

let issuer = "https://token.actions.githubusercontent.com";
let subject_str = "https://github.com/kubewarden-hacker/policy-secure-pod-images/.github/workflows/release.yml@refs/heads/main";
let certificate_subject = CertificateSubject::Uri(subject_str.to_string());
let sl = build_signature_layers_keyless(Some(issuer.to_string()), certificate_subject);

let sl =
build_signature_layers_keyless(Some(issuer.to_string()), certificate_subject, None);

// It has a trailing `/`
let prefix =
Expand Down Expand Up @@ -630,30 +658,43 @@ kvUsh4eKpd1lwkDAzfFDs7yXEExsEkPPuiQJBelDT68n7PDIWB/QEY7mrA==
fn test_github_verifier_success() {
let issuer = "https://token.actions.githubusercontent.com";
let subject_str = "https://github.com/kubewarden/policy-secure-pod-images/.github/workflows/release.yml@refs/heads/main";
let github_workflow_repository = "octocat/policy-secure-pod-images";

let certificate_subject = CertificateSubject::Uri(subject_str.to_string());

let sl = build_signature_layers_keyless(Some(issuer.to_string()), certificate_subject);
let sl = build_signature_layers_keyless(
Some(issuer.to_string()),
certificate_subject,
Some(github_workflow_repository.to_string()),
);

// check specifically this owner/repo
let vc = GitHubVerifier::new("kubewarden", Some("policy-secure-pod-images"), None);
let vc = GitHubVerifier::new("octocat", Some("policy-secure-pod-images"), None);
let is_verified = vc.verify(&sl).expect("Should have been successful");
assert!(is_verified);

// anything from this owner is fine
let vc = GitHubVerifier::new("kubewarden", None, None);
let vc = GitHubVerifier::new("octocat", None, None);
let is_verified = vc.verify(&sl).expect("Should have been successful");
assert!(is_verified);
}

#[test]
fn test_github_verifier_reject() {
// we must fail if the github_workflow_repository doesn't match the
// owner and repo of the verifier, regardless of subject. This is
// specially important when people consume GHA reusable workflows.
let issuer = "https://token.actions.githubusercontent.com";
let subject_str = "https://github.com/kubewarden/policy-secure-pod-images/.github/workflows/release.yml@refs/heads/main";
let github_workflow_repository = "octocat/policy-secure-pod-images";

let certificate_subject = CertificateSubject::Uri(subject_str.to_string());

let sl = build_signature_layers_keyless(Some(issuer.to_string()), certificate_subject);
let sl = build_signature_layers_keyless(
Some(issuer.to_string()),
certificate_subject,
Some(github_workflow_repository.to_string()),
);

// check specifically this owner/repo
let vc = GitHubVerifier::new("kubewarden", Some("psp-one"), None);
Expand All @@ -670,10 +711,15 @@ kvUsh4eKpd1lwkDAzfFDs7yXEExsEkPPuiQJBelDT68n7PDIWB/QEY7mrA==
fn test_github_verifier_reject_because_issuer_url_is_wrong() {
let issuer = "https://google.com";
let subject_str = "https://github.com/kubewarden/policy-secure-pod-images/.github/workflows/release.yml@refs/heads/main";
let github_workflow_repository = "octocat/policy-secure-pod-images";

let certificate_subject = CertificateSubject::Uri(subject_str.to_string());

let sl = build_signature_layers_keyless(Some(issuer.to_string()), certificate_subject);
let sl = build_signature_layers_keyless(
Some(issuer.to_string()),
certificate_subject,
Some(github_workflow_repository.to_string()),
);

let vc = GitHubVerifier::new("kubewarden", None, None);
let is_verified = vc.verify(&sl).expect("Should have been successful");
Expand All @@ -682,12 +728,14 @@ kvUsh4eKpd1lwkDAzfFDs7yXEExsEkPPuiQJBelDT68n7PDIWB/QEY7mrA==

#[test]
fn test_github_verifier_reject_because_certificate_subject_does_not_have_url() {
// it must have URL, as this is a GH Actions signature
let issuer = "https://token.actions.githubusercontent.com";
let subject_str = "https://github.com/kubewarden/policy-secure-pod-images/.github/workflows/release.yml@refs/heads/main";
let subject_str = "octocat@example.com";

let certificate_subject = CertificateSubject::Email(subject_str.to_string());

let sl = build_signature_layers_keyless(Some(issuer.to_string()), certificate_subject);
let sl =
build_signature_layers_keyless(Some(issuer.to_string()), certificate_subject, None);

let vc = GitHubVerifier::new("kubewarden", None, None);
let is_verified = vc.verify(&sl).expect("Should have been successful");
Expand Down