[GitHub] [ozone] fapifta commented on a diff in pull request #5150: HDDS-9013. Fetch root CA certificate list during SCM startup.

["fapifta (via GitHub)" <gi...@apache.org>]

fapifta commented on code in PR #5150:
URL: https://github.com/apache/ozone/pull/5150#discussion_r1287782558


##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ha/SCMHAManagerImpl.java:
##########
@@ -18,6 +18,7 @@
 package org.apache.hadoop.hdds.scm.ha;
 
 import com.google.common.base.Preconditions;
+

Review Comment:
   Nit:
   if we touch imports here, probably we should move TimeUnit import to java.* imports, otherwise I would just not add a newline here. (Please ignore this if no other changes are done on the PR).



##########
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/security/x509/certificate/client/SCMCertificateClient.java:
##########
@@ -228,4 +235,69 @@ public String signAndStoreCertificate(PKCS10CertificationRequest request,
       throw new RuntimeException(e);
     }
   }
+
+  public void refreshCACertificates() throws IOException {
+    if (executorService == null) {
+      executorService = Executors.newSingleThreadExecutor(
+          new ThreadFactoryBuilder().setNameFormat(
+                  getComponentName() + "-refreshCACertificates")
+              .setDaemon(true).build());
+    }
+    executorService.execute(new RefreshCACertificates(getScmSecureClient()));
+  }
+
+  /**
+   * Task to refresh root CA certificates for SCM.
+   */
+  public class RefreshCACertificates implements Runnable {
+    private final SCMSecurityProtocolClientSideTranslatorPB scmSecureClient;
+
+    public RefreshCACertificates(
+        SCMSecurityProtocolClientSideTranslatorPB client) {
+      this.scmSecureClient = client;
+    }
+
+    @Override
+    public void run() {
+      try {
+        // In case root CA certificate is rotated during this SCM is offline
+        // period, fetch the new root CA list from leader SCM and refresh ratis
+        // server's tlsConfig.
+        List<String> rootCAPems = scmSecureClient.getAllRootCaCertificates();
+
+        // SCM certificate client currently sets root CA as CA cert
+        Set<X509Certificate> certList = getAllRootCaCerts();
+        certList = certList.isEmpty() ? getAllCaCerts() : certList;
+
+        List<X509Certificate> rootCAsFromLeaderSCM =
+            OzoneSecurityUtil.convertToX509(rootCAPems);
+        rootCAsFromLeaderSCM.removeAll(certList);
+
+        if (rootCAsFromLeaderSCM.isEmpty()) {
+          LOG.info("CA certificates are not changed.");
+          return;
+        }
+
+        for (X509Certificate cert : rootCAsFromLeaderSCM) {
+          LOG.info("Fetched new root CA certificate {} from leader SCM",
+              cert.getSerialNumber().toString());
+          storeCertificate(
+              CertificateCodec.getPEMEncodedString(cert), CAType.SUBORDINATE);
+        }
+        String scmCertId = getCertificate().getSerialNumber().toString();
+        notifyNotificationReceivers(scmCertId, scmCertId);

Review Comment:
   We are using the same certificate serial id as old and new in the notification. Is this intentional, if so can you please elaborate I do not see why we do so.



##########
hadoop-ozone/dist/src/main/compose/testlib.sh:
##########
@@ -278,7 +278,7 @@ save_container_logs() {
   local output_name=$(get_output_name)
   local c
   for c in $(docker-compose ps "$@" | cut -f1 -d' ' | tail -n +3); do
-    docker logs "${c}" &>> "$RESULT_DIR/docker-${output_name}${c}.log"
+    docker logs "${c}" >> "$RESULT_DIR/docker-${output_name}${c}.log" 2>&1

Review Comment:
   I am not sure what is actually fixed here, but probably this is unrelated to this patch, and should be fixed separately.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org