More error handling

backend/src/ee/services/pki-acme/pki-acme-challenge-service.ts

@@ -5,7 +5,7 @@ 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 { AcmeIncorrectResponseError } from "./pki-acme-errors";
+import { AcmeConnectionError, AcmeDnsFailureError, AcmeIncorrectResponseError } from "./pki-acme-errors";
 import { AcmeAuthStatus, AcmeChallengeStatus, AcmeChallengeType } from "./pki-acme-schemas";
 import { TPkiAcmeChallengeServiceFactory } from "./pki-acme-types";
 import { TPkiAcmeChallenges } from "@app/db/schemas";
@@ -25,7 +25,7 @@ export const pkiAcmeChallengeServiceFactory = ({
   const appCfg = getConfig();
 
   const validateChallengeResponse = async (challengeId: string): Promise<void> => {
-    return await acmeChallengeDAL.transaction(async (tx) => {
+    const error = await acmeChallengeDAL.transaction(async (tx) => {
       logger.info({ challengeId }, "Validating ACME challenge response");
       const challenge = await acmeChallengeDAL.findByIdForChallengeValidation(challengeId, tx);
       if (!challenge) {
@@ -54,6 +54,7 @@ export const pkiAcmeChallengeServiceFactory = ({
         ? `${baseUrl}:${appCfg.ACME_DEVELOPMENT_HTTP01_CHALLENGE_PORT}`
         : baseUrl;
       const challengeUrl = new URL(`/.well-known/acme-challenge/${challenge.auth.token}`, actualBaseUrl);
+      logger.info({ challengeUrl }, "Performing ACME HTTP-01 challenge validation");
       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
@@ -66,17 +67,34 @@ export const pkiAcmeChallengeServiceFactory = ({
         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) {
+        if (challengeResponseBody.trimEnd() !== expectedChallengeResponseBody) {
           throw new AcmeIncorrectResponseError({ message: "ACME challenge response is not correct" });
         }
         await acmeChallengeDAL.markAsValidCascadeById(challengeId, 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.markAsValidCascadeById(challengeId, tx);
-        throw error;
+        // Properly type and inspect the error
+        if (error instanceof TypeError && error.message.includes("fetch failed")) {
+          const cause = error.cause as AggregateError;
+          if (cause?.errors?.[0]?.code === "ECONNREFUSED") {
+            logger.error(error, "Connection refused.");
+            return new AcmeConnectionError({ message: "Connection refused." });
+          } else if (cause?.errors?.[0]?.code === "ENOTFOUND") {
+            logger.error(error, "Hostname could not be resolved (DNS failure).");
+            return new AcmeDnsFailureError({ message: "Hostname could not be resolved (DNS failure)." });
+          }
+        } else if (error instanceof Error) {
+          logger.error(error, "Error validating ACME challenge response");
+        } else {
+          logger.error(error, "Unknown error validating ACME challenge response");
+        }
+        return error;
       }
     });
+    if (error) {
+      throw error;
+    }
   };
 
   return { validateChallengeResponse };

backend/src/ee/services/pki-acme/pki-acme-errors.ts

@@ -3,15 +3,43 @@
  * https://datatracker.ietf.org/doc/html/rfc8555#section-6.2
  */
 
+// RFC 8555 Section 6.7 - Error Types
+export enum AcmeErrorType {
+  AccountDoesNotExist = "accountDoesNotExist",
+  AlreadyRevoked = "alreadyRevoked",
+  BadCsr = "badCSR",
+  BadNonce = "badNonce",
+  BadPublicKey = "badPublicKey",
+  BadRevocationReason = "badRevocationReason",
+  BadSignatureAlgorithm = "badSignatureAlgorithm",
+  CAA = "CAA",
+  Compound = "compound",
+  Connection = "connection",
+  DNS = "DNS",
+  ExternalAccountRequired = "externalAccountRequired",
+  IncorrectResponse = "incorrectResponse",
+  IncorrectContact = "incorrectContact",
+  Malformed = "malformed",
+  OrderNotReady = "orderNotReady",
+  RateLimited = "rateLimited",
+  RejectedIdentifier = "rejectedIdentifier",
+  ServerInternal = "serverInternal",
+  TLS = "tls",
+  Unauthorized = "unauthorized",
+  UnsupportedContact = "unsupportedContact",
+  UnsupportedIdentifier = "unsupportedIdentifier",
+  UserActionRequired = "userActionRequired"
+}
+
 export interface IAcmeError {
-  type: string;
+  type: AcmeErrorType;
   detail: string;
   status: number;
   subproblems?: Array<{ type: string; detail: string; identifier?: { type: string; value: string } }>;
 }
 
 export class AcmeError extends Error implements IAcmeError {
-  type: string;
+  type: AcmeErrorType;
 
   detail: string;
 
@@ -29,7 +57,7 @@ export class AcmeError extends Error implements IAcmeError {
     error,
     message
   }: {
-    type: string;
+    type: AcmeErrorType;
     detail: string;
     status: number;
     subproblems?: Array<{ type: string; detail: string; identifier?: { type: string; value: string } }>;
@@ -69,7 +97,7 @@ export class AcmeMalformedError extends AcmeError {
     message?: string;
   } = {}) {
     super({
-      type: "malformed",
+      type: AcmeErrorType.Malformed,
       detail,
       status: 400,
       error,
@@ -93,7 +121,7 @@ export class AcmeUnauthorizedError extends AcmeError {
     message?: string;
   } = {}) {
     super({
-      type: "unauthorized",
+      type: AcmeErrorType.Unauthorized,
       detail,
       status: 403,
       error,
@@ -118,7 +146,7 @@ export class AcmeAccountDoesNotExistError extends AcmeError {
     message?: string;
   } = {}) {
     super({
-      type: "accountDoesNotExist",
+      type: AcmeErrorType.AccountDoesNotExist,
       detail,
       status: 400,
       error,
@@ -142,7 +170,7 @@ export class AcmeBadNonceError extends AcmeError {
     message?: string;
   } = {}) {
     super({
-      type: "badNonce",
+      type: AcmeErrorType.BadNonce,
       detail,
       status: 400,
       error,
@@ -153,11 +181,11 @@ export class AcmeBadNonceError extends AcmeError {
 }
 
 /**
- * badSignature - The JWS signature is invalid (RFC 8555 Section 6.7.5)
+ * badSignatureAlgorithm - The signature algorithm is invalid (RFC 8555 Section 6.7.5)
  */
-export class AcmeBadSignatureError extends AcmeError {
+export class AcmeBadSignatureAlgorithmError extends AcmeError {
   constructor({
-    detail = "The JWS signature is invalid",
+    detail = "The signature algorithm is invalid",
     error,
     message
   }: {
@@ -166,13 +194,13 @@ export class AcmeBadSignatureError extends AcmeError {
     message?: string;
   } = {}) {
     super({
-      type: "badSignature",
+      type: AcmeErrorType.BadSignatureAlgorithm,
       detail,
       status: 401,
       error,
       message
     });
-    this.name = "AcmeBadSignatureError";
+    this.name = "AcmeBadSignatureAlgorithmError";
   }
 }
 
@@ -190,7 +218,7 @@ export class AcmeBadPublicKeyError extends AcmeError {
     message?: string;
   } = {}) {
     super({
-      type: "badPublicKey",
+      type: AcmeErrorType.BadPublicKey,
       detail,
       status: 400,
       error,
@@ -214,7 +242,7 @@ export class AcmeBadCsrError extends AcmeError {
     message?: string;
   } = {}) {
     super({
-      type: "badCSR",
+      type: AcmeErrorType.BadCsr,
       detail,
       status: 400,
       error,
@@ -239,7 +267,7 @@ export class AcmeBadRevocationReasonError extends AcmeError {
     message?: string;
   } = {}) {
     super({
-      type: "badRevocationReason",
+      type: AcmeErrorType.BadRevocationReason,
       detail,
       status: 400,
       error,
@@ -263,7 +291,7 @@ export class AcmeRateLimitedError extends AcmeError {
     message?: string;
   } = {}) {
     super({
-      type: "rateLimited",
+      type: AcmeErrorType.RateLimited,
       detail,
       status: 429,
       error,
@@ -290,7 +318,7 @@ export class AcmeRejectedIdentifierError extends AcmeError {
     message?: string;
   } = {}) {
     super({
-      type: "rejectedIdentifier",
+      type: AcmeErrorType.RejectedIdentifier,
       detail,
       status: 400,
       subproblems,
@@ -315,7 +343,7 @@ export class AcmeServerInternalError extends AcmeError {
     message?: string;
   } = {}) {
     super({
-      type: "serverInternal",
+      type: AcmeErrorType.ServerInternal,
       detail,
       status: 500,
       error,
@@ -325,30 +353,6 @@ export class AcmeServerInternalError extends AcmeError {
   }
 }
 
-/**
- * serviceUnavailable - The service is unavailable (RFC 8555 Section 6.7.12)
- */
-export class AcmeServiceUnavailableError extends AcmeError {
-  constructor({
-    detail = "The service is unavailable",
-    error,
-    message
-  }: {
-    detail?: string;
-    error?: unknown;
-    message?: string;
-  } = {}) {
-    super({
-      type: "serviceUnavailable",
-      detail,
-      status: 503,
-      error,
-      message
-    });
-    this.name = "AcmeServiceUnavailableError";
-  }
-}
-
 /**
  * unsupportedContact - A contact URL is of an unsupported type (RFC 8555 Section 6.7.13)
  */
@@ -363,7 +367,7 @@ export class AcmeUnsupportedContactError extends AcmeError {
     message?: string;
   } = {}) {
     super({
-      type: "unsupportedContact",
+      type: AcmeErrorType.UnsupportedContact,
       detail,
       status: 400,
       error,
@@ -388,7 +392,7 @@ export class AcmeUnsupportedIdentifierError extends AcmeError {
     message?: string;
   } = {}) {
     super({
-      type: "unsupportedIdentifier",
+      type: AcmeErrorType.UnsupportedIdentifier,
       detail,
       status: 400,
       error,
@@ -417,7 +421,7 @@ export class AcmeUserActionRequiredError extends AcmeError {
     message?: string;
   } = {}) {
     super({
-      type: "userActionRequired",
+      type: AcmeErrorType.UserActionRequired,
       detail,
       status: 403,
       error,
@@ -449,7 +453,7 @@ export class AcmeIncorrectResponseError extends AcmeError {
     message?: string;
   } = {}) {
     super({
-      type: "incorrectResponse",
+      type: AcmeErrorType.IncorrectResponse,
       detail,
       status: 400,
       error,
@@ -458,3 +462,48 @@ export class AcmeIncorrectResponseError extends AcmeError {
     this.name = "AcmeIncorrectResponseError";
   }
 }
+
+/**
+ * connectionError - A connection error occurred (RFC 8555 Section 6.7.17)
+ */
+export class AcmeConnectionError extends AcmeError {
+  constructor({
+    detail = "A connection error occurred",
+    error,
+    message
+  }: {
+    detail?: string;
+    error?: unknown;
+    message?: string;
+  } = {}) {
+    super({
+      type: AcmeErrorType.Connection,
+      detail,
+      status: 400,
+      error,
+      message
+    });
+    this.name = "AcmeConnectionError";
+  }
+}
+
+export class AcmeDnsFailureError extends AcmeError {
+  constructor({
+    detail = "Hostname could not be resolved (DNS failure)",
+    error,
+    message
+  }: {
+    detail?: string;
+    error?: unknown;
+    message?: string;
+  } = {}) {
+    super({
+      type: AcmeErrorType.DNS,
+      detail,
+      status: 400,
+      error,
+      message
+    });
+    this.name = "AcmeDnsFailureError";
+  }
+}

backend/src/ee/services/pki-acme/pki-acme-schemas.ts

@@ -158,7 +158,7 @@ export const GetAcmeAuthorizationResponseSchema = z.object({
   )
 });
 
-export const RespondToAcmeChallengeBodySchema = z.object({});
+export const RespondToAcmeChallengeBodySchema = z.object({}).strict();
 
 export const RespondToAcmeChallengeResponseSchema = z.object({
   type: z.enum(Object.values(AcmeChallengeType) as [string, ...string[]]),