mirror of
https://github.com/shankar0123/certctl.git
synced 2026-06-07 13:51:36 +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).
78 lines
2.0 KiB
Go
78 lines
2.0 KiB
Go
package handler
|
|
|
|
import (
|
|
"crypto/tls"
|
|
"net/http"
|
|
"strings"
|
|
"testing"
|
|
)
|
|
|
|
// TestVerifyESTTransport_Bundle4_M021 covers the EST transport precondition
|
|
// added in Bundle-4 / M-021. See verifyESTTransport doc comment in est.go for
|
|
// scope rationale (RFC 7030 §3.2.3 channel binding is moot without EST mTLS;
|
|
// what we DO enforce is TLS pre-conditions).
|
|
func TestVerifyESTTransport_Bundle4_M021(t *testing.T) {
|
|
cases := []struct {
|
|
name string
|
|
req *http.Request
|
|
wantErr bool
|
|
errContains string
|
|
}{
|
|
{
|
|
name: "plaintext_request_rejected",
|
|
req: &http.Request{TLS: nil},
|
|
wantErr: true,
|
|
errContains: "plaintext",
|
|
},
|
|
{
|
|
name: "incomplete_handshake_rejected",
|
|
req: &http.Request{TLS: &tls.ConnectionState{
|
|
HandshakeComplete: false,
|
|
Version: tls.VersionTLS13,
|
|
}},
|
|
wantErr: true,
|
|
errContains: "handshake",
|
|
},
|
|
{
|
|
name: "tls10_rejected",
|
|
req: &http.Request{TLS: &tls.ConnectionState{
|
|
HandshakeComplete: true,
|
|
Version: tls.VersionTLS10,
|
|
}},
|
|
wantErr: true,
|
|
errContains: "TLS 1.2 minimum",
|
|
},
|
|
{
|
|
name: "tls12_accepted",
|
|
req: &http.Request{TLS: &tls.ConnectionState{
|
|
HandshakeComplete: true,
|
|
Version: tls.VersionTLS12,
|
|
}},
|
|
wantErr: false,
|
|
},
|
|
{
|
|
name: "tls13_accepted",
|
|
req: &http.Request{TLS: &tls.ConnectionState{
|
|
HandshakeComplete: true,
|
|
Version: tls.VersionTLS13,
|
|
}},
|
|
wantErr: false,
|
|
},
|
|
}
|
|
|
|
for _, tc := range cases {
|
|
t.Run(tc.name, func(t *testing.T) {
|
|
err := verifyESTTransport(tc.req)
|
|
if tc.wantErr && err == nil {
|
|
t.Fatalf("verifyESTTransport(%s): expected error, got nil", tc.name)
|
|
}
|
|
if !tc.wantErr && err != nil {
|
|
t.Fatalf("verifyESTTransport(%s): unexpected error: %v", tc.name, err)
|
|
}
|
|
if tc.wantErr && tc.errContains != "" && !strings.Contains(err.Error(), tc.errContains) {
|
|
t.Fatalf("verifyESTTransport(%s): error %q missing substring %q", tc.name, err.Error(), tc.errContains)
|
|
}
|
|
})
|
|
}
|
|
}
|