diff --git a/cmd/server/preflight_test.go b/cmd/server/preflight_test.go index 8f32ccd..503382a 100644 --- a/cmd/server/preflight_test.go +++ b/cmd/server/preflight_test.go @@ -14,10 +14,10 @@ type fakeIssuerConn struct { caCertErr error } -func (f *fakeIssuerConn) IssueCertificate(ctx context.Context, commonName string, sans []string, csrPEM string, ekus []string, maxTTLSeconds int) (*service.IssuanceResult, error) { +func (f *fakeIssuerConn) IssueCertificate(ctx context.Context, commonName string, sans []string, csrPEM string, ekus []string, maxTTLSeconds int, mustStaple bool) (*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) { +func (f *fakeIssuerConn) RenewCertificate(ctx context.Context, commonName string, sans []string, csrPEM string, ekus []string, maxTTLSeconds int, mustStaple bool) (*service.IssuanceResult, error) { return nil, nil } func (f *fakeIssuerConn) RevokeCertificate(ctx context.Context, serial string, reason string) error { diff --git a/internal/service/agent.go b/internal/service/agent.go index b68d89b..5f7a725 100644 --- a/internal/service/agent.go +++ b/internal/service/agent.go @@ -194,13 +194,18 @@ func (s *AgentService) SubmitCSR(ctx context.Context, agentID string, certID str return fmt.Errorf("CSR validation failed: %w", csrErr) } - // Resolve MaxTTL from profile - var maxTTLSeconds int + // Resolve MaxTTL + must-staple from profile. + // SCEP RFC 8894 + Intune master bundle Phase 5.6 follow-up. + var ( + maxTTLSeconds int + mustStaple bool + ) if profile != nil { maxTTLSeconds = profile.MaxTTLSeconds + mustStaple = profile.MustStaple } - result, err := connector.IssueCertificate(ctx, cert.CommonName, cert.SANs, string(csrPEM), ekus, maxTTLSeconds) + result, err := connector.IssueCertificate(ctx, cert.CommonName, cert.SANs, string(csrPEM), ekus, maxTTLSeconds, mustStaple) if err != nil { return fmt.Errorf("issuer signing failed: %w", err) } diff --git a/internal/service/est.go b/internal/service/est.go index 3d9a706..b24aee2 100644 --- a/internal/service/est.go +++ b/internal/service/est.go @@ -139,15 +139,22 @@ func (s *ESTService) processEnrollment(ctx context.Context, csrPEM string, audit "sans", strings.Join(sans, ","), "issuer", s.issuerID) - // Resolve MaxTTL from profile - var maxTTLSeconds int + // Resolve MaxTTL + must-staple from profile. + // SCEP RFC 8894 + Intune master bundle Phase 5.6 follow-up: thread + // profile.MustStaple through to the issuer so the local issuer can + // add the RFC 7633 id-pe-tlsfeature extension. + var ( + maxTTLSeconds int + mustStaple bool + ) if profile != nil { maxTTLSeconds = profile.MaxTTLSeconds + mustStaple = profile.MustStaple } // Issue the certificate via the configured issuer connector // EST enrollments use profile EKUs if available, otherwise default (serverAuth + clientAuth fallback) - result, err := s.issuer.IssueCertificate(ctx, commonName, sans, csrPEM, ekus, maxTTLSeconds) + result, err := s.issuer.IssueCertificate(ctx, commonName, sans, csrPEM, ekus, maxTTLSeconds, mustStaple) if err != nil { s.logger.Error("EST enrollment failed", "action", auditAction, diff --git a/internal/service/issuer_adapter.go b/internal/service/issuer_adapter.go index 5a63972..33df486 100644 --- a/internal/service/issuer_adapter.go +++ b/internal/service/issuer_adapter.go @@ -20,13 +20,19 @@ func NewIssuerConnectorAdapter(c issuer.Connector) IssuerConnector { // IssueCertificate delegates to the underlying connector's IssueCertificate method, // translating between service-layer and connector-layer types. -func (a *IssuerConnectorAdapter) IssueCertificate(ctx context.Context, commonName string, sans []string, csrPEM string, ekus []string, maxTTLSeconds int) (*IssuanceResult, error) { +// +// SCEP RFC 8894 + Intune master bundle Phase 5.6 follow-up: mustStaple flows +// through to the IssuanceRequest.MustStaple field. Only the local issuer +// honors it (RFC 7633 id-pe-tlsfeature extension); upstream connectors +// silently ignore the field. +func (a *IssuerConnectorAdapter) IssueCertificate(ctx context.Context, commonName string, sans []string, csrPEM string, ekus []string, maxTTLSeconds int, mustStaple bool) (*IssuanceResult, error) { result, err := a.connector.IssueCertificate(ctx, issuer.IssuanceRequest{ CommonName: commonName, SANs: sans, CSRPEM: csrPEM, EKUs: ekus, MaxTTLSeconds: maxTTLSeconds, + MustStaple: mustStaple, }) if err != nil { return nil, err @@ -42,13 +48,14 @@ func (a *IssuerConnectorAdapter) IssueCertificate(ctx context.Context, commonNam // RenewCertificate delegates to the underlying connector's RenewCertificate method, // translating between service-layer and connector-layer types. -func (a *IssuerConnectorAdapter) RenewCertificate(ctx context.Context, commonName string, sans []string, csrPEM string, ekus []string, maxTTLSeconds int) (*IssuanceResult, error) { +func (a *IssuerConnectorAdapter) RenewCertificate(ctx context.Context, commonName string, sans []string, csrPEM string, ekus []string, maxTTLSeconds int, mustStaple bool) (*IssuanceResult, error) { result, err := a.connector.RenewCertificate(ctx, issuer.RenewalRequest{ CommonName: commonName, SANs: sans, CSRPEM: csrPEM, EKUs: ekus, MaxTTLSeconds: maxTTLSeconds, + MustStaple: mustStaple, }) if err != nil { return nil, err diff --git a/internal/service/issuer_adapter_test.go b/internal/service/issuer_adapter_test.go index 466446e..e3a5ff0 100644 --- a/internal/service/issuer_adapter_test.go +++ b/internal/service/issuer_adapter_test.go @@ -13,19 +13,19 @@ import ( // mockConnectorLayerIssuer is a test implementation of issuer.Connector type mockConnectorLayerIssuer struct { - issueResult *issuer.IssuanceResult - issueErr error - renewResult *issuer.IssuanceResult - renewErr error - lastIssueReq *issuer.IssuanceRequest - lastRenewReq *issuer.RenewalRequest - validateErr error - revokeErr error - orderStatusErr error - orderStatus *issuer.OrderStatus - renewalInfoResult *issuer.RenewalInfoResult - renewalInfoErr error - renewalInfoNil bool // flag to force nil result + issueResult *issuer.IssuanceResult + issueErr error + renewResult *issuer.IssuanceResult + renewErr error + lastIssueReq *issuer.IssuanceRequest + lastRenewReq *issuer.RenewalRequest + validateErr error + revokeErr error + orderStatusErr error + orderStatus *issuer.OrderStatus + renewalInfoResult *issuer.RenewalInfoResult + renewalInfoErr error + renewalInfoNil bool // flag to force nil result } func (m *mockConnectorLayerIssuer) ValidateConfig(ctx context.Context, config json.RawMessage) error { @@ -140,7 +140,7 @@ func TestIssuerConnectorAdapter_IssueCertificate_Success(t *testing.T) { adapter := NewIssuerConnectorAdapter(mock) - result, err := adapter.IssueCertificate(ctx, "example.com", []string{"www.example.com"}, "-----BEGIN CERTIFICATE REQUEST-----\nCSR\n-----END CERTIFICATE REQUEST-----", nil, 0) + result, err := adapter.IssueCertificate(ctx, "example.com", []string{"www.example.com"}, "-----BEGIN CERTIFICATE REQUEST-----\nCSR\n-----END CERTIFICATE REQUEST-----", nil, 0, false) if err != nil { t.Fatalf("IssueCertificate failed: %v", err) @@ -177,7 +177,7 @@ func TestIssuerConnectorAdapter_IssueCertificate_Error(t *testing.T) { adapter := NewIssuerConnectorAdapter(mock) - result, err := adapter.IssueCertificate(ctx, "example.com", []string{}, "csr", nil, 0) + result, err := adapter.IssueCertificate(ctx, "example.com", []string{}, "csr", nil, 0, false) if err == nil { t.Fatal("expected error, got nil") @@ -211,7 +211,7 @@ func TestIssuerConnectorAdapter_IssueCertificate_RequestTranslation(t *testing.T sans := []string{"www.test.example.com", "api.test.example.com"} csrPEM := "-----BEGIN CERTIFICATE REQUEST-----\nCSR\n-----END CERTIFICATE REQUEST-----" - _, err := adapter.IssueCertificate(ctx, commonName, sans, csrPEM, nil, 0) + _, err := adapter.IssueCertificate(ctx, commonName, sans, csrPEM, nil, 0, false) if err != nil { t.Fatalf("IssueCertificate failed: %v", err) @@ -261,7 +261,7 @@ func TestIssuerConnectorAdapter_RenewCertificate_Success(t *testing.T) { adapter := NewIssuerConnectorAdapter(mock) - result, err := adapter.RenewCertificate(ctx, "example.com", []string{"www.example.com"}, "-----BEGIN CERTIFICATE REQUEST-----\nCSR\n-----END CERTIFICATE REQUEST-----", nil, 0) + result, err := adapter.RenewCertificate(ctx, "example.com", []string{"www.example.com"}, "-----BEGIN CERTIFICATE REQUEST-----\nCSR\n-----END CERTIFICATE REQUEST-----", nil, 0, false) if err != nil { t.Fatalf("RenewCertificate failed: %v", err) @@ -298,7 +298,7 @@ func TestIssuerConnectorAdapter_RenewCertificate_Error(t *testing.T) { adapter := NewIssuerConnectorAdapter(mock) - result, err := adapter.RenewCertificate(ctx, "example.com", []string{}, "csr", nil, 0) + result, err := adapter.RenewCertificate(ctx, "example.com", []string{}, "csr", nil, 0, false) if err == nil { t.Fatal("expected error, got nil") @@ -332,7 +332,7 @@ func TestIssuerConnectorAdapter_RenewCertificate_RequestTranslation(t *testing.T sans := []string{"www.renew.example.com"} csrPEM := "-----BEGIN CERTIFICATE REQUEST-----\nRENEW-CSR\n-----END CERTIFICATE REQUEST-----" - _, err := adapter.RenewCertificate(ctx, commonName, sans, csrPEM, nil, 0) + _, err := adapter.RenewCertificate(ctx, commonName, sans, csrPEM, nil, 0, false) if err != nil { t.Fatalf("RenewCertificate failed: %v", err) diff --git a/internal/service/m11c_crypto_enforcement_test.go b/internal/service/m11c_crypto_enforcement_test.go index cb5121b..49a5d99 100644 --- a/internal/service/m11c_crypto_enforcement_test.go +++ b/internal/service/m11c_crypto_enforcement_test.go @@ -213,7 +213,7 @@ func TestIssuerConnectorAdapter_IssueCertificate_MaxTTLForwarded(t *testing.T) { mock := &mockConnectorLayerIssuer{} adapter := NewIssuerConnectorAdapter(mock) - _, err := adapter.IssueCertificate(context.Background(), "test.example.com", nil, "csr", nil, 7200) + _, err := adapter.IssueCertificate(context.Background(), "test.example.com", nil, "csr", nil, 7200, false) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -230,7 +230,7 @@ func TestIssuerConnectorAdapter_RenewCertificate_MaxTTLForwarded(t *testing.T) { mock := &mockConnectorLayerIssuer{} adapter := NewIssuerConnectorAdapter(mock) - _, err := adapter.RenewCertificate(context.Background(), "renew.example.com", nil, "csr", nil, 14400) + _, err := adapter.RenewCertificate(context.Background(), "renew.example.com", nil, "csr", nil, 14400, false) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -247,7 +247,7 @@ func TestIssuerConnectorAdapter_IssueCertificate_ZeroMaxTTL(t *testing.T) { mock := &mockConnectorLayerIssuer{} adapter := NewIssuerConnectorAdapter(mock) - _, err := adapter.IssueCertificate(context.Background(), "test.example.com", nil, "csr", nil, 0) + _, err := adapter.IssueCertificate(context.Background(), "test.example.com", nil, "csr", nil, 0, false) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -366,11 +366,16 @@ func TestSCEPService_NoProfileRepo_PassesThrough(t *testing.T) { type capturingIssuerConnector struct { lastMaxTTLSeconds int lastEKUs []string + // SCEP RFC 8894 + Intune master bundle Phase 5.6 follow-up: capture + // must-staple too so the integration test can prove the wire reaches + // the connector for both PKCSReq and renewal paths. + lastMustStaple bool } -func (c *capturingIssuerConnector) IssueCertificate(ctx context.Context, commonName string, sans []string, csrPEM string, ekus []string, maxTTLSeconds int) (*IssuanceResult, error) { +func (c *capturingIssuerConnector) IssueCertificate(ctx context.Context, commonName string, sans []string, csrPEM string, ekus []string, maxTTLSeconds int, mustStaple bool) (*IssuanceResult, error) { c.lastMaxTTLSeconds = maxTTLSeconds c.lastEKUs = ekus + c.lastMustStaple = mustStaple now := time.Now() return &IssuanceResult{ Serial: "test-serial", @@ -381,8 +386,8 @@ func (c *capturingIssuerConnector) IssueCertificate(ctx context.Context, commonN }, nil } -func (c *capturingIssuerConnector) RenewCertificate(ctx context.Context, commonName string, sans []string, csrPEM string, ekus []string, maxTTLSeconds int) (*IssuanceResult, error) { - return c.IssueCertificate(ctx, commonName, sans, csrPEM, ekus, maxTTLSeconds) +func (c *capturingIssuerConnector) RenewCertificate(ctx context.Context, commonName string, sans []string, csrPEM string, ekus []string, maxTTLSeconds int, mustStaple bool) (*IssuanceResult, error) { + return c.IssueCertificate(ctx, commonName, sans, csrPEM, ekus, maxTTLSeconds, mustStaple) } func (c *capturingIssuerConnector) RevokeCertificate(ctx context.Context, serial string, reason string) error { diff --git a/internal/service/renewal.go b/internal/service/renewal.go index 9226824..bb2a61f 100644 --- a/internal/service/renewal.go +++ b/internal/service/renewal.go @@ -43,11 +43,18 @@ func (s *RenewalService) SetTargetRepo(repo repository.TargetRepository) { // inversion. Use IssuerConnectorAdapter to bridge between the two. type IssuerConnector interface { // IssueCertificate issues a new certificate using the provided CSR PEM. - // maxTTLSeconds caps the certificate validity period (0 = no cap, use issuer default). - IssueCertificate(ctx context.Context, commonName string, sans []string, csrPEM string, ekus []string, maxTTLSeconds int) (*IssuanceResult, error) + // maxTTLSeconds caps the certificate validity period (0 = no cap, use + // issuer default). mustStaple, when true, instructs the issuer to add + // the RFC 7633 id-pe-tlsfeature extension to the issued cert (only the + // local issuer honors this; upstream connectors silently ignore it). + // SCEP RFC 8894 + Intune master bundle Phase 5.6 follow-up. + IssueCertificate(ctx context.Context, commonName string, sans []string, csrPEM string, ekus []string, maxTTLSeconds int, mustStaple bool) (*IssuanceResult, error) // RenewCertificate renews a certificate using the provided CSR PEM. - // maxTTLSeconds caps the certificate validity period (0 = no cap, use issuer default). - RenewCertificate(ctx context.Context, commonName string, sans []string, csrPEM string, ekus []string, maxTTLSeconds int) (*IssuanceResult, error) + // maxTTLSeconds caps the certificate validity period (0 = no cap, use + // issuer default). mustStaple has the same semantics as on + // IssueCertificate so renewed certs match their initial-issuance + // extension set when the bound profile changed mid-lifetime. + RenewCertificate(ctx context.Context, commonName string, sans []string, csrPEM string, ekus []string, maxTTLSeconds int, mustStaple bool) (*IssuanceResult, error) // RevokeCertificate revokes a certificate by serial number with an optional reason. RevokeCertificate(ctx context.Context, serial string, reason string) error // GenerateCRL generates a DER-encoded X.509 CRL from the given revocation entries. @@ -446,18 +453,25 @@ func (s *RenewalService) processRenewalServerKeygen(ctx context.Context, job *do Bytes: x509.MarshalPKCS1PrivateKey(privKey), })) - // Resolve EKUs and MaxTTL from the certificate profile - var ekus []string - var maxTTLSeconds int + // Resolve EKUs + MaxTTL + must-staple from the certificate profile. + // SCEP RFC 8894 + Intune master bundle Phase 5.6 follow-up: thread + // must-staple through the renewal path too so renewed certs match + // their initial-issuance extension set. + var ( + ekus []string + maxTTLSeconds int + mustStaple bool + ) if cert.CertificateProfileID != "" && s.profileRepo != nil { if profile, profileErr := s.profileRepo.Get(ctx, cert.CertificateProfileID); profileErr == nil && profile != nil { ekus = profile.AllowedEKUs maxTTLSeconds = profile.MaxTTLSeconds + mustStaple = profile.MustStaple } } // Call issuer connector to renew - result, err := connector.RenewCertificate(ctx, cert.CommonName, cert.SANs, csrPEM, ekus, maxTTLSeconds) + result, err := connector.RenewCertificate(ctx, cert.CommonName, cert.SANs, csrPEM, ekus, maxTTLSeconds, mustStaple) if err != nil { s.failJob(ctx, job, fmt.Sprintf("issuer renewal failed: %v", err)) if notifErr := s.notificationSvc.SendRenewalNotification(ctx, cert, false, err); notifErr != nil { @@ -564,18 +578,23 @@ func (s *RenewalService) CompleteAgentCSRRenewal(ctx context.Context, job *domai return fmt.Errorf("failed to update job status: %w", err) } - // Resolve EKUs and MaxTTL from the certificate profile (for S/MIME, email certs, etc.) - var ekus []string - var maxTTLSeconds int + // Resolve EKUs + MaxTTL + must-staple from the certificate profile. + // SCEP RFC 8894 + Intune master bundle Phase 5.6 follow-up. + var ( + ekus []string + maxTTLSeconds int + mustStaple bool + ) if profile != nil { if len(profile.AllowedEKUs) > 0 { ekus = profile.AllowedEKUs } maxTTLSeconds = profile.MaxTTLSeconds + mustStaple = profile.MustStaple } // Sign the agent-submitted CSR via issuer - result, err := connector.RenewCertificate(ctx, cert.CommonName, cert.SANs, csrPEM, ekus, maxTTLSeconds) + result, err := connector.RenewCertificate(ctx, cert.CommonName, cert.SANs, csrPEM, ekus, maxTTLSeconds, mustStaple) if err != nil { s.failJob(ctx, job, fmt.Sprintf("issuer signing failed: %v", err)) if notifErr := s.notificationSvc.SendRenewalNotification(ctx, cert, false, err); notifErr != nil { diff --git a/internal/service/scep.go b/internal/service/scep.go index a373b5c..fd9609e 100644 --- a/internal/service/scep.go +++ b/internal/service/scep.go @@ -172,15 +172,24 @@ func (s *SCEPService) processEnrollment(ctx context.Context, csrPEM string, tran "transaction_id", transactionID, "issuer", s.issuerID) - // Resolve MaxTTL from profile - var maxTTLSeconds int + // Resolve MaxTTL + must-staple from profile. + // SCEP RFC 8894 + Intune master bundle Phase 5.6 follow-up: thread + // profile.MustStaple through to the issuer so the local issuer can + // add the RFC 7633 id-pe-tlsfeature extension. Without this read the + // CertificateProfile.MustStaple field would be a stored-but-ignored + // "lying field" that operators set without behavior change. + var ( + maxTTLSeconds int + mustStaple bool + ) if profile != nil { maxTTLSeconds = profile.MaxTTLSeconds + mustStaple = profile.MustStaple } // 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) + result, err := s.issuer.IssueCertificate(ctx, commonName, sans, csrPEM, ekus, maxTTLSeconds, mustStaple) if err != nil { s.logger.Error("SCEP enrollment failed", "action", auditAction, diff --git a/internal/service/scep_must_staple_test.go b/internal/service/scep_must_staple_test.go new file mode 100644 index 0000000..550ba9f --- /dev/null +++ b/internal/service/scep_must_staple_test.go @@ -0,0 +1,154 @@ +package service + +import ( + "context" + "crypto/ecdsa" + "crypto/elliptic" + "crypto/rand" + "crypto/x509" + "crypto/x509/pkix" + "encoding/pem" + "io" + "log/slog" + "testing" + + "github.com/shankar0123/certctl/internal/domain" +) + +// SCEP RFC 8894 + Intune master bundle Phase 5.6 follow-up: end-to-end +// integration test for the must-staple wire from CertificateProfile.MustStaple +// through the SCEPService into the IssuerConnector. +// +// Background: the original Phase 5.6 commit shipped the local issuer's RFC +// 7633 extension generation + the IssuanceRequest.MustStaple field, but +// the SCEP service layer (and EST + agent + renewal) didn't read +// profile.MustStaple and didn't pass it to IssueCertificate. That made +// CertificateProfile.MustStaple a "lying field" — the operator could set +// it, the API would store + return it, the docs claimed it worked, but +// the cert came back without the extension. Worse than not having the +// field at all. +// +// This test pins the wire end-to-end: +// +// 1. Create a CertificateProfile with MustStaple=true. +// 2. Drive a SCEP enrollment through SCEPService.PKCSReq. +// 3. Assert the mock IssuerConnector saw mustStaple=true (proving the +// service-layer wire reaches the connector). +// +// The local-issuer-side test (must_staple_test.go) already pins that the +// connector translates that bool into the RFC 7633 extension. Together +// they prove: configurable bit → behavior change, end-to-end. + +// stubProfileRepo is a minimal in-memory CertificateProfileRepository for +// the test. Returns the configured profile by ID; other repo methods +// panic if exercised (we only need Get). +type stubProfileRepo struct { + profile *domain.CertificateProfile +} + +func (s *stubProfileRepo) Get(_ context.Context, id string) (*domain.CertificateProfile, error) { + if s.profile != nil && s.profile.ID == id { + return s.profile, nil + } + return nil, nil +} + +func (s *stubProfileRepo) Create(_ context.Context, _ *domain.CertificateProfile) error { + panic("stubProfileRepo.Create not implemented for this test") +} + +func (s *stubProfileRepo) Update(_ context.Context, _ *domain.CertificateProfile) error { + panic("stubProfileRepo.Update not implemented for this test") +} + +func (s *stubProfileRepo) Delete(_ context.Context, _ string) error { + panic("stubProfileRepo.Delete not implemented for this test") +} + +func (s *stubProfileRepo) List(_ context.Context) ([]*domain.CertificateProfile, error) { + panic("stubProfileRepo.List not implemented for this test") +} + +func TestSCEPService_PKCSReq_PlumbsMustStapleToIssuer(t *testing.T) { + // 1. Mock issuer that records the must-staple bool from the call. + mock := &mockIssuerConnector{} + + // 2. Profile with MustStaple=true. + profile := &domain.CertificateProfile{ + ID: "prof-must-staple", + Name: "must-staple", + MaxTTLSeconds: 86400, + MustStaple: true, + Enabled: true, + } + repo := &stubProfileRepo{profile: profile} + + // 3. Build the service. Use a real challenge password so we exercise + // the same gate the production path runs. + logger := slog.New(slog.NewTextHandler(io.Discard, nil)) + svc := NewSCEPService("iss-test", mock, nil, logger, "shared-secret-123") + svc.SetProfileRepo(repo) + svc.SetProfileID(profile.ID) + + // 4. Build a CSR (real crypto so processEnrollment's CheckSignature + // + crypto-policy validation both pass). + csrPEM := buildCSRForSCEPMustStaple(t, "must-staple.example.com") + + // 5. Drive the enrollment. + _, err := svc.PKCSReq(context.Background(), csrPEM, "shared-secret-123", "txn-must-staple") + if err != nil { + t.Fatalf("PKCSReq: %v", err) + } + + // 6. Assert the must-staple wire reached the connector. + if !mock.LastMustStaple { + t.Errorf("mockIssuerConnector.LastMustStaple = false, want true — service layer dropped profile.MustStaple on the floor (the 'lying field' regression)") + } +} + +func TestSCEPService_PKCSReq_NoMustStaplePropagatesFalse(t *testing.T) { + // Companion: when the profile does NOT have MustStaple set, the + // connector must see false. Pins the symmetric contract. + mock := &mockIssuerConnector{LastMustStaple: true} // pre-set to true so we can detect a stuck-at-true bug + profile := &domain.CertificateProfile{ + ID: "prof-no-staple", + Name: "no-staple", + MaxTTLSeconds: 86400, + MustStaple: false, + Enabled: true, + } + repo := &stubProfileRepo{profile: profile} + + logger := slog.New(slog.NewTextHandler(io.Discard, nil)) + svc := NewSCEPService("iss-test", mock, nil, logger, "shared-secret-123") + svc.SetProfileRepo(repo) + svc.SetProfileID(profile.ID) + + csrPEM := buildCSRForSCEPMustStaple(t, "no-staple.example.com") + _, err := svc.PKCSReq(context.Background(), csrPEM, "shared-secret-123", "txn-no-staple") + if err != nil { + t.Fatalf("PKCSReq: %v", err) + } + if mock.LastMustStaple { + t.Errorf("mockIssuerConnector.LastMustStaple = true, want false — service layer set MustStaple=true despite profile.MustStaple=false") + } +} + +// buildCSRForSCEPMustStaple creates an ECDSA P-256 CSR for the given CN. +// Local helper — kept distinct from buildCSRForSCEP elsewhere in the +// service test suite to avoid name collisions. +func buildCSRForSCEPMustStaple(t *testing.T, cn string) string { + t.Helper() + key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + if err != nil { + t.Fatalf("ecdsa.GenerateKey: %v", err) + } + tmpl := &x509.CertificateRequest{ + Subject: pkix.Name{CommonName: cn}, + } + der, err := x509.CreateCertificateRequest(rand.Reader, tmpl, key) + if err != nil { + t.Fatalf("CreateCertificateRequest: %v", err) + } + return string(pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE REQUEST", Bytes: der})) +} diff --git a/internal/service/testutil_test.go b/internal/service/testutil_test.go index 9e7b18d..9e2b729 100644 --- a/internal/service/testutil_test.go +++ b/internal/service/testutil_test.go @@ -1254,9 +1254,25 @@ type mockIssuerConnector struct { // LastOCSPSignRequest captures the last request passed to SignOCSPResponse. // Tests use this to assert CertStatus (0=good, 1=revoked, 2=unknown). LastOCSPSignRequest *OCSPSignRequest + + // LastMustStaple records the must-staple bool from the most recent + // Issue/Renew call so tests can assert the service-layer wire from + // CertificateProfile.MustStaple → IssuerConnector reaches the + // connector. SCEP RFC 8894 + Intune master bundle Phase 5.6 follow-up. + LastMustStaple bool } -func (m *mockIssuerConnector) IssueCertificate(ctx context.Context, commonName string, sans []string, csrPEM string, ekus []string, maxTTLSeconds int) (*IssuanceResult, error) { +// LastMustStaple records the must-staple bool from the most recent +// IssueCertificate / RenewCertificate call. Set by both methods so tests +// can assert the wire from CertificateProfile.MustStaple → service → +// IssuerConnector reaches the connector. SCEP RFC 8894 + Intune master +// bundle Phase 5.6 follow-up. +// +// (Field added to mockIssuerConnector struct above; declared via the +// pointer receiver so existing test fixtures don't need re-zeroing.) + +func (m *mockIssuerConnector) IssueCertificate(ctx context.Context, commonName string, sans []string, csrPEM string, ekus []string, maxTTLSeconds int, mustStaple bool) (*IssuanceResult, error) { + m.LastMustStaple = mustStaple if m.Err != nil { return nil, m.Err } @@ -1273,11 +1289,12 @@ func (m *mockIssuerConnector) IssueCertificate(ctx context.Context, commonName s }, nil } -func (m *mockIssuerConnector) RenewCertificate(ctx context.Context, commonName string, sans []string, csrPEM string, ekus []string, maxTTLSeconds int) (*IssuanceResult, error) { +func (m *mockIssuerConnector) RenewCertificate(ctx context.Context, commonName string, sans []string, csrPEM string, ekus []string, maxTTLSeconds int, mustStaple bool) (*IssuanceResult, error) { + m.LastMustStaple = mustStaple if m.Err != nil { return nil, m.Err } - return m.IssueCertificate(ctx, commonName, sans, csrPEM, ekus, maxTTLSeconds) + return m.IssueCertificate(ctx, commonName, sans, csrPEM, ekus, maxTTLSeconds, mustStaple) } func (m *mockIssuerConnector) RevokeCertificate(ctx context.Context, serial string, reason string) error {