Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Using full cert chain over single certificate #161

Merged
merged 14 commits into from
Mar 17, 2022
Merged

Using full cert chain over single certificate #161

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
33951d2
Using full cert chain over single certificate
zendern Jul 19, 2021
3bfff20
No need to store another certificate just use the chain
zendern Jul 19, 2021
1c3fc8d
No reason we cant deprecate the old constructor and use the new one
zendern Jul 19, 2021
8014c6e
Cleanup checkstyle issues
zendern Mar 7, 2022
84db7da
Better javadoc
zendern Mar 7, 2022
16b05b6
Using constants
zendern Mar 7, 2022
075cb6c
Groovy-ify and use types more
zendern Mar 7, 2022
e960455
Always return an array
zendern Mar 7, 2022
67394be
Add @NonNull
zendern Mar 8, 2022
3b9718d
stop using the old CertificateEvent constructor to fix warning
yawkat Mar 14, 2022
688ccb2
checked cast from generateCertificates
yawkat Mar 14, 2022
ff5e044
add a check that the certificates in CertificateEvent aren't empty
yawkat Mar 14, 2022
a33236c
handle empty cert array
yawkat Mar 14, 2022
6a5dd44
Change AcmeService to return Optional
sdelamo Mar 16, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 27 additions & 3 deletions acme/src/main/java/io/micronaut/acme/events/CertificateEvent.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,27 +22,42 @@
* 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;

/**
* @deprecated {@link #CertificateEvent(KeyPair, boolean, X509Certificate...)} 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.certificate = certificate;
this.domainKeyPair = domainKeyPair;
this.validationCert = validationCert;
this.fullCertificateChain = new X509Certificate[]{certificate};
}

/**
* 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;
}

/**
* @return Certificate created by ACME server
*/
public X509Certificate getCert() {
return certificate;
return fullCertificateChain[0];
}

/**
Expand All @@ -58,4 +73,13 @@ public KeyPair getDomainKeyPair() {
public boolean isValidationCert() {
return validationCert;
}

/**
* Return the full certificate chain.
*
* @return array of certificates in the chain.
*/
public X509Certificate[] getFullCertificateChain() {
return fullCertificateChain;
}
}
33 changes: 29 additions & 4 deletions acme/src/main/java/io/micronaut/acme/services/AcmeService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -67,6 +68,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.
Expand Down Expand Up @@ -123,7 +125,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()));
Expand All @@ -138,6 +140,29 @@ public X509Certificate getCurrentCertificate() {
}
}

/**
* Returns the full certificate chain.
*
* @return array of each of the certificates in the chain
*/
@NonNull
protected X509Certificate[] getFullCertificateChain() {
sdelamo marked this conversation as resolved.
Show resolved Hide resolved
try {
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]);
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Important change was to call cf.generateCertificates here and then pass this into the CertificateEvent and thus into Netty for initialization.

} else {
return new X509Certificate[]{};
}
} catch (CertificateException | IOException e) {
if (LOG.isWarnEnabled()) {
LOG.warn("Could not create certificate from file", e);
}
return new X509Certificate[]{};
}
}

