diff --git a/backend/src/ee/services/pki-acme/pki-acme-challenge-service.ts b/backend/src/ee/services/pki-acme/pki-acme-challenge-service.ts index 6b820032b..f4100c6aa 100644 --- a/backend/src/ee/services/pki-acme/pki-acme-challenge-service.ts +++ b/backend/src/ee/services/pki-acme/pki-acme-challenge-service.ts @@ -1,5 +1,6 @@ import { getConfig } from "@app/lib/config/env"; import { BadRequestError, NotFoundError } from "@app/lib/errors"; +import { isPrivateIp } from "@app/lib/ip/ipRange"; import { logger } from "@app/lib/logger"; import { TPkiAcmeChallengeDALFactory } from "./pki-acme-challenge-dal"; import { @@ -53,6 +54,10 @@ export const pkiAcmeChallengeServiceFactory = ({ throw new BadRequestError({ message: "Only HTTP-01 challenges are supported for now" }); } let host = challenge.auth.identifierValue; + // check if host is a private ip address + if (isPrivateIp(host)) { + throw new BadRequestError({ message: "Private IP addresses are not allowed" }); + } if (appCfg.isAcmeDevelopmentMode && appCfg.ACME_DEVELOPMENT_HTTP01_CHALLENGE_HOST_OVERRIDES[host]) { host = appCfg.ACME_DEVELOPMENT_HTTP01_CHALLENGE_HOST_OVERRIDES[host]; logger.warn( diff --git a/backend/src/ee/services/pki-acme/pki-acme-service.ts b/backend/src/ee/services/pki-acme/pki-acme-service.ts index dfde68891..4f4eb93ad 100644 --- a/backend/src/ee/services/pki-acme/pki-acme-service.ts +++ b/backend/src/ee/services/pki-acme/pki-acme-service.ts @@ -7,6 +7,7 @@ import { TCertificateProfileDALFactory } from "@app/services/certificate-profile import * as x509 from "@peculiar/x509"; import { KeyStorePrefixes, TKeyStoreFactory } from "@app/keystore/keystore"; +import { isPrivateIp } from "@app/lib/ip/ipRange"; import { ActorType } from "@app/services/auth/auth-type"; import { EnrollmentType, @@ -497,7 +498,9 @@ export const pkiAcmeServiceFactory = ({ if (identifier.type !== AcmeIdentifierType.DNS) { throw new AcmeUnsupportedIdentifierError({ detail: "Only DNS identifiers are supported" }); } - // TODO: reuse existing authorizations for this identifier if they exist + if (isPrivateIp(identifier.value)) { + throw new AcmeUnsupportedIdentifierError({ detail: "Private IP addresses are not allowed" }); + } const auth = await acmeAuthDAL.create( { accountId: account.id, @@ -600,8 +603,6 @@ export const pkiAcmeServiceFactory = ({ throw new AcmeOrderNotReadyError({ message: "ACME order has expired" }); } const { csr } = payload; - // TODO: validate the CSR and return badCSR error if it's invalid - // TODO: this should be the same transaction? let errorToReturn: Error | undefined; try { const { certificateId } = await certificateV3Service.signCertificateFromProfile({ diff --git a/backend/src/server/routes/index.ts b/backend/src/server/routes/index.ts index cd8fac508..961c0a940 100644 --- a/backend/src/server/routes/index.ts +++ b/backend/src/server/routes/index.ts @@ -2169,6 +2169,7 @@ export const registerRoutes = async ( certificateAuthorityDAL, certificateProfileDAL, certificateTemplateV2Service, + acmeAccountDAL, internalCaService: internalCertificateAuthorityService, permissionService, certificateSyncDAL, diff --git a/backend/src/services/certificate-v3/certificate-v3-service.test.ts b/backend/src/services/certificate-v3/certificate-v3-service.test.ts index 36823671f..67c3b268b 100644 --- a/backend/src/services/certificate-v3/certificate-v3-service.test.ts +++ b/backend/src/services/certificate-v3/certificate-v3-service.test.ts @@ -30,6 +30,7 @@ import { extractCertificateRequestFromCSR } from "../certificate-common/certificate-csr-utils"; import { certificateV3ServiceFactory, TCertificateV3ServiceFactory } from "./certificate-v3-service"; +import { TPkiAcmeAccountDALFactory } from "@app/ee/services/pki-acme/pki-acme-account-dal"; vi.mock("../certificate-common/certificate-csr-utils", () => ({ extractCertificateRequestFromCSR: vi.fn(), @@ -69,6 +70,10 @@ describe("CertificateV3Service", () => { getTemplateV2ById: vi.fn() }; + const mockAcmeAccountDAL: Pick = { + findById: vi.fn() + }; + const mockInternalCaService: Pick = { signCertFromCa: vi.fn(), @@ -132,6 +137,7 @@ describe("CertificateV3Service", () => { certificateAuthorityDAL: mockCertificateAuthorityDAL, certificateProfileDAL: mockCertificateProfileDAL, certificateTemplateV2Service: mockCertificateTemplateV2Service, + acmeAccountDAL: mockAcmeAccountDAL, internalCaService: mockInternalCaService, permissionService: mockPermissionService, certificateSyncDAL: { diff --git a/backend/src/services/certificate-v3/certificate-v3-service.ts b/backend/src/services/certificate-v3/certificate-v3-service.ts index 5ccd02698..72346c55c 100644 --- a/backend/src/services/certificate-v3/certificate-v3-service.ts +++ b/backend/src/services/certificate-v3/certificate-v3-service.ts @@ -64,12 +64,14 @@ import { TSignCertificateFromProfileDTO, TUpdateRenewalConfigDTO } from "./certificate-v3-types"; +import { TPkiAcmeAccountDALFactory } from "@app/ee/services/pki-acme/pki-acme-account-dal"; type TCertificateV3ServiceFactoryDep = { certificateDAL: Pick; certificateSecretDAL: Pick; certificateAuthorityDAL: Pick; certificateProfileDAL: Pick; + acmeAccountDAL: Pick; certificateTemplateV2Service: Pick< TCertificateTemplateV2ServiceFactory, "validateCertificateRequest" | "getTemplateV2ById" @@ -93,6 +95,7 @@ const validateProfileAndPermissions = async ( actorAuthMethod: ActorAuthMethod, actorOrgId: string, certificateProfileDAL: Pick, + acmeAccountDAL: Pick, permissionService: Pick, requiredEnrollmentType: EnrollmentType ) => { @@ -107,10 +110,16 @@ const validateProfileAndPermissions = async ( }); } - // XXX: NOT SURE IF THIS IS SECURE TO BY PASS THE PERMISSION CHECK FOR ACME ACCOUNTS - // may need to consider this carefully - // TODO: check actor/profile ownership as well if (actor === ActorType.ACME_ACCOUNT && requiredEnrollmentType === EnrollmentType.ACME) { + const account = await acmeAccountDAL.findById(actorId); + if (!account) { + throw new NotFoundError({ message: "ACME account not found" }); + } + if (account.profileId !== profile.id) { + throw new ForbiddenRequestError({ + message: "ACME account is not associated with this profile" + }); + } return profile; } @@ -343,6 +352,7 @@ export const certificateV3ServiceFactory = ({ certificateSecretDAL, certificateAuthorityDAL, certificateProfileDAL, + acmeAccountDAL, certificateTemplateV2Service, internalCaService, permissionService, @@ -365,6 +375,7 @@ export const certificateV3ServiceFactory = ({ actorAuthMethod, actorOrgId, certificateProfileDAL, + acmeAccountDAL, permissionService, EnrollmentType.API );