#36293 contrib: exclude inactive keys from the binary quorum by default

ReviewabilityReviewsAgreementSize
#36293 l0rinc contrib: exclude inactive keys from the binary quorum by defaultReady1Strong0 + 125 tests
  • Prevent release binary verification from accepting expired or revoked keys toward the signature quorum
  • Avoids reporting releases as verified when signatures come only from compromised or defunct keys
  • Allows opt-in inclusion of expired keys when verifying historical releases
  • Ready to review; all reviewer concerns have been addressed in pushed updates
  • Strong support after testing verification on releases 22.1 through 31.1 (willcl-ark)
  • Concerns about verifying older releases resolved by adding an opt-in expired key flag (sedited, willcl-ark)
  • Agreed to leave signature de-duplication to a follow-up (willcl-ark)
  • 0 lines added or modified outside tests
  • 125 lines added or modified in tests
  • 15 lines removed in total
  • 3 files, 5 commits
Build and CI#2 of 56
P1 路 bug fix
  • P1 after comparing with the other PRs here (assessed alone as P2): Raised to P1: correctness of release-signature verification is the category's top concern, and accepting revoked keys toward the quorum is a disclosed security defect on par with #35765's silent wrong-source builds
  • P2 because release verification could pass using revoked keys, weakening release integrity guarantees
  • Prevents false-positive verification runs when all provided signatures are compromised
Ranking notes
Tools and scripts#1 of 16
P1 路 bug fix
  • P1 after comparing with the other PRs here (assessed alone as P2): Raised to P1: release-binary verification tooling counting revoked/expired keys toward the quorum is a disclosed security defect in a tool downstream users rely on, the most consequential problem in this category.
  • P2 because verification tooling counted revoked and expired keys toward minimum required signatures
  • Fixes a responsibly disclosed flaw while retaining historical release verification via an explicit flag
Ranking notes
Reviewability: ReadyStalePaused
Agreement: StrongPositivePositive w/ caveatsNeutralMildDisputedBlockedCrickets
Reviews: current code-review ACKs, then (+stale ACKs) and -NACKs; greener = more ACKs.
Size: lines added or modified outside tests, then in tests; greener = smaller.

Goal

Excludes expired and revoked GPG signatures from counting toward the binary verification quorum in `contrib/verify-binaries/verify.py` by default. Adds an `--allow-expired` CLI flag and `BINVERIFY_ALLOW_EXPIRED` environment variable to permit expired keys when verifying older releases, while keeping revoked signatures unconditionally excluded. Adds parsing for `KEYREVOKED` status records, ensures revocation takes precedence over expiration, and reports expired signatures in a dedicated `expired_sigs` JSON field.

Problem: `contrib/verify-binaries/verify.py` treats signatures from expired or revoked keys as valid signatures satisfying `--min-good-sigs`. A verification run can therefore pass and report binaries as verified even if all supplied signatures come from compromised or defunct keys.

Evidence of importance:

  • PR description: 'This was found and disclosed responsibly by the Red Team 馃煡.'
  • PR description: '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.'

Risk of a mistake: low (tooling, tests, docs). The change is confined to the release binary verification script in contrib; a bug could cause binary verification to reject valid signatures or accept invalid ones, but cannot impact the core node.

Changed since the previous assessment: Added explicit handling for GnuPG's `KEYREVOKED` status tag to prioritize revocation over expiration when both are reported, extracted `build_parser()` to facilitate testing, and added unit tests mocking GnuPG outputs across multiple environment and argument combinations.

Reviewability

Ready

Nothing known that would invalidate a review now.

Waiting on (from the thread read):

  • nothing: all open items addressed and approved by reviewer

Agreement

Strong

substantive support, no open objection (willcl-ark)

Thread notes: l0rinc pushed updates addressing the test assertions and adding a README note about updating keys. willcl-ark tested the changes across multiple releases and provided an ACK.

Objections, suggestions, and questions:

