diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/security/ssl/PemFileBasedKeyStoresFactory.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/security/ssl/PemFileBasedKeyStoresFactory.java deleted file mode 100644 index 028d6c8e0329..000000000000 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/security/ssl/PemFileBasedKeyStoresFactory.java +++ /dev/null @@ -1,165 +0,0 @@ -/** -* Licensed to the Apache Software Foundation (ASF) under one -* or more contributor license agreements. See the NOTICE file -* distributed with this work for additional information -* regarding copyright ownership. The ASF licenses this file -* to you under the Apache License, Version 2.0 (the -* "License"); you may not use this file except in compliance -* with the License. You may obtain a copy of the License at -* -* http://www.apache.org/licenses/LICENSE-2.0 -* -* Unless required by applicable law or agreed to in writing, software -* distributed under the License is distributed on an "AS IS" BASIS, -* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -* See the License for the specific language governing permissions and -* limitations under the License. -*/ -package org.apache.hadoop.hdds.security.ssl; - -import org.apache.hadoop.hdds.annotation.InterfaceAudience; -import org.apache.hadoop.hdds.annotation.InterfaceStability; -import org.apache.hadoop.hdds.security.x509.certificate.client.CertificateClient; -import org.apache.hadoop.hdds.security.x509.certificate.client.CertificateNotification; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - -import javax.net.ssl.KeyManager; -import javax.net.ssl.KeyManagerFactory; -import javax.net.ssl.TrustManager; -import java.io.IOException; -import java.security.GeneralSecurityException; -import java.security.KeyStore; - -/** - * {@link KeyStoresFactory} implementation that reads the certificates from - * private key pem file and certificate pem file. - *

- * If either the truststore or the keystore certificates file changes, it - * would be refreshed under the corresponding wrapper implementation - - * {@link ReloadingX509KeyManager} or {@link ReloadingX509TrustManager}. - *

- */ -@InterfaceAudience.Private -@InterfaceStability.Evolving -public class PemFileBasedKeyStoresFactory implements KeyStoresFactory, - CertificateNotification { - - private static final Logger LOG = - LoggerFactory.getLogger(PemFileBasedKeyStoresFactory.class); - - private KeyManager[] keyManagers; - private TrustManager[] trustManagers; - private final CertificateClient caClient; - - public PemFileBasedKeyStoresFactory(CertificateClient client) { - this.caClient = client; - } - - /** - * Implements logic of initializing the TrustManagers with the options - * to reload truststore. - */ - private void createTrustManagers() throws - GeneralSecurityException, IOException { - ReloadingX509TrustManager trustManager = new ReloadingX509TrustManager(KeyStore.getDefaultType(), caClient); - trustManagers = new TrustManager[] {trustManager}; - } - - /** - * Implements logic of initializing the KeyManagers with the options - * to reload keystores. - */ - private void createKeyManagers() throws - GeneralSecurityException, IOException { - ReloadingX509KeyManager keystoreManager = new ReloadingX509KeyManager(KeyStore.getDefaultType(), caClient); - keyManagers = new KeyManager[] {keystoreManager}; - } - - /** - * Initializes the keystores of the factory. - * - * @param mode if the keystores are to be used in client or server mode. - * @param requireClientAuth whether client authentication is required. Ignore - * for client mode. - * @throws IOException thrown if the keystores could not be initialized due - * to an IO error. - * @throws GeneralSecurityException thrown if the keystores could not be - * initialized due to a security error. - */ - public synchronized void init(Mode mode, boolean requireClientAuth) - throws IOException, GeneralSecurityException { - - // key manager - if (requireClientAuth || mode == Mode.SERVER) { - createKeyManagers(); - } else { - KeyStore keystore = KeyStore.getInstance(KeyStore.getDefaultType()); - keystore.load(null, null); - KeyManagerFactory keyMgrFactory = KeyManagerFactory - .getInstance(KeyManagerFactory.getDefaultAlgorithm()); - - keyMgrFactory.init(keystore, null); - keyManagers = keyMgrFactory.getKeyManagers(); - } - - // trust manager - createTrustManagers(); - caClient.registerNotificationReceiver(this); - } - - /** - * Releases any resources being used. - */ - @Override - public synchronized void destroy() { - if (keyManagers != null) { - keyManagers = null; - } - - if (trustManagers != null) { - trustManagers = null; - } - } - - /** - * Returns the keymanagers for owned certificates. - */ - @Override - public synchronized KeyManager[] getKeyManagers() { - KeyManager[] copy = new KeyManager[keyManagers.length]; - System.arraycopy(keyManagers, 0, copy, 0, keyManagers.length); - return copy; - } - - /** - * Returns the trustmanagers for trusted certificates. - */ - @Override - public synchronized TrustManager[] getTrustManagers() { - TrustManager[] copy = new TrustManager[trustManagers.length]; - System.arraycopy(trustManagers, 0, copy, 0, trustManagers.length); - return copy; - } - - @Override - public synchronized void notifyCertificateRenewed( - CertificateClient certClient, String oldCertId, String newCertId) { - LOG.info("{} notify certificate renewed", certClient.getComponentName()); - if (keyManagers != null) { - for (KeyManager km: keyManagers) { - if (km instanceof ReloadingX509KeyManager) { - ((ReloadingX509KeyManager) km).loadFrom(certClient); - } - } - } - - if (trustManagers != null) { - for (TrustManager tm: trustManagers) { - if (tm instanceof ReloadingX509TrustManager) { - ((ReloadingX509TrustManager) tm).loadFrom(certClient); - } - } - } - } -} diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/security/ssl/ReloadingX509KeyManager.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/security/ssl/ReloadingX509KeyManager.java index d88f40b4be25..32c94d3ddc75 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/security/ssl/ReloadingX509KeyManager.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/security/ssl/ReloadingX509KeyManager.java @@ -20,6 +20,7 @@ import org.apache.hadoop.hdds.annotation.InterfaceAudience; import org.apache.hadoop.hdds.annotation.InterfaceStability; import org.apache.hadoop.hdds.security.x509.certificate.client.CertificateClient; +import org.apache.hadoop.hdds.security.x509.certificate.client.CertificateNotification; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -34,22 +35,20 @@ import java.security.Principal; import java.security.PrivateKey; import java.security.cert.X509Certificate; -import java.util.ArrayList; import java.util.Arrays; import java.util.List; import java.util.Locale; import java.util.concurrent.atomic.AtomicReference; /** - * An implementation of X509KeyManager that exposes a method, - * {@link #loadFrom(CertificateClient)} to reload its configuration. + * An implementation of X509KeyManager that can be notified of certificate changes. * Note that it is necessary to implement the * X509ExtendedKeyManager to properly delegate * the additional methods, otherwise the SSL handshake will fail. */ @InterfaceAudience.Private @InterfaceStability.Evolving -public class ReloadingX509KeyManager extends X509ExtendedKeyManager { +public class ReloadingX509KeyManager extends X509ExtendedKeyManager implements CertificateNotification { public static final Logger LOG = LoggerFactory.getLogger(ReloadingX509KeyManager.class); @@ -67,27 +66,30 @@ public class ReloadingX509KeyManager extends X509ExtendedKeyManager { * materials are changed. */ private PrivateKey currentPrivateKey; - private List currentCertIdsList = new ArrayList<>(); - private String alias; + private List currentTrustChain; + private final String alias; /** * Construct a Reloading509KeystoreManager. * - * @param type type of keystore file, typically 'jks'. - * @param caClient client to get the private key and certificate materials. + * @param type type of keystore file, typically 'jks'. + * @param componentName the name of the component for which the keys are created. + * @param privateKey private key for this key manager. + * @param trustChain list of the trusted certificates. * @throws IOException * @throws GeneralSecurityException */ - public ReloadingX509KeyManager(String type, CertificateClient caClient) + public ReloadingX509KeyManager(String type, String componentName, PrivateKey privateKey, + List trustChain) throws GeneralSecurityException, IOException { this.type = type; + alias = componentName + "_key"; keyManagerRef = new AtomicReference<>(); - keyManagerRef.set(loadKeyManager(caClient)); + keyManagerRef.set(init(privateKey, trustChain)); } @Override - public String chooseEngineClientAlias(String[] strings, - Principal[] principals, SSLEngine sslEngine) { + public String chooseEngineClientAlias(String[] strings, Principal[] principals, SSLEngine sslEngine) { String ret = keyManagerRef.get() .chooseEngineClientAlias(strings, principals, sslEngine); @@ -184,29 +186,9 @@ public PrivateKey getPrivateKey(String s) { return keyManagerRef.get().getPrivateKey(s.toLowerCase(Locale.ROOT)); } - public ReloadingX509KeyManager loadFrom(CertificateClient caClient) { - try { - X509ExtendedKeyManager manager = loadKeyManager(caClient); - if (manager != null) { - keyManagerRef.set(manager); - LOG.info("ReloadingX509KeyManager is reloaded"); - } - } catch (Exception ex) { - // The Consumer.accept interface forces us to convert to unchecked - throw new RuntimeException(ex); - } - return this; - } - - private X509ExtendedKeyManager loadKeyManager(CertificateClient caClient) + private X509ExtendedKeyManager init(PrivateKey newPrivateKey, List newTrustChain) throws GeneralSecurityException, IOException { - PrivateKey privateKey = caClient.getPrivateKey(); - List newCertList = caClient.getTrustChain(); - if (currentPrivateKey != null && currentPrivateKey.equals(privateKey) && - currentCertIdsList.size() > 0 && - newCertList.size() == currentCertIdsList.size() && - newCertList.stream().allMatch(c -> - currentCertIdsList.contains(c.getSerialNumber().toString()))) { + if (isAlreadyUsing(newPrivateKey, newTrustChain)) { // Security materials(key and certificates) keep the same. return null; } @@ -215,30 +197,54 @@ private X509ExtendedKeyManager loadKeyManager(CertificateClient caClient) KeyStore keystore = KeyStore.getInstance(type); keystore.load(null, null); - alias = caClient.getComponentName() + "_key"; - keystore.setKeyEntry(alias, privateKey, EMPTY_PASSWORD, - newCertList.toArray(new X509Certificate[0])); + keystore.setKeyEntry(alias, newPrivateKey, EMPTY_PASSWORD, + newTrustChain.toArray(new X509Certificate[0])); LOG.info("Key manager is loaded with certificate chain"); - for (X509Certificate x509Certificate : newCertList) { + for (X509Certificate x509Certificate : newTrustChain) { LOG.info(x509Certificate.toString()); } KeyManagerFactory keyMgrFactory = KeyManagerFactory.getInstance( KeyManagerFactory.getDefaultAlgorithm()); keyMgrFactory.init(keystore, EMPTY_PASSWORD); - for (KeyManager candidate: keyMgrFactory.getKeyManagers()) { + for (KeyManager candidate : keyMgrFactory.getKeyManagers()) { if (candidate instanceof X509ExtendedKeyManager) { - keyManager = (X509ExtendedKeyManager)candidate; + keyManager = (X509ExtendedKeyManager) candidate; break; } } - currentPrivateKey = privateKey; - currentCertIdsList.clear(); - for (X509Certificate cert: newCertList) { - currentCertIdsList.add(cert.getSerialNumber().toString()); - } + currentPrivateKey = newPrivateKey; + currentTrustChain = newTrustChain; return keyManager; } + + private boolean isAlreadyUsing(PrivateKey privateKey, List newTrustChain) { + return currentPrivateKey != null && currentPrivateKey.equals(privateKey) && + currentTrustChain.size() > 0 && + newTrustChain.size() == currentTrustChain.size() && + newTrustChain.stream() + .allMatch( + newCertificate -> (currentTrustChain.stream() + .anyMatch(oldCert -> oldCert.getSerialNumber().equals(newCertificate.getSerialNumber())) + ) + ); + } + + @Override + public synchronized void notifyCertificateRenewed( + CertificateClient certClient, String oldCertId, String newCertId) { + LOG.info("{} notify certificate renewed", certClient.getComponentName()); + try { + X509ExtendedKeyManager manager = init(certClient.getPrivateKey(), certClient.getTrustChain()); + if (manager != null) { + keyManagerRef.set(manager); + LOG.info("ReloadingX509KeyManager is reloaded"); + } + } catch (Exception ex) { + // The Consumer.accept interface forces us to convert to unchecked + throw new RuntimeException(ex); + } + } } diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/security/ssl/ReloadingX509TrustManager.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/security/ssl/ReloadingX509TrustManager.java index bfc3939cd0a2..0620adf4975b 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/security/ssl/ReloadingX509TrustManager.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/security/ssl/ReloadingX509TrustManager.java @@ -20,6 +20,7 @@ import org.apache.hadoop.hdds.annotation.InterfaceAudience; import org.apache.hadoop.hdds.annotation.InterfaceStability; import org.apache.hadoop.hdds.security.x509.certificate.client.CertificateClient; +import org.apache.hadoop.hdds.security.x509.certificate.client.CertificateNotification; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -42,12 +43,12 @@ /** * A {@link TrustManager} implementation that exposes a method, - * {@link #loadFrom(CertificateClient)} to reload its configuration for + * {@link #init(List)} to reload its configuration for * example when the truststore file on disk changes. */ @InterfaceAudience.Private @InterfaceStability.Evolving -public final class ReloadingX509TrustManager implements X509TrustManager { +public final class ReloadingX509TrustManager implements X509TrustManager, CertificateNotification { public static final Logger LOG = LoggerFactory.getLogger(ReloadingX509TrustManager.class); @@ -57,27 +58,28 @@ public final class ReloadingX509TrustManager implements X509TrustManager { private final String type; private final AtomicReference trustManagerRef; + /** * Current Root CA cert in trustManager, to detect if certificate is changed. */ - private List currentRootCACertIds = new ArrayList<>(); + private List currentRootCACerts = new ArrayList<>(); /** * Creates a reloadable trustmanager. The trustmanager reloads itself * if the underlying truststore materials have changed. * - * @param type type of truststore file, typically 'jks'. - * @param caClient client to get trust certificates. - * @throws IOException thrown if the truststore could not be initialized due - * to an IO error. + * @param type type of truststore file, typically 'jks'. + * @param newRootCaCerts the newest known trusted certificates. + * @throws IOException thrown if the truststore could not be initialized due + * to an IO error. * @throws GeneralSecurityException thrown if the truststore could not be - * initialized due to a security error. + * initialized due to a security error. */ - public ReloadingX509TrustManager(String type, CertificateClient caClient) + public ReloadingX509TrustManager(String type, List newRootCaCerts) throws GeneralSecurityException, IOException { this.type = type; trustManagerRef = new AtomicReference(); - trustManagerRef.set(loadTrustManager(caClient)); + trustManagerRef.set(init(newRootCaCerts)); } @Override @@ -133,39 +135,17 @@ public X509Certificate[] getAcceptedIssuers() { return issuers; } - public ReloadingX509TrustManager loadFrom(CertificateClient caClient) { - try { - X509TrustManager manager = loadTrustManager(caClient); - if (manager != null) { - this.trustManagerRef.set(manager); - LOG.info("ReloadingX509TrustManager is reloaded."); - } - } catch (Exception ex) { - // The Consumer.accept interface forces us to convert to unchecked - throw new RuntimeException(RELOAD_ERROR_MESSAGE, ex); - } - return this; - } - - X509TrustManager loadTrustManager(CertificateClient caClient) + private X509TrustManager init(List newRootCaCerts) throws GeneralSecurityException, IOException { - // SCM certificate client sets root CA as CA cert instead of root CA cert - Set certList = caClient.getAllRootCaCerts(); - Set rootCACerts = certList.isEmpty() ? - caClient.getAllCaCerts() : certList; - // Certificate keeps the same. - if (rootCACerts.size() > 0 && - currentRootCACertIds.size() == rootCACerts.size() && - rootCACerts.stream().allMatch(c -> - currentRootCACertIds.contains(c.getSerialNumber().toString()))) { + if (isAlreadyUsing(newRootCaCerts)) { return null; } X509TrustManager trustManager = null; KeyStore ks = KeyStore.getInstance(type); ks.load(null, null); - insertCertsToKeystore(rootCACerts, ks); + insertCertsToKeystore(newRootCaCerts, ks); TrustManagerFactory trustManagerFactory = TrustManagerFactory.getInstance( TrustManagerFactory.getDefaultAlgorithm()); @@ -177,12 +157,20 @@ X509TrustManager loadTrustManager(CertificateClient caClient) break; } } - currentRootCACertIds.clear(); - rootCACerts.forEach( - c -> currentRootCACertIds.add(c.getSerialNumber().toString())); + currentRootCACerts = newRootCaCerts; return trustManager; } + private boolean isAlreadyUsing(List newRootCaCerts) { + return newRootCaCerts.size() > 0 && + currentRootCACerts.size() == newRootCaCerts.size() && + newRootCaCerts.stream() + .allMatch( + newCert -> currentRootCACerts.stream() + .anyMatch(currentCert -> currentCert.getSerialNumber().equals(newCert.getSerialNumber())) + ); + } + private void insertCertsToKeystore(Iterable certs, KeyStore ks) throws KeyStoreException { LOG.info("Trust manager is loaded with certificates"); @@ -192,4 +180,22 @@ private void insertCertsToKeystore(Iterable certs, LOG.info(certToInsert.toString()); } } + + @Override + public synchronized void notifyCertificateRenewed( + CertificateClient certClient, String oldCertId, String newCertId) { + LOG.info("{} notify certificate renewed", certClient.getComponentName()); + Set certList = certClient.getAllRootCaCerts(); + Set rootCaCerts = certList.isEmpty() ? certClient.getAllCaCerts() : certList; + try { + X509TrustManager manager = init(new ArrayList<>(rootCaCerts)); + if (manager != null) { + this.trustManagerRef.set(manager); + LOG.info("ReloadingX509TrustManager is reloaded."); + } + } catch (Exception ex) { + // The Consumer.accept interface forces us to convert to unchecked + throw new RuntimeException(RELOAD_ERROR_MESSAGE, ex); + } + } } 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 e196d0df9d7f..0c23a846563a 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 @@ -20,7 +20,8 @@ package org.apache.hadoop.hdds.security.x509.certificate.client; import org.apache.hadoop.hdds.security.exception.OzoneSecurityException; -import org.apache.hadoop.hdds.security.ssl.KeyStoresFactory; +import org.apache.hadoop.hdds.security.ssl.ReloadingX509KeyManager; +import org.apache.hadoop.hdds.security.ssl.ReloadingX509TrustManager; import org.apache.hadoop.hdds.security.x509.certificate.utils.CertificateSignRequest; import org.apache.hadoop.hdds.security.x509.exception.CertificateException; @@ -174,15 +175,9 @@ default void assertValidKeysAndCertificate() throws OzoneSecurityException { } } - /** - * Return the store factory for key manager and trust manager for server. - */ - KeyStoresFactory getServerKeyStoresFactory() throws CertificateException; + ReloadingX509KeyManager getKeyManager() throws CertificateException; - /** - * Return the store factory for key manager and trust manager for client. - */ - KeyStoresFactory getClientKeyStoresFactory() throws CertificateException; + ReloadingX509TrustManager getTrustManager() throws CertificateException; /** * 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/transport/server/XceiverServerGrpc.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/transport/server/XceiverServerGrpc.java index 346b05ebb4c1..ad9c5c9d9ca0 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/transport/server/XceiverServerGrpc.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/transport/server/XceiverServerGrpc.java @@ -155,7 +155,7 @@ public XceiverServerGrpc(DatanodeDetails datanodeDetails, if (secConf.isSecurityEnabled() && secConf.isGrpcTlsEnabled()) { try { SslContextBuilder sslClientContextBuilder = SslContextBuilder.forServer( - caClient.getServerKeyStoresFactory().getKeyManagers()[0]); + caClient.getKeyManager()); SslContextBuilder sslContextBuilder = GrpcSslContexts.configure( sslClientContextBuilder, secConf.getGrpcSslProvider()); nettyServerBuilder.sslContext(sslContextBuilder.build()); diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/transport/server/ratis/XceiverServerRatis.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/transport/server/ratis/XceiverServerRatis.java index e1df809c8aea..dc0c4b067603 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/transport/server/ratis/XceiverServerRatis.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/transport/server/ratis/XceiverServerRatis.java @@ -56,7 +56,6 @@ import org.apache.hadoop.hdds.ratis.RatisHelper; import org.apache.hadoop.hdds.scm.pipeline.PipelineID; import org.apache.hadoop.hdds.security.SecurityConfig; -import org.apache.hadoop.hdds.security.ssl.KeyStoresFactory; import org.apache.hadoop.hdds.security.x509.certificate.client.CertificateClient; import org.apache.hadoop.hdds.tracing.TracingUtil; import org.apache.hadoop.hdds.utils.HddsServerUtil; @@ -542,14 +541,12 @@ public static XceiverServerRatis newXceiverServerRatis( private static Parameters createTlsParameters(SecurityConfig conf, CertificateClient caClient) throws IOException { if (conf.isSecurityEnabled() && conf.isGrpcTlsEnabled()) { - KeyStoresFactory managerFactory = - caClient.getServerKeyStoresFactory(); GrpcTlsConfig serverConfig = new GrpcTlsConfig( - managerFactory.getKeyManagers()[0], - managerFactory.getTrustManagers()[0], true); + caClient.getKeyManager(), + caClient.getTrustManager(), true); GrpcTlsConfig clientConfig = new GrpcTlsConfig( - managerFactory.getKeyManagers()[0], - managerFactory.getTrustManagers()[0], false); + caClient.getKeyManager(), + caClient.getTrustManager(), false); return RatisHelper.setServerTlsConf(serverConfig, clientConfig); } diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/ozoneimpl/OzoneContainer.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/ozoneimpl/OzoneContainer.java index aef3965dcd49..4fa211a92c5a 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/ozoneimpl/OzoneContainer.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/ozoneimpl/OzoneContainer.java @@ -260,8 +260,8 @@ public OzoneContainer( if (certClient != null && secConf.isGrpcTlsEnabled()) { tlsClientConfig = new GrpcTlsConfig( - certClient.getClientKeyStoresFactory().getKeyManagers()[0], - certClient.getClientKeyStoresFactory().getTrustManagers()[0], true); + certClient.getKeyManager(), + certClient.getTrustManager(), true); } else { tlsClientConfig = null; } diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/replication/GrpcReplicationClient.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/replication/GrpcReplicationClient.java index 6c9cdc3fef10..ad4f4293b914 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/replication/GrpcReplicationClient.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/replication/GrpcReplicationClient.java @@ -35,7 +35,6 @@ import org.apache.hadoop.hdds.protocol.datanode.proto.IntraDatanodeProtocolServiceGrpc; import org.apache.hadoop.hdds.protocol.datanode.proto.IntraDatanodeProtocolServiceGrpc.IntraDatanodeProtocolServiceStub; import org.apache.hadoop.hdds.security.SecurityConfig; -import org.apache.hadoop.hdds.security.ssl.KeyStoresFactory; import org.apache.hadoop.hdds.security.x509.certificate.client.CertificateClient; import org.apache.hadoop.ozone.OzoneConsts; @@ -82,11 +81,10 @@ public GrpcReplicationClient( SslContextBuilder sslContextBuilder = GrpcSslContexts.forClient(); if (certClient != null) { - KeyStoresFactory factory = certClient.getClientKeyStoresFactory(); sslContextBuilder - .trustManager(factory.getTrustManagers()[0]) + .trustManager(certClient.getTrustManager()) .clientAuth(ClientAuth.REQUIRE) - .keyManager(factory.getKeyManagers()[0]); + .keyManager(certClient.getKeyManager()); } if (secConfig.useTestCert()) { channelBuilder.overrideAuthority("localhost"); diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/replication/ReplicationServer.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/replication/ReplicationServer.java index f72ca2a6881d..b4e92a4a60a2 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/replication/ReplicationServer.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/replication/ReplicationServer.java @@ -115,15 +115,13 @@ public void init(boolean enableZeroCopy) { if (secConf.isSecurityEnabled() && secConf.isGrpcTlsEnabled()) { try { - SslContextBuilder sslContextBuilder = SslContextBuilder.forServer( - caClient.getServerKeyStoresFactory().getKeyManagers()[0]); + SslContextBuilder sslContextBuilder = SslContextBuilder.forServer(caClient.getKeyManager()); sslContextBuilder = GrpcSslContexts.configure( sslContextBuilder, secConf.getGrpcSslProvider()); sslContextBuilder.clientAuth(ClientAuth.REQUIRE); - sslContextBuilder.trustManager( - caClient.getServerKeyStoresFactory().getTrustManagers()[0]); + sslContextBuilder.trustManager(caClient.getTrustManager()); nettyServerBuilder.sslContext(sslContextBuilder.build()); } catch (IOException ex) { 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 8c8c00f6e91b..2fb258e1a29e 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 @@ -27,9 +27,11 @@ import java.nio.file.Path; import java.nio.file.Paths; import java.nio.file.StandardCopyOption; +import java.security.GeneralSecurityException; import java.security.InvalidKeyException; import java.security.KeyFactory; import java.security.KeyPair; +import java.security.KeyStore; import java.security.NoSuchAlgorithmException; import java.security.NoSuchProviderException; import java.security.PrivateKey; @@ -72,14 +74,14 @@ import org.apache.hadoop.hdds.protocol.proto.SCMSecurityProtocolProtos.SCMGetCertResponseProto; import org.apache.hadoop.hdds.protocolPB.SCMSecurityProtocolClientSideTranslatorPB; import org.apache.hadoop.hdds.security.SecurityConfig; -import org.apache.hadoop.hdds.security.ssl.KeyStoresFactory; +import org.apache.hadoop.hdds.security.ssl.ReloadingX509KeyManager; +import org.apache.hadoop.hdds.security.ssl.ReloadingX509TrustManager; import org.apache.hadoop.hdds.security.x509.certificate.authority.CAType; import org.apache.hadoop.hdds.security.x509.certificate.utils.CertificateCodec; import org.apache.hadoop.hdds.security.x509.certificate.utils.CertificateSignRequest; import org.apache.hadoop.hdds.security.x509.exception.CertificateException; import org.apache.hadoop.hdds.security.x509.keys.HDDSKeyGenerator; import org.apache.hadoop.hdds.security.x509.keys.KeyCodec; -import org.apache.hadoop.hdds.security.x509.keys.SecurityUtil; import org.apache.hadoop.ozone.OzoneSecurityUtil; import com.google.common.base.Preconditions; @@ -126,8 +128,8 @@ public abstract class DefaultCertificateClient implements CertificateClient { private final String threadNamePrefix; private List pemEncodedCACerts = null; private Lock pemEncodedCACertsLock = new ReentrantLock(); - private KeyStoresFactory serverKeyStoresFactory; - private KeyStoresFactory clientKeyStoresFactory; + private ReloadingX509KeyManager keyManager; + private ReloadingX509TrustManager trustManager; private ScheduledExecutorService executorService; private Consumer certIdSaveCallback; @@ -1021,21 +1023,32 @@ public List updateCAList() throws IOException { } @Override - public synchronized KeyStoresFactory getServerKeyStoresFactory() - throws CertificateException { - if (serverKeyStoresFactory == null) { - serverKeyStoresFactory = SecurityUtil.getServerKeyStoresFactory(this, true); + public ReloadingX509TrustManager getTrustManager() throws CertificateException { + try { + if (trustManager == null) { + Set newRootCaCerts = rootCaCertificates.isEmpty() ? + caCertificates : rootCaCertificates; + trustManager = new ReloadingX509TrustManager(KeyStore.getDefaultType(), new ArrayList<>(newRootCaCerts)); + notificationReceivers.add(trustManager); + } + return trustManager; + } catch (IOException | GeneralSecurityException e) { + throw new CertificateException("Failed to init trustManager", e, CertificateException.ErrorCode.KEYSTORE_ERROR); } - return serverKeyStoresFactory; } @Override - public KeyStoresFactory getClientKeyStoresFactory() - throws CertificateException { - if (clientKeyStoresFactory == null) { - clientKeyStoresFactory = SecurityUtil.getClientKeyStoresFactory(this, true); + public ReloadingX509KeyManager getKeyManager() throws CertificateException { + try { + if (keyManager == null) { + keyManager = new ReloadingX509KeyManager( + KeyStore.getDefaultType(), getComponentName(), getPrivateKey(), getTrustChain()); + notificationReceivers.add(keyManager); + } + return keyManager; + } catch (IOException | GeneralSecurityException e) { + throw new CertificateException("Failed to init keyManager", e, CertificateException.ErrorCode.KEYSTORE_ERROR); } - return clientKeyStoresFactory; } /** @@ -1071,14 +1084,6 @@ public synchronized void close() throws IOException { if (rootCaRotationPoller != null) { rootCaRotationPoller.close(); } - - if (serverKeyStoresFactory != null) { - serverKeyStoresFactory.destroy(); - } - - if (clientKeyStoresFactory != null) { - clientKeyStoresFactory.destroy(); - } } /** diff --git a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/security/x509/keys/SecurityUtil.java b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/security/x509/keys/SecurityUtil.java index 96fb2a7fd916..41545fb7e9ad 100644 --- a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/security/x509/keys/SecurityUtil.java +++ b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/security/x509/keys/SecurityUtil.java @@ -18,8 +18,6 @@ */ package org.apache.hadoop.hdds.security.x509.keys; -import java.io.IOException; -import java.security.GeneralSecurityException; import java.security.KeyFactory; import java.security.NoSuchAlgorithmException; import java.security.NoSuchProviderException; @@ -30,10 +28,6 @@ import java.security.spec.X509EncodedKeySpec; import org.apache.hadoop.hdds.security.SecurityConfig; -import org.apache.hadoop.hdds.security.ssl.KeyStoresFactory; -import org.apache.hadoop.hdds.security.ssl.PemFileBasedKeyStoresFactory; -import org.apache.hadoop.hdds.security.x509.certificate.client.CertificateClient; -import org.apache.hadoop.hdds.security.x509.exception.CertificateException; /** * Utility functions for Security modules for Ozone. @@ -93,33 +87,4 @@ public static PublicKey getPublicKey(byte[] encodedKey, } return key; } - - public static KeyStoresFactory getServerKeyStoresFactory( - CertificateClient client, - boolean requireClientAuth) throws CertificateException { - PemFileBasedKeyStoresFactory factory = - new PemFileBasedKeyStoresFactory(client); - try { - factory.init(KeyStoresFactory.Mode.SERVER, requireClientAuth); - } catch (IOException | GeneralSecurityException e) { - throw new CertificateException("Failed to init keyStoresFactory", e, - CertificateException.ErrorCode.KEYSTORE_ERROR); - } - return factory; - } - - public static KeyStoresFactory getClientKeyStoresFactory( - CertificateClient client, - boolean requireClientAuth) throws CertificateException { - PemFileBasedKeyStoresFactory factory = - new PemFileBasedKeyStoresFactory(client); - - try { - factory.init(KeyStoresFactory.Mode.CLIENT, requireClientAuth); - } catch (IOException | GeneralSecurityException e) { - throw new CertificateException("Failed to init keyStoresFactory", e, - CertificateException.ErrorCode.KEYSTORE_ERROR); - } - return factory; - } } diff --git a/hadoop-hdds/framework/src/test/java/org/apache/hadoop/hdds/security/ssl/TestReloadingX509KeyManager.java b/hadoop-hdds/framework/src/test/java/org/apache/hadoop/hdds/security/ssl/TestReloadingX509KeyManager.java index c3b515e7ef72..539ec48f6ed1 100644 --- a/hadoop-hdds/framework/src/test/java/org/apache/hadoop/hdds/security/ssl/TestReloadingX509KeyManager.java +++ b/hadoop-hdds/framework/src/test/java/org/apache/hadoop/hdds/security/ssl/TestReloadingX509KeyManager.java @@ -24,7 +24,6 @@ import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; -import javax.net.ssl.KeyManager; import java.security.PrivateKey; import static org.assertj.core.api.Assertions.assertThat; @@ -48,23 +47,20 @@ public static void setUp() throws Exception { @Test public void testReload() throws Exception { - KeyManager km = caClient.getServerKeyStoresFactory().getKeyManagers()[0]; + ReloadingX509KeyManager km = caClient.getKeyManager(); PrivateKey privateKey1 = caClient.getPrivateKey(); - assertEquals(privateKey1, ((ReloadingX509KeyManager)km).getPrivateKey( - caClient.getComponentName() + "_key")); + assertEquals(privateKey1, km.getPrivateKey(caClient.getComponentName() + "_key")); caClient.renewRootCA(); caClient.renewKey(); PrivateKey privateKey2 = caClient.getPrivateKey(); assertNotEquals(privateKey1, privateKey2); - assertEquals(privateKey2, ((ReloadingX509KeyManager)km).getPrivateKey( - caClient.getComponentName() + "_key")); + assertEquals(privateKey2, km.getPrivateKey(caClient.getComponentName() + "_key")); assertThat(reloaderLog.getOutput()).contains("ReloadingX509KeyManager is reloaded"); - // Make sure there is two reloads happened, one for server, one for client - assertEquals(2, StringUtils.countMatches(reloaderLog.getOutput(), - "ReloadingX509KeyManager is reloaded")); + // Only one reload has to happen for the CertificateClient's keyManager. + assertEquals(1, StringUtils.countMatches(reloaderLog.getOutput(), "ReloadingX509KeyManager is reloaded")); } } diff --git a/hadoop-hdds/framework/src/test/java/org/apache/hadoop/hdds/security/ssl/TestReloadingX509TrustManager.java b/hadoop-hdds/framework/src/test/java/org/apache/hadoop/hdds/security/ssl/TestReloadingX509TrustManager.java index 8db0a9801ebe..5575411524ca 100644 --- a/hadoop-hdds/framework/src/test/java/org/apache/hadoop/hdds/security/ssl/TestReloadingX509TrustManager.java +++ b/hadoop-hdds/framework/src/test/java/org/apache/hadoop/hdds/security/ssl/TestReloadingX509TrustManager.java @@ -46,9 +46,7 @@ public static void setUp() throws Exception { @Test public void testReload() throws Exception { - ReloadingX509TrustManager tm = - (ReloadingX509TrustManager) caClient.getServerKeyStoresFactory() - .getTrustManagers()[0]; + ReloadingX509TrustManager tm = caClient.getTrustManager(); X509Certificate cert1 = caClient.getRootCACertificate(); assertThat(tm.getAcceptedIssuers()).containsOnly(cert1); @@ -61,8 +59,7 @@ public void testReload() throws Exception { assertThat(reloaderLog.getOutput()) .contains("ReloadingX509TrustManager is reloaded"); - // Make sure there are two reload happened, one for server, one for client - assertEquals(2, StringUtils.countMatches(reloaderLog.getOutput(), - "ReloadingX509TrustManager is reloaded")); + // Only one reload has to happen for the CertificateClient's trustManager. + assertEquals(1, StringUtils.countMatches(reloaderLog.getOutput(), "ReloadingX509TrustManager is reloaded")); } } diff --git a/hadoop-hdds/framework/src/test/java/org/apache/hadoop/hdds/security/ssl/TestPemFileBasedKeyStoresFactory.java b/hadoop-hdds/framework/src/test/java/org/apache/hadoop/hdds/security/ssl/TestSSLConnectionWithReload.java similarity index 79% rename from hadoop-hdds/framework/src/test/java/org/apache/hadoop/hdds/security/ssl/TestPemFileBasedKeyStoresFactory.java rename to hadoop-hdds/framework/src/test/java/org/apache/hadoop/hdds/security/ssl/TestSSLConnectionWithReload.java index 6efb93c93d65..9e1014788609 100644 --- a/hadoop-hdds/framework/src/test/java/org/apache/hadoop/hdds/security/ssl/TestPemFileBasedKeyStoresFactory.java +++ b/hadoop-hdds/framework/src/test/java/org/apache/hadoop/hdds/security/ssl/TestSSLConnectionWithReload.java @@ -31,6 +31,7 @@ import org.apache.hadoop.hdds.scm.pipeline.PipelineID; import org.apache.hadoop.hdds.security.SecurityConfig; import org.apache.hadoop.hdds.security.x509.certificate.client.CertificateClientTestImpl; +import org.apache.hadoop.hdds.security.x509.exception.CertificateException; import org.apache.hadoop.ozone.container.ContainerTestHelper; import org.apache.ratis.thirdparty.io.grpc.ManagedChannel; import org.apache.ratis.thirdparty.io.grpc.Server; @@ -42,8 +43,6 @@ import org.apache.ratis.thirdparty.io.netty.handler.ssl.SslContextBuilder; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; -import org.junit.jupiter.params.ParameterizedTest; -import org.junit.jupiter.params.provider.ValueSource; import javax.net.ssl.SSLException; import java.util.ArrayList; @@ -52,13 +51,12 @@ import java.util.concurrent.CompletableFuture; import static org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos.Result.SUCCESS; -import static org.junit.jupiter.api.Assertions.assertInstanceOf; import static org.junit.jupiter.api.Assertions.assertEquals; /** - * Test PemFileBasedKeyStoresFactory. + * Test. */ -public class TestPemFileBasedKeyStoresFactory { +public class TestSSLConnectionWithReload { private CertificateClientTestImpl caClient; private SecurityConfig secConf; private static final int RELOAD_INTERVAL = 2000; @@ -70,29 +68,6 @@ public void setup() throws Exception { secConf = new SecurityConfig(conf); } - @ValueSource(booleans = {true, false}) - @ParameterizedTest - public void testInit(boolean clientAuth) throws Exception { - KeyStoresFactory keyStoresFactory = new PemFileBasedKeyStoresFactory( - caClient); - try { - keyStoresFactory.init(KeyStoresFactory.Mode.CLIENT, clientAuth); - assertEquals(clientAuth, keyStoresFactory.getKeyManagers()[0] - instanceof ReloadingX509KeyManager); - assertInstanceOf(ReloadingX509TrustManager.class, keyStoresFactory.getTrustManagers()[0]); - } finally { - keyStoresFactory.destroy(); - } - - try { - keyStoresFactory.init(KeyStoresFactory.Mode.SERVER, clientAuth); - assertInstanceOf(ReloadingX509KeyManager.class, keyStoresFactory.getKeyManagers()[0]); - assertInstanceOf(ReloadingX509TrustManager.class, keyStoresFactory.getTrustManagers()[0]); - } finally { - keyStoresFactory.destroy(); - } - } - @Test public void testConnectionWithCertReload() throws Exception { KeyStoresFactory serverFactory = null; @@ -101,15 +76,11 @@ public void testConnectionWithCertReload() throws Exception { ManagedChannel channel = null; try { // create server - serverFactory = new PemFileBasedKeyStoresFactory(caClient); - serverFactory.init(KeyStoresFactory.Mode.SERVER, true); - server = setupServer(serverFactory); + server = setupServer(); server.start(); // create client - clientFactory = new PemFileBasedKeyStoresFactory(caClient); - clientFactory.init(KeyStoresFactory.Mode.CLIENT, true); - channel = setupClient(clientFactory, server.getPort()); + channel = setupClient(server.getPort()); XceiverClientProtocolServiceStub asyncStub = XceiverClientProtocolServiceGrpc.newStub(channel); @@ -165,6 +136,7 @@ public void onNext(ContainerCommandResponseProto value) { @Override public void onError(Throwable t) { } + @Override public void onCompleted() { } @@ -174,25 +146,25 @@ public void onCompleted() { return replyFuture.get(); } - private ManagedChannel setupClient(KeyStoresFactory factory, int port) - throws SSLException { + private ManagedChannel setupClient(int port) + throws SSLException, CertificateException { NettyChannelBuilder channelBuilder = NettyChannelBuilder.forAddress("localhost", port); SslContextBuilder sslContextBuilder = GrpcSslContexts.forClient(); - sslContextBuilder.trustManager(factory.getTrustManagers()[0]); - sslContextBuilder.keyManager(factory.getKeyManagers()[0]); + sslContextBuilder.trustManager(caClient.getTrustManager()); + sslContextBuilder.keyManager(caClient.getKeyManager()); channelBuilder.useTransportSecurity().sslContext(sslContextBuilder.build()); return channelBuilder.build(); } - private Server setupServer(KeyStoresFactory factory) throws SSLException { + private Server setupServer() throws SSLException, CertificateException { NettyServerBuilder nettyServerBuilder = NettyServerBuilder.forPort(0) .addService(new GrpcService()); SslContextBuilder sslContextBuilder = SslContextBuilder.forServer( - factory.getKeyManagers()[0]); + caClient.getKeyManager()); sslContextBuilder.clientAuth(ClientAuth.REQUIRE); - sslContextBuilder.trustManager(factory.getTrustManagers()[0]); + sslContextBuilder.trustManager(caClient.getTrustManager()); sslContextBuilder = GrpcSslContexts.configure( sslContextBuilder, secConf.getGrpcSslProvider()); nettyServerBuilder.sslContext(sslContextBuilder.build()); 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 30bd9fc32006..00058500f597 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 @@ -18,8 +18,10 @@ import java.io.IOException; import java.math.BigInteger; +import java.security.GeneralSecurityException; import java.security.InvalidKeyException; import java.security.KeyPair; +import java.security.KeyStore; import java.security.NoSuchAlgorithmException; import java.security.NoSuchProviderException; import java.security.PrivateKey; @@ -46,8 +48,9 @@ import com.google.common.util.concurrent.ThreadFactoryBuilder; import org.apache.hadoop.hdds.conf.OzoneConfiguration; -import org.apache.hadoop.hdds.security.ssl.KeyStoresFactory; import org.apache.hadoop.hdds.security.SecurityConfig; +import org.apache.hadoop.hdds.security.ssl.ReloadingX509KeyManager; +import org.apache.hadoop.hdds.security.ssl.ReloadingX509TrustManager; import org.apache.hadoop.hdds.security.x509.certificate.authority.DefaultApprover; import org.apache.hadoop.hdds.security.x509.certificate.authority.profile.DefaultProfile; import org.apache.hadoop.hdds.security.x509.certificate.utils.CertificateSignRequest; @@ -55,8 +58,6 @@ import org.apache.hadoop.hdds.security.x509.exception.CertificateException; import org.apache.hadoop.hdds.security.x509.keys.HDDSKeyGenerator; -import org.apache.hadoop.hdds.security.x509.keys.SecurityUtil; - import static org.apache.hadoop.hdds.HddsConfigKeys.HDDS_X509_DEFAULT_DURATION; import static org.apache.hadoop.hdds.HddsConfigKeys.HDDS_X509_DEFAULT_DURATION_DEFAULT; import static org.apache.hadoop.hdds.HddsConfigKeys.HDDS_X509_MAX_DURATION; @@ -79,8 +80,8 @@ public class CertificateClientTestImpl implements CertificateClient { private HDDSKeyGenerator keyGen; private DefaultApprover approver; - private KeyStoresFactory serverKeyStoresFactory; - private KeyStoresFactory clientKeyStoresFactory; + private ReloadingX509KeyManager keyManager; + private ReloadingX509TrustManager trustManager; private Map certificateMap; private ScheduledExecutorService executorService; private Set notificationReceivers; @@ -135,20 +136,16 @@ public CertificateClientTestImpl(OzoneConfiguration conf, boolean autoRenew) String certDuration = conf.get(HDDS_X509_DEFAULT_DURATION, HDDS_X509_DEFAULT_DURATION_DEFAULT); x509Certificate = approver.sign(securityConfig, rootKeyPair.getPrivate(), - rootCert, - Date.from(start.atZone(ZoneId.systemDefault()).toInstant()), - Date.from(start.plus(Duration.parse(certDuration)) - .atZone(ZoneId.systemDefault()).toInstant()), - csrBuilder.build(), "scm1", "cluster1", - String.valueOf(System.nanoTime())); + rootCert, + Date.from(start.atZone(ZoneId.systemDefault()).toInstant()), + Date.from(start.plus(Duration.parse(certDuration)) + .atZone(ZoneId.systemDefault()).toInstant()), + csrBuilder.build(), "scm1", "cluster1", + String.valueOf(System.nanoTime())); certificateMap.put(x509Certificate.getSerialNumber().toString(), x509Certificate); notificationReceivers = new HashSet<>(); - serverKeyStoresFactory = SecurityUtil.getServerKeyStoresFactory( - this, true); - clientKeyStoresFactory = SecurityUtil.getClientKeyStoresFactory( - this, true); if (autoRenew) { Duration gracePeriod = securityConfig.getRenewalGracePeriod(); @@ -337,13 +334,31 @@ public void run() { } @Override - public KeyStoresFactory getServerKeyStoresFactory() { - return serverKeyStoresFactory; + public ReloadingX509KeyManager getKeyManager() throws CertificateException { + try { + if (keyManager == null) { + keyManager = new ReloadingX509KeyManager( + KeyStore.getDefaultType(), getComponentName(), getPrivateKey(), getTrustChain()); + notificationReceivers.add(keyManager); + } + return keyManager; + } catch (IOException | GeneralSecurityException e) { + throw new CertificateException("Failed to init keyManager", e, CertificateException.ErrorCode.KEYSTORE_ERROR); + } } @Override - public KeyStoresFactory getClientKeyStoresFactory() { - return clientKeyStoresFactory; + public ReloadingX509TrustManager getTrustManager() throws CertificateException { + try { + if (trustManager == null) { + Set newRootCaCerts = getAllRootCaCerts().isEmpty() ? getAllCaCerts() : getAllRootCaCerts(); + trustManager = new ReloadingX509TrustManager(KeyStore.getDefaultType(), new ArrayList<>(newRootCaCerts)); + notificationReceivers.add(trustManager); + } + return trustManager; + } catch (IOException | GeneralSecurityException e) { + throw new CertificateException("Failed to init trustManager", e); + } } @Override @@ -362,14 +377,6 @@ public void registerRootCARotationListener( @Override public void close() throws IOException { - if (serverKeyStoresFactory != null) { - serverKeyStoresFactory.destroy(); - } - - if (clientKeyStoresFactory != null) { - clientKeyStoresFactory.destroy(); - } - if (executorService != null) { executorService.shutdown(); } diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ha/HASecurityUtils.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ha/HASecurityUtils.java index 72f4c4dc870c..f0d78b23079a 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ha/HASecurityUtils.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ha/HASecurityUtils.java @@ -23,7 +23,6 @@ import org.apache.hadoop.hdds.scm.proxy.SCMSecurityProtocolFailoverProxyProvider; import org.apache.hadoop.hdds.scm.server.SCMStorageConfig; import org.apache.hadoop.hdds.security.SecurityConfig; -import org.apache.hadoop.hdds.security.ssl.KeyStoresFactory; import org.apache.hadoop.hdds.security.x509.certificate.authority.CAType; import org.apache.hadoop.hdds.security.x509.certificate.authority.CertificateServer; import org.apache.hadoop.hdds.security.x509.certificate.authority.CertificateStore; @@ -156,11 +155,8 @@ public static CertificateServer initializeRootCertificateServer( public static GrpcTlsConfig createSCMRatisTLSConfig(SecurityConfig conf, CertificateClient certificateClient) throws IOException { if (conf.isSecurityEnabled() && conf.isGrpcTlsEnabled()) { - KeyStoresFactory serverKeyFactory = - certificateClient.getServerKeyStoresFactory(); - - return new GrpcTlsConfig(serverKeyFactory.getKeyManagers()[0], - serverKeyFactory.getTrustManagers()[0], true); + return new GrpcTlsConfig(certificateClient.getKeyManager(), + certificateClient.getTrustManager(), true); } return null; } diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ha/InterSCMGrpcClient.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ha/InterSCMGrpcClient.java index bc5a58472e30..eebcfac03c7c 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ha/InterSCMGrpcClient.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ha/InterSCMGrpcClient.java @@ -24,7 +24,6 @@ import org.apache.hadoop.hdds.protocol.scm.proto.InterSCMProtocolServiceGrpc; import org.apache.hadoop.hdds.scm.ScmConfigKeys; import org.apache.hadoop.hdds.security.SecurityConfig; -import org.apache.hadoop.hdds.security.ssl.KeyStoresFactory; import org.apache.hadoop.hdds.security.x509.certificate.client.CertificateClient; import org.apache.hadoop.ozone.OzoneConsts; import org.apache.ratis.thirdparty.io.grpc.ManagedChannel; @@ -73,11 +72,8 @@ public InterSCMGrpcClient(final String host, if (securityConfig.isSecurityEnabled() && securityConfig.isGrpcTlsEnabled()) { SslContextBuilder sslClientContextBuilder = SslContextBuilder.forClient(); - KeyStoresFactory keyStoreFactory = - scmCertificateClient.getClientKeyStoresFactory(); - sslClientContextBuilder.keyManager(keyStoreFactory.getKeyManagers()[0]); - sslClientContextBuilder.trustManager( - keyStoreFactory.getTrustManagers()[0]); + sslClientContextBuilder.keyManager(scmCertificateClient.getKeyManager()); + sslClientContextBuilder.trustManager(scmCertificateClient.getTrustManager()); SslContextBuilder sslContextBuilder = GrpcSslContexts.configure( sslClientContextBuilder, securityConfig.getGrpcSslProvider()); channelBuilder.sslContext(sslContextBuilder.build()) diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ha/InterSCMGrpcProtocolService.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ha/InterSCMGrpcProtocolService.java index 92f28d07e973..4b45fd4d34f4 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ha/InterSCMGrpcProtocolService.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ha/InterSCMGrpcProtocolService.java @@ -26,7 +26,6 @@ import org.apache.hadoop.hdds.scm.ScmConfigKeys; import org.apache.hadoop.hdds.scm.server.StorageContainerManager; import org.apache.hadoop.hdds.security.SecurityConfig; -import org.apache.hadoop.hdds.security.ssl.KeyStoresFactory; import org.apache.hadoop.hdds.security.x509.certificate.client.CertificateClient; import org.apache.hadoop.ozone.OzoneConsts; import org.apache.ratis.thirdparty.io.grpc.Server; @@ -70,10 +69,9 @@ public class InterSCMGrpcProtocolService { && securityConfig.isGrpcTlsEnabled()) { try { CertificateClient certClient = scm.getScmCertificateClient(); - KeyStoresFactory keyStores = certClient.getServerKeyStoresFactory(); SslContextBuilder sslServerContextBuilder = - forServer(keyStores.getKeyManagers()[0]) - .trustManager(keyStores.getTrustManagers()[0]); + forServer(certClient.getKeyManager()) + .trustManager(certClient.getTrustManager()); SslContextBuilder sslContextBuilder = GrpcSslContexts.configure( sslServerContextBuilder, securityConfig.getGrpcSslProvider()); sslContextBuilder.clientAuth(ClientAuth.REQUIRE); diff --git a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/ha/TestInterSCMGrpcProtocolService.java b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/ha/TestInterSCMGrpcProtocolService.java index dfb3ff5179e0..8c9c643f689b 100644 --- a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/ha/TestInterSCMGrpcProtocolService.java +++ b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/ha/TestInterSCMGrpcProtocolService.java @@ -17,13 +17,15 @@ package org.apache.hadoop.hdds.scm.ha; +import com.google.common.collect.ImmutableList; import org.apache.commons.compress.archivers.tar.TarArchiveInputStream; import org.apache.hadoop.hdds.HddsConfigKeys; import org.apache.hadoop.hdds.conf.OzoneConfiguration; import org.apache.hadoop.hdds.scm.ScmConfigKeys; import org.apache.hadoop.hdds.scm.metadata.SCMMetadataStore; import org.apache.hadoop.hdds.scm.server.StorageContainerManager; -import org.apache.hadoop.hdds.security.ssl.KeyStoresFactory; +import org.apache.hadoop.hdds.security.ssl.ReloadingX509KeyManager; +import org.apache.hadoop.hdds.security.ssl.ReloadingX509TrustManager; import org.apache.hadoop.hdds.security.x509.CertificateTestUtils; import org.apache.hadoop.hdds.security.x509.certificate.client.SCMCertificateClient; import org.apache.hadoop.hdds.utils.TransactionInfo; @@ -36,11 +38,6 @@ import org.junit.jupiter.api.io.TempDir; import org.mockito.ArgumentCaptor; -import javax.net.ssl.KeyManager; -import javax.net.ssl.TrustManager; -import javax.net.ssl.X509KeyManager; -import javax.net.ssl.X509TrustManager; - import java.io.BufferedReader; import java.io.IOException; import java.io.InputStreamReader; @@ -48,6 +45,7 @@ import java.nio.file.Path; import java.nio.file.Paths; import java.security.KeyPair; +import java.security.KeyStore; import java.security.cert.CertificateException; import java.security.cert.X509Certificate; import java.util.concurrent.CompletableFuture; @@ -57,10 +55,10 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.Mockito.any; import static org.mockito.Mockito.anyBoolean; -import static org.mockito.Mockito.doNothing; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; +import static org.mockito.Mockito.spy; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -79,10 +77,10 @@ class TestInterSCMGrpcProtocolService { private X509Certificate serviceCert; private X509Certificate clientCert; - private X509KeyManager serverKeyManager; - private X509TrustManager serverTrustManager; - private X509KeyManager clientKeyManager; - private X509TrustManager clientTrustManager; + private ReloadingX509KeyManager serverKeyManager; + private ReloadingX509TrustManager serverTrustManager; + private ReloadingX509KeyManager clientKeyManager; + private ReloadingX509TrustManager clientTrustManager; @TempDir private Path temp; @@ -205,62 +203,25 @@ private SCMCertificateClient setupCertificateClientForMTLS( serviceCert = createSelfSignedCert(serviceKeys, "service"); clientCert = createSelfSignedCert(clientKeys, "client"); - serverKeyManager = aKeyManagerWith(serviceKeys, serviceCert); - serverTrustManager = aTrustManagerThatTrusts(clientCert); - KeyStoresFactory serverKeyStores = - aKeyStoresFactoryWith(serverKeyManager, serverTrustManager); - - clientKeyManager = aKeyManagerWith(clientKeys, clientCert); - clientTrustManager = aTrustManagerThatTrusts(serviceCert); - KeyStoresFactory clientKeyStores = - aKeyStoresFactoryWith(clientKeyManager, clientTrustManager); + ReloadingX509TrustManager toSpyServerTrustManager = new ReloadingX509TrustManager(KeyStore.getDefaultType(), + ImmutableList.of(clientCert)); + serverTrustManager = spy(toSpyServerTrustManager); + ReloadingX509TrustManager toSpyClientTrustManager = new ReloadingX509TrustManager(KeyStore.getDefaultType(), + ImmutableList.of(serviceCert)); + clientTrustManager = spy(toSpyClientTrustManager); + ReloadingX509KeyManager toSpyServerKeyManager = new ReloadingX509KeyManager(KeyStore.getDefaultType(), "server", + serviceKeys.getPrivate(), ImmutableList.of(serviceCert)); + ReloadingX509KeyManager toSpyClientKeyManager = new ReloadingX509KeyManager(KeyStore.getDefaultType(), "server", + clientKeys.getPrivate(), ImmutableList.of(clientCert)); + clientKeyManager = spy(toSpyClientKeyManager); + serverKeyManager = spy(toSpyServerKeyManager); SCMCertificateClient scmCertClient = mock(SCMCertificateClient.class); - doReturn(serverKeyStores).when(scmCertClient).getServerKeyStoresFactory(); - doReturn(clientKeyStores).when(scmCertClient).getClientKeyStoresFactory(); + doReturn(serverKeyManager, clientKeyManager).when(scmCertClient).getKeyManager(); + doReturn(serverTrustManager, clientTrustManager).when(scmCertClient).getTrustManager(); return scmCertClient; } - private KeyStoresFactory aKeyStoresFactoryWith( - X509KeyManager keyManager, - X509TrustManager trustManager - ) { - KeyStoresFactory serverKeyStores = mock(KeyStoresFactory.class); - doReturn(new KeyManager[]{keyManager}) - .when(serverKeyStores).getKeyManagers(); - doReturn(new TrustManager[]{trustManager}) - .when(serverKeyStores).getTrustManagers(); - return serverKeyStores; - } - - private X509TrustManager aTrustManagerThatTrusts(X509Certificate certificate) - throws CertificateException { - X509TrustManager trustManager = mock(X509TrustManager.class); - doNothing().when(trustManager).checkServerTrusted(any(), any()); - doNothing().when(trustManager).checkClientTrusted(any(), any()); - doReturn(new X509Certificate[] {certificate}) - .when(trustManager).getAcceptedIssuers(); - return trustManager; - } - - private X509KeyManager aKeyManagerWith(KeyPair keyPair, - X509Certificate certificate) { - X509KeyManager keyManager = mock(X509KeyManager.class); - doReturn("server") - .when(keyManager).chooseServerAlias(any(), any(), any()); - doReturn("client") - .when(keyManager).chooseClientAlias(any(), any(), any()); - doReturn(new String[] {"server"}) - .when(keyManager).getServerAliases(any(), any()); - doReturn(new String[] {"client"}) - .when(keyManager).getClientAliases(any(), any()); - doReturn(new X509Certificate[] {certificate}) - .when(keyManager).getCertificateChain(any()); - doReturn(keyPair.getPrivate()) - .when(keyManager).getPrivateKey(any()); - return keyManager; - } - private OzoneConfiguration setupConfiguration(int port) { OzoneConfiguration conf = new OzoneConfiguration(); conf.setInt(ScmConfigKeys.OZONE_SCM_GRPC_PORT_KEY, port); @@ -269,5 +230,4 @@ private OzoneConfiguration setupConfiguration(int port) { return conf; } - } diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/GrpcOzoneManagerServer.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/GrpcOzoneManagerServer.java index 80085db7c6f1..a83304ade459 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/GrpcOzoneManagerServer.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/GrpcOzoneManagerServer.java @@ -28,7 +28,6 @@ import org.apache.hadoop.hdds.HddsUtils; import org.apache.hadoop.hdds.conf.OzoneConfiguration; import org.apache.hadoop.hdds.security.SecurityConfig; -import org.apache.hadoop.hdds.security.ssl.KeyStoresFactory; import org.apache.hadoop.hdds.security.x509.certificate.client.CertificateClient; import org.apache.hadoop.ozone.grpc.metrics.GrpcMetricsServerRequestInterceptor; import org.apache.hadoop.ozone.grpc.metrics.GrpcMetricsServerResponseInterceptor; @@ -163,9 +162,8 @@ public void init(OzoneManagerProtocolServerSideTranslatorPB omTranslator, SecurityConfig secConf = new SecurityConfig(omServerConfig); if (secConf.isSecurityEnabled() && secConf.isGrpcTlsEnabled()) { try { - KeyStoresFactory factory = caClient.getServerKeyStoresFactory(); SslContextBuilder sslClientContextBuilder = - SslContextBuilder.forServer(factory.getKeyManagers()[0]); + SslContextBuilder.forServer(caClient.getKeyManager()); SslContextBuilder sslContextBuilder = GrpcSslContexts.configure( sslClientContextBuilder, SslProvider.valueOf(omServerConfig.get(HDDS_GRPC_TLS_PROVIDER, diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ratis/utils/OzoneManagerRatisUtils.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ratis/utils/OzoneManagerRatisUtils.java index b055a1f92f82..8ff59e091d88 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ratis/utils/OzoneManagerRatisUtils.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ratis/utils/OzoneManagerRatisUtils.java @@ -27,7 +27,6 @@ import org.apache.hadoop.hdds.conf.ConfigurationSource; import org.apache.hadoop.hdds.conf.OzoneConfiguration; import org.apache.hadoop.hdds.security.SecurityConfig; -import org.apache.hadoop.hdds.security.ssl.KeyStoresFactory; import org.apache.hadoop.hdds.security.x509.certificate.client.CertificateClient; import org.apache.hadoop.hdds.server.ServerUtils; import org.apache.hadoop.hdds.utils.HAUtils; @@ -495,9 +494,7 @@ public static void checkLeaderStatus(OzoneManager ozoneManager) public static GrpcTlsConfig createServerTlsConfig(SecurityConfig conf, CertificateClient caClient) throws IOException { if (conf.isSecurityEnabled() && conf.isGrpcTlsEnabled()) { - KeyStoresFactory serverKeyFactory = caClient.getServerKeyStoresFactory(); - return new GrpcTlsConfig(serverKeyFactory.getKeyManagers()[0], - serverKeyFactory.getTrustManagers()[0], true); + return new GrpcTlsConfig(caClient.getKeyManager(), caClient.getTrustManager(), true); } return null;