diff --git a/backend/bdd/features/pki/acme/internal-ca.feature b/backend/bdd/features/pki/acme/internal-ca.feature new file mode 100644 index 000000000..9db4328d2 --- /dev/null +++ b/backend/bdd/features/pki/acme/internal-ca.feature @@ -0,0 +1,33 @@ +Feature: Internal CA + + Scenario: CSR with SANs only + 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 "{acme_profile.eab_kid}" with secret "{acme_profile.eab_secret}" as acme_account + When I create certificate signing request as csr + Then I add names to certificate signing request csr + """ + {} + """ + And I add subject alternative name to certificate signing request csr + """ + [ + "localhost" + ] + """ + And I create a RSA private key pair as cert_key + And I sign the certificate signing request csr with private key cert_key and output it as csr_pem in PEM format + And I submit the certificate signing request PEM csr_pem certificate order to the ACME server as order + And I select challenge with type http-01 for domain localhost from order in order as challenge + And I serve challenge response for challenge at localhost + And I tell ACME server that challenge is ready to be verified + And I poll and finalize the ACME order order as finalized_order + And the value finalized_order.body with jq ".status" should be equal to "valid" + And I parse the full-chain certificate from order finalized_order as cert + And the value cert with jq ".subject.common_name" should be equal to null + And the value cert with jq "[.extensions.subjectAltName.general_names.[].value] | sort" should be equal to json + """ + [ + "localhost" + ] + """ \ No newline at end of file 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 7a3747fed..e084c8f0a 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 @@ -1,3 +1,5 @@ +import axios, { AxiosError } from "axios"; + import { getConfig } from "@app/lib/config/env"; import { BadRequestError, NotFoundError } from "@app/lib/errors"; import { isPrivateIp } from "@app/lib/ip/ipRange"; @@ -13,10 +15,6 @@ import { import { AcmeAuthStatus, AcmeChallengeStatus, AcmeChallengeType } from "./pki-acme-schemas"; import { TPkiAcmeChallengeServiceFactory } from "./pki-acme-types"; -type FetchError = Error & { - code?: string; -}; - type TPkiAcmeChallengeServiceFactoryDep = { acmeChallengeDAL: Pick< TPkiAcmeChallengeDALFactory, @@ -74,18 +72,20 @@ export const pkiAcmeChallengeServiceFactory = ({ // 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 // challenge validation at the same time, it should be fine. - const challengeResponse = await fetch(challengeUrl, { + const challengeResponse = await axios.get(challengeUrl.toString(), { // In case if we override the host in the development mode, still provide the original host in the header // to help the upstream server to validate the request - headers: { Host: host }, - signal: AbortSignal.timeout(timeoutMs) + headers: { Host: challenge.auth.identifierValue }, + timeout: timeoutMs, + responseType: "text", + validateStatus: () => true }); if (challengeResponse.status !== 200) { throw new AcmeIncorrectResponseError({ message: `ACME challenge response is not 200: ${challengeResponse.status}` }); } - const challengeResponseBody = await challengeResponse.text(); + const challengeResponseBody: string = challengeResponse.data; const thumbprint = challenge.auth.account.publicKeyThumbprint; const expectedChallengeResponseBody = `${challenge.auth.token}.${thumbprint}`; if (challengeResponseBody.trimEnd() !== expectedChallengeResponseBody) { @@ -96,35 +96,25 @@ export const pkiAcmeChallengeServiceFactory = ({ // TODO: we should retry the challenge validation a few times, but let's keep it simple for now await acmeChallengeDAL.markAsInvalidCascadeById(challengeId, tx); // Properly type and inspect the error - if (exp instanceof TypeError && exp.message.includes("fetch failed")) { - const { cause } = exp; - let errors: Error[] = []; - if (cause instanceof AggregateError) { - errors = cause.errors as Error[]; - } else if (cause instanceof Error) { - errors = [cause]; + if (axios.isAxiosError(exp)) { + const axiosError = exp as AxiosError; + const errorCode = axiosError.code; + const errorMessage = axiosError.message; + + if (errorCode === "ECONNREFUSED" || errorMessage.includes("ECONNREFUSED")) { + return new AcmeConnectionError({ message: "Connection refused" }); } - // eslint-disable-next-line no-unreachable-loop - for (const err of errors) { - // TODO: handle multiple errors, return a compound error instead of just the first error - const fetchError = err as FetchError; - if (fetchError.code === "ECONNREFUSED" || fetchError.message.includes("ECONNREFUSED")) { - return new AcmeConnectionError({ message: "Connection refused" }); - } - if (fetchError.code === "ENOTFOUND" || fetchError.message.includes("ENOTFOUND")) { - return new AcmeDnsFailureError({ message: "Hostname could not be resolved (DNS failure)" }); - } - logger.error(exp, "Unknown error validating ACME challenge response"); - return new AcmeServerInternalError({ message: "Unknown error validating ACME challenge response" }); + if (errorCode === "ENOTFOUND" || errorMessage.includes("ENOTFOUND")) { + return new AcmeDnsFailureError({ message: "Hostname could not be resolved (DNS failure)" }); } - } else if (exp instanceof DOMException) { - if (exp.name === "TimeoutError") { + if (errorCode === "ECONNABORTED" || errorMessage.includes("timeout")) { logger.error(exp, "Connection timed out while validating ACME challenge response"); return new AcmeConnectionError({ message: "Connection timed out" }); } logger.error(exp, "Unknown error validating ACME challenge response"); return new AcmeServerInternalError({ message: "Unknown error validating ACME challenge response" }); - } else if (exp instanceof Error) { + } + if (exp instanceof Error) { logger.error(exp, "Error validating ACME challenge response"); } else { logger.error(exp, "Unknown error validating ACME challenge response"); 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 4f560ade7..69a6a648f 100644 --- a/backend/src/ee/services/pki-acme/pki-acme-service.ts +++ b/backend/src/ee/services/pki-acme/pki-acme-service.ts @@ -703,9 +703,6 @@ export const pkiAcmeServiceFactory = ({ // Check and validate the CSR const certificateRequest = extractCertificateRequestFromCSR(csr); - if (!certificateRequest.commonName) { - throw new AcmeBadCSRError({ message: "Invalid CSR: Common name is required" }); - } if ( certificateRequest.subjectAlternativeNames?.some( (san) => san.type !== CertSubjectAlternativeNameType.DNS_NAME @@ -721,7 +718,7 @@ export const pkiAcmeServiceFactory = ({ const csrIdentifierValues = new Set( (certificateRequest.subjectAlternativeNames ?? []) .map((san) => san.value.toLowerCase()) - .concat([certificateRequest.commonName.toLowerCase()]) + .concat(certificateRequest.commonName ? [certificateRequest.commonName.toLowerCase()] : []) ); if ( csrIdentifierValues.size !== orderWithAuthorizations.authorizations.length || diff --git a/backend/src/services/certificate-authority/internal/internal-certificate-authority-service.ts b/backend/src/services/certificate-authority/internal/internal-certificate-authority-service.ts index a7292e366..5e42dc383 100644 --- a/backend/src/services/certificate-authority/internal/internal-certificate-authority-service.ts +++ b/backend/src/services/certificate-authority/internal/internal-certificate-authority-service.ts @@ -1716,12 +1716,7 @@ export const internalCertificateAuthorityServiceFactory = ({ const csrObj = new x509.Pkcs10CertificateRequest(csr); const dn = parseDistinguishedName(csrObj.subject); - const cn = commonName || dn.commonName; - - if (!cn) - throw new BadRequestError({ - message: "A common name (CN) is required in the CSR or as a parameter to this endpoint" - }); + const cn = (commonName || dn.commonName) ?? ""; const { caPrivateKey, caSecret } = await getCaCredentials({ caId: ca.id,