More challenge logic

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<void> => {
+  const validateChallengeResponse = async (challengeId: string, tx?: Knex): Promise<void> => {
     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
+      try {
-      //         a long running operation for long time. But assuming we are not performing a tons of
+        // Notice: well, we are in a transaction, ideally we should not hold transaction and perform
-      //         challenge validation at the same time, it should be fine.
+        //         a long running operation for long time. But assuming we are not performing a tons of
-      // TODO: bound it with timeout of the fetch request
+        //         challenge validation at the same time, it should be fine.
-      const challengeResponse = await fetch(challengeUrl);
+        // TODO: bound it with timeout of the fetch request
-      if (challengeResponse.status !== 200) {
+        const challengeResponse = await fetch(challengeUrl);
-        throw new BadRequestError({ message: "ACME challenge response is not 200" });
+        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);
     });
   };
 

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 },