From e81c49500bcaa905ce61acb399dcdc8c10d20fc1 Mon Sep 17 00:00:00 2001 From: x Date: Tue, 29 Apr 2025 20:34:39 -0400 Subject: [PATCH 01/12] get certificate private key endpoint + migrations --- ...9232917_store-cert-secret-key-and-chain.ts | 33 +++++++++++++ backend/src/db/schemas/certificate-bodies.ts | 3 +- backend/src/db/schemas/certificate-secrets.ts | 5 +- backend/src/db/schemas/organizations.ts | 1 - backend/src/db/schemas/projects.ts | 2 +- .../server/routes/v1/certificate-router.ts | 45 +++++++++++++++++ .../services/certificate/certificate-fns.ts | 48 ++++++++++++++++++- .../certificate/certificate-secret-dal.ts | 10 ++++ .../certificate/certificate-service.ts | 40 +++++++++++++++- .../services/certificate/certificate-types.ts | 12 +++++ 10 files changed, 192 insertions(+), 7 deletions(-) create mode 100644 backend/src/db/migrations/20250429232917_store-cert-secret-key-and-chain.ts create mode 100644 backend/src/services/certificate/certificate-secret-dal.ts diff --git a/backend/src/db/migrations/20250429232917_store-cert-secret-key-and-chain.ts b/backend/src/db/migrations/20250429232917_store-cert-secret-key-and-chain.ts new file mode 100644 index 000000000..2062f9a38 --- /dev/null +++ b/backend/src/db/migrations/20250429232917_store-cert-secret-key-and-chain.ts @@ -0,0 +1,33 @@ +import { Knex } from "knex"; + +import { TableName } from "../schemas"; + +export async function up(knex: Knex): Promise { + if (await knex.schema.hasTable(TableName.CertificateBody)) { + await knex.schema.alterTable(TableName.CertificateBody, (t) => { + t.binary("encryptedCertificateChain").nullable(); + }); + } + + if (!(await knex.schema.hasTable(TableName.CertificateSecret))) { + await knex.schema.createTable(TableName.CertificateSecret, (t) => { + t.uuid("id", { primaryKey: true }).defaultTo(knex.fn.uuid()); + t.timestamps(true, true, true); + t.uuid("certId").notNullable().unique(); + t.foreign("certId").references("id").inTable(TableName.Certificate).onDelete("CASCADE"); + t.binary("encryptedPrivateKey").notNullable(); + }); + } +} + +export async function down(knex: Knex): Promise { + if (await knex.schema.hasTable(TableName.Certificate)) { + await knex.schema.dropTable(TableName.CertificateSecret); + } + + if (await knex.schema.hasTable(TableName.CertificateBody)) { + await knex.schema.alterTable(TableName.CertificateBody, (t) => { + t.dropColumn("encryptedCertificateChain"); + }); + } +} diff --git a/backend/src/db/schemas/certificate-bodies.ts b/backend/src/db/schemas/certificate-bodies.ts index 75afbddbd..10171e383 100644 --- a/backend/src/db/schemas/certificate-bodies.ts +++ b/backend/src/db/schemas/certificate-bodies.ts @@ -14,7 +14,8 @@ export const CertificateBodiesSchema = z.object({ createdAt: z.date(), updatedAt: z.date(), certId: z.string().uuid(), - encryptedCertificate: zodBuffer + encryptedCertificate: zodBuffer, + encryptedCertificateChain: zodBuffer.nullable().optional() }); export type TCertificateBodies = z.infer; diff --git a/backend/src/db/schemas/certificate-secrets.ts b/backend/src/db/schemas/certificate-secrets.ts index f8cad74f1..75e6377b2 100644 --- a/backend/src/db/schemas/certificate-secrets.ts +++ b/backend/src/db/schemas/certificate-secrets.ts @@ -5,6 +5,8 @@ import { z } from "zod"; +import { zodBuffer } from "@app/lib/zod"; + import { TImmutableDBKeys } from "./models"; export const CertificateSecretsSchema = z.object({ @@ -12,8 +14,7 @@ export const CertificateSecretsSchema = z.object({ createdAt: z.date(), updatedAt: z.date(), certId: z.string().uuid(), - pk: z.string(), - sk: z.string() + encryptedPrivateKey: zodBuffer }); export type TCertificateSecrets = z.infer; diff --git a/backend/src/db/schemas/organizations.ts b/backend/src/db/schemas/organizations.ts index 902c564a7..eea1808e0 100644 --- a/backend/src/db/schemas/organizations.ts +++ b/backend/src/db/schemas/organizations.ts @@ -23,7 +23,6 @@ export const OrganizationsSchema = z.object({ defaultMembershipRole: z.string().default("member"), enforceMfa: z.boolean().default(false), selectedMfaMethod: z.string().nullable().optional(), - secretShareSendToAnyone: z.boolean().default(true).nullable().optional(), allowSecretSharingOutsideOrganization: z.boolean().default(true).nullable().optional(), shouldUseNewPrivilegeSystem: z.boolean().default(true), privilegeUpgradeInitiatedByUsername: z.string().nullable().optional(), diff --git a/backend/src/db/schemas/projects.ts b/backend/src/db/schemas/projects.ts index 2403d6cf4..297601fd0 100644 --- a/backend/src/db/schemas/projects.ts +++ b/backend/src/db/schemas/projects.ts @@ -27,7 +27,7 @@ export const ProjectsSchema = z.object({ description: z.string().nullable().optional(), type: z.string(), enforceCapitalization: z.boolean().default(false), - hasDeleteProtection: z.boolean().default(true).nullable().optional() + hasDeleteProtection: z.boolean().default(false).nullable().optional() }); export type TProjects = z.infer; diff --git a/backend/src/server/routes/v1/certificate-router.ts b/backend/src/server/routes/v1/certificate-router.ts index ea33e948f..42c515795 100644 --- a/backend/src/server/routes/v1/certificate-router.ts +++ b/backend/src/server/routes/v1/certificate-router.ts @@ -64,6 +64,51 @@ export const registerCertRouter = async (server: FastifyZodProvider) => { } }); + // TODO(andrey): In the future add support for other formats outside of PEM. Adding a "format" query param may be best. + server.route({ + method: "GET", + url: "/:serialNumber/private-key", + config: { + rateLimit: readLimit + }, + onRequest: verifyAuth([AuthMode.JWT, AuthMode.IDENTITY_ACCESS_TOKEN]), + schema: { + hide: false, + tags: [ApiDocsTags.PkiCertificates], + description: "Get certificate private key", + params: z.object({ + serialNumber: z.string().trim().describe(CERTIFICATES.GET.serialNumber) + }), + response: { + 200: z.string().trim() + } + }, + handler: async (req) => { + const { ca, cert, certPrivateKey } = await server.services.certificate.getCertPrivateKey({ + serialNumber: req.params.serialNumber, + actor: req.permission.type, + actorId: req.permission.id, + actorAuthMethod: req.permission.authMethod, + actorOrgId: req.permission.orgId + }); + + await server.services.auditLog.createAuditLog({ + ...req.auditLogInfo, + projectId: ca.projectId, + event: { + type: EventType.GET_CERT, + metadata: { + certId: cert.id, + cn: cert.commonName, + serialNumber: cert.serialNumber + } + } + }); + + return certPrivateKey; + } + }); + server.route({ method: "POST", url: "/issue-certificate", diff --git a/backend/src/services/certificate/certificate-fns.ts b/backend/src/services/certificate/certificate-fns.ts index 45ad5963c..55b731bd7 100644 --- a/backend/src/services/certificate/certificate-fns.ts +++ b/backend/src/services/certificate/certificate-fns.ts @@ -1,6 +1,11 @@ +import crypto from "node:crypto"; + import * as x509 from "@peculiar/x509"; -import { CrlReason } from "./certificate-types"; +import { NotFoundError } from "@app/lib/errors"; + +import { getProjectKmsCertificateKeyId } from "../project/project-fns"; +import { CrlReason, TGetCertificateCredentialsDTO } from "./certificate-types"; export const revocationReasonToCrlCode = (crlReason: CrlReason) => { switch (crlReason) { @@ -46,3 +51,44 @@ export const constructPemChainFromCerts = (certificates: x509.X509Certificate[]) .map((cert) => cert.toString("pem")) .join("\n") .trim(); + +/** + * Return the public and private key of certificate + * Note: credentials are returned as PEM strings + */ +export const getCertificateCredentials = async ({ + certId, + projectId, + certificateSecretDAL, + projectDAL, + kmsService +}: TGetCertificateCredentialsDTO) => { + const certificateSecret = await certificateSecretDAL.findOne({ certId }); + if (!certificateSecret) + throw new NotFoundError({ message: `Certificate secret for certificate with ID '${certId}' not found` }); + + const keyId = await getProjectKmsCertificateKeyId({ + projectId, + projectDAL, + kmsService + }); + + const kmsDecryptor = await kmsService.decryptWithKmsKey({ + kmsId: keyId + }); + const decryptedPrivateKey = await kmsDecryptor({ + cipherTextBlob: certificateSecret.encryptedPrivateKey + }); + + const skObj = crypto.createPrivateKey({ key: decryptedPrivateKey, format: "der", type: "pkcs8" }); + const certPrivateKey = skObj.export({ format: "pem", type: "pkcs8" }).toString(); + + const pkObj = crypto.createPublicKey(skObj); + const certPublicKey = pkObj.export({ format: "pem", type: "spki" }).toString(); + + return { + certificateSecret, + certPrivateKey, + certPublicKey + }; +}; diff --git a/backend/src/services/certificate/certificate-secret-dal.ts b/backend/src/services/certificate/certificate-secret-dal.ts new file mode 100644 index 000000000..d7f3e43ae --- /dev/null +++ b/backend/src/services/certificate/certificate-secret-dal.ts @@ -0,0 +1,10 @@ +import { TDbClient } from "@app/db"; +import { TableName } from "@app/db/schemas"; +import { ormify } from "@app/lib/knex"; + +export type TCertificateSecretDALFactory = ReturnType; + +export const certificateSecretDALFactory = (db: TDbClient) => { + const caSecretOrm = ormify(db, TableName.CertificateSecret); + return caSecretOrm; +}; diff --git a/backend/src/services/certificate/certificate-service.ts b/backend/src/services/certificate/certificate-service.ts index 0ca0d64c6..d72235781 100644 --- a/backend/src/services/certificate/certificate-service.ts +++ b/backend/src/services/certificate/certificate-service.ts @@ -15,11 +15,13 @@ 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 { revocationReasonToCrlCode } from "./certificate-fns"; +import { getCertificateCredentials, revocationReasonToCrlCode } from "./certificate-fns"; +import { TCertificateSecretDALFactory } from "./certificate-secret-dal"; import { CertStatus, TDeleteCertDTO, TGetCertBodyDTO, TGetCertDTO, TRevokeCertDTO } from "./certificate-types"; type TCertificateServiceFactoryDep = { certificateDAL: Pick; + certificateSecretDAL: Pick; certificateBodyDAL: Pick; certificateAuthorityDAL: Pick; certificateAuthorityCertDAL: Pick; @@ -34,6 +36,7 @@ export type TCertificateServiceFactory = ReturnType { + const cert = await certificateDAL.findOne({ serialNumber }); + const ca = await certificateAuthorityDAL.findById(cert.caId); + + const { permission } = await permissionService.getProjectPermission({ + actor, + actorId, + projectId: ca.projectId, + actorAuthMethod, + actorOrgId, + actionProjectType: ActionProjectType.CertificateManager + }); + + // TODO(andrey): Update permission for privateKey fetching. Should be very strict. + ForbiddenError.from(permission).throwUnlessCan(ProjectPermissionActions.Read, ProjectPermissionSub.Certificates); + + const { certPrivateKey } = await getCertificateCredentials({ + certId: ca.id, + projectId: ca.projectId, + certificateSecretDAL, + projectDAL, + kmsService + }); + + return { + ca, + cert, + certPrivateKey + }; + }; + /** * Delete certificate with serial number [serialNumber] */ @@ -203,6 +240,7 @@ export const certificateServiceFactory = ({ return { getCert, + getCertPrivateKey, deleteCert, revokeCert, getCertBody diff --git a/backend/src/services/certificate/certificate-types.ts b/backend/src/services/certificate/certificate-types.ts index ef63f142d..48454f803 100644 --- a/backend/src/services/certificate/certificate-types.ts +++ b/backend/src/services/certificate/certificate-types.ts @@ -2,6 +2,10 @@ import * as x509 from "@peculiar/x509"; import { TProjectPermission } from "@app/lib/types"; +import { TKmsServiceFactory } from "../kms/kms-service"; +import { TProjectDALFactory } from "../project/project-dal"; +import { TCertificateSecretDALFactory } from "./certificate-secret-dal"; + export enum CertStatus { ACTIVE = "active", REVOKED = "revoked" @@ -73,3 +77,11 @@ export type TRevokeCertDTO = { export type TGetCertBodyDTO = { serialNumber: string; } & Omit; + +export type TGetCertificateCredentialsDTO = { + certId: string; + projectId: string; + certificateSecretDAL: Pick; + projectDAL: Pick; + kmsService: Pick; +}; From e47577491051c9ae75369d134932980867be1403 Mon Sep 17 00:00:00 2001 From: x Date: Wed, 30 Apr 2025 00:33:46 -0400 Subject: [PATCH 02/12] made certificates store PK and chain in relation to the main table, added /bundle endpoints, new audit log and permission entries --- .../ee/services/audit-log/audit-log-types.ts | 22 ++++ .../services/permission/project-permission.ts | 9 ++ backend/src/lib/api-docs/constants.ts | 3 +- backend/src/server/routes/index.ts | 4 + .../server/routes/v1/certificate-router.ts | 62 +++++++++- .../certificate-authority-service.ts | 41 +++++-- .../services/certificate/certificate-fns.ts | 3 +- .../certificate/certificate-secret-dal.ts | 4 +- .../certificate/certificate-service.ts | 111 +++++++++++++++++- .../services/certificate/certificate-types.ts | 4 + 10 files changed, 239 insertions(+), 24 deletions(-) diff --git a/backend/src/ee/services/audit-log/audit-log-types.ts b/backend/src/ee/services/audit-log/audit-log-types.ts index b6527f3f4..93d3f03c3 100644 --- a/backend/src/ee/services/audit-log/audit-log-types.ts +++ b/backend/src/ee/services/audit-log/audit-log-types.ts @@ -215,6 +215,8 @@ export enum EventType { DELETE_CERT = "delete-cert", REVOKE_CERT = "revoke-cert", GET_CERT_BODY = "get-cert-body", + GET_CERT_PRIVATE_KEY = "get-cert-private-key", + GET_CERT_BUNDLE = "get-cert-bundle", CREATE_PKI_ALERT = "create-pki-alert", GET_PKI_ALERT = "get-pki-alert", UPDATE_PKI_ALERT = "update-pki-alert", @@ -1719,6 +1721,24 @@ interface GetCertBody { }; } +interface GetCertPrivateKey { + type: EventType.GET_CERT_PRIVATE_KEY; + metadata: { + certId: string; + cn: string; + serialNumber: string; + }; +} + +interface GetCertBundle { + type: EventType.GET_CERT_BUNDLE; + metadata: { + certId: string; + cn: string; + serialNumber: string; + }; +} + interface CreatePkiAlert { type: EventType.CREATE_PKI_ALERT; metadata: { @@ -2691,6 +2711,8 @@ export type Event = | DeleteCert | RevokeCert | GetCertBody + | GetCertPrivateKey + | GetCertBundle | CreatePkiAlert | GetPkiAlert | UpdatePkiAlert diff --git a/backend/src/ee/services/permission/project-permission.ts b/backend/src/ee/services/permission/project-permission.ts index 8e6645073..7d659e99b 100644 --- a/backend/src/ee/services/permission/project-permission.ts +++ b/backend/src/ee/services/permission/project-permission.ts @@ -129,6 +129,7 @@ export enum ProjectPermissionSub { Identity = "identity", CertificateAuthorities = "certificate-authorities", Certificates = "certificates", + CertificatePrivateKey = "certificate-private-key", CertificateTemplates = "certificate-templates", SshCertificateAuthorities = "ssh-certificate-authorities", SshCertificates = "ssh-certificates", @@ -232,6 +233,7 @@ export type ProjectPermissionSet = ] | [ProjectPermissionActions, ProjectPermissionSub.CertificateAuthorities] | [ProjectPermissionActions, ProjectPermissionSub.Certificates] + | [ProjectPermissionActions, ProjectPermissionSub.CertificatePrivateKey] | [ProjectPermissionActions, ProjectPermissionSub.CertificateTemplates] | [ProjectPermissionActions, ProjectPermissionSub.SshCertificateAuthorities] | [ProjectPermissionActions, ProjectPermissionSub.SshCertificates] @@ -480,6 +482,12 @@ const GeneralPermissionSchema = [ "Describe what action an entity can take." ) }), + z.object({ + subject: z.literal(ProjectPermissionSub.CertificatePrivateKey).describe("The entity this permission pertains to."), + action: CASL_ACTION_SCHEMA_NATIVE_ENUM(ProjectPermissionActions).describe( + "Describe what action an entity can take." + ) + }), z.object({ subject: z.literal(ProjectPermissionSub.CertificateTemplates).describe("The entity this permission pertains to."), action: CASL_ACTION_SCHEMA_NATIVE_ENUM(ProjectPermissionActions).describe( @@ -681,6 +689,7 @@ const buildAdminPermissionRules = () => { ProjectPermissionSub.IpAllowList, ProjectPermissionSub.CertificateAuthorities, ProjectPermissionSub.Certificates, + ProjectPermissionSub.CertificatePrivateKey, ProjectPermissionSub.CertificateTemplates, ProjectPermissionSub.PkiAlerts, ProjectPermissionSub.PkiCollections, diff --git a/backend/src/lib/api-docs/constants.ts b/backend/src/lib/api-docs/constants.ts index 19ee7e331..c8fb3820c 100644 --- a/backend/src/lib/api-docs/constants.ts +++ b/backend/src/lib/api-docs/constants.ts @@ -1580,7 +1580,8 @@ export const CERTIFICATES = { serialNumber: "The serial number of the certificate to get the certificate body and certificate chain for.", certificate: "The certificate body of the certificate.", certificateChain: "The certificate chain of the certificate.", - serialNumberRes: "The serial number of the certificate." + serialNumberRes: "The serial number of the certificate.", + privateKey: "The private key of the certificate." } }; diff --git a/backend/src/server/routes/index.ts b/backend/src/server/routes/index.ts index 8ceeba648..f9a2223ee 100644 --- a/backend/src/server/routes/index.ts +++ b/backend/src/server/routes/index.ts @@ -260,6 +260,7 @@ import { registerSecretScannerGhApp } from "../plugins/secret-scanner"; import { registerV1Routes } from "./v1"; import { registerV2Routes } from "./v2"; import { registerV3Routes } from "./v3"; +import { certificateSecretDALFactory } from "@app/services/certificate/certificate-secret-dal"; const histogram = monitorEventLoopDelay({ resolution: 20 }); histogram.enable(); @@ -791,6 +792,7 @@ export const registerRoutes = async ( const certificateDAL = certificateDALFactory(db); const certificateBodyDAL = certificateBodyDALFactory(db); + const certificateSecretDAL = certificateSecretDALFactory(db); const pkiAlertDAL = pkiAlertDALFactory(db); const pkiCollectionDAL = pkiCollectionDALFactory(db); @@ -799,6 +801,7 @@ export const registerRoutes = async ( const certificateService = certificateServiceFactory({ certificateDAL, certificateBodyDAL, + certificateSecretDAL, certificateAuthorityDAL, certificateAuthorityCertDAL, certificateAuthorityCrlDAL, @@ -858,6 +861,7 @@ export const registerRoutes = async ( certificateAuthorityQueue, certificateDAL, certificateBodyDAL, + certificateSecretDAL, pkiCollectionDAL, pkiCollectionItemDAL, projectDAL, diff --git a/backend/src/server/routes/v1/certificate-router.ts b/backend/src/server/routes/v1/certificate-router.ts index 42c515795..d026868e9 100644 --- a/backend/src/server/routes/v1/certificate-router.ts +++ b/backend/src/server/routes/v1/certificate-router.ts @@ -64,7 +64,7 @@ export const registerCertRouter = async (server: FastifyZodProvider) => { } }); - // TODO(andrey): In the future add support for other formats outside of PEM. Adding a "format" query param may be best. + // TODO: In the future add support for other formats outside of PEM (such as DER). Adding a "format" query param may be best. server.route({ method: "GET", url: "/:serialNumber/private-key", @@ -96,7 +96,7 @@ export const registerCertRouter = async (server: FastifyZodProvider) => { ...req.auditLogInfo, projectId: ca.projectId, event: { - type: EventType.GET_CERT, + type: EventType.GET_CERT_PRIVATE_KEY, metadata: { certId: cert.id, cn: cert.commonName, @@ -109,6 +109,62 @@ export const registerCertRouter = async (server: FastifyZodProvider) => { } }); + // TODO: In the future add support for other formats outside of PEM (such as DER). Adding a "format" query param may be best. + server.route({ + method: "GET", + url: "/:serialNumber/bundle", + config: { + rateLimit: readLimit + }, + onRequest: verifyAuth([AuthMode.JWT, AuthMode.IDENTITY_ACCESS_TOKEN]), + schema: { + hide: false, + tags: [ApiDocsTags.PkiCertificates], + description: "Get certificate bundle including the certificate, chain, and private key.", + params: z.object({ + serialNumber: z.string().trim().describe(CERTIFICATES.GET_CERT.serialNumber) + }), + response: { + 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), + serialNumber: z.string().trim().describe(CERTIFICATES.GET_CERT.serialNumberRes) + }) + } + }, + handler: async (req) => { + const { certificate, certificateChain, serialNumber, cert, ca, privateKey } = + await server.services.certificate.getCertBundle({ + serialNumber: req.params.serialNumber, + actor: req.permission.type, + actorId: req.permission.id, + actorAuthMethod: req.permission.authMethod, + actorOrgId: req.permission.orgId + }); + + await server.services.auditLog.createAuditLog({ + ...req.auditLogInfo, + projectId: ca.projectId, + event: { + type: EventType.GET_CERT_BUNDLE, + metadata: { + certId: cert.id, + cn: cert.commonName, + serialNumber: cert.serialNumber + } + } + }); + + return { + certificate, + certificateChain, + serialNumber, + privateKey + }; + } + }); + server.route({ method: "POST", url: "/issue-certificate", @@ -474,7 +530,7 @@ export const registerCertRouter = async (server: FastifyZodProvider) => { ...req.auditLogInfo, projectId: ca.projectId, event: { - type: EventType.DELETE_CERT, + type: EventType.GET_CERT_BODY, metadata: { certId: cert.id, cn: cert.commonName, diff --git a/backend/src/services/certificate-authority/certificate-authority-service.ts b/backend/src/services/certificate-authority/certificate-authority-service.ts index 499a25741..4c40d9ff4 100644 --- a/backend/src/services/certificate-authority/certificate-authority-service.ts +++ b/backend/src/services/certificate-authority/certificate-authority-service.ts @@ -21,6 +21,7 @@ import { TProjectDALFactory } from "@app/services/project/project-dal"; import { getProjectKmsCertificateKeyId } from "@app/services/project/project-fns"; import { TCertificateAuthorityCrlDALFactory } from "../../ee/services/certificate-authority-crl/certificate-authority-crl-dal"; +import { TCertificateSecretDALFactory } from "../certificate/certificate-secret-dal"; import { CertExtendedKeyUsage, CertExtendedKeyUsageOIDToName, @@ -75,6 +76,7 @@ type TCertificateAuthorityServiceFactoryDep = { certificateTemplateDAL: Pick; certificateAuthorityQueue: TCertificateAuthorityQueueFactory; // TODO: Pick certificateDAL: Pick; + certificateSecretDAL: Pick; certificateBodyDAL: Pick; pkiCollectionDAL: Pick; pkiCollectionItemDAL: Pick; @@ -96,6 +98,7 @@ export const certificateAuthorityServiceFactory = ({ certificateTemplateDAL, certificateDAL, certificateBodyDAL, + certificateSecretDAL, pkiCollectionDAL, pkiCollectionItemDAL, projectDAL, @@ -1373,6 +1376,23 @@ export const certificateAuthorityServiceFactory = ({ const { cipherTextBlob: encryptedCertificate } = await kmsEncryptor({ plainText: Buffer.from(new Uint8Array(leafCert.rawData)) }); + const { cipherTextBlob: encryptedPrivateKey } = await kmsEncryptor({ + plainText: Buffer.from(skLeaf) + }); + + const { caCert: issuingCaCertificate, caCertChain } = await getCaCertChain({ + caCertId: caCert.id, + certificateAuthorityDAL, + certificateAuthorityCertDAL, + projectDAL, + kmsService + }); + + const certificateChainPem = `${issuingCaCertificate}\n${caCertChain}`.trim(); + + const { cipherTextBlob: encryptedCertificateChain } = await kmsEncryptor({ + plainText: Buffer.from(certificateChainPem) + }); await certificateDAL.transaction(async (tx) => { const cert = await certificateDAL.create( @@ -1396,7 +1416,16 @@ export const certificateAuthorityServiceFactory = ({ await certificateBodyDAL.create( { certId: cert.id, - encryptedCertificate + encryptedCertificate, + encryptedCertificateChain + }, + tx + ); + + await certificateSecretDAL.create( + { + certId: cert.id, + encryptedPrivateKey }, tx ); @@ -1414,17 +1443,9 @@ export const certificateAuthorityServiceFactory = ({ return cert; }); - const { caCert: issuingCaCertificate, caCertChain } = await getCaCertChain({ - caCertId: caCert.id, - certificateAuthorityDAL, - certificateAuthorityCertDAL, - projectDAL, - kmsService - }); - return { certificate: leafCert.toString("pem"), - certificateChain: `${issuingCaCertificate}\n${caCertChain}`.trim(), + certificateChain: certificateChainPem, issuingCaCertificate, privateKey: skLeaf, serialNumber, diff --git a/backend/src/services/certificate/certificate-fns.ts b/backend/src/services/certificate/certificate-fns.ts index 55b731bd7..6f9963db7 100644 --- a/backend/src/services/certificate/certificate-fns.ts +++ b/backend/src/services/certificate/certificate-fns.ts @@ -72,7 +72,6 @@ export const getCertificateCredentials = async ({ projectDAL, kmsService }); - const kmsDecryptor = await kmsService.decryptWithKmsKey({ kmsId: keyId }); @@ -80,7 +79,7 @@ export const getCertificateCredentials = async ({ cipherTextBlob: certificateSecret.encryptedPrivateKey }); - const skObj = crypto.createPrivateKey({ key: decryptedPrivateKey, format: "der", type: "pkcs8" }); + const skObj = crypto.createPrivateKey({ key: decryptedPrivateKey, format: "pem", type: "pkcs8" }); const certPrivateKey = skObj.export({ format: "pem", type: "pkcs8" }).toString(); const pkObj = crypto.createPublicKey(skObj); diff --git a/backend/src/services/certificate/certificate-secret-dal.ts b/backend/src/services/certificate/certificate-secret-dal.ts index d7f3e43ae..c1493eceb 100644 --- a/backend/src/services/certificate/certificate-secret-dal.ts +++ b/backend/src/services/certificate/certificate-secret-dal.ts @@ -5,6 +5,6 @@ import { ormify } from "@app/lib/knex"; export type TCertificateSecretDALFactory = ReturnType; export const certificateSecretDALFactory = (db: TDbClient) => { - const caSecretOrm = ormify(db, TableName.CertificateSecret); - return caSecretOrm; + const certSecretOrm = ormify(db, TableName.CertificateSecret); + return certSecretOrm; }; diff --git a/backend/src/services/certificate/certificate-service.ts b/backend/src/services/certificate/certificate-service.ts index d72235781..3568c0ca1 100644 --- a/backend/src/services/certificate/certificate-service.ts +++ b/backend/src/services/certificate/certificate-service.ts @@ -17,7 +17,14 @@ import { getProjectKmsCertificateKeyId } from "@app/services/project/project-fns import { getCaCertChain, rebuildCaCrl } from "../certificate-authority/certificate-authority-fns"; import { getCertificateCredentials, revocationReasonToCrlCode } from "./certificate-fns"; import { TCertificateSecretDALFactory } from "./certificate-secret-dal"; -import { CertStatus, TDeleteCertDTO, TGetCertBodyDTO, TGetCertDTO, TRevokeCertDTO } from "./certificate-types"; +import { + CertStatus, + TDeleteCertDTO, + TGetCertBodyDTO, + TGetCertBundleDTO, + TGetCertDTO, + TRevokeCertDTO +} from "./certificate-types"; type TCertificateServiceFactoryDep = { certificateDAL: Pick; @@ -86,11 +93,13 @@ export const certificateServiceFactory = ({ actionProjectType: ActionProjectType.CertificateManager }); - // TODO(andrey): Update permission for privateKey fetching. Should be very strict. - ForbiddenError.from(permission).throwUnlessCan(ProjectPermissionActions.Read, ProjectPermissionSub.Certificates); + ForbiddenError.from(permission).throwUnlessCan( + ProjectPermissionActions.Read, + ProjectPermissionSub.CertificatePrivateKey + ); const { certPrivateKey } = await getCertificateCredentials({ - certId: ca.id, + certId: cert.id, projectId: ca.projectId, certificateSecretDAL, projectDAL, @@ -229,20 +238,110 @@ export const certificateServiceFactory = ({ kmsService }); + 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 (certBody.encryptedCertificateChain) { + const decryptedCertChain = await kmsDecryptor({ + cipherTextBlob: certBody.encryptedCertificateChain + }); + const certChainObj = new x509.X509Certificate(decryptedCertChain); + certificateChain = certChainObj.toString("pem"); + } + return { certificate: certObj.toString("pem"), - certificateChain: `${caCert}\n${caCertChain}`.trim(), + certificateChain, serialNumber: certObj.serialNumber, cert, ca }; }; + /** + * Return certificate body and certificate chain for certificate with + * serial number [serialNumber] + */ + const getCertBundle = async ({ serialNumber, actorId, actorAuthMethod, actor, actorOrgId }: TGetCertBundleDTO) => { + const cert = await certificateDAL.findOne({ serialNumber }); + const ca = await certificateAuthorityDAL.findById(cert.caId); + + const { permission } = await permissionService.getProjectPermission({ + actor, + actorId, + projectId: ca.projectId, + actorAuthMethod, + actorOrgId, + actionProjectType: ActionProjectType.CertificateManager + }); + + ForbiddenError.from(permission).throwUnlessCan(ProjectPermissionActions.Read, ProjectPermissionSub.Certificates); + ForbiddenError.from(permission).throwUnlessCan( + ProjectPermissionActions.Read, + ProjectPermissionSub.CertificatePrivateKey + ); + + const certBody = await certificateBodyDAL.findOne({ certId: cert.id }); + + const certificateManagerKeyId = await getProjectKmsCertificateKeyId({ + projectId: ca.projectId, + projectDAL, + kmsService + }); + + const kmsDecryptor = await kmsService.decryptWithKmsKey({ + kmsId: certificateManagerKeyId + }); + const decryptedCert = await kmsDecryptor({ + cipherTextBlob: certBody.encryptedCertificate + }); + + const certObj = new x509.X509Certificate(decryptedCert); + const certificate = certObj.toString("pem"); + + const { caCert, caCertChain } = await getCaCertChain({ + caCertId: cert.caCertId, + certificateAuthorityDAL, + certificateAuthorityCertDAL, + projectDAL, + kmsService + }); + + 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 (certBody.encryptedCertificateChain) { + const decryptedCertChain = await kmsDecryptor({ + cipherTextBlob: certBody.encryptedCertificateChain + }); + const certChainObj = new x509.X509Certificate(decryptedCertChain); + certificateChain = certChainObj.toString("pem"); + } + + const { certPrivateKey } = await getCertificateCredentials({ + certId: cert.id, + projectId: ca.projectId, + certificateSecretDAL, + projectDAL, + kmsService + }); + + return { + certificate, + certificateChain, + privateKey: certPrivateKey, + serialNumber, + cert, + ca + }; + }; + return { getCert, getCertPrivateKey, deleteCert, revokeCert, - getCertBody + getCertBody, + getCertBundle }; }; diff --git a/backend/src/services/certificate/certificate-types.ts b/backend/src/services/certificate/certificate-types.ts index 48454f803..a36671030 100644 --- a/backend/src/services/certificate/certificate-types.ts +++ b/backend/src/services/certificate/certificate-types.ts @@ -78,6 +78,10 @@ export type TGetCertBodyDTO = { serialNumber: string; } & Omit; +export type TGetCertBundleDTO = { + serialNumber: string; +} & Omit; + export type TGetCertificateCredentialsDTO = { certId: string; projectId: string; From c70b9e665e349f942a238578dfcb04d83b194d74 Mon Sep 17 00:00:00 2001 From: x Date: Wed, 30 Apr 2025 00:39:10 -0400 Subject: [PATCH 03/12] more tweaks and type fix --- .../certificate/certificate-service.ts | 9 ++++++++- .../services/certificate/certificate-types.ts | 4 ++++ .../context/ProjectPermissionContext/types.ts | 1 + .../src/hooks/api/auditLogs/constants.tsx | 2 ++ frontend/src/hooks/api/auditLogs/enums.tsx | 2 ++ frontend/src/hooks/api/auditLogs/types.tsx | 20 +++++++++++++++++++ 6 files changed, 37 insertions(+), 1 deletion(-) diff --git a/backend/src/services/certificate/certificate-service.ts b/backend/src/services/certificate/certificate-service.ts index 3568c0ca1..c75b01382 100644 --- a/backend/src/services/certificate/certificate-service.ts +++ b/backend/src/services/certificate/certificate-service.ts @@ -23,6 +23,7 @@ import { TGetCertBodyDTO, TGetCertBundleDTO, TGetCertDTO, + TGetCertPrivateKeyDTO, TRevokeCertDTO } from "./certificate-types"; @@ -80,7 +81,13 @@ export const certificateServiceFactory = ({ /** * Get certificate private key. */ - const getCertPrivateKey = async ({ serialNumber, actorId, actorAuthMethod, actor, actorOrgId }: TGetCertDTO) => { + const getCertPrivateKey = async ({ + serialNumber, + actorId, + actorAuthMethod, + actor, + actorOrgId + }: TGetCertPrivateKeyDTO) => { const cert = await certificateDAL.findOne({ serialNumber }); const ca = await certificateAuthorityDAL.findById(cert.caId); diff --git a/backend/src/services/certificate/certificate-types.ts b/backend/src/services/certificate/certificate-types.ts index a36671030..71a53bd3f 100644 --- a/backend/src/services/certificate/certificate-types.ts +++ b/backend/src/services/certificate/certificate-types.ts @@ -78,6 +78,10 @@ export type TGetCertBodyDTO = { serialNumber: string; } & Omit; +export type TGetCertPrivateKeyDTO = { + serialNumber: string; +} & Omit; + export type TGetCertBundleDTO = { serialNumber: string; } & Omit; diff --git a/frontend/src/context/ProjectPermissionContext/types.ts b/frontend/src/context/ProjectPermissionContext/types.ts index f7ba69e17..d2463d766 100644 --- a/frontend/src/context/ProjectPermissionContext/types.ts +++ b/frontend/src/context/ProjectPermissionContext/types.ts @@ -170,6 +170,7 @@ export enum ProjectPermissionSub { Identity = "identity", CertificateAuthorities = "certificate-authorities", Certificates = "certificates", + CertificatePrivateKey = "certificate-private-key", CertificateTemplates = "certificate-templates", SshCertificateAuthorities = "ssh-certificate-authorities", SshCertificateTemplates = "ssh-certificate-templates", diff --git a/frontend/src/hooks/api/auditLogs/constants.tsx b/frontend/src/hooks/api/auditLogs/constants.tsx index 229f822da..0c1c986a0 100644 --- a/frontend/src/hooks/api/auditLogs/constants.tsx +++ b/frontend/src/hooks/api/auditLogs/constants.tsx @@ -72,6 +72,8 @@ export const eventToNameMap: { [K in EventType]: string } = { [EventType.DELETE_CERT]: "Delete certificate", [EventType.REVOKE_CERT]: "Revoke certificate", [EventType.GET_CERT_BODY]: "Get certificate body", + [EventType.GET_CERT_PRIVATE_KEY]: "Get certificate private key", + [EventType.GET_CERT_BUNDLE]: "Get certificate bundle", [EventType.CREATE_PKI_ALERT]: "Create PKI alert", [EventType.GET_PKI_ALERT]: "Get PKI alert", [EventType.UPDATE_PKI_ALERT]: "Update PKI alert", diff --git a/frontend/src/hooks/api/auditLogs/enums.tsx b/frontend/src/hooks/api/auditLogs/enums.tsx index d465fb820..74c159aed 100644 --- a/frontend/src/hooks/api/auditLogs/enums.tsx +++ b/frontend/src/hooks/api/auditLogs/enums.tsx @@ -78,6 +78,8 @@ export enum EventType { DELETE_CERT = "delete-cert", REVOKE_CERT = "revoke-cert", GET_CERT_BODY = "get-cert-body", + GET_CERT_PRIVATE_KEY = "get-cert-private-key", + GET_CERT_BUNDLE = "get-cert-bundle", CREATE_PKI_ALERT = "create-pki-alert", GET_PKI_ALERT = "get-pki-alert", UPDATE_PKI_ALERT = "update-pki-alert", diff --git a/frontend/src/hooks/api/auditLogs/types.tsx b/frontend/src/hooks/api/auditLogs/types.tsx index 2524f84f4..04cd60236 100644 --- a/frontend/src/hooks/api/auditLogs/types.tsx +++ b/frontend/src/hooks/api/auditLogs/types.tsx @@ -619,6 +619,24 @@ interface GetCertBody { }; } +interface GetCertPrivateKey { + type: EventType.GET_CERT_PRIVATE_KEY; + metadata: { + certId: string; + cn: string; + serialNumber: string; + }; +} + +interface GetCertBundle { + type: EventType.GET_CERT_BUNDLE; + metadata: { + certId: string; + cn: string; + serialNumber: string; + }; +} + interface CreatePkiAlert { type: EventType.CREATE_PKI_ALERT; metadata: { @@ -878,6 +896,8 @@ export type Event = | DeleteCert | RevokeCert | GetCertBody + | GetCertPrivateKey + | GetCertBundle | CreatePkiAlert | GetPkiAlert | UpdatePkiAlert From 9f487ad026603a284865be5c75d7c8afc2523439 Mon Sep 17 00:00:00 2001 From: x Date: Wed, 30 Apr 2025 00:53:31 -0400 Subject: [PATCH 04/12] frontend type fixes --- backend/src/server/routes/index.ts | 2 +- .../services/secret-v2-bridge/secret-v2-bridge-dal.ts | 2 +- .../src/context/ProjectPermissionContext/types.ts | 1 + frontend/src/layouts/ProjectLayout/ProjectLayout.tsx | 2 +- .../components/ProjectRoleModifySection.utils.tsx | 11 +++++++++++ 5 files changed, 15 insertions(+), 3 deletions(-) diff --git a/backend/src/server/routes/index.ts b/backend/src/server/routes/index.ts index f9a2223ee..dd864412e 100644 --- a/backend/src/server/routes/index.ts +++ b/backend/src/server/routes/index.ts @@ -123,6 +123,7 @@ import { tokenDALFactory } from "@app/services/auth-token/auth-token-dal"; import { tokenServiceFactory } from "@app/services/auth-token/auth-token-service"; import { certificateBodyDALFactory } from "@app/services/certificate/certificate-body-dal"; import { certificateDALFactory } from "@app/services/certificate/certificate-dal"; +import { certificateSecretDALFactory } from "@app/services/certificate/certificate-secret-dal"; import { certificateServiceFactory } from "@app/services/certificate/certificate-service"; import { certificateAuthorityCertDALFactory } from "@app/services/certificate-authority/certificate-authority-cert-dal"; import { certificateAuthorityDALFactory } from "@app/services/certificate-authority/certificate-authority-dal"; @@ -260,7 +261,6 @@ import { registerSecretScannerGhApp } from "../plugins/secret-scanner"; import { registerV1Routes } from "./v1"; import { registerV2Routes } from "./v2"; import { registerV3Routes } from "./v3"; -import { certificateSecretDALFactory } from "@app/services/certificate/certificate-secret-dal"; const histogram = monitorEventLoopDelay({ resolution: 20 }); histogram.enable(); diff --git a/backend/src/services/secret-v2-bridge/secret-v2-bridge-dal.ts b/backend/src/services/secret-v2-bridge/secret-v2-bridge-dal.ts index 6ab348520..cd2773172 100644 --- a/backend/src/services/secret-v2-bridge/secret-v2-bridge-dal.ts +++ b/backend/src/services/secret-v2-bridge/secret-v2-bridge-dal.ts @@ -7,6 +7,7 @@ import { ProjectType, SecretsV2Schema, SecretType, TableName, TSecretsV2, TSecre import { TKeyStoreFactory } from "@app/keystore/keystore"; import { getConfig } from "@app/lib/config/env"; import { generateCacheKeyFromData } from "@app/lib/crypto/cache"; +import { applyJitter } from "@app/lib/dates"; import { BadRequestError, DatabaseError, NotFoundError } from "@app/lib/errors"; import { buildFindFilter, @@ -22,7 +23,6 @@ import type { TFindSecretsByFolderIdsFilter, TGetSecretsDTO } from "@app/services/secret-v2-bridge/secret-v2-bridge-types"; -import { applyJitter } from "@app/lib/dates"; export const SecretServiceCacheKeys = { get productKey() { diff --git a/frontend/src/context/ProjectPermissionContext/types.ts b/frontend/src/context/ProjectPermissionContext/types.ts index d2463d766..67ea65df4 100644 --- a/frontend/src/context/ProjectPermissionContext/types.ts +++ b/frontend/src/context/ProjectPermissionContext/types.ts @@ -270,6 +270,7 @@ export type ProjectPermissionSet = | [ProjectPermissionActions, ProjectPermissionSub.CertificateAuthorities] | [ProjectPermissionActions, ProjectPermissionSub.Certificates] | [ProjectPermissionActions, ProjectPermissionSub.CertificateTemplates] + | [ProjectPermissionActions, ProjectPermissionSub.CertificatePrivateKey] | [ProjectPermissionActions, ProjectPermissionSub.SshCertificateAuthorities] | [ProjectPermissionActions, ProjectPermissionSub.SshCertificateTemplates] | [ProjectPermissionActions, ProjectPermissionSub.SshCertificates] diff --git a/frontend/src/layouts/ProjectLayout/ProjectLayout.tsx b/frontend/src/layouts/ProjectLayout/ProjectLayout.tsx index a862c31a3..8533d7007 100644 --- a/frontend/src/layouts/ProjectLayout/ProjectLayout.tsx +++ b/frontend/src/layouts/ProjectLayout/ProjectLayout.tsx @@ -13,9 +13,9 @@ import { TBreadcrumbFormat } from "@app/components/v2"; import { - useProjectPermission, ProjectPermissionActions, ProjectPermissionSub, + useProjectPermission, useSubscription, useWorkspace } from "@app/context"; diff --git a/frontend/src/pages/project/RoleDetailsBySlugPage/components/ProjectRoleModifySection.utils.tsx b/frontend/src/pages/project/RoleDetailsBySlugPage/components/ProjectRoleModifySection.utils.tsx index de2295940..b68ee332f 100644 --- a/frontend/src/pages/project/RoleDetailsBySlugPage/components/ProjectRoleModifySection.utils.tsx +++ b/frontend/src/pages/project/RoleDetailsBySlugPage/components/ProjectRoleModifySection.utils.tsx @@ -223,6 +223,7 @@ export const projectRoleFormSchema = z.object({ [ProjectPermissionSub.PkiAlerts]: GeneralPolicyActionSchema.array().default([]), [ProjectPermissionSub.PkiCollections]: GeneralPolicyActionSchema.array().default([]), [ProjectPermissionSub.CertificateTemplates]: GeneralPolicyActionSchema.array().default([]), + [ProjectPermissionSub.CertificatePrivateKey]: GeneralPolicyActionSchema.array().default([]), [ProjectPermissionSub.SshCertificateAuthorities]: GeneralPolicyActionSchema.array().default( [] ), @@ -374,6 +375,7 @@ export const rolePermission2Form = (permissions: TProjectPermission[] = []) => { ProjectPermissionSub.PkiAlerts, ProjectPermissionSub.PkiCollections, ProjectPermissionSub.CertificateTemplates, + ProjectPermissionSub.CertificatePrivateKey, ProjectPermissionSub.SecretApproval, ProjectPermissionSub.Tags, ProjectPermissionSub.SecretRotation, @@ -1018,6 +1020,15 @@ export const PROJECT_PERMISSION_OBJECT: TProjectPermissionObject = { { label: "Remove", value: "delete" } ] }, + [ProjectPermissionSub.CertificatePrivateKey]: { + title: "Certificate Private Key", + actions: [ + { label: "Read", value: "read" }, + { label: "Create", value: "create" }, + { label: "Modify", value: "edit" }, + { label: "Remove", value: "delete" } + ] + }, [ProjectPermissionSub.CertificateTemplates]: { title: "Certificate Templates", actions: [ From eedffffc3880b49b89ef9e99155c0a3507b7578e Mon Sep 17 00:00:00 2001 From: x Date: Wed, 30 Apr 2025 02:07:07 -0400 Subject: [PATCH 05/12] review fixes --- ...9232917_store-cert-secret-key-and-chain.ts | 2 +- .../server/routes/v1/certificate-router.ts | 19 +++++-- .../services/certificate/certificate-fns.ts | 49 ++++++++++++++----- .../certificate/certificate-service.ts | 36 ++++++-------- .../services/certificate/certificate-types.ts | 8 +++ 5 files changed, 78 insertions(+), 36 deletions(-) diff --git a/backend/src/db/migrations/20250429232917_store-cert-secret-key-and-chain.ts b/backend/src/db/migrations/20250429232917_store-cert-secret-key-and-chain.ts index 2062f9a38..cb5e44a03 100644 --- a/backend/src/db/migrations/20250429232917_store-cert-secret-key-and-chain.ts +++ b/backend/src/db/migrations/20250429232917_store-cert-secret-key-and-chain.ts @@ -21,7 +21,7 @@ export async function up(knex: Knex): Promise { } export async function down(knex: Knex): Promise { - if (await knex.schema.hasTable(TableName.Certificate)) { + if (await knex.schema.hasTable(TableName.CertificateSecret)) { await knex.schema.dropTable(TableName.CertificateSecret); } diff --git a/backend/src/server/routes/v1/certificate-router.ts b/backend/src/server/routes/v1/certificate-router.ts index d026868e9..ecbb02734 100644 --- a/backend/src/server/routes/v1/certificate-router.ts +++ b/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, diff --git a/backend/src/services/certificate/certificate-fns.ts b/backend/src/services/certificate/certificate-fns.ts index 6f9963db7..5cd4929bb 100644 --- a/backend/src/services/certificate/certificate-fns.ts +++ b/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" }); - const certPrivateKey = skObj.export({ format: "pem", type: "pkcs8" }).toString(); + try { + 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 certPublicKey = pkObj.export({ format: "pem", type: "spki" }).toString(); + const pkObj = crypto.createPublicKey(skObj); + const certPublicKey = pkObj.export({ format: "pem", type: "spki" }).toString(); - return { - certificateSecret, - certPrivateKey, - certPublicKey - }; + return { + certificateSecret, + certPrivateKey, + 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; }; diff --git a/backend/src/services/certificate/certificate-service.ts b/backend/src/services/certificate/certificate-service.ts index c75b01382..d07027fd8 100644 --- a/backend/src/services/certificate/certificate-service.ts +++ b/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(); - - // If the certificate was generated after ~05/01/25 it will have a encryptedCertificateChain attached to it's body - if (certBody.encryptedCertificateChain) { - const decryptedCertChain = await kmsDecryptor({ - cipherTextBlob: certBody.encryptedCertificateChain - }); - const certChainObj = new x509.X509Certificate(decryptedCertChain); - certificateChain = certChainObj.toString("pem"); - } + const certificateChain = await buildCertificateChain({ + caCert, + caCertChain, + kmsId: certificateManagerKeyId, + kmsService, + encryptedCertificateChain: certBody.encryptedCertificateChain || undefined + }); return { certificate: certObj.toString("pem"), @@ -314,16 +311,13 @@ export const certificateServiceFactory = ({ kmsService }); - 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 (certBody.encryptedCertificateChain) { - const decryptedCertChain = await kmsDecryptor({ - cipherTextBlob: certBody.encryptedCertificateChain - }); - const certChainObj = new x509.X509Certificate(decryptedCertChain); - certificateChain = certChainObj.toString("pem"); - } + const certificateChain = await buildCertificateChain({ + caCert, + caCertChain, + kmsId: certificateManagerKeyId, + kmsService, + encryptedCertificateChain: certBody.encryptedCertificateChain || undefined + }); const { certPrivateKey } = await getCertificateCredentials({ certId: cert.id, diff --git a/backend/src/services/certificate/certificate-types.ts b/backend/src/services/certificate/certificate-types.ts index 71a53bd3f..373fa028c 100644 --- a/backend/src/services/certificate/certificate-types.ts +++ b/backend/src/services/certificate/certificate-types.ts @@ -93,3 +93,11 @@ export type TGetCertificateCredentialsDTO = { projectDAL: Pick; kmsService: Pick; }; + +export type TBuildCertificateChainDTO = { + caCert: string; + caCertChain: string; + encryptedCertificateChain?: Buffer; + kmsService: Pick; + kmsId: string; +}; From 235be96ded7a17ef423579cb7f8539bfe7e8fdcc Mon Sep 17 00:00:00 2001 From: x Date: Wed, 30 Apr 2025 14:53:57 -0400 Subject: [PATCH 06/12] tweaks --- backend/src/services/certificate/certificate-fns.ts | 5 ++++- backend/src/services/certificate/certificate-types.ts | 4 ++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/backend/src/services/certificate/certificate-fns.ts b/backend/src/services/certificate/certificate-fns.ts index 5cd4929bb..961fb27ff 100644 --- a/backend/src/services/certificate/certificate-fns.ts +++ b/backend/src/services/certificate/certificate-fns.ts @@ -105,9 +105,12 @@ export const buildCertificateChain = async ({ kmsService, kmsId }: TBuildCertificateChainDTO) => { + if (!encryptedCertificateChain && (!caCert || !caCertChain)) { + return null; + } + 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({ diff --git a/backend/src/services/certificate/certificate-types.ts b/backend/src/services/certificate/certificate-types.ts index 373fa028c..ae04eae6b 100644 --- a/backend/src/services/certificate/certificate-types.ts +++ b/backend/src/services/certificate/certificate-types.ts @@ -95,8 +95,8 @@ export type TGetCertificateCredentialsDTO = { }; export type TBuildCertificateChainDTO = { - caCert: string; - caCertChain: string; + caCert?: string; + caCertChain?: string; encryptedCertificateChain?: Buffer; kmsService: Pick; kmsId: string; From 07e4bc8eed9f68a2afa3872c9777702d143d3e40 Mon Sep 17 00:00:00 2001 From: x Date: Wed, 30 Apr 2025 17:46:05 -0400 Subject: [PATCH 07/12] review fixes --- .../services/permission/project-permission.ts | 41 +++++++++------ backend/src/server/lib/caching.ts | 8 +++ .../server/routes/v1/certificate-router.ts | 17 ++----- .../certificate-authority-service.ts | 13 +++-- .../certificate/certificate-service.ts | 38 ++++++++++---- .../src/services/project/project-service.ts | 6 ++- .../ProjectPermissionContext/index.tsx | 1 + .../context/ProjectPermissionContext/types.ts | 12 +++-- frontend/src/context/index.tsx | 1 + .../CertificatesPage/CertificatesPage.tsx | 11 ++-- .../components/CertificatesSection.tsx | 8 ++- .../components/CertificatesTable.tsx | 14 +++-- .../ProjectRoleModifySection.utils.tsx | 51 ++++++++++++------- 13 files changed, 149 insertions(+), 72 deletions(-) create mode 100644 backend/src/server/lib/caching.ts diff --git a/backend/src/ee/services/permission/project-permission.ts b/backend/src/ee/services/permission/project-permission.ts index 7d659e99b..1dbc5ee2a 100644 --- a/backend/src/ee/services/permission/project-permission.ts +++ b/backend/src/ee/services/permission/project-permission.ts @@ -17,6 +17,14 @@ export enum ProjectPermissionActions { Delete = "delete" } +export enum ProjectPermissionCertificateActions { + Read = "read", + Create = "create", + Edit = "edit", + Delete = "delete", + ReadPrivateKey = "read-private-key" +} + export enum ProjectPermissionSecretActions { DescribeAndReadValue = "read", DescribeSecret = "describeSecret", @@ -129,7 +137,6 @@ export enum ProjectPermissionSub { Identity = "identity", CertificateAuthorities = "certificate-authorities", Certificates = "certificates", - CertificatePrivateKey = "certificate-private-key", CertificateTemplates = "certificate-templates", SshCertificateAuthorities = "ssh-certificate-authorities", SshCertificates = "ssh-certificates", @@ -232,8 +239,7 @@ export type ProjectPermissionSet = ProjectPermissionSub.Identity | (ForcedSubject & IdentityManagementSubjectFields) ] | [ProjectPermissionActions, ProjectPermissionSub.CertificateAuthorities] - | [ProjectPermissionActions, ProjectPermissionSub.Certificates] - | [ProjectPermissionActions, ProjectPermissionSub.CertificatePrivateKey] + | [ProjectPermissionCertificateActions, ProjectPermissionSub.Certificates] | [ProjectPermissionActions, ProjectPermissionSub.CertificateTemplates] | [ProjectPermissionActions, ProjectPermissionSub.SshCertificateAuthorities] | [ProjectPermissionActions, ProjectPermissionSub.SshCertificates] @@ -482,12 +488,6 @@ const GeneralPermissionSchema = [ "Describe what action an entity can take." ) }), - z.object({ - subject: z.literal(ProjectPermissionSub.CertificatePrivateKey).describe("The entity this permission pertains to."), - action: CASL_ACTION_SCHEMA_NATIVE_ENUM(ProjectPermissionActions).describe( - "Describe what action an entity can take." - ) - }), z.object({ subject: z.literal(ProjectPermissionSub.CertificateTemplates).describe("The entity this permission pertains to."), action: CASL_ACTION_SCHEMA_NATIVE_ENUM(ProjectPermissionActions).describe( @@ -688,8 +688,6 @@ const buildAdminPermissionRules = () => { ProjectPermissionSub.AuditLogs, ProjectPermissionSub.IpAllowList, ProjectPermissionSub.CertificateAuthorities, - ProjectPermissionSub.Certificates, - ProjectPermissionSub.CertificatePrivateKey, ProjectPermissionSub.CertificateTemplates, ProjectPermissionSub.PkiAlerts, ProjectPermissionSub.PkiCollections, @@ -708,6 +706,17 @@ const buildAdminPermissionRules = () => { ); }); + can( + [ + ProjectPermissionCertificateActions.Read, + ProjectPermissionCertificateActions.Edit, + ProjectPermissionCertificateActions.Create, + ProjectPermissionCertificateActions.Delete, + ProjectPermissionCertificateActions.ReadPrivateKey + ], + ProjectPermissionSub.Certificates + ); + can( [ ProjectPermissionSshHostActions.Edit, @@ -965,10 +974,10 @@ const buildMemberPermissionRules = () => { can( [ - ProjectPermissionActions.Read, - ProjectPermissionActions.Edit, - ProjectPermissionActions.Create, - ProjectPermissionActions.Delete + ProjectPermissionCertificateActions.Read, + ProjectPermissionCertificateActions.Edit, + ProjectPermissionCertificateActions.Create, + ProjectPermissionCertificateActions.Delete ], ProjectPermissionSub.Certificates ); @@ -1041,7 +1050,7 @@ const buildViewerPermissionRules = () => { can(ProjectPermissionActions.Read, ProjectPermissionSub.AuditLogs); can(ProjectPermissionActions.Read, ProjectPermissionSub.IpAllowList); can(ProjectPermissionActions.Read, ProjectPermissionSub.CertificateAuthorities); - can(ProjectPermissionActions.Read, ProjectPermissionSub.Certificates); + can(ProjectPermissionCertificateActions.Read, ProjectPermissionSub.Certificates); can(ProjectPermissionCmekActions.Read, ProjectPermissionSub.Cmek); can(ProjectPermissionActions.Read, ProjectPermissionSub.SshCertificates); can(ProjectPermissionActions.Read, ProjectPermissionSub.SshCertificateTemplates); diff --git a/backend/src/server/lib/caching.ts b/backend/src/server/lib/caching.ts new file mode 100644 index 000000000..513f2f635 --- /dev/null +++ b/backend/src/server/lib/caching.ts @@ -0,0 +1,8 @@ +import { FastifyReply } from "fastify"; + +export const addNoCacheHeaders = (reply: FastifyReply) => { + void reply.header("Cache-Control", "no-store, no-cache, must-revalidate, proxy-revalidate"); + void reply.header("Pragma", "no-cache"); + void reply.header("Expires", "0"); + void reply.header("Surrogate-Control", "no-store"); +}; diff --git a/backend/src/server/routes/v1/certificate-router.ts b/backend/src/server/routes/v1/certificate-router.ts index ecbb02734..dad1d9a80 100644 --- a/backend/src/server/routes/v1/certificate-router.ts +++ b/backend/src/server/routes/v1/certificate-router.ts @@ -6,6 +6,7 @@ import { EventType } from "@app/ee/services/audit-log/audit-log-types"; import { ApiDocsTags, CERTIFICATE_AUTHORITIES, CERTIFICATES } from "@app/lib/api-docs"; import { ms } from "@app/lib/ms"; import { readLimit, writeLimit } from "@app/server/config/rateLimiter"; +import { addNoCacheHeaders } from "@app/server/lib/caching"; import { getTelemetryDistinctId } from "@app/server/lib/telemetry"; import { verifyAuth } from "@app/server/plugins/auth/verify-auth"; import { AuthMode } from "@app/services/auth/auth-type"; @@ -106,11 +107,7 @@ 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"); + addNoCacheHeaders(reply); return certPrivateKey; } @@ -134,7 +131,7 @@ export const registerCertRouter = async (server: FastifyZodProvider) => { response: { 200: z.object({ certificate: z.string().trim().describe(CERTIFICATES.GET_CERT.certificate), - certificateChain: z.string().trim().describe(CERTIFICATES.GET_CERT.certificateChain), + certificateChain: z.string().trim().nullish().describe(CERTIFICATES.GET_CERT.certificateChain), privateKey: z.string().trim().describe(CERTIFICATES.GET_CERT.privateKey), serialNumber: z.string().trim().describe(CERTIFICATES.GET_CERT.serialNumberRes) }) @@ -163,11 +160,7 @@ 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"); + addNoCacheHeaders(reply); return { certificate, @@ -525,7 +518,7 @@ export const registerCertRouter = async (server: FastifyZodProvider) => { response: { 200: z.object({ certificate: z.string().trim().describe(CERTIFICATES.GET_CERT.certificate), - certificateChain: z.string().trim().describe(CERTIFICATES.GET_CERT.certificateChain), + certificateChain: z.string().trim().nullish().describe(CERTIFICATES.GET_CERT.certificateChain), serialNumber: z.string().trim().describe(CERTIFICATES.GET_CERT.serialNumberRes) }) } diff --git a/backend/src/services/certificate-authority/certificate-authority-service.ts b/backend/src/services/certificate-authority/certificate-authority-service.ts index 4c40d9ff4..e1d7ce5cb 100644 --- a/backend/src/services/certificate-authority/certificate-authority-service.ts +++ b/backend/src/services/certificate-authority/certificate-authority-service.ts @@ -6,7 +6,11 @@ import { z } from "zod"; import { ActionProjectType, ProjectType, TCertificateAuthorities, TCertificateTemplates } from "@app/db/schemas"; import { TPermissionServiceFactory } from "@app/ee/services/permission/permission-service"; -import { ProjectPermissionActions, ProjectPermissionSub } from "@app/ee/services/permission/project-permission"; +import { + ProjectPermissionActions, + ProjectPermissionCertificateActions, + ProjectPermissionSub +} from "@app/ee/services/permission/project-permission"; import { extractX509CertFromChain } from "@app/lib/certificates/extract-certificate"; import { getConfig } from "@app/lib/config/env"; import { BadRequestError, NotFoundError } from "@app/lib/errors"; @@ -1160,7 +1164,10 @@ export const certificateAuthorityServiceFactory = ({ actionProjectType: ActionProjectType.CertificateManager }); - ForbiddenError.from(permission).throwUnlessCan(ProjectPermissionActions.Create, ProjectPermissionSub.Certificates); + ForbiddenError.from(permission).throwUnlessCan( + ProjectPermissionCertificateActions.Create, + ProjectPermissionSub.Certificates + ); if (ca.status === CaStatus.DISABLED) throw new BadRequestError({ message: "CA is disabled" }); if (!ca.activeCaCertId) throw new BadRequestError({ message: "CA does not have a certificate installed" }); @@ -1508,7 +1515,7 @@ export const certificateAuthorityServiceFactory = ({ }); ForbiddenError.from(permission).throwUnlessCan( - ProjectPermissionActions.Create, + ProjectPermissionCertificateActions.Create, ProjectPermissionSub.Certificates ); } diff --git a/backend/src/services/certificate/certificate-service.ts b/backend/src/services/certificate/certificate-service.ts index d07027fd8..73a8caed7 100644 --- a/backend/src/services/certificate/certificate-service.ts +++ b/backend/src/services/certificate/certificate-service.ts @@ -4,7 +4,10 @@ import * as x509 from "@peculiar/x509"; import { ActionProjectType } from "@app/db/schemas"; import { TCertificateAuthorityCrlDALFactory } from "@app/ee/services/certificate-authority-crl/certificate-authority-crl-dal"; import { TPermissionServiceFactory } from "@app/ee/services/permission/permission-service"; -import { ProjectPermissionActions, ProjectPermissionSub } from "@app/ee/services/permission/project-permission"; +import { + ProjectPermissionCertificateActions, + ProjectPermissionSub +} from "@app/ee/services/permission/project-permission"; import { TCertificateBodyDALFactory } from "@app/services/certificate/certificate-body-dal"; import { TCertificateDALFactory } from "@app/services/certificate/certificate-dal"; import { TCertificateAuthorityCertDALFactory } from "@app/services/certificate-authority/certificate-authority-cert-dal"; @@ -70,7 +73,10 @@ export const certificateServiceFactory = ({ actionProjectType: ActionProjectType.CertificateManager }); - ForbiddenError.from(permission).throwUnlessCan(ProjectPermissionActions.Read, ProjectPermissionSub.Certificates); + ForbiddenError.from(permission).throwUnlessCan( + ProjectPermissionCertificateActions.Read, + ProjectPermissionSub.Certificates + ); return { cert, @@ -101,8 +107,8 @@ export const certificateServiceFactory = ({ }); ForbiddenError.from(permission).throwUnlessCan( - ProjectPermissionActions.Read, - ProjectPermissionSub.CertificatePrivateKey + ProjectPermissionCertificateActions.ReadPrivateKey, + ProjectPermissionSub.Certificates ); const { certPrivateKey } = await getCertificateCredentials({ @@ -136,7 +142,10 @@ export const certificateServiceFactory = ({ actionProjectType: ActionProjectType.CertificateManager }); - ForbiddenError.from(permission).throwUnlessCan(ProjectPermissionActions.Delete, ProjectPermissionSub.Certificates); + ForbiddenError.from(permission).throwUnlessCan( + ProjectPermissionCertificateActions.Delete, + ProjectPermissionSub.Certificates + ); const deletedCert = await certificateDAL.deleteById(cert.id); @@ -171,7 +180,10 @@ export const certificateServiceFactory = ({ actionProjectType: ActionProjectType.CertificateManager }); - ForbiddenError.from(permission).throwUnlessCan(ProjectPermissionActions.Delete, ProjectPermissionSub.Certificates); + ForbiddenError.from(permission).throwUnlessCan( + ProjectPermissionCertificateActions.Delete, + ProjectPermissionSub.Certificates + ); if (cert.status === CertStatus.REVOKED) throw new Error("Certificate already revoked"); @@ -218,7 +230,10 @@ export const certificateServiceFactory = ({ actionProjectType: ActionProjectType.CertificateManager }); - ForbiddenError.from(permission).throwUnlessCan(ProjectPermissionActions.Read, ProjectPermissionSub.Certificates); + ForbiddenError.from(permission).throwUnlessCan( + ProjectPermissionCertificateActions.Read, + ProjectPermissionSub.Certificates + ); const certBody = await certificateBodyDAL.findOne({ certId: cert.id }); @@ -279,10 +294,13 @@ export const certificateServiceFactory = ({ actionProjectType: ActionProjectType.CertificateManager }); - ForbiddenError.from(permission).throwUnlessCan(ProjectPermissionActions.Read, ProjectPermissionSub.Certificates); ForbiddenError.from(permission).throwUnlessCan( - ProjectPermissionActions.Read, - ProjectPermissionSub.CertificatePrivateKey + ProjectPermissionCertificateActions.Read, + ProjectPermissionSub.Certificates + ); + ForbiddenError.from(permission).throwUnlessCan( + ProjectPermissionCertificateActions.ReadPrivateKey, + ProjectPermissionSub.Certificates ); const certBody = await certificateBodyDAL.findOne({ certId: cert.id }); diff --git a/backend/src/services/project/project-service.ts b/backend/src/services/project/project-service.ts index 9f2de8a85..db67f49d3 100644 --- a/backend/src/services/project/project-service.ts +++ b/backend/src/services/project/project-service.ts @@ -14,6 +14,7 @@ import { throwIfMissingSecretReadValueOrDescribePermission } from "@app/ee/servi import { TPermissionServiceFactory } from "@app/ee/services/permission/permission-service"; import { ProjectPermissionActions, + ProjectPermissionCertificateActions, ProjectPermissionSecretActions, ProjectPermissionSshHostActions, ProjectPermissionSub @@ -927,7 +928,10 @@ export const projectServiceFactory = ({ actionProjectType: ActionProjectType.CertificateManager }); - ForbiddenError.from(permission).throwUnlessCan(ProjectPermissionActions.Read, ProjectPermissionSub.Certificates); + ForbiddenError.from(permission).throwUnlessCan( + ProjectPermissionCertificateActions.Read, + ProjectPermissionSub.Certificates + ); const cas = await certificateAuthorityDAL.find({ projectId }); diff --git a/frontend/src/context/ProjectPermissionContext/index.tsx b/frontend/src/context/ProjectPermissionContext/index.tsx index 5bc163817..b195571f8 100644 --- a/frontend/src/context/ProjectPermissionContext/index.tsx +++ b/frontend/src/context/ProjectPermissionContext/index.tsx @@ -2,6 +2,7 @@ export { useProjectPermission } from "./ProjectPermissionContext"; export type { ProjectPermissionSet, TProjectPermission } from "./types"; export { ProjectPermissionActions, + ProjectPermissionCertificateActions, ProjectPermissionCmekActions, ProjectPermissionDynamicSecretActions, ProjectPermissionGroupActions, diff --git a/frontend/src/context/ProjectPermissionContext/types.ts b/frontend/src/context/ProjectPermissionContext/types.ts index 67ea65df4..5aee9c483 100644 --- a/frontend/src/context/ProjectPermissionContext/types.ts +++ b/frontend/src/context/ProjectPermissionContext/types.ts @@ -7,6 +7,14 @@ export enum ProjectPermissionActions { Delete = "delete" } +export enum ProjectPermissionCertificateActions { + Read = "read", + Create = "create", + Edit = "edit", + Delete = "delete", + ReadPrivateKey = "read-private-key" +} + export enum ProjectPermissionSecretActions { DescribeAndReadValue = "read", DescribeSecret = "describeSecret", @@ -170,7 +178,6 @@ export enum ProjectPermissionSub { Identity = "identity", CertificateAuthorities = "certificate-authorities", Certificates = "certificates", - CertificatePrivateKey = "certificate-private-key", CertificateTemplates = "certificate-templates", SshCertificateAuthorities = "ssh-certificate-authorities", SshCertificateTemplates = "ssh-certificate-templates", @@ -268,9 +275,8 @@ export type ProjectPermissionSet = ) ] | [ProjectPermissionActions, ProjectPermissionSub.CertificateAuthorities] - | [ProjectPermissionActions, ProjectPermissionSub.Certificates] + | [ProjectPermissionCertificateActions, ProjectPermissionSub.Certificates] | [ProjectPermissionActions, ProjectPermissionSub.CertificateTemplates] - | [ProjectPermissionActions, ProjectPermissionSub.CertificatePrivateKey] | [ProjectPermissionActions, ProjectPermissionSub.SshCertificateAuthorities] | [ProjectPermissionActions, ProjectPermissionSub.SshCertificateTemplates] | [ProjectPermissionActions, ProjectPermissionSub.SshCertificates] diff --git a/frontend/src/context/index.tsx b/frontend/src/context/index.tsx index 51f2797d0..04af3c8a4 100644 --- a/frontend/src/context/index.tsx +++ b/frontend/src/context/index.tsx @@ -10,6 +10,7 @@ export { export type { TProjectPermission } from "./ProjectPermissionContext"; export { ProjectPermissionActions, + ProjectPermissionCertificateActions, ProjectPermissionCmekActions, ProjectPermissionDynamicSecretActions, ProjectPermissionGroupActions, diff --git a/frontend/src/pages/cert-manager/CertificatesPage/CertificatesPage.tsx b/frontend/src/pages/cert-manager/CertificatesPage/CertificatesPage.tsx index c985f313f..4a4a22093 100644 --- a/frontend/src/pages/cert-manager/CertificatesPage/CertificatesPage.tsx +++ b/frontend/src/pages/cert-manager/CertificatesPage/CertificatesPage.tsx @@ -3,7 +3,12 @@ import { useTranslation } from "react-i18next"; import { ProjectPermissionCan } from "@app/components/permissions"; import { PageHeader } from "@app/components/v2"; -import { ProjectPermissionActions, ProjectPermissionSub, useProjectPermission } from "@app/context"; +import { + ProjectPermissionActions, + ProjectPermissionCertificateActions, + ProjectPermissionSub, + useProjectPermission +} from "@app/context"; import { PkiCollectionSection } from "../AlertingPage/components"; import { CertificatesSection } from "./components"; @@ -17,7 +22,7 @@ export const CertificatesPage = () => { ProjectPermissionSub.PkiCollections ); const canAccessCerts = permission.can( - ProjectPermissionActions.Read, + ProjectPermissionCertificateActions.Read, ProjectPermissionSub.Certificates ); @@ -40,7 +45,7 @@ export const CertificatesPage = () => { )} diff --git a/frontend/src/pages/cert-manager/CertificatesPage/components/CertificatesSection.tsx b/frontend/src/pages/cert-manager/CertificatesPage/components/CertificatesSection.tsx index d5f94e7b7..ef8b09dac 100644 --- a/frontend/src/pages/cert-manager/CertificatesPage/components/CertificatesSection.tsx +++ b/frontend/src/pages/cert-manager/CertificatesPage/components/CertificatesSection.tsx @@ -4,7 +4,11 @@ import { FontAwesomeIcon } from "@fortawesome/react-fontawesome"; import { createNotification } from "@app/components/notifications"; import { ProjectPermissionCan } from "@app/components/permissions"; import { Button, DeleteActionModal } from "@app/components/v2"; -import { ProjectPermissionActions, ProjectPermissionSub, useWorkspace } from "@app/context"; +import { + ProjectPermissionCertificateActions, + ProjectPermissionSub, + useWorkspace +} from "@app/context"; import { useDeleteCert } from "@app/hooks/api"; import { usePopUp } from "@app/hooks/usePopUp"; @@ -50,7 +54,7 @@ export const CertificatesSection = () => {

Certificates

{(isAllowed) => ( diff --git a/frontend/src/pages/cert-manager/CertificatesPage/components/CertificatesTable.tsx b/frontend/src/pages/cert-manager/CertificatesPage/components/CertificatesTable.tsx index dcc832bfb..8cf7dda24 100644 --- a/frontend/src/pages/cert-manager/CertificatesPage/components/CertificatesTable.tsx +++ b/frontend/src/pages/cert-manager/CertificatesPage/components/CertificatesTable.tsx @@ -30,7 +30,11 @@ import { Tooltip, Tr } from "@app/components/v2"; -import { ProjectPermissionActions, ProjectPermissionSub, useWorkspace } from "@app/context"; +import { + ProjectPermissionCertificateActions, + ProjectPermissionSub, + useWorkspace +} from "@app/context"; import { useListWorkspaceCertificates } from "@app/hooks/api"; import { CertStatus } from "@app/hooks/api/certificates/enums"; import { UsePopUpState } from "@app/hooks/usePopUp"; @@ -110,7 +114,7 @@ export const CertificatesTable = ({ handlePopUpOpen }: Props) => { {(isAllowed) => ( @@ -131,7 +135,7 @@ export const CertificatesTable = ({ handlePopUpOpen }: Props) => { )} {(isAllowed) => ( @@ -152,7 +156,7 @@ export const CertificatesTable = ({ handlePopUpOpen }: Props) => { )} {(isAllowed) => ( @@ -173,7 +177,7 @@ export const CertificatesTable = ({ handlePopUpOpen }: Props) => { )} {(isAllowed) => ( diff --git a/frontend/src/pages/project/RoleDetailsBySlugPage/components/ProjectRoleModifySection.utils.tsx b/frontend/src/pages/project/RoleDetailsBySlugPage/components/ProjectRoleModifySection.utils.tsx index b68ee332f..6b90b791d 100644 --- a/frontend/src/pages/project/RoleDetailsBySlugPage/components/ProjectRoleModifySection.utils.tsx +++ b/frontend/src/pages/project/RoleDetailsBySlugPage/components/ProjectRoleModifySection.utils.tsx @@ -6,6 +6,7 @@ import { z } from "zod"; import { Tooltip } from "@app/components/v2"; import { ProjectPermissionActions, + ProjectPermissionCertificateActions, ProjectPermissionCmekActions, ProjectPermissionSub } from "@app/context"; @@ -32,6 +33,14 @@ const GeneralPolicyActionSchema = z.object({ create: z.boolean().optional() }); +const CertificatePolicyActionSchema = z.object({ + [ProjectPermissionCertificateActions.Create]: z.boolean().optional(), + [ProjectPermissionCertificateActions.Delete]: z.boolean().optional(), + [ProjectPermissionCertificateActions.Edit]: z.boolean().optional(), + [ProjectPermissionCertificateActions.Read]: z.boolean().optional(), + [ProjectPermissionCertificateActions.ReadPrivateKey]: z.boolean().optional() +}); + const SecretPolicyActionSchema = z.object({ [ProjectPermissionSecretActions.DescribeAndReadValue]: z.boolean().optional(), // existing read, gives both describe and read value [ProjectPermissionSecretActions.DescribeSecret]: z.boolean().optional(), @@ -219,11 +228,10 @@ export const projectRoleFormSchema = z.object({ [ProjectPermissionSub.AuditLogs]: GeneralPolicyActionSchema.array().default([]), [ProjectPermissionSub.IpAllowList]: GeneralPolicyActionSchema.array().default([]), [ProjectPermissionSub.CertificateAuthorities]: GeneralPolicyActionSchema.array().default([]), - [ProjectPermissionSub.Certificates]: GeneralPolicyActionSchema.array().default([]), + [ProjectPermissionSub.Certificates]: CertificatePolicyActionSchema.array().default([]), [ProjectPermissionSub.PkiAlerts]: GeneralPolicyActionSchema.array().default([]), [ProjectPermissionSub.PkiCollections]: GeneralPolicyActionSchema.array().default([]), [ProjectPermissionSub.CertificateTemplates]: GeneralPolicyActionSchema.array().default([]), - [ProjectPermissionSub.CertificatePrivateKey]: GeneralPolicyActionSchema.array().default([]), [ProjectPermissionSub.SshCertificateAuthorities]: GeneralPolicyActionSchema.array().default( [] ), @@ -371,11 +379,9 @@ export const rolePermission2Form = (permissions: TProjectPermission[] = []) => { ProjectPermissionSub.AuditLogs, ProjectPermissionSub.IpAllowList, ProjectPermissionSub.CertificateAuthorities, - ProjectPermissionSub.Certificates, ProjectPermissionSub.PkiAlerts, ProjectPermissionSub.PkiCollections, ProjectPermissionSub.CertificateTemplates, - ProjectPermissionSub.CertificatePrivateKey, ProjectPermissionSub.SecretApproval, ProjectPermissionSub.Tags, ProjectPermissionSub.SecretRotation, @@ -507,6 +513,25 @@ export const rolePermission2Form = (permissions: TProjectPermission[] = []) => { return; } + if (subject === ProjectPermissionSub.Certificates) { + const canRead = action.includes(ProjectPermissionCertificateActions.Read); + const canEdit = action.includes(ProjectPermissionCertificateActions.Edit); + const canDelete = action.includes(ProjectPermissionCertificateActions.Delete); + const canCreate = action.includes(ProjectPermissionCertificateActions.Create); + const canReadPrivateKey = action.includes(ProjectPermissionCertificateActions.ReadPrivateKey); + + if (!formVal[subject]) formVal[subject] = [{}]; + + // from above statement we are sure it won't be undefined + if (canRead) formVal[subject]![0].read = true; + if (canEdit) formVal[subject]![0].edit = true; + if (canCreate) formVal[subject]![0].create = true; + if (canDelete) formVal[subject]![0].delete = true; + if (canReadPrivateKey) + formVal[subject]![0][ProjectPermissionCertificateActions.ReadPrivateKey] = true; + return; + } + if (subject === ProjectPermissionSub.Project) { const canEdit = action.includes(ProjectPermissionActions.Edit); const canDelete = action.includes(ProjectPermissionActions.Delete); @@ -1014,19 +1039,11 @@ export const PROJECT_PERMISSION_OBJECT: TProjectPermissionObject = { [ProjectPermissionSub.Certificates]: { title: "Certificates", actions: [ - { label: "Read", value: "read" }, - { label: "Create", value: "create" }, - { label: "Modify", value: "edit" }, - { label: "Remove", value: "delete" } - ] - }, - [ProjectPermissionSub.CertificatePrivateKey]: { - title: "Certificate Private Key", - actions: [ - { label: "Read", value: "read" }, - { label: "Create", value: "create" }, - { label: "Modify", value: "edit" }, - { label: "Remove", value: "delete" } + { label: "Read", value: ProjectPermissionCertificateActions.Read }, + { label: "Read Private Key", value: ProjectPermissionCertificateActions.ReadPrivateKey }, + { label: "Create", value: ProjectPermissionCertificateActions.Create }, + { label: "Modify", value: ProjectPermissionCertificateActions.Edit }, + { label: "Remove", value: ProjectPermissionCertificateActions.Delete } ] }, [ProjectPermissionSub.CertificateTemplates]: { From 1268bc12388874a8995422cfc3fc872c9726e2dd Mon Sep 17 00:00:00 2001 From: x Date: Wed, 30 Apr 2025 17:50:23 -0400 Subject: [PATCH 08/12] coderabbit review fixes --- backend/src/ee/services/permission/project-permission.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/backend/src/ee/services/permission/project-permission.ts b/backend/src/ee/services/permission/project-permission.ts index 1dbc5ee2a..03c8ed61a 100644 --- a/backend/src/ee/services/permission/project-permission.ts +++ b/backend/src/ee/services/permission/project-permission.ts @@ -484,7 +484,7 @@ const GeneralPermissionSchema = [ }), z.object({ subject: z.literal(ProjectPermissionSub.Certificates).describe("The entity this permission pertains to."), - action: CASL_ACTION_SCHEMA_NATIVE_ENUM(ProjectPermissionActions).describe( + action: CASL_ACTION_SCHEMA_NATIVE_ENUM(ProjectPermissionCertificateActions).describe( "Describe what action an entity can take." ) }), From 1ce06891a562f677c6bf8a909c05fa9c1d977ef1 Mon Sep 17 00:00:00 2001 From: Andrey Lyubavin Date: Mon, 5 May 2025 13:43:38 -0400 Subject: [PATCH 09/12] ui tweak for role policies --- .../components/GeneralPermissionPolicies.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frontend/src/pages/project/RoleDetailsBySlugPage/components/GeneralPermissionPolicies.tsx b/frontend/src/pages/project/RoleDetailsBySlugPage/components/GeneralPermissionPolicies.tsx index f0388a2f4..6773f0658 100644 --- a/frontend/src/pages/project/RoleDetailsBySlugPage/components/GeneralPermissionPolicies.tsx +++ b/frontend/src/pages/project/RoleDetailsBySlugPage/components/GeneralPermissionPolicies.tsx @@ -114,7 +114,7 @@ export const GeneralPermissionPolicies = )}
-
+
Actions
{actions.map(({ label, value }, index) => { From 54927454bf88dc73f55b8f56eb28ccc29ef69d04 Mon Sep 17 00:00:00 2001 From: Andrey Lyubavin Date: Mon, 5 May 2025 14:37:20 -0400 Subject: [PATCH 10/12] ui fetch private key if permission allows it --- .../src/hooks/api/certificates/queries.tsx | 19 +++++++++++++++- .../components/CertificateCertModal.tsx | 22 +++++++++++++++++-- 2 files changed, 38 insertions(+), 3 deletions(-) diff --git a/frontend/src/hooks/api/certificates/queries.tsx b/frontend/src/hooks/api/certificates/queries.tsx index 50c751c06..c53cef471 100644 --- a/frontend/src/hooks/api/certificates/queries.tsx +++ b/frontend/src/hooks/api/certificates/queries.tsx @@ -6,7 +6,8 @@ import { TCertificate } from "./types"; export const certKeys = { getCertById: (serialNumber: string) => [{ serialNumber }, "cert"], - getCertBody: (serialNumber: string) => [{ serialNumber }, "certBody"] + getCertBody: (serialNumber: string) => [{ serialNumber }, "certBody"], + getCertBundle: (serialNumber: string) => [{ serialNumber }, "certBundle"] }; export const useGetCert = (serialNumber: string) => { @@ -38,3 +39,19 @@ export const useGetCertBody = (serialNumber: string) => { enabled: Boolean(serialNumber) }); }; + +export const useGetCertBundle = (serialNumber: string) => { + return useQuery({ + queryKey: certKeys.getCertBundle(serialNumber), + queryFn: async () => { + const { data } = await apiRequest.get<{ + certificate: string; + certificateChain: string; + serialNumber: string; + privateKey: string; + }>(`/api/v1/pki/certificates/${serialNumber}/bundle`); + return data; + }, + enabled: Boolean(serialNumber) + }); +}; diff --git a/frontend/src/pages/cert-manager/CertificatesPage/components/CertificateCertModal.tsx b/frontend/src/pages/cert-manager/CertificatesPage/components/CertificateCertModal.tsx index 01c79589c..081a59e67 100644 --- a/frontend/src/pages/cert-manager/CertificatesPage/components/CertificateCertModal.tsx +++ b/frontend/src/pages/cert-manager/CertificatesPage/components/CertificateCertModal.tsx @@ -3,6 +3,12 @@ import { useGetCertBody } from "@app/hooks/api"; import { UsePopUpState } from "@app/hooks/usePopUp"; import { CertificateContent } from "./CertificateContent"; +import { + ProjectPermissionCertificateActions, + ProjectPermissionSub, + useProjectPermission +} from "@app/context"; +import { useGetCertBundle } from "@app/hooks/api/certificates/queries"; type Props = { popUp: UsePopUpState<["certificateCert"]>; @@ -10,10 +16,20 @@ type Props = { }; export const CertificateCertModal = ({ popUp, handlePopUpToggle }: Props) => { - const { data } = useGetCertBody( - (popUp?.certificateCert?.data as { serialNumber: string })?.serialNumber || "" + const { permission } = useProjectPermission(); + + const serialNumber = + (popUp?.certificateCert?.data as { serialNumber: string })?.serialNumber || ""; + + const canReadPrivateKey = permission.can( + ProjectPermissionCertificateActions.ReadPrivateKey, + ProjectPermissionSub.Certificates ); + const { data } = canReadPrivateKey + ? useGetCertBundle(serialNumber) + : useGetCertBody(serialNumber); + return ( { serialNumber={data.serialNumber} certificate={data.certificate} certificateChain={data.certificateChain} + // A hacky fix for typescript error + privateKey={(data as { privateKey?: string }).privateKey} /> ) : (
From b6e6a3c6bef2e3306cd6baf8ba8214dfcf7318d4 Mon Sep 17 00:00:00 2001 From: x032205 Date: Mon, 5 May 2025 14:50:54 -0400 Subject: [PATCH 11/12] docs changes --- docs/api-reference/endpoints/certificates/bundle.mdx | 8 ++++++++ .../endpoints/certificates/private-key.mdx | 4 ++++ docs/internals/permissions/project-permissions.mdx | 11 ++++++----- 3 files changed, 18 insertions(+), 5 deletions(-) create mode 100644 docs/api-reference/endpoints/certificates/bundle.mdx create mode 100644 docs/api-reference/endpoints/certificates/private-key.mdx diff --git a/docs/api-reference/endpoints/certificates/bundle.mdx b/docs/api-reference/endpoints/certificates/bundle.mdx new file mode 100644 index 000000000..78cf7e073 --- /dev/null +++ b/docs/api-reference/endpoints/certificates/bundle.mdx @@ -0,0 +1,8 @@ +--- +title: "List" +openapi: "GET /api/v2/workspace/{slug}/bundle" +--- + + + You must have the certificate `read-private-key` permission in order to call this endpoint. + diff --git a/docs/api-reference/endpoints/certificates/private-key.mdx b/docs/api-reference/endpoints/certificates/private-key.mdx new file mode 100644 index 000000000..15056fceb --- /dev/null +++ b/docs/api-reference/endpoints/certificates/private-key.mdx @@ -0,0 +1,4 @@ +--- +title: "List" +openapi: "GET /api/v2/workspace/{slug}/private-key" +--- diff --git a/docs/internals/permissions/project-permissions.mdx b/docs/internals/permissions/project-permissions.mdx index 4e0c592cb..acf95485b 100644 --- a/docs/internals/permissions/project-permissions.mdx +++ b/docs/internals/permissions/project-permissions.mdx @@ -252,11 +252,12 @@ Supports conditions and permission inversion #### Subject: `certificates` -| Action | Description | -| -------- | ----------------------------- | -| `read` | View certificates | -| `create` | Issue new certificates | -| `delete` | Revoke or remove certificates | +| Action | Description | +| -------------------- | ----------------------------- | +| `read` | View certificates | +| `read-private-key` | Read certificate private key | +| `create` | Issue new certificates | +| `delete` | Revoke or remove certificates | #### Subject: `certificate-templates` From f6e802c017672192205c8d844db918e856f25765 Mon Sep 17 00:00:00 2001 From: x032205 Date: Mon, 5 May 2025 15:07:57 -0400 Subject: [PATCH 12/12] review fixes: docs + frontend --- .../endpoints/certificates/bundle.mdx | 2 +- .../endpoints/certificates/private-key.mdx | 2 +- .../components/CertificateCertModal.tsx | 26 ++++++++++++------- 3 files changed, 19 insertions(+), 11 deletions(-) diff --git a/docs/api-reference/endpoints/certificates/bundle.mdx b/docs/api-reference/endpoints/certificates/bundle.mdx index 78cf7e073..5fbda7d96 100644 --- a/docs/api-reference/endpoints/certificates/bundle.mdx +++ b/docs/api-reference/endpoints/certificates/bundle.mdx @@ -1,5 +1,5 @@ --- -title: "List" +title: "Get Certificate Bundle" openapi: "GET /api/v2/workspace/{slug}/bundle" --- diff --git a/docs/api-reference/endpoints/certificates/private-key.mdx b/docs/api-reference/endpoints/certificates/private-key.mdx index 15056fceb..244aecea3 100644 --- a/docs/api-reference/endpoints/certificates/private-key.mdx +++ b/docs/api-reference/endpoints/certificates/private-key.mdx @@ -1,4 +1,4 @@ --- -title: "List" +title: "Get Certificate Private Key" openapi: "GET /api/v2/workspace/{slug}/private-key" --- diff --git a/frontend/src/pages/cert-manager/CertificatesPage/components/CertificateCertModal.tsx b/frontend/src/pages/cert-manager/CertificatesPage/components/CertificateCertModal.tsx index 081a59e67..54620f1d6 100644 --- a/frontend/src/pages/cert-manager/CertificatesPage/components/CertificateCertModal.tsx +++ b/frontend/src/pages/cert-manager/CertificatesPage/components/CertificateCertModal.tsx @@ -1,14 +1,14 @@ import { Modal, ModalContent } from "@app/components/v2"; -import { useGetCertBody } from "@app/hooks/api"; -import { UsePopUpState } from "@app/hooks/usePopUp"; - -import { CertificateContent } from "./CertificateContent"; import { ProjectPermissionCertificateActions, ProjectPermissionSub, useProjectPermission } from "@app/context"; +import { useGetCertBody } from "@app/hooks/api"; import { useGetCertBundle } from "@app/hooks/api/certificates/queries"; +import { UsePopUpState } from "@app/hooks/usePopUp"; + +import { CertificateContent } from "./CertificateContent"; type Props = { popUp: UsePopUpState<["certificateCert"]>; @@ -26,9 +26,18 @@ export const CertificateCertModal = ({ popUp, handlePopUpToggle }: Props) => { ProjectPermissionSub.Certificates ); - const { data } = canReadPrivateKey - ? useGetCertBundle(serialNumber) - : useGetCertBody(serialNumber); + // useGetCertBundle fails unless user has the correct permissions + const { data: bundleData } = useGetCertBundle(serialNumber); + const { data: bodyData } = useGetCertBody(serialNumber); + + const data: + | { + certificate: string; + certificateChain: string; + serialNumber: string; + privateKey?: string; + } + | undefined = canReadPrivateKey ? bundleData : bodyData; return ( { serialNumber={data.serialNumber} certificate={data.certificate} certificateChain={data.certificateChain} - // A hacky fix for typescript error - privateKey={(data as { privateKey?: string }).privateKey} + privateKey={data.privateKey} /> ) : (