/**
* Orders a new certificate using ACME protocol.
*
Expand Down Expand Up @@ -278,7 +303,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, false, getFullCertificateChain()));
Copy link
Contributor

@sdelamo sdelamo Mar 8, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if getFullCertificateChain returns an empty array, should we publish the event?. Since CertificateEvent assumes a non empty array due to CertificateEvent::getCert where it gets the first certificate in the array.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Resolved via 6a5dd44

if (LOG.isInfoEnabled()) {
LOG.info("ACME certificate order success! Certificate URL: {}", certificate.getLocation());
}
Expand Down Expand Up @@ -462,7 +487,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()));
Expand All @@ -482,7 +507,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(), false, getFullCertificateChain()));
Copy link
Contributor

@sdelamo sdelamo Mar 8, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if getFullCertificateChain returns an empty array, should we publish the event?. Since CertificateEvent assumes a non empty array due to CertificateEvent::getCert where it gets the first certificate in the array.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Resolved via 6a5dd44

}

/**
Expand Down
2 changes: 1 addition & 1 deletion acme/src/main/java/io/micronaut/acme/ssl/AcmeSSLContextBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
8 changes: 6 additions & 2 deletions acme/src/test/groovy/io/micronaut/acme/AcmeCertRefresherMultiDomainsTaskSpec.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,13 +52,17 @@ class AcmeCertRefresherMultiDomainsTaskSpec extends AcmeBaseSpec {
try {
conn.connect()
Certificate[] certs = conn.getServerCertificates()
certs.length == 1
def cert = (X509Certificate) certs[0]
certs.length == 2
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)

X509Certificate cert2 = certs[1]
cert2.issuerDN.name.contains("Pebble Root CA")
cert2.subjectDN.name.contains("Pebble Intermediate CA")
}finally{
if(conn != null){
conn.disconnect()
Expand Down
8 changes: 6 additions & 2 deletions acme/src/test/groovy/io/micronaut/acme/AcmeCertRefresherTaskSpec.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,11 +52,15 @@ class AcmeCertRefresherTaskSpec extends AcmeBaseSpec {
try {
conn.connect()
Certificate[] certs = conn.getServerCertificates()
certs.length == 1
def cert = (X509Certificate) certs[0]
certs.length == 2
X509Certificate cert = certs[0]
cert.getIssuerDN().getName().contains("Pebble Intermediate CA")
cert.getSubjectDN().getName().contains(EXPECTED_DOMAIN)
cert.getSubjectAlternativeNames().size() == 1

X509Certificate cert2 = certs[1]
cert2.issuerDN.name.contains("Pebble Root CA")
cert2.subjectDN.name.contains("Pebble Intermediate CA")
}finally{
if(conn != null){
conn.disconnect()
Expand Down
8 changes: 6 additions & 2 deletions acme/src/test/groovy/io/micronaut/acme/AcmeCertRefresherTaskWithClasspathKeysSpec.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,11 +67,15 @@ class AcmeCertRefresherTaskWithClasspathKeysSpec extends AcmeBaseSpec {
try {
conn.connect()
Certificate[] certs = conn.getServerCertificates()
certs.length == 1
def cert = (X509Certificate) certs[0]
certs.length == 2
X509Certificate cert = certs[0]
cert.getIssuerDN().getName().contains("Pebble Intermediate CA")
cert.getSubjectDN().getName().contains(EXPECTED_DOMAIN)
cert.getSubjectAlternativeNames().size() == 1

X509Certificate cert2 = certs[1]
cert2.issuerDN.name.contains("Pebble Root CA")
cert2.subjectDN.name.contains("Pebble Intermediate CA")
}finally{
if(conn != null){
conn.disconnect()
Expand Down
8 changes: 6 additions & 2 deletions acme/src/test/groovy/io/micronaut/acme/AcmeCertRefresherTaskWithFileKeysSpec.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,11 +61,15 @@ class AcmeCertRefresherTaskWithFileKeysSpec extends AcmeBaseSpec {
try {
conn.connect()
Certificate[] certs = conn.getServerCertificates()
certs.length == 1
def cert = (X509Certificate) certs[0]
certs.length == 2
X509Certificate cert = certs[0]
cert.getIssuerDN().getName().contains("Pebble Intermediate CA")
cert.getSubjectDN().getName().contains(EXPECTED_DOMAIN)
cert.getSubjectAlternativeNames().size() == 1

X509Certificate cert2 = certs[1]
cert2.issuerDN.name.contains("Pebble Root CA")
cert2.subjectDN.name.contains("Pebble Intermediate CA")
}finally{
if(conn != null){
conn.disconnect()
Expand Down
8 changes: 6 additions & 2 deletions acme/src/test/groovy/io/micronaut/acme/AcmeCertWildcardRefresherTaskSpec.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,13 +57,17 @@ class AcmeCertWildcardRefresherTaskSpec extends AcmeBaseSpec {
try {
conn.connect()
Certificate[] certs = conn.getServerCertificates()
certs.length == 1
def cert = (X509Certificate) certs[0]
certs.length == 2
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)

X509Certificate cert2 = certs[1]
cert2.issuerDN.name.contains("Pebble Root CA")
cert2.subjectDN.name.contains("Pebble Intermediate CA")
}finally{
if(conn != null){
conn.disconnect()
Expand Down
8 changes: 6 additions & 2 deletions .../test/groovy/io/micronaut/acme/challenges/AcmeCertRefresherTaskHttp01ChallengeSpec.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
def cert = (X509Certificate) certs[0]
certs.length == 2
X509Certificate cert = certs[0]
cert.getIssuerDN().getName().contains("Pebble Intermediate CA")
cert.getSubjectDN().getName().contains(EXPECTED_ACME_DOMAIN)
cert.getSubjectAlternativeNames().size() == 1

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"() {
Expand Down
8 changes: 6 additions & 2 deletions ...st/groovy/io/micronaut/acme/challenges/AcmeCertRefresherTaskTlsApln01ChallengeSpec.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
def cert = (X509Certificate) certs[0]
certs.length == 2
X509Certificate cert = certs[0]
cert.getIssuerDN().getName().contains("Pebble Intermediate CA")
cert.getSubjectDN().getName().contains(EXPECTED_ACME_DOMAIN)
cert.getSubjectAlternativeNames().size() == 1

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"() {
Expand Down
Loading