github/infisical
1
0
Fork 0
mirror of https://github.com/Infisical/infisical.git synced 2026-01-09 23:48:05 -05:00
Code Issues Packages Projects Releases Wiki Activity

Only check EAB when creating a new account

Browse Source
This commit is contained in:
Fang-Pen Lin
2025-11-18 09:28:10 -08:00
parent d190fb15c9
commit a668822e19
2 changed files with 38 additions and 34 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@@ -15,15 +15,6 @@ Feature: Account
And the value retrieved_account.uri should be equal to "{account_uri}" And the value retrieved_account.uri should be equal to "{account_uri}"
# Note: This is a very special case for cert-manager. # Note: This is a very special case for cert-manager.
# There's a bug in their ACME client implementation, they don't take the account KID value they have
# and relying on a '{"onlyReturnExisting": true}' new-account request to find out their KID value.
# But the problem is, that new-account request doesn't come with EAB. And while the get existing account operation
# fails, they just discard the error and proceed to request a new order. Since no KID provided, their ACME
# client will send JWK instead. As a result, we are seeing KID not provide in header error for the new-order
# endpoint.
#
# To solve the problem, we lose the check for EAB a bit for the onlyReturnExisting new account request
# ref: https://github.com/cert-manager/cert-manager/issues/7388#issuecomment-3535630925
Scenario: Create a new account with EAB then retrieve it without EAB Scenario: Create a new account with EAB then retrieve it without EAB
Given I have an ACME cert profile as "acme_profile" 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" When I have an ACME client connecting to "{BASE_URL}/api/v1/pki/acme/profiles/{acme_profile.id}/directory"

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

@@ -389,11 +389,48 @@ export const pkiAcmeServiceFactory = ({
payload: TCreateAcmeAccountPayload; payload: TCreateAcmeAccountPayload;
}): Promise<TAcmeResponse<TCreateAcmeAccountResponse>> => { }): Promise<TAcmeResponse<TCreateAcmeAccountResponse>> => {
const profile = await validateAcmeProfile(profileId); const profile = await validateAcmeProfile(profileId);
const publicKeyThumbprint = await calculateJwkThumbprint(jwk, "sha256");
if (onlyReturnExisting) {
const existingAccount: TPkiAcmeAccounts | null = await acmeAccountDAL.findByProfileIdAndPublicKeyThumbprintAndAlg(
profileId,
alg,
publicKeyThumbprint
);
if (!existingAccount) {
throw new AcmeAccountDoesNotExistError({ message: "ACME account not found" });
}
// With the same public key, we found an existing account, just return it
return {
status: 200,
body: {
status: "valid",
contact: existingAccount.emails,
orders: buildUrl(profile.id, `/accounts/${existingAccount.id}/orders`)
},
headers: {
Location: buildUrl(profile.id, `/accounts/${existingAccount.id}`),
Link: `<${buildUrl(profile.id, "/directory")}>;rel="index"`
}
};
}
// Note: We only check EAB for the new account request. This is a very special case for cert-manager.
// There's a bug in their ACME client implementation, they don't take the account KID value they have
// and relying on a '{"onlyReturnExisting": true}' new-account request to find out their KID value.
// But the problem is, that new-account request doesn't come with EAB. And while the get existing account operation
// fails, they just discard the error and proceed to request a new order. Since no KID provided, their ACME
// client will send JWK instead. As a result, we are seeing KID not provide in header error for the new-order
// endpoint.
//
// To solve the problem, we lose the check for EAB a bit for the onlyReturnExisting new account request.
// It should be fine as we've already checked EAB when they created the account.
// And the private key ownership indicating they are the same user.
// ref: https://github.com/cert-manager/cert-manager/issues/7388#issuecomment-3535630925
if (!externalAccountBinding) { if (!externalAccountBinding) {
throw new AcmeExternalAccountRequiredError({ message: "External account binding is required" }); throw new AcmeExternalAccountRequiredError({ message: "External account binding is required" });
} }
const publicKeyThumbprint = await calculateJwkThumbprint(jwk, "sha256");
const certificateManagerKmsId = await getProjectKmsCertificateKeyId({ const certificateManagerKmsId = await getProjectKmsCertificateKeyId({
projectId: profile.projectId, projectId: profile.projectId,
projectDAL, projectDAL,
@@ -444,30 +481,6 @@ export const pkiAcmeServiceFactory = ({
}); });
} }
const existingAccount: TPkiAcmeAccounts | null = await acmeAccountDAL.findByProfileIdAndPublicKeyThumbprintAndAlg(
profileId,
alg,
publicKeyThumbprint
);
if (onlyReturnExisting && !existingAccount) {
throw new AcmeAccountDoesNotExistError({ message: "ACME account not found" });
}
if (existingAccount) {
// With the same public key, we found an existing account, just return it
return {
status: 200,
body: {
status: "valid",
contact: existingAccount.emails,
orders: buildUrl(profile.id, `/accounts/${existingAccount.id}/orders`)
},
headers: {
Location: buildUrl(profile.id, `/accounts/${existingAccount.id}`),
Link: `<${buildUrl(profile.id, "/directory")}>;rel="index"`
}
};
}
const newAccount = await acmeAccountDAL.create({ const newAccount = await acmeAccountDAL.create({
profileId: profile.id, profileId: profile.id,
alg, alg,