Commit 23adf10
feat: add OCSP and CRL revocation checking to connect command (#78)
* feat: add OCSP and CRL revocation checking to connect command
OCSP is checked automatically (best-effort) on every connect — if the
responder is unreachable or the cert has no OCSP URL, the field is
silently omitted. CRL checking is opt-in via --crl. Both revoked
statuses trigger exit code 2.
Also extracts FetchCRL into the library so the standalone crl command
and connect share the same fetching logic.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* fix: harden OCSP/CRL with SSRF validation, signature checks, and test consolidation
- Add ValidateAIAURL and CheckRedirect to OCSP and CRL HTTP clients
- Verify CRL signatures against issuer certificate
- Surface OCSP failures as "unavailable" status instead of silent nil
- Rename CRLCheckResult.URL to DistributionPoint (breaking)
- Remove redundant http.Client timeouts (context deadlines suffice)
- Consolidate connect tests from 1380 to 1015 lines (~26% reduction)
- Extract generateTestCA, generateTestLeafCert, startTLSServer helpers
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* fix: address PR review — dedup formatters, changelog refs
- Extract FormatOCSPLine and FormatCRLLine to eliminate duplication
between library FormatConnectResult and CLI formatConnectVerbose
- Add ([#78]) refs to all new CHANGELOG entries per CL-3
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* fix: surface OCSP skip reasons, add test improvements from adversarial review
- Show "OCSP: skipped (reason)" when no responder URL or no issuer in chain
instead of silently omitting OCSP from output
- Add Detail field to OCSPResult for skip/unavailable context
- Add OCSP "unknown" status test case
- Strengthen FormatOCSPResult assertions to check actual values, not just labels
- Add empty CRL test (zero revoked entries)
- Fix shared atomic counter in parallel OCSP test — create per-subtest servers
- Switch ocsp_test.go to generateTestCA/generateTestLeafCert helpers
- Merge TestCheckOCSP_NilInputs and TestCheckOCSP_NoOCSPURL into single
table-driven TestCheckOCSP_InvalidInputs
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* fix: add HTTP timeouts, CRL freshness check, and CS-5 input struct
- Add 10s HTTP client Timeout to OCSP and CRL fetchers (SEC-1)
- Reject expired CRLs (past NextUpdate) to prevent stale replay attacks
- Refactor checkLeafCRL to use input struct per CS-5 (>2 non-ctx args)
- Add TestConnectTLS_CRL_Expired test
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* fix: OCSP freshness check, improved output, and test hardening
Security:
- Reject expired OCSP responses (past NextUpdate) to prevent replay
of stale "good" responses over HTTP
- Propagate OCSP error details into "unavailable" Detail field
UX:
- OCSP "unavailable" now shows error reason instead of just URL
- OCSP "unknown" explains: "responder does not recognize this certificate"
- FormatCRLLine default case prints raw status instead of empty string
Tests:
- Fix misleading "no CRL distribution points" test (was testing "no issuer")
- Add actual CDP-absent test case exercising checkLeafCRL
- Add CRL "good" integration test (non-revoked leaf)
- Add OCSP serial number verification in integration test
- Add OCSP "unavailable with detail" and "unknown" format tests
- Consolidate crl_test.go: use generateTestCA, merge empty CRL into table
- Strengthen CRLInfoFromList assertions (CRL number, format content)
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* feat: add --no-ocsp flag, unify serial JSON key (CLI-4)
- Add --no-ocsp flag to connect for disabling automatic OCSP check
- Rename OCSPResult JSON field serial_number → serial (CLI-4: same
concept uses the same key everywhere — all other commands use serial)
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* feat: add --ocsp and --crl flags to verify command
Allow revocation checking from local certificate files via the verify
command, consolidating the validation path so most users only need one
command. Both flags are opt-in since they require network access.
Export CheckLeafCRL/CheckLeafCRLInput for reuse across commands.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* fix: surface OCSP skip reasons, add test improvements from adversarial review
Add FetchCRL, CheckLeafCRL, FormatCRLLine, and verify OCSP/CRL unit
tests. Improve revocation error messages to include revocation time
and reason. Clarify --ocsp/--crl flag help text about chain dependency.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* fix: AIA issuer fallback, SSRF regression, test consolidation from adversarial review
- Fix connect OCSP/CRL ignoring AIA-fetched issuer: fall back to
VerifiedChains[0][1] when PeerCertificates has only the leaf
- Add RootCAs field to ConnectTLSInput for custom root pools
- Fix certkit crl rejecting private IPs: add FetchCRLInput struct with
AllowPrivateNetworks flag, bypass SSRF for user-provided URLs
- Fix verify --ocsp/--crl silently omitting results when chain fails:
now reports "skipped" with explanation
- Consolidate 4 CRL integration tests and 4 verify revocation tests
into table-driven tests (T-12)
- Merge standalone FetchCRL/CheckLeafCRL tests into parent tables (T-14)
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* fix: prefer verified chain for issuer resolution in connect
When a server sends duplicate leaf certificates in the TLS chain
(e.g., [leaf, leaf, intermediate]), PeerCertificates[1] is the
duplicate leaf — not the actual issuer. This caused both OCSP and
CRL checks to fail with signature verification errors.
Swap priority to prefer VerifiedChains[0][1] (cryptographically
validated by x509.Verify) over PeerCertificates[1] (raw server-sent).
Fall back to PeerCertificates only when chain verification failed.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* docs: add --no-ocsp to connect flags table, fix FetchCRL SSRF doc
Address review comments:
- Add --no-ocsp flag to README connect flags table
- Clarify FetchCRL doc comment: SSRF blocks literal private/loopback
IPs only (hostnames are allowed through)
- Update PR description to match actual behavior (shows "unavailable"
not "silently skipped")
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* test: add OCSP coverage and detail assertion to duplicate-leaf test
From adversarial test correctness review:
- Enable OCSP in TestConnectTLS_CRL_DuplicateLeafInChain (was
disabled) — verifies issuer resolution fix works for both OCSP
and CRL, not just CRL
- Assert CRL detail contains the revoked serial number
- Add OCSP responder mock signed by the intermediate CA
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* refactor: unify OCSP/CRL URL field names for CLI-4 consistency
Rename OCSPResult.ResponderURL and CRLCheckResult.DistributionPoint
to URL (JSON: "url") on both types. The parent object (ocsp/crl)
provides the context, so the field name should be consistent across
revocation check types per CLI-4.
Breaking: JSON field "responder_url" → "url" in ocsp object,
"distribution_point" → "url" in crl object.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* docs: add --ocsp and --crl to verify flags table in README
Address review comment: verify command's --ocsp and --crl flags
were missing from the README flags table.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* fix: guard PeerCertificates[0] access in revocation check block
Add length check before accessing PeerCertificates[0] for revocation
checks. The diagnostics and verification blocks above are guarded,
but the revocation block was not — would panic on an empty peer chain
from a partially-completed handshake.
Also fix CHANGELOG link ordering ([#78] was out of numeric sequence).
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* fix: address 5 review findings from PR #78 code review
- Security: remove PeerCertificates[1] fallback for issuer resolution
in revocation checks — only use cryptographically verified issuer from
VerifiedChains to prevent forged OCSP/CRL responses from malicious
servers
- CS-5: checkVerifyOCSP now takes CheckOCSPInput struct instead of 3
positional arguments
- ERR-5: startTLSServer test helper now logs Handshake() and Close()
errors with slog.Debug instead of discarding them
- DRY: extract FormatOCSPStatusLine and FormatCRLStatusLine shared
helpers — formatVerifyOCSP/formatVerifyCRL now delegate to root
package instead of duplicating switch logic
- Fix FormatCRLLine/formatVerifyCRL missing "skipped" case — Detail
was dropped when status was "skipped"
Tests updated to provide RootCAs for chain verification, matching the
new security requirement that revocation checks only run against
verified issuers.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* fix: address 3 review findings from PR #78 review round 7
- Fix data race in TestCheckLeafCRL — generate CRL bytes before
starting the test HTTP server to avoid unsynchronized closure
access (CC-3)
- Fix CheckLeafCRL panic on nil Leaf/Issuer — return "unavailable"
result with clear detail instead of dereferencing nil
- Fix verify help text claiming "Exits with code 2 if revoked" —
actually exits 2 for any verification error
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>1 parent 91d8926 commit 23adf10
15 files changed
Lines changed: 1986 additions & 344 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
7 | 7 | | |
8 | 8 | | |
9 | 9 | | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
10 | 50 | | |
11 | 51 | | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
12 | 56 | | |
13 | 57 | | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
14 | 64 | | |
15 | 65 | | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
16 | 80 | | |
17 | 81 | | |
18 | 82 | | |
| |||
68 | 132 | | |
69 | 133 | | |
70 | 134 | | |
71 | | - | |
72 | | - | |
73 | | - | |
74 | | - | |
75 | | - | |
76 | | - | |
77 | | - | |
78 | | - | |
79 | | - | |
80 | | - | |
81 | | - | |
82 | | - | |
83 | | - | |
84 | | - | |
85 | | - | |
86 | | - | |
87 | | - | |
88 | | - | |
89 | | - | |
90 | | - | |
91 | | - | |
92 | | - | |
93 | | - | |
94 | | - | |
95 | | - | |
96 | | - | |
97 | | - | |
98 | | - | |
99 | | - | |
100 | | - | |
101 | | - | |
102 | | - | |
103 | 135 | | |
104 | 136 | | |
| 137 | + | |
| 138 | + | |
| 139 | + | |
| 140 | + | |
| 141 | + | |
| 142 | + | |
| 143 | + | |
105 | 144 | | |
106 | 145 | | |
107 | 146 | | |
| |||
726 | 765 | | |
727 | 766 | | |
728 | 767 | | |
729 | | - | |
| 768 | + | |
730 | 769 | | |
731 | 770 | | |
732 | 771 | | |
| |||
788 | 827 | | |
789 | 828 | | |
790 | 829 | | |
| 830 | + | |
791 | 831 | | |
792 | 832 | | |
793 | 833 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
168 | 168 | | |
169 | 169 | | |
170 | 170 | | |
171 | | - | |
| 171 | + | |
| 172 | + | |
| 173 | + | |
| 174 | + | |
| 175 | + | |
| 176 | + | |
| 177 | + | |
| 178 | + | |
| 179 | + | |
172 | 180 | | |
173 | 181 | | |
174 | 182 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
138 | 138 | | |
139 | 139 | | |
140 | 140 | | |
| 141 | + | |
| 142 | + | |
141 | 143 | | |
142 | 144 | | |
143 | 145 | | |
144 | | - | |
| 146 | + | |
145 | 147 | | |
146 | 148 | | |
147 | 149 | | |
148 | | - | |
149 | | - | |
150 | | - | |
151 | | - | |
| 150 | + | |
| 151 | + | |
| 152 | + | |
| 153 | + | |
| 154 | + | |
| 155 | + | |
152 | 156 | | |
153 | | - | |
| 157 | + | |
154 | 158 | | |
155 | 159 | | |
156 | 160 | | |
| |||
0 commit comments