fix: dropped CIS updates in CertificateManager (#44)

This change fixes a race condition in the CertificateManager
subscribeToServerCertificateUpdates method. If a CIS update occurs while
the initial server certificate is being generated, but before the
generator is added to the shadow monitor, then the CIS update event will
be missed. This will result in the server certificate not including
correct connectivity information.

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;

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,

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<X509Certificate> callback;
 
     /**
@@ -57,6 +59,8 @@ public synchronized void generateCertificate(Supplier<List<String>> connectivity
         List<String> 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<List<String>> 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);
         }