[GitHub] [ozone] Galsza commented on a diff in pull request #4808: HDDS-8588. Add initialization logic in CertificateClient to handle more than one rootCA certificate

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

Galsza commented on code in PR #4808:
URL: https://github.com/apache/ozone/pull/4808#discussion_r1221253444


##########
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/security/x509/certificate/client/DefaultCertificateClient.java:
##########
@@ -340,20 +349,52 @@ public synchronized List<X509Certificate> getTrustChain() {
       }
     } else {
       // case before certificate bundle is supported
-      chain.add(getCertificate());
-      X509Certificate cert = getCACertificate();
-      if (cert != null) {
-        chain.add(getCACertificate());
-      }
-      cert = getRootCACertificate();
-      if (cert != null) {
-        chain.add(cert);
+      X509Certificate lastInsertedCert = getCertificate();
+      chain.add(lastInsertedCert);
+      List<X509Certificate> caCertList = getCaCertList();
+      while (!isRootCa(lastInsertedCert)) {
+        Optional<X509Certificate> issuerOpt =
+            getIssuerForCert(lastInsertedCert, caCertList);
+        if (issuerOpt.isPresent()) {
+          X509Certificate issuer = issuerOpt.get();
+          chain.add(issuer);
+          lastInsertedCert = issuer;
+        } else {
+          throw new CertificateException("No issuer found for certificate: " +
+              lastInsertedCert);
+        }
       }
+      //add self-signed certificate to the chain
+      chain.add(lastInsertedCert);

Review Comment:
   The lastInsertedCert is changed during the iteration. The method here does the following: takes a certificate lastInsertedCert, checks if there is an issuer for that certificate, in that case inserts the cert to the list and continues the same thing with the issuer. We end the iteration when there is no issuer left for the lastInsertedCert. This means that we have reached a certificate whose signer is not in our list. But we still haven't added this certificate to our list, which is inserted at the end.



-- 
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