Conversation
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. Code Coverage & BenchmarksFor details see: https://corecheck.dev/bitcoin/bitcoin/pulls/36293. ReviewsSee the guideline and AI policy for information on the review process. |
|
i get revoked, but are we sure we want to exclude expired keys too? will this break verifying old releases or current releases in the future? i’ve noticed GPG keys lapse (in general, not only in this project) reasonably frequently and this doesn’t mean they’re compromised or bad. |
I think that is the correct thing to do for revoked keys. What to do with expired keys has been a debate since ~forever. How about categorizing sigs from expired keys separately and adding a flag that allows the user to treat them as good? |
I could see that as reasonable. With that optionality a user of "current software, today" should ideally see majority non-expired keys verified by default. Somebody targeting older software (perhaps with automated build tools) could set the optional flag to include expired keys. We should prioritise "verifying current software today" in the most robust way possible. That's the less interesting/important use-case vs "historical verifications" where keys may have expired... |
Extract parser construction so tests can use the command-line defaults and types. Partition signatures by trust in one pass to make lifecycle eligibility a separate decision in the same loop. SigData uses identity equality and its fields do not change here, so membership in good_trusted is equivalent to the trust predicate. Both changes preserve behavior.
Record that expired and revoked GPG signatures satisfy the binary-verification threshold alone or alongside an active signature, and remain in the returned good-signature set when mixed with an active signature. Establish the shared parser, environment and quorum cases up front, including the currently unrecognized expired-key option.
Expired-key signatures can satisfy the binary-verification threshold even when no active key signs. Exclude them before trust classification and warn before checking the threshold, including when verification fails. Exclude keys that GnuPG reports as expired, even if the signature was made before expiration. Co-authored-by: Rob Hamilton <[email protected]>
Revoked-key signatures still count toward the threshold after excluding expired keys. Require an empty lifecycle status so revoked signatures are also warned about and excluded from both the quorum and the returned good-signature groups. Co-authored-by: Rob Hamilton <[email protected]>
Keep expired signatures in a separate result group, including when the quorum fails. Both commands expose it as `expired_sigs` in successful JSON output. Distinguish EXPIRED SIGNATURE and REVOKED SIGNATURE warnings. The threshold still excludes expired signatures. Assert that the remaining inactive status is revoked before reporting it.
Expiration alone does not imply compromise, and excluding expired keys can prevent verification of historical releases. Add `--allow-expired` and its `BINVERIFY_ALLOW_EXPIRED` environment default to count their signatures explicitly while retaining the strict default and excluding revoked keys. Expired signatures remain separately reported even when they count toward the threshold.
Document the default exclusion of expired and revoked signatures, the expired-key opt-in for historical releases, and separate expired-signature reporting in successful JSON output.
f28d32f to
b956ee6
Compare
|
Added |
Problem: The binary verifier counts signatures from expired and revoked keys toward
--min-good-sigs.Enough of these signatures can satisfy the threshold without any active signing key, and they are returned as good signatures.
Fix: Warn about expired and revoked signatures and exclude them from threshold counting by default.
For historical verification,
--allow-expiredlets expired signatures count toward the threshold. Revoked signatures remain excluded.Expired signatures are reported separately, including in the
expired_sigsJSON field.The decision uses GnuPG’s status from the local keyring, even if the signature was created while the key was active.
This was found and disclosed responsibly by the Red Team 🟥.