mirror of
https://github.com/shankar0123/certctl.git
synced 2026-06-07 20:11:31 +00:00
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).
95 lines
3.0 KiB
Go
95 lines
3.0 KiB
Go
package handler
|
|
|
|
import (
|
|
"encoding/hex"
|
|
"testing"
|
|
)
|
|
|
|
// FuzzExtractCSRFromPKCS7 exercises the SCEP PKCS#7 envelope parser at
|
|
// internal/api/handler/scep.go::extractCSRFromPKCS7. Bundle-4 / H-004:
|
|
// this parser is reachable by an anonymous network attacker via
|
|
// POST /scep?operation=PKIOperation. It calls into hand-written ASN.1
|
|
// unmarshaling logic in parseSignedDataForCSR (which uses encoding/asn1
|
|
// from stdlib but with manual structure layouts). Any panic, OOM, or
|
|
// allocation amplification surfaces here.
|
|
//
|
|
// Run locally:
|
|
//
|
|
// go test -run='^$' -fuzz=FuzzExtractCSRFromPKCS7 -fuzztime=10m \
|
|
// ./internal/api/handler/
|
|
//
|
|
// CI gate (Bundle-4 added in .github/workflows/ci.yml): runs at
|
|
// -fuzztime=2m on every PR. The full 10m runs are reserved for the
|
|
// scheduled overnight job to keep PR latency reasonable.
|
|
func FuzzExtractCSRFromPKCS7(f *testing.F) {
|
|
// Seed corpus: a few well-formed envelopes + a few deliberately
|
|
// malformed ones to give the fuzzer mutational starting points.
|
|
seeds := [][]byte{
|
|
// Minimal PKCS#7 ContentInfo OID + empty content.
|
|
mustHex("3013060B2A864886F70D010907020100"),
|
|
// Empty input — fuzzer should return error, not panic.
|
|
{},
|
|
// Single zero byte — parses as ASN.1 boolean false.
|
|
{0x00},
|
|
// Truncated SEQUENCE with bogus length.
|
|
{0x30, 0x81, 0xff},
|
|
// Recursive SEQUENCE wrapping (fuzzer + parser depth check).
|
|
{0x30, 0x80, 0x30, 0x80, 0x30, 0x80, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00},
|
|
}
|
|
for _, seed := range seeds {
|
|
f.Add(seed)
|
|
}
|
|
|
|
f.Fuzz(func(t *testing.T, data []byte) {
|
|
// Bound input size — the fuzzer otherwise tends to chase
|
|
// "find" rewards via 100MB inputs that aren't representative.
|
|
// Real network input is bounded by MaxBytesReader (1MB default).
|
|
if len(data) > 1<<20 {
|
|
return
|
|
}
|
|
// extractCSRFromPKCS7 returns (csrDER, challengePassword, transactionID, error).
|
|
// We don't care about the return values — we care that it doesn't
|
|
// panic, OOM, or allocate unbounded memory. The Go test harness
|
|
// reports panics as test failures.
|
|
_, _, _, _ = extractCSRFromPKCS7(data)
|
|
})
|
|
}
|
|
|
|
// FuzzParseSignedDataForCSR exercises the inner SignedData parser
|
|
// directly (the function extractCSRFromPKCS7 calls). Same scope as
|
|
// FuzzExtractCSRFromPKCS7 but narrower; helps the fuzzer find paths
|
|
// that the wrapping function's fallbacks would otherwise mask.
|
|
//
|
|
// Run locally:
|
|
//
|
|
// go test -run='^$' -fuzz=FuzzParseSignedDataForCSR -fuzztime=10m \
|
|
// ./internal/api/handler/
|
|
func FuzzParseSignedDataForCSR(f *testing.F) {
|
|
seeds := [][]byte{
|
|
mustHex("3013060B2A864886F70D010907020100"),
|
|
{},
|
|
{0x00},
|
|
{0x30, 0x80},
|
|
}
|
|
for _, seed := range seeds {
|
|
f.Add(seed)
|
|
}
|
|
|
|
f.Fuzz(func(t *testing.T, data []byte) {
|
|
if len(data) > 1<<20 {
|
|
return
|
|
}
|
|
_, _ = parseSignedDataForCSR(data)
|
|
})
|
|
}
|
|
|
|
// mustHex decodes a hex string for fuzz seeds. Panics on malformed
|
|
// hex — only used at test setup with hard-coded constants.
|
|
func mustHex(s string) []byte {
|
|
b, err := hex.DecodeString(s)
|
|
if err != nil {
|
|
panic(err)
|
|
}
|
|
return b
|
|
}
|