From dab15385dc8cbc2a5ed65273c620bf12e0f14191 Mon Sep 17 00:00:00 2001 From: Galsza <109229906+Galsza@users.noreply.github.com> Date: Wed, 28 Aug 2024 01:59:41 +0200 Subject: [PATCH] HDDS-11216. Replace HAUtils#buildCAX509List usages with other direct usages (#6981) --- .../hdds/scm/client/ClientTrustManager.java | 0 .../certificate/client/CertificateClient.java | 40 +++--- .../statemachine/DatanodeStateMachine.java | 3 +- .../ECContainerOperationClient.java | 20 ++- .../client/DefaultCertificateClient.java | 50 ++------ .../org/apache/hadoop/hdds/utils/HAUtils.java | 116 ++++-------------- .../client/CertificateClientTestImpl.java | 22 ++-- .../scm/server/StorageContainerManager.java | 3 +- .../scm/cli/ContainerOperationClient.java | 2 +- .../ozone/freon/DNRPCLoadGenerator.java | 10 +- 10 files changed, 94 insertions(+), 172 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 79db6985e76..e74bb1f621a 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; @@ -128,23 +129,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. @@ -176,10 +160,32 @@ 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; + /** * 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 a460e30ede3..71a9e5bca9f 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; @@ -216,7 +217,7 @@ public DatanodeStateMachine(HddsDatanodeService hddsDatanodeService, ReplicationSupervisorMetrics.create(supervisor); ecReconstructionMetrics = ECReconstructionMetrics.create(); - + ClientTrustManager clientTrustManager = null; ecReconstructionCoordinator = new ECReconstructionCoordinator( conf, certClient, secretKeyClient, context, ecReconstructionMetrics, threadNamePrefix); 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 9dedd65565f..487e6d37b28 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,10 @@ 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.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; @@ -69,21 +67,17 @@ public ECContainerOperationClient(ConfigurationSource conf, } @Nonnull - private static XceiverClientManager createClientManager( - ConfigurationSource conf, CertificateClient certificateClient) + 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); + trustManager = certificateClient.createClientTrustManager(); } - 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, trustManager); } public BlockData[] listBlock(long containerId, DatanodeDetails dn, 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 70a475982bd..b277a759cb8 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; @@ -983,43 +984,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 { @@ -1049,8 +1013,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/main/java/org/apache/hadoop/hdds/utils/HAUtils.java b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/HAUtils.java index 342a0400cbd..0dc244bdbc7 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,8 +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.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,80 +371,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<>(); - for (X509Certificate cert : certClient.getAllRootCaCerts()) { - caCertPemList.add(CertificateCodec.getPEMEncodedString(cert)); - } - for (X509Certificate cert : certClient.getAllCaCerts()) { - caCertPemList.add(CertificateCodec.getPEMEncodedString(cert)); - } - return caCertPemList; - } - - /** * Retry forever until CA list matches expected count. * @param task - task to get CA list. @@ -488,23 +412,37 @@ 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 { - 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; + 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); + 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 { + int expectedCount = scmNodes.size() + 1; + if (scmNodes.size() > 1) { + return OzoneSecurityUtil.convertToX509(getCAListWithRetry(() -> waitForCACerts( + scmSecurityProtocolClient::listCACertificate, + expectedCount), waitDuration)); + } else { + return OzoneSecurityUtil.convertToX509(scmSecurityProtocolClient.listCACertificate()); } } - List pemEncodedCerts = HAUtils.buildCAList(certClient, conf); - return OzoneSecurityUtil.convertToX509(pemEncodedCerts); } } 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 fa784b75538..94ef86650c4 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.exception.SCMSecurityException; import org.apache.hadoop.hdds.security.SecurityConfig; import org.apache.hadoop.hdds.security.ssl.ReloadingX509KeyManager; @@ -257,16 +258,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(); @@ -364,6 +355,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-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 aaf6bbfc9c4..868e54f1935 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 @@ -1613,8 +1613,7 @@ private void persistSCMCertificates() throws IOException { if (primaryScmNodeId != null && !primaryScmNodeId.equals( scmStorageConfig.getScmId())) { 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 ba556bf24e9..76334d124ea 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 f83b2a1a4a9..a7527952ca3 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 @@ -33,8 +33,8 @@ import org.apache.hadoop.hdds.scm.pipeline.Pipeline; import org.apache.hadoop.hdds.scm.storage.ContainerProtocolCalls; 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.slf4j.Logger; @@ -150,11 +150,14 @@ 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 XceiverClientCreator(configuration, new ClientTrustManager(caCerts, null)); } else { + omClient = null; xceiverClientManager = new XceiverClientCreator(configuration); } clients = new ArrayList<>(numClients); @@ -169,6 +172,9 @@ public Void call() throws Exception { try { runTests(this::sendRPCReq); } finally { + if (omClient != null) { + omClient.close(); + } for (XceiverClientSpi client : clients) { xceiverClientManager.releaseClient(client, false); }