misc: addressed review comments

backend/src/db/migrations/20240819092916_certificate-template-est-configuration.ts

@@ -12,7 +12,7 @@ export async function up(knex: Knex): Promise<void> {
       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);
     });
 

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,

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;
+};

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<TCertificateAuthorityServiceFactory, "signCertFromCa">;
@@ -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 {

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
       });

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;
+};

frontend/src/views/Project/CertificatesPage/components/CertificatesTab/components/CertificateTemplatesSection.tsx

@@ -54,7 +54,7 @@ export const CertificateTemplatesSection = () => {
         <p className="text-xl font-semibold text-mineshaft-100">Certificate Templates</p>
         <ProjectPermissionCan
           I={ProjectPermissionActions.Create}
-          a={ProjectPermissionSub.Certificates}
+          a={ProjectPermissionSub.CertificateTemplates}
         >
           {(isAllowed) => (
             <Button