review fixes

backend/src/db/migrations/20250429232917_store-cert-secret-key-and-chain.ts

@@ -21,7 +21,7 @@ export async function up(knex: Knex): Promise<void> {
 }
 
 export async function down(knex: Knex): Promise<void> {
-  if (await knex.schema.hasTable(TableName.Certificate)) {
+  if (await knex.schema.hasTable(TableName.CertificateSecret)) {
     await knex.schema.dropTable(TableName.CertificateSecret);
   }
 

backend/src/server/routes/v1/certificate-router.ts

@@ -1,3 +1,4 @@
+/* eslint-disable @typescript-eslint/no-floating-promises */
 import { z } from "zod";
 
 import { CertificatesSchema } from "@app/db/schemas";
@@ -83,7 +84,7 @@ export const registerCertRouter = async (server: FastifyZodProvider) => {
         200: z.string().trim()
       }
     },
-    handler: async (req) => {
+    handler: async (req, reply) => {
       const { ca, cert, certPrivateKey } = await server.services.certificate.getCertPrivateKey({
         serialNumber: req.params.serialNumber,
         actor: req.permission.type,
@@ -105,6 +106,12 @@ export const registerCertRouter = async (server: FastifyZodProvider) => {
         }
       });
 
+      // Prevent proxies from caching sensitive data (private key)
+      reply.header("Cache-Control", "no-store, no-cache, must-revalidate, proxy-revalidate");
+      reply.header("Pragma", "no-cache");
+      reply.header("Expires", "0");
+      reply.header("Surrogate-Control", "no-store");
+
       return certPrivateKey;
     }
   });
@@ -128,12 +135,12 @@ export const registerCertRouter = async (server: FastifyZodProvider) => {
         200: z.object({
           certificate: z.string().trim().describe(CERTIFICATES.GET_CERT.certificate),
           certificateChain: z.string().trim().describe(CERTIFICATES.GET_CERT.certificateChain),
-          privateKey: z.string().trim().describe(CERTIFICATES.GET_CERT.certificateChain),
+          privateKey: z.string().trim().describe(CERTIFICATES.GET_CERT.privateKey),
           serialNumber: z.string().trim().describe(CERTIFICATES.GET_CERT.serialNumberRes)
         })
       }
     },
