From fe2d57154b8de92d031bd108cc50188ef08f8000 Mon Sep 17 00:00:00 2001 From: Carlos Monastyrski Date: Fri, 24 Oct 2025 19:57:06 -0300 Subject: [PATCH] Fix sign-certificate logic and minor improvements on the certificates table --- backend/src/server/routes/index.ts | 1 + .../src/server/routes/v1/project-router.ts | 2 +- .../server/routes/v3/certificates-router.ts | 13 +- .../internal-certificate-authority-service.ts | 3 +- .../certificate-csr-utils.ts | 183 ++++++++++++++++++ .../certificate-est-v3-service.ts | 77 +------- .../certificate-v3-service.test.ts | 67 +++++++ .../certificate-v3/certificate-v3-service.ts | 96 ++++++--- .../certificate-v3/certificate-v3-types.ts | 2 - .../services/certificate/certificate-dal.ts | 40 +++- .../src/services/project/project-service.ts | 4 +- frontend/src/hooks/api/certificates/types.ts | 1 + .../components/CertificatesTable.tsx | 17 +- 13 files changed, 386 insertions(+), 120 deletions(-) create mode 100644 backend/src/services/certificate-common/certificate-csr-utils.ts diff --git a/backend/src/server/routes/index.ts b/backend/src/server/routes/index.ts index d7a84e87f..923a497a0 100644 --- a/backend/src/server/routes/index.ts +++ b/backend/src/server/routes/index.ts @@ -2137,6 +2137,7 @@ export const registerRoutes = async ( const certificateV3Service = certificateV3ServiceFactory({ certificateDAL, + certificateSecretDAL, certificateAuthorityDAL, certificateProfileDAL, certificateTemplateV2Service, diff --git a/backend/src/server/routes/v1/project-router.ts b/backend/src/server/routes/v1/project-router.ts index c1f4140e5..c3bffa2fc 100644 --- a/backend/src/server/routes/v1/project-router.ts +++ b/backend/src/server/routes/v1/project-router.ts @@ -1200,7 +1200,7 @@ export const registerProjectRouter = async (server: FastifyZodProvider) => { }), response: { 200: z.object({ - certificates: z.array(CertificatesSchema), + certificates: z.array(CertificatesSchema.extend({ hasPrivateKey: z.boolean() })), totalCount: z.number() }) } diff --git a/backend/src/server/routes/v3/certificates-router.ts b/backend/src/server/routes/v3/certificates-router.ts index 52ad47772..d2d696596 100644 --- a/backend/src/server/routes/v3/certificates-router.ts +++ b/backend/src/server/routes/v3/certificates-router.ts @@ -18,6 +18,7 @@ import { CertKeyUsageType, CertSubjectAlternativeNameType } from "@app/services/certificate-common/certificate-constants"; +import { extractCertificateRequestFromCSR } from "@app/services/certificate-common/certificate-csr-utils"; import { mapEnumsForValidation } from "@app/services/certificate-common/certificate-utils"; import { validateTemplateRegexField } from "@app/services/certificate-template/certificate-template-validators"; @@ -169,9 +170,7 @@ export const registerCertificatesRouter = async (server: FastifyZodProvider) => .min(1, "TTL cannot be empty") .refine((val) => ms(val) > 0, "TTL must be a positive number"), notBefore: validateCaDateField.optional(), - notAfter: validateCaDateField.optional(), - signatureAlgorithm: z.nativeEnum(CertSignatureAlgorithm), - keyAlgorithm: z.nativeEnum(CertKeyAlgorithm) + notAfter: validateCaDateField.optional() }) .refine(validateTtlAndDateFields, { message: @@ -192,6 +191,8 @@ export const registerCertificatesRouter = async (server: FastifyZodProvider) => }, onRequest: verifyAuth([AuthMode.JWT, AuthMode.IDENTITY_ACCESS_TOKEN]), handler: async (req) => { + const certificateRequest = extractCertificateRequestFromCSR(req.body.csr); + const data = await server.services.certificateV3.signCertificateFromProfile({ actor: req.permission.type, actorId: req.permission.id, @@ -203,9 +204,7 @@ export const registerCertificatesRouter = async (server: FastifyZodProvider) => ttl: req.body.ttl }, notBefore: req.body.notBefore ? new Date(req.body.notBefore) : undefined, - notAfter: req.body.notAfter ? new Date(req.body.notAfter) : undefined, - signatureAlgorithm: req.body.signatureAlgorithm, - keyAlgorithm: req.body.keyAlgorithm + notAfter: req.body.notAfter ? new Date(req.body.notAfter) : undefined }); await server.services.auditLog.createAuditLog({ @@ -217,7 +216,7 @@ export const registerCertificatesRouter = async (server: FastifyZodProvider) => certificateProfileId: req.body.profileId, certificateId: data.certificateId, profileName: data.profileName, - commonName: "" + commonName: certificateRequest.commonName || "" } } }); diff --git a/backend/src/services/certificate-authority/internal/internal-certificate-authority-service.ts b/backend/src/services/certificate-authority/internal/internal-certificate-authority-service.ts index 94c74939a..a7292e366 100644 --- a/backend/src/services/certificate-authority/internal/internal-certificate-authority-service.ts +++ b/backend/src/services/certificate-authority/internal/internal-certificate-authority-service.ts @@ -1728,7 +1728,8 @@ export const internalCertificateAuthorityServiceFactory = ({ certificateAuthorityDAL, certificateAuthoritySecretDAL, projectDAL, - kmsService + kmsService, + signatureAlgorithm: alg }); const caCrl = await certificateAuthorityCrlDAL.findOne({ caSecretId: caSecret.id }); diff --git a/backend/src/services/certificate-common/certificate-csr-utils.ts b/backend/src/services/certificate-common/certificate-csr-utils.ts new file mode 100644 index 000000000..7578950af --- /dev/null +++ b/backend/src/services/certificate-common/certificate-csr-utils.ts @@ -0,0 +1,183 @@ +import * as x509 from "@peculiar/x509"; + +import { BadRequestError } from "@app/lib/errors"; + +import { + CertExtendedKeyUsageOIDToName, + CertKeyAlgorithm, + CertKeyUsage, + CertSignatureAlgorithm, + mapLegacyAltNameType, + TAltNameMapping, + TAltNameType +} from "../certificate/certificate-types"; +import { parseDistinguishedName } from "../certificate-authority/certificate-authority-fns"; +import { validateAndMapAltNameType } from "../certificate-authority/certificate-authority-validators"; +import { TCertificateRequest } from "../certificate-template-v2/certificate-template-v2-types"; +import { mapLegacyExtendedKeyUsageToStandard, mapLegacyKeyUsageToStandard } from "./certificate-constants"; + +/** + * Extracts certificate request data from a CSR string + * @param csr - The CSR in PEM format + * @returns TCertificateRequest object with parsed CSR data + */ +export const extractCertificateRequestFromCSR = (csr: string): TCertificateRequest => { + const csrObj = new x509.Pkcs10CertificateRequest(csr); + const subject = parseDistinguishedName(csrObj.subject); + + const certificateRequest: TCertificateRequest = { + commonName: subject.commonName, + organization: subject.organization, + organizationUnit: subject.ou, + locality: subject.locality, + state: subject.province, + country: subject.country + }; + + const csrKeyUsageExtension = csrObj.getExtension("2.5.29.15") as x509.KeyUsagesExtension; + if (csrKeyUsageExtension) { + const csrKeyUsages = Object.values(CertKeyUsage).filter( + // eslint-disable-next-line no-bitwise + (keyUsage) => (x509.KeyUsageFlags[keyUsage] & csrKeyUsageExtension.usages) !== 0 + ); + certificateRequest.keyUsages = csrKeyUsages.map(mapLegacyKeyUsageToStandard); + } + + const csrExtendedKeyUsageExtension = csrObj.getExtension("2.5.29.37") as x509.ExtendedKeyUsageExtension; + if (csrExtendedKeyUsageExtension) { + const csrExtendedKeyUsages = csrExtendedKeyUsageExtension.usages.map( + (ekuOid) => CertExtendedKeyUsageOIDToName[ekuOid as string] + ); + certificateRequest.extendedKeyUsages = csrExtendedKeyUsages.map(mapLegacyExtendedKeyUsageToStandard); + } + + const sanExtension = csrObj.extensions.find((ext) => ext.type === "2.5.29.17"); + if (sanExtension) { + const sanNames = new x509.GeneralNames(sanExtension.value); + const altNamesArray: TAltNameMapping[] = sanNames.items + .filter( + (value) => + value.type === TAltNameType.EMAIL || + value.type === TAltNameType.DNS || + value.type === TAltNameType.IP || + value.type === TAltNameType.URL + ) + .map((name): TAltNameMapping => { + const altNameType = validateAndMapAltNameType(name.value); + if (!altNameType) { + throw new BadRequestError({ message: `Invalid altName from CSR: ${name.value}` }); + } + return altNameType; + }); + + certificateRequest.subjectAlternativeNames = altNamesArray.map((altName) => ({ + type: mapLegacyAltNameType(altName.type), + value: altName.value + })); + } + + return certificateRequest; +}; + +/** + * Extracts the key algorithm and signature algorithm from a CSR + * @param csr - The CSR in PEM format + * @returns Object containing keyAlgorithm and signatureAlgorithm + */ +export const extractAlgorithmsFromCSR = (csr: string) => { + const csrObj = new x509.Pkcs10CertificateRequest(csr); + + // Extract key algorithm from public key + const { publicKey } = csrObj; + let keyAlgorithm: CertKeyAlgorithm; + + if (publicKey.algorithm.name === "RSASSA-PKCS1-v1_5") { + const rsaPublicKey = publicKey as unknown as { algorithm: { modulusLength: number } }; + const keySize = rsaPublicKey.algorithm.modulusLength; + switch (keySize) { + case 2048: + keyAlgorithm = CertKeyAlgorithm.RSA_2048; + break; + case 3072: + keyAlgorithm = CertKeyAlgorithm.RSA_3072; + break; + case 4096: + keyAlgorithm = CertKeyAlgorithm.RSA_4096; + break; + default: + throw new BadRequestError({ + message: `Unsupported RSA key size in CSR: ${keySize}. Supported: 2048, 3072, 4096` + }); + } + } else if (publicKey.algorithm.name === "ECDSA") { + const ecPublicKey = publicKey as unknown as { algorithm: { namedCurve: string } }; + const { namedCurve } = ecPublicKey.algorithm; + switch (namedCurve) { + case "P-256": + keyAlgorithm = CertKeyAlgorithm.ECDSA_P256; + break; + case "P-384": + keyAlgorithm = CertKeyAlgorithm.ECDSA_P384; + break; + case "P-521": + keyAlgorithm = CertKeyAlgorithm.ECDSA_P521; + break; + default: + throw new BadRequestError({ + message: `Unsupported ECDSA curve in CSR: ${namedCurve}. Supported: P-256, P-384, P-521` + }); + } + } else { + throw new BadRequestError({ + message: `Unsupported key algorithm in CSR: ${publicKey.algorithm.name}. Supported: RSASSA-PKCS1-v1_5, ECDSA` + }); + } + + const signatureAlgorithm = csrObj.signatureAlgorithm.name; + const hashName = (csrObj.signatureAlgorithm as unknown as { hash?: { name: string } }).hash?.name; + + let normalizedSignatureAlg: CertSignatureAlgorithm; + + if (signatureAlgorithm === "RSASSA-PKCS1-v1_5") { + switch (hashName) { + case "SHA-256": + normalizedSignatureAlg = CertSignatureAlgorithm.RSA_SHA256; + break; + case "SHA-384": + normalizedSignatureAlg = CertSignatureAlgorithm.RSA_SHA384; + break; + case "SHA-512": + normalizedSignatureAlg = CertSignatureAlgorithm.RSA_SHA512; + break; + default: + throw new BadRequestError({ + message: `Unsupported RSA hash algorithm in CSR: ${hashName}. Supported: SHA-256, SHA-384, SHA-512` + }); + } + } else if (signatureAlgorithm === "ECDSA") { + switch (hashName) { + case "SHA-256": + normalizedSignatureAlg = CertSignatureAlgorithm.ECDSA_SHA256; + break; + case "SHA-384": + normalizedSignatureAlg = CertSignatureAlgorithm.ECDSA_SHA384; + break; + case "SHA-512": + normalizedSignatureAlg = CertSignatureAlgorithm.ECDSA_SHA512; + break; + default: + throw new BadRequestError({ + message: `Unsupported ECDSA hash algorithm in CSR: ${hashName}. Supported: SHA-256, SHA-384, SHA-512` + }); + } + } else { + throw new BadRequestError({ + message: `Unsupported signature algorithm in CSR: ${signatureAlgorithm}. Supported: RSASSA-PKCS1-v1_5, ECDSA` + }); + } + + return { + keyAlgorithm, + signatureAlgorithm: normalizedSignatureAlg + }; +}; diff --git a/backend/src/services/certificate-est-v3/certificate-est-v3-service.ts b/backend/src/services/certificate-est-v3/certificate-est-v3-service.ts index f6dbfba52..0d8ef30d0 100644 --- a/backend/src/services/certificate-est-v3/certificate-est-v3-service.ts +++ b/backend/src/services/certificate-est-v3/certificate-est-v3-service.ts @@ -3,31 +3,15 @@ import * as x509 from "@peculiar/x509"; import { extractX509CertFromChain } from "@app/lib/certificates/extract-certificate"; import { BadRequestError, NotFoundError, UnauthorizedError } from "@app/lib/errors"; import { isCertChainValid } from "@app/services/certificate/certificate-fns"; -import { - CertExtendedKeyUsageOIDToName, - CertKeyUsage, - mapLegacyAltNameType, - TAltNameMapping, - TAltNameType -} from "@app/services/certificate/certificate-types"; import { TCertificateAuthorityCertDALFactory } from "@app/services/certificate-authority/certificate-authority-cert-dal"; import { TCertificateAuthorityDALFactory } from "@app/services/certificate-authority/certificate-authority-dal"; -import { - getCaCertChain, - getCaCertChains, - parseDistinguishedName -} from "@app/services/certificate-authority/certificate-authority-fns"; -import { validateAndMapAltNameType } from "@app/services/certificate-authority/certificate-authority-validators"; +import { getCaCertChain, getCaCertChains } from "@app/services/certificate-authority/certificate-authority-fns"; import { TInternalCertificateAuthorityServiceFactory } from "@app/services/certificate-authority/internal/internal-certificate-authority-service"; -import { - mapLegacyExtendedKeyUsageToStandard, - mapLegacyKeyUsageToStandard -} from "@app/services/certificate-common/certificate-constants"; +import { extractCertificateRequestFromCSR } from "@app/services/certificate-common/certificate-csr-utils"; import { mapEnumsForValidation } from "@app/services/certificate-common/certificate-utils"; import { TCertificateProfileDALFactory } from "@app/services/certificate-profile/certificate-profile-dal"; import { EnrollmentType } from "@app/services/certificate-profile/certificate-profile-types"; import { TCertificateTemplateV2ServiceFactory } from "@app/services/certificate-template-v2/certificate-template-v2-service"; -import { TCertificateRequest } from "@app/services/certificate-template-v2/certificate-template-v2-types"; import { TEstEnrollmentConfigDALFactory } from "@app/services/enrollment-config/est-enrollment-config-dal"; import { TKmsServiceFactory } from "@app/services/kms/kms-service"; import { TProjectDALFactory } from "@app/services/project/project-dal"; @@ -61,63 +45,6 @@ export const certificateEstV3ServiceFactory = ({ certificateProfileDAL, estEnrollmentConfigDAL }: TCertificateEstV3ServiceFactoryDep) => { - const extractCertificateRequestFromCSR = (csr: string): TCertificateRequest => { - const csrObj = new x509.Pkcs10CertificateRequest(csr); - const subject = parseDistinguishedName(csrObj.subject); - - const certificateRequest: TCertificateRequest = { - commonName: subject.commonName, - organization: subject.organization, - organizationUnit: subject.ou, - locality: subject.locality, - state: subject.province, - country: subject.country - }; - - const csrKeyUsageExtension = csrObj.getExtension("2.5.29.15") as x509.KeyUsagesExtension; - if (csrKeyUsageExtension) { - const csrKeyUsages = Object.values(CertKeyUsage).filter( - // eslint-disable-next-line no-bitwise - (keyUsage) => (x509.KeyUsageFlags[keyUsage] & csrKeyUsageExtension.usages) !== 0 - ); - certificateRequest.keyUsages = csrKeyUsages.map(mapLegacyKeyUsageToStandard); - } - - const csrExtendedKeyUsageExtension = csrObj.getExtension("2.5.29.37") as x509.ExtendedKeyUsageExtension; - if (csrExtendedKeyUsageExtension) { - const csrExtendedKeyUsages = csrExtendedKeyUsageExtension.usages.map( - (ekuOid) => CertExtendedKeyUsageOIDToName[ekuOid as string] - ); - certificateRequest.extendedKeyUsages = csrExtendedKeyUsages.map(mapLegacyExtendedKeyUsageToStandard); - } - - const sanExtension = csrObj.extensions.find((ext) => ext.type === "2.5.29.17"); - if (sanExtension) { - const sanNames = new x509.GeneralNames(sanExtension.value); - const altNamesArray: TAltNameMapping[] = sanNames.items - .filter( - (value) => - value.type === TAltNameType.EMAIL || - value.type === TAltNameType.DNS || - value.type === TAltNameType.IP || - value.type === TAltNameType.URL - ) - .map((name): TAltNameMapping => { - const altNameType = validateAndMapAltNameType(name.value); - if (!altNameType) { - throw new BadRequestError({ message: `Invalid altName from CSR: ${name.value}` }); - } - return altNameType; - }); - - certificateRequest.subjectAlternativeNames = altNamesArray.map((altName) => ({ - type: mapLegacyAltNameType(altName.type), - value: altName.value - })); - } - - return certificateRequest; - }; const simpleEnrollByProfile = async ({ csr, profileId, diff --git a/backend/src/services/certificate-v3/certificate-v3-service.test.ts b/backend/src/services/certificate-v3/certificate-v3-service.test.ts index 2dab87b4a..0c70571dd 100644 --- a/backend/src/services/certificate-v3/certificate-v3-service.test.ts +++ b/backend/src/services/certificate-v3/certificate-v3-service.test.ts @@ -9,6 +9,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { TPermissionServiceFactory } from "@app/ee/services/permission/permission-service-types"; import { BadRequestError, ForbiddenRequestError, NotFoundError } from "@app/lib/errors"; import { TCertificateDALFactory } from "@app/services/certificate/certificate-dal"; +import { TCertificateSecretDALFactory } from "@app/services/certificate/certificate-secret-dal"; import { ACMESANType, CertificateOrderStatus, CertStatus } from "@app/services/certificate/certificate-types"; import { TCertificateAuthorityDALFactory } from "@app/services/certificate-authority/certificate-authority-dal"; import { CaStatus } from "@app/services/certificate-authority/certificate-authority-enums"; @@ -24,8 +25,17 @@ import { EnrollmentType } from "@app/services/certificate-profile/certificate-pr import { TCertificateTemplateV2ServiceFactory } from "@app/services/certificate-template-v2/certificate-template-v2-service"; import { ActorType, AuthMethod } from "../auth/auth-type"; +import { + extractAlgorithmsFromCSR, + extractCertificateRequestFromCSR +} from "../certificate-common/certificate-csr-utils"; import { certificateV3ServiceFactory, TCertificateV3ServiceFactory } from "./certificate-v3-service"; +vi.mock("../certificate-common/certificate-csr-utils", () => ({ + extractCertificateRequestFromCSR: vi.fn(), + extractAlgorithmsFromCSR: vi.fn() +})); + describe("CertificateV3Service", () => { let service: TCertificateV3ServiceFactory; @@ -39,6 +49,10 @@ describe("CertificateV3Service", () => { }) }; + const mockCertificateSecretDAL: Pick = { + findOne: vi.fn() + }; + const mockCertificateAuthorityDAL: Pick = { findByIdWithAssociatedCa: vi.fn() }; @@ -101,8 +115,20 @@ describe("CertificateV3Service", () => { } }); + vi.mocked(extractCertificateRequestFromCSR).mockReturnValue({ + commonName: "test.example.com", + keyUsages: [CertKeyUsageType.DIGITAL_SIGNATURE], + extendedKeyUsages: [CertExtendedKeyUsageType.SERVER_AUTH] + }); + + vi.mocked(extractAlgorithmsFromCSR).mockReturnValue({ + keyAlgorithm: "RSA_2048" as any, + signatureAlgorithm: "RSA-SHA256" as any + }); + service = certificateV3ServiceFactory({ certificateDAL: mockCertificateDAL, + certificateSecretDAL: mockCertificateSecretDAL, certificateAuthorityDAL: mockCertificateAuthorityDAL, certificateProfileDAL: mockCertificateProfileDAL, certificateTemplateV2Service: mockCertificateTemplateV2Service, @@ -647,6 +673,11 @@ describe("CertificateV3Service", () => { vi.mocked(mockCertificateProfileDAL.findByIdWithConfigs).mockResolvedValue(mockProfile); vi.mocked(mockCertificateAuthorityDAL.findByIdWithAssociatedCa).mockResolvedValue(mockCA); vi.mocked(mockCertificateTemplateV2Service.getTemplateV2ById).mockResolvedValue(mockTemplate); + vi.mocked(mockCertificateTemplateV2Service.validateCertificateRequest).mockResolvedValue({ + isValid: true, + errors: [], + warnings: [] + }); vi.mocked(mockInternalCaService.signCertFromCa).mockResolvedValue(mockSignResult as any); vi.mocked(mockCertificateDAL.findOne).mockResolvedValue(mockCertRecord); vi.mocked(mockCertificateDAL.updateById).mockResolvedValue(mockCertRecord); @@ -1586,6 +1617,7 @@ describe("CertificateV3Service", () => { it("should successfully renew eligible certificate", async () => { // Mock the initial findById call vi.mocked(mockCertificateDAL.findById).mockResolvedValue(mockOriginalCert); + 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(mockCertificateTemplateV2Service.getTemplateV2ById).mockResolvedValue(mockTemplate); @@ -1650,6 +1682,7 @@ describe("CertificateV3Service", () => { errors: ["Subject alternative name not allowed"], warnings: [] }); + vi.mocked(mockCertificateSecretDAL.findOne).mockResolvedValue({ id: "secret-123", certId: "cert-123" } as any); // Mock updateById to handle the renewal error logging vi.mocked(mockCertificateDAL.updateById).mockResolvedValue(mockOriginalCert); @@ -1705,11 +1738,37 @@ describe("CertificateV3Service", () => { ).rejects.toThrow("Only certificates issued from a profile can be renewed"); }); + it("should reject renewal if certificate was issued from CSR (external private key)", async () => { + vi.mocked(mockCertificateDAL.findById).mockResolvedValue(mockOriginalCert); + vi.mocked(mockCertificateProfileDAL.findByIdWithConfigs).mockResolvedValue(mockProfile); + vi.mocked(mockCertificateSecretDAL.findOne).mockResolvedValue(null as any); + + vi.mocked(mockCertificateDAL.transaction).mockImplementation(async (callback: (tx: any) => Promise) => { + const mockTx = {}; + return callback(mockTx); + }); + + await expect( + service.renewCertificate({ + certificateId: "cert-123", + ...mockActor + }) + ).rejects.toThrow(ForbiddenRequestError); + + await expect( + service.renewCertificate({ + certificateId: "cert-123", + ...mockActor + }) + ).rejects.toThrow("certificates issued from CSR (external private key) cannot be renewed"); + }); + it("should reject renewal if certificate is already renewed", async () => { const alreadyRenewedCert = { ...mockOriginalCert, renewedByCertificateId: "cert-456" }; vi.mocked(mockCertificateDAL.findById).mockResolvedValue(alreadyRenewedCert); vi.mocked(mockCertificateProfileDAL.findByIdWithConfigs).mockResolvedValue(mockProfile); vi.mocked(mockCertificateAuthorityDAL.findByIdWithAssociatedCa).mockResolvedValue(mockCA); + vi.mocked(mockCertificateSecretDAL.findOne).mockResolvedValue({ id: "secret-123", certId: "cert-123" } as any); // Mock updateById to handle the renewal error logging vi.mocked(mockCertificateDAL.updateById).mockResolvedValue(alreadyRenewedCert); @@ -1743,6 +1802,7 @@ describe("CertificateV3Service", () => { vi.mocked(mockCertificateDAL.findById).mockResolvedValue(expiredCert); vi.mocked(mockCertificateProfileDAL.findByIdWithConfigs).mockResolvedValue(mockProfile); vi.mocked(mockCertificateAuthorityDAL.findByIdWithAssociatedCa).mockResolvedValue(mockCA); + vi.mocked(mockCertificateSecretDAL.findOne).mockResolvedValue({ id: "secret-123", certId: "cert-123" } as any); // Mock updateById to handle the renewal error logging vi.mocked(mockCertificateDAL.updateById).mockResolvedValue(expiredCert); @@ -1776,6 +1836,7 @@ describe("CertificateV3Service", () => { vi.mocked(mockCertificateDAL.findById).mockResolvedValue(revokedCert); vi.mocked(mockCertificateProfileDAL.findByIdWithConfigs).mockResolvedValue(mockProfile); vi.mocked(mockCertificateAuthorityDAL.findByIdWithAssociatedCa).mockResolvedValue(mockCA); + vi.mocked(mockCertificateSecretDAL.findOne).mockResolvedValue({ id: "secret-123", certId: "cert-123" } as any); // Mock updateById to handle the renewal error logging vi.mocked(mockCertificateDAL.updateById).mockResolvedValue(revokedCert); @@ -1806,6 +1867,7 @@ describe("CertificateV3Service", () => { vi.mocked(mockCertificateDAL.findById).mockResolvedValue(mockOriginalCert); vi.mocked(mockCertificateProfileDAL.findByIdWithConfigs).mockResolvedValue(mockProfile); vi.mocked(mockCertificateAuthorityDAL.findByIdWithAssociatedCa).mockResolvedValue(inactiveCA); + vi.mocked(mockCertificateSecretDAL.findOne).mockResolvedValue({ id: "secret-123", certId: "cert-123" } as any); // Mock updateById to handle the renewal error logging vi.mocked(mockCertificateDAL.updateById).mockResolvedValue(mockOriginalCert); @@ -1842,6 +1904,7 @@ describe("CertificateV3Service", () => { vi.mocked(mockCertificateDAL.findById).mockResolvedValue(mockOriginalCert); vi.mocked(mockCertificateProfileDAL.findByIdWithConfigs).mockResolvedValue(mockProfile); vi.mocked(mockCertificateAuthorityDAL.findByIdWithAssociatedCa).mockResolvedValue(shortLivedCA); + vi.mocked(mockCertificateSecretDAL.findOne).mockResolvedValue({ id: "secret-123", certId: "cert-123" } as any); // Mock updateById to handle the renewal error logging vi.mocked(mockCertificateDAL.updateById).mockResolvedValue(mockOriginalCert); @@ -1873,6 +1936,7 @@ describe("CertificateV3Service", () => { vi.mocked(mockCertificateDAL.findById).mockResolvedValue(mockOriginalCert); vi.mocked(mockCertificateProfileDAL.findByIdWithConfigs).mockResolvedValue(mockProfile); vi.mocked(mockCertificateAuthorityDAL.findByIdWithAssociatedCa).mockResolvedValue(mockCA); + vi.mocked(mockCertificateSecretDAL.findOne).mockResolvedValue({ id: "secret-123", certId: "cert-123" } as any); vi.mocked(mockCertificateTemplateV2Service.getTemplateV2ById).mockResolvedValue(mockTemplate); vi.mocked(mockCertificateTemplateV2Service.validateCertificateRequest).mockResolvedValue({ isValid: true, @@ -1929,6 +1993,7 @@ describe("CertificateV3Service", () => { vi.mocked(mockCertificateDAL.findById).mockResolvedValue(mockCert as any); vi.mocked(mockCertificateProfileDAL.findByIdWithConfigs).mockResolvedValue(mockProfile as any); + vi.mocked(mockCertificateSecretDAL.findOne).mockResolvedValue({ id: "secret-123", certId: "cert-123" } as any); vi.mocked(mockCertificateDAL.updateById).mockResolvedValue(mockCert as any); const result = await service.updateRenewalConfig({ @@ -2004,6 +2069,7 @@ describe("CertificateV3Service", () => { vi.mocked(mockCertificateDAL.findById).mockResolvedValue(mockCert as any); vi.mocked(mockCertificateProfileDAL.findByIdWithConfigs).mockResolvedValue(mockProfile as any); + vi.mocked(mockCertificateSecretDAL.findOne).mockResolvedValue({ id: "secret-123", certId: "cert-123" } as any); await expect( service.updateRenewalConfig({ @@ -2048,6 +2114,7 @@ describe("CertificateV3Service", () => { vi.mocked(mockCertificateDAL.findById).mockResolvedValue(mockCert as any); vi.mocked(mockCertificateProfileDAL.findByIdWithConfigs).mockResolvedValue(mockProfile as any); + vi.mocked(mockCertificateSecretDAL.findOne).mockResolvedValue({ id: "secret-123", certId: "cert-123" } as any); await expect( service.updateRenewalConfig({ diff --git a/backend/src/services/certificate-v3/certificate-v3-service.ts b/backend/src/services/certificate-v3/certificate-v3-service.ts index 42980f8a7..0a721b2db 100644 --- a/backend/src/services/certificate-v3/certificate-v3-service.ts +++ b/backend/src/services/certificate-v3/certificate-v3-service.ts @@ -12,6 +12,7 @@ import { import { BadRequestError, ForbiddenRequestError, NotFoundError } from "@app/lib/errors"; import { ActorAuthMethod, ActorType } from "@app/services/auth/auth-type"; import { TCertificateDALFactory } from "@app/services/certificate/certificate-dal"; +import { TCertificateSecretDALFactory } from "@app/services/certificate/certificate-secret-dal"; import { CertExtendedKeyUsage, CertificateOrderStatus, @@ -32,6 +33,10 @@ import { EnrollmentType } from "@app/services/certificate-profile/certificate-pr import { TCertificateTemplateV2ServiceFactory } from "@app/services/certificate-template-v2/certificate-template-v2-service"; import { CertSubjectAlternativeNameType } from "../certificate-common/certificate-constants"; +import { + extractAlgorithmsFromCSR, + extractCertificateRequestFromCSR +} from "../certificate-common/certificate-csr-utils"; import { bufferToString, buildCertificateSubjectFromTemplate, @@ -58,6 +63,7 @@ import { type TCertificateV3ServiceFactoryDep = { certificateDAL: Pick; + certificateSecretDAL: Pick; certificateAuthorityDAL: Pick; certificateProfileDAL: Pick; certificateTemplateV2Service: Pick< @@ -296,8 +302,28 @@ const parseTtlToDays = (ttl: string): number => { } }; +const calculateFinalRenewBeforeDays = ( + profile: { apiConfig?: { autoRenew?: boolean; renewBeforeDays?: number } }, + ttl: string, + certificateExpiryDate: Date +): number | undefined => { + if (!profile.apiConfig?.autoRenew || !profile.apiConfig.renewBeforeDays) { + return undefined; + } + + const certificateTtlInDays = parseTtlToDays(ttl); + const renewBeforeDays = calculateRenewalThreshold(profile.apiConfig.renewBeforeDays, certificateTtlInDays); + + if (!renewBeforeDays) { + return undefined; + } + + return isValidRenewalTiming(renewBeforeDays, certificateExpiryDate) ? renewBeforeDays : undefined; +}; + export const certificateV3ServiceFactory = ({ certificateDAL, + certificateSecretDAL, certificateAuthorityDAL, certificateProfileDAL, certificateTemplateV2Service, @@ -412,11 +438,11 @@ export const certificateV3ServiceFactory = ({ throw new NotFoundError({ message: "Certificate was issued but could not be found in database" }); } - const certificateTtlInDays = parseTtlToDays(certificateRequest.validity.ttl); - const renewBeforeDays = calculateRenewalThreshold(profile.apiConfig?.renewBeforeDays, certificateTtlInDays); - - const finalRenewBeforeDays = - renewBeforeDays && isValidRenewalTiming(renewBeforeDays, new Date(cert.notAfter)) ? renewBeforeDays : undefined; + const finalRenewBeforeDays = calculateFinalRenewBeforeDays( + profile, + certificateRequest.validity.ttl, + new Date(cert.notAfter) + ); await certificateDAL.updateById(cert.id, { profileId, @@ -442,8 +468,6 @@ export const certificateV3ServiceFactory = ({ validity, notBefore, notAfter, - signatureAlgorithm, - keyAlgorithm, actor, actorId, actorAuthMethod, @@ -480,22 +504,27 @@ export const certificateV3ServiceFactory = ({ throw new NotFoundError({ message: "Certificate template not found for this profile" }); } + const certificateRequest = extractCertificateRequestFromCSR(csr); + const mappedCertificateRequest = mapEnumsForValidation(certificateRequest); + + const { keyAlgorithm: extractedKeyAlgorithm, signatureAlgorithm: extractedSignatureAlgorithm } = + extractAlgorithmsFromCSR(csr); + + const validationResult = await certificateTemplateV2Service.validateCertificateRequest( + profile.certificateTemplateId, + mappedCertificateRequest + ); + + if (!validationResult.isValid) { + throw new BadRequestError({ + message: `Certificate request validation failed: ${validationResult.errors.join(", ")}` + }); + } + validateAlgorithmCompatibility(ca, template); - const effectiveSignatureAlgorithm = signatureAlgorithm; - const effectiveKeyAlgorithm = keyAlgorithm; - - if (template.algorithms?.keyAlgorithm && !effectiveKeyAlgorithm) { - throw new BadRequestError({ - message: "Key algorithm is required by template policy but not provided in request" - }); - } - - if (template.algorithms?.signature && !effectiveSignatureAlgorithm) { - throw new BadRequestError({ - message: "Signature algorithm is required by template policy but not provided in request" - }); - } + const effectiveSignatureAlgorithm = extractedSignatureAlgorithm; + const effectiveKeyAlgorithm = extractedKeyAlgorithm; const { certificate, certificateChain, issuingCaCertificate, serialNumber } = await internalCaService.signCertFromCa({ @@ -516,11 +545,7 @@ export const certificateV3ServiceFactory = ({ throw new NotFoundError({ message: "Certificate was signed but could not be found in database" }); } - const certificateTtlInDays = parseTtlToDays(validity.ttl); - const renewBeforeDays = calculateRenewalThreshold(profile.apiConfig?.renewBeforeDays, certificateTtlInDays); - - const finalRenewBeforeDays = - renewBeforeDays && isValidRenewalTiming(renewBeforeDays, new Date(cert.notAfter)) ? renewBeforeDays : undefined; + const finalRenewBeforeDays = calculateFinalRenewBeforeDays(profile, validity.ttl, new Date(cert.notAfter)); await certificateDAL.updateById(cert.id, { profileId, @@ -675,6 +700,14 @@ export const certificateV3ServiceFactory = ({ }); } + const certificateSecret = await certificateSecretDAL.findOne({ certId: originalCert.id }, tx); + if (!certificateSecret) { + throw new ForbiddenRequestError({ + message: + "Certificate is not eligible for renewal: certificates issued from CSR (external private key) cannot be renewed" + }); + } + if (!internal) { const { permission } = await permissionService.getProjectPermission({ actor, @@ -791,8 +824,7 @@ export const certificateV3ServiceFactory = ({ const notBefore = new Date(); const notAfter = new Date(Date.now() + parseTtlToDays(ttl) * 24 * 60 * 60 * 1000); - const certificateTtlInDays = parseTtlToDays(ttl); - const finalRenewBeforeDays = calculateRenewalThreshold(profile.apiConfig?.renewBeforeDays, certificateTtlInDays); + const finalRenewBeforeDays = calculateFinalRenewBeforeDays(profile, ttl, notAfter); const { certificate, certificateChain, issuingCaCertificate, serialNumber } = await internalCaService.issueCertFromCa({ @@ -907,6 +939,14 @@ export const certificateV3ServiceFactory = ({ }); } + const certificateSecret = await certificateSecretDAL.findOne({ certId: certificate.id }); + if (!certificateSecret) { + throw new ForbiddenRequestError({ + message: + "Certificate is not eligible for auto-renewal: certificates issued from CSR (external private key) cannot be auto-renewed" + }); + } + if (certificate.status !== CertStatus.ACTIVE) { throw new BadRequestError({ message: `Certificate is not eligible for auto-renewal: certificate status is ${certificate.status}, must be active` diff --git a/backend/src/services/certificate-v3/certificate-v3-types.ts b/backend/src/services/certificate-v3/certificate-v3-types.ts index 9bbc7f743..a62a25b73 100644 --- a/backend/src/services/certificate-v3/certificate-v3-types.ts +++ b/backend/src/services/certificate-v3/certificate-v3-types.ts @@ -35,8 +35,6 @@ export type TSignCertificateFromProfileDTO = { }; notBefore?: Date; notAfter?: Date; - signatureAlgorithm?: string; - keyAlgorithm?: string; } & Omit; export type TOrderCertificateFromProfileDTO = { diff --git a/backend/src/services/certificate/certificate-dal.ts b/backend/src/services/certificate/certificate-dal.ts index d69f063ff..eb40b85a5 100644 --- a/backend/src/services/certificate/certificate-dal.ts +++ b/backend/src/services/certificate/certificate-dal.ts @@ -134,6 +134,7 @@ export const certificateDALFactory = (db: TDbClient) => { `${TableName.Certificate}.profileId`, `${TableName.PkiCertificateProfile}.id` ) + .innerJoin(TableName.CertificateSecret, `${TableName.Certificate}.id`, `${TableName.CertificateSecret}.certId`) .where(`${TableName.Certificate}.status`, CertStatus.ACTIVE) .whereNull(`${TableName.Certificate}.renewedByCertificateId`) .whereNull(`${TableName.Certificate}.renewalError`) @@ -157,6 +158,42 @@ export const certificateDALFactory = (db: TDbClient) => { } }; + const findWithPrivateKeyInfo = async ( + filter: Partial, + options?: { offset?: number; limit?: number; sort?: [string, "asc" | "desc"][] } + ): Promise<(TCertificates & { hasPrivateKey: boolean })[]> => { + try { + let query = db + .replicaNode()(TableName.Certificate) + .leftJoin(TableName.CertificateSecret, `${TableName.Certificate}.id`, `${TableName.CertificateSecret}.certId`) + .select(selectAllTableCols(TableName.Certificate)) + .select(db.ref(`${TableName.CertificateSecret}.certId`).as("privateKeyRef")) + .where(filter); + + if (options?.offset) { + query = query.offset(options.offset); + } + if (options?.limit) { + query = query.limit(options.limit); + } + if (options?.sort) { + options.sort.forEach(([column, direction]) => { + query = query.orderBy(column, direction); + }); + } + + const results = await query; + return results.map((row) => { + return { + ...row, + hasPrivateKey: row.privateKeyRef !== null + }; + }); + } catch (error) { + throw new DatabaseError({ error, name: "Find certificates with private key info" }); + } + }; + return { ...certificateOrm, countCertificatesInProject, @@ -164,6 +201,7 @@ export const certificateDALFactory = (db: TDbClient) => { findLatestActiveCertForSubscriber, findAllActiveCertsForSubscriber, findExpiredSyncedCertificates, - findCertificatesEligibleForRenewal + findCertificatesEligibleForRenewal, + findWithPrivateKeyInfo }; }; diff --git a/backend/src/services/project/project-service.ts b/backend/src/services/project/project-service.ts index f38764dd9..05b007a6a 100644 --- a/backend/src/services/project/project-service.ts +++ b/backend/src/services/project/project-service.ts @@ -154,7 +154,7 @@ type TProjectServiceFactoryDep = { >; pkiSubscriberDAL: Pick; certificateAuthorityDAL: Pick; - certificateDAL: Pick; + certificateDAL: Pick; certificateTemplateDAL: Pick; pkiAlertDAL: Pick; pkiCollectionDAL: Pick; @@ -938,7 +938,7 @@ export const projectServiceFactory = ({ ProjectPermissionSub.Certificates ); - const certificates = await certificateDAL.find( + const certificates = await certificateDAL.findWithPrivateKeyInfo( { projectId, ...(friendlyName && { friendlyName }), diff --git a/frontend/src/hooks/api/certificates/types.ts b/frontend/src/hooks/api/certificates/types.ts index d38fcace0..622276e24 100644 --- a/frontend/src/hooks/api/certificates/types.ts +++ b/frontend/src/hooks/api/certificates/types.ts @@ -19,6 +19,7 @@ export type TCertificate = { renewedFromCertificateId?: string; renewedByCertificateId?: string; renewalError?: string; + hasPrivateKey?: boolean; }; export type TDeleteCertDTO = { diff --git a/frontend/src/pages/cert-manager/CertificatesPage/components/CertificatesTable.tsx b/frontend/src/pages/cert-manager/CertificatesPage/components/CertificatesTable.tsx index 6114583d6..11aa2356e 100644 --- a/frontend/src/pages/cert-manager/CertificatesPage/components/CertificatesTable.tsx +++ b/frontend/src/pages/cert-manager/CertificatesPage/components/CertificatesTable.tsx @@ -70,7 +70,7 @@ const getAutoRenewalInfo = (certificate: TCertificate) => { return { text: "Not Available", variant: "instance" as const, - tooltip: "Auto-renewal is not available for revoked certificates" + tooltip: "Renewal is not available for revoked certificates" }; } @@ -78,7 +78,7 @@ const getAutoRenewalInfo = (certificate: TCertificate) => { return { text: "Not Available", variant: "instance" as const, - tooltip: "Auto-renewal is not available for expired certificates" + tooltip: "Renewal is not available for expired certificates" }; } @@ -86,7 +86,15 @@ const getAutoRenewalInfo = (certificate: TCertificate) => { return { text: "Not Available", variant: "instance" as const, - tooltip: "Auto-renewal requires a certificate profile" + tooltip: "Renewal requires a certificate profile" + }; + } + + if (certificate.hasPrivateKey === false) { + return { + text: "Not Available", + variant: "instance" as const, + tooltip: "Renewal is not available for certificates with externally generated private keys" }; } @@ -344,6 +352,7 @@ export const CertificatesTable = ({ handlePopUpOpen }: Props) => { {(() => { const canManageRenewal = certificate.profileId && + certificate.hasPrivateKey !== false && !certificate.renewedByCertificateId && !isRevoked && !isExpired && @@ -407,6 +416,7 @@ export const CertificatesTable = ({ handlePopUpOpen }: Props) => { {(() => { const canDisableRenewal = certificate.profileId && + certificate.hasPrivateKey !== false && !certificate.renewedByCertificateId && !isRevoked && !isExpired && @@ -445,6 +455,7 @@ export const CertificatesTable = ({ handlePopUpOpen }: Props) => { {(() => { const canRenew = certificate.profileId && + certificate.hasPrivateKey !== false && !certificate.renewedByCertificateId && !isRevoked && !isExpired;