Sigstore identity extraction + deferred-work tracking - #10
Closed
electricapp wants to merge 2 commits into
Closed
Conversation
Adds cert-embedded identity evidence to the SLSA attestation check and
shores up test coverage for the three features committed earlier in
this v2 pass (cross-repo taint, historical replay, SLSA).
Sigstore identity (src/github/sigstore.rs, ~370 lines):
* Minimal DER walker (length-prefixed TLV traversal, no external
x509 crate) that extracts two fields from the attestation
bundle's verificationMaterial cert:
- Subject Alternative Name URI (Fulcio encodes the workflow
identity as a URL like
https://github.com/owner/repo/.github/workflows/build.yml
@refs/tags/v1.2.3 -- tells a user *what exactly* minted
the attestation)
- Issuer Common Name (lets us distinguish Fulcio-shaped CAs
from anything else)
* Supports both modern bundle shape (x509CertificateChain.
certificates[].rawBytes) and legacy single-certificate shape
(certificate.rawBytes).
* SignerIdentity::looks_like_fulcio() checks issuer CN against
sigstore / fulcio substrings -- not cryptographic but a useful
evidence layer until full chain validation lands.
What is still v2.2 (explicitly documented in the module header): full
DSSE signature verification (ECDSA over PAE) and cert-chain validation
to the bundled Fulcio root. Both require ~500 additional lines plus
adding ring as a direct dep; not attempted in this session. The
absence is called out in the module doc so a future pass has a clear
starting point.
AttestationVerdict (src/github/slsa.rs):
* Verified { workflow_ref, builder_id, signer_identity }
-- now carries the extracted identity when available
* New UntrustedIssuer { issuer_cn, subject_uri } variant --
emerges when the cert was issued by a non-Fulcio CA
Provenance integration (src/github/provenance.rs): emit_slsa_finding
gains a match arm for UntrustedIssuer that surfaces a HIGH finding
naming both the issuer CN and (when extracted) the workflow identity.
Additional tests landed in this commit:
* 6 new sigstore unit tests, including a synthetic-cert end-to-end
that builds a valid DER blob by hand and asserts the walker
extracts issuer CN + SAN URI correctly
* tests/replay_history.rs (4 integration tests): covers a multi-
commit repo where a vulnerable past state gets fixed, exit-code
semantics, markdown output, JSON output
* tests/external_artifacts.rs (3 integration tests): covers the
CRIT curl-in-PR pattern, SHA-pinned raw URL exemption, and go
install @latest flag
Totals: 464 tests pass (up from 433), clippy clean on
--all-targets -- -D warnings.
The previous v2 commits referenced 'what's still v2.2' in one place
(sigstore module header) but other gaps were only documented in chat,
not in the tree. Make the deferred work discoverable via grep.
TODOs added:
* src/github/sigstore.rs: expanded the v2.2 note into two labelled
TODO items (DSSE signature verification; Fulcio cert-chain
validation) and points at docs/SECURITY.md for the full story
* src/supply_chain_graph.rs: TODO(v2.7) on collect_online_signals
flagging the hermetic-test gap, with the MockApi pattern as the
template for closing it
docs/SECURITY.md gains a 'Known limitations' section listing three
shipped gaps:
1. SLSA attestation check does not cryptographically verify DSSE
signatures nor build-chain-to-Fulcio-root
2. Tree online signals have no hermetic unit test
3. hasp diff integration tests don't assert sandbox confinement
was actually applied (could silently revert to inline scan if
the sandbox layer breaks)
No behavior change -- pure documentation + discoverability. 464
tests still pass, clippy clean.
Owner
Author
|
Superseded: a newer, more complete version of this work (tree online signals, Sigstore identity, SLSA crypto verification, replay, external-artifact checks, docs split) has been merged to main. Closing this stale April snapshot. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two commits: add Sigstore cert-identity extraction to the SLSA verdict, and track the remaining gaps as TODOs.
Commit 1: Sigstore identity extraction (
src/github/sigstore.rs, ~370 lines)Minimal DER walker (length-prefixed TLV traversal, no external x509 crate) that extracts two fields from the attestation bundle's
verificationMaterialcert:https://github.com/owner/repo/.github/workflows/build.yml@refs/tags/v1.2.3. Tells a user what exactly minted the attestation.Supports both modern bundle shape (
x509CertificateChain.certificates[].rawBytes) and legacy single-certificate shape (certificate.rawBytes).AttestationVerdict changes (
src/github/slsa.rs):Verified { workflow_ref, builder_id, signer_identity }— now carries the extracted identity when availableUntrustedIssuer { issuer_cn, subject_uri }variantProvenance integration:
emit_slsa_findinggains a match arm forUntrustedIssuerthat surfaces a HIGH finding naming both the issuer CN and (when extracted) the workflow identity.Commit 2: TODO bookkeeping + SECURITY.md limitations section
Adds
TODO(v2.x)markers in source for the remaining gaps, plus a "Known limitations" section indocs/SECURITY.mdlisting three shipped gaps:hasp diffintegration tests don't assert sandbox confinement was actually appliedThe subsequent stacked PRs close #1 and #2.
Tests: 6 new sigstore unit tests including a synthetic-cert end-to-end + replay and external-artifacts integration tests. Full suite passes; clippy clean.