From df56ab07a8c544521bfded694802f6b72c48aec3 Mon Sep 17 00:00:00 2001 From: Nathan Zender Date: Mon, 22 Jun 2020 22:47:55 -0400 Subject: [PATCH] Moving and shakin Instead of doing everything post app startup we can move some things into pre-app startup. Things moved: 1. Login and ordering a certificate if needed 2. If cert already downloaded and ready set it up Things that cannot be moved: 1. Authorizations, reason this cannot be moved is because it needs to be able to respond from requests from the ACME server. --- .../background/AcmeCertRefresherTask.java | 83 ++++++++++++------- .../micronaut/acme/services/AcmeService.java | 28 +++++-- ...cmeCertRefresherTaskSetsTimeoutSpec.groovy | 15 ++-- .../acme/AcmeCertRefresherTaskUnitSpec.groovy | 26 ++++-- 4 files changed, 105 insertions(+), 47 deletions(-) diff --git a/acme/src/main/java/io/micronaut/acme/background/AcmeCertRefresherTask.java b/acme/src/main/java/io/micronaut/acme/background/AcmeCertRefresherTask.java index 60fcfcc8..d75f225a 100644 --- a/acme/src/main/java/io/micronaut/acme/background/AcmeCertRefresherTask.java +++ b/acme/src/main/java/io/micronaut/acme/background/AcmeCertRefresherTask.java @@ -17,10 +17,13 @@ import io.micronaut.acme.AcmeConfiguration; import io.micronaut.acme.services.AcmeService; +import io.micronaut.context.event.StartupEvent; +import io.micronaut.http.server.exceptions.ServerStartupException; import io.micronaut.runtime.event.ApplicationStartupEvent; import io.micronaut.runtime.event.annotation.EventListener; import io.micronaut.runtime.exceptions.ApplicationStartupException; import io.micronaut.scheduling.annotation.Scheduled; +import org.shredzone.acme4j.Order; import org.shredzone.acme4j.exception.AcmeException; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -42,6 +45,7 @@ public final class AcmeCertRefresherTask { private AcmeService acmeService; private final AcmeConfiguration acmeConfiguration; + private Order order; /** * Constructs a new Acme cert refresher background task. @@ -66,36 +70,71 @@ void backgroundRenewal() throws AcmeException { if (LOG.isDebugEnabled()) { LOG.debug("Running background/scheduled renewal process"); } - renewCertIfNeeded(); + if (!acmeConfiguration.isTosAgree()) { + throw new IllegalStateException(String.format("Cannot refresh certificates until terms of service is accepted. Please review the TOS for Let's Encrypt and set \"%s\" to \"%s\" in configuration once complete", "acme.tos-agree", "true")); + } + List domains = getDomains(); + if (needsToOrderNewCertificate()) { + Order order = acmeService.orderCertificate(domains); + acmeService.authorizeCertificate(domains, order); + } } - /** - * Checks to see if certificate needs renewed on app startup. - * - * @param startupEvent Startup event - */ @EventListener - void onStartup(ApplicationStartupEvent startupEvent) { + void onServerStartup(StartupEvent startupEvent) { try { if (LOG.isDebugEnabled()) { - LOG.debug("Running startup renewal process"); + LOG.debug("Running server startup setup process"); + } + if (!acmeConfiguration.isTosAgree()) { + throw new IllegalStateException(String.format("Cannot refresh certificates until terms of service is accepted. Please review the TOS for Let's Encrypt and set \"%s\" to \"%s\" in configuration once complete", "acme.tos-agree", "true")); + } + if (needsToOrderNewCertificate()) { + order = acmeService.orderCertificate(getDomains()); + } else { + acmeService.setupCurrentCertificate(); } - renewCertIfNeeded(); } catch (Exception e) { LOG.error("Failed to initialize certificate for SSL no requests would be secure. Stopping application", e); - throw new ApplicationStartupException("Failed to start due to SSL configuration issue.", e); + throw new ServerStartupException("Failed to start due to SSL configuration issue.", e); } } /** - * Does the work to actually renew the certificate if it needs to be done. - * @throws AcmeException if any issues occur during certificate renewal + * Checks to see if certificate needs renewed on app startup. + * + * @param startupEvent Startup event */ - protected void renewCertIfNeeded() throws AcmeException { - if (!acmeConfiguration.isTosAgree()) { - throw new IllegalStateException(String.format("Cannot refresh certificates until terms of service is accepted. Please review the TOS for Let's Encrypt and set \"%s\" to \"%s\" in configuration once complete", "acme.tos-agree", "true")); + @EventListener + void onStartup(ApplicationStartupEvent startupEvent) { + if (needsToOrderNewCertificate()) { + try { + if (LOG.isDebugEnabled()) { + LOG.debug("Running application startup order/authorization process"); + } + acmeService.authorizeCertificate(getDomains(), order); + } catch (Exception e) { + LOG.error("Failed to initialize certificate for SSL no requests would be secure. Stopping application", e); + throw new ApplicationStartupException("Failed to start due to SSL configuration issue.", e); + } + } + } + + private boolean needsToOrderNewCertificate() { + boolean orderCertificate = false; + X509Certificate currentCertificate = acmeService.getCurrentCertificate(); + if (currentCertificate != null) { + long daysTillExpiration = ChronoUnit.SECONDS.between(Instant.now(), currentCertificate.getNotAfter().toInstant()); + if (daysTillExpiration <= acmeConfiguration.getRenewWitin().getSeconds()) { + orderCertificate = true; + } + } else { + orderCertificate = true; } + return orderCertificate; + } + private List getDomains() { List domains = new ArrayList<>(); for (String domain : acmeConfiguration.getDomains()) { domains.add(domain); @@ -107,18 +146,6 @@ protected void renewCertIfNeeded() throws AcmeException { domains.add(baseDomain); } } - - X509Certificate currentCertificate = acmeService.getCurrentCertificate(); - if (currentCertificate != null) { - long daysTillExpiration = ChronoUnit.SECONDS.between(Instant.now(), currentCertificate.getNotAfter().toInstant()); - - if (daysTillExpiration <= acmeConfiguration.getRenewWitin().getSeconds()) { - acmeService.orderCertificate(domains); - } else { - acmeService.setupCurrentCertificate(); - } - } else { - acmeService.orderCertificate(domains); - } + return domains; } } diff --git a/acme/src/main/java/io/micronaut/acme/services/AcmeService.java b/acme/src/main/java/io/micronaut/acme/services/AcmeService.java index dcdf2bd2..4018e4e2 100644 --- a/acme/src/main/java/io/micronaut/acme/services/AcmeService.java +++ b/acme/src/main/java/io/micronaut/acme/services/AcmeService.java @@ -143,10 +143,10 @@ public X509Certificate getCurrentCertificate() { * * @param domains List of domains to order a certificate for * @throws AcmeException if any issues occur during ordering of certificate + * + * @return order for the given list of domains */ - public void orderCertificate(List domains) throws AcmeException { - AtomicInteger orderRetryAttempts = new AtomicInteger(acmeConfiguration.getOrder().getRefreshAttempts()); - + public Order orderCertificate(List domains) throws AcmeException { Session session = new Session(acmeServerUrl); if (timeout != null) { session.networkSettings().setTimeout(timeout); @@ -156,14 +156,26 @@ public void orderCertificate(List domains) throws AcmeException { try { accountKeyPair = getKeyPairFromConfigValue(this.accountKeyString); } catch (IOException e) { - if (LOG.isErrorEnabled()) { - LOG.error("ACME certificate order failed. Failed to read the account keys", e); - } - return; + throw new AcmeException("ACME certificate order failed. Failed to read the account keys", e); } Login login = doLogin(session, accountKeyPair); - Order order = createOrder(domains, login); + return createOrder(domains, login); + } + + /** + * Authorizes an order and if successful emits a certificate via an event. + * + * @param domains List of domains to order a certificate for + * @param order acme order for the given set of domains + * @throws AcmeException if any issues occur during authorization of a certificate order + */ + public void authorizeCertificate(List domains, Order order) throws AcmeException { + if (order == null) { + throw new AcmeException("Order is required before you can authorize it."); + } + + AtomicInteger orderRetryAttempts = new AtomicInteger(acmeConfiguration.getOrder().getRefreshAttempts()); for (Authorization auth : order.getAuthorizations()) { try { authorize(auth); diff --git a/acme/src/test/groovy/io/micronaut/acme/AcmeCertRefresherTaskSetsTimeoutSpec.groovy b/acme/src/test/groovy/io/micronaut/acme/AcmeCertRefresherTaskSetsTimeoutSpec.groovy index 1b76813c..f59c79ef 100644 --- a/acme/src/test/groovy/io/micronaut/acme/AcmeCertRefresherTaskSetsTimeoutSpec.groovy +++ b/acme/src/test/groovy/io/micronaut/acme/AcmeCertRefresherTaskSetsTimeoutSpec.groovy @@ -2,6 +2,7 @@ package io.micronaut.acme import io.micronaut.context.ApplicationContext import io.micronaut.core.io.socket.SocketUtils +import io.micronaut.http.server.exceptions.ServerStartupException import io.micronaut.mock.slow.SlowAcmeServer import io.micronaut.mock.slow.SlowServerConfig import io.micronaut.runtime.exceptions.ApplicationStartupException @@ -98,7 +99,7 @@ class AcmeCertRefresherTaskSetsTimeoutSpec extends Specification { } @Unroll - def "validate timeout applied if signup is slow"(SlowServerConfig config) { + def "validate timeout applied if signup is slow"(SlowServerConfig config, Class exType) { given: "we have all the ports we could ever need" expectedHttpPort = SocketUtils.findAvailableTcpPort() expectedSecurePort = SocketUtils.findAvailableTcpPort() @@ -121,7 +122,9 @@ class AcmeCertRefresherTaskSetsTimeoutSpec extends Specification { "test") then: "we get network errors b/c of the timeout" - ApplicationStartupException ex = thrown() + def ex = thrown(Throwable) + + ex.class == exType def ane = ExceptionUtils.getThrowables(ex).find { it instanceof AcmeNetworkException } ane?.message == "Network error" @@ -135,10 +138,10 @@ class AcmeCertRefresherTaskSetsTimeoutSpec extends Specification { mockAcmeServer?.stop() where: - config | _ - new ActualSlowServerConfig(slowSignup: true) | _ - new ActualSlowServerConfig(slowOrdering: true) | _ - new ActualSlowServerConfig(slowAuthorization: true) | _ + config | exType + new ActualSlowServerConfig(slowSignup: true) | ServerStartupException + new ActualSlowServerConfig(slowOrdering: true) | ServerStartupException + new ActualSlowServerConfig(slowAuthorization: true) | ApplicationStartupException } class ActualSlowServerConfig implements SlowServerConfig { diff --git a/acme/src/test/groovy/io/micronaut/acme/AcmeCertRefresherTaskUnitSpec.groovy b/acme/src/test/groovy/io/micronaut/acme/AcmeCertRefresherTaskUnitSpec.groovy index d9d1c827..47df6047 100644 --- a/acme/src/test/groovy/io/micronaut/acme/AcmeCertRefresherTaskUnitSpec.groovy +++ b/acme/src/test/groovy/io/micronaut/acme/AcmeCertRefresherTaskUnitSpec.groovy @@ -2,6 +2,7 @@ package io.micronaut.acme import io.micronaut.acme.background.AcmeCertRefresherTask import io.micronaut.acme.services.AcmeService +import io.micronaut.http.server.exceptions.ServerStartupException import io.micronaut.runtime.EmbeddedApplication import io.micronaut.runtime.event.ApplicationStartupEvent import io.micronaut.runtime.exceptions.ApplicationStartupException @@ -23,7 +24,22 @@ class AcmeCertRefresherTaskUnitSpec extends Specification { def task = new AcmeCertRefresherTask(Mock(AcmeService), Mock(AcmeConfiguration)) when: - task.renewCertIfNeeded() + task.onServerStartup(null) + + then: + def ex = thrown(ServerStartupException.class) + + Throwable rootEx = ExceptionUtils.getRootCause(ex) + rootEx instanceof IllegalStateException + rootEx.message == "Cannot refresh certificates until terms of service is accepted. Please review the TOS for Let's Encrypt and set \"acme.tos-agree\" to \"true\" in configuration once complete" + } + + def "throw exception if TOS has not been accepted when doing background reneal"() { + given: + def task = new AcmeCertRefresherTask(Mock(AcmeService), Mock(AcmeConfiguration)) + + when: + task.backgroundRenewal() then: def ex = thrown(IllegalStateException.class) @@ -39,7 +55,7 @@ class AcmeCertRefresherTaskUnitSpec extends Specification { def task = new AcmeCertRefresherTask(mockAcmeSerivce, config) when: - task.renewCertIfNeeded() + task.onServerStartup(null) then: 1 * mockAcmeSerivce.getCurrentCertificate() >> new SelfSignedCertificate(expectedDomain, new Date(), new Date() + 31).cert() @@ -56,7 +72,7 @@ class AcmeCertRefresherTaskUnitSpec extends Specification { def task = new AcmeCertRefresherTask(mockAcmeSerivce, config) when: - task.renewCertIfNeeded() + task.onServerStartup(null) then: 1 * mockAcmeSerivce.getCurrentCertificate() >> new SelfSignedCertificate(expectedDomain, new Date(), new Date() + 31).cert() @@ -77,10 +93,10 @@ class AcmeCertRefresherTaskUnitSpec extends Specification { def task = new AcmeCertRefresherTask(mockAcmeSerivce, config) when: - task.onStartup(new ApplicationStartupEvent(Mock(EmbeddedApplication))) + task.onServerStartup(null) then: - def ex = thrown(ApplicationStartupException) + def ex = thrown(ServerStartupException) ex.message == "Failed to start due to SSL configuration issue." and: