fix(pki): honor CA-issuance policy denial on certificate renewal

backend/src/services/certificate-common/certificate-issuance-utils.ts

@@ -15,7 +15,12 @@ import {
   signatureAlgorithmToAlgCfg
 } from "@app/services/certificate-authority/certificate-authority-fns";
 
-import { CertExtendedKeyUsageType, CertKeyUsageType, CertSubjectAlternativeNameType } from "./certificate-constants";
+import {
+  CertExtendedKeyUsageType,
+  CertKeyUsageType,
+  CertPolicyState,
+  CertSubjectAlternativeNameType
+} from "./certificate-constants";
 import {
   bufferToString,
   buildCertificateSubjectFromTemplate,
@@ -202,6 +207,31 @@ export const validateAlgorithmCompatibility = (
   }
 };
 
+/**
+ * Validates that a request asking to issue a CA certificate (basicConstraints.isCA) is permitted
+ * by the policy's CA state, throwing the standard denial error otherwise. Returns the resolved
+ * policy CA state so callers that need it for further checks (e.g. max path length) don't have
+ * to re-derive it.
+ *
+ * Centralizes the CA-denial check shared by every issuance path (direct issuance, CSR/profile
+ * issuance, approval finalization, and renewal) so a path can't silently skip it.
+ */
+export const assertCaIssuancePolicyAllowed = (
+  policy: { basicConstraints?: { isCA?: string } | null } | null | undefined,
+  shouldIssueAsCA: boolean
+): CertPolicyState => {
+  const policyIsCAState = (policy?.basicConstraints?.isCA as CertPolicyState) || CertPolicyState.DENIED;
+
+  if (shouldIssueAsCA && policyIsCAState === CertPolicyState.DENIED) {
+    throw new BadRequestError({
+      message:
+        "CA certificate issuance is not allowed by this policy. The policy's CA:true basicConstraints must be set to 'allowed' or 'required'."
+    });
+  }
+
+  return policyIsCAState;
+};
+
 /**
  * Gets the effective algorithms with fallbacks
  */

backend/src/services/certificate-v3/certificate-approval-fns.ts

