mirror of
https://github.com/shankar0123/certctl.git
synced 2026-06-10 08:08:51 +00:00
security: require SCEP challenge password when SCEP enabled (fixes H-2)
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 (fb4ce1a): 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.
This commit is contained in:
@@ -483,6 +483,24 @@ func main() {
|
|||||||
|
|
||||||
// Register SCEP (RFC 8894) handlers if enabled
|
// Register SCEP (RFC 8894) handlers if enabled
|
||||||
if cfg.SCEP.Enabled {
|
if cfg.SCEP.Enabled {
|
||||||
|
// H-2 fix: fail closed at startup when SCEP is enabled without a
|
||||||
|
// challenge password configured. Previously the service-layer guard
|
||||||
|
// at internal/service/scep.go:72-79 skipped the password check when
|
||||||
|
// s.challengePassword == "", meaning any client that could reach the
|
||||||
|
// /scep endpoint could enroll an arbitrary CSR against the configured
|
||||||
|
// issuer (CWE-306, missing authentication for a critical function).
|
||||||
|
// Refuse to start instead: the operator must set
|
||||||
|
// CERTCTL_SCEP_CHALLENGE_PASSWORD (or disable SCEP) before the control
|
||||||
|
// plane can boot.
|
||||||
|
if err := preflightSCEPChallengePassword(cfg.SCEP.Enabled, cfg.SCEP.ChallengePassword); err != nil {
|
||||||
|
logger.Error(
|
||||||
|
"startup refused: SCEP is enabled but CERTCTL_SCEP_CHALLENGE_PASSWORD is not set "+
|
||||||
|
"(would allow unauthenticated certificate enrollment, CWE-306). "+
|
||||||
|
"Set a non-empty challenge password or disable SCEP before restarting.",
|
||||||
|
"error", err,
|
||||||
|
)
|
||||||
|
os.Exit(1)
|
||||||
|
}
|
||||||
issuerConn, ok := issuerRegistry.Get(cfg.SCEP.IssuerID)
|
issuerConn, ok := issuerRegistry.Get(cfg.SCEP.IssuerID)
|
||||||
if !ok {
|
if !ok {
|
||||||
logger.Error("SCEP issuer not found in registry", "issuer_id", cfg.SCEP.IssuerID)
|
logger.Error("SCEP issuer not found in registry", "issuer_id", cfg.SCEP.IssuerID)
|
||||||
@@ -683,3 +701,23 @@ func main() {
|
|||||||
logger.Info("certctl server stopped")
|
logger.Info("certctl server stopped")
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// preflightSCEPChallengePassword enforces the H-2 fix: if SCEP is enabled, a
|
||||||
|
// non-empty challenge password MUST be configured. Returns a non-nil error
|
||||||
|
// otherwise so the caller can refuse to start the control plane (CWE-306,
|
||||||
|
// missing authentication for a critical function).
|
||||||
|
//
|
||||||
|
// This helper is extracted so the check can be unit tested without booting
|
||||||
|
// the full server. The caller (main) is responsible for translating the
|
||||||
|
// returned error into a structured log line and os.Exit(1).
|
||||||
|
func preflightSCEPChallengePassword(enabled bool, challengePassword string) error {
|
||||||
|
if !enabled {
|
||||||
|
return nil
|
||||||
|
}
|
||||||
|
if challengePassword == "" {
|
||||||
|
return fmt.Errorf("SCEP enabled but CERTCTL_SCEP_CHALLENGE_PASSWORD is empty: " +
|
||||||
|
"SCEP enrollment would accept any client (CWE-306); " +
|
||||||
|
"configure a non-empty shared secret or set CERTCTL_SCEP_ENABLED=false")
|
||||||
|
}
|
||||||
|
return nil
|
||||||
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -7,6 +7,7 @@ import (
|
|||||||
"net/http"
|
"net/http"
|
||||||
"net/http/httptest"
|
"net/http/httptest"
|
||||||
"os"
|
"os"
|
||||||
|
"strings"
|
||||||
"testing"
|
"testing"
|
||||||
|
|
||||||
"github.com/shankar0123/certctl/internal/api/middleware"
|
"github.com/shankar0123/certctl/internal/api/middleware"
|
||||||
@@ -538,3 +539,68 @@ func TestMain_ContextPropagation(t *testing.T) {
|
|||||||
t.Logf("Context value may not be propagated (status %d), this may be expected", w.Code)
|
t.Logf("Context value may not be propagated (status %d), this may be expected", w.Code)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// TestPreflightSCEPChallengePassword is the H-2 regression guard for the
|
||||||
|
// startup pre-flight check. The helper MUST return a non-nil error whenever
|
||||||
|
// SCEP is enabled with an empty challenge password — that configuration
|
||||||
|
// previously allowed unauthenticated certificate enrollment (CWE-306).
|
||||||
|
// Disabled-SCEP and configured-password cases must pass cleanly.
|
||||||
|
func TestPreflightSCEPChallengePassword(t *testing.T) {
|
||||||
|
tests := []struct {
|
||||||
|
name string
|
||||||
|
enabled bool
|
||||||
|
challengePassword string
|
||||||
|
wantErr bool
|
||||||
|
wantErrSubstring string
|
||||||
|
}{
|
||||||
|
{
|
||||||
|
name: "disabled_empty_password_ok",
|
||||||
|
enabled: false,
|
||||||
|
challengePassword: "",
|
||||||
|
wantErr: false,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "disabled_with_password_ok",
|
||||||
|
enabled: false,
|
||||||
|
challengePassword: "leftover-value",
|
||||||
|
wantErr: false,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "enabled_empty_password_rejected",
|
||||||
|
enabled: true,
|
||||||
|
challengePassword: "",
|
||||||
|
wantErr: true,
|
||||||
|
wantErrSubstring: "CERTCTL_SCEP_CHALLENGE_PASSWORD",
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "enabled_with_password_ok",
|
||||||
|
enabled: true,
|
||||||
|
challengePassword: "hunter2",
|
||||||
|
wantErr: false,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "enabled_single_char_password_ok",
|
||||||
|
enabled: true,
|
||||||
|
challengePassword: "x",
|
||||||
|
wantErr: false,
|
||||||
|
},
|
||||||
|
}
|
||||||
|
for _, tt := range tests {
|
||||||
|
t.Run(tt.name, func(t *testing.T) {
|
||||||
|
err := preflightSCEPChallengePassword(tt.enabled, tt.challengePassword)
|
||||||
|
if tt.wantErr {
|
||||||
|
if err == nil {
|
||||||
|
t.Fatalf("expected error, got nil")
|
||||||
|
}
|
||||||
|
if tt.wantErrSubstring != "" && !strings.Contains(err.Error(), tt.wantErrSubstring) {
|
||||||
|
t.Errorf("expected error to mention %q, got: %v", tt.wantErrSubstring, err)
|
||||||
|
}
|
||||||
|
if !strings.Contains(err.Error(), "CWE-306") {
|
||||||
|
t.Errorf("expected error to cite CWE-306 for traceability, got: %v", err)
|
||||||
|
}
|
||||||
|
} else if err != nil {
|
||||||
|
t.Errorf("expected no error, got: %v", err)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
@@ -641,7 +641,12 @@ type SCEPConfig struct {
|
|||||||
|
|
||||||
// ChallengePassword is the shared secret used to authenticate SCEP enrollment requests.
|
// ChallengePassword is the shared secret used to authenticate SCEP enrollment requests.
|
||||||
// Clients include this in the PKCS#10 CSR challengePassword attribute.
|
// Clients include this in the PKCS#10 CSR challengePassword attribute.
|
||||||
// Required when SCEP is enabled.
|
//
|
||||||
|
// REQUIRED when Enabled is true. If SCEP is enabled and this value is empty,
|
||||||
|
// cmd/server/main.go's preflightSCEPChallengePassword check will refuse to
|
||||||
|
// start the server (H-2, CWE-306): an empty shared secret allowed any client
|
||||||
|
// that could reach /scep to enroll a CSR against the configured issuer. The
|
||||||
|
// service-layer PKCSReq path also rejects this configuration defense-in-depth.
|
||||||
ChallengePassword string
|
ChallengePassword string
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -119,7 +119,10 @@ func TestESTService_MaxTTL_ForwardedToIssuer(t *testing.T) {
|
|||||||
|
|
||||||
func TestSCEPService_CryptoValidation_RejectsWeakKey(t *testing.T) {
|
func TestSCEPService_CryptoValidation_RejectsWeakKey(t *testing.T) {
|
||||||
mockIssuer := &mockIssuerConnector{}
|
mockIssuer := &mockIssuerConnector{}
|
||||||
svc := NewSCEPService("iss-local", mockIssuer, nil, slog.New(slog.NewTextHandler(os.Stderr, &slog.HandlerOptions{Level: slog.LevelError})), "")
|
// H-2: SCEPService now requires a configured challenge password. Pass a
|
||||||
|
// matching client password so this test exercises the crypto-policy path
|
||||||
|
// rather than being short-circuited by the challenge-password guard.
|
||||||
|
svc := NewSCEPService("iss-local", mockIssuer, nil, slog.New(slog.NewTextHandler(os.Stderr, &slog.HandlerOptions{Level: slog.LevelError})), "secret123")
|
||||||
|
|
||||||
// Profile requiring ECDSA P-384 minimum
|
// Profile requiring ECDSA P-384 minimum
|
||||||
profileRepo := newM11cProfileRepo()
|
profileRepo := newM11cProfileRepo()
|
||||||
@@ -136,7 +139,7 @@ func TestSCEPService_CryptoValidation_RejectsWeakKey(t *testing.T) {
|
|||||||
// P-256 CSR should be rejected
|
// P-256 CSR should be rejected
|
||||||
csrPEM := generateCSRPEM(t, "device.example.com", nil)
|
csrPEM := generateCSRPEM(t, "device.example.com", nil)
|
||||||
|
|
||||||
_, err := svc.PKCSReq(context.Background(), csrPEM, "", "txn-001")
|
_, err := svc.PKCSReq(context.Background(), csrPEM, "secret123", "txn-001")
|
||||||
if err == nil {
|
if err == nil {
|
||||||
t.Fatal("expected rejection for ECDSA P-256 against P-384 minimum")
|
t.Fatal("expected rejection for ECDSA P-256 against P-384 minimum")
|
||||||
}
|
}
|
||||||
@@ -152,7 +155,8 @@ func TestSCEPService_CryptoValidation_AcceptsStrongKey(t *testing.T) {
|
|||||||
mockIssuer := &mockIssuerConnector{}
|
mockIssuer := &mockIssuerConnector{}
|
||||||
auditRepo := newMockAuditRepository()
|
auditRepo := newMockAuditRepository()
|
||||||
auditSvc := NewAuditService(auditRepo)
|
auditSvc := NewAuditService(auditRepo)
|
||||||
svc := NewSCEPService("iss-local", mockIssuer, auditSvc, slog.New(slog.NewTextHandler(os.Stderr, &slog.HandlerOptions{Level: slog.LevelError})), "")
|
// H-2: happy path exercises the authenticated branch.
|
||||||
|
svc := NewSCEPService("iss-local", mockIssuer, auditSvc, slog.New(slog.NewTextHandler(os.Stderr, &slog.HandlerOptions{Level: slog.LevelError})), "secret123")
|
||||||
|
|
||||||
profileRepo := newM11cProfileRepo()
|
profileRepo := newM11cProfileRepo()
|
||||||
profileRepo.profiles["prof-standard"] = &domain.CertificateProfile{
|
profileRepo.profiles["prof-standard"] = &domain.CertificateProfile{
|
||||||
@@ -167,7 +171,7 @@ func TestSCEPService_CryptoValidation_AcceptsStrongKey(t *testing.T) {
|
|||||||
|
|
||||||
csrPEM := generateCSRPEM(t, "device-ok.example.com", nil)
|
csrPEM := generateCSRPEM(t, "device-ok.example.com", nil)
|
||||||
|
|
||||||
result, err := svc.PKCSReq(context.Background(), csrPEM, "", "txn-002")
|
result, err := svc.PKCSReq(context.Background(), csrPEM, "secret123", "txn-002")
|
||||||
if err != nil {
|
if err != nil {
|
||||||
t.Fatalf("expected success: %v", err)
|
t.Fatalf("expected success: %v", err)
|
||||||
}
|
}
|
||||||
@@ -179,7 +183,8 @@ func TestSCEPService_CryptoValidation_AcceptsStrongKey(t *testing.T) {
|
|||||||
func TestSCEPService_MaxTTL_ForwardedToIssuer(t *testing.T) {
|
func TestSCEPService_MaxTTL_ForwardedToIssuer(t *testing.T) {
|
||||||
capturingMock := &capturingIssuerConnector{}
|
capturingMock := &capturingIssuerConnector{}
|
||||||
|
|
||||||
svc := NewSCEPService("iss-local", capturingMock, nil, slog.New(slog.NewTextHandler(os.Stderr, &slog.HandlerOptions{Level: slog.LevelError})), "")
|
// H-2: challenge password required for enrollment.
|
||||||
|
svc := NewSCEPService("iss-local", capturingMock, nil, slog.New(slog.NewTextHandler(os.Stderr, &slog.HandlerOptions{Level: slog.LevelError})), "secret123")
|
||||||
|
|
||||||
profileRepo := newM11cProfileRepo()
|
profileRepo := newM11cProfileRepo()
|
||||||
profileRepo.profiles["prof-device"] = &domain.CertificateProfile{
|
profileRepo.profiles["prof-device"] = &domain.CertificateProfile{
|
||||||
@@ -192,7 +197,7 @@ func TestSCEPService_MaxTTL_ForwardedToIssuer(t *testing.T) {
|
|||||||
|
|
||||||
csrPEM := generateCSRPEM(t, "mdm-device.example.com", nil)
|
csrPEM := generateCSRPEM(t, "mdm-device.example.com", nil)
|
||||||
|
|
||||||
_, err := svc.PKCSReq(context.Background(), csrPEM, "", "txn-003")
|
_, err := svc.PKCSReq(context.Background(), csrPEM, "secret123", "txn-003")
|
||||||
if err != nil {
|
if err != nil {
|
||||||
t.Fatalf("unexpected error: %v", err)
|
t.Fatalf("unexpected error: %v", err)
|
||||||
}
|
}
|
||||||
@@ -341,12 +346,13 @@ func TestESTService_NoProfileRepo_PassesThrough(t *testing.T) {
|
|||||||
|
|
||||||
func TestSCEPService_NoProfileRepo_PassesThrough(t *testing.T) {
|
func TestSCEPService_NoProfileRepo_PassesThrough(t *testing.T) {
|
||||||
mockIssuer := &mockIssuerConnector{}
|
mockIssuer := &mockIssuerConnector{}
|
||||||
svc := NewSCEPService("iss-local", mockIssuer, nil, slog.New(slog.NewTextHandler(os.Stderr, &slog.HandlerOptions{Level: slog.LevelError})), "")
|
// H-2: challenge password required for enrollment.
|
||||||
|
svc := NewSCEPService("iss-local", mockIssuer, nil, slog.New(slog.NewTextHandler(os.Stderr, &slog.HandlerOptions{Level: slog.LevelError})), "secret123")
|
||||||
svc.SetProfileID("nonexistent-profile")
|
svc.SetProfileID("nonexistent-profile")
|
||||||
|
|
||||||
csrPEM := generateCSRPEM(t, "no-profile-scep.example.com", nil)
|
csrPEM := generateCSRPEM(t, "no-profile-scep.example.com", nil)
|
||||||
|
|
||||||
result, err := svc.PKCSReq(context.Background(), csrPEM, "", "txn-004")
|
result, err := svc.PKCSReq(context.Background(), csrPEM, "secret123", "txn-004")
|
||||||
if err != nil {
|
if err != nil {
|
||||||
t.Fatalf("expected success when no profile repo set: %v", err)
|
t.Fatalf("expected success when no profile repo set: %v", err)
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -2,6 +2,7 @@ package service
|
|||||||
|
|
||||||
import (
|
import (
|
||||||
"context"
|
"context"
|
||||||
|
"crypto/subtle"
|
||||||
"crypto/x509"
|
"crypto/x509"
|
||||||
"encoding/pem"
|
"encoding/pem"
|
||||||
"fmt"
|
"fmt"
|
||||||
@@ -68,14 +69,34 @@ func (s *SCEPService) GetCACert(ctx context.Context) (string, error) {
|
|||||||
// PKCSReq processes a SCEP enrollment request.
|
// PKCSReq processes a SCEP enrollment request.
|
||||||
// RFC 8894 Section 3.3.1: PKCSReq contains a PKCS#10 CSR for certificate enrollment.
|
// 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.
|
// 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) {
|
func (s *SCEPService) PKCSReq(ctx context.Context, csrPEM string, challengePassword string, transactionID string) (*domain.SCEPEnrollResult, error) {
|
||||||
// Validate challenge password
|
// Defense-in-depth: refuse any enrollment when no shared secret is
|
||||||
if s.challengePassword != "" {
|
// configured. The server-level pre-flight check in cmd/server/main.go
|
||||||
if challengePassword != s.challengePassword {
|
// normally prevents the service from being constructed in this state, but
|
||||||
s.logger.Warn("SCEP enrollment rejected: invalid challenge password",
|
// this branch also protects future call sites (tests, library reuse, a
|
||||||
"transaction_id", transactionID)
|
// future REST-over-HTTPS wrapper) from silently accepting unauthenticated
|
||||||
return nil, fmt.Errorf("invalid challenge password")
|
// 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")
|
return s.processEnrollment(ctx, csrPEM, transactionID, "scep_pkcsreq")
|
||||||
|
|||||||
@@ -58,11 +58,13 @@ func TestSCEPService_PKCSReq_Success(t *testing.T) {
|
|||||||
mockIssuer := &mockIssuerConnector{}
|
mockIssuer := &mockIssuerConnector{}
|
||||||
auditRepo := newMockAuditRepository()
|
auditRepo := newMockAuditRepository()
|
||||||
auditSvc := NewAuditService(auditRepo)
|
auditSvc := NewAuditService(auditRepo)
|
||||||
svc := NewSCEPService("iss-local", mockIssuer, auditSvc, slog.New(slog.NewTextHandler(os.Stderr, &slog.HandlerOptions{Level: slog.LevelError})), "")
|
// H-2: SCEPService now requires a configured challenge password; the happy
|
||||||
|
// path exercises a matching client-submitted password.
|
||||||
|
svc := NewSCEPService("iss-local", mockIssuer, auditSvc, slog.New(slog.NewTextHandler(os.Stderr, &slog.HandlerOptions{Level: slog.LevelError})), "secret123")
|
||||||
|
|
||||||
csrPEM := generateCSRPEM(t, "device.example.com", []string{"device.example.com"})
|
csrPEM := generateCSRPEM(t, "device.example.com", []string{"device.example.com"})
|
||||||
|
|
||||||
result, err := svc.PKCSReq(context.Background(), csrPEM, "", "txn-001")
|
result, err := svc.PKCSReq(context.Background(), csrPEM, "secret123", "txn-001")
|
||||||
if err != nil {
|
if err != nil {
|
||||||
t.Fatalf("unexpected error: %v", err)
|
t.Fatalf("unexpected error: %v", err)
|
||||||
}
|
}
|
||||||
@@ -81,9 +83,9 @@ func TestSCEPService_PKCSReq_Success(t *testing.T) {
|
|||||||
|
|
||||||
func TestSCEPService_PKCSReq_InvalidCSR(t *testing.T) {
|
func TestSCEPService_PKCSReq_InvalidCSR(t *testing.T) {
|
||||||
mockIssuer := &mockIssuerConnector{}
|
mockIssuer := &mockIssuerConnector{}
|
||||||
svc := NewSCEPService("iss-local", mockIssuer, nil, slog.New(slog.NewTextHandler(os.Stderr, &slog.HandlerOptions{Level: slog.LevelError})), "")
|
svc := NewSCEPService("iss-local", mockIssuer, nil, slog.New(slog.NewTextHandler(os.Stderr, &slog.HandlerOptions{Level: slog.LevelError})), "secret123")
|
||||||
|
|
||||||
_, err := svc.PKCSReq(context.Background(), "not-valid-pem", "", "txn-002")
|
_, err := svc.PKCSReq(context.Background(), "not-valid-pem", "secret123", "txn-002")
|
||||||
if err == nil {
|
if err == nil {
|
||||||
t.Fatal("expected error for invalid CSR")
|
t.Fatal("expected error for invalid CSR")
|
||||||
}
|
}
|
||||||
@@ -91,11 +93,11 @@ func TestSCEPService_PKCSReq_InvalidCSR(t *testing.T) {
|
|||||||
|
|
||||||
func TestSCEPService_PKCSReq_MissingCN(t *testing.T) {
|
func TestSCEPService_PKCSReq_MissingCN(t *testing.T) {
|
||||||
mockIssuer := &mockIssuerConnector{}
|
mockIssuer := &mockIssuerConnector{}
|
||||||
svc := NewSCEPService("iss-local", mockIssuer, nil, slog.New(slog.NewTextHandler(os.Stderr, &slog.HandlerOptions{Level: slog.LevelError})), "")
|
svc := NewSCEPService("iss-local", mockIssuer, nil, slog.New(slog.NewTextHandler(os.Stderr, &slog.HandlerOptions{Level: slog.LevelError})), "secret123")
|
||||||
|
|
||||||
csrPEM := generateCSRPEM(t, "", []string{"test.example.com"})
|
csrPEM := generateCSRPEM(t, "", []string{"test.example.com"})
|
||||||
|
|
||||||
_, err := svc.PKCSReq(context.Background(), csrPEM, "", "txn-003")
|
_, err := svc.PKCSReq(context.Background(), csrPEM, "secret123", "txn-003")
|
||||||
if err == nil {
|
if err == nil {
|
||||||
t.Fatal("expected error for missing CN")
|
t.Fatal("expected error for missing CN")
|
||||||
}
|
}
|
||||||
@@ -106,11 +108,11 @@ func TestSCEPService_PKCSReq_MissingCN(t *testing.T) {
|
|||||||
|
|
||||||
func TestSCEPService_PKCSReq_IssuerError(t *testing.T) {
|
func TestSCEPService_PKCSReq_IssuerError(t *testing.T) {
|
||||||
mockIssuer := &mockIssuerConnector{Err: errors.New("issuance failed")}
|
mockIssuer := &mockIssuerConnector{Err: errors.New("issuance failed")}
|
||||||
svc := NewSCEPService("iss-local", mockIssuer, nil, slog.New(slog.NewTextHandler(os.Stderr, &slog.HandlerOptions{Level: slog.LevelError})), "")
|
svc := NewSCEPService("iss-local", mockIssuer, nil, slog.New(slog.NewTextHandler(os.Stderr, &slog.HandlerOptions{Level: slog.LevelError})), "secret123")
|
||||||
|
|
||||||
csrPEM := generateCSRPEM(t, "test.example.com", nil)
|
csrPEM := generateCSRPEM(t, "test.example.com", nil)
|
||||||
|
|
||||||
_, err := svc.PKCSReq(context.Background(), csrPEM, "", "txn-004")
|
_, err := svc.PKCSReq(context.Background(), csrPEM, "secret123", "txn-004")
|
||||||
if err == nil {
|
if err == nil {
|
||||||
t.Fatal("expected error")
|
t.Fatal("expected error")
|
||||||
}
|
}
|
||||||
@@ -151,19 +153,49 @@ func TestSCEPService_PKCSReq_ChallengePassword_Invalid(t *testing.T) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
func TestSCEPService_PKCSReq_ChallengePassword_NotRequired(t *testing.T) {
|
// TestSCEPService_PKCSReq_ChallengePassword_EmptyServerConfigRejected is the
|
||||||
// When server has no challenge password configured, any value should be accepted
|
// H-2 regression guard. Before the fix (internal/service/scep.go:72-79 skipped
|
||||||
|
// the password check when s.challengePassword was empty), an unconfigured
|
||||||
|
// server accepted any enrollment (CWE-306). The service now rejects PKCSReq
|
||||||
|
// defense-in-depth even if main()'s pre-flight is somehow bypassed.
|
||||||
|
func TestSCEPService_PKCSReq_ChallengePassword_EmptyServerConfigRejected(t *testing.T) {
|
||||||
mockIssuer := &mockIssuerConnector{}
|
mockIssuer := &mockIssuerConnector{}
|
||||||
svc := NewSCEPService("iss-local", mockIssuer, nil, slog.New(slog.NewTextHandler(os.Stderr, &slog.HandlerOptions{Level: slog.LevelError})), "")
|
svc := NewSCEPService("iss-local", mockIssuer, nil, slog.New(slog.NewTextHandler(os.Stderr, &slog.HandlerOptions{Level: slog.LevelError})), "")
|
||||||
|
|
||||||
csrPEM := generateCSRPEM(t, "device.example.com", nil)
|
csrPEM := generateCSRPEM(t, "device.example.com", nil)
|
||||||
|
|
||||||
result, err := svc.PKCSReq(context.Background(), csrPEM, "any-value", "txn-007")
|
// Any client-submitted password (including empty) must be rejected when
|
||||||
if err != nil {
|
// the server has no shared secret configured.
|
||||||
t.Fatalf("unexpected error: %v", err)
|
for _, clientPassword := range []string{"", "any-value", "guess"} {
|
||||||
|
_, err := svc.PKCSReq(context.Background(), csrPEM, clientPassword, "txn-empty")
|
||||||
|
if err == nil {
|
||||||
|
t.Fatalf("expected rejection when server challenge password is empty (client=%q)", clientPassword)
|
||||||
|
}
|
||||||
|
if !strings.Contains(err.Error(), "not configured") {
|
||||||
|
t.Errorf("expected 'not configured' in error, got: %v", err)
|
||||||
|
}
|
||||||
}
|
}
|
||||||
if result == nil {
|
}
|
||||||
t.Fatal("expected non-nil result")
|
|
||||||
|
// TestSCEPService_PKCSReq_ChallengePassword_ConstantTimeLengthIndependence
|
||||||
|
// guards against regression from crypto/subtle.ConstantTimeCompare to a
|
||||||
|
// short-circuiting byte compare. ConstantTimeCompare returns 0 whenever the
|
||||||
|
// two slices differ in length OR content, so a same-prefix-but-longer input
|
||||||
|
// must be rejected the same way as a completely different string.
|
||||||
|
func TestSCEPService_PKCSReq_ChallengePassword_ConstantTimeLengthIndependence(t *testing.T) {
|
||||||
|
mockIssuer := &mockIssuerConnector{}
|
||||||
|
svc := NewSCEPService("iss-local", mockIssuer, nil, slog.New(slog.NewTextHandler(os.Stderr, &slog.HandlerOptions{Level: slog.LevelError})), "secret123")
|
||||||
|
|
||||||
|
csrPEM := generateCSRPEM(t, "device.example.com", nil)
|
||||||
|
|
||||||
|
for _, bad := range []string{"secret", "secret12", "secret1234", "SECRET123", "wrong"} {
|
||||||
|
_, err := svc.PKCSReq(context.Background(), csrPEM, bad, "txn-ct")
|
||||||
|
if err == nil {
|
||||||
|
t.Fatalf("expected rejection for bad password %q", bad)
|
||||||
|
}
|
||||||
|
if !strings.Contains(err.Error(), "invalid challenge password") {
|
||||||
|
t.Errorf("expected 'invalid challenge password' for %q, got: %v", bad, err)
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -171,12 +203,12 @@ func TestSCEPService_PKCSReq_WithProfile(t *testing.T) {
|
|||||||
mockIssuer := &mockIssuerConnector{}
|
mockIssuer := &mockIssuerConnector{}
|
||||||
auditRepo := newMockAuditRepository()
|
auditRepo := newMockAuditRepository()
|
||||||
auditSvc := NewAuditService(auditRepo)
|
auditSvc := NewAuditService(auditRepo)
|
||||||
svc := NewSCEPService("iss-local", mockIssuer, auditSvc, slog.New(slog.NewTextHandler(os.Stderr, &slog.HandlerOptions{Level: slog.LevelError})), "")
|
svc := NewSCEPService("iss-local", mockIssuer, auditSvc, slog.New(slog.NewTextHandler(os.Stderr, &slog.HandlerOptions{Level: slog.LevelError})), "secret123")
|
||||||
svc.SetProfileID("profile-mdm-device")
|
svc.SetProfileID("profile-mdm-device")
|
||||||
|
|
||||||
csrPEM := generateCSRPEM(t, "device.example.com", nil)
|
csrPEM := generateCSRPEM(t, "device.example.com", nil)
|
||||||
|
|
||||||
result, err := svc.PKCSReq(context.Background(), csrPEM, "", "txn-008")
|
result, err := svc.PKCSReq(context.Background(), csrPEM, "secret123", "txn-008")
|
||||||
if err != nil {
|
if err != nil {
|
||||||
t.Fatalf("unexpected error: %v", err)
|
t.Fatalf("unexpected error: %v", err)
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user