You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ozone.apache.org by pi...@apache.org on 2023/02/27 10:26:36 UTC

[ozone] branch master updated: HDDS-7590. Use keyManager and trustManager provided by keyStoreFactory in om grpc services (#4145)

This is an automated email from the ASF dual-hosted git repository.

pifta pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/ozone.git


The following commit(s) were added to refs/heads/master by this push:
     new ce57071a5f HDDS-7590. Use keyManager and trustManager provided by keyStoreFactory in om grpc services (#4145)
ce57071a5f is described below

commit ce57071a5f1e61103dac3e0b14e73d82c27048b8
Author: Sammi Chen <sa...@apache.org>
AuthorDate: Mon Feb 27 18:26:27 2023 +0800

    HDDS-7590. Use keyManager and trustManager provided by keyStoreFactory in om grpc services (#4145)
---
 .../hdds/security/ssl/ReloadingX509KeyManager.java |   2 +-
 .../certificate/authority/DefaultCAServer.java     |  27 +--
 .../client/DefaultCertificateClient.java           |  22 ++-
 .../certificate/client/SCMCertificateClient.java   |   8 +-
 .../apache/hadoop/hdds/utils/HddsServerUtil.java   |   6 +-
 .../hdds/scm/server/SCMSecurityProtocolServer.java |   4 +-
 .../hdds/scm/server/StorageContainerManager.java   |   4 +-
 .../hadoop/ozone/om/exceptions/OMException.java    |   2 +-
 .../ozone/om/protocolPB/GrpcOmTransport.java       |  26 +--
 .../hadoop/ozone/TestSecureOzoneCluster.java       | 194 ++++++++++++++++++++-
 .../ozone/client/CertificateClientTestImpl.java    |   2 +-
 .../src/main/proto/OmClientProtocol.proto          |   2 +-
 .../hadoop/ozone/om/GrpcOzoneManagerServer.java    |  24 ++-
 .../org/apache/hadoop/ozone/om/OzoneManager.java   |  10 +-
 .../hadoop/ozone/om/OzoneManagerServiceGrpc.java   |   9 +-
 15 files changed, 273 insertions(+), 69 deletions(-)

diff --git a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/security/ssl/ReloadingX509KeyManager.java b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/security/ssl/ReloadingX509KeyManager.java
index ee2805cb1c..89a0ab9f14 100644
--- a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/security/ssl/ReloadingX509KeyManager.java
+++ b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/security/ssl/ReloadingX509KeyManager.java
@@ -49,7 +49,7 @@ import java.util.concurrent.atomic.AtomicReference;
 public class ReloadingX509KeyManager extends X509ExtendedKeyManager {
 
   public static final Logger LOG =
-      LoggerFactory.getLogger(ReloadingX509TrustManager.class);
+      LoggerFactory.getLogger(ReloadingX509KeyManager.class);
 
   static final String RELOAD_ERROR_MESSAGE =
       "Could not reload keystore (keep using existing one) : ";
diff --git a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/security/x509/certificate/authority/DefaultCAServer.java b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/security/x509/certificate/authority/DefaultCAServer.java
index 6fa860cdac..a07d86412a 100644
--- a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/security/x509/certificate/authority/DefaultCAServer.java
+++ b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/security/x509/certificate/authority/DefaultCAServer.java
@@ -58,6 +58,7 @@ import java.security.spec.InvalidKeySpecException;
 import java.time.LocalDate;
 import java.time.LocalDateTime;
 import java.time.LocalTime;
+import java.time.ZoneId;
 import java.util.Date;
 import java.util.List;
 import java.util.Optional;
@@ -136,6 +137,7 @@ public class DefaultCAServer implements CertificateServer {
   private CRLApprover crlApprover;
   private CertificateStore store;
   private Lock lock;
+  private static boolean testSecureFlag;
 
   /**
    * Create an Instance of DefaultCAServer.
@@ -224,16 +226,14 @@ public class DefaultCAServer implements CertificateServer {
   public Future<CertPath> requestCertificate(
       PKCS10CertificationRequest csr,
       CertificateApprover.ApprovalType approverType, NodeType role) {
-    LocalDate beginDate = LocalDate.now().atStartOfDay().toLocalDate();
-    LocalDateTime temp = LocalDateTime.of(beginDate, LocalTime.MIDNIGHT);
-
-    LocalDate endDate;
+    LocalDateTime beginDate = LocalDateTime.now();
+    LocalDateTime endDate;
     // When issuing certificates for sub-ca use the max certificate duration
     // similar to self signed root certificate.
     if (role == NodeType.SCM) {
-      endDate = temp.plus(config.getMaxCertificateDuration()).toLocalDate();
+      endDate = beginDate.plus(config.getMaxCertificateDuration());
     } else {
-      endDate = temp.plus(config.getDefaultCertDuration()).toLocalDate();
+      endDate = beginDate.plus(config.getDefaultCertDuration());
     }
 
     CompletableFuture<X509CertificateHolder> xcertHolder =
@@ -283,8 +283,8 @@ public class DefaultCAServer implements CertificateServer {
     return xCertHolders;
   }
 
-  private X509CertificateHolder signAndStoreCertificate(LocalDate beginDate,
-      LocalDate endDate, PKCS10CertificationRequest csr, NodeType role)
+  private X509CertificateHolder signAndStoreCertificate(LocalDateTime beginDate,
+      LocalDateTime endDate, PKCS10CertificationRequest csr, NodeType role)
       throws IOException,
       OperatorCreationException, CertificateException, TimeoutException {
 
@@ -293,8 +293,10 @@ public class DefaultCAServer implements CertificateServer {
     try {
       xcert = approver.sign(config,
           getCAKeys().getPrivate(),
-          getCACertificate(), java.sql.Date.valueOf(beginDate),
-          java.sql.Date.valueOf(endDate), csr, scmID, clusterID);
+          getCACertificate(),
+          Date.from(beginDate.atZone(ZoneId.systemDefault()).toInstant()),
+          Date.from(endDate.atZone(ZoneId.systemDefault()).toInstant()),
+          csr, scmID, clusterID);
       if (store != null) {
         store.checkValidCertID(xcert.getSerialNumber());
         store.storeValidCertificate(xcert.getSerialNumber(),
@@ -671,4 +673,9 @@ public class DefaultCAServer implements CertificateServer {
     MISSING_CERTIFICATE, /* Keys exist, but root certificate missing.*/
     INITIALIZE /* All artifacts are missing, we should init the system. */
   }
+
+  @VisibleForTesting
+  public static void setTestSecureFlag(boolean flag) {
+    testSecureFlag = flag;
+  }
 }
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 b680b2023c..b0b979f586 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
@@ -99,6 +99,7 @@ import static org.apache.hadoop.hdds.security.x509.exception.CertificateExceptio
 import static org.apache.hadoop.hdds.security.x509.exception.CertificateException.ErrorCode.ROLLBACK_ERROR;
 import static org.apache.hadoop.hdds.utils.HddsServerUtil.getScmSecurityClientWithMaxRetry;
 
+import org.apache.hadoop.security.UserGroupInformation;
 import org.bouncycastle.pkcs.PKCS10CertificationRequest;
 import org.slf4j.Logger;
 
@@ -111,9 +112,7 @@ public abstract class DefaultCertificateClient implements CertificateClient {
 
   private static final Random RANDOM = new SecureRandom();
 
-  private static final String CERT_FILE_NAME_FORMAT = "%s.crt";
-  private static final int CA_CERT_PREFIX_LEN = 3;
-  private static final int ROOT_CA_PREFIX_LEN = 7;
+  public static final String CERT_FILE_NAME_FORMAT = "%s.crt";
   private final Logger logger;
   private final SecurityConfig securityConfig;
   private final KeyCodec keyCodec;
@@ -144,6 +143,7 @@ public abstract class DefaultCertificateClient implements CertificateClient {
   private Runnable shutdownCallback;
   private SCMSecurityProtocolClientSideTranslatorPB scmSecurityProtocolClient;
   private Set<CertificateNotification> notificationReceivers;
+  private static UserGroupInformation ugi;
 
   DefaultCertificateClient(SecurityConfig securityConfig, Logger log,
       String certSerialId, String component,
@@ -203,7 +203,8 @@ public abstract class DefaultCertificateClient implements CertificateClient {
                   String certFileName = FilenameUtils.getBaseName(
                       file.getName());
                   long tmpCaCertSerailId = NumberUtils.toLong(
-                      certFileName.substring(CA_CERT_PREFIX_LEN));
+                      certFileName.substring(
+                          CAType.SUBORDINATE.getFileNamePrefix().length()));
                   if (tmpCaCertSerailId > latestCaCertSerailId) {
                     latestCaCertSerailId = tmpCaCertSerailId;
                   }
@@ -214,7 +215,8 @@ public abstract class DefaultCertificateClient implements CertificateClient {
                   String certFileName = FilenameUtils.getBaseName(
                       file.getName());
                   long tmpRootCaCertSerailId = NumberUtils.toLong(
-                      certFileName.substring(ROOT_CA_PREFIX_LEN));
+                      certFileName.substring(
+                          CAType.ROOT.getFileNamePrefix().length()));
                   if (tmpRootCaCertSerailId > latestRootCaCertSerialId) {
                     latestRootCaCertSerialId = tmpRootCaCertSerailId;
                   }
@@ -1397,7 +1399,7 @@ public abstract class DefaultCertificateClient implements CertificateClient {
     if (scmSecurityProtocolClient == null) {
       scmSecurityProtocolClient =
           getScmSecurityClientWithMaxRetry(
-              (OzoneConfiguration) securityConfig.getConfiguration());
+              (OzoneConfiguration) securityConfig.getConfiguration(), ugi);
     }
     return scmSecurityProtocolClient;
   }
@@ -1408,6 +1410,11 @@ public abstract class DefaultCertificateClient implements CertificateClient {
     scmSecurityProtocolClient = client;
   }
 
+  @VisibleForTesting
+  public static void setUgi(UserGroupInformation user) {
+    ugi = user;
+  }
+
   public synchronized void startCertificateMonitor() {
     Preconditions.checkNotNull(getCertificate(),
         "Component certificate should not be empty");
@@ -1421,7 +1428,8 @@ public abstract class DefaultCertificateClient implements CertificateClient {
 
     if (executorService == null) {
       executorService = Executors.newScheduledThreadPool(1,
-          new ThreadFactoryBuilder().setNameFormat("CertificateLifetimeMonitor")
+          new ThreadFactoryBuilder().setNameFormat(
+              getComponentName() + "-CertificateLifetimeMonitor")
               .setDaemon(true).build());
     }
     this.executorService.scheduleAtFixedRate(new CertificateLifetimeMonitor(),
diff --git a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/security/x509/certificate/client/SCMCertificateClient.java b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/security/x509/certificate/client/SCMCertificateClient.java
index 655bbba3db..3b6b520ee1 100644
--- a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/security/x509/certificate/client/SCMCertificateClient.java
+++ b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/security/x509/certificate/client/SCMCertificateClient.java
@@ -146,13 +146,15 @@ public class SCMCertificateClient extends DefaultCertificateClient {
 
   @Override
   public String signAndStoreCertificate(PKCS10CertificationRequest request,
-      Path certificatePath) throws CertificateException {
-    return null;
+      Path certPath) throws CertificateException {
+    throw new UnsupportedOperationException("signAndStoreCertificate of " +
+        " SCMCertificateClient is not supported currently");
   }
 
   @Override
   public CertificateSignRequest.Builder getCSRBuilder(KeyPair keyPair)
       throws CertificateException {
-    return null;
+    throw new UnsupportedOperationException("getCSRBuilder of " +
+        "SCMCertificateClient is not supported currently");
   }
 }
\ No newline at end of file
diff --git a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/HddsServerUtil.java b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/HddsServerUtil.java
index ca11919284..33d8c178c7 100644
--- a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/HddsServerUtil.java
+++ b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/HddsServerUtil.java
@@ -447,8 +447,8 @@ public final class HddsServerUtil {
   }
 
   public static SCMSecurityProtocolClientSideTranslatorPB
-      getScmSecurityClientWithMaxRetry(OzoneConfiguration conf)
-      throws IOException {
+      getScmSecurityClientWithMaxRetry(OzoneConfiguration conf,
+      UserGroupInformation ugi) throws IOException {
     // Certificate from SCM is required for DN startup to succeed, so retry
     // for ever. In this way DN start up is resilient to SCM service running
     // status.
@@ -461,7 +461,7 @@ public final class HddsServerUtil {
 
     return new SCMSecurityProtocolClientSideTranslatorPB(
         new SCMSecurityProtocolFailoverProxyProvider(configuration,
-            UserGroupInformation.getCurrentUser()));
+            ugi == null ? UserGroupInformation.getCurrentUser() : ugi));
   }
 
   public static SCMSecurityProtocolClientSideTranslatorPB
diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/SCMSecurityProtocolServer.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/SCMSecurityProtocolServer.java
index f79cd86310..f394daa82e 100644
--- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/SCMSecurityProtocolServer.java
+++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/SCMSecurityProtocolServer.java
@@ -421,9 +421,9 @@ public class SCMSecurityProtocolServer implements SCMSecurityProtocol {
   }
 
   public void join() throws InterruptedException {
-    LOGGER.trace("Join RPC server for SCMSecurityProtocolServer.");
+    LOGGER.info("Join RPC server for SCMSecurityProtocolServer.");
     getRpcServer().join();
-    LOGGER.trace("Join gRPC server for SCMSecurityProtocolServer.");
+    LOGGER.info("Join gRPC server for SCMSecurityProtocolServer.");
     getGrpcUpdateServer().join();
 
   }
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 5530e1cbf2..7a96a6f088 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
@@ -1572,7 +1572,9 @@ public final class StorageContainerManager extends ServiceRuntimeInfoImpl
 
     try {
       LOG.info("Stopping Storage Container Manager HTTP server.");
-      httpServer.stop();
+      if (httpServer != null) {
+        httpServer.stop();
+      }
     } catch (Exception ex) {
       LOG.error("Storage Container Manager HTTP server stop failed.", ex);
     }
diff --git a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/exceptions/OMException.java b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/exceptions/OMException.java
index fb920583b8..62a1f717a1 100644
--- a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/exceptions/OMException.java
+++ b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/exceptions/OMException.java
@@ -260,7 +260,7 @@ public class OMException extends IOException {
     FEATURE_NOT_ENABLED,
 
     INVALID_SNAPSHOT_ERROR,
-
     CONTAINS_SNAPSHOT,
+    SSL_CONNECTION_FAILURE
   }
 }
diff --git a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/protocolPB/GrpcOmTransport.java b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/protocolPB/GrpcOmTransport.java
index b899efcd9d..2bad2d32b3 100644
--- a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/protocolPB/GrpcOmTransport.java
+++ b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/protocolPB/GrpcOmTransport.java
@@ -62,12 +62,13 @@ import static org.apache.hadoop.ozone.om.OMConfigKeys
     .OZONE_OM_GRPC_MAXIMUM_RESPONSE_LENGTH;
 import static org.apache.hadoop.ozone.om.OMConfigKeys
     .OZONE_OM_GRPC_MAXIMUM_RESPONSE_LENGTH_DEFAULT;
+import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.SSL_CONNECTION_FAILURE;
 
 /**
  * Grpc transport for grpc between s3g and om.
  */
 public class GrpcOmTransport implements OmTransport {
-  private static final Logger LOG =
+  public static final Logger LOG =
       LoggerFactory.getLogger(GrpcOmTransport.class);
 
   private static final String CLIENT_NAME = "GrpcOmTransport";
@@ -142,21 +143,16 @@ public class GrpcOmTransport implements OmTransport {
               .usePlaintext()
               .maxInboundMessageSize(maxSize);
 
-      if (secConfig.isGrpcTlsEnabled()) {
+      if (secConfig.isSecurityEnabled() && secConfig.isGrpcTlsEnabled()) {
         try {
           SslContextBuilder sslContextBuilder = GrpcSslContexts.forClient();
-          if (secConfig.isSecurityEnabled()) {
-            if (caCerts != null) {
-              sslContextBuilder.trustManager(caCerts);
-            } else {
-              LOG.error("x509Certicates empty");
-            }
-            channelBuilder.useTransportSecurity().
-                sslContext(sslContextBuilder.build());
+          if (caCerts != null) {
+            sslContextBuilder.trustManager(caCerts);
           } else {
-            LOG.error("ozone.security not enabled when TLS specified," +
-                " using plaintext");
+            LOG.error("x509Certificates empty");
           }
+          channelBuilder.useTransportSecurity().
+              sslContext(sslContextBuilder.build());
         } catch (Exception ex) {
           LOG.error("cannot establish TLS for grpc om transport client");
         }
@@ -189,7 +185,12 @@ public class GrpcOmTransport implements OmTransport {
       try {
         resp = clients.get(host.get()).submitRequest(payload);
       } catch (StatusRuntimeException e) {
+        LOG.error("Failed to submit request", e);
         if (e.getStatus().getCode() == Status.Code.UNAVAILABLE) {
+          if (e.getCause() != null &&
+              e.getCause() instanceof javax.net.ssl.SSLHandshakeException) {
+            throw new OMException(SSL_CONNECTION_FAILURE);
+          }
           resultCode = ResultCodes.TIMEOUT;
         }
         Exception exp = new Exception(e);
@@ -302,6 +303,7 @@ public class GrpcOmTransport implements OmTransport {
             entry.getKey(), e);
       }
     }
+    LOG.info("{}: stopped", CLIENT_NAME);
   }
 
   @Override
diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestSecureOzoneCluster.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestSecureOzoneCluster.java
index f3befc8da2..8f7b82b140 100644
--- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestSecureOzoneCluster.java
+++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestSecureOzoneCluster.java
@@ -31,12 +31,16 @@ import java.security.cert.X509Certificate;
 import java.time.Duration;
 import java.time.LocalDate;
 import java.time.LocalDateTime;
+import java.time.ZoneId;
 import java.time.temporal.ChronoUnit;
+import java.util.ArrayList;
 import java.util.Date;
+import java.util.List;
 import java.util.Properties;
 import java.util.UUID;
 import java.util.concurrent.Callable;
 
+import org.apache.hadoop.hdds.HddsConfigKeys;
 import org.apache.hadoop.hdds.annotation.InterfaceAudience;
 import org.apache.hadoop.hdds.conf.DefaultConfigManager;
 import org.apache.hadoop.hdds.conf.OzoneConfiguration;
@@ -62,8 +66,15 @@ import org.apache.hadoop.hdds.scm.server.StorageContainerManager;
 import org.apache.hadoop.hdds.security.exception.SCMSecurityException;
 import org.apache.hadoop.hdds.security.token.ContainerTokenIdentifier;
 import org.apache.hadoop.hdds.security.x509.SecurityConfig;
+import org.apache.hadoop.hdds.security.x509.certificate.authority.CAType;
+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.client.CertificateClient;
 import org.apache.hadoop.hdds.security.x509.certificate.client.DNCertificateClient;
+import org.apache.hadoop.hdds.security.x509.certificate.authority.DefaultCAServer;
+import org.apache.hadoop.hdds.security.x509.certificate.client.DefaultCertificateClient;
 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.certificate.utils.SelfSignedCertificate;
 import org.apache.hadoop.hdds.security.x509.exception.CertificateException;
 import org.apache.hadoop.hdds.security.x509.keys.HDDSKeyGenerator;
@@ -77,6 +88,8 @@ import org.apache.hadoop.metrics2.lib.DefaultMetricsSystem;
 import org.apache.hadoop.minikdc.MiniKdc;
 import org.apache.hadoop.net.NetUtils;
 import org.apache.hadoop.ozone.client.CertificateClientTestImpl;
+import org.apache.hadoop.ozone.client.OzoneClient;
+import org.apache.hadoop.ozone.client.OzoneClientFactory;
 import org.apache.hadoop.ozone.common.Storage;
 import org.apache.hadoop.ozone.om.OMConfigKeys;
 import org.apache.hadoop.ozone.om.OMStorage;
@@ -84,6 +97,9 @@ import org.apache.hadoop.ozone.om.OzoneManager;
 import org.apache.hadoop.ozone.om.exceptions.OMException;
 import org.apache.hadoop.ozone.om.helpers.OmVolumeArgs;
 import org.apache.hadoop.ozone.om.helpers.S3SecretValue;
+import org.apache.hadoop.ozone.om.helpers.ServiceInfoEx;
+import org.apache.hadoop.ozone.om.protocolPB.GrpcOmTransport;
+import org.apache.hadoop.ozone.om.protocolPB.GrpcOmTransportFactory;
 import org.apache.hadoop.ozone.om.protocolPB.OmTransportFactory;
 import org.apache.hadoop.ozone.om.protocolPB.OzoneManagerProtocolClientSideTranslatorPB;
 import org.apache.hadoop.ozone.security.OMCertificateClient;
@@ -106,7 +122,11 @@ import static org.apache.hadoop.hdds.HddsConfigKeys.HDDS_BLOCK_TOKEN_EXPIRY_TIME
 import static org.apache.hadoop.hdds.HddsConfigKeys.HDDS_CONTAINER_TOKEN_ENABLED;
 import static org.apache.hadoop.hdds.HddsConfigKeys.HDDS_X509_DEFAULT_DURATION;
 import static org.apache.hadoop.hdds.HddsConfigKeys.HDDS_X509_MAX_DURATION;
+import static org.apache.hadoop.hdds.HddsConfigKeys.HDDS_X509_DEFAULT_DURATION_DEFAULT;
 import static org.apache.hadoop.hdds.HddsConfigKeys.HDDS_X509_RENEW_GRACE_DURATION;
+import static org.apache.hadoop.hdds.HddsConfigKeys.HDDS_GRPC_TLS_ENABLED;
+import static org.apache.hadoop.hdds.HddsConfigKeys.HDDS_SECURITY_SSL_KEYSTORE_RELOAD_INTERVAL;
+import static org.apache.hadoop.hdds.HddsConfigKeys.HDDS_SECURITY_SSL_TRUSTSTORE_RELOAD_INTERVAL;
 import static org.apache.hadoop.hdds.HddsConfigKeys.OZONE_METADATA_DIRS;
 import static org.apache.hadoop.hdds.scm.ScmConfig.ConfigStrings.HDDS_SCM_KERBEROS_KEYTAB_FILE_KEY;
 import static org.apache.hadoop.hdds.scm.ScmConfig.ConfigStrings.HDDS_SCM_KERBEROS_PRINCIPAL_KEY;
@@ -125,6 +145,7 @@ import static org.apache.hadoop.hdds.scm.server.SCMHTTPServerConfig.ConfigString
 import static org.apache.hadoop.hdds.scm.server.SCMHTTPServerConfig.ConfigStrings.HDDS_SCM_HTTP_KERBEROS_PRINCIPAL_KEY;
 import static org.apache.hadoop.net.ServerSocketUtil.getPort;
 import static org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_ADMINISTRATORS;
+import static org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_CLIENT_FAILOVER_MAX_ATTEMPTS_KEY;
 import static org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_SECURITY_ENABLED_KEY;
 import static org.apache.hadoop.ozone.om.OMConfigKeys.DELEGATION_TOKEN_MAX_LIFETIME_KEY;
 import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_HTTP_KERBEROS_KEYTAB_FILE;
@@ -132,6 +153,8 @@ import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_HTTP_KERBEROS_PRI
 import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_KERBEROS_KEYTAB_FILE_KEY;
 import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_KERBEROS_PRINCIPAL_KEY;
 import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_ADDRESS_KEY;
+import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_S3_GPRC_SERVER_ENABLED;
+import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_TRANSPORT_CLASS;
 import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.TOKEN_EXPIRED;
 import static org.apache.hadoop.security.UserGroupInformation.AuthenticationMethod.KERBEROS;
 import org.apache.ratis.protocol.ClientId;
@@ -220,7 +243,8 @@ public final class TestSecureOzoneCluster {
       // use the same base ports as MiniOzoneHACluster
       conf.setInt(OZONE_SCM_RATIS_PORT_KEY, getPort(1200, 100));
       conf.setInt(OZONE_SCM_GRPC_PORT_KEY, getPort(1201, 100));
-      conf.set(OZONE_OM_ADDRESS_KEY, "localhost:1202");
+      conf.set(OZONE_OM_ADDRESS_KEY,
+          InetAddress.getLocalHost().getCanonicalHostName() + ":1202");
       conf.setBoolean(ScmConfigKeys.OZONE_SCM_HA_ENABLE_KEY, false);
 
       DefaultMetricsSystem.setMiniClusterMode(true);
@@ -256,11 +280,12 @@ public final class TestSecureOzoneCluster {
       stopMiniKdc();
       if (scm != null) {
         scm.stop();
+        scm.join();
       }
       if (om != null) {
         om.stop();
+        om.join();
       }
-      IOUtils.closeQuietly(om);
       IOUtils.closeQuietly(omClient);
     } catch (Exception e) {
       LOG.error("Failed to stop TestSecureOzoneCluster", e);
@@ -421,6 +446,8 @@ public final class TestSecureOzoneCluster {
     Path scmPath = Paths.get(path, "scm-meta");
     Files.createDirectories(scmPath);
     conf.set(OZONE_METADATA_DIRS, scmPath.toString());
+
+    DefaultCAServer.setTestSecureFlag(true);
     SCMStorageConfig scmStore = new SCMStorageConfig(conf);
     scmStore.setClusterId(clusterId);
     scmStore.setScmId(scmId);
@@ -647,8 +674,7 @@ public final class TestSecureOzoneCluster {
       omLogs.clearOutput();
 
     } finally {
-      om.stop();
-      om.join();
+      IOUtils.closeQuietly(om);
     }
   }
 
@@ -797,6 +823,8 @@ public final class TestSecureOzoneCluster {
           "SCM signed certificate"));
 
       conf.setBoolean(OZONE_SECURITY_ENABLED_KEY, true);
+      conf.setBoolean(OZONE_OM_S3_GPRC_SERVER_ENABLED, true);
+
       OzoneManager.omInit(conf);
       om.stop();
       om = OzoneManager.createOm(conf);
@@ -1153,9 +1181,6 @@ public final class TestSecureOzoneCluster {
       assertTrue(expiryTime > 0);
       assertTrue(new Date(expiryTime).before(omCert.getNotAfter()));
     } finally {
-      if (om != null) {
-        om.stop();
-      }
       IOUtils.closeQuietly(om);
     }
   }
@@ -1232,6 +1257,128 @@ public final class TestSecureOzoneCluster {
     }
   }
 
+  /**
+   * Test functionality to get SCM signed certificate for OM.
+   */
+  @Test
+  public void testOMGrpcServerCertificateRenew() throws Exception {
+    initSCM();
+    try {
+      scm = HddsTestUtils.getScmSimple(conf);
+      scm.start();
+
+      conf.set(OZONE_METADATA_DIRS, omMetaDirPath.toString());
+      int certLifetime = 30; // second
+      conf.set(HDDS_X509_DEFAULT_DURATION,
+          Duration.ofSeconds(certLifetime).toString());
+      conf.set(HDDS_SECURITY_SSL_KEYSTORE_RELOAD_INTERVAL, "1s");
+      conf.set(HDDS_SECURITY_SSL_TRUSTSTORE_RELOAD_INTERVAL, "1s");
+      conf.setInt(OZONE_CLIENT_FAILOVER_MAX_ATTEMPTS_KEY, 2);
+
+      // initialize OmStorage, save om Cert and CA Certs to disk
+      OMStorage omStore = new OMStorage(conf);
+      omStore.setClusterId(clusterId);
+      omStore.setOmId(omId);
+
+      // Prepare the certificates for OM before OM start
+      SecurityConfig securityConfig = new SecurityConfig(conf);
+      CertificateClient scmCertClient = scm.getScmCertificateClient();
+      CertificateCodec certCodec = new CertificateCodec(securityConfig, "om");
+      X509Certificate scmCert = scmCertClient.getCertificate();
+      X509Certificate rootCert = scmCertClient.getCACertificate();
+      X509CertificateHolder certHolder = generateX509CertHolder(conf, keyPair,
+          new KeyPair(scmCertClient.getPublicKey(),
+              scmCertClient.getPrivateKey()), scmCert,
+          Duration.ofSeconds(certLifetime),
+          InetAddress.getLocalHost().getCanonicalHostName(), clusterId);
+      String certId = certHolder.getSerialNumber().toString();
+      certCodec.writeCertificate(certHolder);
+      certCodec.writeCertificate(CertificateCodec.getCertificateHolder(scmCert),
+          String.format(DefaultCertificateClient.CERT_FILE_NAME_FORMAT,
+          CAType.SUBORDINATE.getFileNamePrefix() +
+              scmCert.getSerialNumber().toString()));
+      certCodec.writeCertificate(CertificateCodec.getCertificateHolder(
+          scmCertClient.getCACertificate()),
+          String.format(DefaultCertificateClient.CERT_FILE_NAME_FORMAT,
+              CAType.ROOT.getFileNamePrefix() +
+                  rootCert.getSerialNumber().toString()));
+      omStore.setOmCertSerialId(certId);
+      omStore.initialize();
+
+      conf.setBoolean(HDDS_GRPC_TLS_ENABLED, true);
+      conf.setBoolean(OZONE_OM_S3_GPRC_SERVER_ENABLED, true);
+      conf.setBoolean(HddsConfigKeys.HDDS_GRPC_TLS_TEST_CERT, true);
+      OzoneManager.setTestSecureOmFlag(true);
+      UserGroupInformation ugi = UserGroupInformation.getCurrentUser();
+      // In this process, SCM has already login using Kerberos. So pass
+      // specific UGI to DefaultCertificateClient and OzoneManager to avoid
+      // conflict with SCM procedure.
+      DefaultCertificateClient.setUgi(ugi);
+      OzoneManager.setUgi(ugi);
+      om = OzoneManager.createOm(conf);
+      om.start();
+
+      CertificateClient omCertClient = om.getCertificateClient();
+      X509Certificate omCert = omCertClient.getCertificate();
+      X509Certificate caCert = omCertClient.getCACertificate();
+      X509Certificate rootCaCert = omCertClient.getRootCACertificate();
+      List certList = new ArrayList<>();
+      certList.add(caCert);
+      certList.add(rootCaCert);
+      // set certificates in GrpcOmTransport
+      GrpcOmTransport.setCaCerts(certList);
+
+      GenericTestUtils.waitFor(() -> om.isLeaderReady(), 500, 10000);
+      String transportCls = GrpcOmTransportFactory.class.getName();
+      conf.set(OZONE_OM_TRANSPORT_CLASS, transportCls);
+      OzoneClient client = OzoneClientFactory.getRpcClient(conf);
+
+      ServiceInfoEx serviceInfoEx = client.getObjectStore()
+          .getClientProxy().getOzoneManagerClient().getServiceInfo();
+      Assert.assertTrue(serviceInfoEx.getCaCertificate().equals(
+          CertificateCodec.getPEMEncodedString(caCert)));
+
+      // Wait for OM certificate to renewed
+      GenericTestUtils.waitFor(() ->
+              !omCert.getSerialNumber().toString().equals(
+                  omCertClient.getCertificate().getSerialNumber().toString()),
+          500, certLifetime * 1000);
+
+      // rerun the command using old client, it should succeed
+      serviceInfoEx = client.getObjectStore()
+          .getClientProxy().getOzoneManagerClient().getServiceInfo();
+      Assert.assertTrue(serviceInfoEx.getCaCertificate().equals(
+          CertificateCodec.getPEMEncodedString(caCert)));
+      client.close();
+
+      // get new client, it should succeed.
+      try {
+        OzoneClient client1 = OzoneClientFactory.getRpcClient(conf);
+        client1.close();
+      } catch (Exception e) {
+        System.out.println("OzoneClientFactory.getRpcClient failed for " +
+            e.getMessage());
+        fail("Create client should succeed for certificate is renewed");
+      }
+
+      // Wait for old OM certificate to expire
+      GenericTestUtils.waitFor(() -> omCert.getNotAfter().before(new Date()),
+          500, certLifetime * 1000);
+      // get new client, it should succeed too.
+      try {
+        OzoneClientFactory.getRpcClient(conf);
+      } catch (Exception e) {
+        System.out.println("OzoneClientFactory.getRpcClient failed for " +
+            e.getMessage());
+        fail("Create client should succeed for certificate is renewed");
+      }
+    } finally {
+      DefaultCertificateClient.setUgi(null);
+      OzoneManager.setUgi(null);
+      GrpcOmTransport.setCaCerts(null);
+    }
+  }
+
   public void validateCertificate(X509Certificate cert) throws Exception {
 
     // Assert that we indeed have a self signed certificate.
@@ -1297,4 +1444,37 @@ public final class TestSecureOzoneCluster {
         .setScmID("test")
         .build();
   }
+
+  private static X509CertificateHolder generateX509CertHolder(
+      OzoneConfiguration conf, KeyPair keyPair, KeyPair rootKeyPair,
+      X509Certificate rootCert, Duration certLifetime, String subject,
+      String clusterId) throws Exception {
+    // Generate normal certificate, signed by RootCA certificate
+    SecurityConfig secConfig = new SecurityConfig(conf);
+    DefaultApprover approver = new DefaultApprover(new DefaultProfile(),
+        secConfig);
+
+    CertificateSignRequest.Builder csrBuilder =
+        new CertificateSignRequest.Builder();
+    // Get host name.
+    csrBuilder.setKey(keyPair)
+        .setConfiguration(conf)
+        .setScmID("test")
+        .setClusterID(clusterId)
+        .setSubject(subject)
+        .setDigitalSignature(true)
+        .setDigitalEncryption(true);
+
+    LocalDateTime start = LocalDateTime.now();
+    String certDuration = conf.get(HDDS_X509_DEFAULT_DURATION,
+        HDDS_X509_DEFAULT_DURATION_DEFAULT);
+    X509CertificateHolder certificateHolder =
+        approver.sign(secConfig, rootKeyPair.getPrivate(),
+            new X509CertificateHolder(rootCert.getEncoded()),
+            Date.from(start.atZone(ZoneId.systemDefault()).toInstant()),
+            Date.from(start.plus(Duration.parse(certDuration))
+                .atZone(ZoneId.systemDefault()).toInstant()),
+            csrBuilder.build(), "test", clusterId);
+    return certificateHolder;
+  }
 }
diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/CertificateClientTestImpl.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/CertificateClientTestImpl.java
index 5a6bd9c48a..f13e830cc0 100644
--- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/CertificateClientTestImpl.java
+++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/CertificateClientTestImpl.java
@@ -332,7 +332,7 @@ public class CertificateClientTestImpl implements CertificateClient {
 
   @Override
   public String getComponentName() {
-    return null;
+    return this.getClass().getSimpleName();
   }
 
   @Override
diff --git a/hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto b/hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto
index 01b0eab792..a2b2994cf9 100644
--- a/hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto
+++ b/hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto
@@ -464,10 +464,10 @@ enum Status {
     TENANT_NOT_EMPTY = 85;
 
     FEATURE_NOT_ENABLED = 86;
-
     INVALID_SNAPSHOT_ERROR = 87;
 
     CONTAINS_SNAPSHOT = 88;
+    SSL_CONNECTION_FAILURE = 89;
 }
 
 /**
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 2231c28c85..085affb819 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
@@ -23,6 +23,7 @@ import java.util.concurrent.TimeUnit;
 
 import org.apache.hadoop.hdds.HddsUtils;
 import org.apache.hadoop.hdds.conf.OzoneConfiguration;
+import org.apache.hadoop.hdds.security.ssl.KeyStoresFactory;
 import org.apache.hadoop.ozone.ha.ConfUtils;
 import org.apache.hadoop.ozone.protocolPB.OzoneManagerProtocolServerSideTranslatorPB;
 import org.apache.hadoop.ozone.om.protocolPB.GrpcOmTransport;
@@ -96,21 +97,16 @@ public class GrpcOzoneManagerServer {
             omServerConfig));
 
     SecurityConfig secConf = new SecurityConfig(omServerConfig);
-    if (secConf.isGrpcTlsEnabled()) {
+    if (secConf.isSecurityEnabled() && secConf.isGrpcTlsEnabled()) {
       try {
-        if (secConf.isSecurityEnabled()) {
-          SslContextBuilder sslClientContextBuilder =
-              SslContextBuilder.forServer(caClient.getPrivateKey(),
-                  caClient.getCertificate());
-          SslContextBuilder sslContextBuilder = GrpcSslContexts.configure(
-              sslClientContextBuilder,
-              SslProvider.valueOf(omServerConfig.get(HDDS_GRPC_TLS_PROVIDER,
-                  HDDS_GRPC_TLS_PROVIDER_DEFAULT)));
-          nettyServerBuilder.sslContext(sslContextBuilder.build());
-        } else {
-          LOG.error("ozone.security not enabled when TLS specified," +
-                            " creating Om S3g GRPC channel using plaintext");
-        }
+        KeyStoresFactory factory = caClient.getServerKeyStoresFactory();
+        SslContextBuilder sslClientContextBuilder =
+            SslContextBuilder.forServer(factory.getKeyManagers()[0]);
+        SslContextBuilder sslContextBuilder = GrpcSslContexts.configure(
+            sslClientContextBuilder,
+            SslProvider.valueOf(omServerConfig.get(HDDS_GRPC_TLS_PROVIDER,
+                HDDS_GRPC_TLS_PROVIDER_DEFAULT)));
+        nettyServerBuilder.sslContext(sslContextBuilder.build());
       } catch (Exception ex) {
         LOG.error("Unable to setup TLS for secure Om S3g GRPC channel.", ex);
       }
diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java
index d25633a673..e469a98f70 100644
--- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java
+++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java
@@ -419,6 +419,7 @@ public final class OzoneManager extends ServiceRuntimeInfoImpl
   // Test flags
   private static boolean testReloadConfigFlag = false;
   private static boolean testSecureOmFlag = false;
+  private static UserGroupInformation testUgi;
 
   private final OzoneLockProvider ozoneLockProvider;
   private OMPerformanceMetrics perfMetrics;
@@ -575,7 +576,7 @@ public final class OzoneManager extends ServiceRuntimeInfoImpl
             ResultCodes.SCM_VERSION_MISMATCH_ERROR);
       }
     } else {
-      scmInfo = new ScmInfo.Builder().setScmId("testSecureOm").build();
+      scmInfo = new ScmInfo.Builder().setScmId("test").build();
     }
 
     RPC.setProtocolEngine(configuration, OzoneManagerProtocolPB.class,
@@ -1248,7 +1249,7 @@ public final class OzoneManager extends ServiceRuntimeInfoImpl
   private static void loginOMUserIfSecurityEnabled(OzoneConfiguration conf)
       throws IOException, AuthenticationException {
     securityEnabled = OzoneSecurityUtil.isSecurityEnabled(conf);
-    if (securityEnabled) {
+    if (securityEnabled && testUgi == null) {
       loginOMUser(conf);
     }
   }
@@ -3859,6 +3860,11 @@ public final class OzoneManager extends ServiceRuntimeInfoImpl
     OzoneManager.testSecureOmFlag = testSecureOmFlag;
   }
 
+  @VisibleForTesting
+  public static void setUgi(UserGroupInformation user) {
+    OzoneManager.testUgi = user;
+  }
+
   public OMNodeDetails getNodeDetails() {
     return omNodeDetails;
   }
diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManagerServiceGrpc.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManagerServiceGrpc.java
index a88e259a28..5951c393b6 100644
--- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManagerServiceGrpc.java
+++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManagerServiceGrpc.java
@@ -87,11 +87,12 @@ public class OzoneManagerServiceGrpc extends OzoneManagerServiceImplBase {
           submitRequest(NULL_RPC_CONTROLLER, request);
       responseObserver.onNext(omResponse);
     } catch (Throwable e) {
+      LOG.error("Failed to submit request", e);
       IOException ex = new IOException(e.getCause());
-      responseObserver.onError(Status
-          .INTERNAL
-          .withDescription(ex.getMessage())
-          .asRuntimeException());
+      responseObserver.onError(
+          Status.INTERNAL.withDescription(ex.getMessage())
+              .asRuntimeException());
+      return;
     }
     responseObserver.onCompleted();
   }


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