mirror of
https://github.com/shankar0123/certctl.git
synced 2026-06-07 16:01:30 +00:00
shankar0123
1c099071d1
Closes 3 findings (1 High + 1 Medium + 1 Low) from
/Users/shankar/Desktop/cowork/comprehensive-audit-2026-04-25/.
Bundle 4 hardens the only attack surface reachable by an anonymous network
attacker in certctl: the unauthenticated EST + SCEP enrollment endpoints.
Findings closed:
- H-004 (High): Hand-rolled ASN.1 parser had no fuzz target.
The audit's original framing pointed at internal/pkcs7/, but recon
confirmed that package is an ASN.1 ENCODER (BuildCertsOnlyPKCS7,
ASN1Wrap*, ASN1EncodeLength) — not a parser. The actual hand-rolled
PKCS#7 PARSING reachable via anonymous network is in
internal/api/handler/scep.go::extractCSRFromPKCS7 +
parseSignedDataForCSR. Added native go fuzz targets:
* internal/api/handler/scep_fuzz_test.go::FuzzExtractCSRFromPKCS7
* internal/api/handler/scep_fuzz_test.go::FuzzParseSignedDataForCSR
* internal/pkcs7/pkcs7_fuzz_test.go::FuzzPEMToDERChain (defense-in-depth)
* internal/pkcs7/pkcs7_fuzz_test.go::FuzzASN1EncodeLength (defense-in-depth)
Local 15s fuzz session: 150k execs on FuzzExtractCSRFromPKCS7,
937k on FuzzPEMToDERChain, 925k on FuzzASN1EncodeLength — zero panics.
- M-021 (Medium): EST TLS-Unique channel binding (RFC 7030 §3.2.3).
Added internal/api/handler/est.go::verifyESTTransport — defense-in-depth
TLS pre-conditions (r.TLS != nil; HandshakeComplete; TLS ≥ 1.2).
The full §3.2.3 channel binding only applies when EST mTLS is in use;
certctl does not currently support EST mTLS, so the §3.2.3 requirement
is moot today. RFC 9266 (TLS 1.3 tls-exporter) and EST mTLS are
documented as deferred follow-ups in the verifyESTTransport doc comment.
- L-005 (Low): EST/SCEP issuer-binding fail-loud at startup.
Pre-Bundle-4 cmd/server/main.go validated that CERTCTL_EST_ISSUER_ID and
CERTCTL_SCEP_ISSUER_ID existed in the registry but did NOT validate the
issuer TYPE could emit a CA cert. An operator binding EST to an ACME
issuer (whose GetCACertPEM returns explicit error) booted successfully
and only failed at first /est/cacerts request. Post-Bundle-4: new
preflightEnrollmentIssuer helper calls GetCACertPEM(ctx) at startup
with a 10s timeout. Failure logs the connector error + the candidate
issuer types and os.Exit(1).
Tests added/modified:
- internal/api/handler/est_transport_test.go (new) — 5 verifyESTTransport
table cases covering plaintext-rejected, incomplete-handshake-rejected,
TLS 1.0 rejected, TLS 1.2/1.3 accepted
- cmd/server/preflight_test.go (new) — TestPreflightEnrollmentIssuer
covering nil-connector, error-from-issuer, empty-PEM, valid cases
- internal/api/handler/est_handler_test.go (modified) — 7 POST sites
now stamp r.TLS to satisfy the new transport pre-condition
- internal/integration/negative_test.go (modified) — setupTestServer
wraps the test handler with a fake-TLS-state injector so the EST
handler receives r.TLS != nil; production paths still rely on the
real TLS listener
Threat model reference: TB-11 (EST/SCEP client ↔ Server) per
cowork/comprehensive-audit-2026-04-25/threat-model.md.
Standards: RFC 7030 §3.2.3, RFC 8894 §3, RFC 5652, RFC 9266 (deferred).
80 lines
2.6 KiB
Go
80 lines
2.6 KiB
Go
package pkcs7
|
|
|
|
import (
|
|
"testing"
|
|
)
|
|
|
|
// FuzzPEMToDERChain exercises the PEM-to-DER converter in
|
|
// internal/pkcs7/pkcs7.go::PEMToDERChain. Bundle-4 / H-004 (defense in depth):
|
|
// this function isn't directly network-reachable today (callers pass
|
|
// trusted PEM from issuer connectors), but it operates on byte input
|
|
// that traces back to upstream CA responses; a malicious-CA scenario
|
|
// could feed crafted PEM. Fuzz to ensure no panic, no allocation
|
|
// amplification.
|
|
//
|
|
// Run locally:
|
|
//
|
|
// go test -run='^$' -fuzz=FuzzPEMToDERChain -fuzztime=10m ./internal/pkcs7/
|
|
func FuzzPEMToDERChain(f *testing.F) {
|
|
seeds := []string{
|
|
// Empty input.
|
|
"",
|
|
// Minimal valid PEM (an empty CERTIFICATE block — not a real cert).
|
|
"-----BEGIN CERTIFICATE-----\nAA==\n-----END CERTIFICATE-----\n",
|
|
// Truncated header.
|
|
"-----BEGIN CERTIFICATE",
|
|
// Multiple BEGIN, no END.
|
|
"-----BEGIN CERTIFICATE-----\n-----BEGIN CERTIFICATE-----\n",
|
|
// Body with binary garbage.
|
|
"-----BEGIN CERTIFICATE-----\n\x00\xff\xfe\x80\n-----END CERTIFICATE-----\n",
|
|
}
|
|
for _, seed := range seeds {
|
|
f.Add(seed)
|
|
}
|
|
|
|
f.Fuzz(func(t *testing.T, data string) {
|
|
// Bound input — same rationale as the SCEP fuzz.
|
|
if len(data) > 1<<20 {
|
|
return
|
|
}
|
|
_, _ = PEMToDERChain(data)
|
|
})
|
|
}
|
|
|
|
// FuzzASN1EncodeLength exercises the hand-rolled BER length encoder.
|
|
// Bundle-4 / H-004: the encoder is used when building PKCS#7 envelopes
|
|
// returned to EST/SCEP clients, so an attacker cannot directly feed
|
|
// untrusted bytes into it — but a future caller that did would be
|
|
// vulnerable to integer overflow / unbounded allocation. Fuzz the
|
|
// length values to confirm the encoder handles boundary conditions
|
|
// (negative, zero, MaxInt, etc.).
|
|
//
|
|
// Run locally:
|
|
//
|
|
// go test -run='^$' -fuzz=FuzzASN1EncodeLength -fuzztime=2m ./internal/pkcs7/
|
|
func FuzzASN1EncodeLength(f *testing.F) {
|
|
seeds := []int{0, 1, 127, 128, 255, 256, 65535, 65536, 1 << 20, 1 << 30, -1}
|
|
for _, seed := range seeds {
|
|
f.Add(seed)
|
|
}
|
|
|
|
f.Fuzz(func(t *testing.T, length int) {
|
|
// Bound input — fuzz-generated lengths in the billions cause
|
|
// the encoder to allocate huge byte slices. Real PKCS#7 envelopes
|
|
// from certctl never exceed a few MB.
|
|
if length > 1<<24 || length < 0 {
|
|
return
|
|
}
|
|
out := ASN1EncodeLength(length)
|
|
// Sanity: encoder always returns at least one byte.
|
|
if len(out) == 0 {
|
|
t.Fatalf("ASN1EncodeLength(%d) returned empty slice", length)
|
|
}
|
|
// Sanity: encoder never returns more than 5 bytes for int input
|
|
// (1 length-of-length byte + 4 bytes for a 32-bit length).
|
|
if len(out) > 5 {
|
|
t.Fatalf("ASN1EncodeLength(%d) returned %d bytes; expected ≤5", length, len(out))
|
|
}
|
|
})
|
|
}
|