From 33951d25388b67bb1205a8735626150c1bcab5c4 Mon Sep 17 00:00:00 2001 From: Nathan Zender Date: Sun, 18 Jul 2021 21:15:19 -0400 Subject: [PATCH 01/14] Using full cert chain over single certificate Found that we were parsing the cert using CertificateFactory.getCertificate. This was not giving us the full chain but instead just the domain cert. Which in most places work but not in all. To address this we now will get the full chain using CertificateFactor.getCertificates and pass that on the event to Netty so that it can be used for initalization. --- .../acme/events/CertificateEvent.java | 13 ++ .../micronaut/acme/services/AcmeService.java | 23 +++- .../acme/ssl/AcmeSSLContextBuilder.java | 2 +- ...meCertRefresherMultiDomainsTaskSpec.groovy | 6 +- .../acme/AcmeCertRefresherTaskSpec.groovy | 6 +- ...tRefresherTaskWithClasspathKeysSpec.groovy | 6 +- ...meCertRefresherTaskWithFileKeysSpec.groovy | 6 +- .../AcmeCertWildcardRefresherTaskSpec.groovy | 6 +- ...ertRefresherTaskHttp01ChallengeSpec.groovy | 6 +- ...RefresherTaskTlsApln01ChallengeSpec.groovy | 6 +- .../acme/events/CertificateEventSpec.groovy | 120 ++++++++++++++++++ 11 files changed, 190 insertions(+), 10 deletions(-) create mode 100644 acme/src/test/groovy/io/micronaut/acme/events/CertificateEventSpec.groovy diff --git a/acme/src/main/java/io/micronaut/acme/events/CertificateEvent.java b/acme/src/main/java/io/micronaut/acme/events/CertificateEvent.java index 0f648a9b..5211f61d 100644 --- a/acme/src/main/java/io/micronaut/acme/events/CertificateEvent.java +++ b/acme/src/main/java/io/micronaut/acme/events/CertificateEvent.java @@ -24,6 +24,7 @@ public class CertificateEvent { private final X509Certificate certificate; private final KeyPair domainKeyPair; + private final X509Certificate[] fullCertificateChain; private boolean validationCert; /** @@ -36,6 +37,14 @@ public CertificateEvent(X509Certificate certificate, KeyPair domainKeyPair, bool this.certificate = certificate; this.domainKeyPair = domainKeyPair; this.validationCert = validationCert; + this.fullCertificateChain = new X509Certificate[]{certificate}; + } + + public CertificateEvent(KeyPair domainKeyPair, X509Certificate... fullCertificateChain) { + this.validationCert = false; + this.domainKeyPair = domainKeyPair; + this.certificate = fullCertificateChain[0]; + this.fullCertificateChain = fullCertificateChain; } /** @@ -58,4 +67,8 @@ public KeyPair getDomainKeyPair() { public boolean isValidationCert() { return validationCert; } + + public X509Certificate[] getFullCertificateChain() { + return fullCertificateChain; + } } 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 0e112f92..775c1e2d 100644 --- a/acme/src/main/java/io/micronaut/acme/services/AcmeService.java +++ b/acme/src/main/java/io/micronaut/acme/services/AcmeService.java @@ -47,6 +47,8 @@ import java.security.cert.X509Certificate; import java.time.Duration; import java.time.Instant; +import java.util.ArrayList; +import java.util.Collection; import java.util.List; import java.util.Optional; import java.util.concurrent.CancellationException; @@ -138,6 +140,23 @@ public X509Certificate getCurrentCertificate() { } } + protected X509Certificate[] getFullCertificateChain() { + try { + CertificateFactory cf = CertificateFactory.getInstance("X.509"); + File certificate = new File(certLocation, DOMAIN_CRT); + if (certificate.exists()) { + return cf.generateCertificates(Files.newInputStream(certificate.toPath())).toArray(new X509Certificate[0]); + } else { + return new X509Certificate[]{}; + } + } catch (CertificateException | IOException e) { + if (LOG.isWarnEnabled()) { + LOG.warn("Could not create certificate from file", e); + } + return null; + } + } + /** * Orders a new certificate using ACME protocol. * @@ -278,7 +297,7 @@ private boolean writeCombinedFile(Certificate certificate) { try (BufferedWriter writer = Files.newBufferedWriter(domainCsr.toPath(), WRITE, CREATE, TRUNCATE_EXISTING)) { certificate.writeCertificate(writer); } - eventPublisher.publishEvent(new CertificateEvent(getCurrentCertificate(), domainKeyPair, false)); + eventPublisher.publishEvent(new CertificateEvent(domainKeyPair, getFullCertificateChain())); if (LOG.isInfoEnabled()) { LOG.info("ACME certificate order success! Certificate URL: {}", certificate.getLocation()); } @@ -482,7 +501,7 @@ private void doChallengeSpecificSetup(Authorization auth, Challenge challenge) t * Setup the certificate that has been saved to disk and configures it for use. */ public void setupCurrentCertificate() { - eventPublisher.publishEvent(new CertificateEvent(getCurrentCertificate(), getDomainKeyPair(), false)); + eventPublisher.publishEvent(new CertificateEvent(getDomainKeyPair(), getFullCertificateChain())); } /** diff --git a/acme/src/main/java/io/micronaut/acme/ssl/AcmeSSLContextBuilder.java b/acme/src/main/java/io/micronaut/acme/ssl/AcmeSSLContextBuilder.java index 8efff2d0..53127610 100644 --- a/acme/src/main/java/io/micronaut/acme/ssl/AcmeSSLContextBuilder.java +++ b/acme/src/main/java/io/micronaut/acme/ssl/AcmeSSLContextBuilder.java @@ -77,7 +77,7 @@ void onNewCertificate(CertificateEvent certificateEvent) { delegatedSslContext.setNewSslContext(sslContext); } else { SslContext sslContext = SslContextBuilder - .forServer(certificateEvent.getDomainKeyPair().getPrivate(), certificateEvent.getCert()) + .forServer(certificateEvent.getDomainKeyPair().getPrivate(), certificateEvent.getFullCertificateChain()) .build(); delegatedSslContext.setNewSslContext(sslContext); } diff --git a/acme/src/test/groovy/io/micronaut/acme/AcmeCertRefresherMultiDomainsTaskSpec.groovy b/acme/src/test/groovy/io/micronaut/acme/AcmeCertRefresherMultiDomainsTaskSpec.groovy index 54981b4f..6a84ed62 100644 --- a/acme/src/test/groovy/io/micronaut/acme/AcmeCertRefresherMultiDomainsTaskSpec.groovy +++ b/acme/src/test/groovy/io/micronaut/acme/AcmeCertRefresherMultiDomainsTaskSpec.groovy @@ -52,13 +52,17 @@ class AcmeCertRefresherMultiDomainsTaskSpec extends AcmeBaseSpec { try { conn.connect() Certificate[] certs = conn.getServerCertificates() - certs.length == 1 + certs.length == 2 def cert = (X509Certificate) certs[0] cert.getIssuerDN().getName().contains("Pebble Intermediate CA") cert.getSubjectDN().getName().contains(EXPECTED_DOMAIN) cert.getSubjectAlternativeNames().size() == 2 cert.getSubjectAlternativeNames().collect({d-> d.get(1)}).contains(EXPECTED_DOMAIN) cert.getSubjectAlternativeNames().collect({d-> d.get(1)}).contains(EXPECTED_ACME_DOMAIN) + + def cert2 = (X509Certificate) certs[1] + cert2.getIssuerDN().getName().contains("Pebble Root CA") + cert2.getSubjectDN().getName().contains("Pebble Intermediate CA") }finally{ if(conn != null){ conn.disconnect() diff --git a/acme/src/test/groovy/io/micronaut/acme/AcmeCertRefresherTaskSpec.groovy b/acme/src/test/groovy/io/micronaut/acme/AcmeCertRefresherTaskSpec.groovy index f5cd4816..7f9716e4 100644 --- a/acme/src/test/groovy/io/micronaut/acme/AcmeCertRefresherTaskSpec.groovy +++ b/acme/src/test/groovy/io/micronaut/acme/AcmeCertRefresherTaskSpec.groovy @@ -52,11 +52,15 @@ class AcmeCertRefresherTaskSpec extends AcmeBaseSpec { try { conn.connect() Certificate[] certs = conn.getServerCertificates() - certs.length == 1 + certs.length == 2 def cert = (X509Certificate) certs[0] cert.getIssuerDN().getName().contains("Pebble Intermediate CA") cert.getSubjectDN().getName().contains(EXPECTED_DOMAIN) cert.getSubjectAlternativeNames().size() == 1 + + def cert2 = (X509Certificate) certs[1] + cert2.getIssuerDN().getName().contains("Pebble Root CA") + cert2.getSubjectDN().getName().contains("Pebble Intermediate CA") }finally{ if(conn != null){ conn.disconnect() diff --git a/acme/src/test/groovy/io/micronaut/acme/AcmeCertRefresherTaskWithClasspathKeysSpec.groovy b/acme/src/test/groovy/io/micronaut/acme/AcmeCertRefresherTaskWithClasspathKeysSpec.groovy index faab4830..bae29063 100644 --- a/acme/src/test/groovy/io/micronaut/acme/AcmeCertRefresherTaskWithClasspathKeysSpec.groovy +++ b/acme/src/test/groovy/io/micronaut/acme/AcmeCertRefresherTaskWithClasspathKeysSpec.groovy @@ -67,11 +67,15 @@ class AcmeCertRefresherTaskWithClasspathKeysSpec extends AcmeBaseSpec { try { conn.connect() Certificate[] certs = conn.getServerCertificates() - certs.length == 1 + certs.length == 2 def cert = (X509Certificate) certs[0] cert.getIssuerDN().getName().contains("Pebble Intermediate CA") cert.getSubjectDN().getName().contains(EXPECTED_DOMAIN) cert.getSubjectAlternativeNames().size() == 1 + + def cert2 = (X509Certificate) certs[1] + cert2.getIssuerDN().getName().contains("Pebble Root CA") + cert2.getSubjectDN().getName().contains("Pebble Intermediate CA") }finally{ if(conn != null){ conn.disconnect() diff --git a/acme/src/test/groovy/io/micronaut/acme/AcmeCertRefresherTaskWithFileKeysSpec.groovy b/acme/src/test/groovy/io/micronaut/acme/AcmeCertRefresherTaskWithFileKeysSpec.groovy index bbf345e5..70a96da6 100644 --- a/acme/src/test/groovy/io/micronaut/acme/AcmeCertRefresherTaskWithFileKeysSpec.groovy +++ b/acme/src/test/groovy/io/micronaut/acme/AcmeCertRefresherTaskWithFileKeysSpec.groovy @@ -61,11 +61,15 @@ class AcmeCertRefresherTaskWithFileKeysSpec extends AcmeBaseSpec { try { conn.connect() Certificate[] certs = conn.getServerCertificates() - certs.length == 1 + certs.length == 2 def cert = (X509Certificate) certs[0] cert.getIssuerDN().getName().contains("Pebble Intermediate CA") cert.getSubjectDN().getName().contains(EXPECTED_DOMAIN) cert.getSubjectAlternativeNames().size() == 1 + + def cert2 = (X509Certificate) certs[1] + cert2.getIssuerDN().getName().contains("Pebble Root CA") + cert2.getSubjectDN().getName().contains("Pebble Intermediate CA") }finally{ if(conn != null){ conn.disconnect() diff --git a/acme/src/test/groovy/io/micronaut/acme/AcmeCertWildcardRefresherTaskSpec.groovy b/acme/src/test/groovy/io/micronaut/acme/AcmeCertWildcardRefresherTaskSpec.groovy index aecf2afc..9671391d 100644 --- a/acme/src/test/groovy/io/micronaut/acme/AcmeCertWildcardRefresherTaskSpec.groovy +++ b/acme/src/test/groovy/io/micronaut/acme/AcmeCertWildcardRefresherTaskSpec.groovy @@ -57,13 +57,17 @@ class AcmeCertWildcardRefresherTaskSpec extends AcmeBaseSpec { try { conn.connect() Certificate[] certs = conn.getServerCertificates() - certs.length == 1 + certs.length == 2 def cert = (X509Certificate) certs[0] cert.getIssuerDN().getName().contains("Pebble Intermediate CA") cert.getSubjectDN().getName().contains(WILDCARD_DOMAIN) cert.getSubjectAlternativeNames().size() == 2 cert.getSubjectAlternativeNames().collect({d-> d.get(1)}).contains(WILDCARD_DOMAIN) cert.getSubjectAlternativeNames().collect({d-> d.get(1)}).contains(EXPECTED_BASE_DOMAIN) + + def cert2 = (X509Certificate) certs[1] + cert2.getIssuerDN().getName().contains("Pebble Root CA") + cert2.getSubjectDN().getName().contains("Pebble Intermediate CA") }finally{ if(conn != null){ conn.disconnect() diff --git a/acme/src/test/groovy/io/micronaut/acme/challenges/AcmeCertRefresherTaskHttp01ChallengeSpec.groovy b/acme/src/test/groovy/io/micronaut/acme/challenges/AcmeCertRefresherTaskHttp01ChallengeSpec.groovy index 7fbf620c..91414ed0 100644 --- a/acme/src/test/groovy/io/micronaut/acme/challenges/AcmeCertRefresherTaskHttp01ChallengeSpec.groovy +++ b/acme/src/test/groovy/io/micronaut/acme/challenges/AcmeCertRefresherTaskHttp01ChallengeSpec.groovy @@ -61,11 +61,15 @@ class AcmeCertRefresherTaskHttp01ChallengeSpec extends AcmeBaseSpec { Certificate[] certs = conn.getServerCertificates() then: "we make sure they are from the pebble test server and the domain is as expected" - certs.length == 1 + certs.length == 2 def cert = (X509Certificate) certs[0] cert.getIssuerDN().getName().contains("Pebble Intermediate CA") cert.getSubjectDN().getName().contains(EXPECTED_ACME_DOMAIN) cert.getSubjectAlternativeNames().size() == 1 + + def cert2 = (X509Certificate) certs[1] + cert2.getIssuerDN().getName().contains("Pebble Root CA") + cert2.getSubjectDN().getName().contains("Pebble Intermediate CA") } void "test send https request when the cert is in place"() { diff --git a/acme/src/test/groovy/io/micronaut/acme/challenges/AcmeCertRefresherTaskTlsApln01ChallengeSpec.groovy b/acme/src/test/groovy/io/micronaut/acme/challenges/AcmeCertRefresherTaskTlsApln01ChallengeSpec.groovy index 763473d0..b70403ac 100644 --- a/acme/src/test/groovy/io/micronaut/acme/challenges/AcmeCertRefresherTaskTlsApln01ChallengeSpec.groovy +++ b/acme/src/test/groovy/io/micronaut/acme/challenges/AcmeCertRefresherTaskTlsApln01ChallengeSpec.groovy @@ -59,11 +59,15 @@ class AcmeCertRefresherTaskTlsApln01ChallengeSpec extends AcmeBaseSpec { Certificate[] certs = conn.getServerCertificates() then: "we make sure they are from the pebble test server and the domain is as expected" - certs.length == 1 + certs.length == 2 def cert = (X509Certificate) certs[0] cert.getIssuerDN().getName().contains("Pebble Intermediate CA") cert.getSubjectDN().getName().contains(EXPECTED_ACME_DOMAIN) cert.getSubjectAlternativeNames().size() == 1 + + def cert2 = (X509Certificate) certs[1] + cert2.getIssuerDN().getName().contains("Pebble Root CA") + cert2.getSubjectDN().getName().contains("Pebble Intermediate CA") } void "test send https request when the cert is in place"() { diff --git a/acme/src/test/groovy/io/micronaut/acme/events/CertificateEventSpec.groovy b/acme/src/test/groovy/io/micronaut/acme/events/CertificateEventSpec.groovy new file mode 100644 index 00000000..92c6e604 --- /dev/null +++ b/acme/src/test/groovy/io/micronaut/acme/events/CertificateEventSpec.groovy @@ -0,0 +1,120 @@ +package io.micronaut.acme.events + + +import org.shredzone.acme4j.util.KeyPairUtils +import spock.lang.Specification + +import java.security.KeyPair +import java.security.cert.CertificateFactory +import java.security.cert.X509Certificate + +class CertificateEventSpec extends Specification { + def DOMAIN_CERT = """ +-----BEGIN CERTIFICATE----- +MIIDUDCCAjigAwIBAgIIHuspA0mthF8wDQYJKoZIhvcNAQELBQAwIDEeMBwGA1UE +AxMVUGViYmxlIFJvb3QgQ0EgMTU5ZjdmMCAXDTIxMDcxODIxMDczNloYDzIwNTEw +NzE4MjEwNzM2WjAoMSYwJAYDVQQDEx1QZWJibGUgSW50ZXJtZWRpYXRlIENBIDY2 +NjViYjCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEBALnw4xa4KQCxbhiW +1o1VYVUbof2mWwkhsWRuws4Uc1kvAVM7k1RZvwNxGQLgJkdXjbUrctddHdFMtksN +imNy/nmB6LKoulzwDL1omCdaiYOxJr93cGYQC3FTm/RaTpaHuec+BaB2Y1iOzbBj +sLL9121eRWUZ0vjaqKwNO8NUlK/geELNgoteIJ1MjOzWp1bryjnaszBfg0eiidD8 +4gV36fvrM1UVJZJ4LBV4QHrKVXl7JA5hn9uk7zucH/XEG87DO2DCWJIwZK9Fm8wD +qMmvx/QH+dwXOXe6kXTDuyu7jJMoHDBLNQ9o4gjkqqHxA1f0ewgo64ObJ09hx+96 +fuC49x8CAwEAAaOBgzCBgDAOBgNVHQ8BAf8EBAMCAoQwHQYDVR0lBBYwFAYIKwYB +BQUHAwEGCCsGAQUFBwMCMA8GA1UdEwEB/wQFMAMBAf8wHQYDVR0OBBYEFA56gdVb ++6pOjZzcKcjPhe7fWr5WMB8GA1UdIwQYMBaAFD8kVZzZs7SeJSiQFrf5AsH/EFmv +MA0GCSqGSIb3DQEBCwUAA4IBAQApcSZ5s0VGT1KgsXh3GrqxwlSyFfVuE4qvMabf +rXAhUbG3C6hgdA2AWA5IUvI9fRqul6m88hLZc8hrgOJ0vGDAD2u/PMdrqtAz8fV4 +gch5z+Jn4J+9Af7hOm3DSFtVRqvbtyWTT2ht7wJbtxAOsuD7+Wa6lr+lZxhHXbRv +RpY6uVNZNlnC5k8BFnx8S9SdsK+upYtkgyKLoFpDhyXgmFMJPGA7UY6NQQ1sA/2x +dwYXMfCY829k0hcxcXYC4SYDjwHxF6YIM4lYS8pT0Z8d98H5cK7WNwFmW+izu6cx +87DDk/ZlkyArnozVQ6GFJClfhbKZfPKty1r1Y1psSOAUcUD1 +-----END CERTIFICATE----- +""" + + def ROOT_CERTIFICATE = """ +-----BEGIN CERTIFICATE----- +MIIDezCCAmOgAwIBAgIIBzCDqTIFEj8wDQYJKoZIhvcNAQELBQAwKDEmMCQGA1UE +AxMdUGViYmxlIEludGVybWVkaWF0ZSBDQSA2NjY1YmIwHhcNMjEwNzE4MjEwNzQx +WhcNMjYwNzE4MjEwNzQxWjAnMSUwIwYDVQQDExxob3N0LnRlc3Rjb250YWluZXJz +LmludGVybmFsMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAiwwiBiCF +q1oMiXSOvEjCKkSR5lGu9CDW9UFQgN/UhVG2RyuDojImUOQjOjHe/DWn7g1XKovT +3it/M1onAnmksvqFd6YwSUKT8epL1K0dyVzgwaPAgjpJZgt/IZvA9ATWILuMJDGB +jdRRUQ+xex3AVbwa5UJYPlK2t1yqL5YPP9WpZ8H3c1F6M2by5VbwIi78LSxPc47m +H35efxWX2DalsDYirgP3bL0/X/yeVw058Iga+9MsF5MELDMuh9fe5N81TcrtKHvW +W4DfBPUFSnA/52G/nltZdgXxyMgErgwHx86dQphZMAGAD+wCXnzewAI9ZWN4iU27 +IiP1kQqVP33AoQIDAQABo4GpMIGmMA4GA1UdDwEB/wQEAwIFoDAdBgNVHSUEFjAU +BggrBgEFBQcDAQYIKwYBBQUHAwIwDAYDVR0TAQH/BAIwADAdBgNVHQ4EFgQUFXuU +GdoN4vOZ3IiyvXkQw2Dd92cwHwYDVR0jBBgwFoAUDnqB1Vv7qk6NnNwpyM+F7t9a +vlYwJwYDVR0RBCAwHoIcaG9zdC50ZXN0Y29udGFpbmVycy5pbnRlcm5hbDANBgkq +hkiG9w0BAQsFAAOCAQEAEJNY7olfzudkko1FcGq5bCauwB9240uu67YUIJG7y54G +tq2XWYWQ19FAqgb/7iWKq5X2hjp3Ut3x76SCwOKy5Q0dArcxwQYVgMMj9znxH6LL +QBJOPgQDvnxysEXEu4zvR/GV6ZS5ndFKJAPJxklZkdGhhqp15gUnP/1qTGPLEC5j +7gR/TfCwWsiMvkBmkYyacDvIHPd8QHtISNhL5Y+dww8DeL+F4ALC1dFLdAaT/bdx +RVv802SMY7YAh8FAsnTsKLYNbSk6ZHVbJuBcVbHqGuWueZ43hwmOTF6pIDaIoBg1 +zSti1w9hjz913WF0dTg7RWFLU8e3Jo1O9MCnORtcgg== +-----END CERTIFICATE-----""" + + def FULL_CHAIN_CERT = """ +${ROOT_CERTIFICATE} +${DOMAIN_CERT} +""" + + def "can get domain keypair"(){ + given: + CertificateFactory cf = CertificateFactory.getInstance("X.509") + X509Certificate cert = cf.generateCertificate(new ByteArrayInputStream(FULL_CHAIN_CERT.bytes)) + KeyPair keyPair = KeyPairUtils.createKeyPair(2048) + + when : + CertificateEvent event = new CertificateEvent(cert, keyPair, new Random().nextBoolean()) + + then: + event.getDomainKeyPair() == keyPair + } + + def "can determine if the event is a validation certificate or not"(){ + given: + CertificateFactory cf = CertificateFactory.getInstance("X.509") + X509Certificate cert = cf.generateCertificate(new ByteArrayInputStream(FULL_CHAIN_CERT.bytes)) + KeyPair keyPair = KeyPairUtils.createKeyPair(2048) + def validationCert = new Random().nextBoolean() + + when : + CertificateEvent event = new CertificateEvent(cert, keyPair, validationCert) + + then: + event.isValidationCert() == validationCert + } + + def "when pass single cert the full chain only contains that cert"(){ + given: + CertificateFactory cf = CertificateFactory.getInstance("X.509") + X509Certificate domainCert = cf.generateCertificate(new ByteArrayInputStream(FULL_CHAIN_CERT.bytes)) + KeyPair keyPair = KeyPairUtils.createKeyPair(2048) + + when : + CertificateEvent event = new CertificateEvent(domainCert, keyPair, new Random().nextBoolean()) + + then: + event.getCert() == domainCert + event.getFullCertificateChain().length == 1 + event.getFullCertificateChain()[0] == domainCert + } + + def "when full certificate chain passed we can still get the domain specific cert"(){ + given: + CertificateFactory cf = CertificateFactory.getInstance("X.509") + X509Certificate domainCert = cf.generateCertificate(new ByteArrayInputStream(FULL_CHAIN_CERT.bytes)) + Collection certs = cf.generateCertificates(new ByteArrayInputStream(FULL_CHAIN_CERT.bytes)) + KeyPair keyPair = KeyPairUtils.createKeyPair(2048) + + when : + CertificateEvent event = new CertificateEvent(keyPair, certs as X509Certificate[]) + + then: + event.getCert() == domainCert + event.getFullCertificateChain().length == 2 + event.getFullCertificateChain() == certs.toArray() + } +} From 3bfff2007527b147984d53ce1ecbe885bc747bdc Mon Sep 17 00:00:00 2001 From: Nathan Zender Date: Mon, 19 Jul 2021 09:16:50 -0400 Subject: [PATCH 02/14] No need to store another certificate just use the chain --- .../main/java/io/micronaut/acme/events/CertificateEvent.java | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/acme/src/main/java/io/micronaut/acme/events/CertificateEvent.java b/acme/src/main/java/io/micronaut/acme/events/CertificateEvent.java index 5211f61d..e44e031c 100644 --- a/acme/src/main/java/io/micronaut/acme/events/CertificateEvent.java +++ b/acme/src/main/java/io/micronaut/acme/events/CertificateEvent.java @@ -22,7 +22,6 @@ * Event used to alert when a new ACME certificate is ready for use. */ public class CertificateEvent { - private final X509Certificate certificate; private final KeyPair domainKeyPair; private final X509Certificate[] fullCertificateChain; private boolean validationCert; @@ -34,7 +33,6 @@ public class CertificateEvent { * @param validationCert if this certificate is to be used for tls-apln-01 account validation */ public CertificateEvent(X509Certificate certificate, KeyPair domainKeyPair, boolean validationCert) { - this.certificate = certificate; this.domainKeyPair = domainKeyPair; this.validationCert = validationCert; this.fullCertificateChain = new X509Certificate[]{certificate}; @@ -43,7 +41,6 @@ public CertificateEvent(X509Certificate certificate, KeyPair domainKeyPair, bool public CertificateEvent(KeyPair domainKeyPair, X509Certificate... fullCertificateChain) { this.validationCert = false; this.domainKeyPair = domainKeyPair; - this.certificate = fullCertificateChain[0]; this.fullCertificateChain = fullCertificateChain; } @@ -51,7 +48,7 @@ public CertificateEvent(KeyPair domainKeyPair, X509Certificate... fullCertificat * @return Certificate created by ACME server */ public X509Certificate getCert() { - return certificate; + return fullCertificateChain[0]; } /** From 1c3fc8de87bdbb0e419a1267b38c18d8fb328e4c Mon Sep 17 00:00:00 2001 From: Nathan Zender Date: Mon, 19 Jul 2021 09:21:52 -0400 Subject: [PATCH 03/14] No reason we cant deprecate the old constructor and use the new one --- .../io/micronaut/acme/events/CertificateEvent.java | 13 +++++++++++-- .../io/micronaut/acme/services/AcmeService.java | 6 +++--- .../acme/events/CertificateEventSpec.groovy | 4 +++- 3 files changed, 17 insertions(+), 6 deletions(-) diff --git a/acme/src/main/java/io/micronaut/acme/events/CertificateEvent.java b/acme/src/main/java/io/micronaut/acme/events/CertificateEvent.java index e44e031c..3127cb39 100644 --- a/acme/src/main/java/io/micronaut/acme/events/CertificateEvent.java +++ b/acme/src/main/java/io/micronaut/acme/events/CertificateEvent.java @@ -27,19 +27,28 @@ public class CertificateEvent { private boolean validationCert; /** + * @deprecated See constructor that takes full certificate chain instead. + * * Creates a new CertificateEvent. * @param certificate X509 certificate file * @param domainKeyPair key pair used to encrypt the certificate * @param validationCert if this certificate is to be used for tls-apln-01 account validation */ + @Deprecated public CertificateEvent(X509Certificate certificate, KeyPair domainKeyPair, boolean validationCert) { this.domainKeyPair = domainKeyPair; this.validationCert = validationCert; this.fullCertificateChain = new X509Certificate[]{certificate}; } - public CertificateEvent(KeyPair domainKeyPair, X509Certificate... fullCertificateChain) { - this.validationCert = false; + /** + * Creates a new CertificateEvent containing the full certificate chain + * @param domainKeyPair key pair used to encrypt the certificate + * @param validationCert if this certificate is to be used for tls-apln-01 account validation + * @param fullCertificateChain X509 certificate file + */ + public CertificateEvent(KeyPair domainKeyPair, boolean validationCert, X509Certificate... fullCertificateChain) { + this.validationCert = validationCert; this.domainKeyPair = domainKeyPair; this.fullCertificateChain = fullCertificateChain; } 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 775c1e2d..a685340a 100644 --- a/acme/src/main/java/io/micronaut/acme/services/AcmeService.java +++ b/acme/src/main/java/io/micronaut/acme/services/AcmeService.java @@ -297,7 +297,7 @@ private boolean writeCombinedFile(Certificate certificate) { try (BufferedWriter writer = Files.newBufferedWriter(domainCsr.toPath(), WRITE, CREATE, TRUNCATE_EXISTING)) { certificate.writeCertificate(writer); } - eventPublisher.publishEvent(new CertificateEvent(domainKeyPair, getFullCertificateChain())); + eventPublisher.publishEvent(new CertificateEvent(domainKeyPair, false, getFullCertificateChain())); if (LOG.isInfoEnabled()) { LOG.info("ACME certificate order success! Certificate URL: {}", certificate.getLocation()); } @@ -481,7 +481,7 @@ private void doChallengeSpecificSetup(Authorization auth, Challenge challenge) t } KeyPair domainKeyPair = getDomainKeyPair(); X509Certificate tlsAlpn01Certificate = CertificateUtils.createTlsAlpn01Certificate(domainKeyPair, auth.getIdentifier(), ((TlsAlpn01Challenge) challenge).getAcmeValidation()); - eventPublisher.publishEvent(new CertificateEvent(tlsAlpn01Certificate, domainKeyPair, true)); + eventPublisher.publishEvent(new CertificateEvent(domainKeyPair, true, tlsAlpn01Certificate)); } else if (challenge instanceof Http01Challenge) { Http01Challenge http01Challenge = (Http01Challenge) challenge; eventPublisher.publishEvent(new HttpChallengeDetails(http01Challenge.getToken(), http01Challenge.getAuthorization())); @@ -501,7 +501,7 @@ private void doChallengeSpecificSetup(Authorization auth, Challenge challenge) t * Setup the certificate that has been saved to disk and configures it for use. */ public void setupCurrentCertificate() { - eventPublisher.publishEvent(new CertificateEvent(getDomainKeyPair(), getFullCertificateChain())); + eventPublisher.publishEvent(new CertificateEvent(getDomainKeyPair(), false, getFullCertificateChain())); } /** diff --git a/acme/src/test/groovy/io/micronaut/acme/events/CertificateEventSpec.groovy b/acme/src/test/groovy/io/micronaut/acme/events/CertificateEventSpec.groovy index 92c6e604..adca44d8 100644 --- a/acme/src/test/groovy/io/micronaut/acme/events/CertificateEventSpec.groovy +++ b/acme/src/test/groovy/io/micronaut/acme/events/CertificateEventSpec.groovy @@ -108,12 +108,14 @@ ${DOMAIN_CERT} X509Certificate domainCert = cf.generateCertificate(new ByteArrayInputStream(FULL_CHAIN_CERT.bytes)) Collection certs = cf.generateCertificates(new ByteArrayInputStream(FULL_CHAIN_CERT.bytes)) KeyPair keyPair = KeyPairUtils.createKeyPair(2048) + def expectedValidationCert = new Random().nextBoolean() when : - CertificateEvent event = new CertificateEvent(keyPair, certs as X509Certificate[]) + CertificateEvent event = new CertificateEvent(keyPair, expectedValidationCert, certs as X509Certificate[]) then: event.getCert() == domainCert + event.isValidationCert() == expectedValidationCert event.getFullCertificateChain().length == 2 event.getFullCertificateChain() == certs.toArray() } From 8014c6ed78c912886f0a453092627b6c12bf2b97 Mon Sep 17 00:00:00 2001 From: Nathan Zender Date: Sun, 6 Mar 2022 20:52:54 -0500 Subject: [PATCH 04/14] Cleanup checkstyle issues --- .../java/io/micronaut/acme/events/CertificateEvent.java | 7 ++++++- .../main/java/io/micronaut/acme/services/AcmeService.java | 7 +++++-- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/acme/src/main/java/io/micronaut/acme/events/CertificateEvent.java b/acme/src/main/java/io/micronaut/acme/events/CertificateEvent.java index 3127cb39..e85241d8 100644 --- a/acme/src/main/java/io/micronaut/acme/events/CertificateEvent.java +++ b/acme/src/main/java/io/micronaut/acme/events/CertificateEvent.java @@ -42,7 +42,7 @@ public CertificateEvent(X509Certificate certificate, KeyPair domainKeyPair, bool } /** - * Creates a new CertificateEvent containing the full certificate chain + * Creates a new CertificateEvent containing the full certificate chain. * @param domainKeyPair key pair used to encrypt the certificate * @param validationCert if this certificate is to be used for tls-apln-01 account validation * @param fullCertificateChain X509 certificate file @@ -74,6 +74,11 @@ public boolean isValidationCert() { return validationCert; } + /** + * Return the full certificate chain. + * + * @return array of certificates in the chain. + */ public X509Certificate[] getFullCertificateChain() { return fullCertificateChain; } 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 a685340a..5b40ed9c 100644 --- a/acme/src/main/java/io/micronaut/acme/services/AcmeService.java +++ b/acme/src/main/java/io/micronaut/acme/services/AcmeService.java @@ -47,8 +47,6 @@ import java.security.cert.X509Certificate; import java.time.Duration; import java.time.Instant; -import java.util.ArrayList; -import java.util.Collection; import java.util.List; import java.util.Optional; import java.util.concurrent.CancellationException; @@ -140,6 +138,11 @@ public X509Certificate getCurrentCertificate() { } } + /** + * Returns the full certificate chain. + * + * @return array of each of the certificates in the chain + */ protected X509Certificate[] getFullCertificateChain() { try { CertificateFactory cf = CertificateFactory.getInstance("X.509"); From 84db7da34769c2c6423db67454f9482ef2b59c4b Mon Sep 17 00:00:00 2001 From: Nathan Zender Date: Mon, 7 Mar 2022 13:23:36 -0500 Subject: [PATCH 05/14] Better javadoc --- .../main/java/io/micronaut/acme/events/CertificateEvent.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/acme/src/main/java/io/micronaut/acme/events/CertificateEvent.java b/acme/src/main/java/io/micronaut/acme/events/CertificateEvent.java index e85241d8..1c7f3aef 100644 --- a/acme/src/main/java/io/micronaut/acme/events/CertificateEvent.java +++ b/acme/src/main/java/io/micronaut/acme/events/CertificateEvent.java @@ -27,7 +27,7 @@ public class CertificateEvent { private boolean validationCert; /** - * @deprecated See constructor that takes full certificate chain instead. + * @deprecated {@link #CertificateEvent(KeyPair, boolean, X509Certificate...)} instead. * * Creates a new CertificateEvent. * @param certificate X509 certificate file From 16b05b6e8c8bbd5488dbbed41345b60c623ffbd5 Mon Sep 17 00:00:00 2001 From: Nathan Zender Date: Mon, 7 Mar 2022 18:10:38 -0500 Subject: [PATCH 06/14] Using constants --- .../java/io/micronaut/acme/services/AcmeService.java | 5 +++-- .../micronaut/acme/events/CertificateEventSpec.groovy | 10 ++++++---- 2 files changed, 9 insertions(+), 6 deletions(-) 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 5b40ed9c..6c2fbc98 100644 --- a/acme/src/main/java/io/micronaut/acme/services/AcmeService.java +++ b/acme/src/main/java/io/micronaut/acme/services/AcmeService.java @@ -67,6 +67,7 @@ public class AcmeService { private static final Logger LOG = LoggerFactory.getLogger(AcmeService.class); private static final String DOMAIN_CRT = "domain.crt"; private static final String DOMAIN_CSR = "domain.csr"; + private static final String X509_CERT = "X.509"; /** * Let's Encrypt has different production vs test servers. @@ -123,7 +124,7 @@ public AcmeService(ApplicationEventPublisher eventPublisher, */ public X509Certificate getCurrentCertificate() { try { - CertificateFactory cf = CertificateFactory.getInstance("X.509"); + CertificateFactory cf = CertificateFactory.getInstance(X509_CERT); File certificate = new File(certLocation, DOMAIN_CRT); if (certificate.exists()) { return (X509Certificate) cf.generateCertificate(Files.newInputStream(certificate.toPath())); @@ -145,7 +146,7 @@ public X509Certificate getCurrentCertificate() { */ protected X509Certificate[] getFullCertificateChain() { try { - CertificateFactory cf = CertificateFactory.getInstance("X.509"); + CertificateFactory cf = CertificateFactory.getInstance(X509_CERT); File certificate = new File(certLocation, DOMAIN_CRT); if (certificate.exists()) { return cf.generateCertificates(Files.newInputStream(certificate.toPath())).toArray(new X509Certificate[0]); diff --git a/acme/src/test/groovy/io/micronaut/acme/events/CertificateEventSpec.groovy b/acme/src/test/groovy/io/micronaut/acme/events/CertificateEventSpec.groovy index adca44d8..736030fe 100644 --- a/acme/src/test/groovy/io/micronaut/acme/events/CertificateEventSpec.groovy +++ b/acme/src/test/groovy/io/micronaut/acme/events/CertificateEventSpec.groovy @@ -9,6 +9,8 @@ import java.security.cert.CertificateFactory import java.security.cert.X509Certificate class CertificateEventSpec extends Specification { + static final String X509_CERT = "X.509" + def DOMAIN_CERT = """ -----BEGIN CERTIFICATE----- MIIDUDCCAjigAwIBAgIIHuspA0mthF8wDQYJKoZIhvcNAQELBQAwIDEeMBwGA1UE @@ -62,7 +64,7 @@ ${DOMAIN_CERT} def "can get domain keypair"(){ given: - CertificateFactory cf = CertificateFactory.getInstance("X.509") + CertificateFactory cf = CertificateFactory.getInstance(X509_CERT) X509Certificate cert = cf.generateCertificate(new ByteArrayInputStream(FULL_CHAIN_CERT.bytes)) KeyPair keyPair = KeyPairUtils.createKeyPair(2048) @@ -75,7 +77,7 @@ ${DOMAIN_CERT} def "can determine if the event is a validation certificate or not"(){ given: - CertificateFactory cf = CertificateFactory.getInstance("X.509") + CertificateFactory cf = CertificateFactory.getInstance(X509_CERT) X509Certificate cert = cf.generateCertificate(new ByteArrayInputStream(FULL_CHAIN_CERT.bytes)) KeyPair keyPair = KeyPairUtils.createKeyPair(2048) def validationCert = new Random().nextBoolean() @@ -89,7 +91,7 @@ ${DOMAIN_CERT} def "when pass single cert the full chain only contains that cert"(){ given: - CertificateFactory cf = CertificateFactory.getInstance("X.509") + CertificateFactory cf = CertificateFactory.getInstance(X509_CERT) X509Certificate domainCert = cf.generateCertificate(new ByteArrayInputStream(FULL_CHAIN_CERT.bytes)) KeyPair keyPair = KeyPairUtils.createKeyPair(2048) @@ -104,7 +106,7 @@ ${DOMAIN_CERT} def "when full certificate chain passed we can still get the domain specific cert"(){ given: - CertificateFactory cf = CertificateFactory.getInstance("X.509") + CertificateFactory cf = CertificateFactory.getInstance(X509_CERT) X509Certificate domainCert = cf.generateCertificate(new ByteArrayInputStream(FULL_CHAIN_CERT.bytes)) Collection certs = cf.generateCertificates(new ByteArrayInputStream(FULL_CHAIN_CERT.bytes)) KeyPair keyPair = KeyPairUtils.createKeyPair(2048) From 075cb6cd314f4d3d933115e453b9cad544a2c4b7 Mon Sep 17 00:00:00 2001 From: Nathan Zender Date: Mon, 7 Mar 2022 18:10:58 -0500 Subject: [PATCH 07/14] Groovy-ify and use types more --- .../acme/AcmeCertRefresherMultiDomainsTaskSpec.groovy | 8 ++++---- .../io/micronaut/acme/AcmeCertRefresherTaskSpec.groovy | 8 ++++---- .../AcmeCertRefresherTaskWithClasspathKeysSpec.groovy | 8 ++++---- .../acme/AcmeCertRefresherTaskWithFileKeysSpec.groovy | 8 ++++---- .../acme/AcmeCertWildcardRefresherTaskSpec.groovy | 8 ++++---- .../AcmeCertRefresherTaskHttp01ChallengeSpec.groovy | 8 ++++---- .../AcmeCertRefresherTaskTlsApln01ChallengeSpec.groovy | 8 ++++---- .../io/micronaut/acme/events/CertificateEventSpec.groovy | 4 ++-- 8 files changed, 30 insertions(+), 30 deletions(-) diff --git a/acme/src/test/groovy/io/micronaut/acme/AcmeCertRefresherMultiDomainsTaskSpec.groovy b/acme/src/test/groovy/io/micronaut/acme/AcmeCertRefresherMultiDomainsTaskSpec.groovy index 6a84ed62..f28dd96f 100644 --- a/acme/src/test/groovy/io/micronaut/acme/AcmeCertRefresherMultiDomainsTaskSpec.groovy +++ b/acme/src/test/groovy/io/micronaut/acme/AcmeCertRefresherMultiDomainsTaskSpec.groovy @@ -53,16 +53,16 @@ class AcmeCertRefresherMultiDomainsTaskSpec extends AcmeBaseSpec { conn.connect() Certificate[] certs = conn.getServerCertificates() certs.length == 2 - def cert = (X509Certificate) certs[0] + X509Certificate cert = certs[0] cert.getIssuerDN().getName().contains("Pebble Intermediate CA") cert.getSubjectDN().getName().contains(EXPECTED_DOMAIN) cert.getSubjectAlternativeNames().size() == 2 cert.getSubjectAlternativeNames().collect({d-> d.get(1)}).contains(EXPECTED_DOMAIN) cert.getSubjectAlternativeNames().collect({d-> d.get(1)}).contains(EXPECTED_ACME_DOMAIN) - def cert2 = (X509Certificate) certs[1] - cert2.getIssuerDN().getName().contains("Pebble Root CA") - cert2.getSubjectDN().getName().contains("Pebble Intermediate CA") + X509Certificate cert2 = certs[1] + cert2.issuerDN.name.contains("Pebble Root CA") + cert2.subjectDN.name.contains("Pebble Intermediate CA") }finally{ if(conn != null){ conn.disconnect() diff --git a/acme/src/test/groovy/io/micronaut/acme/AcmeCertRefresherTaskSpec.groovy b/acme/src/test/groovy/io/micronaut/acme/AcmeCertRefresherTaskSpec.groovy index 7f9716e4..a69992eb 100644 --- a/acme/src/test/groovy/io/micronaut/acme/AcmeCertRefresherTaskSpec.groovy +++ b/acme/src/test/groovy/io/micronaut/acme/AcmeCertRefresherTaskSpec.groovy @@ -53,14 +53,14 @@ class AcmeCertRefresherTaskSpec extends AcmeBaseSpec { conn.connect() Certificate[] certs = conn.getServerCertificates() certs.length == 2 - def cert = (X509Certificate) certs[0] + X509Certificate cert = certs[0] cert.getIssuerDN().getName().contains("Pebble Intermediate CA") cert.getSubjectDN().getName().contains(EXPECTED_DOMAIN) cert.getSubjectAlternativeNames().size() == 1 - def cert2 = (X509Certificate) certs[1] - cert2.getIssuerDN().getName().contains("Pebble Root CA") - cert2.getSubjectDN().getName().contains("Pebble Intermediate CA") + X509Certificate cert2 = certs[1] + cert2.issuerDN.name.contains("Pebble Root CA") + cert2.subjectDN.name.contains("Pebble Intermediate CA") }finally{ if(conn != null){ conn.disconnect() diff --git a/acme/src/test/groovy/io/micronaut/acme/AcmeCertRefresherTaskWithClasspathKeysSpec.groovy b/acme/src/test/groovy/io/micronaut/acme/AcmeCertRefresherTaskWithClasspathKeysSpec.groovy index bae29063..4f2187e0 100644 --- a/acme/src/test/groovy/io/micronaut/acme/AcmeCertRefresherTaskWithClasspathKeysSpec.groovy +++ b/acme/src/test/groovy/io/micronaut/acme/AcmeCertRefresherTaskWithClasspathKeysSpec.groovy @@ -68,14 +68,14 @@ class AcmeCertRefresherTaskWithClasspathKeysSpec extends AcmeBaseSpec { conn.connect() Certificate[] certs = conn.getServerCertificates() certs.length == 2 - def cert = (X509Certificate) certs[0] + X509Certificate cert = certs[0] cert.getIssuerDN().getName().contains("Pebble Intermediate CA") cert.getSubjectDN().getName().contains(EXPECTED_DOMAIN) cert.getSubjectAlternativeNames().size() == 1 - def cert2 = (X509Certificate) certs[1] - cert2.getIssuerDN().getName().contains("Pebble Root CA") - cert2.getSubjectDN().getName().contains("Pebble Intermediate CA") + X509Certificate cert2 = certs[1] + cert2.issuerDN.name.contains("Pebble Root CA") + cert2.subjectDN.name.contains("Pebble Intermediate CA") }finally{ if(conn != null){ conn.disconnect() diff --git a/acme/src/test/groovy/io/micronaut/acme/AcmeCertRefresherTaskWithFileKeysSpec.groovy b/acme/src/test/groovy/io/micronaut/acme/AcmeCertRefresherTaskWithFileKeysSpec.groovy index 70a96da6..4b4e64cc 100644 --- a/acme/src/test/groovy/io/micronaut/acme/AcmeCertRefresherTaskWithFileKeysSpec.groovy +++ b/acme/src/test/groovy/io/micronaut/acme/AcmeCertRefresherTaskWithFileKeysSpec.groovy @@ -62,14 +62,14 @@ class AcmeCertRefresherTaskWithFileKeysSpec extends AcmeBaseSpec { conn.connect() Certificate[] certs = conn.getServerCertificates() certs.length == 2 - def cert = (X509Certificate) certs[0] + X509Certificate cert = certs[0] cert.getIssuerDN().getName().contains("Pebble Intermediate CA") cert.getSubjectDN().getName().contains(EXPECTED_DOMAIN) cert.getSubjectAlternativeNames().size() == 1 - def cert2 = (X509Certificate) certs[1] - cert2.getIssuerDN().getName().contains("Pebble Root CA") - cert2.getSubjectDN().getName().contains("Pebble Intermediate CA") + X509Certificate cert2 = certs[1] + cert2.issuerDN.name.contains("Pebble Root CA") + cert2.subjectDN.name.contains("Pebble Intermediate CA") }finally{ if(conn != null){ conn.disconnect() diff --git a/acme/src/test/groovy/io/micronaut/acme/AcmeCertWildcardRefresherTaskSpec.groovy b/acme/src/test/groovy/io/micronaut/acme/AcmeCertWildcardRefresherTaskSpec.groovy index 9671391d..d41a4ed3 100644 --- a/acme/src/test/groovy/io/micronaut/acme/AcmeCertWildcardRefresherTaskSpec.groovy +++ b/acme/src/test/groovy/io/micronaut/acme/AcmeCertWildcardRefresherTaskSpec.groovy @@ -58,16 +58,16 @@ class AcmeCertWildcardRefresherTaskSpec extends AcmeBaseSpec { conn.connect() Certificate[] certs = conn.getServerCertificates() certs.length == 2 - def cert = (X509Certificate) certs[0] + X509Certificate cert = certs[0] cert.getIssuerDN().getName().contains("Pebble Intermediate CA") cert.getSubjectDN().getName().contains(WILDCARD_DOMAIN) cert.getSubjectAlternativeNames().size() == 2 cert.getSubjectAlternativeNames().collect({d-> d.get(1)}).contains(WILDCARD_DOMAIN) cert.getSubjectAlternativeNames().collect({d-> d.get(1)}).contains(EXPECTED_BASE_DOMAIN) - def cert2 = (X509Certificate) certs[1] - cert2.getIssuerDN().getName().contains("Pebble Root CA") - cert2.getSubjectDN().getName().contains("Pebble Intermediate CA") + X509Certificate cert2 = certs[1] + cert2.issuerDN.name.contains("Pebble Root CA") + cert2.subjectDN.name.contains("Pebble Intermediate CA") }finally{ if(conn != null){ conn.disconnect() diff --git a/acme/src/test/groovy/io/micronaut/acme/challenges/AcmeCertRefresherTaskHttp01ChallengeSpec.groovy b/acme/src/test/groovy/io/micronaut/acme/challenges/AcmeCertRefresherTaskHttp01ChallengeSpec.groovy index 91414ed0..ee5e611b 100644 --- a/acme/src/test/groovy/io/micronaut/acme/challenges/AcmeCertRefresherTaskHttp01ChallengeSpec.groovy +++ b/acme/src/test/groovy/io/micronaut/acme/challenges/AcmeCertRefresherTaskHttp01ChallengeSpec.groovy @@ -62,14 +62,14 @@ class AcmeCertRefresherTaskHttp01ChallengeSpec extends AcmeBaseSpec { then: "we make sure they are from the pebble test server and the domain is as expected" certs.length == 2 - def cert = (X509Certificate) certs[0] + X509Certificate cert = certs[0] cert.getIssuerDN().getName().contains("Pebble Intermediate CA") cert.getSubjectDN().getName().contains(EXPECTED_ACME_DOMAIN) cert.getSubjectAlternativeNames().size() == 1 - def cert2 = (X509Certificate) certs[1] - cert2.getIssuerDN().getName().contains("Pebble Root CA") - cert2.getSubjectDN().getName().contains("Pebble Intermediate CA") + X509Certificate cert2 = certs[1] + cert2.issuerDN.name.contains("Pebble Root CA") + cert2.subjectDN.name.contains("Pebble Intermediate CA") } void "test send https request when the cert is in place"() { diff --git a/acme/src/test/groovy/io/micronaut/acme/challenges/AcmeCertRefresherTaskTlsApln01ChallengeSpec.groovy b/acme/src/test/groovy/io/micronaut/acme/challenges/AcmeCertRefresherTaskTlsApln01ChallengeSpec.groovy index b70403ac..cb6515e0 100644 --- a/acme/src/test/groovy/io/micronaut/acme/challenges/AcmeCertRefresherTaskTlsApln01ChallengeSpec.groovy +++ b/acme/src/test/groovy/io/micronaut/acme/challenges/AcmeCertRefresherTaskTlsApln01ChallengeSpec.groovy @@ -60,14 +60,14 @@ class AcmeCertRefresherTaskTlsApln01ChallengeSpec extends AcmeBaseSpec { then: "we make sure they are from the pebble test server and the domain is as expected" certs.length == 2 - def cert = (X509Certificate) certs[0] + X509Certificate cert = certs[0] cert.getIssuerDN().getName().contains("Pebble Intermediate CA") cert.getSubjectDN().getName().contains(EXPECTED_ACME_DOMAIN) cert.getSubjectAlternativeNames().size() == 1 - def cert2 = (X509Certificate) certs[1] - cert2.getIssuerDN().getName().contains("Pebble Root CA") - cert2.getSubjectDN().getName().contains("Pebble Intermediate CA") + X509Certificate cert2 = certs[1] + cert2.issuerDN.name.contains("Pebble Root CA") + cert2.subjectDN.name.contains("Pebble Intermediate CA") } void "test send https request when the cert is in place"() { diff --git a/acme/src/test/groovy/io/micronaut/acme/events/CertificateEventSpec.groovy b/acme/src/test/groovy/io/micronaut/acme/events/CertificateEventSpec.groovy index 736030fe..04c21d90 100644 --- a/acme/src/test/groovy/io/micronaut/acme/events/CertificateEventSpec.groovy +++ b/acme/src/test/groovy/io/micronaut/acme/events/CertificateEventSpec.groovy @@ -80,7 +80,7 @@ ${DOMAIN_CERT} CertificateFactory cf = CertificateFactory.getInstance(X509_CERT) X509Certificate cert = cf.generateCertificate(new ByteArrayInputStream(FULL_CHAIN_CERT.bytes)) KeyPair keyPair = KeyPairUtils.createKeyPair(2048) - def validationCert = new Random().nextBoolean() + boolean validationCert = new Random().nextBoolean() when : CertificateEvent event = new CertificateEvent(cert, keyPair, validationCert) @@ -110,7 +110,7 @@ ${DOMAIN_CERT} X509Certificate domainCert = cf.generateCertificate(new ByteArrayInputStream(FULL_CHAIN_CERT.bytes)) Collection certs = cf.generateCertificates(new ByteArrayInputStream(FULL_CHAIN_CERT.bytes)) KeyPair keyPair = KeyPairUtils.createKeyPair(2048) - def expectedValidationCert = new Random().nextBoolean() + boolean expectedValidationCert = new Random().nextBoolean() when : CertificateEvent event = new CertificateEvent(keyPair, expectedValidationCert, certs as X509Certificate[]) From e96045515e7a2ff8239505cbe82abd2435e8983f Mon Sep 17 00:00:00 2001 From: Nathan Zender Date: Mon, 7 Mar 2022 18:16:04 -0500 Subject: [PATCH 08/14] Always return an array --- acme/src/main/java/io/micronaut/acme/services/AcmeService.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 6c2fbc98..d1e8a102 100644 --- a/acme/src/main/java/io/micronaut/acme/services/AcmeService.java +++ b/acme/src/main/java/io/micronaut/acme/services/AcmeService.java @@ -157,7 +157,7 @@ protected X509Certificate[] getFullCertificateChain() { if (LOG.isWarnEnabled()) { LOG.warn("Could not create certificate from file", e); } - return null; + return new X509Certificate[]{}; } } From 67394be7d076e149a7c4b43006f02457ce9850c7 Mon Sep 17 00:00:00 2001 From: Nathan Zender Date: Tue, 8 Mar 2022 07:44:25 -0500 Subject: [PATCH 09/14] Add @NonNull --- acme/src/main/java/io/micronaut/acme/services/AcmeService.java | 2 ++ 1 file changed, 2 insertions(+) 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 d1e8a102..06d22b66 100644 --- a/acme/src/main/java/io/micronaut/acme/services/AcmeService.java +++ b/acme/src/main/java/io/micronaut/acme/services/AcmeService.java @@ -20,6 +20,7 @@ import io.micronaut.acme.challenge.http.endpoint.HttpChallengeDetails; import io.micronaut.acme.events.CertificateEvent; import io.micronaut.context.event.ApplicationEventPublisher; +import io.micronaut.core.annotation.NonNull; import io.micronaut.core.io.IOUtils; import io.micronaut.core.io.ResourceResolver; import io.micronaut.scheduling.TaskScheduler; @@ -144,6 +145,7 @@ public X509Certificate getCurrentCertificate() { * * @return array of each of the certificates in the chain */ + @NonNull protected X509Certificate[] getFullCertificateChain() { try { CertificateFactory cf = CertificateFactory.getInstance(X509_CERT); From 3b9718d5c8123729725bd365830d80c21d7471d9 Mon Sep 17 00:00:00 2001 From: yawkat Date: Mon, 14 Mar 2022 10:22:05 +0100 Subject: [PATCH 10/14] stop using the old CertificateEvent constructor to fix warning --- .../io/micronaut/acme/events/CertificateEventSpec.groovy | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/acme/src/test/groovy/io/micronaut/acme/events/CertificateEventSpec.groovy b/acme/src/test/groovy/io/micronaut/acme/events/CertificateEventSpec.groovy index 04c21d90..b5d08c1a 100644 --- a/acme/src/test/groovy/io/micronaut/acme/events/CertificateEventSpec.groovy +++ b/acme/src/test/groovy/io/micronaut/acme/events/CertificateEventSpec.groovy @@ -69,7 +69,7 @@ ${DOMAIN_CERT} KeyPair keyPair = KeyPairUtils.createKeyPair(2048) when : - CertificateEvent event = new CertificateEvent(cert, keyPair, new Random().nextBoolean()) + CertificateEvent event = new CertificateEvent(keyPair, new Random().nextBoolean(), cert) then: event.getDomainKeyPair() == keyPair @@ -83,7 +83,7 @@ ${DOMAIN_CERT} boolean validationCert = new Random().nextBoolean() when : - CertificateEvent event = new CertificateEvent(cert, keyPair, validationCert) + CertificateEvent event = new CertificateEvent(keyPair, validationCert, cert) then: event.isValidationCert() == validationCert @@ -96,7 +96,7 @@ ${DOMAIN_CERT} KeyPair keyPair = KeyPairUtils.createKeyPair(2048) when : - CertificateEvent event = new CertificateEvent(domainCert, keyPair, new Random().nextBoolean()) + CertificateEvent event = new CertificateEvent(keyPair, new Random().nextBoolean(), domainCert) then: event.getCert() == domainCert From 688ccb22391232948ceaafb07d423afe53cdee9d Mon Sep 17 00:00:00 2001 From: yawkat Date: Mon, 14 Mar 2022 10:25:03 +0100 Subject: [PATCH 11/14] checked cast from generateCertificates --- .../src/main/java/io/micronaut/acme/services/AcmeService.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) 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 06d22b66..81a5528d 100644 --- a/acme/src/main/java/io/micronaut/acme/services/AcmeService.java +++ b/acme/src/main/java/io/micronaut/acme/services/AcmeService.java @@ -151,7 +151,9 @@ protected X509Certificate[] getFullCertificateChain() { CertificateFactory cf = CertificateFactory.getInstance(X509_CERT); File certificate = new File(certLocation, DOMAIN_CRT); if (certificate.exists()) { - return cf.generateCertificates(Files.newInputStream(certificate.toPath())).toArray(new X509Certificate[0]); + return cf.generateCertificates(Files.newInputStream(certificate.toPath())).stream() + .map(X509Certificate.class::cast) + .toArray(X509Certificate[]::new); } else { return new X509Certificate[]{}; } From ff5e0446e0728fbb25c2d652766f5729a298d781 Mon Sep 17 00:00:00 2001 From: yawkat Date: Mon, 14 Mar 2022 10:27:03 +0100 Subject: [PATCH 12/14] add a check that the certificates in CertificateEvent aren't empty --- .../main/java/io/micronaut/acme/events/CertificateEvent.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/acme/src/main/java/io/micronaut/acme/events/CertificateEvent.java b/acme/src/main/java/io/micronaut/acme/events/CertificateEvent.java index 1c7f3aef..f8dae604 100644 --- a/acme/src/main/java/io/micronaut/acme/events/CertificateEvent.java +++ b/acme/src/main/java/io/micronaut/acme/events/CertificateEvent.java @@ -48,6 +48,9 @@ public CertificateEvent(X509Certificate certificate, KeyPair domainKeyPair, bool * @param fullCertificateChain X509 certificate file */ public CertificateEvent(KeyPair domainKeyPair, boolean validationCert, X509Certificate... fullCertificateChain) { + if (fullCertificateChain == null || fullCertificateChain.length == 0) { + throw new IllegalArgumentException("Certificate chain must not be empty"); + } this.validationCert = validationCert; this.domainKeyPair = domainKeyPair; this.fullCertificateChain = fullCertificateChain; From a33236c507b49eac154176e835499ad2b9a72aa6 Mon Sep 17 00:00:00 2001 From: yawkat Date: Mon, 14 Mar 2022 10:46:47 +0100 Subject: [PATCH 13/14] handle empty cert array --- .../micronaut/acme/services/AcmeService.java | 31 ++++++++++++++----- 1 file changed, 23 insertions(+), 8 deletions(-) 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 81a5528d..5d1d9618 100644 --- a/acme/src/main/java/io/micronaut/acme/services/AcmeService.java +++ b/acme/src/main/java/io/micronaut/acme/services/AcmeService.java @@ -20,7 +20,7 @@ import io.micronaut.acme.challenge.http.endpoint.HttpChallengeDetails; import io.micronaut.acme.events.CertificateEvent; import io.micronaut.context.event.ApplicationEventPublisher; -import io.micronaut.core.annotation.NonNull; +import io.micronaut.core.annotation.Nullable; import io.micronaut.core.io.IOUtils; import io.micronaut.core.io.ResourceResolver; import io.micronaut.scheduling.TaskScheduler; @@ -145,7 +145,7 @@ public X509Certificate getCurrentCertificate() { * * @return array of each of the certificates in the chain */ - @NonNull + @Nullable protected X509Certificate[] getFullCertificateChain() { try { CertificateFactory cf = CertificateFactory.getInstance(X509_CERT); @@ -155,13 +155,13 @@ protected X509Certificate[] getFullCertificateChain() { .map(X509Certificate.class::cast) .toArray(X509Certificate[]::new); } else { - return new X509Certificate[]{}; + return null; } } catch (CertificateException | IOException e) { if (LOG.isWarnEnabled()) { LOG.warn("Could not create certificate from file", e); } - return new X509Certificate[]{}; + return null; } } @@ -305,9 +305,17 @@ private boolean writeCombinedFile(Certificate certificate) { try (BufferedWriter writer = Files.newBufferedWriter(domainCsr.toPath(), WRITE, CREATE, TRUNCATE_EXISTING)) { certificate.writeCertificate(writer); } - eventPublisher.publishEvent(new CertificateEvent(domainKeyPair, false, getFullCertificateChain())); - if (LOG.isInfoEnabled()) { - LOG.info("ACME certificate order success! Certificate URL: {}", certificate.getLocation()); + X509Certificate[] chain = getFullCertificateChain(); + if (chain != null) { + eventPublisher.publishEvent(new CertificateEvent(domainKeyPair, false, chain)); + if (LOG.isInfoEnabled()) { + LOG.info("ACME certificate order success! Certificate URL: {}", certificate.getLocation()); + } + } else { + if (LOG.isErrorEnabled()) { + LOG.error("ACME certificate chain could not be loaded from file."); + } + result = true; } } catch (IOException e) { if (LOG.isErrorEnabled()) { @@ -509,7 +517,14 @@ private void doChallengeSpecificSetup(Authorization auth, Challenge challenge) t * Setup the certificate that has been saved to disk and configures it for use. */ public void setupCurrentCertificate() { - eventPublisher.publishEvent(new CertificateEvent(getDomainKeyPair(), false, getFullCertificateChain())); + X509Certificate[] fullCertificateChain = getFullCertificateChain(); + if (fullCertificateChain == null) { + if (LOG.isErrorEnabled()) { + LOG.error("ACME certificate chain could not be loaded from file."); + } + } else { + eventPublisher.publishEvent(new CertificateEvent(getDomainKeyPair(), false, fullCertificateChain)); + } } /** From 6a5dd4466aadfa3ee97dec776869d8940a036cdc Mon Sep 17 00:00:00 2001 From: Sergio del Amo Date: Wed, 16 Mar 2022 18:30:37 +0100 Subject: [PATCH 14/14] Change AcmeService to return Optional --- .../acme/events/CertificateEvent.java | 2 ++ .../micronaut/acme/services/AcmeService.java | 28 +++++++++---------- 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/acme/src/main/java/io/micronaut/acme/events/CertificateEvent.java b/acme/src/main/java/io/micronaut/acme/events/CertificateEvent.java index f8dae604..3b53d3f0 100644 --- a/acme/src/main/java/io/micronaut/acme/events/CertificateEvent.java +++ b/acme/src/main/java/io/micronaut/acme/events/CertificateEvent.java @@ -15,6 +15,7 @@ */ package io.micronaut.acme.events; +import io.micronaut.core.annotation.NonNull; import java.security.KeyPair; import java.security.cert.X509Certificate; @@ -82,6 +83,7 @@ public boolean isValidationCert() { * * @return array of certificates in the chain. */ + @NonNull public X509Certificate[] getFullCertificateChain() { return fullCertificateChain; } 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 5d1d9618..cadbb7ea 100644 --- a/acme/src/main/java/io/micronaut/acme/services/AcmeService.java +++ b/acme/src/main/java/io/micronaut/acme/services/AcmeService.java @@ -20,7 +20,7 @@ import io.micronaut.acme.challenge.http.endpoint.HttpChallengeDetails; import io.micronaut.acme.events.CertificateEvent; import io.micronaut.context.event.ApplicationEventPublisher; -import io.micronaut.core.annotation.Nullable; +import io.micronaut.core.annotation.NonNull; import io.micronaut.core.io.IOUtils; import io.micronaut.core.io.ResourceResolver; import io.micronaut.scheduling.TaskScheduler; @@ -145,24 +145,22 @@ public X509Certificate getCurrentCertificate() { * * @return array of each of the certificates in the chain */ - @Nullable - protected X509Certificate[] getFullCertificateChain() { + @NonNull + protected Optional getFullCertificateChain() { try { CertificateFactory cf = CertificateFactory.getInstance(X509_CERT); File certificate = new File(certLocation, DOMAIN_CRT); if (certificate.exists()) { - return cf.generateCertificates(Files.newInputStream(certificate.toPath())).stream() + return Optional.of(cf.generateCertificates(Files.newInputStream(certificate.toPath())).stream() .map(X509Certificate.class::cast) - .toArray(X509Certificate[]::new); - } else { - return null; + .toArray(X509Certificate[]::new)); } } catch (CertificateException | IOException e) { if (LOG.isWarnEnabled()) { LOG.warn("Could not create certificate from file", e); } - return null; } + return Optional.empty(); } /** @@ -305,9 +303,9 @@ private boolean writeCombinedFile(Certificate certificate) { try (BufferedWriter writer = Files.newBufferedWriter(domainCsr.toPath(), WRITE, CREATE, TRUNCATE_EXISTING)) { certificate.writeCertificate(writer); } - X509Certificate[] chain = getFullCertificateChain(); - if (chain != null) { - eventPublisher.publishEvent(new CertificateEvent(domainKeyPair, false, chain)); + Optional chainOptional = getFullCertificateChain(); + if (chainOptional.isPresent()) { + eventPublisher.publishEvent(new CertificateEvent(domainKeyPair, false, chainOptional.get())); if (LOG.isInfoEnabled()) { LOG.info("ACME certificate order success! Certificate URL: {}", certificate.getLocation()); } @@ -517,13 +515,13 @@ private void doChallengeSpecificSetup(Authorization auth, Challenge challenge) t * Setup the certificate that has been saved to disk and configures it for use. */ public void setupCurrentCertificate() { - X509Certificate[] fullCertificateChain = getFullCertificateChain(); - if (fullCertificateChain == null) { + Optional fullCertificateChainOptional = getFullCertificateChain(); + if (fullCertificateChainOptional.isPresent()) { + eventPublisher.publishEvent(new CertificateEvent(getDomainKeyPair(), false, fullCertificateChainOptional.get())); + } else { if (LOG.isErrorEnabled()) { LOG.error("ACME certificate chain could not be loaded from file."); } - } else { - eventPublisher.publishEvent(new CertificateEvent(getDomainKeyPair(), false, fullCertificateChain)); } }