diff --git a/backend/src/ee/routes/v1/pki-acme-router.ts b/backend/src/ee/routes/v1/pki-acme-router.ts index a66967bbc..8e300f8f6 100644 --- a/backend/src/ee/routes/v1/pki-acme-router.ts +++ b/backend/src/ee/routes/v1/pki-acme-router.ts @@ -33,8 +33,9 @@ export interface MyRequestInterface { export const registerPkiAcmeRouter = async (server: FastifyZodProvider) => { const validateExistingAccount = async < + // eslint-disable-next-line @typescript-eslint/no-explicit-any R extends FastifyRequest, - TSchema extends z.ZodSchema | undefined = undefined, + TSchema extends z.ZodSchema | undefined = undefined, T = TSchema extends z.ZodSchema ? R : string >({ req, @@ -70,7 +71,7 @@ export const registerPkiAcmeRouter = async (server: FastifyZodProvider) => { if (!strBody) { done(null, undefined); } - const json: unknown = JSON.parse(strBody as string); + const json = JSON.parse(strBody as string); done(null, json); } catch (err) { const error = err as Error; 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 52a277c31..9567072a1 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,7 +2,12 @@ import { getConfig } from "@app/lib/config/env"; import { BadRequestError, NotFoundError } from "@app/lib/errors"; import { logger } from "@app/lib/logger"; import { TPkiAcmeChallengeDALFactory } from "./pki-acme-challenge-dal"; -import { AcmeConnectionError, AcmeDnsFailureError, AcmeIncorrectResponseError } from "./pki-acme-errors"; +import { + AcmeConnectionError, + AcmeDnsFailureError, + AcmeIncorrectResponseError, + AcmeServerInternalError +} from "./pki-acme-errors"; import { AcmeAuthStatus, AcmeChallengeStatus, AcmeChallengeType } from "./pki-acme-schemas"; import { TPkiAcmeChallengeServiceFactory } from "./pki-acme-types"; @@ -23,7 +28,7 @@ export const pkiAcmeChallengeServiceFactory = ({ const appCfg = getConfig(); const validateChallengeResponse = async (challengeId: string): Promise => { - const error = await acmeChallengeDAL.transaction(async (tx) => { + const error: Error | undefined = await acmeChallengeDAL.transaction(async (tx) => { logger.info({ challengeId }, "Validating ACME challenge response"); const challenge = await acmeChallengeDAL.findByIdForChallengeValidation(challengeId, tx); if (!challenge) { @@ -98,6 +103,7 @@ export const pkiAcmeChallengeServiceFactory = ({ logger.error(exp, "Error validating ACME challenge response"); } else { logger.error(exp, "Unknown error validating ACME challenge response"); + return new AcmeServerInternalError({ message: "Unknown error validating ACME challenge response" }); } return exp; } diff --git a/backend/src/ee/services/pki-acme/pki-acme-order-dal.ts b/backend/src/ee/services/pki-acme/pki-acme-order-dal.ts index bb3671daa..5aab0be63 100644 --- a/backend/src/ee/services/pki-acme/pki-acme-order-dal.ts +++ b/backend/src/ee/services/pki-acme/pki-acme-order-dal.ts @@ -48,8 +48,8 @@ export const pkiAcmeOrderDALFactory = (db: TDbClient) => { label: "authorizations" as const, mapper: ({ authId, identifierType, identifierValue, authExpiresAt }) => ({ id: authId, - identifierType: identifierType, - identifierValue: identifierValue, + identifierType, + identifierValue, expiresAt: authExpiresAt }) } 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 9f848748e..91e444e8d 100644 --- a/backend/src/ee/services/pki-acme/pki-acme-service.ts +++ b/backend/src/ee/services/pki-acme/pki-acme-service.ts @@ -441,14 +441,13 @@ export const pkiAcmeServiceFactory = ({ const deactivateAcmeAccount = async ({ profileId, - accountId, - payload: { status } = { status: "deactivated" } + accountId }: { profileId: string; accountId: string; payload?: TDeactivateAcmeAccountPayload; }): Promise> => { - const profile = await validateAcmeProfile(profileId); + await validateAcmeProfile(profileId); // FIXME: Implement ACME account deactivation return { status: 200, @@ -494,36 +493,35 @@ export const pkiAcmeServiceFactory = ({ ); const authorizations: TPkiAcmeAuths[] = await Promise.all( payload.identifiers.map(async (identifier) => { - if (identifier.type === AcmeIdentifierType.DNS) { - // TODO: reuse existing authorizations for this identifier if they exist - const auth = await acmeAuthDAL.create( - { - accountId: account.id, - status: AcmeAuthStatus.Pending, - identifierType: identifier.type, - identifierValue: identifier.value, - // RFC 8555 suggests a token with at least 128 bits of entropy - // We are using 256 bits of entropy here, should be enough for now - // ref: https://datatracker.ietf.org/doc/html/rfc8555#section-11.3 - token: crypto.randomBytes(32).toString("base64url"), - // TODO: read config from the profile to get the expiration time instead - expiresAt: new Date(Date.now() + 24 * 60 * 60 * 1000) - }, - tx - ); - // TODO: support other challenge types here. Currently only HTTP-01 is supported. - await acmeChallengeDAL.create( - { - authId: auth.id, - status: AcmeChallengeStatus.Pending, - type: AcmeChallengeType.HTTP_01 - }, - tx - ); - return auth; - } else { + if (identifier.type !== AcmeIdentifierType.DNS) { throw new AcmeUnsupportedIdentifierError({ detail: "Only DNS identifiers are supported" }); } + // TODO: reuse existing authorizations for this identifier if they exist + const auth = await acmeAuthDAL.create( + { + accountId: account.id, + status: AcmeAuthStatus.Pending, + identifierType: identifier.type, + identifierValue: identifier.value, + // RFC 8555 suggests a token with at least 128 bits of entropy + // We are using 256 bits of entropy here, should be enough for now + // ref: https://datatracker.ietf.org/doc/html/rfc8555#section-11.3 + token: crypto.randomBytes(32).toString("base64url"), + // TODO: read config from the profile to get the expiration time instead + expiresAt: new Date(Date.now() + 24 * 60 * 60 * 1000) + }, + tx + ); + // TODO: support other challenge types here. Currently only HTTP-01 is supported. + await acmeChallengeDAL.create( + { + authId: auth.id, + status: AcmeChallengeStatus.Pending, + type: AcmeChallengeType.HTTP_01 + }, + tx + ); + return auth; }) ); @@ -591,13 +589,13 @@ export const pkiAcmeServiceFactory = ({ } if (order.status === AcmeOrderStatus.Ready) { const { order: updatedOrder, error } = await acmeOrderDAL.transaction(async (tx) => { - const order = (await acmeOrderDAL.findByIdForFinalization(orderId, tx))!; + const finalizingOrder = (await acmeOrderDAL.findByIdForFinalization(orderId, tx))!; // TODO: ideally, this should be doen with onRequest: verifyAuth([AuthMode.ACME_JWS_SIGNATURE]), instead? const { ownerOrgId: actorOrgId } = (await certificateProfileDAL.findByIdWithOwnerOrgId(profileId, tx))!; - if (order.status !== AcmeOrderStatus.Ready) { + if (finalizingOrder.status !== AcmeOrderStatus.Ready) { throw new AcmeOrderNotReadyError({ message: "ACME order is not ready" }); } - if (order.expiresAt < new Date()) { + if (finalizingOrder.expiresAt < new Date()) { throw new AcmeOrderNotReadyError({ message: "ACME order has expired" }); } const { csr } = payload; @@ -612,8 +610,8 @@ export const pkiAcmeServiceFactory = ({ actorOrgId, profileId, csr, - notBefore: order.notBefore ? new Date(order.notBefore) : undefined, - notAfter: order.notAfter ? new Date(order.notAfter) : undefined, + notBefore: finalizingOrder.notBefore ? new Date(finalizingOrder.notBefore) : undefined, + notAfter: finalizingOrder.notAfter ? new Date(finalizingOrder.notAfter) : undefined, validity: { // TODO: read config from the profile to get the expiration time instead ttl: (24 * 60 * 60 * 1000).toString() @@ -630,20 +628,20 @@ export const pkiAcmeServiceFactory = ({ }, tx ); - } catch (error) { + } catch (exp) { await acmeOrderDAL.updateById( orderId, { csr, status: AcmeOrderStatus.Invalid, - error: error instanceof Error ? error.message : "Unknown error" + error: exp instanceof Error ? exp.message : "Unknown error" }, tx ); - logger.error(error, "Failed to sign certificate"); + logger.error(exp, "Failed to sign certificate"); // TODO: audit log the error - if (error instanceof BadRequestError) { - errorToReturn = new AcmeBadCSRError({ detail: `Invalid CSR: ${error.message}` }); + if (exp instanceof BadRequestError) { + errorToReturn = new AcmeBadCSRError({ detail: `Invalid CSR: ${exp.message}` }); } else { errorToReturn = new AcmeServerInternalError({ detail: "Failed to sign certificate with internal error" }); } @@ -710,15 +708,14 @@ export const pkiAcmeServiceFactory = ({ }); const certificateChain = decryptedCertChain.toString(); + const certLeaf = certObj.toString("pem").trim().replace("\n", "\r\n"); + const certChain = certificateChain.trim().replace("\n", "\r\n"); return { status: 200, body: - certObj.toString("pem").trim().replace("\n", "\r\n") + - "\r\n" + - certificateChain.trim().replace("\n", "\r\n") + // The final line is needed, otherwise some clients will not parse the certificate chain correctly // ref: https://github.com/certbot/certbot/blob/4d5d5f7ae8164884c841969e46caed8db1ad34af/certbot/src/certbot/crypto_util.py#L506-L514 - "\r\n", + `${certLeaf}\r\n${certChain}\r\n`, headers: { Location: buildUrl(profileId, `/orders/${orderId}/certificate`), Link: `<${buildUrl(profileId, "/directory")}>;rel="index"`