mirror of
https://github.com/shankar0123/certctl.git
synced 2026-06-07 22:51:30 +00:00
b813660c74
Problem (CWE-306 Missing Authentication for Critical Function):
internal/service/scep.go PKCSReq skipped the shared-secret check when
s.challengePassword was empty. An unconfigured-but-enabled SCEP server
accepted any unauthenticated client reaching /scep and issued a
certificate against the configured issuer for any CSR with a valid
signature. No audit trail distinguished authenticated from
unauthenticated enrollments. This matches the two-layer fail-closed
pattern already used for C-2 (f549a7a): reject at startup AND reject
at the service boundary.
Fix (two layers, defense-in-depth):
Layer 1 — startup pre-flight in cmd/server/main.go:
preflightSCEPChallengePassword returns a non-nil error when SCEP is
enabled and CERTCTL_SCEP_CHALLENGE_PASSWORD is empty. main logs and
os.Exit(1)s before the SCEP service is constructed. Disabled SCEP is
unaffected. The helper is unit-testable in isolation.
Layer 2 — service-layer rejection in internal/service/scep.go:
PKCSReq refuses enrollment when s.challengePassword == "" even though
main already blocks this state — protects future call sites (tests,
library reuse, a REST-over-HTTPS wrapper). When a secret is
configured, the comparison now uses crypto/subtle.ConstantTimeCompare
so response time does not leak the configured secret through a
short-circuiting byte compare.
Files:
- cmd/server/main.go: preflightSCEPChallengePassword helper; call site
inside the `if cfg.SCEP.Enabled` block before issuer lookup; fatal
slog error references CWE-306 and names the env var so operators can
diagnose the startup failure without reading code.
- cmd/server/main_test.go: TestPreflightSCEPChallengePassword with five
table-driven subtests (disabled empty, disabled set, enabled empty
rejected, enabled set, single-char boundary). The enabled-empty case
asserts the error string contains both CERTCTL_SCEP_CHALLENGE_PASSWORD
and CWE-306 so the log message remains actionable.
- internal/config/config.go: SCEPConfig.ChallengePassword godoc now
states the field is REQUIRED when SCEP.Enabled and cross-references
preflightSCEPChallengePassword.
- internal/service/scep.go: imports crypto/subtle; PKCSReq rewritten
with the two-layer check; comment block cites H-2 / CWE-306 and the
constant-time rationale.
- internal/service/scep_test.go: existing tests that relied on the
vulnerable empty-password path now configure a secret on both sides.
TestSCEPService_PKCSReq_ChallengePassword_NotRequired is replaced by
TestSCEPService_PKCSReq_ChallengePassword_EmptyServerConfigRejected
which iterates ["", "any-value", "guess"] against an unconfigured
server and asserts "not configured" in the error. A new
TestSCEPService_PKCSReq_ChallengePassword_ConstantTimeLengthIndependence
exercises same-prefix-longer and wrong-case inputs to guard against a
regression from ConstantTimeCompare to a short-circuiting byte compare.
- internal/service/m11c_crypto_enforcement_test.go: four tests
(RejectsWeakKey, AcceptsStrongKey, MaxTTL_ForwardedToIssuer,
NoProfileRepo_PassesThrough) constructed NewSCEPService with an empty
challenge password and exercised PKCSReq through the now-rejected
vulnerable path. All four now configure "secret123" on both sides with
an inline H-2 comment; the crypto/MaxTTL/profile behavior they assert
is unchanged.
Wire-format / behavioral invariants preserved:
- RFC 8894 SCEP handler is untouched (internal/api/handler/scep.go and
internal/pkcs7/*): GetCACaps/GetCACert responses, PKIOperation request
parsing, and the PKCS#7 certs-only response format are byte-identical.
- RFC 7030 EST handler is untouched
(internal/api/handler/est.go + internal/pkcs7/*).
- Revocation idempotency composite key (H-1, migration 000012) untouched.
- AES-256-GCM config encryption (C-2) untouched.
- CRL DER bytes and OCSP response bytes unchanged.
Verification:
- go build ./... silent success
- go vet ./... silent success
- go test -race -count=1 ./internal/service/ ./cmd/server/
./internal/api/handler/ ./internal/integration/ all OK
- Coverage with comfortable headroom over CI gates:
service 67.8% (gate 55%)
handler 79.0% (gate 60%)
domain 92.7% (gate 40%)
middleware 80.0% (gate 30%)
cmd/server 1.6% (preflightSCEPChallengePassword: 100%)
internal/service/scep.go PKCSReq statement coverage: 100%.
- rg sweeps: no `s.challengePassword != ""` remains;
no `challengePassword != s.challengePassword` remains.
Operational note: operators with SCEP enabled but no challenge password
set will see a fatal startup error and a log line citing
CERTCTL_SCEP_CHALLENGE_PASSWORD and CWE-306 after upgrading. This is the
intended fail-closed behavior. Fix by either setting the env var to a
non-empty shared secret or setting CERTCTL_SCEP_ENABLED=false.
Audit report: certctl-audit-report.md (revision 5) logs this under
H-2 Resolution Log.
213 lines
7.6 KiB
Go
213 lines
7.6 KiB
Go
package service
|
|
|
|
import (
|
|
"context"
|
|
"crypto/subtle"
|
|
"crypto/x509"
|
|
"encoding/pem"
|
|
"fmt"
|
|
"log/slog"
|
|
"strings"
|
|
|
|
"github.com/shankar0123/certctl/internal/domain"
|
|
"github.com/shankar0123/certctl/internal/repository"
|
|
)
|
|
|
|
// SCEPService implements the SCEP (RFC 8894) enrollment protocol.
|
|
// It delegates certificate operations to an existing IssuerConnector and records
|
|
// enrollment events in the audit trail.
|
|
type SCEPService struct {
|
|
issuer IssuerConnector
|
|
issuerID string
|
|
auditService *AuditService
|
|
logger *slog.Logger
|
|
profileID string // optional: constrain enrollments to a specific profile
|
|
profileRepo repository.CertificateProfileRepository
|
|
challengePassword string // shared secret for enrollment authentication
|
|
}
|
|
|
|
// NewSCEPService creates a new SCEPService for the given issuer connector.
|
|
func NewSCEPService(issuerID string, issuer IssuerConnector, auditService *AuditService, logger *slog.Logger, challengePassword string) *SCEPService {
|
|
return &SCEPService{
|
|
issuer: issuer,
|
|
issuerID: issuerID,
|
|
auditService: auditService,
|
|
logger: logger,
|
|
challengePassword: challengePassword,
|
|
}
|
|
}
|
|
|
|
// SetProfileID constrains SCEP enrollments to a specific certificate profile.
|
|
func (s *SCEPService) SetProfileID(profileID string) {
|
|
s.profileID = profileID
|
|
}
|
|
|
|
// SetProfileRepo sets the profile repository for crypto policy enforcement during enrollment.
|
|
func (s *SCEPService) SetProfileRepo(repo repository.CertificateProfileRepository) {
|
|
s.profileRepo = repo
|
|
}
|
|
|
|
// GetCACaps returns the capabilities of this SCEP server.
|
|
// RFC 8894 Section 3.5.2: GetCACaps returns a list of capabilities, one per line.
|
|
func (s *SCEPService) GetCACaps(ctx context.Context) string {
|
|
return "POSTPKIOperation\nSHA-256\nAES\nSCEPStandard\n"
|
|
}
|
|
|
|
// GetCACert returns the PEM-encoded CA certificate chain for this SCEP server.
|
|
// RFC 8894 Section 3.5.1: GetCACert distributes the CA certificate(s).
|
|
func (s *SCEPService) GetCACert(ctx context.Context) (string, error) {
|
|
caPEM, err := s.issuer.GetCACertPEM(ctx)
|
|
if err != nil {
|
|
return "", fmt.Errorf("failed to get CA certificates from issuer %s: %w", s.issuerID, err)
|
|
}
|
|
if caPEM == "" {
|
|
return "", fmt.Errorf("issuer %s does not provide CA certificates for SCEP", s.issuerID)
|
|
}
|
|
return caPEM, nil
|
|
}
|
|
|
|
// PKCSReq processes a SCEP enrollment request.
|
|
// RFC 8894 Section 3.3.1: PKCSReq contains a PKCS#10 CSR for certificate enrollment.
|
|
// The CSR PEM and challenge password are extracted by the handler from the PKCS#7 envelope.
|
|
//
|
|
// H-2 fix (CWE-306): the previous implementation skipped the shared-secret
|
|
// check entirely when s.challengePassword was empty, meaning any unauthenticated
|
|
// client that could reach /scep could enroll a CSR against the configured
|
|
// issuer. Reject that configuration defense-in-depth even though main() already
|
|
// refuses to start in the same state (see preflightSCEPChallengePassword). The
|
|
// non-empty branch now uses crypto/subtle.ConstantTimeCompare to avoid leaking
|
|
// the shared secret through a response-time side channel.
|
|
func (s *SCEPService) PKCSReq(ctx context.Context, csrPEM string, challengePassword string, transactionID string) (*domain.SCEPEnrollResult, error) {
|
|
// Defense-in-depth: refuse any enrollment when no shared secret is
|
|
// configured. The server-level pre-flight check in cmd/server/main.go
|
|
// normally prevents the service from being constructed in this state, but
|
|
// this branch also protects future call sites (tests, library reuse, a
|
|
// future REST-over-HTTPS wrapper) from silently accepting unauthenticated
|
|
// CSRs.
|
|
if s.challengePassword == "" {
|
|
s.logger.Warn("SCEP enrollment rejected: server has no challenge password configured",
|
|
"transaction_id", transactionID)
|
|
return nil, fmt.Errorf("SCEP challenge password not configured on server")
|
|
}
|
|
// Constant-time compare avoids leaking the configured secret through
|
|
// response-time variance. ConstantTimeCompare returns 1 only when both
|
|
// slices have equal length AND equal content; a mismatched-length input
|
|
// still takes the same path as a content mismatch.
|
|
if subtle.ConstantTimeCompare([]byte(challengePassword), []byte(s.challengePassword)) != 1 {
|
|
s.logger.Warn("SCEP enrollment rejected: invalid challenge password",
|
|
"transaction_id", transactionID)
|
|
return nil, fmt.Errorf("invalid challenge password")
|
|
}
|
|
|
|
return s.processEnrollment(ctx, csrPEM, transactionID, "scep_pkcsreq")
|
|
}
|
|
|
|
// processEnrollment handles the common enrollment logic.
|
|
func (s *SCEPService) processEnrollment(ctx context.Context, csrPEM string, transactionID string, auditAction string) (*domain.SCEPEnrollResult, error) {
|
|
// Parse the CSR to extract CN and SANs
|
|
block, _ := pem.Decode([]byte(csrPEM))
|
|
if block == nil {
|
|
return nil, fmt.Errorf("invalid CSR PEM")
|
|
}
|
|
|
|
csr, err := x509.ParseCertificateRequest(block.Bytes)
|
|
if err != nil {
|
|
return nil, fmt.Errorf("failed to parse CSR: %w", err)
|
|
}
|
|
|
|
if err := csr.CheckSignature(); err != nil {
|
|
return nil, fmt.Errorf("CSR signature verification failed: %w", err)
|
|
}
|
|
|
|
commonName := csr.Subject.CommonName
|
|
if commonName == "" {
|
|
return nil, fmt.Errorf("CSR must include a Common Name")
|
|
}
|
|
|
|
// Collect SANs
|
|
var sans []string
|
|
for _, dns := range csr.DNSNames {
|
|
sans = append(sans, dns)
|
|
}
|
|
for _, ip := range csr.IPAddresses {
|
|
sans = append(sans, ip.String())
|
|
}
|
|
for _, email := range csr.EmailAddresses {
|
|
sans = append(sans, email)
|
|
}
|
|
for _, uri := range csr.URIs {
|
|
sans = append(sans, uri.String())
|
|
}
|
|
|
|
// Validate CSR key algorithm/size against profile (crypto policy enforcement)
|
|
var profile *domain.CertificateProfile
|
|
var ekus []string
|
|
if s.profileID != "" && s.profileRepo != nil {
|
|
if p, profileErr := s.profileRepo.Get(ctx, s.profileID); profileErr == nil && p != nil {
|
|
profile = p
|
|
ekus = profile.AllowedEKUs
|
|
}
|
|
}
|
|
if _, csrErr := ValidateCSRAgainstProfile(csrPEM, profile); csrErr != nil {
|
|
s.logger.Error("SCEP enrollment rejected: crypto policy violation",
|
|
"action", auditAction,
|
|
"common_name", commonName,
|
|
"transaction_id", transactionID,
|
|
"error", csrErr)
|
|
return nil, fmt.Errorf("SCEP enrollment rejected: %w", csrErr)
|
|
}
|
|
|
|
s.logger.Info("SCEP enrollment request",
|
|
"action", auditAction,
|
|
"common_name", commonName,
|
|
"sans", strings.Join(sans, ","),
|
|
"transaction_id", transactionID,
|
|
"issuer", s.issuerID)
|
|
|
|
// Resolve MaxTTL from profile
|
|
var maxTTLSeconds int
|
|
if profile != nil {
|
|
maxTTLSeconds = profile.MaxTTLSeconds
|
|
}
|
|
|
|
// Issue the certificate via the configured issuer connector
|
|
// SCEP enrollments use profile EKUs if available, otherwise default (serverAuth + clientAuth fallback)
|
|
result, err := s.issuer.IssueCertificate(ctx, commonName, sans, csrPEM, ekus, maxTTLSeconds)
|
|
if err != nil {
|
|
s.logger.Error("SCEP enrollment failed",
|
|
"action", auditAction,
|
|
"common_name", commonName,
|
|
"transaction_id", transactionID,
|
|
"error", err)
|
|
return nil, fmt.Errorf("certificate issuance failed: %w", err)
|
|
}
|
|
|
|
// Audit the enrollment
|
|
if s.auditService != nil {
|
|
details := map[string]interface{}{
|
|
"common_name": commonName,
|
|
"sans": sans,
|
|
"issuer_id": s.issuerID,
|
|
"serial": result.Serial,
|
|
"transaction_id": transactionID,
|
|
"protocol": "SCEP",
|
|
}
|
|
if s.profileID != "" {
|
|
details["profile_id"] = s.profileID
|
|
}
|
|
_ = s.auditService.RecordEvent(ctx, "scep-client", "system", auditAction, "certificate", result.Serial, details)
|
|
}
|
|
|
|
s.logger.Info("SCEP enrollment successful",
|
|
"action", auditAction,
|
|
"common_name", commonName,
|
|
"serial", result.Serial,
|
|
"transaction_id", transactionID,
|
|
"not_after", result.NotAfter)
|
|
|
|
return &domain.SCEPEnrollResult{
|
|
CertPEM: result.CertPEM,
|
|
ChainPEM: result.ChainPEM,
|
|
}, nil
|
|
}
|