Keep an attached certificate chain that carries no root - #5046
Closed
Eljees wants to merge 1 commit into
Closed
Conversation
The intermediate pool built from a signature's attached certificate chain assumed the chain always ends with a root, so a chain holding a single intermediate was dropped and verification could only succeed if the intermediate was supplied again alongside the root CA. Fixes sigstore#3976 Signed-off-by: Eljees <3.14hell@gmail.com>
Contributor
|
Thank you for looking into this, but this code path will be removed as of #4959, so no need to fix these edge cases at this point. |
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
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.
Fixes #3976.
When a signature carries an attached certificate chain, the intermediate pool is built like this:
Both parts assume the chain ends with a root. That holds when the signer attaches the whole chain, but not when the root CA is supplied separately — then every certificate in the chain is an intermediate. With a single intermediate attached,
len(chain) > 1is false, the pool is never built, and the attached intermediate is dropped entirely.That is the asymmetry in the report. Running the current logic verbatim against generated certificates:
Which matches both observations: attaching intermediate + root works (with the root duplicated), attaching the intermediate alone fails unless it is also passed on the command line.
The pool building moves into
intermediatePoolFromChain, which drops the last element only when it is actually self-signed, and keeps it otherwise. Existing behaviour for chains that do end with a root is unchanged.Covered by a table test over the five shapes: empty, intermediate only, intermediate + root, two intermediates, root only.
pkg/cosignis green with the change and on a clean tree.go build ./...could not finish here — the module proxy timed out fetchingopen-policy-agent/opa, unrelated to this change;go vet ./pkg/cosign/andgofmtare clean.AI-assisted (LLM used for drafting); the reproduction and runs above are mine.