From e81d5298b57ae295d98dca93be5ac9c90f9d80a8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Szabolcs=20G=C3=A1l?= Date: Sat, 13 Jul 2024 15:09:58 +0200 Subject: [PATCH 01/10] Inline buildCaList method --- .../org/apache/hadoop/hdds/utils/HAUtils.java | 113 ++++++++---------- 1 file changed, 50 insertions(+), 63 deletions(-) diff --git a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/HAUtils.java b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/HAUtils.java index 342a0400cbd6..a317443cb046 100644 --- a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/HAUtils.java +++ b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/HAUtils.java @@ -373,67 +373,6 @@ public static List getExistingSstFiles(File db) throws IOException { return sstList; } - /** - * Build CA list which need to be passed to client. - * - * If certificate client is null, obtain the list of CA using SCM security - * client, else it uses certificate client. - * @return list of CA - */ - public static List buildCAList(CertificateClient certClient, - ConfigurationSource configuration) throws IOException { - long waitDuration = - configuration.getTimeDuration(OZONE_SCM_CA_LIST_RETRY_INTERVAL, - OZONE_SCM_CA_LIST_RETRY_INTERVAL_DEFAULT, TimeUnit.SECONDS); - if (certClient != null) { - if (!SCMHAUtils.isSCMHAEnabled(configuration)) { - return generateCAList(certClient); - } else { - Collection scmNodes = SCMHAUtils.getSCMNodeIds(configuration); - int expectedCount = scmNodes.size() + 1; - if (scmNodes.size() > 1) { - // First check if cert client has ca list initialized. - // This is being done, when this method is called multiple times we - // don't make call to SCM, we return from in-memory. - List caCertPemList = certClient.getCAList(); - if (caCertPemList != null && caCertPemList.size() == expectedCount) { - return caCertPemList; - } - return getCAListWithRetry(() -> - waitForCACerts(certClient::updateCAList, expectedCount), - waitDuration); - } else { - return generateCAList(certClient); - } - } - } else { - SCMSecurityProtocolClientSideTranslatorPB scmSecurityProtocolClient = - HddsServerUtil.getScmSecurityClient(configuration); - if (!SCMHAUtils.isSCMHAEnabled(configuration)) { - List caCertPemList = new ArrayList<>(); - SCMGetCertResponseProto scmGetCertResponseProto = - scmSecurityProtocolClient.getCACert(); - if (scmGetCertResponseProto.hasX509Certificate()) { - caCertPemList.add(scmGetCertResponseProto.getX509Certificate()); - } - if (scmGetCertResponseProto.hasX509RootCACertificate()) { - caCertPemList.add(scmGetCertResponseProto.getX509RootCACertificate()); - } - return caCertPemList; - } else { - Collection scmNodes = SCMHAUtils.getSCMNodeIds(configuration); - int expectedCount = scmNodes.size() + 1; - if (scmNodes.size() > 1) { - return getCAListWithRetry(() -> waitForCACerts( - scmSecurityProtocolClient::listCACertificate, - expectedCount), waitDuration); - } else { - return scmSecurityProtocolClient.listCACertificate(); - } - } - } - } - private static List generateCAList(CertificateClient certClient) throws IOException { List caCertPemList = new ArrayList<>(); @@ -503,8 +442,56 @@ public static List buildCAX509List( return x509Certificates; } } - List pemEncodedCerts = HAUtils.buildCAList(certClient, conf); - return OzoneSecurityUtil.convertToX509(pemEncodedCerts); + long waitDuration = + conf.getTimeDuration(OZONE_SCM_CA_LIST_RETRY_INTERVAL, + OZONE_SCM_CA_LIST_RETRY_INTERVAL_DEFAULT, TimeUnit.SECONDS); + if (certClient != null) { + if (!SCMHAUtils.isSCMHAEnabled(conf)) { + return OzoneSecurityUtil.convertToX509(generateCAList(certClient)); + } else { + Collection scmNodes = SCMHAUtils.getSCMNodeIds(conf); + int expectedCount = scmNodes.size() + 1; + if (scmNodes.size() > 1) { + // First check if cert client has ca list initialized. + // This is being done, when this method is called multiple times we + // don't make call to SCM, we return from in-memory. + List caCertPemList = certClient.getCAList(); + if (caCertPemList != null && caCertPemList.size() == expectedCount) { + return OzoneSecurityUtil.convertToX509(caCertPemList); + } + return OzoneSecurityUtil.convertToX509(getCAListWithRetry(() -> + waitForCACerts(certClient::updateCAList, expectedCount), + waitDuration)); + } else { + return OzoneSecurityUtil.convertToX509(generateCAList(certClient)); + } + } + } else { + SCMSecurityProtocolClientSideTranslatorPB scmSecurityProtocolClient = + HddsServerUtil.getScmSecurityClient(conf); + if (!SCMHAUtils.isSCMHAEnabled(conf)) { + List caCertPemList = new ArrayList<>(); + SCMGetCertResponseProto scmGetCertResponseProto = + scmSecurityProtocolClient.getCACert(); + if (scmGetCertResponseProto.hasX509Certificate()) { + caCertPemList.add(scmGetCertResponseProto.getX509Certificate()); + } + if (scmGetCertResponseProto.hasX509RootCACertificate()) { + caCertPemList.add(scmGetCertResponseProto.getX509RootCACertificate()); + } + return OzoneSecurityUtil.convertToX509(caCertPemList); + } else { + Collection scmNodes = SCMHAUtils.getSCMNodeIds(conf); + int expectedCount = scmNodes.size() + 1; + if (scmNodes.size() > 1) { + return OzoneSecurityUtil.convertToX509(getCAListWithRetry(() -> waitForCACerts( + scmSecurityProtocolClient::listCACertificate, + expectedCount), waitDuration)); + } else { + return OzoneSecurityUtil.convertToX509(scmSecurityProtocolClient.listCACertificate()); + } + } + } } } From 3642769327faba12b821405e5afd1637b8631316 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Szabolcs=20G=C3=A1l?= Date: Sat, 13 Jul 2024 15:11:02 +0200 Subject: [PATCH 02/10] Delete code repeating the same functionality --- .../java/org/apache/hadoop/hdds/utils/HAUtils.java | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/HAUtils.java b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/HAUtils.java index a317443cb046..9646a6ffdd2b 100644 --- a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/HAUtils.java +++ b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/HAUtils.java @@ -432,16 +432,6 @@ private static List waitForCACerts( public static List buildCAX509List( CertificateClient certClient, ConfigurationSource conf) throws IOException { - if (certClient != null) { - // Do this here to avoid extra conversion of X509 to pem and again to - // X509 by buildCAList. - if (!SCMHAUtils.isSCMHAEnabled(conf)) { - List x509Certificates = new ArrayList<>(); - x509Certificates.addAll(certClient.getAllCaCerts()); - x509Certificates.addAll(certClient.getAllRootCaCerts()); - return x509Certificates; - } - } long waitDuration = conf.getTimeDuration(OZONE_SCM_CA_LIST_RETRY_INTERVAL, OZONE_SCM_CA_LIST_RETRY_INTERVAL_DEFAULT, TimeUnit.SECONDS); From 24d14f7c153e6afe42ce9c79be39118adf0d090a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Szabolcs=20G=C3=A1l?= Date: Sat, 13 Jul 2024 15:13:17 +0200 Subject: [PATCH 03/10] Simplify generateCaList --- .../org/apache/hadoop/hdds/utils/HAUtils.java | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/HAUtils.java b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/HAUtils.java index 9646a6ffdd2b..43327078f653 100644 --- a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/HAUtils.java +++ b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/HAUtils.java @@ -36,7 +36,6 @@ import org.apache.hadoop.hdds.scm.proxy.SCMContainerLocationFailoverProxyProvider; import org.apache.hadoop.hdds.security.exception.SCMSecurityException; import org.apache.hadoop.hdds.security.x509.certificate.client.CertificateClient; -import org.apache.hadoop.hdds.security.x509.certificate.utils.CertificateCodec; import org.apache.hadoop.hdds.tracing.TracingUtil; import org.apache.hadoop.hdds.utils.db.DBDefinition; import org.apache.hadoop.hdds.utils.db.DBColumnFamilyDefinition; @@ -373,15 +372,9 @@ public static List getExistingSstFiles(File db) throws IOException { return sstList; } - private static List generateCAList(CertificateClient certClient) - throws IOException { - List caCertPemList = new ArrayList<>(); - for (X509Certificate cert : certClient.getAllRootCaCerts()) { - caCertPemList.add(CertificateCodec.getPEMEncodedString(cert)); - } - for (X509Certificate cert : certClient.getAllCaCerts()) { - caCertPemList.add(CertificateCodec.getPEMEncodedString(cert)); - } + private static List generateCAList(CertificateClient certClient) { + List caCertPemList = new ArrayList<>(certClient.getAllRootCaCerts()); + caCertPemList.addAll(certClient.getAllCaCerts()); return caCertPemList; } @@ -437,7 +430,7 @@ public static List buildCAX509List( OZONE_SCM_CA_LIST_RETRY_INTERVAL_DEFAULT, TimeUnit.SECONDS); if (certClient != null) { if (!SCMHAUtils.isSCMHAEnabled(conf)) { - return OzoneSecurityUtil.convertToX509(generateCAList(certClient)); + return generateCAList(certClient); } else { Collection scmNodes = SCMHAUtils.getSCMNodeIds(conf); int expectedCount = scmNodes.size() + 1; @@ -453,7 +446,7 @@ public static List buildCAX509List( waitForCACerts(certClient::updateCAList, expectedCount), waitDuration)); } else { - return OzoneSecurityUtil.convertToX509(generateCAList(certClient)); + return generateCAList(certClient); } } } else { From 8e30ce1389b2c5a402957819cbbf7af0792e104d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Szabolcs=20G=C3=A1l?= Date: Sat, 13 Jul 2024 22:38:06 +0200 Subject: [PATCH 04/10] Rearrange logic for simpler buildCAList --- .../org/apache/hadoop/hdds/utils/HAUtils.java | 72 +++++++++---------- 1 file changed, 33 insertions(+), 39 deletions(-) diff --git a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/HAUtils.java b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/HAUtils.java index 43327078f653..35c64824437d 100644 --- a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/HAUtils.java +++ b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/HAUtils.java @@ -428,51 +428,45 @@ public static List buildCAX509List( long waitDuration = conf.getTimeDuration(OZONE_SCM_CA_LIST_RETRY_INTERVAL, OZONE_SCM_CA_LIST_RETRY_INTERVAL_DEFAULT, TimeUnit.SECONDS); + Collection scmNodes = SCMHAUtils.getSCMNodeIds(conf); if (certClient != null) { - if (!SCMHAUtils.isSCMHAEnabled(conf)) { - return generateCAList(certClient); - } else { - Collection scmNodes = SCMHAUtils.getSCMNodeIds(conf); + //There can't be more than 1 SCM without HA enabled + if (scmNodes.size() > 1) { + // First check if cert client has ca list initialized. + // This is being done, when this method is called multiple times we + // don't make call to SCM, we return from in-memory. + List caCertPemList = certClient.getCAList(); int expectedCount = scmNodes.size() + 1; - if (scmNodes.size() > 1) { - // First check if cert client has ca list initialized. - // This is being done, when this method is called multiple times we - // don't make call to SCM, we return from in-memory. - List caCertPemList = certClient.getCAList(); - if (caCertPemList != null && caCertPemList.size() == expectedCount) { - return OzoneSecurityUtil.convertToX509(caCertPemList); - } - return OzoneSecurityUtil.convertToX509(getCAListWithRetry(() -> - waitForCACerts(certClient::updateCAList, expectedCount), - waitDuration)); - } else { - return generateCAList(certClient); + if (caCertPemList != null && caCertPemList.size() == expectedCount) { + return OzoneSecurityUtil.convertToX509(caCertPemList); } + return OzoneSecurityUtil.convertToX509(getCAListWithRetry(() -> + waitForCACerts(certClient::updateCAList, expectedCount), + waitDuration)); + } + return generateCAList(certClient); + } + SCMSecurityProtocolClientSideTranslatorPB scmSecurityProtocolClient = + HddsServerUtil.getScmSecurityClient(conf); + if (!SCMHAUtils.isSCMHAEnabled(conf)) { + List caCertPemList = new ArrayList<>(); + SCMGetCertResponseProto scmGetCertResponseProto = + scmSecurityProtocolClient.getCACert(); + if (scmGetCertResponseProto.hasX509Certificate()) { + caCertPemList.add(scmGetCertResponseProto.getX509Certificate()); + } + if (scmGetCertResponseProto.hasX509RootCACertificate()) { + caCertPemList.add(scmGetCertResponseProto.getX509RootCACertificate()); } + return OzoneSecurityUtil.convertToX509(caCertPemList); } else { - SCMSecurityProtocolClientSideTranslatorPB scmSecurityProtocolClient = - HddsServerUtil.getScmSecurityClient(conf); - if (!SCMHAUtils.isSCMHAEnabled(conf)) { - List caCertPemList = new ArrayList<>(); - SCMGetCertResponseProto scmGetCertResponseProto = - scmSecurityProtocolClient.getCACert(); - if (scmGetCertResponseProto.hasX509Certificate()) { - caCertPemList.add(scmGetCertResponseProto.getX509Certificate()); - } - if (scmGetCertResponseProto.hasX509RootCACertificate()) { - caCertPemList.add(scmGetCertResponseProto.getX509RootCACertificate()); - } - return OzoneSecurityUtil.convertToX509(caCertPemList); + int expectedCount = scmNodes.size() + 1; + if (scmNodes.size() > 1) { + return OzoneSecurityUtil.convertToX509(getCAListWithRetry(() -> waitForCACerts( + scmSecurityProtocolClient::listCACertificate, + expectedCount), waitDuration)); } else { - Collection scmNodes = SCMHAUtils.getSCMNodeIds(conf); - int expectedCount = scmNodes.size() + 1; - if (scmNodes.size() > 1) { - return OzoneSecurityUtil.convertToX509(getCAListWithRetry(() -> waitForCACerts( - scmSecurityProtocolClient::listCACertificate, - expectedCount), waitDuration)); - } else { - return OzoneSecurityUtil.convertToX509(scmSecurityProtocolClient.listCACertificate()); - } + return OzoneSecurityUtil.convertToX509(scmSecurityProtocolClient.listCACertificate()); } } } From 6afae7e6d5762e7ad2420cec0366290922b08ab8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Szabolcs=20G=C3=A1l?= Date: Wed, 17 Jul 2024 17:39:08 +0200 Subject: [PATCH 05/10] Change who calls BuildCaList, Remove unused methods from certclient --- .../certificate/client/CertificateClient.java | 17 --------- .../ECContainerOperationClient.java | 16 +++++--- .../client/DefaultCertificateClient.java | 37 ------------------- .../org/apache/hadoop/hdds/utils/HAUtils.java | 22 +---------- .../client/CertificateClientTestImpl.java | 10 ----- .../scm/server/StorageContainerManager.java | 4 +- .../scm/cli/ContainerOperationClient.java | 2 +- .../ozone/freon/DNRPCLoadGenerator.java | 10 ++++- 8 files changed, 23 insertions(+), 95 deletions(-) diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/security/x509/certificate/client/CertificateClient.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/security/x509/certificate/client/CertificateClient.java index 0c23a846563a..13deab100c0e 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/security/x509/certificate/client/CertificateClient.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/security/x509/certificate/client/CertificateClient.java @@ -127,23 +127,6 @@ X509Certificate getCertificate(String certSerialId) */ Set getAllCaCerts(); - /** - * Return the pem encoded CA certificate list. - *

- * If initialized return list of pem encoded CA certificates, else return - * null. - * - * @return list of pem encoded CA certificates. - */ - List getCAList(); - - /** - * Update and returns the pem encoded CA certificate list. - * @return list of pem encoded CA certificates. - * @throws IOException - */ - List updateCAList() throws IOException; - /** * Verifies a digital Signature, given the signature and the certificate of * the signer. diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/ec/reconstruction/ECContainerOperationClient.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/ec/reconstruction/ECContainerOperationClient.java index 9dedd65565f5..8d6e981c8058 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/ec/reconstruction/ECContainerOperationClient.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/ec/reconstruction/ECContainerOperationClient.java @@ -31,7 +31,6 @@ import org.apache.hadoop.hdds.scm.pipeline.PipelineID; import org.apache.hadoop.hdds.scm.storage.ContainerProtocolCalls; import org.apache.hadoop.hdds.security.x509.certificate.client.CertificateClient; -import org.apache.hadoop.hdds.utils.HAUtils; import org.apache.hadoop.ozone.OzoneSecurityUtil; import org.apache.hadoop.ozone.container.common.helpers.BlockData; import org.apache.hadoop.security.token.Token; @@ -43,6 +42,8 @@ import java.io.Closeable; import java.io.IOException; +import java.security.cert.X509Certificate; +import java.util.ArrayList; import java.util.List; import java.util.Objects; import java.util.Set; @@ -73,12 +74,15 @@ private static XceiverClientManager createClientManager( ConfigurationSource conf, CertificateClient certificateClient) throws IOException { ClientTrustManager trustManager = null; + if (OzoneSecurityUtil.isSecurityEnabled(conf)) { - CACertificateProvider localCaCerts = - () -> HAUtils.buildCAX509List(certificateClient, conf); - CACertificateProvider remoteCacerts = - () -> HAUtils.buildCAX509List(null, conf); - trustManager = new ClientTrustManager(remoteCacerts, localCaCerts); + CACertificateProvider localCaCerts = () -> { + List caCerts = new ArrayList<>(); + caCerts.addAll(certificateClient.getAllCaCerts()); + caCerts.addAll(certificateClient.getAllRootCaCerts()); + return caCerts; + }; + trustManager = new ClientTrustManager(localCaCerts, localCaCerts); } return new XceiverClientManager(conf, new XceiverClientManager.XceiverClientManagerConfigBuilder() diff --git a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/security/x509/certificate/client/DefaultCertificateClient.java b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/security/x509/certificate/client/DefaultCertificateClient.java index 2fb258e1a29e..344e6158b5e1 100644 --- a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/security/x509/certificate/client/DefaultCertificateClient.java +++ b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/security/x509/certificate/client/DefaultCertificateClient.java @@ -985,43 +985,6 @@ public Set getAllCaCerts() { return certs; } - @Override - public List getCAList() { - pemEncodedCACertsLock.lock(); - try { - return pemEncodedCACerts; - } finally { - pemEncodedCACertsLock.unlock(); - } - } - - public List listCA() throws IOException { - pemEncodedCACertsLock.lock(); - try { - if (pemEncodedCACerts == null) { - updateCAList(); - } - return pemEncodedCACerts; - } finally { - pemEncodedCACertsLock.unlock(); - } - } - - @Override - public List updateCAList() throws IOException { - pemEncodedCACertsLock.lock(); - try { - pemEncodedCACerts = getScmSecureClient().listCACertificate(); - return pemEncodedCACerts; - } catch (Exception e) { - getLogger().error("Error during updating CA list", e); - throw new CertificateException("Error during updating CA list", e, - CERTIFICATE_ERROR); - } finally { - pemEncodedCACertsLock.unlock(); - } - } - @Override public ReloadingX509TrustManager getTrustManager() throws CertificateException { try { diff --git a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/HAUtils.java b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/HAUtils.java index 35c64824437d..ed3b0f85ce30 100644 --- a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/HAUtils.java +++ b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/HAUtils.java @@ -420,32 +420,14 @@ private static List waitForCACerts( * Build CA List in the format of X509Certificate. * If certificate client is null, obtain the list of CA using SCM * security client, else it uses certificate client. + * * @return list of CA X509Certificates. */ - public static List buildCAX509List( - CertificateClient certClient, - ConfigurationSource conf) throws IOException { + public static List buildCAX509List(ConfigurationSource conf) throws IOException { long waitDuration = conf.getTimeDuration(OZONE_SCM_CA_LIST_RETRY_INTERVAL, OZONE_SCM_CA_LIST_RETRY_INTERVAL_DEFAULT, TimeUnit.SECONDS); Collection scmNodes = SCMHAUtils.getSCMNodeIds(conf); - if (certClient != null) { - //There can't be more than 1 SCM without HA enabled - if (scmNodes.size() > 1) { - // First check if cert client has ca list initialized. - // This is being done, when this method is called multiple times we - // don't make call to SCM, we return from in-memory. - List caCertPemList = certClient.getCAList(); - int expectedCount = scmNodes.size() + 1; - if (caCertPemList != null && caCertPemList.size() == expectedCount) { - return OzoneSecurityUtil.convertToX509(caCertPemList); - } - return OzoneSecurityUtil.convertToX509(getCAListWithRetry(() -> - waitForCACerts(certClient::updateCAList, expectedCount), - waitDuration)); - } - return generateCAList(certClient); - } SCMSecurityProtocolClientSideTranslatorPB scmSecurityProtocolClient = HddsServerUtil.getScmSecurityClient(conf); if (!SCMHAUtils.isSCMHAEnabled(conf)) { diff --git a/hadoop-hdds/framework/src/test/java/org/apache/hadoop/hdds/security/x509/certificate/client/CertificateClientTestImpl.java b/hadoop-hdds/framework/src/test/java/org/apache/hadoop/hdds/security/x509/certificate/client/CertificateClientTestImpl.java index 00058500f597..7df50e34c17f 100644 --- a/hadoop-hdds/framework/src/test/java/org/apache/hadoop/hdds/security/x509/certificate/client/CertificateClientTestImpl.java +++ b/hadoop-hdds/framework/src/test/java/org/apache/hadoop/hdds/security/x509/certificate/client/CertificateClientTestImpl.java @@ -255,16 +255,6 @@ public Set getAllCaCerts() { return rootCerts; } - @Override - public List getCAList() { - return null; - } - - @Override - public List updateCAList() throws IOException { - return null; - } - public void renewRootCA() throws Exception { LocalDateTime start = LocalDateTime.now(); Duration rootCACertDuration = securityConfig.getMaxCertificateDuration(); diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/StorageContainerManager.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/StorageContainerManager.java index 5c0248f162d1..174ef6f823c9 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/StorageContainerManager.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/StorageContainerManager.java @@ -1610,9 +1610,9 @@ private void persistSCMCertificates() throws IOException { // TODO: see if we can avoid doing this during every restart. if (primaryScmNodeId != null && !primaryScmNodeId.equals( scmStorageConfig.getScmId())) { + getScmSecurityClientWithMaxRetry(configuration, getCurrentUser()); List pemEncodedCerts = - scmCertificateClient.listCA(); - + getScmSecurityClientWithMaxRetry(configuration, getCurrentUser()).listCACertificate(); // Write the primary SCM CA and Root CA during startup. for (String cert : pemEncodedCerts) { X509Certificate x509Certificate = CertificateCodec.getX509Certificate( diff --git a/hadoop-hdds/tools/src/main/java/org/apache/hadoop/hdds/scm/cli/ContainerOperationClient.java b/hadoop-hdds/tools/src/main/java/org/apache/hadoop/hdds/scm/cli/ContainerOperationClient.java index ba556bf24e98..76334d124ea5 100644 --- a/hadoop-hdds/tools/src/main/java/org/apache/hadoop/hdds/scm/cli/ContainerOperationClient.java +++ b/hadoop-hdds/tools/src/main/java/org/apache/hadoop/hdds/scm/cli/ContainerOperationClient.java @@ -116,7 +116,7 @@ private XceiverClientManager newXCeiverClientManager(ConfigurationSource conf) throws IOException { XceiverClientManager manager; if (OzoneSecurityUtil.isSecurityEnabled(conf)) { - CACertificateProvider caCerts = () -> HAUtils.buildCAX509List(null, conf); + CACertificateProvider caCerts = () -> HAUtils.buildCAX509List(conf); manager = new XceiverClientManager(conf, conf.getObject(XceiverClientManager.ScmClientConfig.class), new ClientTrustManager(caCerts, null)); diff --git a/hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/freon/DNRPCLoadGenerator.java b/hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/freon/DNRPCLoadGenerator.java index 5d3d3af9e1c4..1e84f7f4ba09 100644 --- a/hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/freon/DNRPCLoadGenerator.java +++ b/hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/freon/DNRPCLoadGenerator.java @@ -23,8 +23,8 @@ import org.apache.hadoop.hdds.client.StandaloneReplicationConfig; import org.apache.hadoop.hdds.scm.client.ClientTrustManager; import org.apache.hadoop.hdds.security.x509.certificate.client.CACertificateProvider; -import org.apache.hadoop.hdds.utils.HAUtils; import org.apache.hadoop.ozone.OzoneSecurityUtil; +import org.apache.hadoop.ozone.om.protocolPB.OzoneManagerProtocolClientSideTranslatorPB; import org.apache.hadoop.ozone.util.PayloadUtils; import org.apache.ratis.thirdparty.com.google.protobuf.ByteString; import org.apache.hadoop.hdds.cli.HddsVersionProvider; @@ -150,12 +150,15 @@ public Void call() throws Exception { } encodedContainerToken = scmClient.getEncodedContainerToken(containerID); XceiverClientFactory xceiverClientManager; + OzoneManagerProtocolClientSideTranslatorPB omClient; if (OzoneSecurityUtil.isSecurityEnabled(configuration)) { - CACertificateProvider caCerts = () -> HAUtils.buildCAX509List(null, configuration); + omClient = createOmClient(configuration, null); + CACertificateProvider caCerts = () -> omClient.getServiceInfo().provideCACerts(); xceiverClientManager = new XceiverClientManager(configuration, configuration.getObject(XceiverClientManager.ScmClientConfig.class), new ClientTrustManager(caCerts, null)); } else { + omClient = null; xceiverClientManager = new XceiverClientManager(configuration); } clients = new ArrayList<>(numClients); @@ -170,6 +173,9 @@ public Void call() throws Exception { try { runTests(this::sendRPCReq); } finally { + if (omClient != null) { + omClient.close(); + } for (XceiverClientSpi client : clients) { xceiverClientManager.releaseClient(client, false); } From b375f2fa973dced1a5b3e30b19637e25b1ad9b4a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Szabolcs=20G=C3=A1l?= Date: Fri, 19 Jul 2024 14:53:13 +0200 Subject: [PATCH 06/10] Use CreateClienttrustManager in ECReconstructionCoordinator --- .../hdds/scm/client/ClientTrustManager.java | 0 .../certificate/client/CertificateClient.java | 3 +++ .../statemachine/DatanodeStateMachine.java | 5 ++-- .../ECContainerOperationClient.java | 26 ++++++------------- .../ECReconstructionCoordinator.java | 5 ++-- .../client/DefaultCertificateClient.java | 13 ++++++++++ .../client/CertificateClientTestImpl.java | 12 +++++++++ .../scm/storage/TestContainerCommandsEC.java | 16 ++++++------ 8 files changed, 50 insertions(+), 30 deletions(-) rename hadoop-hdds/{client => common}/src/main/java/org/apache/hadoop/hdds/scm/client/ClientTrustManager.java (100%) diff --git a/hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/client/ClientTrustManager.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/client/ClientTrustManager.java similarity index 100% rename from hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/client/ClientTrustManager.java rename to hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/client/ClientTrustManager.java diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/security/x509/certificate/client/CertificateClient.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/security/x509/certificate/client/CertificateClient.java index 13deab100c0e..1c059c2c1d57 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/security/x509/certificate/client/CertificateClient.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/security/x509/certificate/client/CertificateClient.java @@ -19,6 +19,7 @@ package org.apache.hadoop.hdds.security.x509.certificate.client; +import org.apache.hadoop.hdds.scm.client.ClientTrustManager; import org.apache.hadoop.hdds.security.exception.OzoneSecurityException; import org.apache.hadoop.hdds.security.ssl.ReloadingX509KeyManager; import org.apache.hadoop.hdds.security.ssl.ReloadingX509TrustManager; @@ -162,6 +163,8 @@ default void assertValidKeysAndCertificate() throws OzoneSecurityException { ReloadingX509TrustManager getTrustManager() throws CertificateException; + ClientTrustManager createClientTrustManager() throws IOException; + /** * Register a receiver that will be called after the certificate renewed. * diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/statemachine/DatanodeStateMachine.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/statemachine/DatanodeStateMachine.java index 6046af1e0a79..eef621c8254f 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/statemachine/DatanodeStateMachine.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/statemachine/DatanodeStateMachine.java @@ -39,6 +39,7 @@ import org.apache.hadoop.hdds.protocol.proto.StorageContainerDatanodeProtocolProtos.ContainerReportsProto; import org.apache.hadoop.hdds.protocol.proto.StorageContainerDatanodeProtocolProtos.NodeReportProto; import org.apache.hadoop.hdds.protocol.proto.StorageContainerDatanodeProtocolProtos.PipelineReportsProto; +import org.apache.hadoop.hdds.scm.client.ClientTrustManager; import org.apache.hadoop.hdds.security.symmetric.SecretKeyClient; import org.apache.hadoop.hdds.security.x509.certificate.client.CertificateClient; import org.apache.hadoop.hdds.upgrade.HDDSLayoutVersionManager; @@ -214,9 +215,9 @@ public DatanodeStateMachine(DatanodeDetails datanodeDetails, ReplicationSupervisorMetrics.create(supervisor); ecReconstructionMetrics = ECReconstructionMetrics.create(); - + certClient.createClientTrustManager(); ecReconstructionCoordinator = new ECReconstructionCoordinator( - conf, certClient, secretKeyClient, context, ecReconstructionMetrics, + conf, certClient.createClientTrustManager(), secretKeyClient, context, ecReconstructionMetrics, threadNamePrefix); // This is created as an instance variable as Mockito needs to access it in diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/ec/reconstruction/ECContainerOperationClient.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/ec/reconstruction/ECContainerOperationClient.java index 8d6e981c8058..9feddc02293d 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/ec/reconstruction/ECContainerOperationClient.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/ec/reconstruction/ECContainerOperationClient.java @@ -65,29 +65,19 @@ public ECContainerOperationClient(XceiverClientManager clientManager) { } public ECContainerOperationClient(ConfigurationSource conf, - CertificateClient certificateClient) throws IOException { - this(createClientManager(conf, certificateClient)); + ClientTrustManager clientTrustManager) throws IOException { + this(createClientManager(conf, clientTrustManager)); } @Nonnull private static XceiverClientManager createClientManager( - ConfigurationSource conf, CertificateClient certificateClient) + ConfigurationSource conf, ClientTrustManager clientTrustManager) throws IOException { - ClientTrustManager trustManager = null; - - if (OzoneSecurityUtil.isSecurityEnabled(conf)) { - CACertificateProvider localCaCerts = () -> { - List caCerts = new ArrayList<>(); - caCerts.addAll(certificateClient.getAllCaCerts()); - caCerts.addAll(certificateClient.getAllRootCaCerts()); - return caCerts; - }; - trustManager = new ClientTrustManager(localCaCerts, localCaCerts); - } - return new XceiverClientManager(conf, - new XceiverClientManager.XceiverClientManagerConfigBuilder() - .setMaxCacheSize(256).setStaleThresholdMs(10 * 1000).build(), - trustManager); + XceiverClientManager.ScmClientConfig scmClientConfig = new XceiverClientManager.XceiverClientManagerConfigBuilder() + .setMaxCacheSize(256) + .setStaleThresholdMs(10 * 1000) + .build(); + return new XceiverClientManager(conf, scmClientConfig, clientTrustManager); } public BlockData[] listBlock(long containerId, DatanodeDetails dn, diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/ec/reconstruction/ECReconstructionCoordinator.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/ec/reconstruction/ECReconstructionCoordinator.java index 8fadd19b67d3..2069542a34ff 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/ec/reconstruction/ECReconstructionCoordinator.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/ec/reconstruction/ECReconstructionCoordinator.java @@ -29,6 +29,7 @@ import org.apache.hadoop.hdds.scm.ContainerClientMetrics; import org.apache.hadoop.hdds.scm.OzoneClientConfig; import org.apache.hadoop.hdds.scm.StreamBufferArgs; +import org.apache.hadoop.hdds.scm.client.ClientTrustManager; import org.apache.hadoop.hdds.scm.container.ContainerID; import org.apache.hadoop.hdds.scm.pipeline.Pipeline; import org.apache.hadoop.hdds.scm.pipeline.PipelineID; @@ -118,13 +119,13 @@ public class ECReconstructionCoordinator implements Closeable { private final OzoneClientConfig ozoneClientConfig; public ECReconstructionCoordinator( - ConfigurationSource conf, CertificateClient certificateClient, + ConfigurationSource conf, ClientTrustManager clientTrustManager, SecretKeySignerClient secretKeyClient, StateContext context, ECReconstructionMetrics metrics, String threadNamePrefix) throws IOException { this.context = context; this.containerOperationClient = new ECContainerOperationClient(conf, - certificateClient); + clientTrustManager); this.byteBufferPool = new ElasticByteBufferPool(); ozoneClientConfig = conf.getObject(OzoneClientConfig.class); this.ecReconstructReadExecutor = createThreadPoolExecutor( diff --git a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/security/x509/certificate/client/DefaultCertificateClient.java b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/security/x509/certificate/client/DefaultCertificateClient.java index 344e6158b5e1..48cbaaee99ce 100644 --- a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/security/x509/certificate/client/DefaultCertificateClient.java +++ b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/security/x509/certificate/client/DefaultCertificateClient.java @@ -73,6 +73,7 @@ import org.apache.commons.io.FileUtils; import org.apache.hadoop.hdds.protocol.proto.SCMSecurityProtocolProtos.SCMGetCertResponseProto; import org.apache.hadoop.hdds.protocolPB.SCMSecurityProtocolClientSideTranslatorPB; +import org.apache.hadoop.hdds.scm.client.ClientTrustManager; import org.apache.hadoop.hdds.security.SecurityConfig; import org.apache.hadoop.hdds.security.ssl.ReloadingX509KeyManager; import org.apache.hadoop.hdds.security.ssl.ReloadingX509TrustManager; @@ -1014,8 +1015,20 @@ public ReloadingX509KeyManager getKeyManager() throws CertificateException { } } + @Override + public ClientTrustManager createClientTrustManager() throws IOException { + CACertificateProvider caCertificateProvider = () -> { + List caCerts = new ArrayList<>(); + caCerts.addAll(getAllCaCerts()); + caCerts.addAll(getAllRootCaCerts()); + return caCerts; + }; + return new ClientTrustManager(caCertificateProvider, caCertificateProvider); + } + /** * Register a receiver that will be called after the certificate renewed. + * * @param receiver */ @Override diff --git a/hadoop-hdds/framework/src/test/java/org/apache/hadoop/hdds/security/x509/certificate/client/CertificateClientTestImpl.java b/hadoop-hdds/framework/src/test/java/org/apache/hadoop/hdds/security/x509/certificate/client/CertificateClientTestImpl.java index 7df50e34c17f..e92f6afca354 100644 --- a/hadoop-hdds/framework/src/test/java/org/apache/hadoop/hdds/security/x509/certificate/client/CertificateClientTestImpl.java +++ b/hadoop-hdds/framework/src/test/java/org/apache/hadoop/hdds/security/x509/certificate/client/CertificateClientTestImpl.java @@ -48,6 +48,7 @@ import com.google.common.util.concurrent.ThreadFactoryBuilder; import org.apache.hadoop.hdds.conf.OzoneConfiguration; +import org.apache.hadoop.hdds.scm.client.ClientTrustManager; import org.apache.hadoop.hdds.security.SecurityConfig; import org.apache.hadoop.hdds.security.ssl.ReloadingX509KeyManager; import org.apache.hadoop.hdds.security.ssl.ReloadingX509TrustManager; @@ -351,6 +352,17 @@ public ReloadingX509TrustManager getTrustManager() throws CertificateException { } } + @Override + public ClientTrustManager createClientTrustManager() throws IOException { + CACertificateProvider caCertificateProvider = () -> { + List caCerts = new ArrayList<>(); + caCerts.addAll(getAllCaCerts()); + caCerts.addAll(getAllRootCaCerts()); + return caCerts; + }; + return new ClientTrustManager(caCertificateProvider, caCertificateProvider); + } + @Override public void registerNotificationReceiver(CertificateNotification receiver) { synchronized (notificationReceivers) { diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/hdds/scm/storage/TestContainerCommandsEC.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/hdds/scm/storage/TestContainerCommandsEC.java index c274d8fea301..a2433497d79e 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/hdds/scm/storage/TestContainerCommandsEC.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/hdds/scm/storage/TestContainerCommandsEC.java @@ -363,7 +363,7 @@ public void testOrphanBlock() throws Exception { } try (ECReconstructionCoordinator coordinator = - new ECReconstructionCoordinator(config, certClient, + new ECReconstructionCoordinator(config, certClient.createClientTrustManager(), secretKeyClient, null, ECReconstructionMetrics.create(), "")) { @@ -673,7 +673,7 @@ private void testECReconstructionCoordinator(List missingIndexes, XceiverClientManager xceiverClientManager = new XceiverClientManager(config); ECReconstructionCoordinator coordinator = - new ECReconstructionCoordinator(config, certClient, secretKeyClient, + new ECReconstructionCoordinator(config, certClient.createClientTrustManager(), secretKeyClient, null, ECReconstructionMetrics.create(), "2")) { ECReconstructionMetrics metrics = @@ -723,14 +723,14 @@ private void testECReconstructionCoordinator(List missingIndexes, List blockDataArrList = new ArrayList<>(); try (ECContainerOperationClient ecContainerOperationClient = - new ECContainerOperationClient(config, certClient)) { + new ECContainerOperationClient(config, certClient.createClientTrustManager())) { for (int j = 0; j < containerToDeletePipeline.size(); j++) { Pipeline p = containerToDeletePipeline.get(j); org.apache.hadoop.ozone.container.common.helpers.BlockData[] blockData = ecContainerOperationClient.listBlock( - conID, p.getFirstNode(), - (ECReplicationConfig) p.getReplicationConfig(), - cToken); + conID, p.getFirstNode(), + (ECReplicationConfig) p.getReplicationConfig(), + cToken); blockDataArrList.add(blockData); // Delete the first index container XceiverClientSpi client = xceiverClientManager.acquireClient( @@ -869,7 +869,7 @@ public void testECReconstructionCoordinatorShouldCleanupContainersOnFailure() assertThrows(IOException.class, () -> { try (ECReconstructionCoordinator coordinator = - new ECReconstructionCoordinator(config, certClient, + new ECReconstructionCoordinator(config, certClient.createClientTrustManager(), secretKeyClient, null, ECReconstructionMetrics.create(), "")) { coordinator.reconstructECContainerGroup(conID, @@ -881,7 +881,7 @@ public void testECReconstructionCoordinatorShouldCleanupContainersOnFailure() StorageContainerException ex = assertThrows(StorageContainerException.class, () -> { try (ECContainerOperationClient client = - new ECContainerOperationClient(config, certClient)) { + new ECContainerOperationClient(config, certClient.createClientTrustManager())) { client.listBlock(conID, targetDNToCheckContainerCLeaned, new ECReplicationConfig(3, 2), cToken); } From 8e15cabb79b8c35e65e737a7c29c4825728b0168 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Szabolcs=20G=C3=A1l?= Date: Sun, 21 Jul 2024 09:00:17 +0200 Subject: [PATCH 07/10] Fix checkstyle and findbugs issues --- .../common/statemachine/DatanodeStateMachine.java | 8 ++++++-- .../ec/reconstruction/ECContainerOperationClient.java | 5 ----- .../ec/reconstruction/ECReconstructionCoordinator.java | 1 - 3 files changed, 6 insertions(+), 8 deletions(-) diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/statemachine/DatanodeStateMachine.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/statemachine/DatanodeStateMachine.java index eef621c8254f..4780a98237a1 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/statemachine/DatanodeStateMachine.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/statemachine/DatanodeStateMachine.java @@ -46,6 +46,7 @@ import org.apache.hadoop.hdds.utils.IOUtils; import org.apache.hadoop.hdds.utils.NettyMetrics; import org.apache.hadoop.ozone.HddsDatanodeStopService; +import org.apache.hadoop.ozone.OzoneSecurityUtil; import org.apache.hadoop.ozone.container.common.DatanodeLayoutStorage; import org.apache.hadoop.ozone.container.common.report.ReportManager; import org.apache.hadoop.ozone.container.common.statemachine.commandhandler.CloseContainerCommandHandler; @@ -215,9 +216,12 @@ public DatanodeStateMachine(DatanodeDetails datanodeDetails, ReplicationSupervisorMetrics.create(supervisor); ecReconstructionMetrics = ECReconstructionMetrics.create(); - certClient.createClientTrustManager(); + ClientTrustManager clientTrustManager = null; + if (OzoneSecurityUtil.isSecurityEnabled(conf)) { + clientTrustManager = certClient.createClientTrustManager(); + } ecReconstructionCoordinator = new ECReconstructionCoordinator( - conf, certClient.createClientTrustManager(), secretKeyClient, context, ecReconstructionMetrics, + conf, clientTrustManager, secretKeyClient, context, ecReconstructionMetrics, threadNamePrefix); // This is created as an instance variable as Mockito needs to access it in diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/ec/reconstruction/ECContainerOperationClient.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/ec/reconstruction/ECContainerOperationClient.java index 9feddc02293d..f9a132e42df5 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/ec/reconstruction/ECContainerOperationClient.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/ec/reconstruction/ECContainerOperationClient.java @@ -26,12 +26,9 @@ import org.apache.hadoop.hdds.scm.XceiverClientManager; import org.apache.hadoop.hdds.scm.XceiverClientSpi; import org.apache.hadoop.hdds.scm.client.ClientTrustManager; -import org.apache.hadoop.hdds.security.x509.certificate.client.CACertificateProvider; import org.apache.hadoop.hdds.scm.pipeline.Pipeline; import org.apache.hadoop.hdds.scm.pipeline.PipelineID; import org.apache.hadoop.hdds.scm.storage.ContainerProtocolCalls; -import org.apache.hadoop.hdds.security.x509.certificate.client.CertificateClient; -import org.apache.hadoop.ozone.OzoneSecurityUtil; import org.apache.hadoop.ozone.container.common.helpers.BlockData; import org.apache.hadoop.security.token.Token; import org.apache.hadoop.security.token.TokenIdentifier; @@ -42,8 +39,6 @@ import java.io.Closeable; import java.io.IOException; -import java.security.cert.X509Certificate; -import java.util.ArrayList; import java.util.List; import java.util.Objects; import java.util.Set; diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/ec/reconstruction/ECReconstructionCoordinator.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/ec/reconstruction/ECReconstructionCoordinator.java index 2069542a34ff..4cb12182c9ba 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/ec/reconstruction/ECReconstructionCoordinator.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/ec/reconstruction/ECReconstructionCoordinator.java @@ -39,7 +39,6 @@ import org.apache.hadoop.hdds.security.SecurityConfig; import org.apache.hadoop.hdds.security.symmetric.SecretKeySignerClient; import org.apache.hadoop.hdds.security.token.ContainerTokenIdentifier; -import org.apache.hadoop.hdds.security.x509.certificate.client.CertificateClient; import org.apache.hadoop.hdds.utils.IOUtils; import org.apache.hadoop.io.ByteBufferPool; import org.apache.hadoop.io.ElasticByteBufferPool; From ad6068f50dbc705a5c299ab7da2a8c08497e424b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Szabolcs=20G=C3=A1l?= Date: Tue, 30 Jul 2024 13:36:27 +0200 Subject: [PATCH 08/10] Add API documentation for recently changed methods. --- .../certificate/client/CertificateClient.java | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/security/x509/certificate/client/CertificateClient.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/security/x509/certificate/client/CertificateClient.java index 1c059c2c1d57..8a436fdddf73 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/security/x509/certificate/client/CertificateClient.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/security/x509/certificate/client/CertificateClient.java @@ -159,10 +159,30 @@ default void assertValidKeysAndCertificate() throws OzoneSecurityException { } } + /** + * Gets a KeyManager containing this CertificateClient's key material and trustchain. + * During certificate rotation this KeyManager is automatically updated with the new keys/certificates. + * + * @return A KeyManager containing keys and the trustchain for this CertificateClient. + * @throws CertificateException + */ ReloadingX509KeyManager getKeyManager() throws CertificateException; + /** + * Gets a TrustManager containing the trusted certificates of this CertificateClient. + * During certificate rotation this TrustManager is automatically updated with the new certificates. + * + * @return A TrustManager containing trusted certificates for this CertificateClient. + * @throws CertificateException + */ ReloadingX509TrustManager getTrustManager() throws CertificateException; + /** + * Creates a ClientTrustManager instance using the trusted certificates of this certificate client. + * + * @return The new ClientTrustManager instance. + * @throws IOException + */ ClientTrustManager createClientTrustManager() throws IOException; /** From e2727515451f6a20a7fe1ca301f7ce2b9b0aae6c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Szabolcs=20G=C3=A1l?= Date: Wed, 31 Jul 2024 14:55:22 +0200 Subject: [PATCH 09/10] Propagate CertificateClient instead of creating ClientTrustManager immediately in ECReconstructionCoordinator --- .../common/statemachine/DatanodeStateMachine.java | 6 +----- .../ECContainerOperationClient.java | 15 ++++++++++----- .../ECReconstructionCoordinator.java | 6 +++--- .../hdds/scm/storage/TestContainerCommandsEC.java | 10 +++++----- 4 files changed, 19 insertions(+), 18 deletions(-) diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/statemachine/DatanodeStateMachine.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/statemachine/DatanodeStateMachine.java index 4780a98237a1..5bca0feff97c 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/statemachine/DatanodeStateMachine.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/statemachine/DatanodeStateMachine.java @@ -46,7 +46,6 @@ import org.apache.hadoop.hdds.utils.IOUtils; import org.apache.hadoop.hdds.utils.NettyMetrics; import org.apache.hadoop.ozone.HddsDatanodeStopService; -import org.apache.hadoop.ozone.OzoneSecurityUtil; import org.apache.hadoop.ozone.container.common.DatanodeLayoutStorage; import org.apache.hadoop.ozone.container.common.report.ReportManager; import org.apache.hadoop.ozone.container.common.statemachine.commandhandler.CloseContainerCommandHandler; @@ -217,11 +216,8 @@ public DatanodeStateMachine(DatanodeDetails datanodeDetails, ecReconstructionMetrics = ECReconstructionMetrics.create(); ClientTrustManager clientTrustManager = null; - if (OzoneSecurityUtil.isSecurityEnabled(conf)) { - clientTrustManager = certClient.createClientTrustManager(); - } ecReconstructionCoordinator = new ECReconstructionCoordinator( - conf, clientTrustManager, secretKeyClient, context, ecReconstructionMetrics, + conf, certClient, secretKeyClient, context, ecReconstructionMetrics, threadNamePrefix); // This is created as an instance variable as Mockito needs to access it in diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/ec/reconstruction/ECContainerOperationClient.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/ec/reconstruction/ECContainerOperationClient.java index f9a132e42df5..487e6d37b282 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/ec/reconstruction/ECContainerOperationClient.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/ec/reconstruction/ECContainerOperationClient.java @@ -29,6 +29,8 @@ import org.apache.hadoop.hdds.scm.pipeline.Pipeline; import org.apache.hadoop.hdds.scm.pipeline.PipelineID; import org.apache.hadoop.hdds.scm.storage.ContainerProtocolCalls; +import org.apache.hadoop.hdds.security.x509.certificate.client.CertificateClient; +import org.apache.hadoop.ozone.OzoneSecurityUtil; import org.apache.hadoop.ozone.container.common.helpers.BlockData; import org.apache.hadoop.security.token.Token; import org.apache.hadoop.security.token.TokenIdentifier; @@ -60,19 +62,22 @@ public ECContainerOperationClient(XceiverClientManager clientManager) { } public ECContainerOperationClient(ConfigurationSource conf, - ClientTrustManager clientTrustManager) throws IOException { - this(createClientManager(conf, clientTrustManager)); + CertificateClient certificateClient) throws IOException { + this(createClientManager(conf, certificateClient)); } @Nonnull - private static XceiverClientManager createClientManager( - ConfigurationSource conf, ClientTrustManager clientTrustManager) + private static XceiverClientManager createClientManager(ConfigurationSource conf, CertificateClient certificateClient) throws IOException { + ClientTrustManager trustManager = null; + if (OzoneSecurityUtil.isSecurityEnabled(conf)) { + trustManager = certificateClient.createClientTrustManager(); + } XceiverClientManager.ScmClientConfig scmClientConfig = new XceiverClientManager.XceiverClientManagerConfigBuilder() .setMaxCacheSize(256) .setStaleThresholdMs(10 * 1000) .build(); - return new XceiverClientManager(conf, scmClientConfig, clientTrustManager); + return new XceiverClientManager(conf, scmClientConfig, trustManager); } public BlockData[] listBlock(long containerId, DatanodeDetails dn, diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/ec/reconstruction/ECReconstructionCoordinator.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/ec/reconstruction/ECReconstructionCoordinator.java index 4cb12182c9ba..a37f99ab17e9 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/ec/reconstruction/ECReconstructionCoordinator.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/ec/reconstruction/ECReconstructionCoordinator.java @@ -29,7 +29,6 @@ import org.apache.hadoop.hdds.scm.ContainerClientMetrics; import org.apache.hadoop.hdds.scm.OzoneClientConfig; import org.apache.hadoop.hdds.scm.StreamBufferArgs; -import org.apache.hadoop.hdds.scm.client.ClientTrustManager; import org.apache.hadoop.hdds.scm.container.ContainerID; import org.apache.hadoop.hdds.scm.pipeline.Pipeline; import org.apache.hadoop.hdds.scm.pipeline.PipelineID; @@ -39,6 +38,7 @@ import org.apache.hadoop.hdds.security.SecurityConfig; import org.apache.hadoop.hdds.security.symmetric.SecretKeySignerClient; import org.apache.hadoop.hdds.security.token.ContainerTokenIdentifier; +import org.apache.hadoop.hdds.security.x509.certificate.client.CertificateClient; import org.apache.hadoop.hdds.utils.IOUtils; import org.apache.hadoop.io.ByteBufferPool; import org.apache.hadoop.io.ElasticByteBufferPool; @@ -118,13 +118,13 @@ public class ECReconstructionCoordinator implements Closeable { private final OzoneClientConfig ozoneClientConfig; public ECReconstructionCoordinator( - ConfigurationSource conf, ClientTrustManager clientTrustManager, + ConfigurationSource conf, CertificateClient certClient, SecretKeySignerClient secretKeyClient, StateContext context, ECReconstructionMetrics metrics, String threadNamePrefix) throws IOException { this.context = context; this.containerOperationClient = new ECContainerOperationClient(conf, - clientTrustManager); + certClient); this.byteBufferPool = new ElasticByteBufferPool(); ozoneClientConfig = conf.getObject(OzoneClientConfig.class); this.ecReconstructReadExecutor = createThreadPoolExecutor( diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/hdds/scm/storage/TestContainerCommandsEC.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/hdds/scm/storage/TestContainerCommandsEC.java index a2433497d79e..0b93fb450ede 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/hdds/scm/storage/TestContainerCommandsEC.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/hdds/scm/storage/TestContainerCommandsEC.java @@ -363,7 +363,7 @@ public void testOrphanBlock() throws Exception { } try (ECReconstructionCoordinator coordinator = - new ECReconstructionCoordinator(config, certClient.createClientTrustManager(), + new ECReconstructionCoordinator(config, certClient, secretKeyClient, null, ECReconstructionMetrics.create(), "")) { @@ -673,7 +673,7 @@ private void testECReconstructionCoordinator(List missingIndexes, XceiverClientManager xceiverClientManager = new XceiverClientManager(config); ECReconstructionCoordinator coordinator = - new ECReconstructionCoordinator(config, certClient.createClientTrustManager(), secretKeyClient, + new ECReconstructionCoordinator(config, certClient, secretKeyClient, null, ECReconstructionMetrics.create(), "2")) { ECReconstructionMetrics metrics = @@ -723,7 +723,7 @@ private void testECReconstructionCoordinator(List missingIndexes, List blockDataArrList = new ArrayList<>(); try (ECContainerOperationClient ecContainerOperationClient = - new ECContainerOperationClient(config, certClient.createClientTrustManager())) { + new ECContainerOperationClient(config, certClient)) { for (int j = 0; j < containerToDeletePipeline.size(); j++) { Pipeline p = containerToDeletePipeline.get(j); org.apache.hadoop.ozone.container.common.helpers.BlockData[] @@ -869,7 +869,7 @@ public void testECReconstructionCoordinatorShouldCleanupContainersOnFailure() assertThrows(IOException.class, () -> { try (ECReconstructionCoordinator coordinator = - new ECReconstructionCoordinator(config, certClient.createClientTrustManager(), + new ECReconstructionCoordinator(config, certClient, secretKeyClient, null, ECReconstructionMetrics.create(), "")) { coordinator.reconstructECContainerGroup(conID, @@ -881,7 +881,7 @@ public void testECReconstructionCoordinatorShouldCleanupContainersOnFailure() StorageContainerException ex = assertThrows(StorageContainerException.class, () -> { try (ECContainerOperationClient client = - new ECContainerOperationClient(config, certClient.createClientTrustManager())) { + new ECContainerOperationClient(config, certClient)) { client.listBlock(conID, targetDNToCheckContainerCLeaned, new ECReplicationConfig(3, 2), cToken); } From 79f2e2a9de7c6f648f3eb9c1277b461823fffacf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Szabolcs=20G=C3=A1l?= Date: Tue, 27 Aug 2024 20:46:33 +0200 Subject: [PATCH 10/10] Fix minor issues found during review --- .../ec/reconstruction/ECReconstructionCoordinator.java | 4 ++-- .../main/java/org/apache/hadoop/hdds/utils/HAUtils.java | 8 -------- .../hadoop/hdds/scm/server/StorageContainerManager.java | 1 - .../hadoop/hdds/scm/storage/TestContainerCommandsEC.java | 8 ++++---- 4 files changed, 6 insertions(+), 15 deletions(-) diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/ec/reconstruction/ECReconstructionCoordinator.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/ec/reconstruction/ECReconstructionCoordinator.java index a37f99ab17e9..8fadd19b67d3 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/ec/reconstruction/ECReconstructionCoordinator.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/ec/reconstruction/ECReconstructionCoordinator.java @@ -118,13 +118,13 @@ public class ECReconstructionCoordinator implements Closeable { private final OzoneClientConfig ozoneClientConfig; public ECReconstructionCoordinator( - ConfigurationSource conf, CertificateClient certClient, + ConfigurationSource conf, CertificateClient certificateClient, SecretKeySignerClient secretKeyClient, StateContext context, ECReconstructionMetrics metrics, String threadNamePrefix) throws IOException { this.context = context; this.containerOperationClient = new ECContainerOperationClient(conf, - certClient); + certificateClient); this.byteBufferPool = new ElasticByteBufferPool(); ozoneClientConfig = conf.getObject(OzoneClientConfig.class); this.ecReconstructReadExecutor = createThreadPoolExecutor( diff --git a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/HAUtils.java b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/HAUtils.java index ed3b0f85ce30..0dc244bdbc79 100644 --- a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/HAUtils.java +++ b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/HAUtils.java @@ -35,7 +35,6 @@ import org.apache.hadoop.hdds.scm.proxy.SCMClientConfig; import org.apache.hadoop.hdds.scm.proxy.SCMContainerLocationFailoverProxyProvider; import org.apache.hadoop.hdds.security.exception.SCMSecurityException; -import org.apache.hadoop.hdds.security.x509.certificate.client.CertificateClient; import org.apache.hadoop.hdds.tracing.TracingUtil; import org.apache.hadoop.hdds.utils.db.DBDefinition; import org.apache.hadoop.hdds.utils.db.DBColumnFamilyDefinition; @@ -372,13 +371,6 @@ public static List getExistingSstFiles(File db) throws IOException { return sstList; } - private static List generateCAList(CertificateClient certClient) { - List caCertPemList = new ArrayList<>(certClient.getAllRootCaCerts()); - caCertPemList.addAll(certClient.getAllCaCerts()); - return caCertPemList; - } - - /** * Retry forever until CA list matches expected count. * @param task - task to get CA list. diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/StorageContainerManager.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/StorageContainerManager.java index 174ef6f823c9..da1576957bc9 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/StorageContainerManager.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/StorageContainerManager.java @@ -1610,7 +1610,6 @@ private void persistSCMCertificates() throws IOException { // TODO: see if we can avoid doing this during every restart. if (primaryScmNodeId != null && !primaryScmNodeId.equals( scmStorageConfig.getScmId())) { - getScmSecurityClientWithMaxRetry(configuration, getCurrentUser()); List pemEncodedCerts = getScmSecurityClientWithMaxRetry(configuration, getCurrentUser()).listCACertificate(); // Write the primary SCM CA and Root CA during startup. diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/hdds/scm/storage/TestContainerCommandsEC.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/hdds/scm/storage/TestContainerCommandsEC.java index 0b93fb450ede..c274d8fea301 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/hdds/scm/storage/TestContainerCommandsEC.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/hdds/scm/storage/TestContainerCommandsEC.java @@ -728,9 +728,9 @@ private void testECReconstructionCoordinator(List missingIndexes, Pipeline p = containerToDeletePipeline.get(j); org.apache.hadoop.ozone.container.common.helpers.BlockData[] blockData = ecContainerOperationClient.listBlock( - conID, p.getFirstNode(), - (ECReplicationConfig) p.getReplicationConfig(), - cToken); + conID, p.getFirstNode(), + (ECReplicationConfig) p.getReplicationConfig(), + cToken); blockDataArrList.add(blockData); // Delete the first index container XceiverClientSpi client = xceiverClientManager.acquireClient( @@ -881,7 +881,7 @@ public void testECReconstructionCoordinatorShouldCleanupContainersOnFailure() StorageContainerException ex = assertThrows(StorageContainerException.class, () -> { try (ECContainerOperationClient client = - new ECContainerOperationClient(config, certClient)) { + new ECContainerOperationClient(config, certClient)) { client.listBlock(conID, targetDNToCheckContainerCLeaned, new ECReplicationConfig(3, 2), cToken); }