From 904ee3264fb9db2eaaeac450eb6067e2e89495ed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Niccol=C3=B2=20Maltoni?= Date: Wed, 22 Jul 2026 12:18:38 +0200 Subject: [PATCH 01/10] build(deps): bump acme4j to 3.5.0 Replace deprecated update() with fetch() on challenge and order polling, as recommended by the acme4j migration guide before upgrading to 4.x. --- .../org/carapaceproxy/server/certificates/ACMEClient.java | 4 ++-- pom.xml | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/carapace-server/src/main/java/org/carapaceproxy/server/certificates/ACMEClient.java b/carapace-server/src/main/java/org/carapaceproxy/server/certificates/ACMEClient.java index 5cebe3e9c..4e6f6cda6 100644 --- a/carapace-server/src/main/java/org/carapaceproxy/server/certificates/ACMEClient.java +++ b/carapace-server/src/main/java/org/carapaceproxy/server/certificates/ACMEClient.java @@ -170,7 +170,7 @@ public Status checkResponseForChallenge(Challenge challenge) throws AcmeExceptio // The authorization is already valid. No need to process a challenge. // or the challenge is already verified, there's no need to execute it again. } else if (status != Status.INVALID) { - challenge.update(); + challenge.fetch(); } return status; } @@ -200,7 +200,7 @@ public Status checkResponseForOrder(Order order) throws AcmeException { if (status == Status.VALID) { LOG.info("Order has been completed."); } else if (status != Status.INVALID) { - order.update(); + order.fetch(); } return status; } diff --git a/pom.xml b/pom.xml index 3c6990b48..fb5104214 100644 --- a/pom.xml +++ b/pom.xml @@ -96,7 +96,7 @@ 2024.0.6 4.1.121.Final - 3.4.0 + 3.5.0 1.84 2.17.113 3.0.5 From 7a27f74f68f2a985d30e8115545e0f2d9ddc4024 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Niccol=C3=B2=20Maltoni?= Date: Wed, 22 Jul 2026 12:22:55 +0200 Subject: [PATCH 02/10] build(deps): bump acme4j to 4.0.0 No code changes needed: deprecated APIs were already dropped in the 3.5.0 step, and the new Java 17 baseline is below our Java 21 build. --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index fb5104214..c801003f2 100644 --- a/pom.xml +++ b/pom.xml @@ -96,7 +96,7 @@ 2024.0.6 4.1.121.Final - 3.5.0 + 4.0.0 1.84 2.17.113 3.0.5 From 4996478c23d5007bc2d4d4896ef494c0ce475862 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Niccol=C3=B2=20Maltoni?= Date: Wed, 22 Jul 2026 12:31:40 +0200 Subject: [PATCH 03/10] build(deps): bump acme4j to 5.0.0 Login.getKeyPair() was removed as a security precaution: private key handling now stays inside acme4j, and only Login.getPublicKey() is exposed. Tests now stub getPublicKey(), which the challenge digest computation relies on. --- .../server/certificates/DynamicCertificatesManagerTest.java | 6 +++--- pom.xml | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/carapace-server/src/test/java/org/carapaceproxy/server/certificates/DynamicCertificatesManagerTest.java b/carapace-server/src/test/java/org/carapaceproxy/server/certificates/DynamicCertificatesManagerTest.java index b30480dca..198712875 100644 --- a/carapace-server/src/test/java/org/carapaceproxy/server/certificates/DynamicCertificatesManagerTest.java +++ b/carapace-server/src/test/java/org/carapaceproxy/server/certificates/DynamicCertificatesManagerTest.java @@ -338,7 +338,7 @@ public void testWildcardCertificateStateManagement(String runCase) throws Except )); when(session.connect()).thenReturn(conn); when(login.getSession()).thenReturn(session); - when(login.getKeyPair()).thenReturn(KeyPairUtils.createKeyPair(DEFAULT_KEYPAIRS_SIZE)); + when(login.getPublicKey()).thenReturn(KeyPairUtils.createKeyPair(DEFAULT_KEYPAIRS_SIZE).getPublic()); Dns01Challenge c = mock(Dns01Challenge.class); when(c.getDigest()).thenReturn(""); @@ -465,7 +465,7 @@ public void testSanCertificateStateManagement(String runCase) throws Exception { )); when(session.connect()).thenReturn(conn); when(login.getSession()).thenReturn(session); - when(login.getKeyPair()).thenReturn(KeyPairUtils.createKeyPair(DEFAULT_KEYPAIRS_SIZE)); + when(login.getPublicKey()).thenReturn(KeyPairUtils.createKeyPair(DEFAULT_KEYPAIRS_SIZE).getPublic()); // challenges when(ac.checkResponseForChallenge(any())).thenReturn(runCase.equals("challenge_failed")? INVALID : VALID); @@ -594,7 +594,7 @@ public void testDomainReachabilityCheck(String domainCase) throws Exception { Session session = mock(Session.class); when(session.connect()).thenReturn(mock(Connection.class)); when(login.getSession()).thenReturn(session); - when(login.getKeyPair()).thenReturn(KeyPairUtils.createKeyPair(DEFAULT_KEYPAIRS_SIZE)); + when(login.getPublicKey()).thenReturn(KeyPairUtils.createKeyPair(DEFAULT_KEYPAIRS_SIZE).getPublic()); Http01Challenge c = mock(Http01Challenge.class); when(c.getToken()).thenReturn(""); diff --git a/pom.xml b/pom.xml index c801003f2..6a67415a6 100644 --- a/pom.xml +++ b/pom.xml @@ -96,7 +96,7 @@ 2024.0.6 4.1.121.Final - 4.0.0 + 5.0.0 1.84 2.17.113 3.0.5 From feefcb4695788272be665e63199db7358dc0a9f4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Niccol=C3=B2=20Maltoni?= Date: Wed, 22 Jul 2026 12:35:25 +0200 Subject: [PATCH 04/10] build(deps): bump acme4j to 5.1.0 --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index 6a67415a6..ccfbb1c6e 100644 --- a/pom.xml +++ b/pom.xml @@ -96,7 +96,7 @@ 2024.0.6 4.1.121.Final - 5.0.0 + 5.1.0 1.84 2.17.113 3.0.5 From fef7ea0ae046013e4b7d42ab1a0602cf1603211b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Niccol=C3=B2=20Maltoni?= Date: Wed, 22 Jul 2026 12:40:37 +0200 Subject: [PATCH 05/10] build(deps): bump slf4j to 2.0.18 and logback to 1.5.38 Align slf4j with the version acme4j 5.1.0 builds against. --- pom.xml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pom.xml b/pom.xml index ccfbb1c6e..85f3168be 100644 --- a/pom.xml +++ b/pom.xml @@ -119,8 +119,8 @@ 2.35 2.14.3 3.27.0-GA - 2.0.16 - 1.5.35 + 2.0.18 + 1.5.38 33.3.1-jre 1.18.32 4.5.3 From 9bb8f1c8a5242723b97a33446ab3f981536f4361 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Niccol=C3=B2=20Maltoni?= Date: Wed, 22 Jul 2026 12:45:30 +0200 Subject: [PATCH 06/10] refactor: use typed findChallenge lookups in ACMEClient acme4j's Authorization.findChallenge(Class) makes the manual casts unnecessary. --- .../org/carapaceproxy/server/certificates/ACMEClient.java | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/carapace-server/src/main/java/org/carapaceproxy/server/certificates/ACMEClient.java b/carapace-server/src/main/java/org/carapaceproxy/server/certificates/ACMEClient.java index 4e6f6cda6..4259f7a39 100644 --- a/carapace-server/src/main/java/org/carapaceproxy/server/certificates/ACMEClient.java +++ b/carapace-server/src/main/java/org/carapaceproxy/server/certificates/ACMEClient.java @@ -136,8 +136,7 @@ public Map getChallengesForOrder(Order order) throws AcmeExce * @return {@link Http01Challenge} to verify */ private Http01Challenge httpChallenge(Authorization auth) throws AcmeException { - Http01Challenge challenge = auth.findChallenge(Http01Challenge.TYPE) - .map(Http01Challenge.class::cast) + Http01Challenge challenge = auth.findChallenge(Http01Challenge.class) .orElseThrow(() -> new AcmeException("Found no " + Http01Challenge.TYPE + " challenge, don't know what to do...")); LOG.debug("It must be reachable at: http://{}/.well-known/acme-challenge/{}", auth.getIdentifier().getDomain(), challenge.getToken() @@ -157,8 +156,7 @@ private Http01Challenge httpChallenge(Authorization auth) throws AcmeException { */ public Challenge dnsChallenge(Authorization auth) throws AcmeException { Dns01Challenge challenge = auth - .findChallenge(Dns01Challenge.TYPE) - .map(Dns01Challenge.class::cast) + .findChallenge(Dns01Challenge.class) .orElseThrow(() -> new AcmeException("Found no " + Dns01Challenge.TYPE + " challenge, don't know what to do...")); LOG.info("DNS-challenge _acme-challenge.{}. to save as TXT-record with content {}", auth.getIdentifier().getDomain(), challenge.getDigest()); return challenge; From b0ac464761647feec2bd22a15eb5d7dc99b7e4ea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Niccol=C3=B2=20Maltoni?= Date: Wed, 22 Jul 2026 16:53:31 +0200 Subject: [PATCH 07/10] fix: return refreshed status after fetch in ACME polling checkResponseForChallenge and checkResponseForOrder returned the status read before fetch(), so callers always saw the stale value and the certificate state machine converged one cycle late. Return the post-fetch status instead, and cover the polling logic with direct ACMEClient tests. --- .../server/certificates/ACMEClient.java | 12 ++- .../server/certificates/ACMEClientTest.java | 99 +++++++++++++++++++ 2 files changed, 107 insertions(+), 4 deletions(-) create mode 100644 carapace-server/src/test/java/org/carapaceproxy/server/certificates/ACMEClientTest.java diff --git a/carapace-server/src/main/java/org/carapaceproxy/server/certificates/ACMEClient.java b/carapace-server/src/main/java/org/carapaceproxy/server/certificates/ACMEClient.java index 4259f7a39..a609b781e 100644 --- a/carapace-server/src/main/java/org/carapaceproxy/server/certificates/ACMEClient.java +++ b/carapace-server/src/main/java/org/carapaceproxy/server/certificates/ACMEClient.java @@ -167,10 +167,12 @@ public Status checkResponseForChallenge(Challenge challenge) throws AcmeExceptio if (status == Status.VALID) { // The authorization is already valid. No need to process a challenge. // or the challenge is already verified, there's no need to execute it again. - } else if (status != Status.INVALID) { + return status; + } + if (status != Status.INVALID) { challenge.fetch(); } - return status; + return challenge.getStatus(); } /** @@ -197,10 +199,12 @@ public Status checkResponseForOrder(Order order) throws AcmeException { Status status = order.getStatus(); if (status == Status.VALID) { LOG.info("Order has been completed."); - } else if (status != Status.INVALID) { + return status; + } + if (status != Status.INVALID) { order.fetch(); } - return status; + return order.getStatus(); } public Certificate fetchCertificateForOrder(Order order) throws AcmeException { diff --git a/carapace-server/src/test/java/org/carapaceproxy/server/certificates/ACMEClientTest.java b/carapace-server/src/test/java/org/carapaceproxy/server/certificates/ACMEClientTest.java new file mode 100644 index 000000000..b16b98dd3 --- /dev/null +++ b/carapace-server/src/test/java/org/carapaceproxy/server/certificates/ACMEClientTest.java @@ -0,0 +1,99 @@ +/* + * Licensed to Diennea S.r.l. under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. Diennea S.r.l. licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + * + */ +package org.carapaceproxy.server.certificates; + +import static org.carapaceproxy.server.certificates.DynamicCertificatesManager.DEFAULT_KEYPAIRS_SIZE; +import static org.junit.Assert.assertEquals; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; +import java.util.List; +import org.junit.Test; +import org.shredzone.acme4j.Identifier; +import org.shredzone.acme4j.Order; +import org.shredzone.acme4j.Status; +import org.shredzone.acme4j.challenge.Challenge; +import org.shredzone.acme4j.util.KeyPairUtils; + +public class ACMEClientTest { + + private final ACMEClient client = new ACMEClient(KeyPairUtils.createKeyPair(DEFAULT_KEYPAIRS_SIZE), true); + + @Test + public void testCheckResponseForChallengeReturnsRefreshedStatus() throws Exception { + Challenge challenge = mock(Challenge.class); + when(challenge.getStatus()).thenReturn(Status.PROCESSING, Status.VALID); + + assertEquals(Status.VALID, client.checkResponseForChallenge(challenge)); + verify(challenge).fetch(); + } + + @Test + public void testCheckResponseForChallengeSkipsFetchWhenAlreadyValid() throws Exception { + Challenge challenge = mock(Challenge.class); + when(challenge.getStatus()).thenReturn(Status.VALID); + + assertEquals(Status.VALID, client.checkResponseForChallenge(challenge)); + verify(challenge, never()).fetch(); + } + + @Test + public void testCheckResponseForChallengeSkipsFetchWhenInvalid() throws Exception { + Challenge challenge = mock(Challenge.class); + when(challenge.getStatus()).thenReturn(Status.INVALID); + + assertEquals(Status.INVALID, client.checkResponseForChallenge(challenge)); + verify(challenge, never()).fetch(); + } + + @Test + public void testCheckResponseForOrderReturnsRefreshedStatus() throws Exception { + Order order = mockOrder(); + when(order.getStatus()).thenReturn(Status.PROCESSING, Status.VALID); + + assertEquals(Status.VALID, client.checkResponseForOrder(order)); + verify(order).fetch(); + } + + @Test + public void testCheckResponseForOrderSkipsFetchWhenAlreadyValid() throws Exception { + Order order = mockOrder(); + when(order.getStatus()).thenReturn(Status.VALID); + + assertEquals(Status.VALID, client.checkResponseForOrder(order)); + verify(order, never()).fetch(); + } + + @Test + public void testCheckResponseForOrderSkipsFetchWhenInvalid() throws Exception { + Order order = mockOrder(); + when(order.getStatus()).thenReturn(Status.INVALID); + + assertEquals(Status.INVALID, client.checkResponseForOrder(order)); + verify(order, never()).fetch(); + } + + private static Order mockOrder() { + Order order = mock(Order.class); + when(order.getIdentifiers()).thenReturn(List.of(Identifier.dns("example.com"))); + return order; + } +} From 1eb9b25b9c924cc074adb30936ab7d4c39695648 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Niccol=C3=B2=20Maltoni?= Date: Thu, 23 Jul 2026 15:26:08 +0200 Subject: [PATCH 08/10] feat: support custom ACME providers Any RFC 8555 CA via acme..* config (EAB kid/hmac supported, validated at parse time), per-certificate provider selection, per-provider account keys. Let's Encrypt stays default with the legacy account key. Closes #536 --- .../api/CertificatesResource.java | 41 +++- .../configstore/CertificateData.java | 3 + .../configstore/ConfigurationStore.java | 17 +- .../configstore/HerdDBConfigurationStore.java | 29 ++- .../PropertiesConfigurationStore.java | 14 +- .../carapaceproxy/core/HttpProxyServer.java | 10 + .../core/RuntimeServerConfiguration.java | 85 ++++++- .../server/certificates/ACMEClient.java | 40 +++- .../DynamicCertificatesManager.java | 57 +++-- .../config/AcmeProviderConfiguration.java | 41 ++++ .../config/SSLCertificateConfiguration.java | 11 + .../resources/conf/server.dynamic.properties | 13 ++ .../configstore/ConfigurationStoreTest.java | 15 +- .../core/RuntimeServerConfigurationTest.java | 215 ++++++++++++++++++ .../server/certificates/CertificatesTest.java | 56 ++++- .../DynamicCertificatesManagerTest.java | 72 +++++- .../components/certificates/Certificate.vue | 1 + .../certificates/CertificateForm.vue | 34 ++- .../components/certificates/Certificates.vue | 13 +- 19 files changed, 709 insertions(+), 58 deletions(-) create mode 100644 carapace-server/src/main/java/org/carapaceproxy/server/config/AcmeProviderConfiguration.java create mode 100644 carapace-server/src/test/java/org/carapaceproxy/core/RuntimeServerConfigurationTest.java diff --git a/carapace-server/src/main/java/org/carapaceproxy/api/CertificatesResource.java b/carapace-server/src/main/java/org/carapaceproxy/api/CertificatesResource.java index facc2de01..7fd8228c4 100644 --- a/carapace-server/src/main/java/org/carapaceproxy/api/CertificatesResource.java +++ b/carapace-server/src/main/java/org/carapaceproxy/api/CertificatesResource.java @@ -37,6 +37,7 @@ import java.security.PrivateKey; import java.security.cert.Certificate; import java.security.cert.X509Certificate; +import java.util.ArrayList; import java.util.Base64; import java.util.Collection; import java.util.Collections; @@ -68,6 +69,7 @@ import org.carapaceproxy.core.RuntimeServerConfiguration; import org.carapaceproxy.server.certificates.DynamicCertificateState; import org.carapaceproxy.server.certificates.DynamicCertificatesManager; +import org.carapaceproxy.server.config.AcmeProviderConfiguration; import org.carapaceproxy.server.config.ConfigurationChangeInProgressException; import org.carapaceproxy.server.config.ConfigurationNotValidException; import org.carapaceproxy.server.config.SSLCertificateConfiguration; @@ -96,14 +98,23 @@ public static final class CertificatesResponse { private final Collection certificates; private final String localStorePath; + private final Collection acmeProviders; public CertificatesResponse(final Collection certificates, final HttpProxyServer server) { this.certificates = certificates; this.localStorePath = server.getCurrentConfiguration().getLocalCertificatesStorePath(); + this.acmeProviders = availableProviders(server.getCurrentConfiguration()); } } + private static Collection availableProviders(final RuntimeServerConfiguration conf) { + final var providers = new ArrayList(); + providers.add(AcmeProviderConfiguration.DEFAULT_PROVIDER_NAME); + conf.getAcmeProviders().keySet().stream().sorted().forEach(providers::add); + return providers; + } + @Data public static final class CertificateBean { @@ -119,6 +130,7 @@ public static final class CertificateBean { private String serialNumber; private int attemptsCount; private String message; + private String provider; public CertificateBean( final String id, @@ -207,6 +219,7 @@ private static void fillCertificateBean( } if (certificate.isAcme()) { bean.setDaysBeforeRenewal(certificate.getDaysBeforeRenewal() + ""); + bean.setProvider(certificate.getProvider()); } bean.setStatus(certificateStateToString(state)); } catch (GeneralSecurityException | IOException ex) { @@ -232,6 +245,7 @@ public static final class CertificateForm { private Set subjectAltNames; private String type; private int daysBeforeRenewal = DEFAULT_DAYS_BEFORE_RENEWAL; + private String provider = AcmeProviderConfiguration.DEFAULT_PROVIDER_NAME; } @POST @@ -254,6 +268,11 @@ public Response createCertificate(CertificateForm form) { if (form.daysBeforeRenewal < 0) { return FormValidationResponse.fieldInvalid("daysBeforeRenewal"); } + final var server = (HttpProxyServer) context.getAttribute("server"); + final var provider = normalizeProvider(form.provider); + if (!isKnownProvider(provider, server)) { + return FormValidationResponse.fieldInvalid("provider"); + } if (findCertificateById(form.domain) != null) { return FormValidationResponse.fieldConflict("domain"); } @@ -261,8 +280,9 @@ public Response createCertificate(CertificateForm form) { final var cert = new CertificateData(form.domain, null, WAITING); cert.setSubjectAltNames(form.subjectAltNames); cert.setDaysBeforeRenewal(form.daysBeforeRenewal); + cert.setProvider(provider); try { - ((HttpProxyServer) context.getAttribute("server")).updateDynamicCertificateForDomain(cert); + server.updateDynamicCertificateForDomain(cert); } catch (Exception e) { return FormValidationResponse.error(e); } @@ -302,6 +322,15 @@ public Response downloadCertificateById(@PathParam("certId") final String certId .build(); } + private static String normalizeProvider(final String provider) { + return provider == null || provider.isBlank() ? AcmeProviderConfiguration.DEFAULT_PROVIDER_NAME : provider; + } + + private static boolean isKnownProvider(final String provider, final HttpProxyServer server) { + return AcmeProviderConfiguration.DEFAULT_PROVIDER_NAME.equals(provider) + || server.getCurrentConfiguration().getAcmeProviders().containsKey(provider); + } + private CertificateBean findCertificateById(final String certId) { HttpProxyServer server = (HttpProxyServer) context.getAttribute("server"); SSLCertificateConfiguration certificate = server.getCurrentConfiguration().getCertificates().get(certId); @@ -329,6 +358,7 @@ public Response uploadCertificate( @QueryParam("subjectaltnames") final List subjectAltNames, @QueryParam("type") @DefaultValue("manual") final String type, @QueryParam("daysbeforerenewal") final Integer daysbeforerenewal, + @QueryParam("provider") final String provider, final InputStream uploadedInputStream) throws Exception { try (InputStream input = uploadedInputStream) { @@ -354,6 +384,14 @@ public Response uploadCertificate( } } + final var acmeProvider = normalizeProvider(provider); + if (provider != null && !provider.isBlank() && !CertificateMode.ACME.equals(certType)) { + return Response.status(422).entity("ERROR: param 'provider' available for type 'acme' only").build(); + } + if (!isKnownProvider(acmeProvider, (HttpProxyServer) context.getAttribute("server"))) { + return Response.status(422).entity("ERROR: unknown ACME provider '" + acmeProvider + "'").build(); + } + String encodedData = ""; DynamicCertificateState state = WAITING; if (data != null && data.length > 0) { @@ -367,6 +405,7 @@ public Response uploadCertificate( cert.setManual(MANUAL.equals(certType)); cert.setSubjectAltNames(Set.copyOf(subjectAltNames)); cert.setDaysBeforeRenewal(daysbeforerenewal != null ? daysbeforerenewal : DEFAULT_DAYS_BEFORE_RENEWAL); + cert.setProvider(acmeProvider); ((HttpProxyServer) context.getAttribute("server")).updateDynamicCertificateForDomain(cert); diff --git a/carapace-server/src/main/java/org/carapaceproxy/configstore/CertificateData.java b/carapace-server/src/main/java/org/carapaceproxy/configstore/CertificateData.java index 3f7801866..912ad2517 100644 --- a/carapace-server/src/main/java/org/carapaceproxy/configstore/CertificateData.java +++ b/carapace-server/src/main/java/org/carapaceproxy/configstore/CertificateData.java @@ -34,6 +34,7 @@ import lombok.EqualsAndHashCode; import lombok.ToString; import org.carapaceproxy.server.certificates.DynamicCertificateState; +import org.carapaceproxy.server.config.AcmeProviderConfiguration; import org.shredzone.acme4j.toolbox.JSON; /** @@ -57,6 +58,8 @@ public class CertificateData { @EqualsAndHashCode.Exclude private Map pendingChallengesData; private boolean manual; + // not stored in db: re-set from the certificate configuration, like manual and daysBeforeRenewal + private String provider = AcmeProviderConfiguration.DEFAULT_PROVIDER_NAME; private int daysBeforeRenewal; private Date expiringDate; private String serialNumber; // hex diff --git a/carapace-server/src/main/java/org/carapaceproxy/configstore/ConfigurationStore.java b/carapace-server/src/main/java/org/carapaceproxy/configstore/ConfigurationStore.java index c31eb23e8..958bb02fa 100644 --- a/carapace-server/src/main/java/org/carapaceproxy/configstore/ConfigurationStore.java +++ b/carapace-server/src/main/java/org/carapaceproxy/configstore/ConfigurationStore.java @@ -187,9 +187,22 @@ default void close() { default void commitConfiguration(ConfigurationStore newConfigurationStore) { } - KeyPair loadAcmeUserKeyPair(); + /** + * Load the ACME account key pair for the given provider. + * + * @param providerName the name of the ACME provider the account belongs to + * @return the key pair, or null if none was saved yet + */ + KeyPair loadAcmeUserKeyPair(String providerName); - boolean saveAcmeUserKey(KeyPair pair); + /** + * Save the ACME account key pair for the given provider, without overwriting an existing one. + * + * @param pair the key pair to save + * @param providerName the name of the ACME provider the account belongs to + * @return true if the key pair was saved, false if one already exists (e.g. saved concurrently by another peer) + */ + boolean saveAcmeUserKey(KeyPair pair, String providerName); KeyPair loadKeyPairForDomain(String domain); diff --git a/carapace-server/src/main/java/org/carapaceproxy/configstore/HerdDBConfigurationStore.java b/carapace-server/src/main/java/org/carapaceproxy/configstore/HerdDBConfigurationStore.java index 715e0b310..68badbecc 100644 --- a/carapace-server/src/main/java/org/carapaceproxy/configstore/HerdDBConfigurationStore.java +++ b/carapace-server/src/main/java/org/carapaceproxy/configstore/HerdDBConfigurationStore.java @@ -52,6 +52,7 @@ import java.util.stream.Collectors; import org.apache.bookkeeper.stats.StatsLogger; import org.carapaceproxy.server.certificates.DynamicCertificateState; +import org.carapaceproxy.server.config.AcmeProviderConfiguration; import org.carapaceproxy.utils.StringUtils; import org.shredzone.acme4j.toolbox.JSON; import org.slf4j.Logger; @@ -329,29 +330,39 @@ public void commitConfiguration(ConfigurationStore newConfigurationStore) { } @Override - public KeyPair loadAcmeUserKeyPair() { + public KeyPair loadAcmeUserKeyPair(String providerName) { try { - return loadKeyPair(ACME_USER_KEY); + return loadKeyPair(acmeUserKeyName(providerName)); } catch (Exception err) { - LOG.error("Error while performing KeyPair loading for ACME user.", err); + LOG.error("Error while performing KeyPair loading for ACME user of provider {}.", providerName, err); throw new ConfigurationStoreException(err); } } @Override - public boolean saveAcmeUserKey(KeyPair pair) { + public boolean saveAcmeUserKey(KeyPair pair, String providerName) { try { - return saveKeyPair(pair, ACME_USER_KEY, false); + return saveKeyPair(pair, acmeUserKeyName(providerName), false); } catch (Exception err) { - LOG.error("Error while performing KeyPar saving for ACME user.", err); + LOG.error("Error while performing KeyPar saving for ACME user of provider {}.", providerName, err); throw new ConfigurationStoreException(err); } } + /** + * The account key of the built-in provider keeps the legacy {@link #ACME_USER_KEY} name, + * so existing Let's Encrypt accounts survive the upgrade; other providers get a dedicated key. + */ + private static String acmeUserKeyName(String providerName) { + return AcmeProviderConfiguration.DEFAULT_PROVIDER_NAME.equals(providerName) + ? ACME_USER_KEY + : ACME_USER_KEY + "_" + providerName; + } + @Override public KeyPair loadKeyPairForDomain(String domain) { try { - if (domain.equals(ACME_USER_KEY)) { + if (domain.startsWith(ACME_USER_KEY)) { return null; } return loadKeyPair(domain); @@ -364,7 +375,7 @@ public KeyPair loadKeyPairForDomain(String domain) { @Override public boolean saveKeyPairForDomain(KeyPair pair, String domain, boolean update) { try { - if (!domain.equals(ACME_USER_KEY)) { + if (!domain.startsWith(ACME_USER_KEY)) { return saveKeyPair(pair, domain, update); } } catch (Exception err) { @@ -414,7 +425,7 @@ private boolean saveKeyPair(KeyPair pair, String pk, boolean update) { @Override public CertificateData loadCertificateForDomain(String domain) { - if (domain.equals(ACME_USER_KEY)) { + if (domain.startsWith(ACME_USER_KEY)) { return null; } try (Connection con = datasource.getConnection()) { diff --git a/carapace-server/src/main/java/org/carapaceproxy/configstore/PropertiesConfigurationStore.java b/carapace-server/src/main/java/org/carapaceproxy/configstore/PropertiesConfigurationStore.java index a5de5dab2..7de66f6b7 100644 --- a/carapace-server/src/main/java/org/carapaceproxy/configstore/PropertiesConfigurationStore.java +++ b/carapace-server/src/main/java/org/carapaceproxy/configstore/PropertiesConfigurationStore.java @@ -43,7 +43,7 @@ public class PropertiesConfigurationStore implements ConfigurationStore { private final ConcurrentHashMap certificates = new ConcurrentHashMap<>(); private final ConcurrentHashMap domainsKeyPair = new ConcurrentHashMap<>(); private final ConcurrentHashMap acmeChallengeTokens = new ConcurrentHashMap<>(); - private KeyPair acmeUserKey; + private final ConcurrentHashMap acmeUserKeys = new ConcurrentHashMap<>(); public PropertiesConfigurationStore(Properties properties) { this.properties = properties; @@ -69,17 +69,13 @@ public void forEach(String prefix, BiConsumer consumer) { } @Override - public KeyPair loadAcmeUserKeyPair() { - return acmeUserKey; + public KeyPair loadAcmeUserKeyPair(String providerName) { + return acmeUserKeys.get(providerName); } @Override - public boolean saveAcmeUserKey(KeyPair pair) { - if (acmeUserKey == null) { - acmeUserKey = pair; - return true; - } - return false; + public boolean saveAcmeUserKey(KeyPair pair, String providerName) { + return acmeUserKeys.putIfAbsent(providerName, pair) == null; } @Override diff --git a/carapace-server/src/main/java/org/carapaceproxy/core/HttpProxyServer.java b/carapace-server/src/main/java/org/carapaceproxy/core/HttpProxyServer.java index b1b79bca3..567869a3b 100644 --- a/carapace-server/src/main/java/org/carapaceproxy/core/HttpProxyServer.java +++ b/carapace-server/src/main/java/org/carapaceproxy/core/HttpProxyServer.java @@ -87,6 +87,7 @@ import org.carapaceproxy.server.cache.ContentsCache; import org.carapaceproxy.server.certificates.DynamicCertificatesManager; import org.carapaceproxy.server.certificates.ocsp.OcspStaplingManager; +import org.carapaceproxy.server.config.AcmeProviderConfiguration; import org.carapaceproxy.server.config.BackendConfiguration; import org.carapaceproxy.server.config.ConfigurationChangeInProgressException; import org.carapaceproxy.server.config.ConfigurationNotValidException; @@ -687,8 +688,14 @@ private void performCertificateUpdate(Properties props, String key, CertificateD props.setProperty(key.replace("hostname", "mode"), cert.isManual() ? "manual" : "acme"); if (cert.isManual()) { props.remove(key.replace("hostname", "daysbeforerenewal")); // type changed from acme to manual + props.remove(key.replace("hostname", "provider")); } else { props.setProperty(key.replace("hostname", "daysbeforerenewal"), cert.getDaysBeforeRenewal() + ""); + if (AcmeProviderConfiguration.DEFAULT_PROVIDER_NAME.equals(cert.getProvider())) { + props.remove(key.replace("hostname", "provider")); // default provider is implicit + } else { + props.setProperty(key.replace("hostname", "provider"), cert.getProvider()); + } } if (cert.getSubjectAltNames() != null && !cert.getSubjectAltNames().isEmpty()) { props.setProperty( @@ -707,6 +714,9 @@ private void performCertificateCreate(Properties props, CertificateData cert) { props.setProperty(prefix + "mode", cert.isManual() ? "manual" : "acme"); if (!cert.isManual()) { props.setProperty(prefix + "daysbeforerenewal", cert.getDaysBeforeRenewal() + ""); + if (!AcmeProviderConfiguration.DEFAULT_PROVIDER_NAME.equals(cert.getProvider())) { + props.setProperty(prefix + "provider", cert.getProvider()); + } } if (cert.getSubjectAltNames() != null && !cert.getSubjectAltNames().isEmpty()) { props.setProperty(prefix + "san", String.join(",", cert.getSubjectAltNames())); diff --git a/carapace-server/src/main/java/org/carapaceproxy/core/RuntimeServerConfiguration.java b/carapace-server/src/main/java/org/carapaceproxy/core/RuntimeServerConfiguration.java index b0e1de625..0e593fbde 100644 --- a/carapace-server/src/main/java/org/carapaceproxy/core/RuntimeServerConfiguration.java +++ b/carapace-server/src/main/java/org/carapaceproxy/core/RuntimeServerConfiguration.java @@ -21,6 +21,7 @@ import static org.carapaceproxy.server.certificates.DynamicCertificatesManager.DEFAULT_DAYS_BEFORE_RENEWAL; import static org.carapaceproxy.server.certificates.DynamicCertificatesManager.DEFAULT_KEYPAIRS_SIZE; +import static org.carapaceproxy.server.config.AcmeProviderConfiguration.DEFAULT_PROVIDER_NAME; import static org.carapaceproxy.server.config.ConnectionPoolConfiguration.DEFAULT_BORROW_TIMEOUT; import static org.carapaceproxy.server.config.ConnectionPoolConfiguration.DEFAULT_CONNECT_TIMEOUT; import static org.carapaceproxy.server.config.ConnectionPoolConfiguration.DEFAULT_DISPOSE_TIMEOUT; @@ -39,11 +40,14 @@ import io.netty.channel.group.DefaultChannelGroup; import io.netty.util.concurrent.DefaultEventExecutor; import java.io.File; +import java.net.URI; +import java.net.URISyntaxException; import java.security.NoSuchAlgorithmException; import java.sql.Timestamp; import java.text.SimpleDateFormat; import java.time.Duration; import java.util.ArrayList; +import java.util.Base64; import java.util.HashMap; import java.util.LinkedHashMap; import java.util.List; @@ -53,6 +57,7 @@ import javax.net.ssl.SSLContext; import lombok.Data; import org.carapaceproxy.configstore.ConfigurationStore; +import org.carapaceproxy.server.config.AcmeProviderConfiguration; import org.carapaceproxy.server.config.ConfigurationNotValidException; import org.carapaceproxy.server.config.ConnectionPoolConfiguration; import org.carapaceproxy.server.config.NetworkListenerConfiguration; @@ -60,6 +65,7 @@ import org.carapaceproxy.server.config.SSLCertificateConfiguration; import org.carapaceproxy.server.config.SSLCertificateConfiguration.CertificateMode; import org.carapaceproxy.server.mapper.StandardEndpointMapper; +import org.shredzone.acme4j.Session; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import reactor.netty.http.HttpProtocol; @@ -84,6 +90,7 @@ public class RuntimeServerConfiguration { public static final long DEFAULT_WARMUP_PERIOD = Duration.ofSeconds(30).toMillis(); private final Map listeners = new LinkedHashMap<>(); + private final Map acmeProviders = new HashMap<>(); private final Map certificates = new HashMap<>(); private final List requestFilters = new ArrayList<>(); private final Map connectionPools = new HashMap<>(); @@ -229,6 +236,7 @@ public void configure(ConfigurationStore properties) throws ConfigurationNotVali LOG.info("accesslog.advanced.enabled={}", accessLogAdvancedEnabled); LOG.info("accesslog.advanced.body.size={}", accessLogAdvancedBodySize); + configureAcmeProviders(properties); configureCertificates(properties); configureListeners(properties); configureFilters(properties); @@ -312,6 +320,75 @@ public void configure(ConfigurationStore properties) throws ConfigurationNotVali LOG.info("cache.cachealways={}", alwaysCachedExtensions); } + private void configureAcmeProviders(ConfigurationStore properties) throws ConfigurationNotValidException { + final var max = properties.findMaxIndexForPrefix("acme"); + for (int i = 0; i <= max; i++) { + final var prefix = "acme." + i + "."; + final var name = properties.getString(prefix + "name", ""); + if (name.isEmpty()) { + continue; + } + if (DEFAULT_PROVIDER_NAME.equals(name)) { + throw new ConfigurationNotValidException( + "Invalid value '" + name + "' for " + prefix + "name: it is the built-in provider and cannot be redefined" + ); + } + if (!name.matches("[a-z0-9][a-z0-9_-]*")) { + throw new ConfigurationNotValidException( + "Invalid value '" + name + "' for " + prefix + "name: only lowercase letters, digits, '-' and '_' are allowed" + ); + } + if (acmeProviders.containsKey(name)) { + throw new ConfigurationNotValidException( + "Invalid value '" + name + "' for " + prefix + "name: duplicate provider name" + ); + } + final var url = properties.getString(prefix + "url", ""); + if (url.isEmpty()) { + throw new ConfigurationNotValidException( + "Invalid ACME provider configuration " + prefix + ": url cannot be empty" + ); + } + try { + // acme4j accepts both https directory URLs and acme: provider URIs (e.g. acme://pebble) + // acme4j resolves providers by exact scheme match, so no case-insensitive comparison here + final var scheme = new URI(url).getScheme(); + if (!"https".equals(scheme) && !"acme".equals(scheme)) { + throw new ConfigurationNotValidException( + "Invalid value '" + url + "' for " + prefix + "url: scheme must be https or acme (lowercase)" + ); + } + // resolves the acme: provider via ServiceLoader and validates the URL, without network I/O + new Session(url); + } catch (URISyntaxException | IllegalArgumentException e) { + throw new ConfigurationNotValidException( + "Invalid value '" + url + "' for " + prefix + "url: " + e.getMessage() + ); + } + final var kid = properties.getString(prefix + "kid", ""); + final var hmac = properties.getString(prefix + "hmac", ""); + if (kid.isEmpty() != hmac.isEmpty()) { + throw new ConfigurationNotValidException( + "Invalid ACME provider configuration " + prefix + + ": kid and hmac must be both set (external account binding) or both empty" + ); + } + if (!hmac.isEmpty()) { + try { + // acme4j expects the MAC key base64url-encoded, fail at configuration time instead of at account login + Base64.getUrlDecoder().decode(hmac); + } catch (IllegalArgumentException e) { + throw new ConfigurationNotValidException( + "Invalid ACME provider configuration " + prefix + ": hmac is not a valid base64url-encoded key" + ); + } + } + final var provider = new AcmeProviderConfiguration(name, url, kid, hmac); + acmeProviders.put(name, provider); + LOG.info("Configured ACME provider acme.{}: name={}, url={}, external account binding: {}", i, name, url, provider.hasExternalAccountBinding()); + } + } + private void configureCertificates(ConfigurationStore properties) throws ConfigurationNotValidException { final var max = properties.findMaxIndexForPrefix("certificate"); for (int i = 0; i <= max; i++) { @@ -323,8 +400,14 @@ private void configureCertificates(ConfigurationStore properties) throws Configu final var pw = properties.getString(prefix + "password", ""); final var mode = properties.getString(prefix + "mode", "static"); final var daysBeforeRenewal = properties.getInt(prefix + "daysbeforerenewal", DEFAULT_DAYS_BEFORE_RENEWAL); + final var provider = properties.getString(prefix + "provider", DEFAULT_PROVIDER_NAME); + if (!DEFAULT_PROVIDER_NAME.equals(provider) && !acmeProviders.containsKey(provider)) { + throw new ConfigurationNotValidException( + "Invalid value '" + provider + "' for " + prefix + "provider. No ACME provider configured with such name" + ); + } try { - final var config = new SSLCertificateConfiguration(hostname, subjectAltNames, file, pw, CertificateMode.valueOf(mode.toUpperCase())); + final var config = new SSLCertificateConfiguration(hostname, subjectAltNames, file, pw, CertificateMode.valueOf(mode.toUpperCase()), provider); if (config.isAcme()) { config.setDaysBeforeRenewal(daysBeforeRenewal); } diff --git a/carapace-server/src/main/java/org/carapaceproxy/server/certificates/ACMEClient.java b/carapace-server/src/main/java/org/carapaceproxy/server/certificates/ACMEClient.java index a609b781e..7e77f6338 100644 --- a/carapace-server/src/main/java/org/carapaceproxy/server/certificates/ACMEClient.java +++ b/carapace-server/src/main/java/org/carapaceproxy/server/certificates/ACMEClient.java @@ -48,7 +48,10 @@ /** * - * Let's Encrypt ACME client for automatic SSL certificate issuing. + * ACME client for automatic SSL certificate issuing. + *

