mirror of
https://github.com/shankar0123/certctl.git
synced 2026-06-07 23: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).
101 lines
2.9 KiB
Go
101 lines
2.9 KiB
Go
package main
|
|
|
|
import (
|
|
"context"
|
|
"strings"
|
|
"testing"
|
|
|
|
"github.com/shankar0123/certctl/internal/service"
|
|
)
|
|
|
|
// fakeIssuerConn implements service.IssuerConnector enough for preflight tests.
|
|
type fakeIssuerConn struct {
|
|
caCertPEM string
|
|
caCertErr error
|
|
}
|
|
|
|
func (f *fakeIssuerConn) IssueCertificate(ctx context.Context, commonName string, sans []string, csrPEM string, ekus []string, maxTTLSeconds int) (*service.IssuanceResult, error) {
|
|
return nil, nil
|
|
}
|
|
func (f *fakeIssuerConn) RenewCertificate(ctx context.Context, commonName string, sans []string, csrPEM string, ekus []string, maxTTLSeconds int) (*service.IssuanceResult, error) {
|
|
return nil, nil
|
|
}
|
|
func (f *fakeIssuerConn) RevokeCertificate(ctx context.Context, serial string, reason string) error {
|
|
return nil
|
|
}
|
|
func (f *fakeIssuerConn) GenerateCRL(ctx context.Context, revokedCerts []service.CRLEntry) ([]byte, error) {
|
|
return nil, nil
|
|
}
|
|
func (f *fakeIssuerConn) SignOCSPResponse(ctx context.Context, req service.OCSPSignRequest) ([]byte, error) {
|
|
return nil, nil
|
|
}
|
|
func (f *fakeIssuerConn) GetCACertPEM(ctx context.Context) (string, error) {
|
|
return f.caCertPEM, f.caCertErr
|
|
}
|
|
func (f *fakeIssuerConn) GetRenewalInfo(ctx context.Context, certPEM string) (*service.RenewalInfoResult, error) {
|
|
return nil, nil
|
|
}
|
|
|
|
// TestPreflightEnrollmentIssuer covers Bundle-4 / L-005 startup validation
|
|
// for EST/SCEP issuer binding.
|
|
func TestPreflightEnrollmentIssuer(t *testing.T) {
|
|
cases := []struct {
|
|
name string
|
|
issuer service.IssuerConnector
|
|
wantErr bool
|
|
errContains string
|
|
}{
|
|
{
|
|
name: "nil_connector_fails",
|
|
issuer: nil,
|
|
wantErr: true,
|
|
errContains: "connector is nil",
|
|
},
|
|
{
|
|
name: "issuer_returns_error_fails",
|
|
issuer: &fakeIssuerConn{
|
|
caCertErr: errStub("ACME issuers do not provide a static CA certificate"),
|
|
},
|
|
wantErr: true,
|
|
errContains: "cannot serve CA certificate",
|
|
},
|
|
{
|
|
name: "issuer_returns_empty_pem_fails",
|
|
issuer: &fakeIssuerConn{
|
|
caCertPEM: "",
|
|
caCertErr: nil,
|
|
},
|
|
wantErr: true,
|
|
errContains: "empty PEM",
|
|
},
|
|
{
|
|
name: "issuer_returns_valid_pem_succeeds",
|
|
issuer: &fakeIssuerConn{
|
|
caCertPEM: "-----BEGIN CERTIFICATE-----\nMIIB...\n-----END CERTIFICATE-----",
|
|
caCertErr: nil,
|
|
},
|
|
wantErr: false,
|
|
},
|
|
}
|
|
for _, tc := range cases {
|
|
t.Run(tc.name, func(t *testing.T) {
|
|
err := preflightEnrollmentIssuer(context.Background(), "EST", "iss-test", tc.issuer)
|
|
if tc.wantErr && err == nil {
|
|
t.Fatalf("expected error, got nil")
|
|
}
|
|
if !tc.wantErr && err != nil {
|
|
t.Fatalf("unexpected error: %v", err)
|
|
}
|
|
if tc.wantErr && tc.errContains != "" && !strings.Contains(err.Error(), tc.errContains) {
|
|
t.Fatalf("error %q missing substring %q", err.Error(), tc.errContains)
|
|
}
|
|
})
|
|
}
|
|
}
|
|
|
|
// errStub is a tiny error wrapper so test cases can use string literals
|
|
// without importing fmt in every test struct entry.
|
|
type errStub string
|
|
|
|
func (e errStub) Error() string { return string(e) }
|