diff --git a/src/main/java/com/aws/greengrass/certificatemanager/CertificateManager.java b/src/main/java/com/aws/greengrass/certificatemanager/CertificateManager.java index 6be256753..ea7be3ce7 100644 --- a/src/main/java/com/aws/greengrass/certificatemanager/CertificateManager.java +++ b/src/main/java/com/aws/greengrass/certificatemanager/CertificateManager.java @@ -126,9 +126,13 @@ public void subscribeToServerCertificateUpdates(@NonNull String csr, @NonNull Co JcaPKCS10CertificationRequest jcaRequest = new JcaPKCS10CertificationRequest(pkcs10CertificationRequest); CertificateGenerator certificateGenerator = new ServerCertificateGenerator( jcaRequest.getSubject(), jcaRequest.getPublicKey(), cb, certificateStore); - certificateGenerator.generateCertificate(cisClient::getCachedHostAddresses); + + // Add certificate generator to monitors first in order to avoid missing events + // that happen while the initial certificate is being generated. certExpiryMonitor.addToMonitor(certificateGenerator); cisShadowMonitor.addToMonitor(certificateGenerator); + + certificateGenerator.generateCertificate(cisClient::getCachedHostAddresses); } catch (KeyStoreException e) { logger.atError().setCause(e).log("unable to subscribe to certificate update"); throw e; diff --git a/src/main/java/com/aws/greengrass/certificatemanager/certificate/CISShadowMonitor.java b/src/main/java/com/aws/greengrass/certificatemanager/certificate/CISShadowMonitor.java index 13ba09359..76131b1de 100644 --- a/src/main/java/com/aws/greengrass/certificatemanager/certificate/CISShadowMonitor.java +++ b/src/main/java/com/aws/greengrass/certificatemanager/certificate/CISShadowMonitor.java @@ -206,6 +206,9 @@ private synchronized void handleNewCloudVersion(int newVersion) { return; } + // NOTE: This method executes in an MQTT CRT thread. Since certificate generation is a compute intensive + // operation (particularly on low end devices) it is imperative that we process this event asynchronously + // to avoid blocking other MQTT subscribers in the Nucleus getConnectivityFuture = CompletableFuture.supplyAsync(() -> { try { RetryUtils.runWithRetry(GET_CONNECTIVITY_RETRY_CONFIG, cisClient::getConnectivityInfo, diff --git a/src/main/java/com/aws/greengrass/certificatemanager/certificate/ServerCertificateGenerator.java b/src/main/java/com/aws/greengrass/certificatemanager/certificate/ServerCertificateGenerator.java index aea281ae3..6e012b29f 100644 --- a/src/main/java/com/aws/greengrass/certificatemanager/certificate/ServerCertificateGenerator.java +++ b/src/main/java/com/aws/greengrass/certificatemanager/certificate/ServerCertificateGenerator.java @@ -5,6 +5,8 @@ package com.aws.greengrass.certificatemanager.certificate; +import com.aws.greengrass.logging.api.Logger; +import com.aws.greengrass.logging.impl.LogManager; import org.bouncycastle.asn1.x500.X500Name; import org.bouncycastle.operator.OperatorCreationException; @@ -22,7 +24,7 @@ import java.util.function.Supplier; public class ServerCertificateGenerator extends CertificateGenerator { - + private static final Logger logger = LogManager.getLogger(ServerCertificateGenerator.class); private final Consumer callback; /** @@ -57,6 +59,8 @@ public synchronized void generateCertificate(Supplier> connectivity List connectivityInfo = new ArrayList<>(connectivityInfoSupplier.get()); connectivityInfo.add("localhost"); + logger.atInfo().kv("subject", subject).kv("connectivityInfo", connectivityInfo) + .log("Generating new server certificate"); try { certificate = CertificateHelper.signServerCertificateRequest( certificateStore.getCACertificate(), @@ -67,6 +71,7 @@ public synchronized void generateCertificate(Supplier> connectivity Date.from(now), Date.from(now.plusSeconds(DEFAULT_CERT_EXPIRY_SECONDS))); } catch (NoSuchAlgorithmException | OperatorCreationException | CertificateException | IOException e) { + logger.atError().cause(e).log("Failed to generate new server certificate"); throw new CertificateGenerationException(e); }