Skip to content

Commit e08c835

Browse files
committed
fix: bind GitHub release identity to verified bundle
1 parent 055570b commit e08c835

1 file changed

Lines changed: 72 additions & 30 deletions

File tree

crates/rattler_build_source_cache/src/sigstore.rs

Lines changed: 72 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -412,30 +412,31 @@ fn identity_matches(expected: &str, actual: &str) -> bool {
412412
/// GitHub's release attester has the fixed certificate identity
413413
/// `https://dotcom.releases.github.com`; the repository identity is instead
414414
/// cryptographically bound into the signed subject as a package URL.
415-
fn github_release_repository_subjects(bundles: &[sigstore_types::Bundle]) -> Vec<String> {
416-
let mut repositories = Vec::new();
417-
for bundle in bundles {
418-
let SignatureContent::DsseEnvelope(envelope) = &bundle.content else {
419-
continue;
420-
};
421-
if envelope.payload_type != "application/vnd.in-toto+json" {
422-
continue;
423-
}
424-
let Ok(statement) = serde_json::from_slice::<serde_json::Value>(&envelope.decode_payload())
425-
else {
426-
continue;
427-
};
428-
let Some(subjects) = statement.get("subject").and_then(|value| value.as_array()) else {
429-
continue;
430-
};
431-
repositories.extend(subjects.iter().filter_map(|subject| {
415+
fn github_release_repository_subjects(bundle: &sigstore_types::Bundle) -> Vec<String> {
416+
let SignatureContent::DsseEnvelope(envelope) = &bundle.content else {
417+
return Vec::new();
418+
};
419+
if envelope.payload_type != "application/vnd.in-toto+json" {
420+
return Vec::new();
421+
}
422+
let Ok(statement) = serde_json::from_slice::<serde_json::Value>(&envelope.decode_payload())
423+
else {
424+
return Vec::new();
425+
};
426+
let Some(subjects) = statement.get("subject").and_then(|value| value.as_array()) else {
427+
return Vec::new();
428+
};
429+
430+
let mut repositories: Vec<_> = subjects
431+
.iter()
432+
.filter_map(|subject| {
432433
subject
433434
.get("uri")
434435
.and_then(|value| value.as_str())
435436
.filter(|uri| uri.starts_with("pkg:github/"))
436437
.map(ToOwned::to_owned)
437-
}));
438-
}
438+
})
439+
.collect();
439440
repositories.sort();
440441
repositories.dedup();
441442
repositories
@@ -445,13 +446,16 @@ fn github_release_matches_repository(
445446
repository_subjects: &[String],
446447
expected_identity: &str,
447448
) -> bool {
448-
let Some(repository) = expected_identity.strip_prefix("https://github.com/") else {
449+
let Some(expected_repository) = expected_identity.strip_prefix("https://github.com/") else {
449450
return false;
450451
};
451-
let expected_uri_prefix = format!("pkg:github/{repository}@");
452-
repository_subjects
453-
.iter()
454-
.any(|uri| uri.starts_with(&expected_uri_prefix))
452+
repository_subjects.iter().any(|uri| {
453+
uri.strip_prefix("pkg:github/")
454+
.and_then(|package| package.split_once('@'))
455+
.is_some_and(|(repository, _version)| {
456+
repository.eq_ignore_ascii_case(expected_repository)
457+
})
458+
})
455459
}
456460

457461
pub(crate) async fn verify_attestation(
@@ -536,13 +540,21 @@ pub(crate) async fn verify_attestation(
536540
let mut matched = false;
537541
let mut found_identities: Vec<String> = Vec::new();
538542
let mut verification_errors: Vec<String> = Vec::new();
543+
// Keep an aggregate only for error reporting. Acceptance below checks
544+
// the repository subject from the same bundle that verifies the artifact,
545+
// preventing subjects from separate bundles from being mixed together.
539546
let repository_subjects = if parsed.from_github {
540-
github_release_repository_subjects(&parsed.bundles)
547+
let mut subjects: Vec<_> = parsed
548+
.bundles
549+
.iter()
550+
.flat_map(github_release_repository_subjects)
551+
.collect();
552+
subjects.sort();
553+
subjects.dedup();
554+
subjects
541555
} else {
542556
Vec::new()
543557
};
544-
let repository_matches = !parsed.from_github
545-
|| github_release_matches_repository(&repository_subjects, &check.identity);
546558

547559
for bundle in &parsed.bundles {
548560
// GitHub immutable releases are signed by GitHub's release service,
@@ -571,8 +583,12 @@ pub(crate) async fn verify_attestation(
571583
if let Some(ref actual_identity) = result.identity {
572584
let identity_ok = if parsed.from_github {
573585
// The policy checked the release-service certificate;
574-
// also require the repository in the signed statement.
575-
repository_matches
586+
// also require the repository in this same signed
587+
// statement. Do not combine evidence across bundles.
588+
github_release_matches_repository(
589+
&github_release_repository_subjects(bundle),
590+
&check.identity,
591+
)
576592
} else {
577593
identity_matches(&check.identity, actual_identity)
578594
};
@@ -898,18 +914,44 @@ mod tests {
898914
"uri": "pkg:github/astral-sh/uv@0.12.1",
899915
"digest": { "sha1": "abc123" }
900916
})]);
901-
let subjects = github_release_repository_subjects(&[bundle]);
917+
let subjects = github_release_repository_subjects(&bundle);
902918
assert_eq!(subjects, ["pkg:github/astral-sh/uv@0.12.1"]);
903919
assert!(github_release_matches_repository(
904920
&subjects,
905921
"https://github.com/astral-sh/uv"
906922
));
923+
assert!(github_release_matches_repository(
924+
&subjects,
925+
"https://github.com/ASTRAL-SH/UV"
926+
));
907927
assert!(!github_release_matches_repository(
908928
&subjects,
909929
"https://github.com/astral-sh/uv-extra"
910930
));
911931
}
912932

933+
#[test]
934+
fn test_github_repository_identity_is_scoped_to_one_bundle() {
935+
let expected_repository_bundle = make_bundle_with_raw_subjects(vec![serde_json::json!({
936+
"uri": "pkg:github/victim/project@1.0.0",
937+
"digest": { "sha1": "abc123" }
938+
})]);
939+
let artifact_bundle = make_bundle_with_raw_subjects(vec![serde_json::json!({
940+
"uri": "pkg:github/attacker/project@1.0.0",
941+
"digest": { "sha1": "def456" }
942+
})]);
943+
944+
let expected_identity = "https://github.com/victim/project";
945+
assert!(github_release_matches_repository(
946+
&github_release_repository_subjects(&expected_repository_bundle),
947+
expected_identity,
948+
));
949+
assert!(!github_release_matches_repository(
950+
&github_release_repository_subjects(&artifact_bundle),
951+
expected_identity,
952+
));
953+
}
954+
913955
#[test]
914956
fn test_verify_artifact_subject_matching_digest() {
915957
let artifact = b"hello world";

0 commit comments

Comments
 (0)