diff --git a/backend/bdd/features/pki/acme/account.feature b/backend/bdd/features/pki/acme/account.feature index 5082155a9..5685167b1 100644 --- a/backend/bdd/features/pki/acme/account.feature +++ b/backend/bdd/features/pki/acme/account.feature @@ -16,13 +16,13 @@ Feature: Account Given I have an ACME cert profile as "acme_profile" When I have an ACME client connecting to {BASE_URL}/api/v1/pki/acme/profiles/{acme_profile.id}/directory Then I register a new ACME account with email fangpen@infisical.com and EAB key id "" with secret "" as acme_account - Then the value error with jq ".type" should be equal to "urn:ietf:params:acme:error:malformed" - Then the value error with jq ".detail" should be equal to "Invalid external account binding JWS signature" + Then the value error with jq ".type" should be equal to "" + Then the value error with jq ".detail" should be equal to "" Examples: Bad Credentials - | eab_kid | eab_secret | - | bad | Cg== | - | bad | Cg== | - | {acme_profile.eab_kid} | Cg== | - | {acme_profile.eab_kid} | YmFkLXNjcmV0Cg== | - | 4bc7959c-fe2d-4447-ae91-0cd893667af6 | {acme_profile.eab_secret} | + | eab_kid | eab_secret | error_type | error_msg | + | bad | Cg== | urn:ietf:params:acme:error:externalAccountRequired | fixme | + | bad | Cg== | urn:ietf:params:acme:error:externalAccountRequired | fixme | + | {acme_profile.eab_kid} | Cg== | urn:ietf:params:acme:error:externalAccountRequired | fixme | + | {acme_profile.eab_kid} | YmFkLXNjcmV0Cg== | urn:ietf:params:acme:error:externalAccountRequired | fixme | + | 4bc7959c-fe2d-4447-ae91-0cd893667af6 | {acme_profile.eab_secret} | urn:ietf:params:acme:error:externalAccountRequired | fixme | diff --git a/backend/bdd/features/steps/pki_acme.py b/backend/bdd/features/steps/pki_acme.py index c07a712de..0a753a827 100644 --- a/backend/bdd/features/steps/pki_acme.py +++ b/backend/bdd/features/steps/pki_acme.py @@ -251,8 +251,8 @@ def step_impl(context: Context, email: str, kid: str, secret: str, account_var: ) try: context.vars[account_var] = acme_client.new_account(registration) - except Exception: - context.vars["error"] = acme_client.new_account(registration) + except Exception as exp: + context.vars["error"] = exp @then("I register a new ACME account with email {email} without EAB") diff --git a/backend/src/ee/services/pki-acme/pki-acme-errors.ts b/backend/src/ee/services/pki-acme/pki-acme-errors.ts index febce5e81..98a89a731 100644 --- a/backend/src/ee/services/pki-acme/pki-acme-errors.ts +++ b/backend/src/ee/services/pki-acme/pki-acme-errors.ts @@ -35,15 +35,14 @@ export enum AcmeErrorType { export interface IAcmeError { type: AcmeErrorType; - detail: string; + message: string; status: number; subproblems?: Array<{ type: string; detail: string; identifier?: { type: string; value: string } }>; } export class AcmeError extends Error implements IAcmeError { type: AcmeErrorType; - - detail: string; + message: string; status: number; @@ -53,22 +52,20 @@ export class AcmeError extends Error implements IAcmeError { constructor({ type, - detail, + message, status, subproblems, - error, - message + error }: { type: AcmeErrorType; - detail: string; + message: string; status: number; subproblems?: Array<{ type: string; detail: string; identifier?: { type: string; value: string } }>; error?: unknown; - message?: string; }) { - super(message || detail); + super(message); this.type = type; - this.detail = detail; + this.message = message; this.status = status; this.subproblems = subproblems; this.error = error; @@ -78,7 +75,7 @@ export class AcmeError extends Error implements IAcmeError { toAcmeResponse(): IAcmeError { return { type: this.type, - detail: this.detail, + message: this.message, status: this.status, subproblems: this.subproblems }; @@ -90,20 +87,17 @@ export class AcmeError extends Error implements IAcmeError { */ export class AcmeMalformedError extends AcmeError { constructor({ - detail = "The request message was malformed", - error, - message + message = "The request message was malformed", + error }: { - detail?: string; - error?: unknown; message?: string; + error?: unknown; } = {}) { super({ type: AcmeErrorType.Malformed, - detail, + message, status: 400, - error, - message + error }); this.name = "AcmeMalformedError"; } @@ -114,20 +108,17 @@ export class AcmeMalformedError extends AcmeError { */ export class AcmeUnauthorizedError extends AcmeError { constructor({ - detail = "The client lacks sufficient authorization", - error, - message + message = "The client lacks sufficient authorization", + error }: { - detail?: string; - error?: unknown; message?: string; + error?: unknown; } = {}) { super({ type: AcmeErrorType.Unauthorized, - detail, + message, status: 403, - error, - message + error }); this.name = "AcmeUnauthorizedError"; } @@ -139,20 +130,17 @@ export class AcmeUnauthorizedError extends AcmeError { */ export class AcmeAccountDoesNotExistError extends AcmeError { constructor({ - detail = "The request specified an account that does not exist", - error, - message + message = "The request specified an account that does not exist", + error }: { - detail?: string; - error?: unknown; message?: string; + error?: unknown; } = {}) { super({ type: AcmeErrorType.AccountDoesNotExist, - detail, + message, status: 400, - error, - message + error }); this.name = "AcmeAccountDoesNotExistError"; } @@ -163,20 +151,17 @@ export class AcmeAccountDoesNotExistError extends AcmeError { */ export class AcmeBadNonceError extends AcmeError { constructor({ - detail = "The client sent an unacceptable anti-replay nonce", - error, - message + message = "The client sent an unacceptable anti-replay nonce", + error }: { - detail?: string; - error?: unknown; message?: string; + error?: unknown; } = {}) { super({ type: AcmeErrorType.BadNonce, - detail, + message, status: 400, - error, - message + error }); this.name = "AcmeBadNonceError"; } @@ -187,20 +172,17 @@ export class AcmeBadNonceError extends AcmeError { */ export class AcmeBadSignatureAlgorithmError extends AcmeError { constructor({ - detail = "The signature algorithm is invalid", - error, - message + message = "The signature algorithm is invalid", + error }: { - detail?: string; - error?: unknown; message?: string; + error?: unknown; } = {}) { super({ type: AcmeErrorType.BadSignatureAlgorithm, - detail, + message, status: 401, - error, - message + error }); this.name = "AcmeBadSignatureAlgorithmError"; } @@ -211,20 +193,17 @@ export class AcmeBadSignatureAlgorithmError extends AcmeError { */ export class AcmeBadPublicKeyError extends AcmeError { constructor({ - detail = "The public key is not acceptable", - error, - message + message = "The public key is not acceptable", + error }: { - detail?: string; - error?: unknown; message?: string; + error?: unknown; } = {}) { super({ type: AcmeErrorType.BadPublicKey, - detail, + message, status: 400, - error, - message + error }); this.name = "AcmeBadPublicKeyError"; } @@ -235,20 +214,17 @@ export class AcmeBadPublicKeyError extends AcmeError { */ export class AcmeBadCsrError extends AcmeError { constructor({ - detail = "The CSR is unacceptable", - error, - message + message = "The CSR is unacceptable", + error }: { - detail?: string; - error?: unknown; message?: string; + error?: unknown; } = {}) { super({ type: AcmeErrorType.BadCsr, - detail, + message, status: 400, - error, - message + error }); this.name = "AcmeBadCsrError"; } @@ -260,20 +236,17 @@ export class AcmeBadCsrError extends AcmeError { */ export class AcmeBadRevocationReasonError extends AcmeError { constructor({ - detail = "The revocation reason provided is not allowed", - error, - message + message = "The revocation reason provided is not allowed", + error }: { - detail?: string; - error?: unknown; message?: string; + error?: unknown; } = {}) { super({ type: AcmeErrorType.BadRevocationReason, - detail, + message, status: 400, - error, - message + error }); this.name = "AcmeBadRevocationReasonError"; } @@ -284,20 +257,17 @@ export class AcmeBadRevocationReasonError extends AcmeError { */ export class AcmeRateLimitedError extends AcmeError { constructor({ - detail = "The client has exceeded a rate limit", - error, - message + message = "The client has exceeded a rate limit", + error }: { - detail?: string; - error?: unknown; message?: string; + error?: unknown; } = {}) { super({ type: AcmeErrorType.RateLimited, - detail, + message, status: 429, - error, - message + error }); this.name = "AcmeRateLimitedError"; } @@ -309,23 +279,20 @@ export class AcmeRateLimitedError extends AcmeError { */ export class AcmeRejectedIdentifierError extends AcmeError { constructor({ - detail = "The server will not issue certificates for the identifier", + message = "The server will not issue certificates for the identifier", subproblems, - error, - message + error }: { - detail?: string; + message?: string; subproblems?: Array<{ type: string; detail: string; identifier?: { type: string; value: string } }>; error?: unknown; - message?: string; } = {}) { super({ type: AcmeErrorType.RejectedIdentifier, - detail, + message, status: 400, subproblems, - error, - message + error }); this.name = "AcmeRejectedIdentifierError"; } @@ -336,20 +303,17 @@ export class AcmeRejectedIdentifierError extends AcmeError { */ export class AcmeServerInternalError extends AcmeError { constructor({ - detail = "An internal error occurred", - error, - message + message = "An internal error occurred", + error }: { - detail?: string; - error?: unknown; message?: string; + error?: unknown; } = {}) { super({ type: AcmeErrorType.ServerInternal, - detail, + message, status: 500, - error, - message + error }); this.name = "AcmeServerInternalError"; } @@ -360,20 +324,17 @@ export class AcmeServerInternalError extends AcmeError { */ export class AcmeUnsupportedContactError extends AcmeError { constructor({ - detail = "A contact URL is of an unsupported type", - error, - message + message = "A contact URL is of an unsupported type", + error }: { - detail?: string; - error?: unknown; message?: string; + error?: unknown; } = {}) { super({ type: AcmeErrorType.UnsupportedContact, - detail, + message, status: 400, - error, - message + error }); this.name = "AcmeUnsupportedContactError"; } @@ -385,20 +346,17 @@ export class AcmeUnsupportedContactError extends AcmeError { */ export class AcmeUnsupportedIdentifierError extends AcmeError { constructor({ - detail = "An identifier is of an unsupported type", - error, - message + message = "An identifier is of an unsupported type", + error }: { - detail?: string; - error?: unknown; message?: string; + error?: unknown; } = {}) { super({ type: AcmeErrorType.UnsupportedIdentifier, - detail, + message, status: 400, - error, - message + error }); this.name = "AcmeUnsupportedIdentifierError"; } @@ -412,22 +370,19 @@ export class AcmeUserActionRequiredError extends AcmeError { instance?: string; constructor({ - detail = "Visit the instance URL and take actions specified there", + message = "Visit the instance URL and take actions specified there", instance, - error, - message + error }: { - detail?: string; + message?: string; instance?: string; error?: unknown; - message?: string; } = {}) { super({ type: AcmeErrorType.UserActionRequired, - detail, + message, status: 403, - error, - message + error }); this.instance = instance; this.name = "AcmeUserActionRequiredError"; @@ -446,20 +401,17 @@ export class AcmeUserActionRequiredError extends AcmeError { */ export class AcmeIncorrectResponseError extends AcmeError { constructor({ - detail = "The response is incorrect", - error, - message + message = "The response is incorrect", + error }: { - detail?: string; - error?: unknown; message?: string; + error?: unknown; } = {}) { super({ type: AcmeErrorType.IncorrectResponse, - detail, + message, status: 400, - error, - message + error }); this.name = "AcmeIncorrectResponseError"; } @@ -470,20 +422,17 @@ export class AcmeIncorrectResponseError extends AcmeError { */ export class AcmeConnectionError extends AcmeError { constructor({ - detail = "A connection error occurred", - error, - message + message = "A connection error occurred", + error }: { - detail?: string; - error?: unknown; message?: string; + error?: unknown; } = {}) { super({ type: AcmeErrorType.Connection, - detail, + message, status: 400, - error, - message + error }); this.name = "AcmeConnectionError"; } @@ -491,20 +440,17 @@ export class AcmeConnectionError extends AcmeError { export class AcmeDnsFailureError extends AcmeError { constructor({ - detail = "Hostname could not be resolved (DNS failure)", - error, - message + message = "Hostname could not be resolved (DNS failure)", + error }: { - detail?: string; - error?: unknown; message?: string; + error?: unknown; } = {}) { super({ type: AcmeErrorType.DNS, - detail, + message, status: 400, - error, - message + error }); this.name = "AcmeDnsFailureError"; } @@ -512,20 +458,17 @@ export class AcmeDnsFailureError extends AcmeError { export class AcmeOrderNotReadyError extends AcmeError { constructor({ - detail = "The order is not ready", - error, - message + message = "The order is not ready", + error }: { - detail?: string; - error?: unknown; message?: string; + error?: unknown; } = {}) { super({ type: AcmeErrorType.OrderNotReady, - detail, + message, status: 403, - error, - message + error }); this.name = "AcmeOrderNotReadyError"; } @@ -533,20 +476,17 @@ export class AcmeOrderNotReadyError extends AcmeError { export class AcmeBadCSRError extends AcmeError { constructor({ - detail = "The CSR is unacceptable", - error, - message + message = "The CSR is unacceptable", + error }: { - detail?: string; - error?: unknown; message?: string; + error?: unknown; } = {}) { super({ type: AcmeErrorType.BadCsr, - detail, + message, status: 400, - error, - message + error }); this.name = "AcmeBadCSRError"; } @@ -554,20 +494,17 @@ export class AcmeBadCSRError extends AcmeError { export class AcmeExternalAccountRequiredError extends AcmeError { constructor({ - detail = "External account binding is required", - error, - message + message = "External account binding is required", + error }: { - detail?: string; - error?: unknown; message?: string; + error?: unknown; } = {}) { super({ type: AcmeErrorType.ExternalAccountRequired, - detail, + message, status: 400, - error, - message + error }); this.name = "AcmeExternalAccountRequiredError"; } diff --git a/backend/src/ee/services/pki-acme/pki-acme-fns.ts b/backend/src/ee/services/pki-acme/pki-acme-fns.ts index 828e0801b..08659c853 100644 --- a/backend/src/ee/services/pki-acme/pki-acme-fns.ts +++ b/backend/src/ee/services/pki-acme/pki-acme-fns.ts @@ -13,7 +13,7 @@ export const buildUrl = (profileId: string, path: string): string => { export const extractAccountIdFromKid = (kid: string, profileId: string): string => { const kidPrefix = buildUrl(profileId, "/accounts/"); if (!kid.startsWith(kidPrefix)) { - throw new AcmeMalformedError({ detail: "KID must start with the profile account URL" }); + throw new AcmeMalformedError({ message: "KID must start with the profile account URL" }); } return z.string().uuid().parse(kid.slice(kidPrefix.length)); }; 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 09faaf8bc..d928990a6 100644 --- a/backend/src/ee/services/pki-acme/pki-acme-service.ts +++ b/backend/src/ee/services/pki-acme/pki-acme-service.ts @@ -148,7 +148,7 @@ export const pkiAcmeServiceFactory = ({ try { result = await flattenedVerify(rawJwsPayload, async (protectedHeader: JWSHeaderParameters | undefined) => { if (protectedHeader === undefined) { - throw new AcmeMalformedError({ detail: "Protected header is required" }); + throw new AcmeMalformedError({ message: "Protected header is required" }); } const jwk = await getJWK(protectedHeader); const key = await importJWK(jwk, protectedHeader.alg); @@ -159,28 +159,28 @@ export const pkiAcmeServiceFactory = ({ throw error; } if (error instanceof ZodError) { - throw new AcmeMalformedError({ detail: `Invalid JWS payload: ${error.message}` }); + throw new AcmeMalformedError({ message: `Invalid JWS payload: ${error.message}` }); } if (error instanceof errors.JWSSignatureVerificationFailed) { - throw new AcmeBadPublicKeyError({ detail: "Invalid JWS payload" }); + throw new AcmeBadPublicKeyError({ message: "Invalid JWS payload" }); } logger.error(error, "Unexpected error while verifying JWS payload"); - throw new AcmeServerInternalError({ detail: "Failed to verify JWS payload" }); + throw new AcmeServerInternalError({ message: "Failed to verify JWS payload" }); } const { protectedHeader: rawProtectedHeader, payload: rawPayload } = result; try { const protectedHeader = ProtectedHeaderSchema.parse(rawProtectedHeader); // Validate the URL if (new URL(protectedHeader.url).href !== url.href) { - throw new AcmeUnauthorizedError({ detail: "URL mismatch in the protected header" }); + throw new AcmeUnauthorizedError({ message: "URL mismatch in the protected header" }); } // Consume the nonce if (!protectedHeader.nonce) { - throw new AcmeMalformedError({ detail: "Nonce is required in the protected header" }); + throw new AcmeMalformedError({ message: "Nonce is required in the protected header" }); } const deleted = await keyStore.deleteItem(KeyStorePrefixes.PkiAcmeNonce(protectedHeader.nonce)); if (deleted !== 1) { - throw new AcmeBadNonceError({ detail: "Invalid nonce" }); + throw new AcmeBadNonceError({ message: "Invalid nonce" }); } // Parse the payload @@ -196,10 +196,10 @@ export const pkiAcmeServiceFactory = ({ throw error; } if (error instanceof ZodError) { - throw new AcmeMalformedError({ detail: `Invalid JWS payload: ${error.message}` }); + throw new AcmeMalformedError({ message: `Invalid JWS payload: ${error.message}` }); } logger.error(error, "Unexpected error while parsing JWS payload"); - throw new AcmeMalformedError({ detail: "Failed to verify JWS payload" }); + throw new AcmeMalformedError({ message: "Failed to verify JWS payload" }); } }; @@ -215,7 +215,7 @@ export const pkiAcmeServiceFactory = ({ rawJwsPayload, getJWK: async (protectedHeader) => { if (!protectedHeader.jwk) { - throw new AcmeMalformedError({ detail: "JWK is required in the protected header" }); + throw new AcmeMalformedError({ message: "JWK is required in the protected header" }); } return protectedHeader.jwk as unknown as JsonWebKey; }, @@ -246,7 +246,7 @@ export const pkiAcmeServiceFactory = ({ rawJwsPayload, getJWK: async (protectedHeader) => { if (!protectedHeader.kid) { - throw new AcmeMalformedError({ detail: "KID is required in the protected header" }); + throw new AcmeMalformedError({ message: "KID is required in the protected header" }); } const accountId = extractAccountIdFromKid(protectedHeader.kid, profileId); if (expectedAccountId && accountId !== expectedAccountId) { @@ -257,7 +257,7 @@ export const pkiAcmeServiceFactory = ({ throw new AcmeAccountDoesNotExistError({ message: "ACME account not found" }); } if (account.alg !== protectedHeader.alg) { - throw new AcmeMalformedError({ detail: "ACME account algorithm mismatch" }); + throw new AcmeMalformedError({ message: "ACME account algorithm mismatch" }); } return account.publicKey as JsonWebKey; }, @@ -344,7 +344,7 @@ export const pkiAcmeServiceFactory = ({ }): Promise> => { const profile = await validateAcmeProfile(profileId); if (!externalAccountBinding) { - throw new AcmeExternalAccountRequiredError({ detail: "External account binding is required" }); + throw new AcmeExternalAccountRequiredError({ message: "External account binding is required" }); } const publicKeyThumbprint = await calculateJwkThumbprint(jwk, "sha256"); @@ -363,26 +363,28 @@ export const pkiAcmeServiceFactory = ({ return { eabPayload: result.payload, eabProtectedHeader: result.protectedHeader }; } catch (error) { if (error instanceof errors.JWSSignatureVerificationFailed) { - throw new AcmeMalformedError({ detail: "Invalid external account binding JWS signature" }); + throw new AcmeExternalAccountRequiredError({ message: "Invalid external account binding JWS signature" }); } logger.error(error, "Unexpected error while verifying EAB JWS signature"); - throw new AcmeServerInternalError({ detail: "Failed to verify EAB JWS signature" }); + throw new AcmeServerInternalError({ message: "Failed to verify EAB JWS signature" }); } })(); const { alg: eabAlg, kid: eabKid } = eabProtectedHeader!; if (!["HS256", "HS384", "HS512"].includes(eabAlg!)) { - throw new AcmeMalformedError({ detail: "Invalid algorithm for external account binding JWS payload" }); + throw new AcmeExternalAccountRequiredError({ + message: "Invalid algorithm for external account binding JWS payload" + }); } // Make sure the KID in the EAB payload matches the profile ID if (eabKid !== profile.id) { - throw new UnauthorizedError({ message: "External account binding KID mismatch" }); + throw new AcmeExternalAccountRequiredError({ message: "External account binding KID mismatch" }); } // Make sure the URL matches the expected URL const url = eabProtectedHeader!.url!; if (url !== buildUrl(profile.id, "/new-account")) { - throw new UnauthorizedError({ message: "External account binding URL mismatch" }); + throw new AcmeExternalAccountRequiredError({ message: "External account binding URL mismatch" }); } // Make sure the JWK in the EAB payload matches the one provided in the outer JWS payload @@ -497,10 +499,10 @@ export const pkiAcmeServiceFactory = ({ const authorizations: TPkiAcmeAuths[] = await Promise.all( payload.identifiers.map(async (identifier) => { if (identifier.type !== AcmeIdentifierType.DNS) { - throw new AcmeUnsupportedIdentifierError({ detail: "Only DNS identifiers are supported" }); + throw new AcmeUnsupportedIdentifierError({ message: "Only DNS identifiers are supported" }); } if (isPrivateIp(identifier.value)) { - throw new AcmeUnsupportedIdentifierError({ detail: "Private IP addresses are not allowed" }); + throw new AcmeUnsupportedIdentifierError({ message: "Private IP addresses are not allowed" }); } const auth = await acmeAuthDAL.create( { @@ -647,9 +649,9 @@ export const pkiAcmeServiceFactory = ({ logger.error(exp, "Failed to sign certificate"); // TODO: audit log the error if (exp instanceof BadRequestError) { - errorToReturn = new AcmeBadCSRError({ detail: `Invalid CSR: ${exp.message}` }); + errorToReturn = new AcmeBadCSRError({ message: `Invalid CSR: ${exp.message}` }); } else { - errorToReturn = new AcmeServerInternalError({ detail: "Failed to sign certificate with internal error" }); + errorToReturn = new AcmeServerInternalError({ message: "Failed to sign certificate with internal error" }); } } return { diff --git a/backend/src/server/plugins/error-handler.ts b/backend/src/server/plugins/error-handler.ts index 4b29f6930..e703df5ef 100644 --- a/backend/src/server/plugins/error-handler.ts +++ b/backend/src/server/plugins/error-handler.ts @@ -252,8 +252,7 @@ export const fastifyErrHandler = fastifyPlugin(async (server: FastifyZodProvider error: error.name, status: error.status, type: `urn:ietf:params:acme:error:${error.type}`, - detail: error.detail, - message: error.message + detail: error.message // TODO: add subproblems if they exist }); } else {