From 0613a497950a365191b398783113d3ef989645d7 Mon Sep 17 00:00:00 2001 From: Fang-Pen Lin Date: Fri, 31 Oct 2025 13:23:30 -0700 Subject: [PATCH] More challenge logic --- .../pki-acme/pki-acme-challenge-service.ts | 56 +++++++++++-------- .../ee/services/pki-acme/pki-acme-service.ts | 3 + 2 files changed, 36 insertions(+), 23 deletions(-) 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 03a5c1d30..9091b6926 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 @@ -2,6 +2,7 @@ import { Knex } from "knex"; import { getConfig } from "@app/lib/config/env"; import { BadRequestError, NotFoundError } from "@app/lib/errors"; +import { logger } from "@app/lib/logger"; import { TPkiAcmeAuthDALFactory } from "./pki-acme-auth-dal"; import { TPkiAcmeChallengeDALFactory } from "./pki-acme-challenge-dal"; import { AcmeAuthStatus, AcmeChallengeStatus, AcmeChallengeType } from "./pki-acme-schemas"; @@ -18,15 +19,16 @@ export const pkiAcmeChallengeServiceFactory = ({ }: TPkiAcmeChallengeServiceFactoryDep): TPkiAcmeChallengeServiceFactory => { const appCfg = getConfig(); - const validateChallengeResponse = async (challengeId: string): Promise => { + const validateChallengeResponse = async (challengeId: string, tx?: Knex): Promise => { return await acmeChallengeDAL.transaction(async (tx: Knex) => { + logger.info({ challengeId }, "Validating ACME challenge response"); const challenge = await acmeChallengeDAL.findByIdForChallengeValidation(challengeId, tx); if (!challenge) { throw new NotFoundError({ message: "ACME challenge not found" }); } - if (challenge.status !== AcmeChallengeStatus.Processing) { + if (challenge.status !== AcmeChallengeStatus.Pending) { throw new BadRequestError({ - message: `ACME challenge is ${challenge.status} instead of ${AcmeChallengeStatus.Processing}` + message: `ACME challenge is ${challenge.status} instead of ${AcmeChallengeStatus.Pending}` }); } if (challenge.auth.expiresAt < new Date()) { @@ -46,28 +48,36 @@ export const pkiAcmeChallengeServiceFactory = ({ const actualBaseUrl = appCfg.isAcmeDevelopmentMode ? `${baseUrl}:${appCfg.ACME_DEVELOPMENT_HTTP01_CHALLENGE_PORT}` : baseUrl; - const challengeUrl = new URL(`/.well-known/acme-challenge/${challenge.auth.token}`, actualBaseUrl); - // Notice: well, we are in a transaction, ideally we should not hold transaction and perform - // a long running operation for long time. But assuming we are not performing a tons of - // challenge validation at the same time, it should be fine. - // TODO: bound it with timeout of the fetch request - const challengeResponse = await fetch(challengeUrl); - if (challengeResponse.status !== 200) { - throw new BadRequestError({ message: "ACME challenge response is not 200" }); + try { + // Notice: well, we are in a transaction, ideally we should not hold transaction and perform + // a long running operation for long time. But assuming we are not performing a tons of + // challenge validation at the same time, it should be fine. + // TODO: bound it with timeout of the fetch request + const challengeResponse = await fetch(challengeUrl); + if (challengeResponse.status !== 200) { + throw new BadRequestError({ message: "ACME challenge response is not 200" }); + } + const challengeResponseBody = await challengeResponse.text(); + const thumbprint = Buffer.from(challenge.auth.account.publicKeyThumbprint, "utf-8").toString("base64url"); + const expectedChallengeResponseBody = `${challenge.auth.token}.${thumbprint}`; + if (challengeResponseBody !== expectedChallengeResponseBody) { + throw new BadRequestError({ message: "ACME challenge response is not correct" }); + } + await acmeChallengeDAL.updateById( + challengeId, + { status: AcmeChallengeStatus.Valid, validatedAt: new Date() }, + tx + ); + await acmeAuthDAL.updateById(challenge.auth.account.id, { status: AcmeAuthStatus.Valid }, tx); + await acmeAuthDAL.updateById(challenge.auth.account.id, { status: AcmeAuthStatus.Valid }, tx); + } catch (error) { + logger.error(error, "Error validating ACME challenge response"); + // TODO: we should retry the challenge validation a few times, but let's keep it simple for now + await acmeChallengeDAL.updateById(challengeId, { status: AcmeChallengeStatus.Invalid }, tx); + await acmeAuthDAL.updateById(challenge.auth.account.id, { status: AcmeAuthStatus.Invalid }, tx); + throw error; } - const challengeResponseBody = await challengeResponse.text(); - const thumbprint = Buffer.from(challenge.auth.account.publicKeyThumbprint, "utf-8").toString("base64url"); - const expectedChallengeResponseBody = `${challenge.auth.token}.${thumbprint}`; - if (challengeResponseBody !== expectedChallengeResponseBody) { - throw new BadRequestError({ message: "ACME challenge response is not correct" }); - } - await acmeChallengeDAL.updateById( - challengeId, - { status: AcmeChallengeStatus.Valid, validatedAt: new Date() }, - tx - ); - await acmeAuthDAL.updateById(challenge.auth.account.id, { status: AcmeAuthStatus.Valid }, tx); }); }; 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 335af2825..8cc96c1b0 100644 --- a/backend/src/ee/services/pki-acme/pki-acme-service.ts +++ b/backend/src/ee/services/pki-acme/pki-acme-service.ts @@ -631,6 +631,9 @@ export const pkiAcmeServiceFactory = ({ if (!challenge.auth.token) { throw new AcmeServerInternalError({ message: "ACME challenge token is required" }); } + if (challenge.type !== AcmeChallengeType.HTTP_01) { + throw new BadRequestError({ message: "Only HTTP-01 challenges are supported for now" }); + } const updatedChallenge = await acmeChallengeDAL.updateById( challengeId, { status: AcmeChallengeStatus.Pending },