+ * It works against any RFC 8555 compliant CA, given its directory URL; + * Let's Encrypt is the default one, and CAs requiring External Account Binding (e.g. DigiCert) are supported too. * * @author paolo.venturi * @@ -63,13 +66,35 @@ public class ACMEClient { // For testing server use public static final String TESTING_CA = "https://acme-staging-v02.api.letsencrypt.org/directory"; //"acme://letsencrypt.org/staging"; - private final boolean testingModeOn; private final KeyPair userKey; + private final String directoryUrl; + private final String kid; + private final String hmac; + /** + * Build a client for the Let's Encrypt CA. + * + * @param userKey the ACME account key pair + * @param testingMode whether to use the staging endpoint instead of the production one + */ public ACMEClient(KeyPair userKey, boolean testingMode) { + this(userKey, testingMode ? TESTING_CA : PRODUCTION_CA, null, null); + } + + /** + * Build a client for any RFC 8555 compliant CA. + * + * @param userKey the ACME account key pair + * @param directoryUrl the directory URL of the CA + * @param kid the key identifier for External Account Binding, or null if the CA doesn't require it + * @param hmac the base64-encoded MAC key for External Account Binding, or null if the CA doesn't require it + */ + public ACMEClient(KeyPair userKey, String directoryUrl, String kid, String hmac) { Security.addProvider(new BouncyCastleProvider()); this.userKey = userKey; - this.testingModeOn = testingMode; + this.directoryUrl = directoryUrl; + this.kid = kid; + this.hmac = hmac; } /** @@ -81,10 +106,13 @@ public ACMEClient(KeyPair userKey, boolean testingMode) { * @throws org.shredzone.acme4j.exception.AcmeException */ public Login getLogin() throws AcmeException { - return new AccountBuilder() + final var accountBuilder = new AccountBuilder() .agreeToTermsOfService() - .useKeyPair(userKey) - .createLogin(new Session(testingModeOn ? TESTING_CA : PRODUCTION_CA)); + .useKeyPair(userKey); + if (kid != null && !kid.isEmpty()) { + accountBuilder.withKeyIdentifier(kid, hmac); + } + return accountBuilder.createLogin(new Session(directoryUrl)); } /* diff --git a/carapace-server/src/main/java/org/carapaceproxy/server/certificates/DynamicCertificatesManager.java b/carapace-server/src/main/java/org/carapaceproxy/server/certificates/DynamicCertificatesManager.java index dbfe32dd8..4b49bf861 100644 --- a/carapace-server/src/main/java/org/carapaceproxy/server/certificates/DynamicCertificatesManager.java +++ b/carapace-server/src/main/java/org/carapaceproxy/server/certificates/DynamicCertificatesManager.java @@ -31,6 +31,7 @@ import static org.carapaceproxy.server.certificates.DynamicCertificateState.VERIFIED; import static org.carapaceproxy.server.certificates.DynamicCertificateState.VERIFYING; import static org.carapaceproxy.server.certificates.DynamicCertificateState.WAITING; +import static org.carapaceproxy.server.config.AcmeProviderConfiguration.DEFAULT_PROVIDER_NAME; import static org.carapaceproxy.server.config.SSLCertificateConfiguration.CertificateMode.MANUAL; import static org.carapaceproxy.utils.CertificatesUtils.isCertificateExpired; import static org.carapaceproxy.utils.CertificatesUtils.readChainFromKeystore; @@ -106,7 +107,7 @@ public class DynamicCertificatesManager implements Runnable { private static final String EVENT_CERTIFICATES_REQUEST_STORE = "certificates_request_store"; private volatile Map certificates = new ConcurrentHashMap<>(); - private ACMEClient acmeClient; // Let's Encrypt client + private volatile Map acmeClients = Map.of(); // one client per ACME provider, keyed by provider name private Route53Client r53Client; private String awsAccessKey; private String awsSecretKey; @@ -170,9 +171,15 @@ public synchronized void reloadConfiguration(RuntimeServerConfiguration configur throw new DynamicCertificatesManagerException("ConfigurationStore not set."); } keyPairsSize = configuration.getKeyPairsSize(); - if (acmeClient == null) { - acmeClient = new ACMEClient(loadOrCreateAcmeUserKeyPair(), TESTING_MODE); - } + // rebuilt on every reload, so that changes to provider url/kid/hmac take effect + final var clients = new HashMap(); + clients.put(DEFAULT_PROVIDER_NAME, new ACMEClient(loadOrCreateAcmeUserKeyPair(DEFAULT_PROVIDER_NAME), TESTING_MODE)); + for (final var provider : configuration.getAcmeProviders().values()) { + clients.put(provider.name(), new ACMEClient( + loadOrCreateAcmeUserKeyPair(provider.name()), provider.url(), provider.kid(), provider.hmac() + )); + } + acmeClients = Map.copyOf(clients); domainsCheckerIPAddresses = configuration.getDomainsCheckerIPAddresses(); loadCertificates(configuration.getCertificates()); period = configuration.getDynamicCertificatesManagerPeriod(); @@ -184,18 +191,26 @@ public synchronized void reloadConfiguration(RuntimeServerConfiguration configur } } - private KeyPair loadOrCreateAcmeUserKeyPair() { - KeyPair pair = store.loadAcmeUserKeyPair(); + private KeyPair loadOrCreateAcmeUserKeyPair(String providerName) { + KeyPair pair = store.loadAcmeUserKeyPair(providerName); if (pair == null) { pair = KeyPairUtils.createKeyPair(keyPairsSize > 0 ? keyPairsSize : DEFAULT_KEYPAIRS_SIZE); - if (!store.saveAcmeUserKey(pair)) { - pair = store.loadAcmeUserKeyPair(); // load key created concurrently by another peer + if (!store.saveAcmeUserKey(pair, providerName)) { + pair = store.loadAcmeUserKeyPair(providerName); // load key created concurrently by another peer } } return pair; } + private ACMEClient acmeClientFor(CertificateData cert) { + final var client = acmeClients.get(cert.getProvider()); + if (client == null) { + throw new IllegalStateException("No ACME client available for provider " + cert.getProvider()); + } + return client; + } + private void loadCertificates(Map certificates) throws ConfigurationNotValidException { try { final var _certificates = new ConcurrentHashMap(); @@ -210,7 +225,7 @@ private void loadCertificates(Map certifica } boolean forceManual = MANUAL == config.getMode(); _certificates.put(domain, loadOrCreateDynamicCertificateForDomain( - domain, config.getSubjectAltNames(), forceManual, config.getDaysBeforeRenewal() + domain, config.getSubjectAltNames(), forceManual, config.getDaysBeforeRenewal(), config.getProvider() )); } } @@ -223,7 +238,8 @@ private void loadCertificates(Map certifica private CertificateData loadOrCreateDynamicCertificateForDomain(String domain, Set subjectAltNames, boolean forceManual, - int daysBeforeRenewal) throws GeneralSecurityException, MalformedURLException { + int daysBeforeRenewal, + String provider) throws GeneralSecurityException, MalformedURLException { CertificateData cert = store.loadCertificateForDomain(domain); if (cert == null) { cert = new CertificateData(domain, subjectAltNames, null, WAITING, null, null); @@ -240,6 +256,7 @@ private CertificateData loadOrCreateDynamicCertificateForDomain(String domain, cert.setManual(forceManual); if (!forceManual) { // only for ACME cert.setDaysBeforeRenewal(daysBeforeRenewal); + cert.setProvider(provider); } return cert; } @@ -293,7 +310,7 @@ private void certificatesLifecycle() { final var domain = data.getDomain(); try { // this has to be always fetch from db! - CertificateData cert = loadOrCreateDynamicCertificateForDomain(domain, data.getSubjectAltNames(), false, data.getDaysBeforeRenewal()); + CertificateData cert = loadOrCreateDynamicCertificateForDomain(domain, data.getSubjectAltNames(), false, data.getDaysBeforeRenewal(), data.getProvider()); switch (cert.getState()) { // certificate waiting to be issues/renew case WAITING -> startCertificateProcessing(domain, cert); @@ -310,12 +327,12 @@ private void certificatesLifecycle() { // challenge succeeded case VERIFIED -> { LOG.info("Certificate for domain {} VERIFIED.", domain); - Order pendingOrder = acmeClient.getLogin().bindOrder(cert.getPendingOrderLocation()); + Order pendingOrder = acmeClientFor(cert).getLogin().bindOrder(cert.getPendingOrderLocation()); // if the order is already valid, we have to skip finalization if (pendingOrder.getStatus() != Status.VALID) { try { KeyPair keys = loadOrCreateKeyPairForDomain(domain); - acmeClient.orderCertificate(pendingOrder, keys); + acmeClientFor(cert).orderCertificate(pendingOrder, keys); } catch (AcmeException ex) { // order finalization failed LOG.error("Certificate order finalization for domain {} FAILED.", domain, ex); cert.error(ex.getMessage()); @@ -327,6 +344,7 @@ private void certificatesLifecycle() { // certificate ordering case ORDERING -> { LOG.info("ORDERING certificate for domain {}.", domain); + final var acmeClient = acmeClientFor(cert); Order order = acmeClient.getLogin().bindOrder(cert.getPendingOrderLocation()); Status status = acmeClient.checkResponseForOrder(order); if (status == Status.VALID) { @@ -367,7 +385,9 @@ private void certificatesLifecycle() { store.saveCertificate(cert); flushCache = true; } - } catch (AcmeException | IOException | GeneralSecurityException | IllegalStateException ex) { + } catch (AcmeException | IOException | GeneralSecurityException | RuntimeException ex) { + // RuntimeException included on purpose, as an escaped one would silently cancel the scheduled task; + // this would kill the renewal loop for every certificate LOG.error("Error while handling dynamic certificate for domain {}", domain, ex); } } @@ -425,6 +445,7 @@ private Set matchDomainIps(String domain) throws AcmeException, UnknownH * */ private void createOrderAndChallengesForCertificate(CertificateData cert) throws AcmeException { + final var acmeClient = acmeClientFor(cert); Order order = acmeClient.createOrderForDomain(cert.getNames()); cert.setPendingOrderLocation(order.getLocation()); LOG.info("Pending order location for domain {}: {}", cert.getDomain(), cert.getPendingOrderLocation()); @@ -504,7 +525,7 @@ private void checkDnsChallengesReachabilityForCertificate(CertificateData cert) private Map getChallengesFromCertificate(CertificateData cert) throws AcmeException { final var challengesData = new HashMap(); - final var login = acmeClient.getLogin(); + final var login = acmeClientFor(cert).getLogin(); for (final var e: cert.getPendingChallengesData().entrySet()) { final var domain = e.getKey(); final var challenge = e.getValue(); @@ -525,7 +546,7 @@ private void checkChallengesResponsesForCertificate(CertificateData cert) throws final var domain = c.getKey(); final var challenge = c.getValue(); LOG.info("VERIFYING challenge response for domain {}: {}", domain, challenge); - final var status = acmeClient.checkResponseForChallenge(challenge); // checks response and updates the challenge + final var status = acmeClientFor(cert).checkResponseForChallenge(challenge); // checks response and updates the challenge cert.getPendingChallengesData().put(domain, challenge.getJSON()); if (status == Status.VALID) { okCount++; @@ -696,8 +717,8 @@ private void reloadCertificatesFromDBInternal() { for (Entry entry : certificates.entrySet()) { String domain = entry.getKey(); CertificateData cert = entry.getValue(); - // "wildcard" and "manual" flags and "daysBeforeRenewal" are not stored in db > have to be re-set from existing config - CertificateData freshCert = loadOrCreateDynamicCertificateForDomain(domain, cert.getSubjectAltNames(), cert.isManual(), cert.getDaysBeforeRenewal()); + // "wildcard" and "manual" flags, "daysBeforeRenewal" and "provider" are not stored in db > have to be re-set from existing config + CertificateData freshCert = loadOrCreateDynamicCertificateForDomain(domain, cert.getSubjectAltNames(), cert.isManual(), cert.getDaysBeforeRenewal(), cert.getProvider()); newCertificates.put(domain, freshCert); LOG.info("RELOADED certificate for domain {}: {}", domain, freshCert); } diff --git a/carapace-server/src/main/java/org/carapaceproxy/server/config/AcmeProviderConfiguration.java b/carapace-server/src/main/java/org/carapaceproxy/server/config/AcmeProviderConfiguration.java new file mode 100644 index 000000000..9d0fe5518 --- /dev/null +++ b/carapace-server/src/main/java/org/carapaceproxy/server/config/AcmeProviderConfiguration.java @@ -0,0 +1,41 @@ +/* + Licensed to Diennea S.r.l. under one + or more contributor license agreements. See the NOTICE file + distributed with this work for additional information + regarding copyright ownership. Diennea S.r.l. licenses this file + to you under the Apache License, Version 2.0 (the + "License"); you may not use this file except in compliance + with the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, + software distributed under the License is distributed on an + "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + KIND, either express or implied. See the License for the + specific language governing permissions and limitations + under the License. + + */ +package org.carapaceproxy.server.config; + +/** + * Configuration of a custom ACME provider (any RFC 8555 compliant CA). + *

+ * The built-in {@link #DEFAULT_PROVIDER_NAME} provider (Let's Encrypt) is always available + * and cannot be redefined; custom providers are configured with the {@code acme..*} + * property family and referenced by certificates via {@code certificate..provider}. + * + * @param name the provider name, unique across the configuration + * @param url the ACME directory URL of the CA + * @param kid the key identifier for External Account Binding, if the CA requires it + * @param hmac the base64-encoded MAC key for External Account Binding, if the CA requires it + */ +public record AcmeProviderConfiguration(String name, String url, String kid, String hmac) { + + public static final String DEFAULT_PROVIDER_NAME = "letsencrypt"; + + public boolean hasExternalAccountBinding() { + return kid != null && !kid.isEmpty(); + } +} diff --git a/carapace-server/src/main/java/org/carapaceproxy/server/config/SSLCertificateConfiguration.java b/carapace-server/src/main/java/org/carapaceproxy/server/config/SSLCertificateConfiguration.java index 56690dde4..fed1e5e78 100644 --- a/carapace-server/src/main/java/org/carapaceproxy/server/config/SSLCertificateConfiguration.java +++ b/carapace-server/src/main/java/org/carapaceproxy/server/config/SSLCertificateConfiguration.java @@ -49,6 +49,7 @@ public enum CertificateMode { private final String password; private final boolean wildcard; private final CertificateMode mode; + private final String provider; // ACME provider name, only meaningful for ACME certificates private int daysBeforeRenewal; public SSLCertificateConfiguration(String hostname, @@ -56,6 +57,15 @@ public SSLCertificateConfiguration(String hostname, String file, String password, CertificateMode mode) { + this(hostname, subjectAltNames, file, password, mode, AcmeProviderConfiguration.DEFAULT_PROVIDER_NAME); + } + + public SSLCertificateConfiguration(String hostname, + Set subjectAltNames, + String file, + String password, + CertificateMode mode, + String provider) { this.id = Objects.requireNonNull(hostname); if (hostname.equals("*")) { this.hostname = ""; @@ -75,6 +85,7 @@ public SSLCertificateConfiguration(String hostname, this.file = file; this.password = password; this.mode = mode; + this.provider = provider; } public boolean isWildcard() { diff --git a/carapace-server/src/main/resources/conf/server.dynamic.properties b/carapace-server/src/main/resources/conf/server.dynamic.properties index fcea81c7e..6a58abf2b 100644 --- a/carapace-server/src/main/resources/conf/server.dynamic.properties +++ b/carapace-server/src/main/resources/conf/server.dynamic.properties @@ -14,6 +14,19 @@ certificate.1.password=testproxy #certificate.100.hostname=mypublicdnsname.mycompany.com #certificate.100.mode=acme +# example of a custom ACME provider (any RFC 8555 compliant CA) +# certificates use Let's Encrypt unless certificate..provider names a configured provider +#acme.1.name=digicert +#acme.1.url=https://acme.digicert.com/v2/acme/directory +# external account binding credentials (both or neither), provided by the CA +#acme.1.kid=your-key-identifier +#acme.1.hmac=your-base64-encoded-hmac-key + +# example of a Dynamic certificate issued by a custom ACME provider +#certificate.101.hostname=another.mycompany.com +#certificate.101.mode=acme +#certificate.101.provider=digicert + listener.2.host=0.0.0.0 listener.2.port=4089 listener.2.ssl=true diff --git a/carapace-server/src/test/java/org/carapaceproxy/configstore/ConfigurationStoreTest.java b/carapace-server/src/test/java/org/carapaceproxy/configstore/ConfigurationStoreTest.java index 0b0872f9b..d3dd657fb 100644 --- a/carapace-server/src/test/java/org/carapaceproxy/configstore/ConfigurationStoreTest.java +++ b/carapace-server/src/test/java/org/carapaceproxy/configstore/ConfigurationStoreTest.java @@ -20,6 +20,7 @@ package org.carapaceproxy.configstore; import static org.carapaceproxy.server.certificates.DynamicCertificatesManager.DEFAULT_KEYPAIRS_SIZE; +import static org.carapaceproxy.server.config.AcmeProviderConfiguration.DEFAULT_PROVIDER_NAME; import static org.carapaceproxy.utils.TestUtils.assertEqualsKey; import static org.hamcrest.CoreMatchers.hasItems; import static org.hamcrest.CoreMatchers.is; @@ -231,8 +232,12 @@ public void testCertiticatesConfigurationStore(String type) throws Exception { private void testKeyPairOperations() { // KeyPairs generation + saving KeyPair acmePair = KeyPairUtils.createKeyPair(DEFAULT_KEYPAIRS_SIZE); - store.saveAcmeUserKey(acmePair); - store.saveAcmeUserKey(KeyPairUtils.createKeyPair(DEFAULT_KEYPAIRS_SIZE)); // key not overwritten + store.saveAcmeUserKey(acmePair, DEFAULT_PROVIDER_NAME); + store.saveAcmeUserKey(KeyPairUtils.createKeyPair(DEFAULT_KEYPAIRS_SIZE), DEFAULT_PROVIDER_NAME); // key not overwritten + + KeyPair customProviderPair = KeyPairUtils.createKeyPair(DEFAULT_KEYPAIRS_SIZE); + store.saveAcmeUserKey(customProviderPair, "customprovider"); // each provider has its own account key + store.saveAcmeUserKey(KeyPairUtils.createKeyPair(DEFAULT_KEYPAIRS_SIZE), "customprovider"); // key not overwritten store.saveKeyPairForDomain(KeyPairUtils.createKeyPair(DEFAULT_KEYPAIRS_SIZE), d1, true); KeyPair domain1Pair = KeyPairUtils.createKeyPair(DEFAULT_KEYPAIRS_SIZE); @@ -243,10 +248,14 @@ private void testKeyPairOperations() { store.saveKeyPairForDomain(KeyPairUtils.createKeyPair(DEFAULT_KEYPAIRS_SIZE), d2, false); // key not overwritten // KeyPairs loading - KeyPair loadedPair = store.loadAcmeUserKeyPair(); + KeyPair loadedPair = store.loadAcmeUserKeyPair(DEFAULT_PROVIDER_NAME); assertEqualsKey(acmePair.getPrivate(), loadedPair.getPrivate()); assertEqualsKey(acmePair.getPublic(), loadedPair.getPublic()); + loadedPair = store.loadAcmeUserKeyPair("customprovider"); + assertEqualsKey(customProviderPair.getPrivate(), loadedPair.getPrivate()); + assertEqualsKey(customProviderPair.getPublic(), loadedPair.getPublic()); + loadedPair = store.loadKeyPairForDomain(d1); assertEqualsKey(domain1Pair.getPrivate(), loadedPair.getPrivate()); assertEqualsKey(domain1Pair.getPublic(), loadedPair.getPublic()); diff --git a/carapace-server/src/test/java/org/carapaceproxy/core/RuntimeServerConfigurationTest.java b/carapace-server/src/test/java/org/carapaceproxy/core/RuntimeServerConfigurationTest.java new file mode 100644 index 000000000..e0669f131 --- /dev/null +++ b/carapace-server/src/test/java/org/carapaceproxy/core/RuntimeServerConfigurationTest.java @@ -0,0 +1,215 @@ +/* + Licensed to Diennea S.r.l. under one + or more contributor license agreements. See the NOTICE file + distributed with this work for additional information + regarding copyright ownership. Diennea S.r.l. licenses this file + to you under the Apache License, Version 2.0 (the + "License"); you may not use this file except in compliance + with the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, + software distributed under the License is distributed on an + "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + KIND, either express or implied. See the License for the + specific language governing permissions and limitations + under the License. + + */ +package org.carapaceproxy.core; + +import static org.carapaceproxy.server.config.AcmeProviderConfiguration.DEFAULT_PROVIDER_NAME; +import static org.hamcrest.CoreMatchers.containsString; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertThrows; +import static org.junit.Assert.assertTrue; +import java.util.Properties; +import org.carapaceproxy.configstore.PropertiesConfigurationStore; +import org.carapaceproxy.server.config.ConfigurationNotValidException; +import org.junit.Test; + +/** + * Tests for the {@code acme..*} provider family and the {@code certificate..provider} property. + */ +public class RuntimeServerConfigurationTest { + + private static RuntimeServerConfiguration configure(String... keyValues) throws ConfigurationNotValidException { + final var props = new Properties(); + for (int i = 0; i < keyValues.length; i += 2) { + props.setProperty(keyValues[i], keyValues[i + 1]); + } + final var config = new RuntimeServerConfiguration(); + config.configure(new PropertiesConfigurationStore(props)); + return config; + } + + @Test + public void testConfigureAcmeProviders() throws Exception { + final var config = configure( + "acme.1.name", "digicert", + "acme.1.url", "https://acme.digicert.com/v2/acme/directory", + "acme.1.kid", "my-kid", + "acme.1.hmac", "my-base64-hmac", + "acme.2.name", "pebble", + "acme.2.url", "https://localhost:14000/dir" + ); + assertEquals(2, config.getAcmeProviders().size()); + + final var digicert = config.getAcmeProviders().get("digicert"); + assertEquals("https://acme.digicert.com/v2/acme/directory", digicert.url()); + assertEquals("my-kid", digicert.kid()); + assertEquals("my-base64-hmac", digicert.hmac()); + assertTrue(digicert.hasExternalAccountBinding()); + + final var pebble = config.getAcmeProviders().get("pebble"); + assertEquals("https://localhost:14000/dir", pebble.url()); + assertFalse(pebble.hasExternalAccountBinding()); + } + + @Test + public void testAcmeProviderWithoutNameIsSkipped() throws Exception { + final var config = configure("acme.1.url", "https://acme.example.com/directory"); + assertTrue(config.getAcmeProviders().isEmpty()); + } + + @Test + public void testAcmeProviderReservedName() { + final var e = assertThrows(ConfigurationNotValidException.class, () -> configure( + "acme.1.name", DEFAULT_PROVIDER_NAME, + "acme.1.url", "https://acme.example.com/directory" + )); + assertThat(e.getMessage(), containsString("built-in")); + } + + @Test + public void testAcmeProviderInvalidName() { + final var e = assertThrows(ConfigurationNotValidException.class, () -> configure( + "acme.1.name", "Not A Valid Name!", + "acme.1.url", "https://acme.example.com/directory" + )); + assertThat(e.getMessage(), containsString("acme.1.name")); + } + + @Test + public void testAcmeProviderDuplicateName() { + final var e = assertThrows(ConfigurationNotValidException.class, () -> configure( + "acme.1.name", "digicert", + "acme.1.url", "https://acme.example.com/directory", + "acme.2.name", "digicert", + "acme.2.url", "https://acme.example.org/directory" + )); + assertThat(e.getMessage(), containsString("duplicate")); + } + + @Test + public void testAcmeProviderMissingUrl() { + final var e = assertThrows(ConfigurationNotValidException.class, () -> configure( + "acme.1.name", "digicert" + )); + assertThat(e.getMessage(), containsString("url")); + } + + @Test + public void testAcmeProviderAcmeUriAccepted() throws Exception { + final var config = configure( + "acme.1.name", "pebble", + "acme.1.url", "acme://pebble" + ); + assertEquals("acme://pebble", config.getAcmeProviders().get("pebble").url()); + } + + @Test + public void testAcmeProviderUnknownAcmeUri() { + final var e = assertThrows(ConfigurationNotValidException.class, () -> configure( + "acme.1.name", "pebble", + "acme.1.url", "acme://pebbel" // typo: no such acme4j provider + )); + assertThat(e.getMessage(), containsString("acme.1.url")); + } + + @Test + public void testAcmeProviderUppercaseSchemeRejected() { + // acme4j resolves providers by exact scheme match, so accepting HTTPS:// at parse time + // would only defer the failure to the first login attempt + final var e = assertThrows(ConfigurationNotValidException.class, () -> configure( + "acme.1.name", "digicert", + "acme.1.url", "HTTPS://acme.example.com/directory" + )); + assertThat(e.getMessage(), containsString("scheme")); + } + + @Test + public void testAcmeProviderMalformedUrl() { + final var e = assertThrows(ConfigurationNotValidException.class, () -> configure( + "acme.1.name", "digicert", + "acme.1.url", "not a valid url" + )); + assertThat(e.getMessage(), containsString("acme.1.url")); + } + + @Test + public void testAcmeProviderUnsupportedUrlScheme() { + for (final var url : new String[]{"ftp://acme.example.com/directory", "http://acme.example.com/directory"}) { + final var e = assertThrows(ConfigurationNotValidException.class, () -> configure( + "acme.1.name", "digicert", + "acme.1.url", url + )); + assertThat(e.getMessage(), containsString("scheme")); + } + } + + @Test + public void testAcmeProviderInvalidHmac() { + final var e = assertThrows(ConfigurationNotValidException.class, () -> configure( + "acme.1.name", "digicert", + "acme.1.url", "https://acme.example.com/directory", + "acme.1.kid", "my-kid", + "acme.1.hmac", "not+valid/base64url!" + )); + assertThat(e.getMessage(), containsString("base64url")); + } + + @Test + public void testAcmeProviderKidWithoutHmac() { + final var e = assertThrows(ConfigurationNotValidException.class, () -> configure( + "acme.1.name", "digicert", + "acme.1.url", "https://acme.example.com/directory", + "acme.1.kid", "my-kid" + )); + assertThat(e.getMessage(), containsString("hmac")); + } + + @Test + public void testCertificateDefaultProvider() throws Exception { + final var config = configure( + "certificate.0.hostname", "example.com", + "certificate.0.mode", "acme" + ); + assertEquals(DEFAULT_PROVIDER_NAME, config.getCertificates().get("example.com").getProvider()); + } + + @Test + public void testCertificateWithCustomProvider() throws Exception { + final var config = configure( + "acme.1.name", "digicert", + "acme.1.url", "https://acme.digicert.com/v2/acme/directory", + "certificate.0.hostname", "example.com", + "certificate.0.mode", "acme", + "certificate.0.provider", "digicert" + ); + assertEquals("digicert", config.getCertificates().get("example.com").getProvider()); + } + + @Test + public void testCertificateWithUnknownProvider() { + final var e = assertThrows(ConfigurationNotValidException.class, () -> configure( + "certificate.0.hostname", "example.com", + "certificate.0.mode", "acme", + "certificate.0.provider", "unknown" + )); + assertThat(e.getMessage(), containsString("certificate.0.provider")); + } +} diff --git a/carapace-server/src/test/java/org/carapaceproxy/server/certificates/CertificatesTest.java b/carapace-server/src/test/java/org/carapaceproxy/server/certificates/CertificatesTest.java index 3edfe809d..874a7e476 100644 --- a/carapace-server/src/test/java/org/carapaceproxy/server/certificates/CertificatesTest.java +++ b/carapace-server/src/test/java/org/carapaceproxy/server/certificates/CertificatesTest.java @@ -26,6 +26,7 @@ import static org.carapaceproxy.configstore.ConfigurationStoreUtils.base64DecodeCertificateChain; import static org.carapaceproxy.server.certificates.DynamicCertificatesManager.DEFAULT_DAYS_BEFORE_RENEWAL; import static org.carapaceproxy.server.certificates.DynamicCertificatesManager.DEFAULT_KEYPAIRS_SIZE; +import static org.carapaceproxy.server.config.AcmeProviderConfiguration.DEFAULT_PROVIDER_NAME; import static org.carapaceproxy.utils.CertificatesTestUtils.generateSampleChain; import static org.carapaceproxy.utils.CertificatesTestUtils.uploadCertificate; import static org.carapaceproxy.utils.CertificatesUtils.createKeystore; @@ -419,6 +420,55 @@ public void testUploadTypedCertificatesWithDaysBeforeRenewal(String type) throws } } + @Test + @Parameters({"acme", "manual"}) + public void testUploadTypedCertificatesWithProvider(String type) throws Exception { + configureAndStartServer(); + DynamicCertificatesManager dynCertsMan = server.getDynamicCertificatesManager(); + KeyPair endUserKeyPair = KeyPairUtils.createKeyPair(DEFAULT_KEYPAIRS_SIZE); + Certificate[] chain = generateSampleChain(endUserKeyPair, false); + byte[] chainData = createKeystore(chain, endUserKeyPair.getPrivate()); + + // configure a custom ACME provider + config.put("acme.1.name", "custom"); + config.put("acme.1.url", "https://acme.example.com/directory"); + changeDynamicConfiguration(config); + + try (RawHttpClient client = new RawHttpClient("localhost", DEFAULT_ADMIN_PORT)) { + // unknown provider + HttpResponse resp = uploadCertificate("localhost2", "type=" + type + "&provider=unknown", chainData, client, credentials); + if (type.equals("manual")) { + assertTrue(resp.getBodyString().contains("ERROR: param 'provider' available for type 'acme' only")); + } else { + assertTrue(resp.getBodyString().contains("ERROR: unknown ACME provider")); + } + + // custom provider + resp = uploadCertificate("localhost2", "type=" + type + "&provider=custom", chainData, client, credentials); + if (type.equals("manual")) { + assertTrue(resp.getBodyString().contains("ERROR: param 'provider' available for type 'acme' only")); + return; + } + assertTrue(resp.getBodyString().contains("SUCCESS")); + CertificateData data = dynCertsMan.getCertificateDataForDomain("localhost2"); + assertNotNull(data); + assertEquals("custom", data.getProvider()); + assertEquals("custom", server.getCurrentConfiguration().getCertificates().get("localhost2").getProvider()); + ConfigurationStore store = server.getDynamicConfigurationStore(); + assertTrue(store.anyPropertyMatches((k, v) -> + k.matches("certificate\\.[0-9]+\\.provider") && v.equals("custom") + )); + + // update back to the default provider: the property gets removed + resp = uploadCertificate("localhost2", "type=acme", chainData, client, credentials); + assertTrue(resp.getBodyString().contains("SUCCESS")); + data = dynCertsMan.getCertificateDataForDomain("localhost2"); + assertNotNull(data); + assertEquals(DEFAULT_PROVIDER_NAME, data.getProvider()); + assertFalse(store.anyPropertyMatches((k, v) -> k.matches("certificate\\.[0-9]+\\.provider"))); + } + } + @Test public void testOCSP() throws Exception { configureAndStartServer(); @@ -528,7 +578,7 @@ public void testCertificatesRenew() throws Exception { List renewed = Arrays.asList((X509Certificate[]) generateSampleChain(keyPair, false)); when(_cert.getCertificateChain()).thenReturn(renewed); when(ac.fetchCertificateForOrder(any())).thenReturn(_cert); - Whitebox.setInternalState(dcMan, ac); + Whitebox.setInternalState(dcMan, "acmeClients", Map.of(DEFAULT_PROVIDER_NAME, ac)); // by-name because there are other map fields // Renew dcMan.run(); @@ -630,7 +680,7 @@ public void testLocalCertificatesStoring() throws Exception { List renewed = Arrays.asList((X509Certificate[]) generateSampleChain(keyPair, false)); when(_cert.getCertificateChain()).thenReturn(renewed); when(ac.fetchCertificateForOrder(any())).thenReturn(_cert); - Whitebox.setInternalState(dcMan, ac); + Whitebox.setInternalState(dcMan, "acmeClients", Map.of(DEFAULT_PROVIDER_NAME, ac)); // by-name because there are other map fields // Renew File certsDir = tmpDir.newFolder("certs"); @@ -791,6 +841,8 @@ public void testLocalCertificatesStoring() throws Exception { when(_cert.getCertificateChain()).thenReturn(renewed); server.getCurrentConfiguration().setLocalCertificatesStorePath(certsDir.getAbsolutePath()); + // the certificate upload above reloaded the configuration, rebuilding the ACME clients: re-inject the mock + Whitebox.setInternalState(dcMan, "acmeClients", Map.of(DEFAULT_PROVIDER_NAME, ac)); dcMan.run(); updated = dcMan.getCertificateDataForDomain("localhost2"); assertNotNull(updated); diff --git a/carapace-server/src/test/java/org/carapaceproxy/server/certificates/DynamicCertificatesManagerTest.java b/carapace-server/src/test/java/org/carapaceproxy/server/certificates/DynamicCertificatesManagerTest.java index 198712875..d32c28b31 100644 --- a/carapace-server/src/test/java/org/carapaceproxy/server/certificates/DynamicCertificatesManagerTest.java +++ b/carapace-server/src/test/java/org/carapaceproxy/server/certificates/DynamicCertificatesManagerTest.java @@ -30,6 +30,7 @@ import static org.carapaceproxy.server.certificates.DynamicCertificateState.VERIFYING; import static org.carapaceproxy.server.certificates.DynamicCertificateState.WAITING; import static org.carapaceproxy.server.certificates.DynamicCertificatesManager.DEFAULT_KEYPAIRS_SIZE; +import static org.carapaceproxy.server.config.AcmeProviderConfiguration.DEFAULT_PROVIDER_NAME; import static org.carapaceproxy.utils.CertificatesTestUtils.generateSampleChain; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; @@ -86,6 +87,12 @@ @RunWith(JUnitParamsRunner.class) public class DynamicCertificatesManagerTest { + static { + // DynamicCertificatesManager reads the limit in a static final field, so the property must be set + // before the class is loaded by whatever test method happens to run first + System.setProperty("carapace.acme.dnschallengereachabilitycheck.limit", "2"); + } + protected static final int MAX_ATTEMPTS = 7; @Test @@ -162,7 +169,6 @@ public void testCertificateSimpleStateManagement(String runCase, boolean maxedOu when(parent.getListeners()).thenReturn(mock(Listeners.class)); DynamicCertificatesManager man = new DynamicCertificatesManager(parent); man.attachGroupMembershipHandler(new NullGroupMembershipHandler()); - Whitebox.setInternalState(man, ac); // Store mocking ConfigurationStore store = mock(ConfigurationStore.class); @@ -212,6 +218,8 @@ public void testCertificateSimpleStateManagement(String runCase, boolean maxedOu conf.configure(configStore); when(parent.getCurrentConfiguration()).thenReturn(conf); man.reloadConfiguration(conf); + // after the reload, so that the rebuilt client map gets replaced with the mock; by-name because there are other map fields + Whitebox.setInternalState(man, "acmeClients", Map.of(DEFAULT_PROVIDER_NAME, ac)); assertCertificateState(d0, AVAILABLE, cycleCount, man); assertCertificateState(d2, AVAILABLE, 0, man); @@ -359,7 +367,6 @@ public void testWildcardCertificateStateManagement(String runCase) throws Except when(parent.getListeners()).thenReturn(mock(Listeners.class)); DynamicCertificatesManager man = new DynamicCertificatesManager(parent); man.attachGroupMembershipHandler(new NullGroupMembershipHandler()); - Whitebox.setInternalState(man, ac); // Route53Cliente mocking man.initAWSClient("access", "secret"); @@ -390,6 +397,8 @@ public void testWildcardCertificateStateManagement(String runCase) throws Except conf.configure(configStore); when(parent.getCurrentConfiguration()).thenReturn(conf); man.reloadConfiguration(conf); + // after the reload, so that the rebuilt client map gets replaced with the mock; by-name because there are other map fields + Whitebox.setInternalState(man, "acmeClients", Map.of(DEFAULT_PROVIDER_NAME, ac)); int saveCounter = 0; // at every run the certificate has to be saved to the db (whether not AVAILABLE). @@ -498,7 +507,6 @@ public void testSanCertificateStateManagement(String runCase) throws Exception { when(parent.getListeners()).thenReturn(mock(Listeners.class)); DynamicCertificatesManager man = new DynamicCertificatesManager(parent); man.attachGroupMembershipHandler(new NullGroupMembershipHandler()); - Whitebox.setInternalState(man, ac); // Route53Cliente mocking man.initAWSClient("access", "secret"); @@ -530,6 +538,8 @@ public void testSanCertificateStateManagement(String runCase) throws Exception { conf.configure(configStore); when(parent.getCurrentConfiguration()).thenReturn(conf); man.reloadConfiguration(conf); + // after the reload, so that the rebuilt client map gets replaced with the mock; by-name because there are other map fields + Whitebox.setInternalState(man, "acmeClients", Map.of(DEFAULT_PROVIDER_NAME, ac)); int saveCounter = 0; // at every run the certificate has to be saved to the db (whether not AVAILABLE). @@ -615,7 +625,6 @@ public void testDomainReachabilityCheck(String domainCase) throws Exception { when(parent.getListeners()).thenReturn(mock(Listeners.class)); DynamicCertificatesManager man = new DynamicCertificatesManager(parent); man.attachGroupMembershipHandler(new NullGroupMembershipHandler()); - Whitebox.setInternalState(man, ac); // Store mocking ConfigurationStore store = mock(ConfigurationStore.class); @@ -641,6 +650,8 @@ public void testDomainReachabilityCheck(String domainCase) throws Exception { conf.configure(configStore); when(parent.getCurrentConfiguration()).thenReturn(conf); man.reloadConfiguration(conf); + // after the reload, so that the rebuilt client map gets replaced with the mock; by-name because there are other map fields + Whitebox.setInternalState(man, "acmeClients", Map.of(DEFAULT_PROVIDER_NAME, ac)); int saveCounter = 0; // at every run the certificate has to be saved to the db (whether not AVAILABLE). @@ -661,4 +672,57 @@ public void testDomainReachabilityCheck(String domainCase) throws Exception { } } + @Test + public void testCertificateProviderRouting() throws Exception { + // one mocked ACME client per provider + ACMEClient letsencryptClient = mock(ACMEClient.class); + ACMEClient customClient = mock(ACMEClient.class); + Order o = mock(Order.class); + when(o.getLocation()).thenReturn(URI.create("https://localhost/index").toURL()); + for (ACMEClient ac : List.of(letsencryptClient, customClient)) { + Login login = mock(Login.class); + when(login.bindOrder(any())).thenReturn(o); + when(ac.getLogin()).thenReturn(login); + when(ac.createOrderForDomain(any())).thenReturn(o); + when(ac.getChallengesForOrder(any())).thenReturn(Collections.emptyMap()); + } + + HttpProxyServer parent = mock(HttpProxyServer.class); + when(parent.getListeners()).thenReturn(mock(Listeners.class)); + DynamicCertificatesManager man = new DynamicCertificatesManager(parent); + man.attachGroupMembershipHandler(new NullGroupMembershipHandler()); + + // Store mocking + ConfigurationStore store = mock(ConfigurationStore.class); + KeyPair keyPair = KeyPairUtils.createKeyPair(DEFAULT_KEYPAIRS_SIZE); + when(store.loadKeyPairForDomain(anyString())).thenReturn(keyPair); + when(store.loadCertificateForDomain(eq("le.local"))).thenReturn(new CertificateData("le.local", null, WAITING)); + when(store.loadCertificateForDomain(eq("custom.local"))).thenReturn(new CertificateData("custom.local", null, WAITING)); + man.setConfigurationStore(store); + + // Manager setup: one certificate on the default provider, one on the custom one + Properties props = new Properties(); + props.setProperty("acme.1.name", "custom"); + props.setProperty("acme.1.url", "https://acme.example.com/directory"); + props.setProperty("certificate.0.hostname", "le.local"); + props.setProperty("certificate.0.mode", "acme"); + props.setProperty("certificate.1.hostname", "custom.local"); + props.setProperty("certificate.1.mode", "acme"); + props.setProperty("certificate.1.provider", "custom"); + RuntimeServerConfiguration conf = new RuntimeServerConfiguration(); + conf.configure(new PropertiesConfigurationStore(props)); + when(parent.getCurrentConfiguration()).thenReturn(conf); + man.reloadConfiguration(conf); + // after the reload, so that the rebuilt client map gets replaced with the mocks; by-name because there are other map fields + Whitebox.setInternalState(man, "acmeClients", Map.of( + DEFAULT_PROVIDER_NAME, letsencryptClient, + "custom", customClient + )); + + man.run(); // both certificates WAITING > order created on each provider's client + + verify(letsencryptClient, times(1)).createOrderForDomain(eq(new CertificateData("le.local", null, WAITING).getNames())); + verify(customClient, times(1)).createOrderForDomain(eq(new CertificateData("custom.local", null, WAITING).getNames())); + } + } diff --git a/carapace-ui/src/main/webapp/src/components/certificates/Certificate.vue b/carapace-ui/src/main/webapp/src/components/certificates/Certificate.vue index a2451db35..6139fab24 100644 --- a/carapace-ui/src/main/webapp/src/components/certificates/Certificate.vue +++ b/carapace-ui/src/main/webapp/src/components/certificates/Certificate.vue @@ -17,6 +17,7 @@

  • Hostname: {{certificate.hostname}}
  • Subject Alternative Names: {{certificate.subjectAltNames}}
  • Mode: {{certificate.mode}}
    • +
  • ACME provider: {{certificate.provider}}
  • Dynamic: {{certificate.dynamic | symbolFormat}}
  • diff --git a/carapace-ui/src/main/webapp/src/components/certificates/CertificateForm.vue b/carapace-ui/src/main/webapp/src/components/certificates/CertificateForm.vue index 46db30ddb..46ab40948 100644 --- a/carapace-ui/src/main/webapp/src/components/certificates/CertificateForm.vue +++ b/carapace-ui/src/main/webapp/src/components/certificates/CertificateForm.vue @@ -64,6 +64,23 @@ > + + + + import {doPost} from "../../serverapi"; import StatusBox from "../StatusBox.vue"; + + // must match the backend built-in provider name (AcmeProviderConfiguration.DEFAULT_PROVIDER_NAME) + const DEFAULT_PROVIDER = 'letsencrypt'; + export default { name: "CertificateForm", components: { "status-box": StatusBox }, props: { - id: String + id: String, + acmeProviders: { + type: Array, + default: () => [DEFAULT_PROVIDER] + } }, data() { return { @@ -106,7 +131,8 @@ import {doPost} from "../../serverapi"; domain: '', subjectAltNames: [], type: 'acme', - daysBeforeRenewal: 30 + daysBeforeRenewal: 30, + provider: DEFAULT_PROVIDER }, globalError: null, showOverlay: false @@ -117,6 +143,9 @@ import {doPost} from "../../serverapi"; return [ {value: 'acme', text: 'Acme'} ] + }, + providers() { + return this.acmeProviders.map(name => ({value: name, text: name})) } }, methods: { @@ -125,6 +154,7 @@ import {doPost} from "../../serverapi"; this.form.subjectAltNames = [] this.form.type = 'acme' this.form.daysBeforeRenewal = 30 + this.form.provider = DEFAULT_PROVIDER this.clearFormErrors() }, clearFormErrors() { diff --git a/carapace-ui/src/main/webapp/src/components/certificates/Certificates.vue b/carapace-ui/src/main/webapp/src/components/certificates/Certificates.vue index 4823f56e3..39a8178d3 100644 --- a/carapace-ui/src/main/webapp/src/components/certificates/Certificates.vue +++ b/carapace-ui/src/main/webapp/src/components/certificates/Certificates.vue @@ -15,7 +15,7 @@ Create certificate
    - + @@ -67,6 +67,9 @@ import {toBooleanSymbol} from '../../lib/formatter'; import CertificateForm from './CertificateForm.vue'; import StatusBox from '../StatusBox.vue'; +// must match the backend built-in provider name (AcmeProviderConfiguration.DEFAULT_PROVIDER_NAME) +const DEFAULT_PROVIDER = 'letsencrypt'; + /** * Certificate status options for dynamic SSL certificates. * @@ -79,6 +82,7 @@ import StatusBox from '../StatusBox.vue'; * @property {string} hostname - The hostname the certificate is issued for. * @property {string[]} subjectAltNames - Array of Subject Alternative Names (SANs). * @property {string} mode - The mode of the certificate. + * @property {string} provider - The ACME provider name (ACME certificates only). * @property {boolean} dynamic - Indicates if the certificate is dynamic. * @property {CertificateStatus} status - The current status of the certificate. * @property {number} attemptsCount - Number of attempts made to renew the certificate. @@ -108,6 +112,11 @@ export default { */ certificates: [], localStorePath: null, + /** + * Names of the configured ACME providers. + * @type {string[]} + */ + acmeProviders: [DEFAULT_PROVIDER], loading: true, opSuccess: null, opMessage: '', @@ -131,6 +140,7 @@ export default { { key: "hostname", label: "Hostname", sortable: true }, { key: "subjectAltNames", label: "SANs", sortable: true }, { key: "mode", label: "Mode", sortable: true }, + { key: "provider", label: "Provider", sortable: true }, { key: "dynamic", label: "Dynamic", sortable: true, formatter: toBooleanSymbol }, { key: "status", label: "Status", sortable: true }, { key: "attemptsCount", label: "Attempts count", sortable: true }, @@ -182,6 +192,7 @@ export default { doGet("/api/certificates", data => { this.certificates = data.certificates || []; this.localStorePath = data.localStorePath; + this.acmeProviders = data.acmeProviders || [DEFAULT_PROVIDER]; this.loading = false; }); }, From 23535fb04c3a1eb6542e9ac4838be70b5523b7f6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Niccol=C3=B2=20Maltoni?= Date: Thu, 23 Jul 2026 15:26:29 +0200 Subject: [PATCH 09/10] fix: harden ACME failure handling in renewal loop CA rejections (stale orders, malformed responses) bump the cert attempts and fall back to a fresh order; network errors, rate limits and CA outages retry in the same state. Order polls fetch once per cycle. Policy lives in AcmeFailureClassifier with fast pure unit tests replacing five heavy harness runs. --- .../configstore/HerdDBConfigurationStore.java | 26 ++++-- .../core/RuntimeServerConfiguration.java | 20 +++-- .../server/certificates/ACMEClient.java | 42 ++++++---- .../certificates/AcmeFailureClassifier.java | 73 +++++++++++++++++ .../DynamicCertificatesManager.java | 53 ++++++++---- .../config/AcmeProviderConfiguration.java | 19 ++++- .../resources/conf/server.dynamic.properties | 4 +- .../configstore/ConfigurationStoreTest.java | 12 ++- .../core/RuntimeServerConfigurationTest.java | 2 + .../server/certificates/ACMEClientTest.java | 24 ++---- .../AcmeFailureClassifierTest.java | 74 +++++++++++++++++ .../server/certificates/CertificatesTest.java | 37 +++++---- .../DynamicCertificatesManagerTest.java | 82 ++++++++++++++++--- .../components/certificates/Certificate.vue | 4 +- .../certificates/CertificateForm.vue | 19 +++-- .../components/certificates/Certificates.vue | 9 +- 16 files changed, 387 insertions(+), 113 deletions(-) create mode 100644 carapace-server/src/main/java/org/carapaceproxy/server/certificates/AcmeFailureClassifier.java create mode 100644 carapace-server/src/test/java/org/carapaceproxy/server/certificates/AcmeFailureClassifierTest.java diff --git a/carapace-server/src/main/java/org/carapaceproxy/configstore/HerdDBConfigurationStore.java b/carapace-server/src/main/java/org/carapaceproxy/configstore/HerdDBConfigurationStore.java index 68badbecc..6e5a3e3e9 100644 --- a/carapace-server/src/main/java/org/carapaceproxy/configstore/HerdDBConfigurationStore.java +++ b/carapace-server/src/main/java/org/carapaceproxy/configstore/HerdDBConfigurationStore.java @@ -344,7 +344,7 @@ public boolean saveAcmeUserKey(KeyPair pair, String providerName) { try { return saveKeyPair(pair, acmeUserKeyName(providerName), false); } catch (Exception err) { - LOG.error("Error while performing KeyPar saving for ACME user of provider {}.", providerName, err); + LOG.error("Error while performing KeyPair saving for ACME user of provider {}.", providerName, err); throw new ConfigurationStoreException(err); } } @@ -352,6 +352,12 @@ public boolean saveAcmeUserKey(KeyPair pair, String providerName) { /** * The account key of the built-in provider keeps the legacy {@link #ACME_USER_KEY} name, * so existing Let's Encrypt accounts survive the upgrade; other providers get a dedicated key. + *

    + * Renaming a provider intentionally registers a fresh ACME account under the new name; + * the row of the old one stays around, unused but harmless. + * + * @param providerName the name of the ACME provider + * @return the primary key of the provider account key pair in the keypairs table */ private static String acmeUserKeyName(String providerName) { return AcmeProviderConfiguration.DEFAULT_PROVIDER_NAME.equals(providerName) @@ -359,10 +365,20 @@ private static String acmeUserKeyName(String providerName) { : ACME_USER_KEY + "_" + providerName; } + /** + * Whether the primary key belongs to a provider account key pair, hence off-limits for domain key pairs. + * + * @param pk a primary key of the key pairs table + * @return true if it is an {@link #acmeUserKeyName(String) account key name} + */ + private static boolean isAcmeUserKey(String pk) { + return pk.equals(ACME_USER_KEY) || pk.startsWith(ACME_USER_KEY + "_"); + } + @Override public KeyPair loadKeyPairForDomain(String domain) { try { - if (domain.startsWith(ACME_USER_KEY)) { + if (isAcmeUserKey(domain)) { return null; } return loadKeyPair(domain); @@ -375,11 +391,11 @@ public KeyPair loadKeyPairForDomain(String domain) { @Override public boolean saveKeyPairForDomain(KeyPair pair, String domain, boolean update) { try { - if (!domain.startsWith(ACME_USER_KEY)) { + if (!isAcmeUserKey(domain)) { return saveKeyPair(pair, domain, update); } } catch (Exception err) { - LOG.error("Error while performing KeyPar saving for domain {}.", domain, err); + LOG.error("Error while performing KeyPair saving for domain {}.", domain, err); throw new ConfigurationStoreException(err); } return false; @@ -425,7 +441,7 @@ private boolean saveKeyPair(KeyPair pair, String pk, boolean update) { @Override public CertificateData loadCertificateForDomain(String domain) { - if (domain.startsWith(ACME_USER_KEY)) { + if (isAcmeUserKey(domain)) { return null; } try (Connection con = datasource.getConnection()) { diff --git a/carapace-server/src/main/java/org/carapaceproxy/core/RuntimeServerConfiguration.java b/carapace-server/src/main/java/org/carapaceproxy/core/RuntimeServerConfiguration.java index 0e593fbde..ebbb71821 100644 --- a/carapace-server/src/main/java/org/carapaceproxy/core/RuntimeServerConfiguration.java +++ b/carapace-server/src/main/java/org/carapaceproxy/core/RuntimeServerConfiguration.java @@ -330,12 +330,12 @@ private void configureAcmeProviders(ConfigurationStore properties) throws Config } if (DEFAULT_PROVIDER_NAME.equals(name)) { throw new ConfigurationNotValidException( - "Invalid value '" + name + "' for " + prefix + "name: it is the built-in provider and cannot be redefined" + "Invalid value '" + name + "' for " + prefix + "name: reserved for the built-in provider" ); } if (!name.matches("[a-z0-9][a-z0-9_-]*")) { throw new ConfigurationNotValidException( - "Invalid value '" + name + "' for " + prefix + "name: only lowercase letters, digits, '-' and '_' are allowed" + "Invalid value '" + name + "' for " + prefix + "name: must match [a-z0-9][a-z0-9_-]*" ); } if (acmeProviders.containsKey(name)) { @@ -355,10 +355,10 @@ private void configureAcmeProviders(ConfigurationStore properties) throws Config final var scheme = new URI(url).getScheme(); if (!"https".equals(scheme) && !"acme".equals(scheme)) { throw new ConfigurationNotValidException( - "Invalid value '" + url + "' for " + prefix + "url: scheme must be https or acme (lowercase)" + "Invalid value '" + url + "' for " + prefix + "url: scheme must be https or acme" ); } - // resolves the acme: provider via ServiceLoader and validates the URL, without network I/O + // fail now on unknown acme: providers or invalid URLs; resolution does no network I/O new Session(url); } catch (URISyntaxException | IllegalArgumentException e) { throw new ConfigurationNotValidException( @@ -375,17 +375,18 @@ private void configureAcmeProviders(ConfigurationStore properties) throws Config } if (!hmac.isEmpty()) { try { - // acme4j expects the MAC key base64url-encoded, fail at configuration time instead of at account login + // acme4j expects base64url: fail at configuration time instead of at account login Base64.getUrlDecoder().decode(hmac); } catch (IllegalArgumentException e) { throw new ConfigurationNotValidException( - "Invalid ACME provider configuration " + prefix + ": hmac is not a valid base64url-encoded key" + "Invalid ACME provider configuration " + prefix + ": hmac is not valid base64url" ); } } final var provider = new AcmeProviderConfiguration(name, url, kid, hmac); acmeProviders.put(name, provider); - LOG.info("Configured ACME provider acme.{}: name={}, url={}, external account binding: {}", i, name, url, provider.hasExternalAccountBinding()); + LOG.info("Configured ACME provider acme.{}: name={}, url={}, external account binding: {}", + i, name, url, provider.hasExternalAccountBinding()); } } @@ -403,11 +404,12 @@ private void configureCertificates(ConfigurationStore properties) throws Configu final var provider = properties.getString(prefix + "provider", DEFAULT_PROVIDER_NAME); if (!DEFAULT_PROVIDER_NAME.equals(provider) && !acmeProviders.containsKey(provider)) { throw new ConfigurationNotValidException( - "Invalid value '" + provider + "' for " + prefix + "provider. No ACME provider configured with such name" + "Invalid value '" + provider + "' for " + prefix + "provider: no such ACME provider" ); } try { - final var config = new SSLCertificateConfiguration(hostname, subjectAltNames, file, pw, CertificateMode.valueOf(mode.toUpperCase()), provider); + final var config = new SSLCertificateConfiguration( + hostname, subjectAltNames, file, pw, CertificateMode.valueOf(mode.toUpperCase()), provider); if (config.isAcme()) { config.setDaysBeforeRenewal(daysBeforeRenewal); } diff --git a/carapace-server/src/main/java/org/carapaceproxy/server/certificates/ACMEClient.java b/carapace-server/src/main/java/org/carapaceproxy/server/certificates/ACMEClient.java index 7e77f6338..241371185 100644 --- a/carapace-server/src/main/java/org/carapaceproxy/server/certificates/ACMEClient.java +++ b/carapace-server/src/main/java/org/carapaceproxy/server/certificates/ACMEClient.java @@ -20,7 +20,7 @@ package org.carapaceproxy.server.certificates; import java.io.IOException; -import java.net.URI; +import java.net.URL; import java.security.KeyPair; import java.security.Security; import java.util.Collection; @@ -87,9 +87,13 @@ public ACMEClient(KeyPair userKey, boolean testingMode) { * @param userKey the ACME account key pair * @param directoryUrl the directory URL of the CA * @param kid the key identifier for External Account Binding, or null if the CA doesn't require it - * @param hmac the base64-encoded MAC key for External Account Binding, or null if the CA doesn't require it + * @param hmac the base64url-encoded MAC key for External Account Binding, or null if the CA doesn't require it + * @throws IllegalArgumentException if only one of {@code kid} and {@code hmac} is provided */ public ACMEClient(KeyPair userKey, String directoryUrl, String kid, String hmac) { + if ((kid == null || kid.isEmpty()) != (hmac == null || hmac.isEmpty())) { + throw new IllegalArgumentException("kid and hmac must be provided together (external account binding)"); + } Security.addProvider(new BouncyCastleProvider()); this.userKey = userKey; this.directoryUrl = directoryUrl; @@ -98,12 +102,16 @@ public ACMEClient(KeyPair userKey, String directoryUrl, String kid, String hmac) } /** - * Finds your {@link Account} at the ACME server.It will be found by your user's public key.If your key is not known to the server yet, a new account will be created.

    - * This is a simple way of finding your {@link Account}. A better way is to get the URL and KeyIdentifier of your new account with {@link Account#getLocation()} - * {@link Session#getKeyIdentifier()} and store it somewhere. If you need to get access to your account later, reconnect to it via {@link Account#bind(Session, URI)} by using the stored location. + * Finds your {@link Account} at the ACME server. It will be found by your user's public key. + * If your key is not known to the server yet, a new account will be created. + *

    + * This is a simple way of finding your {@link Account}. A better way is to get the URL of your + * new account with {@link Account#getLocation()} and store it somewhere. If you need to get + * access to your account later, reconnect to it via {@link Session#login(URL, KeyPair)} + * by using the stored location. * - * @return - * @throws org.shredzone.acme4j.exception.AcmeException + * @return the {@link Login} bound to the account + * @throws AcmeException if the account cannot be found or created */ public Login getLogin() throws AcmeException { final var accountBuilder = new AccountBuilder() @@ -223,24 +231,22 @@ public void orderCertificate(Order order, KeyPair domainKeyPair) throws IOExcept } public Status checkResponseForOrder(Order order) throws AcmeException { - LOG.info("Certificate order checking for domain {}", order.getIdentifiers().get(0).getDomain()); + // fetch eagerly: on a freshly bound order any accessor would lazy-fetch anyway, + // so this guarantees a single server round-trip per poll + order.fetch(); + LOG.info("Certificate order checking for domain {}", order.getIdentifiers().getFirst().getDomain()); Status status = order.getStatus(); if (status == Status.VALID) { LOG.info("Order has been completed."); - return status; } - if (status != Status.INVALID) { - order.fetch(); - } - return order.getStatus(); + return status; } - public Certificate fetchCertificateForOrder(Order order) throws AcmeException { + public Certificate fetchCertificateForOrder(Order order) { + // getCertificate() itself throws if the order is not ready Certificate certificate = order.getCertificate(); - if (certificate == null) { - throw new AcmeException("Certificate not fetched"); - } - LOG.info("Success! The certificate for domain {} has been generated!", order.getIdentifiers().get(0).getDomain()); + LOG.info("Success! The certificate for domain {} has been generated!", + order.getIdentifiers().getFirst().getDomain()); LOG.info("Certificate URL: {}", certificate.getLocation()); return certificate; } diff --git a/carapace-server/src/main/java/org/carapaceproxy/server/certificates/AcmeFailureClassifier.java b/carapace-server/src/main/java/org/carapaceproxy/server/certificates/AcmeFailureClassifier.java new file mode 100644 index 000000000..838178484 --- /dev/null +++ b/carapace-server/src/main/java/org/carapaceproxy/server/certificates/AcmeFailureClassifier.java @@ -0,0 +1,73 @@ +/* + Licensed to Diennea S.r.l. under one + or more contributor license agreements. See the NOTICE file + distributed with this work for additional information + regarding copyright ownership. Diennea S.r.l. licenses this file + to you under the Apache License, Version 2.0 (the + "License"); you may not use this file except in compliance + with the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, + software distributed under the License is distributed on an + "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + KIND, either express or implied. See the License for the + specific language governing permissions and limitations + under the License. + + */ +package org.carapaceproxy.server.certificates; + +import java.net.URI; +import java.util.Set; +import java.util.regex.Pattern; +import org.shredzone.acme4j.exception.AcmeException; +import org.shredzone.acme4j.exception.AcmeLazyLoadingException; +import org.shredzone.acme4j.exception.AcmeNetworkException; +import org.shredzone.acme4j.exception.AcmeProtocolException; +import org.shredzone.acme4j.exception.AcmeRateLimitedException; +import org.shredzone.acme4j.exception.AcmeServerException; + +/** + * Classification of the failures raised while talking to an ACME CA. + */ +final class AcmeFailureClassifier { + + private static final Set TRANSIENT_ACME_PROBLEMS = Set.of( + URI.create("urn:ietf:params:acme:error:serverInternal"), // CA outage + URI.create("urn:ietf:params:acme:error:badNonce") // escaped acme4j's own retry loop + ); + // matches the bare AcmeException("HTTP 503") thrown by acme4j (as of 5.1.0) on 5xx without a problem document + private static final Pattern HTTP_SERVER_ERROR = Pattern.compile("\\bHTTP 5\\d\\d\\b"); + + private AcmeFailureClassifier() { + } + + /** + * Whether the CA actively rejected the request, + * as opposed to a transient failure (network error, rate limiting, CA outage) worth retrying in the same state. + *

    + * A rejection means the persisted certificate state is no longer usable, + * e.g. a pending order belonging to a different CA after a provider change, + * so the failure has to be {@link CertificateData#error(String) counted on the certificate} + * for it to fall back to a fresh order. + * acme4j may defer the server round-trip of bound resources, wrapping failures in {@link AcmeLazyLoadingException}. + * + * @param ex the failure raised while processing a certificate + * @return true if the CA rejected the request + */ + static boolean isRejectedByProvider(Exception ex) { + final var cause = ex instanceof AcmeLazyLoadingException lazy ? lazy.getCause() : ex; + return switch (cause) { + case AcmeNetworkException ignored -> false; + case AcmeRateLimitedException ignored -> false; + case AcmeServerException server -> !TRANSIENT_ACME_PROBLEMS.contains(server.getType()); + // a 5xx without a problem document (e.g., a CA outage) surfaces as a bare exception; + // an unrecognizable message counts as rejection: worst case is a fresh order instead of being stuck forever + case AcmeException e -> e.getMessage() == null || !HTTP_SERVER_ERROR.matcher(e.getMessage()).find(); + case AcmeProtocolException ignored -> true; // malformed CA response, retrying it won't help + default -> false; + }; + } +} diff --git a/carapace-server/src/main/java/org/carapaceproxy/server/certificates/DynamicCertificatesManager.java b/carapace-server/src/main/java/org/carapaceproxy/server/certificates/DynamicCertificatesManager.java index 4b49bf861..171fa93ad 100644 --- a/carapace-server/src/main/java/org/carapaceproxy/server/certificates/DynamicCertificatesManager.java +++ b/carapace-server/src/main/java/org/carapaceproxy/server/certificates/DynamicCertificatesManager.java @@ -107,7 +107,7 @@ public class DynamicCertificatesManager implements Runnable { private static final String EVENT_CERTIFICATES_REQUEST_STORE = "certificates_request_store"; private volatile Map certificates = new ConcurrentHashMap<>(); - private volatile Map acmeClients = Map.of(); // one client per ACME provider, keyed by provider name + private volatile Map acmeClients = Map.of(); private Route53Client r53Client; private String awsAccessKey; private String awsSecretKey; @@ -173,7 +173,8 @@ public synchronized void reloadConfiguration(RuntimeServerConfiguration configur keyPairsSize = configuration.getKeyPairsSize(); // rebuilt on every reload, so that changes to provider url/kid/hmac take effect final var clients = new HashMap(); - clients.put(DEFAULT_PROVIDER_NAME, new ACMEClient(loadOrCreateAcmeUserKeyPair(DEFAULT_PROVIDER_NAME), TESTING_MODE)); + clients.put(DEFAULT_PROVIDER_NAME, + new ACMEClient(loadOrCreateAcmeUserKeyPair(DEFAULT_PROVIDER_NAME), TESTING_MODE)); for (final var provider : configuration.getAcmeProviders().values()) { clients.put(provider.name(), new ACMEClient( loadOrCreateAcmeUserKeyPair(provider.name()), provider.url(), provider.kid(), provider.hmac() @@ -203,6 +204,13 @@ private KeyPair loadOrCreateAcmeUserKeyPair(String providerName) { return pair; } + /** + * Resolve the ACME client of the provider the certificate is configured with. + * + * @param cert the certificate being processed + * @return the client for the certificate provider + * @throws IllegalStateException if no client exists for the provider + */ private ACMEClient acmeClientFor(CertificateData cert) { final var client = acmeClients.get(cert.getProvider()); if (client == null) { @@ -225,7 +233,8 @@ private void loadCertificates(Map certifica } boolean forceManual = MANUAL == config.getMode(); _certificates.put(domain, loadOrCreateDynamicCertificateForDomain( - domain, config.getSubjectAltNames(), forceManual, config.getDaysBeforeRenewal(), config.getProvider() + domain, config.getSubjectAltNames(), forceManual, + config.getDaysBeforeRenewal(), config.getProvider() )); } } @@ -239,7 +248,8 @@ private CertificateData loadOrCreateDynamicCertificateForDomain(String domain, Set subjectAltNames, boolean forceManual, int daysBeforeRenewal, - String provider) throws GeneralSecurityException, MalformedURLException { + String provider) + throws GeneralSecurityException, MalformedURLException { CertificateData cert = store.loadCertificateForDomain(domain); if (cert == null) { cert = new CertificateData(domain, subjectAltNames, null, WAITING, null, null); @@ -308,9 +318,11 @@ private void certificatesLifecycle() { for (CertificateData data : _certificates) { var updateCertificate = true; final var domain = data.getDomain(); + CertificateData cert = null; try { // this has to be always fetch from db! - CertificateData cert = loadOrCreateDynamicCertificateForDomain(domain, data.getSubjectAltNames(), false, data.getDaysBeforeRenewal(), data.getProvider()); + cert = loadOrCreateDynamicCertificateForDomain( + domain, data.getSubjectAltNames(), false, data.getDaysBeforeRenewal(), data.getProvider()); switch (cert.getState()) { // certificate waiting to be issues/renew case WAITING -> startCertificateProcessing(domain, cert); @@ -327,17 +339,12 @@ private void certificatesLifecycle() { // challenge succeeded case VERIFIED -> { LOG.info("Certificate for domain {} VERIFIED.", domain); - Order pendingOrder = acmeClientFor(cert).getLogin().bindOrder(cert.getPendingOrderLocation()); + final var acmeClient = acmeClientFor(cert); + Order pendingOrder = acmeClient.getLogin().bindOrder(cert.getPendingOrderLocation()); // if the order is already valid, we have to skip finalization if (pendingOrder.getStatus() != Status.VALID) { - try { - KeyPair keys = loadOrCreateKeyPairForDomain(domain); - acmeClientFor(cert).orderCertificate(pendingOrder, keys); - } catch (AcmeException ex) { // order finalization failed - LOG.error("Certificate order finalization for domain {} FAILED.", domain, ex); - cert.error(ex.getMessage()); - break; - } + KeyPair keys = loadOrCreateKeyPairForDomain(domain); + acmeClient.orderCertificate(pendingOrder, keys); } cert.step(ORDERING); } @@ -389,6 +396,16 @@ private void certificatesLifecycle() { // RuntimeException included on purpose, as an escaped one would silently cancel the scheduled task; // this would kill the renewal loop for every certificate LOG.error("Error while handling dynamic certificate for domain {}", domain, ex); + if (cert != null && AcmeFailureClassifier.isRejectedByProvider(ex)) { + cert.error(ex.getMessage() != null ? ex.getMessage() : ex.toString()); + try { + store.saveCertificate(cert); + flushCache = true; + } catch (RuntimeException storeEx) { + // same rationale as above: a db hiccup must not cancel the scheduled task + LOG.error("Error while saving failed certificate for domain {}", domain, storeEx); + } + } } } if (flushCache) { @@ -546,7 +563,7 @@ private void checkChallengesResponsesForCertificate(CertificateData cert) throws final var domain = c.getKey(); final var challenge = c.getValue(); LOG.info("VERIFYING challenge response for domain {}: {}", domain, challenge); - final var status = acmeClientFor(cert).checkResponseForChallenge(challenge); // checks response and updates the challenge + final var status = acmeClientFor(cert).checkResponseForChallenge(challenge); cert.getPendingChallengesData().put(domain, challenge.getJSON()); if (status == Status.VALID) { okCount++; @@ -717,8 +734,10 @@ private void reloadCertificatesFromDBInternal() { for (Entry entry : certificates.entrySet()) { String domain = entry.getKey(); CertificateData cert = entry.getValue(); - // "wildcard" and "manual" flags, "daysBeforeRenewal" and "provider" are not stored in db > have to be re-set from existing config - CertificateData freshCert = loadOrCreateDynamicCertificateForDomain(domain, cert.getSubjectAltNames(), cert.isManual(), cert.getDaysBeforeRenewal(), cert.getProvider()); + // "wildcard", "manual", "daysBeforeRenewal" and "provider" are not stored in db: re-set from config + CertificateData freshCert = loadOrCreateDynamicCertificateForDomain( + domain, cert.getSubjectAltNames(), cert.isManual(), + cert.getDaysBeforeRenewal(), cert.getProvider()); newCertificates.put(domain, freshCert); LOG.info("RELOADED certificate for domain {}: {}", domain, freshCert); } diff --git a/carapace-server/src/main/java/org/carapaceproxy/server/config/AcmeProviderConfiguration.java b/carapace-server/src/main/java/org/carapaceproxy/server/config/AcmeProviderConfiguration.java index 9d0fe5518..419623228 100644 --- a/carapace-server/src/main/java/org/carapaceproxy/server/config/AcmeProviderConfiguration.java +++ b/carapace-server/src/main/java/org/carapaceproxy/server/config/AcmeProviderConfiguration.java @@ -29,13 +29,30 @@ * @param name the provider name, unique across the configuration * @param url the ACME directory URL of the CA * @param kid the key identifier for External Account Binding, if the CA requires it - * @param hmac the base64-encoded MAC key for External Account Binding, if the CA requires it + * @param hmac the base64url-encoded MAC key for External Account Binding, if the CA requires it */ public record AcmeProviderConfiguration(String name, String url, String kid, String hmac) { + /** Name of the built-in Let's Encrypt provider, used when a certificate doesn't specify one. */ public static final String DEFAULT_PROVIDER_NAME = "letsencrypt"; + /** + * Whether the CA requires External Account Binding credentials. + * + * @return true if a key identifier is configured + */ public boolean hasExternalAccountBinding() { return kid != null && !kid.isEmpty(); } + + /** + * The hmac is masked: it is a secret and must not leak into logs or debug dumps. + * + * @return the record representation, with the hmac masked + */ + @Override + public String toString() { + return "AcmeProviderConfiguration[name=%s, url=%s, kid=%s, hmac=%s]" + .formatted(name, url, kid, hmac == null || hmac.isEmpty() ? hmac : "******"); + } } diff --git a/carapace-server/src/main/resources/conf/server.dynamic.properties b/carapace-server/src/main/resources/conf/server.dynamic.properties index 6a58abf2b..adfff97b9 100644 --- a/carapace-server/src/main/resources/conf/server.dynamic.properties +++ b/carapace-server/src/main/resources/conf/server.dynamic.properties @@ -18,9 +18,9 @@ certificate.1.password=testproxy # certificates use Let's Encrypt unless certificate..provider names a configured provider #acme.1.name=digicert #acme.1.url=https://acme.digicert.com/v2/acme/directory -# external account binding credentials (both or neither), provided by the CA +# external account binding credentials (both or neither), provided by the CA; the hmac key must be base64url-encoded #acme.1.kid=your-key-identifier -#acme.1.hmac=your-base64-encoded-hmac-key +#acme.1.hmac=your-base64url-encoded-hmac-key # example of a Dynamic certificate issued by a custom ACME provider #certificate.101.hostname=another.mycompany.com diff --git a/carapace-server/src/test/java/org/carapaceproxy/configstore/ConfigurationStoreTest.java b/carapace-server/src/test/java/org/carapaceproxy/configstore/ConfigurationStoreTest.java index d3dd657fb..b30594434 100644 --- a/carapace-server/src/test/java/org/carapaceproxy/configstore/ConfigurationStoreTest.java +++ b/carapace-server/src/test/java/org/carapaceproxy/configstore/ConfigurationStoreTest.java @@ -233,11 +233,14 @@ private void testKeyPairOperations() { // KeyPairs generation + saving KeyPair acmePair = KeyPairUtils.createKeyPair(DEFAULT_KEYPAIRS_SIZE); store.saveAcmeUserKey(acmePair, DEFAULT_PROVIDER_NAME); - store.saveAcmeUserKey(KeyPairUtils.createKeyPair(DEFAULT_KEYPAIRS_SIZE), DEFAULT_PROVIDER_NAME); // key not overwritten + // key isn't overwritten + store.saveAcmeUserKey(KeyPairUtils.createKeyPair(DEFAULT_KEYPAIRS_SIZE), DEFAULT_PROVIDER_NAME); + // each provider has its own account key KeyPair customProviderPair = KeyPairUtils.createKeyPair(DEFAULT_KEYPAIRS_SIZE); - store.saveAcmeUserKey(customProviderPair, "customprovider"); // each provider has its own account key - store.saveAcmeUserKey(KeyPairUtils.createKeyPair(DEFAULT_KEYPAIRS_SIZE), "customprovider"); // key not overwritten + store.saveAcmeUserKey(customProviderPair, "customprovider"); + // key isn't overwritten + store.saveAcmeUserKey(KeyPairUtils.createKeyPair(DEFAULT_KEYPAIRS_SIZE), "customprovider"); store.saveKeyPairForDomain(KeyPairUtils.createKeyPair(DEFAULT_KEYPAIRS_SIZE), d1, true); KeyPair domain1Pair = KeyPairUtils.createKeyPair(DEFAULT_KEYPAIRS_SIZE); @@ -245,7 +248,8 @@ private void testKeyPairOperations() { KeyPair domain2Pair = KeyPairUtils.createKeyPair(DEFAULT_KEYPAIRS_SIZE); store.saveKeyPairForDomain(domain2Pair, d2, false); - store.saveKeyPairForDomain(KeyPairUtils.createKeyPair(DEFAULT_KEYPAIRS_SIZE), d2, false); // key not overwritten + // key isn't overwritten + store.saveKeyPairForDomain(KeyPairUtils.createKeyPair(DEFAULT_KEYPAIRS_SIZE), d2, false); // KeyPairs loading KeyPair loadedPair = store.loadAcmeUserKeyPair(DEFAULT_PROVIDER_NAME); diff --git a/carapace-server/src/test/java/org/carapaceproxy/core/RuntimeServerConfigurationTest.java b/carapace-server/src/test/java/org/carapaceproxy/core/RuntimeServerConfigurationTest.java index e0669f131..05075d4d5 100644 --- a/carapace-server/src/test/java/org/carapaceproxy/core/RuntimeServerConfigurationTest.java +++ b/carapace-server/src/test/java/org/carapaceproxy/core/RuntimeServerConfigurationTest.java @@ -21,6 +21,7 @@ import static org.carapaceproxy.server.config.AcmeProviderConfiguration.DEFAULT_PROVIDER_NAME; import static org.hamcrest.CoreMatchers.containsString; +import static org.hamcrest.CoreMatchers.not; import static org.hamcrest.MatcherAssert.assertThat; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; @@ -63,6 +64,7 @@ public void testConfigureAcmeProviders() throws Exception { assertEquals("my-kid", digicert.kid()); assertEquals("my-base64-hmac", digicert.hmac()); assertTrue(digicert.hasExternalAccountBinding()); + assertThat(digicert.toString(), not(containsString(digicert.hmac()))); // the hmac is a secret final var pebble = config.getAcmeProviders().get("pebble"); assertEquals("https://localhost:14000/dir", pebble.url()); diff --git a/carapace-server/src/test/java/org/carapaceproxy/server/certificates/ACMEClientTest.java b/carapace-server/src/test/java/org/carapaceproxy/server/certificates/ACMEClientTest.java index b16b98dd3..e46d333e7 100644 --- a/carapace-server/src/test/java/org/carapaceproxy/server/certificates/ACMEClientTest.java +++ b/carapace-server/src/test/java/org/carapaceproxy/server/certificates/ACMEClientTest.java @@ -23,6 +23,7 @@ import static org.junit.Assert.assertEquals; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; +import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import java.util.List; @@ -65,30 +66,23 @@ public void testCheckResponseForChallengeSkipsFetchWhenInvalid() throws Exceptio } @Test - public void testCheckResponseForOrderReturnsRefreshedStatus() throws Exception { - Order order = mockOrder(); - when(order.getStatus()).thenReturn(Status.PROCESSING, Status.VALID); - - assertEquals(Status.VALID, client.checkResponseForOrder(order)); - verify(order).fetch(); - } - - @Test - public void testCheckResponseForOrderSkipsFetchWhenAlreadyValid() throws Exception { + public void testCheckResponseForOrderReturnsFetchedStatus() throws Exception { Order order = mockOrder(); when(order.getStatus()).thenReturn(Status.VALID); assertEquals(Status.VALID, client.checkResponseForOrder(order)); - verify(order, never()).fetch(); + verify(order, times(1)).fetch(); } @Test - public void testCheckResponseForOrderSkipsFetchWhenInvalid() throws Exception { + public void testCheckResponseForOrderFetchesOncePerPoll() throws Exception { + // a freshly bound order has no cached state and any accessor would lazy-fetch, + // so the explicit eager fetch has to be the only server round-trip of the poll Order order = mockOrder(); - when(order.getStatus()).thenReturn(Status.INVALID); + when(order.getStatus()).thenReturn(Status.PROCESSING); - assertEquals(Status.INVALID, client.checkResponseForOrder(order)); - verify(order, never()).fetch(); + assertEquals(Status.PROCESSING, client.checkResponseForOrder(order)); + verify(order, times(1)).fetch(); } private static Order mockOrder() { diff --git a/carapace-server/src/test/java/org/carapaceproxy/server/certificates/AcmeFailureClassifierTest.java b/carapace-server/src/test/java/org/carapaceproxy/server/certificates/AcmeFailureClassifierTest.java new file mode 100644 index 000000000..5dfb1afa8 --- /dev/null +++ b/carapace-server/src/test/java/org/carapaceproxy/server/certificates/AcmeFailureClassifierTest.java @@ -0,0 +1,74 @@ +/* + * Licensed to Diennea S.r.l. under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. Diennea S.r.l. licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + * + */ +package org.carapaceproxy.server.certificates; + +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; +import java.io.IOException; +import java.net.URI; +import java.util.List; +import org.junit.Test; +import org.shredzone.acme4j.Order; +import org.shredzone.acme4j.Problem; +import org.shredzone.acme4j.exception.AcmeException; +import org.shredzone.acme4j.exception.AcmeLazyLoadingException; +import org.shredzone.acme4j.exception.AcmeNetworkException; +import org.shredzone.acme4j.exception.AcmeProtocolException; +import org.shredzone.acme4j.exception.AcmeRateLimitedException; +import org.shredzone.acme4j.exception.AcmeServerException; +import org.shredzone.acme4j.toolbox.JSON; + +public class AcmeFailureClassifierTest { + + @Test + public void testRejections() throws Exception { + // the persisted state is unusable, retrying it won't help + assertTrue(AcmeFailureClassifier.isRejectedByProvider(new AcmeException("unknown order"))); + assertTrue(AcmeFailureClassifier.isRejectedByProvider(new AcmeProtocolException("malformed CA response"))); + assertTrue(AcmeFailureClassifier.isRejectedByProvider(new AcmeLazyLoadingException( + Order.class, URI.create("https://localhost/order").toURL(), new AcmeException("unknown order")))); + assertTrue(AcmeFailureClassifier.isRejectedByProvider( + new AcmeServerException(problemOfType("urn:ietf:params:acme:error:unauthorized")))); + } + + @Test + public void testTransientFailures() throws Exception { + // worth retrying in the same state + assertFalse(AcmeFailureClassifier.isRejectedByProvider(new AcmeNetworkException(new IOException("io")))); + assertFalse(AcmeFailureClassifier.isRejectedByProvider(new AcmeRateLimitedException( + problemOfType("urn:ietf:params:acme:error:rateLimited"), null, List.of()))); + assertFalse(AcmeFailureClassifier.isRejectedByProvider( + new AcmeServerException(problemOfType("urn:ietf:params:acme:error:serverInternal")))); + assertFalse(AcmeFailureClassifier.isRejectedByProvider( + new AcmeServerException(problemOfType("urn:ietf:params:acme:error:badNonce")))); + // a 5xx without a problem document, e.g., from a load balancer + assertFalse(AcmeFailureClassifier.isRejectedByProvider(new AcmeException("HTTP 503"))); + } + + @Test + public void testNonAcmeFailures() { + // anything non-ACME is not the CA rejecting the request + assertFalse(AcmeFailureClassifier.isRejectedByProvider(new IllegalStateException("boom"))); + } + + private static Problem problemOfType(String type) throws Exception { + return new Problem(JSON.parse("{\"type\": \"" + type + "\"}"), URI.create("https://localhost/order").toURL()); + } +} diff --git a/carapace-server/src/test/java/org/carapaceproxy/server/certificates/CertificatesTest.java b/carapace-server/src/test/java/org/carapaceproxy/server/certificates/CertificatesTest.java index 874a7e476..6167adac9 100644 --- a/carapace-server/src/test/java/org/carapaceproxy/server/certificates/CertificatesTest.java +++ b/carapace-server/src/test/java/org/carapaceproxy/server/certificates/CertificatesTest.java @@ -93,6 +93,7 @@ import org.junit.runner.RunWith; import org.powermock.reflect.Whitebox; import org.shredzone.acme4j.Login; +import org.shredzone.acme4j.Order; import org.shredzone.acme4j.util.KeyPairUtils; /** @@ -421,8 +422,7 @@ public void testUploadTypedCertificatesWithDaysBeforeRenewal(String type) throws } @Test - @Parameters({"acme", "manual"}) - public void testUploadTypedCertificatesWithProvider(String type) throws Exception { + public void testUploadTypedCertificatesWithProvider() throws Exception { configureAndStartServer(); DynamicCertificatesManager dynCertsMan = server.getDynamicCertificatesManager(); KeyPair endUserKeyPair = KeyPairUtils.createKeyPair(DEFAULT_KEYPAIRS_SIZE); @@ -435,20 +435,17 @@ public void testUploadTypedCertificatesWithProvider(String type) throws Exceptio changeDynamicConfiguration(config); try (RawHttpClient client = new RawHttpClient("localhost", DEFAULT_ADMIN_PORT)) { + // provider is an ACME-only parameter + HttpResponse resp = uploadCertificate( + "localhost2", "type=manual&provider=custom", chainData, client, credentials); + assertTrue(resp.getBodyString().contains("ERROR: param 'provider' available for type 'acme' only")); + // unknown provider - HttpResponse resp = uploadCertificate("localhost2", "type=" + type + "&provider=unknown", chainData, client, credentials); - if (type.equals("manual")) { - assertTrue(resp.getBodyString().contains("ERROR: param 'provider' available for type 'acme' only")); - } else { - assertTrue(resp.getBodyString().contains("ERROR: unknown ACME provider")); - } + resp = uploadCertificate("localhost2", "type=acme&provider=unknown", chainData, client, credentials); + assertTrue(resp.getBodyString().contains("ERROR: unknown ACME provider")); // custom provider - resp = uploadCertificate("localhost2", "type=" + type + "&provider=custom", chainData, client, credentials); - if (type.equals("manual")) { - assertTrue(resp.getBodyString().contains("ERROR: param 'provider' available for type 'acme' only")); - return; - } + resp = uploadCertificate("localhost2", "type=acme&provider=custom", chainData, client, credentials); assertTrue(resp.getBodyString().contains("SUCCESS")); CertificateData data = dynCertsMan.getCertificateDataForDomain("localhost2"); assertNotNull(data); @@ -572,13 +569,16 @@ public void testCertificatesRenew() throws Exception { // ACME mocking ACMEClient ac = mock(ACMEClient.class); - when(ac.getLogin()).thenReturn(mock(Login.class)); + Login login = mock(Login.class); + when(login.bindOrder(any())).thenReturn(mock(Order.class)); + when(ac.getLogin()).thenReturn(login); when(ac.checkResponseForOrder(any())).thenReturn(VALID); org.shredzone.acme4j.Certificate _cert = mock(org.shredzone.acme4j.Certificate.class); List renewed = Arrays.asList((X509Certificate[]) generateSampleChain(keyPair, false)); when(_cert.getCertificateChain()).thenReturn(renewed); when(ac.fetchCertificateForOrder(any())).thenReturn(_cert); - Whitebox.setInternalState(dcMan, "acmeClients", Map.of(DEFAULT_PROVIDER_NAME, ac)); // by-name because there are other map fields + // by-name, because there are other map fields + Whitebox.setInternalState(dcMan, "acmeClients", Map.of(DEFAULT_PROVIDER_NAME, ac)); // Renew dcMan.run(); @@ -674,13 +674,16 @@ public void testLocalCertificatesStoring() throws Exception { // ACME mocking ACMEClient ac = mock(ACMEClient.class); - when(ac.getLogin()).thenReturn(mock(Login.class)); + Login login = mock(Login.class); + when(login.bindOrder(any())).thenReturn(mock(Order.class)); + when(ac.getLogin()).thenReturn(login); when(ac.checkResponseForOrder(any())).thenReturn(VALID); org.shredzone.acme4j.Certificate _cert = mock(org.shredzone.acme4j.Certificate.class); List renewed = Arrays.asList((X509Certificate[]) generateSampleChain(keyPair, false)); when(_cert.getCertificateChain()).thenReturn(renewed); when(ac.fetchCertificateForOrder(any())).thenReturn(_cert); - Whitebox.setInternalState(dcMan, "acmeClients", Map.of(DEFAULT_PROVIDER_NAME, ac)); // by-name because there are other map fields + // by-name, because there are other map fields + Whitebox.setInternalState(dcMan, "acmeClients", Map.of(DEFAULT_PROVIDER_NAME, ac)); // Renew File certsDir = tmpDir.newFolder("certs"); diff --git a/carapace-server/src/test/java/org/carapaceproxy/server/certificates/DynamicCertificatesManagerTest.java b/carapace-server/src/test/java/org/carapaceproxy/server/certificates/DynamicCertificatesManagerTest.java index d32c28b31..c33169acd 100644 --- a/carapace-server/src/test/java/org/carapaceproxy/server/certificates/DynamicCertificatesManagerTest.java +++ b/carapace-server/src/test/java/org/carapaceproxy/server/certificates/DynamicCertificatesManagerTest.java @@ -46,6 +46,7 @@ import static org.mockito.Mockito.when; import static org.shredzone.acme4j.Status.INVALID; import static org.shredzone.acme4j.Status.VALID; +import java.io.IOException; import java.net.URI; import java.security.KeyPair; import java.security.cert.X509Certificate; @@ -74,10 +75,11 @@ import org.shredzone.acme4j.challenge.Dns01Challenge; import org.shredzone.acme4j.challenge.Http01Challenge; import org.shredzone.acme4j.connector.Connection; +import org.shredzone.acme4j.Problem; import org.shredzone.acme4j.exception.AcmeException; +import org.shredzone.acme4j.exception.AcmeNetworkException; import org.shredzone.acme4j.toolbox.JSON; import org.shredzone.acme4j.util.KeyPairUtils; -import org.shredzone.acme4j.Problem; /** * Test for DynamicCertificatesManager. @@ -88,8 +90,9 @@ public class DynamicCertificatesManagerTest { static { - // DynamicCertificatesManager reads the limit in a static final field, so the property must be set - // before the class is loaded by whatever test method happens to run first + // DynamicCertificatesManager reads the limit into a static final field, so the property must be set + // before whatever test method happens to run first loads that class. + // No effect on other test classes: surefire runs each one in its own JVM (reuseForks=false). System.setProperty("carapace.acme.dnschallengereachabilitycheck.limit", "2"); } @@ -218,7 +221,7 @@ public void testCertificateSimpleStateManagement(String runCase, boolean maxedOu conf.configure(configStore); when(parent.getCurrentConfiguration()).thenReturn(conf); man.reloadConfiguration(conf); - // after the reload, so that the rebuilt client map gets replaced with the mock; by-name because there are other map fields + // after the reload, which rebuilds the client map; by-name, because there are other map fields Whitebox.setInternalState(man, "acmeClients", Map.of(DEFAULT_PROVIDER_NAME, ac)); assertCertificateState(d0, AVAILABLE, cycleCount, man); @@ -397,7 +400,7 @@ public void testWildcardCertificateStateManagement(String runCase) throws Except conf.configure(configStore); when(parent.getCurrentConfiguration()).thenReturn(conf); man.reloadConfiguration(conf); - // after the reload, so that the rebuilt client map gets replaced with the mock; by-name because there are other map fields + // after the reload, which rebuilds the client map; by-name, because there are other map fields Whitebox.setInternalState(man, "acmeClients", Map.of(DEFAULT_PROVIDER_NAME, ac)); int saveCounter = 0; // at every run the certificate has to be saved to the db (whether not AVAILABLE). @@ -538,7 +541,7 @@ public void testSanCertificateStateManagement(String runCase) throws Exception { conf.configure(configStore); when(parent.getCurrentConfiguration()).thenReturn(conf); man.reloadConfiguration(conf); - // after the reload, so that the rebuilt client map gets replaced with the mock; by-name because there are other map fields + // after the reload, which rebuilds the client map; by-name, because there are other map fields Whitebox.setInternalState(man, "acmeClients", Map.of(DEFAULT_PROVIDER_NAME, ac)); int saveCounter = 0; // at every run the certificate has to be saved to the db (whether not AVAILABLE). @@ -650,7 +653,7 @@ public void testDomainReachabilityCheck(String domainCase) throws Exception { conf.configure(configStore); when(parent.getCurrentConfiguration()).thenReturn(conf); man.reloadConfiguration(conf); - // after the reload, so that the rebuilt client map gets replaced with the mock; by-name because there are other map fields + // after the reload, which rebuilds the client map; by-name, because there are other map fields Whitebox.setInternalState(man, "acmeClients", Map.of(DEFAULT_PROVIDER_NAME, ac)); int saveCounter = 0; // at every run the certificate has to be saved to the db (whether not AVAILABLE). @@ -697,7 +700,8 @@ public void testCertificateProviderRouting() throws Exception { KeyPair keyPair = KeyPairUtils.createKeyPair(DEFAULT_KEYPAIRS_SIZE); when(store.loadKeyPairForDomain(anyString())).thenReturn(keyPair); when(store.loadCertificateForDomain(eq("le.local"))).thenReturn(new CertificateData("le.local", null, WAITING)); - when(store.loadCertificateForDomain(eq("custom.local"))).thenReturn(new CertificateData("custom.local", null, WAITING)); + when(store.loadCertificateForDomain(eq("custom.local"))) + .thenReturn(new CertificateData("custom.local", null, WAITING)); man.setConfigurationStore(store); // Manager setup: one certificate on the default provider, one on the custom one @@ -713,7 +717,7 @@ public void testCertificateProviderRouting() throws Exception { conf.configure(new PropertiesConfigurationStore(props)); when(parent.getCurrentConfiguration()).thenReturn(conf); man.reloadConfiguration(conf); - // after the reload, so that the rebuilt client map gets replaced with the mocks; by-name because there are other map fields + // after the reload, which rebuilds the client map; by-name, because there are other map fields Whitebox.setInternalState(man, "acmeClients", Map.of( DEFAULT_PROVIDER_NAME, letsencryptClient, "custom", customClient @@ -721,8 +725,64 @@ public void testCertificateProviderRouting() throws Exception { man.run(); // both certificates WAITING > order created on each provider's client - verify(letsencryptClient, times(1)).createOrderForDomain(eq(new CertificateData("le.local", null, WAITING).getNames())); - verify(customClient, times(1)).createOrderForDomain(eq(new CertificateData("custom.local", null, WAITING).getNames())); + verify(letsencryptClient, times(1)) + .createOrderForDomain(eq(new CertificateData("le.local", null, WAITING).getNames())); + verify(customClient, times(1)) + .createOrderForDomain(eq(new CertificateData("custom.local", null, WAITING).getNames())); + } + + @Test + @Parameters({"stale", "transient"}) + public void testPendingOrderPollFailure(String failureCase) throws Exception { + // ACME mocking: the pending order cannot be polled back from the CA + ACMEClient ac = mock(ACMEClient.class); + Login login = mock(Login.class); + Order order = mock(Order.class); + when(login.bindOrder(any())).thenReturn(order); + when(ac.getLogin()).thenReturn(login); + doThrow(failureCase.equals("transient") + ? new AcmeNetworkException(new IOException("connection reset")) + // e.g., the order belongs to a different CA after a provider change + : new AcmeException("unknown order") + ).when(ac).checkResponseForOrder(any()); + + HttpProxyServer parent = mock(HttpProxyServer.class); + when(parent.getListeners()).thenReturn(mock(Listeners.class)); + DynamicCertificatesManager man = new DynamicCertificatesManager(parent); + man.attachGroupMembershipHandler(new NullGroupMembershipHandler()); + + // Store mocking + ConfigurationStore store = mock(ConfigurationStore.class); + String domain = "localhost"; + CertificateData cd = new CertificateData(domain, null, ORDERING); + cd.setPendingOrderLocation(URI.create("https://old-ca.example.com/order/1").toURL()); + when(store.loadCertificateForDomain(eq(domain))).thenReturn(cd); + man.setConfigurationStore(store); + + // Manager setup + Properties props = new Properties(); + props.setProperty("certificate.1.hostname", domain); + props.setProperty("certificate.1.mode", "acme"); + props.setProperty("dynamiccertificatesmanager.errors.maxattempts", String.valueOf(MAX_ATTEMPTS)); + RuntimeServerConfiguration conf = new RuntimeServerConfiguration(); + conf.configure(new PropertiesConfigurationStore(props)); + when(parent.getCurrentConfiguration()).thenReturn(conf); + man.reloadConfiguration(conf); + // after the reload, which rebuilds the client map; by-name, because there are other map fields + Whitebox.setInternalState(man, "acmeClients", Map.of(DEFAULT_PROVIDER_NAME, ac)); + + man.run(); + + if (failureCase.equals("stale")) { + // failure counted, so the certificate falls back to WAITING and a fresh order + assertCertificateState(domain, REQUEST_FAILED, 1, man); + man.run(); + assertCertificateState(domain, WAITING, 1, man); + } else { + // state untouched, will be retried at the next cycle + assertCertificateState(domain, ORDERING, 0, man); + } } + } diff --git a/carapace-ui/src/main/webapp/src/components/certificates/Certificate.vue b/carapace-ui/src/main/webapp/src/components/certificates/Certificate.vue index 6139fab24..92e82ad0c 100644 --- a/carapace-ui/src/main/webapp/src/components/certificates/Certificate.vue +++ b/carapace-ui/src/main/webapp/src/components/certificates/Certificate.vue @@ -17,7 +17,9 @@

  • Hostname: {{certificate.hostname}}
  • Subject Alternative Names: {{certificate.subjectAltNames}}
  • Mode: {{certificate.mode}}
    • -
  • ACME provider: {{certificate.provider}}
    • +
  • + ACME provider: {{certificate.provider}} +
  • Dynamic: {{certificate.dynamic | symbolFormat}}
  • diff --git a/carapace-ui/src/main/webapp/src/components/certificates/CertificateForm.vue b/carapace-ui/src/main/webapp/src/components/certificates/CertificateForm.vue index 46ab40948..eafb8da72 100644 --- a/carapace-ui/src/main/webapp/src/components/certificates/CertificateForm.vue +++ b/carapace-ui/src/main/webapp/src/components/certificates/CertificateForm.vue @@ -110,9 +110,6 @@ import {doPost} from "../../serverapi"; import StatusBox from "../StatusBox.vue"; - // must match the backend built-in provider name (AcmeProviderConfiguration.DEFAULT_PROVIDER_NAME) - const DEFAULT_PROVIDER = 'letsencrypt'; - export default { name: "CertificateForm", components: { @@ -122,7 +119,7 @@ import {doPost} from "../../serverapi"; id: String, acmeProviders: { type: Array, - default: () => [DEFAULT_PROVIDER] + default: () => [] } }, data() { @@ -132,7 +129,7 @@ import {doPost} from "../../serverapi"; subjectAltNames: [], type: 'acme', daysBeforeRenewal: 30, - provider: DEFAULT_PROVIDER + provider: this.acmeProviders[0] || '' }, globalError: null, showOverlay: false @@ -148,13 +145,21 @@ import {doPost} from "../../serverapi"; return this.acmeProviders.map(name => ({value: name, text: name})) } }, + watch: { + // the provider list may arrive after the modal is opened + acmeProviders(providers) { + if (!this.form.provider) { + this.form.provider = providers[0] || '' + } + } + }, methods: { resetModal() { this.form.domain = '' this.form.subjectAltNames = [] this.form.type = 'acme' this.form.daysBeforeRenewal = 30 - this.form.provider = DEFAULT_PROVIDER + this.form.provider = this.acmeProviders[0] || '' this.clearFormErrors() }, clearFormErrors() { @@ -217,4 +222,4 @@ import {doPost} from "../../serverapi"; padding-bottom: 0.2rem !important; } } - \ No newline at end of file + diff --git a/carapace-ui/src/main/webapp/src/components/certificates/Certificates.vue b/carapace-ui/src/main/webapp/src/components/certificates/Certificates.vue index 39a8178d3..793749621 100644 --- a/carapace-ui/src/main/webapp/src/components/certificates/Certificates.vue +++ b/carapace-ui/src/main/webapp/src/components/certificates/Certificates.vue @@ -67,9 +67,6 @@ import {toBooleanSymbol} from '../../lib/formatter'; import CertificateForm from './CertificateForm.vue'; import StatusBox from '../StatusBox.vue'; -// must match the backend built-in provider name (AcmeProviderConfiguration.DEFAULT_PROVIDER_NAME) -const DEFAULT_PROVIDER = 'letsencrypt'; - /** * Certificate status options for dynamic SSL certificates. * @@ -113,10 +110,10 @@ export default { certificates: [], localStorePath: null, /** - * Names of the configured ACME providers. + * Names of the configured ACME providers, the default one first. * @type {string[]} */ - acmeProviders: [DEFAULT_PROVIDER], + acmeProviders: [], loading: true, opSuccess: null, opMessage: '', @@ -192,7 +189,7 @@ export default { doGet("/api/certificates", data => { this.certificates = data.certificates || []; this.localStorePath = data.localStorePath; - this.acmeProviders = data.acmeProviders || [DEFAULT_PROVIDER]; + this.acmeProviders = data.acmeProviders || []; this.loading = false; }); }, From a696ea297c75af40a12c6ed04905533289048b3e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Niccol=C3=B2=20Maltoni?= Date: Thu, 23 Jul 2026 15:26:29 +0200 Subject: [PATCH 10/10] fix: count unexpected ACME loop failures on the cert Unknown local failures now bump attemptsCount and surface in the UI instead of retrying invisibly forever. Also review nits: hoisted server local, documented reserved _acmeuserkey prefix. --- .../api/CertificatesResource.java | 5 +-- .../configstore/HerdDBConfigurationStore.java | 4 +++ .../certificates/AcmeFailureClassifier.java | 32 +++++++++---------- .../DynamicCertificatesManager.java | 2 +- .../AcmeFailureClassifierTest.java | 22 ++++++------- 5 files changed, 35 insertions(+), 30 deletions(-) diff --git a/carapace-server/src/main/java/org/carapaceproxy/api/CertificatesResource.java b/carapace-server/src/main/java/org/carapaceproxy/api/CertificatesResource.java index 7fd8228c4..0fe93cb60 100644 --- a/carapace-server/src/main/java/org/carapaceproxy/api/CertificatesResource.java +++ b/carapace-server/src/main/java/org/carapaceproxy/api/CertificatesResource.java @@ -361,6 +361,7 @@ public Response uploadCertificate( @QueryParam("provider") final String provider, final InputStream uploadedInputStream) throws Exception { + final var server = (HttpProxyServer) context.getAttribute("server"); try (InputStream input = uploadedInputStream) { // Certificate type (manual | acme) CertificateMode certType = stringToCertificateMode(type); @@ -388,7 +389,7 @@ public Response uploadCertificate( if (provider != null && !provider.isBlank() && !CertificateMode.ACME.equals(certType)) { return Response.status(422).entity("ERROR: param 'provider' available for type 'acme' only").build(); } - if (!isKnownProvider(acmeProvider, (HttpProxyServer) context.getAttribute("server"))) { + if (!isKnownProvider(acmeProvider, server)) { return Response.status(422).entity("ERROR: unknown ACME provider '" + acmeProvider + "'").build(); } @@ -407,7 +408,7 @@ public Response uploadCertificate( cert.setDaysBeforeRenewal(daysbeforerenewal != null ? daysbeforerenewal : DEFAULT_DAYS_BEFORE_RENEWAL); cert.setProvider(acmeProvider); - ((HttpProxyServer) context.getAttribute("server")).updateDynamicCertificateForDomain(cert); + server.updateDynamicCertificateForDomain(cert); return Response.status(200).entity("SUCCESS: Certificate saved").build(); } diff --git a/carapace-server/src/main/java/org/carapaceproxy/configstore/HerdDBConfigurationStore.java b/carapace-server/src/main/java/org/carapaceproxy/configstore/HerdDBConfigurationStore.java index 6e5a3e3e9..af4f6f2ea 100644 --- a/carapace-server/src/main/java/org/carapaceproxy/configstore/HerdDBConfigurationStore.java +++ b/carapace-server/src/main/java/org/carapaceproxy/configstore/HerdDBConfigurationStore.java @@ -367,6 +367,10 @@ private static String acmeUserKeyName(String providerName) { /** * Whether the primary key belongs to a provider account key pair, hence off-limits for domain key pairs. + *

    + * The whole {@code _acmeuserkey} prefix is reserved: a domain literally named like that + * would be silently skipped by the domain lookups. + * Safe assumption, as hostnames cannot start with {@code _} and the suffix is a validated provider name. * * @param pk a primary key of the key pairs table * @return true if it is an {@link #acmeUserKeyName(String) account key name} diff --git a/carapace-server/src/main/java/org/carapaceproxy/server/certificates/AcmeFailureClassifier.java b/carapace-server/src/main/java/org/carapaceproxy/server/certificates/AcmeFailureClassifier.java index 838178484..a4eb3f336 100644 --- a/carapace-server/src/main/java/org/carapaceproxy/server/certificates/AcmeFailureClassifier.java +++ b/carapace-server/src/main/java/org/carapaceproxy/server/certificates/AcmeFailureClassifier.java @@ -22,6 +22,7 @@ import java.net.URI; import java.util.Set; import java.util.regex.Pattern; +import org.carapaceproxy.configstore.CertificateData; import org.shredzone.acme4j.exception.AcmeException; import org.shredzone.acme4j.exception.AcmeLazyLoadingException; import org.shredzone.acme4j.exception.AcmeNetworkException; @@ -45,28 +46,27 @@ private AcmeFailureClassifier() { } /** - * Whether the CA actively rejected the request, - * as opposed to a transient failure (network error, rate limiting, CA outage) worth retrying in the same state. + * Whether the failure is transient (network error, rate limiting, CA outage) and worth retrying in the same state. *

    - * A rejection means the persisted certificate state is no longer usable, - * e.g. a pending order belonging to a different CA after a provider change, - * so the failure has to be {@link CertificateData#error(String) counted on the certificate} - * for it to fall back to a fresh order. - * acme4j may defer the server round-trip of bound resources, wrapping failures in {@link AcmeLazyLoadingException}. + * Anything else has to be {@link CertificateData#error(String) counted on the certificate}: + * a CA rejection means the persisted state is no longer usable + * (e.g., a pending order belonging to a different CA after a provider change), + * and an unexpected local failure would otherwise retry the same broken state forever + * without ever surfacing to the user; counting caps both at the configured attempts. + * ACME4j may defer the server round-trip of bound resources, wrapping failures in {@link AcmeLazyLoadingException}. * * @param ex the failure raised while processing a certificate - * @return true if the CA rejected the request + * @return true if the failure is worth retrying in the same certificate state */ - static boolean isRejectedByProvider(Exception ex) { + static boolean isTransient(Exception ex) { final var cause = ex instanceof AcmeLazyLoadingException lazy ? lazy.getCause() : ex; return switch (cause) { - case AcmeNetworkException ignored -> false; - case AcmeRateLimitedException ignored -> false; - case AcmeServerException server -> !TRANSIENT_ACME_PROBLEMS.contains(server.getType()); - // a 5xx without a problem document (e.g., a CA outage) surfaces as a bare exception; - // an unrecognizable message counts as rejection: worst case is a fresh order instead of being stuck forever - case AcmeException e -> e.getMessage() == null || !HTTP_SERVER_ERROR.matcher(e.getMessage()).find(); - case AcmeProtocolException ignored -> true; // malformed CA response, retrying it won't help + case AcmeNetworkException ignored -> true; + case AcmeRateLimitedException ignored -> true; + case AcmeServerException server -> TRANSIENT_ACME_PROBLEMS.contains(server.getType()); + // a 5xx without a problem document (e.g., a CA outage) surfaces as a bare exception + case AcmeException e -> e.getMessage() != null && HTTP_SERVER_ERROR.matcher(e.getMessage()).find(); + case AcmeProtocolException ignored -> false; // malformed CA response, retrying it won't help default -> false; }; } diff --git a/carapace-server/src/main/java/org/carapaceproxy/server/certificates/DynamicCertificatesManager.java b/carapace-server/src/main/java/org/carapaceproxy/server/certificates/DynamicCertificatesManager.java index 171fa93ad..ee42d0244 100644 --- a/carapace-server/src/main/java/org/carapaceproxy/server/certificates/DynamicCertificatesManager.java +++ b/carapace-server/src/main/java/org/carapaceproxy/server/certificates/DynamicCertificatesManager.java @@ -396,7 +396,7 @@ private void certificatesLifecycle() { // RuntimeException included on purpose, as an escaped one would silently cancel the scheduled task; // this would kill the renewal loop for every certificate LOG.error("Error while handling dynamic certificate for domain {}", domain, ex); - if (cert != null && AcmeFailureClassifier.isRejectedByProvider(ex)) { + if (cert != null && !AcmeFailureClassifier.isTransient(ex)) { cert.error(ex.getMessage() != null ? ex.getMessage() : ex.toString()); try { store.saveCertificate(cert); diff --git a/carapace-server/src/test/java/org/carapaceproxy/server/certificates/AcmeFailureClassifierTest.java b/carapace-server/src/test/java/org/carapaceproxy/server/certificates/AcmeFailureClassifierTest.java index 5dfb1afa8..78b517510 100644 --- a/carapace-server/src/test/java/org/carapaceproxy/server/certificates/AcmeFailureClassifierTest.java +++ b/carapace-server/src/test/java/org/carapaceproxy/server/certificates/AcmeFailureClassifierTest.java @@ -40,32 +40,32 @@ public class AcmeFailureClassifierTest { @Test public void testRejections() throws Exception { // the persisted state is unusable, retrying it won't help - assertTrue(AcmeFailureClassifier.isRejectedByProvider(new AcmeException("unknown order"))); - assertTrue(AcmeFailureClassifier.isRejectedByProvider(new AcmeProtocolException("malformed CA response"))); - assertTrue(AcmeFailureClassifier.isRejectedByProvider(new AcmeLazyLoadingException( + assertFalse(AcmeFailureClassifier.isTransient(new AcmeException("unknown order"))); + assertFalse(AcmeFailureClassifier.isTransient(new AcmeProtocolException("malformed CA response"))); + assertFalse(AcmeFailureClassifier.isTransient(new AcmeLazyLoadingException( Order.class, URI.create("https://localhost/order").toURL(), new AcmeException("unknown order")))); - assertTrue(AcmeFailureClassifier.isRejectedByProvider( + assertFalse(AcmeFailureClassifier.isTransient( new AcmeServerException(problemOfType("urn:ietf:params:acme:error:unauthorized")))); } @Test public void testTransientFailures() throws Exception { // worth retrying in the same state - assertFalse(AcmeFailureClassifier.isRejectedByProvider(new AcmeNetworkException(new IOException("io")))); - assertFalse(AcmeFailureClassifier.isRejectedByProvider(new AcmeRateLimitedException( + assertTrue(AcmeFailureClassifier.isTransient(new AcmeNetworkException(new IOException("io")))); + assertTrue(AcmeFailureClassifier.isTransient(new AcmeRateLimitedException( problemOfType("urn:ietf:params:acme:error:rateLimited"), null, List.of()))); - assertFalse(AcmeFailureClassifier.isRejectedByProvider( + assertTrue(AcmeFailureClassifier.isTransient( new AcmeServerException(problemOfType("urn:ietf:params:acme:error:serverInternal")))); - assertFalse(AcmeFailureClassifier.isRejectedByProvider( + assertTrue(AcmeFailureClassifier.isTransient( new AcmeServerException(problemOfType("urn:ietf:params:acme:error:badNonce")))); // a 5xx without a problem document, e.g., from a load balancer - assertFalse(AcmeFailureClassifier.isRejectedByProvider(new AcmeException("HTTP 503"))); + assertTrue(AcmeFailureClassifier.isTransient(new AcmeException("HTTP 503"))); } @Test public void testNonAcmeFailures() { - // anything non-ACME is not the CA rejecting the request - assertFalse(AcmeFailureClassifier.isRejectedByProvider(new IllegalStateException("boom"))); + // an unexpected local failure must be counted too, or it would retry invisibly forever + assertFalse(AcmeFailureClassifier.isTransient(new IllegalStateException("boom"))); } private static Problem problemOfType(String type) throws Exception {