awatertrevi/infisical
1
0
Fork 0
mirror of https://github.com/awatertrevi/infisical.git synced 2026-09-22 13:39:35 +00:00
Code Issues Packages Projects Releases Wiki Activity

Detail and message are mostly the same for acme error, use just one to avoid conufsing

Browse Source
This commit is contained in:
Fang-Pen Lin
2025-11-06 19:03:10 -08:00
parent 159eb3138d
commit f11c4084ac
6 changed files with 144 additions and 206 deletions
Download Patch File Download Diff File Expand all files Collapse all files

16
backend/bdd/features/pki/acme/account.feature
View File

@@ -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 "<eab_kid>" with secret "<eab_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 "<error_type>"
Then the value error with jq ".detail" should be equal to "<error_msg>"
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 |

4
backend/bdd/features/steps/pki_acme.py
View File

@@ -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")

279
backend/src/ee/services/pki-acme/pki-acme-errors.ts
View File

@@ -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";
}

2
backend/src/ee/services/pki-acme/pki-acme-fns.ts
View File

@@ -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));
};

46
backend/src/ee/services/pki-acme/pki-acme-service.ts
View File

@@ -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<TAcmeResponse<TCreateAcmeAccountResponse>> => {
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 {

3
backend/src/server/plugins/error-handler.ts
View File

@@ -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 {