From 44b1f9436272d963b046d1592244f5e6ffd165e0 Mon Sep 17 00:00:00 2001 From: Paolo Venturi <37150740+pv3nturi@users.noreply.github.com> Date: Wed, 12 May 2021 13:51:13 +0200 Subject: [PATCH] In-memory certificates data improvement (#282) * In-memory certificates data improvement * fix unit tests * Certificates exp calc improvement Co-authored-by: Paolo Venturi --- .../api/CertificatesResource.java | 24 ++++++++------- .../configstore/CertificateData.java | 30 ++++++++++++++++++- .../DynamicCertificatesManager.java | 22 +++++++++----- .../utils/CertificatesUtils.java | 21 ++++--------- .../carapaceproxy/api/StartAPIServerTest.java | 5 ++-- .../server/certificates/CertificatesTest.java | 8 ++--- .../certificates/CertificatesUtilsTest.java | 14 +++++---- 7 files changed, 78 insertions(+), 46 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 ce72068c1..41c0a2ec9 100644 --- a/carapace-server/src/main/java/org/carapaceproxy/api/CertificatesResource.java +++ b/carapace-server/src/main/java/org/carapaceproxy/api/CertificatesResource.java @@ -175,28 +175,30 @@ public Map getAllCertificates() { private static void fillCertificateBean(CertificateBean bean, SSLCertificateConfiguration certificate, DynamicCertificatesManager dCManager, HttpProxyServer server) { try { - Certificate[] chain; DynamicCertificateState state = null; if (certificate.isDynamic()) { CertificateData cert = dCManager.getCertificateDataForDomain(certificate.getId()); if (cert == null) { return; } - chain = base64DecodeCertificateChain(cert.getChain()); - state = cert.getState(); + state = certificate.isAcme() + ? cert.getState() + : CertificatesUtils.isCertificateExpired(cert.getExpiringDate(), 0) ? DynamicCertificateState.EXPIRED : DynamicCertificateState.AVAILABLE; + bean.setExpiringDate(cert.getExpiringDate() != null ? cert.getExpiringDate().toString() : ""); + bean.setSerialNumber(cert.getSerialNumber()); } else { KeyStore keystore = loadKeyStoreFromFile(certificate.getFile(), certificate.getPassword(), server.getBasePath()); if (keystore == null) { return; } - chain = CertificatesUtils.readChainFromKeystore(keystore); - } - if (chain != null && chain.length > 0) { - X509Certificate _cert = ((X509Certificate) chain[0]); - bean.setExpiringDate(_cert.getNotAfter().toString()); - bean.setSerialNumber(_cert.getSerialNumber().toString(16).toUpperCase()); // HEX - if (!certificate.isAcme()) { - state = CertificatesUtils.isCertificateExpired(chain, 0) ? DynamicCertificateState.EXPIRED : DynamicCertificateState.AVAILABLE; + Certificate[] chain = CertificatesUtils.readChainFromKeystore(keystore); + if (chain != null && chain.length > 0) { + X509Certificate _cert = ((X509Certificate) chain[0]); + bean.setExpiringDate(_cert.getNotAfter().toString()); + bean.setSerialNumber(_cert.getSerialNumber().toString(16).toUpperCase()); // HEX + if (!certificate.isAcme()) { + state = CertificatesUtils.isCertificateExpired(_cert.getNotAfter(), 0) ? DynamicCertificateState.EXPIRED : DynamicCertificateState.AVAILABLE; + } } } if (certificate.isAcme()) { 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 379b41973..e700d8352 100644 --- a/carapace-server/src/main/java/org/carapaceproxy/configstore/CertificateData.java +++ b/carapace-server/src/main/java/org/carapaceproxy/configstore/CertificateData.java @@ -20,6 +20,7 @@ package org.carapaceproxy.configstore; import java.net.URL; +import java.util.Date; import java.util.Objects; import org.carapaceproxy.server.certificates.DynamicCertificateState; import org.shredzone.acme4j.toolbox.JSON; @@ -35,7 +36,7 @@ public class CertificateData { private String domain; private String privateKey; // base64 encoded string. private String chain; // base64 encoded string of the KeyStore. - private DynamicCertificateState state; + private volatile DynamicCertificateState state; private String pendingOrderLocation; private String pendingChallengeData; @@ -43,6 +44,9 @@ public class CertificateData { private boolean wildcard; private boolean manual; private int daysBeforeRenewal; + private Date expiringDate; + private String serialNumber; // hex + private byte[] keystoreData; // decoded chain public CertificateData(String domain, String privateKey, String chain, DynamicCertificateState state, String orderLocation, String challengeData) { @@ -134,6 +138,30 @@ public void setDaysBeforeRenewal(int daysBeforeRenewal) { this.daysBeforeRenewal = daysBeforeRenewal; } + public Date getExpiringDate() { + return expiringDate; + } + + public void setExpiringDate(Date expiringDate) { + this.expiringDate = expiringDate; + } + + public String getSerialNumber() { + return serialNumber; + } + + public void setSerialNumber(String serialNumber) { + this.serialNumber = serialNumber; + } + + public byte[] getKeystoreData() { + return keystoreData; + } + + public void setKeystoreData(byte[] keystoreData) { + this.keystoreData = keystoreData; + } + @Override public int hashCode() { int hash = 7; 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 1f55753db..613d1894e 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 @@ -19,7 +19,6 @@ */ package org.carapaceproxy.server.certificates; -import static org.carapaceproxy.configstore.ConfigurationStoreUtils.base64DecodeCertificateChain; import static org.carapaceproxy.configstore.ConfigurationStoreUtils.base64EncodeCertificateChain; import static org.carapaceproxy.server.certificates.DynamicCertificateState.AVAILABLE; import static org.carapaceproxy.server.certificates.DynamicCertificateState.DNS_CHALLENGE_WAIT; @@ -50,6 +49,7 @@ import org.carapaceproxy.server.config.SSLCertificateConfiguration; import static org.carapaceproxy.server.config.SSLCertificateConfiguration.CertificateMode.MANUAL; import static org.carapaceproxy.utils.CertificatesUtils.isCertificateExpired; +import static org.carapaceproxy.utils.CertificatesUtils.readChainFromKeystore; import java.io.IOException; import java.net.InetAddress; import java.net.URL; @@ -214,6 +214,15 @@ private CertificateData loadOrCreateDynamicCertificateForDomain(String domain, CertificateData cert = store.loadCertificateForDomain(domain); if (cert == null) { cert = new CertificateData(domain, "", "", WAITING, "", ""); + } else if (cert.getChain() != null && !cert.getChain().isEmpty()) { + byte[] keystoreData = Base64.getDecoder().decode(cert.getChain()); + cert.setKeystoreData(keystoreData); + Certificate[] chain = readChainFromKeystore(keystoreData); + if (chain != null && chain.length > 0) { + X509Certificate _cert = ((X509Certificate) chain[0]); + cert.setExpiringDate(_cert.getNotAfter()); + cert.setSerialNumber(_cert.getSerialNumber().toString(16).toUpperCase()); // HEX + } } cert.setWildcard(wildcard); cert.setManual(forceManual); @@ -273,11 +282,10 @@ private void certificatesLifecycle() { .sorted((e1, e2) -> e1.getKey().compareTo(e2.getKey())) .map(e -> e.getValue()) .collect(Collectors.toList()); - for (CertificateData data : _certificates) { + for (CertificateData cert : _certificates) { boolean updateCertificate = true; - final String domain = data.getDomain(); + final String domain = cert.getDomain(); try { - CertificateData cert = loadOrCreateDynamicCertificateForDomain(domain, data.isWildcard(), false, data.getDaysBeforeRenewal()); switch (cert.getState()) { case WAITING: // certificate waiting to be issues/renew case DOMAIN_UNREACHABLE: { // certificate domain reported as unreachable for issuing/renewing @@ -340,7 +348,7 @@ private void certificatesLifecycle() { break; } case AVAILABLE: { // certificate saved/available/not expired - if (isCertificateExpired(base64DecodeCertificateChain(cert.getChain()), cert.getDaysBeforeRenewal())) { + if (isCertificateExpired(cert.getExpiringDate(), cert.getDaysBeforeRenewal())) { cert.setState(EXPIRED); } else { updateCertificate = false; @@ -522,11 +530,11 @@ public void setStateOfCertificate(String id, DynamicCertificateState state) { */ public byte[] getCertificateForDomain(String domain) throws GeneralSecurityException { CertificateData cert = certificates.get(domain); // certs always retrived from cache - if (cert == null || cert.getChain() == null || cert.getChain().isEmpty()) { + if (cert == null || cert.getKeystoreData() == null || cert.getKeystoreData().length == 0) { LOG.log(Level.SEVERE, "No dynamic certificate available for domain {0}", domain); return null; } - return Base64.getDecoder().decode(cert.getChain()); + return cert.getKeystoreData(); } public CertificateData getCertificateDataForDomain(String domain) throws GeneralSecurityException { diff --git a/carapace-server/src/main/java/org/carapaceproxy/utils/CertificatesUtils.java b/carapace-server/src/main/java/org/carapaceproxy/utils/CertificatesUtils.java index e8cf47aef..b45dd0d58 100644 --- a/carapace-server/src/main/java/org/carapaceproxy/utils/CertificatesUtils.java +++ b/carapace-server/src/main/java/org/carapaceproxy/utils/CertificatesUtils.java @@ -32,11 +32,7 @@ import java.security.cert.Certificate; import java.security.cert.CertificateEncodingException; import java.security.cert.CertificateException; -import java.security.cert.CertificateExpiredException; -import java.security.cert.CertificateNotYetValidException; -import java.security.cert.X509Certificate; import java.util.Arrays; -import java.util.Calendar; import java.util.Date; import java.util.Iterator; @@ -51,6 +47,8 @@ public final class CertificatesUtils { private static final String KEYSTORE_CERT_ALIAS = "cert-chain"; public static final char[] KEYSTORE_PW = new char[0]; + private static final long DAY_TO_MILLIS = 24 * 60 * 60 * 1_000; + /** * * @param chain to store into a keystore @@ -146,19 +144,12 @@ public static KeyStore loadKeyStoreData(byte[] data, String password) return ks; } - public static boolean isCertificateExpired(Certificate[] chain, int daysBeforeRenewal) throws GeneralSecurityException { - if (chain == null || chain.length == 0) { + public static boolean isCertificateExpired(Date expiringDate, int daysBeforeRenewal) throws GeneralSecurityException { + if (expiringDate == null) { return false; } - try { - Calendar cal = Calendar.getInstance(); - cal.setTime(new Date()); - cal.add(Calendar.DATE, daysBeforeRenewal); - ((X509Certificate) chain[0]).checkValidity(cal.getTime()); - } catch (CertificateNotYetValidException | CertificateExpiredException ex) { - return true; - } - return false; + + return System.currentTimeMillis() + (daysBeforeRenewal * DAY_TO_MILLIS) >= expiringDate.getTime(); } /** diff --git a/carapace-server/src/test/java/org/carapaceproxy/api/StartAPIServerTest.java b/carapace-server/src/test/java/org/carapaceproxy/api/StartAPIServerTest.java index f8e53e94a..8058bb951 100644 --- a/carapace-server/src/test/java/org/carapaceproxy/api/StartAPIServerTest.java +++ b/carapace-server/src/test/java/org/carapaceproxy/api/StartAPIServerTest.java @@ -405,11 +405,12 @@ public void testCertificates() throws Exception { // Downloading CertificateData cert = store.loadCertificateForDomain(dynDomain); - cert.setChain(Base64.getEncoder().encodeToString("CHAIN".getBytes())); + byte[] newKeystore = createKeystore(generateSampleChain(endUserKeyPair, false), KeyPairUtils.createKeyPair(DEFAULT_KEYPAIRS_SIZE).getPrivate()); + cert.setChain(Base64.getEncoder().encodeToString(newKeystore)); store.saveCertificate(cert); man.setStateOfCertificate(dynDomain, DynamicCertificateState.AVAILABLE); response = client.get("/api/certificates/" + dynDomain + "/download", credentials); - assertEquals("CHAIN", response.getBodyString()); + assertTrue(Arrays.equals(newKeystore, response.getBody())); } // Manual certificate 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 612d5bbb4..130e7af83 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 @@ -494,12 +494,12 @@ public void testCertificatesRenew() throws Exception { ConfigurationStore store = dcMan.getConfigurationStore(); store.saveKeyPairForDomain(keyPair, "localhost", false); - CertificateData cert = store.loadCertificateForDomain("localhost"); + CertificateData cert = dcMan.getCertificateDataForDomain("localhost"); cert.setState(DynamicCertificateState.ORDERING); cert.setPendingOrderLocation("https://localhost/orderlocation"); - store.saveCertificate(cert); - assertEquals(DynamicCertificateState.ORDERING, dcMan.getStateOfCertificate("localhost")); - assertNotNull(dcMan.getCertificateForDomain("localhost")); + cert = dcMan.getCertificateDataForDomain("localhost"); + assertNotNull(cert); + assertEquals(DynamicCertificateState.ORDERING, cert.getState()); // ACME mocking ACMEClient ac = mock(ACMEClient.class); diff --git a/carapace-server/src/test/java/org/carapaceproxy/server/certificates/CertificatesUtilsTest.java b/carapace-server/src/test/java/org/carapaceproxy/server/certificates/CertificatesUtilsTest.java index df43b5f96..195baf99f 100644 --- a/carapace-server/src/test/java/org/carapaceproxy/server/certificates/CertificatesUtilsTest.java +++ b/carapace-server/src/test/java/org/carapaceproxy/server/certificates/CertificatesUtilsTest.java @@ -33,6 +33,8 @@ import org.junit.Test; import org.shredzone.acme4j.util.KeyPairUtils; import static org.carapaceproxy.utils.CertificatesUtils.compareChains; +import java.security.cert.X509Certificate; +import java.util.Date; import org.carapaceproxy.utils.CertificatesUtils; /** @@ -70,16 +72,16 @@ public void testCertificatesExpiration() throws Exception { { KeyPair endUserKeyPair = KeyPairUtils.createKeyPair(DEFAULT_KEYPAIRS_SIZE); Certificate[] chain = generateSampleChain(endUserKeyPair, false); // not before == not after == today - assertFalse(CertificatesUtils.isCertificateExpired(chain, 0)); - assertTrue(CertificatesUtils.isCertificateExpired(chain, -30)); // not before - assertTrue(CertificatesUtils.isCertificateExpired(chain, 30)); // not after + Date expiringDate = ((X509Certificate) chain[0]).getNotAfter(); + assertFalse(CertificatesUtils.isCertificateExpired(expiringDate, 0)); + assertTrue(CertificatesUtils.isCertificateExpired(expiringDate, 30)); // not after } { KeyPair endUserKeyPair = KeyPairUtils.createKeyPair(DEFAULT_KEYPAIRS_SIZE); Certificate[] chain = generateSampleChain(endUserKeyPair, true); // not before == not after == today - assertTrue(CertificatesUtils.isCertificateExpired(chain, 0)); - assertTrue(CertificatesUtils.isCertificateExpired(chain, -30)); // not before - assertTrue(CertificatesUtils.isCertificateExpired(chain, 30)); // not after + Date expiringDate = ((X509Certificate) chain[0]).getNotAfter(); + assertTrue(CertificatesUtils.isCertificateExpired(expiringDate, 0)); + assertTrue(CertificatesUtils.isCertificateExpired(expiringDate, 30)); // not after } } }