-    handler: async (req) => {
+    handler: async (req, reply) => {
       const { certificate, certificateChain, serialNumber, cert, ca, privateKey } =
         await server.services.certificate.getCertBundle({
           serialNumber: req.params.serialNumber,
@@ -156,6 +163,12 @@ export const registerCertRouter = async (server: FastifyZodProvider) => {
         }
       });
 
+      // Prevent proxies from caching sensitive data (private key)
+      reply.header("Cache-Control", "no-store, no-cache, must-revalidate, proxy-revalidate");
+      reply.header("Pragma", "no-cache");
+      reply.header("Expires", "0");
+      reply.header("Surrogate-Control", "no-store");
+
       return {
         certificate,
         certificateChain,

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

@@ -2,10 +2,10 @@ import crypto from "node:crypto";
 
 import * as x509 from "@peculiar/x509";
 
-import { NotFoundError } from "@app/lib/errors";
+import { BadRequestError, NotFoundError } from "@app/lib/errors";
 
 import { getProjectKmsCertificateKeyId } from "../project/project-fns";
-import { CrlReason, TGetCertificateCredentialsDTO } from "./certificate-types";
+import { CrlReason, TBuildCertificateChainDTO, TGetCertificateCredentialsDTO } from "./certificate-types";
 
 export const revocationReasonToCrlCode = (crlReason: CrlReason) => {
   switch (crlReason) {
@@ -79,15 +79,42 @@ export const getCertificateCredentials = async ({
     cipherTextBlob: certificateSecret.encryptedPrivateKey
   });
 
-  const skObj = crypto.createPrivateKey({ key: decryptedPrivateKey, format: "pem", type: "pkcs8" });
+  try {
-  const certPrivateKey = skObj.export({ format: "pem", type: "pkcs8" }).toString();
+    const skObj = crypto.createPrivateKey({ key: decryptedPrivateKey, format: "pem", type: "pkcs8" });
+    const certPrivateKey = skObj.export({ format: "pem", type: "pkcs8" }).toString();
 
-  const pkObj = crypto.createPublicKey(skObj);
+    const pkObj = crypto.createPublicKey(skObj);
-  const certPublicKey = pkObj.export({ format: "pem", type: "spki" }).toString();
+    const certPublicKey = pkObj.export({ format: "pem", type: "spki" }).toString();
 
-  return {
+    return {
-    certificateSecret,
+      certificateSecret,
-    certPrivateKey,
+      certPrivateKey,
-    certPublicKey
+      certPublicKey
-  };
+    };
+  } catch (error) {
+    throw new BadRequestError({ message: `Failed to process private key for certificate with ID '${certId}'` });
+  }
+};
+
+// If the certificate was generated after ~05/01/25 it will have a encryptedCertificateChain attached to it's body
+// Otherwise we'll fallback to manually building the chain
+export const buildCertificateChain = async ({
+  caCert,
+  caCertChain,
+  encryptedCertificateChain,
+  kmsService,
+  kmsId
+}: TBuildCertificateChainDTO) => {
+  let certificateChain = `${caCert}\n${caCertChain}`.trim();
+
+  // If the certificate was generated after ~05/01/25 it will have a encryptedCertificateChain attached to it's body
+  if (encryptedCertificateChain) {
+    const kmsDecryptor = await kmsService.decryptWithKmsKey({ kmsId });
+    const decryptedCertChain = await kmsDecryptor({
+      cipherTextBlob: encryptedCertificateChain
+    });
+    certificateChain = decryptedCertChain.toString();
+  }
+
+  return certificateChain;
 };

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

@@ -15,7 +15,7 @@ import { TProjectDALFactory } from "@app/services/project/project-dal";
 import { getProjectKmsCertificateKeyId } from "@app/services/project/project-fns";
 
 import { getCaCertChain, rebuildCaCrl } from "../certificate-authority/certificate-authority-fns";
-import { getCertificateCredentials, revocationReasonToCrlCode } from "./certificate-fns";
+import { buildCertificateChain, getCertificateCredentials, revocationReasonToCrlCode } from "./certificate-fns";
 import { TCertificateSecretDALFactory } from "./certificate-secret-dal";
 import {
   CertStatus,
@@ -245,16 +245,13 @@ export const certificateServiceFactory = ({
       kmsService
     });
 
-    let certificateChain = `${caCert}\n${caCertChain}`.trim();
+    const certificateChain = await buildCertificateChain({
-
+      caCert,
-    // If the certificate was generated after ~05/01/25 it will have a encryptedCertificateChain attached to it's body
+      caCertChain,
-    if (certBody.encryptedCertificateChain) {
+      kmsId: certificateManagerKeyId,
-      const decryptedCertChain = await kmsDecryptor({
+      kmsService,
-        cipherTextBlob: certBody.encryptedCertificateChain
+      encryptedCertificateChain: certBody.encryptedCertificateChain || undefined
-      });
+    });
-      const certChainObj = new x509.X509Certificate(decryptedCertChain);
-      certificateChain = certChainObj.toString("pem");
-    }
 
     return {
       certificate: certObj.toString("pem"),
@@ -314,16 +311,13 @@ export const certificateServiceFactory = ({
       kmsService
     });
 
-    let certificateChain = `${caCert}\n${caCertChain}`.trim();
+    const certificateChain = await buildCertificateChain({
-
+      caCert,
-    // If the certificate was generated after ~05/01/25 it will have a encryptedCertificateChain attached to it's body
+      caCertChain,
-    if (certBody.encryptedCertificateChain) {
+      kmsId: certificateManagerKeyId,
-      const decryptedCertChain = await kmsDecryptor({
+      kmsService,
-        cipherTextBlob: certBody.encryptedCertificateChain
+      encryptedCertificateChain: certBody.encryptedCertificateChain || undefined
-      });
+    });
-      const certChainObj = new x509.X509Certificate(decryptedCertChain);
-      certificateChain = certChainObj.toString("pem");
-    }
 
     const { certPrivateKey } = await getCertificateCredentials({
       certId: cert.id,

backend/src/services/certificate/certificate-types.ts

@@ -93,3 +93,11 @@ export type TGetCertificateCredentialsDTO = {
   projectDAL: Pick<TProjectDALFactory, "findOne" | "updateById" | "transaction">;
   kmsService: Pick<TKmsServiceFactory, "decryptWithKmsKey" | "generateKmsKey">;
 };
+
+export type TBuildCertificateChainDTO = {
+  caCert: string;
+  caCertChain: string;
+  encryptedCertificateChain?: Buffer;
+  kmsService: Pick<TKmsServiceFactory, "decryptWithKmsKey">;
+  kmsId: string;
+};