@@ -46,6 +46,7 @@ import {
   CertPolicyState
 } from "../certificate-common/certificate-constants";
 import {
+  assertCaIssuancePolicyAllowed,
   calculateFinalRenewBeforeDays,
   extractCertificateFromBuffer,
   generateSelfSignedCertificate,
@@ -462,15 +463,7 @@ export const certificateApprovalServiceFactory = (
     validateAlgorithmCompatibility(ca, certPolicy);
 
     const csrBasicConstraints = certRequest.basicConstraints as { isCA: boolean; pathLength?: number } | undefined;
-    const policyIsCAState: CertPolicyState =
-      (certPolicy.basicConstraints?.isCA as CertPolicyState) || CertPolicyState.DENIED;
-
-    if (csrBasicConstraints?.isCA && policyIsCAState === CertPolicyState.DENIED) {
-      throw new BadRequestError({
-        message:
-          "CA certificate issuance is not allowed by the current policy. The policy's CA:true basicConstraints must be set to 'allowed' or 'required'."
-      });
-    }
+    const policyIsCAState = assertCaIssuancePolicyAllowed(certPolicy, csrBasicConstraints?.isCA === true);
 
     let effectiveBasicConstraints = csrBasicConstraints;
     const storedPathLength = csrBasicConstraints?.pathLength;

backend/src/services/certificate-v3/certificate-v3-service.test.ts

@@ -19,6 +19,7 @@ import {
   CertExtendedKeyUsageType,
   CertIncludeType,
   CertKeyUsageType,
+  CertPolicyState,
   CertSubjectAlternativeNameType,
   CertSubjectAttributeType
 } from "@app/services/certificate-common/certificate-constants";
@@ -2184,6 +2185,90 @@ describe("CertificateV3Service", () => {
 
       expect(result).toHaveProperty("certificate", "renewed-cert");
     });
+
+    it("should reject renewal of a CA certificate when the policy now denies CA issuance", async () => {
+      const caCert = { ...mockOriginalCert, isCA: true, pathLength: 0 };
+
+      vi.mocked(mockCertificateDAL.findById).mockResolvedValue(caCert);
+      vi.mocked(mockCertificateSecretDAL.findOne).mockResolvedValue({ id: "secret-123", certId: "cert-123" } as any);
+      vi.mocked(mockCertificateProfileDAL.findByIdWithConfigs).mockResolvedValue(mockProfile);
+      vi.mocked(mockCertificateAuthorityDAL.findByIdWithAssociatedCa).mockResolvedValue(mockCA);
+      vi.mocked(mockCertificatePolicyService.getPolicyById).mockResolvedValue({
+        ...mockPolicy,
+        basicConstraints: { isCA: CertPolicyState.DENIED }
+      } as any);
+      vi.mocked(mockCertificatePolicyService.validateCertificateRequest).mockResolvedValue({
+        isValid: true,
+        errors: [],
+        warnings: []
+      });
+
+      vi.mocked(mockCertificateDAL.transaction).mockImplementation(async (callback: (tx: any) => Promise<unknown>) => {
+        const mockTx = {};
+        return callback(mockTx);
+      });
+
+      await expect(
+        service.renewCertificate({
+          certificateId: "cert-123",
+          ...mockActor
+        })
+      ).rejects.toThrow(/CA certificate issuance is not allowed by this policy/);
+
+      expect(mockInternalCaService.issueCertFromCa).not.toHaveBeenCalled();
+    });
+
+    it("should carry the CA basicConstraints and path length through to the renewed certificate", async () => {
+      const caCert = { ...mockOriginalCert, isCA: true, pathLength: 0 };
+
+      vi.mocked(mockCertificateDAL.findById)
+        .mockResolvedValueOnce(caCert)
+        .mockResolvedValueOnce({ ...caCert, id: "cert-456", serialNumber: "789012" });
+      vi.mocked(mockCertificateSecretDAL.findOne).mockResolvedValue({ id: "secret-123", certId: "cert-123" } as any);
+      vi.mocked(mockCertificateProfileDAL.findByIdWithConfigs).mockResolvedValue(mockProfile);
+      vi.mocked(mockCertificateAuthorityDAL.findByIdWithAssociatedCa).mockResolvedValue(mockCA);
+      vi.mocked(mockCertificatePolicyService.getPolicyById).mockResolvedValue({
+        ...mockPolicy,
+        basicConstraints: { isCA: CertPolicyState.ALLOWED, maxPathLength: 1 }
+      } as any);
+      vi.mocked(mockCertificatePolicyService.validateCertificateRequest).mockResolvedValue({
+        isValid: true,
+        errors: [],
+        warnings: []
+      });
+      vi.mocked(mockInternalCaService.issueCertFromCa).mockResolvedValue({
+        certificate: "renewed-ca-cert",
+        certificateChain: "renewed-chain",
+        issuingCaCertificate: "issuing-ca",
+        privateKey: "private-key",
+        serialNumber: "789012",
+        certificateId: "cert-456",
+        commonName: "test.example.com",
+        ca: mockCA
+      });
+
+      const newCert = { ...caCert, id: "cert-456", serialNumber: "789012" };
+      vi.mocked(mockCertificateDAL.findOne).mockResolvedValue(newCert);
+      vi.mocked(mockCertificateDAL.updateById).mockResolvedValue(newCert);
+
+      vi.mocked(mockCertificateDAL.transaction).mockImplementation(async (callback: (tx: any) => Promise<unknown>) => {
+        const mockTx = {};
+        return callback(mockTx);
+      });
+
+      const result = await service.renewCertificate({
+        certificateId: "cert-123",
+        ...mockActor
+      });
+
+      expect(result).toHaveProperty("certificate", "renewed-ca-cert");
+      expect(mockInternalCaService.issueCertFromCa).toHaveBeenCalledWith(
+        expect.objectContaining({
+          basicConstraints: { isCA: true, pathLength: 1 },
+          pathLength: 0
+        })
+      );
+    });
   });
 
   describe("updateRenewalConfig", () => {

backend/src/services/certificate-v3/certificate-v3-service.ts

@@ -69,7 +69,6 @@ import { TUserDALFactory } from "@app/services/user/user-dal";
 import {
   CertExtendedKeyUsageType,
   CertKeyUsageType,
-  CertPolicyState,
   mapExtendedKeyUsageToLegacy,
   mapKeyUsageToLegacy,
   mapLegacyExtendedKeyUsageToStandard,
@@ -80,6 +79,7 @@ import {
   extractCertificateRequestFromCSR
 } from "../certificate-common/certificate-csr-utils";
 import {
+  assertCaIssuancePolicyAllowed,
   calculateFinalRenewBeforeDays,
   detectSanType,
   extractCertificateFromBuffer,
@@ -1284,15 +1284,7 @@ export const certificateV3ServiceFactory = ({
     validateAlgorithmCompatibility(ca, policy);
 
     const shouldIssueAsCA = certificateRequest.basicConstraints?.isCA === true;
-    const policyIsCAState: CertPolicyState =
-      (policy.basicConstraints?.isCA as CertPolicyState) || CertPolicyState.DENIED;
-
-    if (shouldIssueAsCA && policyIsCAState === CertPolicyState.DENIED) {
-      throw new BadRequestError({
-        message:
-          "CA certificate issuance is not allowed by this policy. The policy's CA:true basicConstraints must be set to 'allowed' or 'required'."
-      });
-    }
+    assertCaIssuancePolicyAllowed(policy, shouldIssueAsCA);
 
     const caBasicConstraints = shouldIssueAsCA
       ? { isCA: true, pathLength: policy.basicConstraints?.maxPathLength }
@@ -1602,15 +1594,7 @@ export const certificateV3ServiceFactory = ({
 
     validateAlgorithmCompatibility(ca, policy);
 
-    const policyIsCAState: CertPolicyState =
-      (policy.basicConstraints?.isCA as CertPolicyState) || CertPolicyState.DENIED;
-
-    if (shouldIssueAsCA && policyIsCAState === CertPolicyState.DENIED) {
-      throw new BadRequestError({
-        message:
-          "CA certificate issuance is not allowed by this policy. The policy's CA:true basicConstraints must be set to 'allowed' or 'required'."
-      });
-    }
+    assertCaIssuancePolicyAllowed(policy, shouldIssueAsCA);
 
     const csrApprovalFactory = APPROVAL_POLICY_FACTORY_MAP[ApprovalPolicyType.CertRequest](
       ApprovalPolicyType.CertRequest
@@ -2455,6 +2439,10 @@ export const certificateV3ServiceFactory = ({
 
       const ttl = getSimpleTtl(originalCert.notBefore, originalCert.notAfter);
 
+      // The renewed certificate must keep the original's CA status (and stay subject to the
+      // same CA-issuance policy checks every other issuance path runs) — see basicConstraints below.
+      const shouldIssueAsCA = originalCert.isCA === true;
+
       const certificateRequest = {
         commonName: originalCert.commonName || undefined,
         organization: originalCert.subjectOrganization || undefined,
@@ -2471,7 +2459,8 @@ export const certificateV3ServiceFactory = ({
           ttl
         },
         signatureAlgorithm: originalCert.signatureAlgorithm || undefined,
-        keyAlgorithm: originalCert.keyAlgorithm || undefined
+        keyAlgorithm: originalCert.keyAlgorithm || undefined,
+        basicConstraints: shouldIssueAsCA ? { isCA: true, pathLength: originalCert.pathLength ?? undefined } : undefined
       };
 
       let validationResult: { isValid: boolean; errors: string[] } = { isValid: true, errors: [] };
@@ -2496,6 +2485,8 @@ export const certificateV3ServiceFactory = ({
         });
       }
 
+      assertCaIssuancePolicyAllowed(policy, shouldIssueAsCA);
+
       const notBefore = new Date();
       const notAfter = new Date(Date.now() + parseTtlToMs(ttl));
 
@@ -2550,6 +2541,10 @@ export const certificateV3ServiceFactory = ({
             signatureAlgorithm: originalSignatureAlgorithm,
             keyAlgorithm: originalKeyAlgorithm,
             isFromProfile: true,
+            basicConstraints: shouldIssueAsCA
+              ? { isCA: true, pathLength: policy?.basicConstraints?.maxPathLength }
+              : undefined,
+            pathLength: originalCert.pathLength ?? undefined,
             organization: originalCert.subjectOrganization || undefined,
             ou: originalCert.subjectOrganizationalUnit || undefined,
             country: originalCert.subjectCountry || undefined,