ReviewerTypeHarm, request, or questionStatusMust beAuthor repliedQuote
willcl-ark (member)objection, approachexcluding expired keys by default could break verifying old releases or future verification of current releases when keys lapsefixed 路 fix pushed 路 author implemented --allow-expired flag as suggested by sedited and willcl-ark, who found the end result decentansweredyes2026-09-18: 'will this break verifying old releases or current releases in the future?'
Status from: 2026-09-23: willcl-ark: 'The end result looks decent'
sedited (member)suggestion, interfacecategorize sigs from expired keys separately and add a flag that allows the user to treat them as goodfixed 路 fix pushed 路 author added --allow-expired and reported expired sigs separatelyansweredyes2026-09-18: 'How about categorizing sigs from expired keys separately and adding a flag that allows the user to treat them as good?'
Status from: 2026-09-18: l0rinc: 'Added `--allow-expired` for historical verification, with expired signatures reported separately'
willcl-ark (member)suggestion, interfaceprompt or remind users that they should update keys periodically in local gpgfixed 路 fix pushed 路 author added README note on stale local keyrings and reviewer approvedansweredyes2026-09-24: 'I wonder if we should prompt/remind users that they should ideally update keys periodically.'
Status from: 2026-09-25: l0rinc: 'The latest push adds a short README note that stale local keyrings may miss revocations and keyserver updates can reveal requested keys'
willcl-ark (member)question, correctnesswhether the test should have an assertion for expired_sigs fieldsfixed 路 fix pushed 路 author added test assertions for expired_sigs and reviewer approvedansweredyes2026-09-24: 'Do you think the test should have an assertion for these `expired_sigs` fields?'
Status from: 2026-09-25: l0rinc: 'Sure, the `pub --json` test now checks that `expired_sigs` is a list, and the quorum test checks the expired signature鈥檚 full representation.'
willcl-ark (member)suggestion, correctnessde-duplicate signatures by key or fingerprint rather than summing lengthsaccepted 路 both agreed to leave deduplication for a follow-up PRansweredyes2026-09-24: 'should we be de-duplicating by key or fingerprint perhaps?'
Status from: 2026-09-24: willcl-ark: 'Anyway, probably for a followup...'

Support:

  • willcl-ark (member) (ACK, 2026-09-25) [tested]: verified recent releases (22.1 through 31.1) with and without --allow-expired; all passed default quorum with local keyring without exception

Participants:

  • sedited (member, neutral, 1 statements 2026-09-18 to 2026-09-18): suggested categorizing expired signatures separately and adding a flag to allow them
  • willcl-ark (member, support, 7 statements 2026-09-18 to 2026-09-25): ACKed after testing release verifications with and without --allow-expired

Files

140 lines under test/bench/ci.

  • contrib/verify-binaries/test.py +72/-0
  • contrib/verify-binaries/verify.py +38/-14
  • contrib/verify-binaries/README.md +15/-1

Card

The short text the ranking pass compares across PRs.

Excludes expired and revoked GPG signatures from counting toward the binary verification threshold in contrib/verify-binaries/verify.py by default. Previously, release binary verification could succeed using only expired or revoked keys, an issue disclosed responsibly by the Red Team. To accommodate historical releases signed by keys that have since expired, an --allow-expired flag allows expired keys to count toward the threshold while revoked keys remain strictly excluded. Expired signatures are emitted separately in JSON output, and revocation status is given precedence over expiration.

Data

dossier JSON 路 extract JSON 路 model ledger, generated 2026-10-05T01:16, confidence high, input hash 6d4d254e95a172af
thread read through 2026-09-25 (14 statements), head 034271d4e1
labels: Scripts and tools

Categories

Build and CI

P1 路 bug fix (assessed alone as P2; Raised to P1: correctness of release-signature verification is the category's top concern, and accepting revoked keys toward the quorum is a disclosed security defect on par with #35765's silent wrong-source builds)

Fixes a security flaw where expired and revoked GPG keys satisfied the release verification quorum. As reported by the Red Team disclosure, the verifier counted revoked keys toward `--min-good-sigs`, which could allow binaries to pass verification even if all signatures were from compromised keys.

Membership: Modifies `contrib/verify-binaries/`, the release-verification tooling under contrib that guards release integrity.

Factors: security/stability 2, bug 2, performance 0, user value 1, leverage 0

Tools and scripts

P1 路 bug fix (assessed alone as P2; Raised to P1: release-binary verification tooling counting revoked/expired keys toward the quorum is a disclosed security defect in a tool downstream users rely on, the most consequential problem in this category.)

Corrects key-validation logic in a critical contrib verification tool. The script previously counted expired and revoked keys as good signatures toward quorum; this change excludes inactive keys by default while adding `--allow-expired` for older releases.

Membership: Carries the Scripts and tools label and modifies `contrib/verify-binaries/verify.py` and its tests.

Factors: security/stability 2, bug 2, performance 0, user value 1, leverage 0