From 00f86cfd00781896a7ed9d9695c98cf25c2e3eb0 Mon Sep 17 00:00:00 2001 From: Sheen Capadngan Date: Mon, 26 Aug 2024 21:10:29 +0800 Subject: [PATCH] misc: addressed review comments --- ..._certificate-template-est-configuration.ts | 2 +- .../certificate-authority-service.ts | 3 - .../certificate-est/certificate-est-fns.ts | 22 ++++++ .../certificate-est-service.ts | 70 ++++++------------- .../certificate-template-service.ts | 27 +++++++ .../services/certificate/certificate-fns.ts | 21 ++++++ .../CertificateTemplatesSection.tsx | 2 +- 7 files changed, 93 insertions(+), 54 deletions(-) create mode 100644 backend/src/services/certificate-est/certificate-est-fns.ts diff --git a/backend/src/db/migrations/20240819092916_certificate-template-est-configuration.ts b/backend/src/db/migrations/20240819092916_certificate-template-est-configuration.ts index 4aaff2c8c..38dae1710 100644 --- a/backend/src/db/migrations/20240819092916_certificate-template-est-configuration.ts +++ b/backend/src/db/migrations/20240819092916_certificate-template-est-configuration.ts @@ -12,7 +12,7 @@ export async function up(knex: Knex): Promise { tb.foreign("certificateTemplateId").references("id").inTable(TableName.CertificateTemplate).onDelete("CASCADE"); tb.binary("encryptedCaChain").notNullable(); tb.string("hashedPassphrase").notNullable(); - tb.boolean("isEnabled"); + tb.boolean("isEnabled").notNullable(); tb.timestamps(true, true, true); }); diff --git a/backend/src/services/certificate-authority/certificate-authority-service.ts b/backend/src/services/certificate-authority/certificate-authority-service.ts index 4a72cefaa..dd7815820 100644 --- a/backend/src/services/certificate-authority/certificate-authority-service.ts +++ b/backend/src/services/certificate-authority/certificate-authority-service.ts @@ -1554,9 +1554,6 @@ export const certificateAuthorityServiceFactory = ({ return { certificate: leafCert, - // certificate: leafCert.toString("pem"), - // certificateObj: leafCert, - // rawCertificate: leafCert.rawData, certificateChain: `${issuingCaCertificate}\n${caCertChain}`.trim(), issuingCaCertificate, serialNumber, diff --git a/backend/src/services/certificate-est/certificate-est-fns.ts b/backend/src/services/certificate-est/certificate-est-fns.ts new file mode 100644 index 000000000..91e72e840 --- /dev/null +++ b/backend/src/services/certificate-est/certificate-est-fns.ts @@ -0,0 +1,22 @@ +import { Certificate, ContentInfo, EncapsulatedContentInfo, SignedData } from "pkijs"; + +export const convertRawCertsToPkcs7 = (rawCertificate: ArrayBuffer[]) => { + const certs = rawCertificate.map((rawCert) => Certificate.fromBER(rawCert)); + const cmsSigned = new SignedData({ + encapContentInfo: new EncapsulatedContentInfo({ + eContentType: "1.2.840.113549.1.7.1" // not encrypted and not compressed data + }), + certificates: certs + }); + + const cmsContent = new ContentInfo({ + contentType: "1.2.840.113549.1.7.2", // SignedData + // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment + content: cmsSigned.toSchema() + }); + + const derBuffer = cmsContent.toSchema().toBER(false); + const base64Pkcs7 = Buffer.from(derBuffer).toString("base64"); + + return base64Pkcs7; +}; diff --git a/backend/src/services/certificate-est/certificate-est-service.ts b/backend/src/services/certificate-est/certificate-est-service.ts index 1634d0996..a2d14d744 100644 --- a/backend/src/services/certificate-est/certificate-est-service.ts +++ b/backend/src/services/certificate-est/certificate-est-service.ts @@ -2,6 +2,7 @@ import * as x509 from "@peculiar/x509"; import { BadRequestError, NotFoundError, UnauthorizedError } from "@app/lib/errors"; +import { isCertChainValid } from "../certificate/certificate-fns"; import { TCertificateAuthorityCertDALFactory } from "../certificate-authority/certificate-authority-cert-dal"; import { TCertificateAuthorityDALFactory } from "../certificate-authority/certificate-authority-dal"; import { getCaCertChain, getCaCertChains } from "../certificate-authority/certificate-authority-fns"; @@ -10,6 +11,7 @@ import { TCertificateTemplateDALFactory } from "../certificate-template/certific import { TCertificateTemplateServiceFactory } from "../certificate-template/certificate-template-service"; import { TKmsServiceFactory } from "../kms/kms-service"; import { TProjectDALFactory } from "../project/project-dal"; +import { convertRawCertsToPkcs7 } from "./certificate-est-fns"; type TCertificateEstServiceFactoryDep = { certificateAuthorityService: Pick; @@ -62,15 +64,7 @@ export const certificateEstServiceFactory = ({ throw new UnauthorizedError({ message: "Missing client certificate" }); } - const clientCertBody = leafCertificate - .replace("-----BEGIN CERTIFICATE-----", "") - .replace("-----END CERTIFICATE-----", "") - .replace(/\n/g, "") - .replace(/ /g, "") - .trim(); - - const cert = new x509.X509Certificate(clientCertBody); - + const cert = new x509.X509Certificate(leafCertificate); // We have to assert that the client certificate provided can be traced back to the Root CA const caCertChains = await getCaCertChains({ caId: certTemplate.caId, @@ -80,21 +74,15 @@ export const certificateEstServiceFactory = ({ kmsService }); - const caChainBuilders = caCertChains.map((chain) => { - const caCert = new x509.X509Certificate(chain.certificate); - const caChain = - chain.certificateChain - .match(/-----BEGIN CERTIFICATE-----[\s\S]+?-----END CERTIFICATE-----/g) - ?.map((c) => new x509.X509Certificate(c)) || []; - return new x509.X509ChainBuilder({ - certificates: [caCert, ...caChain] - }); - }); - const verifiedChains = await Promise.all( - caChainBuilders.map(async (caChainBuilder) => { - const chainItems = await caChainBuilder.build(cert); - return chainItems.length === caChainBuilder.certificates.length + 1; + caCertChains.map((chain) => { + const caCert = new x509.X509Certificate(chain.certificate); + const caChain = + chain.certificateChain + .match(/-----BEGIN CERTIFICATE-----[\s\S]+?-----END CERTIFICATE-----/g) + ?.map((c) => new x509.X509Certificate(c)) || []; + + return isCertChainValid([cert, caCert, ...caChain]); }) ); @@ -138,8 +126,7 @@ export const certificateEstServiceFactory = ({ csr }); - const certs = new x509.X509Certificates([certificate]); - return certs.export("base64"); + return convertRawCertsToPkcs7([certificate.rawData]); }; const simpleEnroll = async ({ @@ -173,10 +160,6 @@ export const certificateEstServiceFactory = ({ if (!caCerts) throw new BadRequestError({ message: "Failed to parse certificate chain" }); - const caChain = new x509.X509ChainBuilder({ - certificates: caCerts - }); - const leafCertificate = decodeURIComponent(sslClientCert).match( /-----BEGIN CERTIFICATE-----[\s\S]+?-----END CERTIFICATE-----/g )?.[0]; @@ -185,17 +168,10 @@ export const certificateEstServiceFactory = ({ throw new BadRequestError({ message: "Missing client certificate" }); } - const clientCertBody = leafCertificate - .replace("-----BEGIN CERTIFICATE-----", "") - .replace("-----END CERTIFICATE-----", "") - .replace(/\n/g, "") - .replace(/ /g, "") - .trim(); - - const certObj = new x509.X509Certificate(clientCertBody); - const chainItems = await caChain.build(certObj); - - if (chainItems.length !== caCerts.length + 1) throw new BadRequestError({ message: "Invalid certificate chain" }); + const certObj = new x509.X509Certificate(leafCertificate); + if (!(await isCertChainValid([certObj, ...caCerts]))) { + throw new BadRequestError({ message: "Invalid certificate chain" }); + } const { certificate } = await certificateAuthorityService.signCertFromCa({ isInternal: true, @@ -203,8 +179,7 @@ export const certificateEstServiceFactory = ({ csr }); - const certs = new x509.X509Certificates([certificate]); - return certs.export("base64"); + return convertRawCertsToPkcs7([certificate.rawData]); }; /** @@ -240,16 +215,13 @@ export const certificateEstServiceFactory = ({ if (!certificates) throw new BadRequestError({ message: "Failed to parse certificate chain" }); - const chain = new x509.X509ChainBuilder({ - certificates - }); + const caCertificate = new x509.X509Certificate(caCert); - const chainItems = await chain.build(new x509.X509Certificate(caCert)); - - if (chainItems.length !== certificates.length + 1) + if (!(await isCertChainValid([caCertificate, ...certificates]))) { throw new BadRequestError({ message: "Invalid certificate chain" }); + } - return chainItems.export("base64"); + return convertRawCertsToPkcs7([caCertificate.rawData, ...certificates.map((cert) => cert.rawData)]); }; return { diff --git a/backend/src/services/certificate-template/certificate-template-service.ts b/backend/src/services/certificate-template/certificate-template-service.ts index cb1caa39f..31745e752 100644 --- a/backend/src/services/certificate-template/certificate-template-service.ts +++ b/backend/src/services/certificate-template/certificate-template-service.ts @@ -1,4 +1,5 @@ import { ForbiddenError } from "@casl/ability"; +import * as x509 from "@peculiar/x509"; import bcrypt from "bcrypt"; import { TCertificateTemplateEstConfigsUpdate } from "@app/db/schemas"; @@ -7,6 +8,7 @@ import { ProjectPermissionActions, ProjectPermissionSub } from "@app/ee/services import { getConfig } from "@app/lib/config/env"; import { BadRequestError, NotFoundError } from "@app/lib/errors"; +import { isCertChainValid } from "../certificate/certificate-fns"; import { TCertificateAuthorityDALFactory } from "../certificate-authority/certificate-authority-dal"; import { TKmsServiceFactory } from "../kms/kms-service"; import { TProjectDALFactory } from "../project/project-dal"; @@ -241,6 +243,19 @@ export const certificateTemplateServiceFactory = ({ kmsService }); + // validate CA chain + const certificates = caChain + .match(/-----BEGIN CERTIFICATE-----[\s\S]+?-----END CERTIFICATE-----/g) + ?.map((cert) => new x509.X509Certificate(cert)); + + if (!certificates) { + throw new BadRequestError({ message: "Failed to parse certificate chain" }); + } + + if (!(await isCertChainValid(certificates))) { + throw new BadRequestError({ message: "Invalid certificate chain" }); + } + const kmsEncryptor = await kmsService.encryptWithKmsKey({ kmsId: certificateManagerKmsId }); @@ -313,6 +328,18 @@ export const certificateTemplateServiceFactory = ({ }; if (caChain) { + const certificates = caChain + .match(/-----BEGIN CERTIFICATE-----[\s\S]+?-----END CERTIFICATE-----/g) + ?.map((cert) => new x509.X509Certificate(cert)); + + if (!certificates) { + throw new BadRequestError({ message: "Failed to parse certificate chain" }); + } + + if (!(await isCertChainValid(certificates))) { + throw new BadRequestError({ message: "Invalid certificate chain" }); + } + const kmsEncryptor = await kmsService.encryptWithKmsKey({ kmsId: certificateManagerKmsId }); diff --git a/backend/src/services/certificate/certificate-fns.ts b/backend/src/services/certificate/certificate-fns.ts index dfbd50551..0da09f200 100644 --- a/backend/src/services/certificate/certificate-fns.ts +++ b/backend/src/services/certificate/certificate-fns.ts @@ -24,3 +24,24 @@ export const revocationReasonToCrlCode = (crlReason: CrlReason) => { return x509.X509CrlReason.unspecified; } }; + +export const isCertChainValid = async (certificates: x509.X509Certificate[]) => { + if (certificates.length === 1) { + return true; + } + + // check for self-signed + if (certificates.length === 2 && certificates[0].equal(certificates[1])) { + return true; + } + + const leafCert = certificates[0]; + const chain = new x509.X509ChainBuilder({ + certificates: certificates.slice(1) + }); + + const chainItems = await chain.build(leafCert); + + // chain.build() implicitly verifies the chain + return chainItems.length === certificates.length; +}; diff --git a/frontend/src/views/Project/CertificatesPage/components/CertificatesTab/components/CertificateTemplatesSection.tsx b/frontend/src/views/Project/CertificatesPage/components/CertificatesTab/components/CertificateTemplatesSection.tsx index b74ab201a..a3451bed2 100644 --- a/frontend/src/views/Project/CertificatesPage/components/CertificatesTab/components/CertificateTemplatesSection.tsx +++ b/frontend/src/views/Project/CertificatesPage/components/CertificatesTab/components/CertificateTemplatesSection.tsx @@ -54,7 +54,7 @@ export const CertificateTemplatesSection = () => {

Certificate Templates

{(isAllowed) => (