xiaoyuyao commented on a change in pull request #2000:
URL: https://github.com/apache/ozone/pull/2000#discussion_r600778695
##########
File path:
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/ozoneimpl/OzoneContainer.java
##########
@@ -167,8 +171,23 @@ public OzoneContainer(
blockDeletingService =
new BlockDeletingService(this, svcInterval.toMillis(), serviceTimeout,
TimeUnit.MILLISECONDS, config);
- tlsClientConfig = RatisHelper.createTlsClientConfig(
- secConf, certClient != null ? certClient.getCACertificate() : null);
+
+ List< X509Certificate > x509Certificates = null;
+ if (certClient != null) {
+ List<String> pemEncodedCerts = HAUtils.buildCAList(certClient, conf);
+ x509Certificates = new ArrayList<>(pemEncodedCerts.size());
Review comment:
NIT: can we wrap line 178-186 into HAUtils.buildCAList?
##########
File path:
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/transport/server/ratis/XceiverServerRatis.java
##########
@@ -465,9 +466,14 @@ private static Parameters
createTlsParameters(SecurityConfig conf,
Parameters parameters = new Parameters();
if (conf.isSecurityEnabled() && conf.isGrpcTlsEnabled()) {
+ ArrayList< X509Certificate > listCA = new ArrayList<>();
Review comment:
NIT: List<X509Certificate>
##########
File path:
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/StorageContainerManager.java
##########
@@ -793,7 +880,18 @@ public static boolean scmBootstrap(OzoneConfiguration conf)
// SCM Node info containing hostname to scm Id mappings
// will be persisted into the version file once this node gets added
// to existing SCM ring post node regular start up.
+
+ if(OzoneSecurityUtil.isSecurityEnabled(conf)) {
+ HASecurityUtils.initializeSecurity(scmStorageConfig,
+ scmInfo.getScmId(), config, getScmAddress(scmhaNodeDetails,
conf),
+ false);
+ }
+ scmStorageConfig.setPrimaryScmNodeId(scmInfo.getScmId());
Review comment:
bq. will be no rootCA running on that SCM, so bootstrap won't be
successful
If I remember correctly, the primary SCM(rootCA) does not have to be the
same as the Ratis leader. But when primary SCM issue SCM certificate, it will
go though via Ratis.
##########
File path:
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/SCMStorageConfig.java
##########
@@ -58,6 +60,28 @@ public void setScmId(String scmId) throws IOException {
}
}
+ public void setScmCertSerialId(String certSerialId) throws IOException {
Review comment:
Do we need the primary cert serialId saved separately?
##########
File path:
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/SCMSecurityProtocolServer.java
##########
@@ -268,7 +286,7 @@ public String getCACertificate() throws IOException {
public List<String> listCertificate(NodeType role,
long startSerialId, int count, boolean isRevoked) throws IOException {
List<X509Certificate> certificates =
- certificateServer.listCertificate(role, startSerialId, count,
+ rootCertificateServer.listCertificate(role, startSerialId, count,
Review comment:
the API is changed to list all SCM certificates. Can we add new API for
this instead of changing the semantics of existing ones?
##########
File path:
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/SCMSecurityProtocolServer.java
##########
@@ -225,7 +231,7 @@ public String getCertificate(String certSerialId) throws
IOException {
certSerialId);
try {
X509Certificate certificate =
- certificateServer.getCertificate(certSerialId);
+ rootCertificateServer.getCertificate(certSerialId);
Review comment:
here we change to only get SCM CA certificate from this API?
##########
File path:
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/StorageContainerManager.java
##########
@@ -558,44 +579,105 @@ private void initializeSystemManagers(OzoneConfiguration
conf,
*/
private void initializeCAnSecurityProtocol(OzoneConfiguration conf,
SCMConfigurator configurator) throws IOException {
- if(configurator.getCertificateServer() != null) {
- this.certificateServer = configurator.getCertificateServer();
+
+ // TODO: Support Certificate Server loading via Class Name loader.
+ // So it is easy to use different Certificate Servers if needed.
+ if(this.scmMetadataStore == null) {
+ LOG.error("Cannot initialize Certificate Server without a valid meta " +
+ "data layer.");
+ throw new SCMException("Cannot initialize CA without a valid metadata " +
+ "store", ResultCodes.SCM_NOT_INITIALIZED);
+ }
+
+ certificateStore =
+ new SCMCertStore.Builder().setMetadaStore(scmMetadataStore)
+ .setRatisServer(scmHAManager.getRatisServer())
+ .setCRLSequenceId(getLastSequenceIdForCRL()).build();
+
+
+ // If primary SCM node Id is set it means this is a cluster which has
+ // performed init with SCM HA version code.
+ if (primaryScmNodeId != null) {
Review comment:
Can we use explicit state to represent the init vs. normal run on
primary?
##########
File path:
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ha/HASecurityUtils.java
##########
@@ -0,0 +1,291 @@
+/*
+ * 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
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * 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.scm.ha;
+
+import org.apache.hadoop.hdds.conf.OzoneConfiguration;
+import org.apache.hadoop.hdds.protocol.proto.HddsProtos;
+import
org.apache.hadoop.hdds.protocol.proto.SCMSecurityProtocolProtos.SCMGetCertResponseProto;
+import
org.apache.hadoop.hdds.protocolPB.SCMSecurityProtocolClientSideTranslatorPB;
+import org.apache.hadoop.hdds.scm.server.SCMCertStore;
+import org.apache.hadoop.hdds.scm.server.SCMStorageConfig;
+import org.apache.hadoop.hdds.security.x509.SecurityConfig;
+import
org.apache.hadoop.hdds.security.x509.certificate.authority.CertificateServer;
+import
org.apache.hadoop.hdds.security.x509.certificate.authority.DefaultCAServer;
+import
org.apache.hadoop.hdds.security.x509.certificate.authority.PKIProfiles.DefaultCAProfile;
+import
org.apache.hadoop.hdds.security.x509.certificate.client.CertificateClient;
+import
org.apache.hadoop.hdds.security.x509.certificate.client.SCMCertificateClient;
+import org.apache.hadoop.hdds.security.x509.certificate.utils.CertificateCodec;
+import
org.apache.hadoop.hdds.security.x509.certificates.utils.CertificateSignRequest;
+import org.apache.hadoop.hdds.utils.HddsServerUtil;
+import org.apache.ratis.conf.Parameters;
+import org.apache.ratis.grpc.GrpcConfigKeys;
+import org.apache.ratis.grpc.GrpcTlsConfig;
+import org.bouncycastle.cert.X509CertificateHolder;
+import org.bouncycastle.pkcs.PKCS10CertificationRequest;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.net.InetAddress;
+import java.net.InetSocketAddress;
+import java.nio.file.Paths;
+import java.security.KeyPair;
+import java.security.cert.CertificateException;
+import java.security.cert.X509Certificate;
+import java.util.concurrent.ExecutionException;
+
+import static org.apache.hadoop.hdds.protocol.proto.HddsProtos.NodeType.SCM;
+import static
org.apache.hadoop.hdds.security.x509.certificate.authority.CertificateApprover.ApprovalType.KERBEROS_TRUSTED;
+import static
org.apache.hadoop.hdds.security.x509.certificates.utils.CertificateSignRequest.getEncodedString;
+import static org.apache.hadoop.ozone.OzoneConsts.SCM_CA_CERT_STORAGE_DIR;
+import static org.apache.hadoop.ozone.OzoneConsts.SCM_CA_PATH;
+
+public final class HASecurityUtils {
+
+ private HASecurityUtils() {
+ }
+
+ public static final Logger LOG =
+ LoggerFactory.getLogger(HASecurityUtils.class);
+
+ /**
+ * Initialize Security which generates public, private key pair and get SCM
+ * signed certificate and persist to local disk.
+ * @param scmStorageConfig
+ * @param fetchedScmId
+ * @param conf
+ * @param scmAddress
+ * @throws IOException
+ */
+ public static void initializeSecurity(SCMStorageConfig scmStorageConfig,
+ String fetchedScmId, OzoneConfiguration conf,
+ InetSocketAddress scmAddress, boolean primaryscm)
+ throws IOException {
+ LOG.info("Initializing secure StorageContainerManager.");
+
+ CertificateClient certClient =
+ new SCMCertificateClient(new SecurityConfig(conf));
+ CertificateClient.InitResponse response = certClient.init();
+ LOG.info("Init response: {}", response);
+ switch (response) {
+ case SUCCESS:
+ LOG.info("Initialization successful.");
+ break;
+ case GETCERT:
+ if (!primaryscm) {
+ getSCMSignedCert(certClient, conf, fetchedScmId, scmStorageConfig,
+ scmAddress);
+ } else {
+ getPrimarySCMSignedCert(certClient, conf, fetchedScmId,
+ scmStorageConfig, scmAddress);
+ }
+ LOG.info("Successfully stored SCM signed certificate.");
+ break;
+ case FAILURE:
+ LOG.error("SCM security initialization failed.");
+ throw new RuntimeException("OM security initialization failed.");
+ case RECOVER:
+ LOG.error("SCM security initialization failed. SCM certificate is " +
+ "missing.");
+ throw new RuntimeException("SCM security initialization failed.");
+ default:
+ LOG.error("SCM security initialization failed. Init response: {}",
+ response);
+ throw new RuntimeException("SCM security initialization failed.");
+ }
+ }
+
+ /**
+ * Get SCM signed certificate and store it using certificate client.
+ */
+ private static void getSCMSignedCert(CertificateClient client,
+ OzoneConfiguration config, String fetchedSCMId,
+ SCMStorageConfig scmStorageConfig, InetSocketAddress scmAddress)
+ throws IOException {
+
+ PKCS10CertificationRequest csr = generateCSR(client, scmStorageConfig,
+ config, scmAddress, fetchedSCMId);
+
+ HddsProtos.ScmNodeDetailsProto scmNodeDetailsProto =
+ HddsProtos.ScmNodeDetailsProto.newBuilder()
+ .setClusterId(scmStorageConfig.getClusterID())
+ .setHostName(scmAddress.getHostName())
+ .setScmNodeId(fetchedSCMId).build();
+
+ SCMSecurityProtocolClientSideTranslatorPB secureScmClient =
+ HddsServerUtil.getScmSecurityClient(config);
+
+ SCMGetCertResponseProto response = secureScmClient.
+ getSCMCertChain(scmNodeDetailsProto, getEncodedString(csr));
+ String pemEncodedCert = response.getX509Certificate();
+
+ try {
+
+ // Store SCM CA certificate.
+ if (response.hasX509CACertificate()) {
+ String pemEncodedRootCert = response.getX509CACertificate();
+ client.storeCertificate(pemEncodedRootCert, true, true);
+ client.storeCertificate(pemEncodedCert, true);
+
+
+ // Write to the location, so that Default CA server can read the
+ // certificate generated by cert client.
+ CertificateCodec certCodec =
+ new CertificateCodec(new SecurityConfig(config),
+ client.getComponentName());
+
+ X509Certificate certificate =
+ CertificateCodec.getX509Certificate(pemEncodedCert);
+
certCodec.writeCertificate(certCodec.getCertificateHolder(certificate));
+
+ // Persist scm cert serial ID.
+ scmStorageConfig.setScmCertSerialId(certificate.getSerialNumber()
+ .toString());
+ } else {
+ throw new RuntimeException("Unable to retrieve SCM certificate chain");
+ }
+ } catch (IOException | CertificateException e) {
+ LOG.error("Error while storing SCM signed certificate.", e);
+ throw new RuntimeException(e);
+ }
+ }
+
+ private static void getPrimarySCMSignedCert(CertificateClient client,
+ OzoneConfiguration config, String fetchedSCMId,
+ SCMStorageConfig scmStorageConfig, InetSocketAddress scmAddress)
+ throws IOException {
+
+ PKCS10CertificationRequest csr = generateCSR(client, scmStorageConfig,
+ config, scmAddress, fetchedSCMId);
+
+ CertificateServer rootCAServer =
+ initializeRootCertificateServer(scmStorageConfig.getClusterID(),
+ scmStorageConfig.getScmId(), null);
+
+ rootCAServer.init(new SecurityConfig(config),
+ CertificateServer.CAType.SELF_SIGNED_CA);
+
+ try {
+ X509CertificateHolder certificateHolder = rootCAServer.
+ requestCertificate(csr, KERBEROS_TRUSTED, SCM).get();
+
+ X509CertificateHolder rootCAServerCACertificateHolder =
+ rootCAServer.getCACertificate();
+
+ String pemEncodedCert =
+ CertificateCodec.getPEMEncodedString(certificateHolder);
+
+ String pemEncodedRootCert =
+
CertificateCodec.getPEMEncodedString(rootCAServerCACertificateHolder);
+
+
+ client.storeCertificate(pemEncodedRootCert, true, true);
+ client.storeCertificate(pemEncodedCert, true);
+
+ // Write to the location, so that Default CA server can read the
+ // certificate generated by cert client.
+ CertificateCodec certCodec =
+ new CertificateCodec(new SecurityConfig(config),
+ client.getComponentName());
+
+ certCodec.writeCertificate(certificateHolder);
+
+ // Persist scm cert serial ID.
+ scmStorageConfig.setScmCertSerialId(certificateHolder.getSerialNumber()
+ .toString());
+ } catch (InterruptedException e) {
+ LOG.error("Error while storing SCM signed certificate.", e);
+ Thread.currentThread().interrupt();
+ throw new RuntimeException(e);
+ } catch (ExecutionException e) {
+ LOG.error("Error while storing SCM signed certificate.", e);
+ throw new RuntimeException(e);
+ } catch (IOException | CertificateException e) {
+ LOG.error("Error while storing SCM signed certificate.", e);
+ throw new RuntimeException(e);
+ }
+
+ }
+
+ /**
+ * This function creates/initializes a certificate server as needed.
+ * This function is idempotent, so calling this again and again after the
+ * server is initialized is not a problem.
+ *
+ * @param clusterID - Cluster ID
+ * @param scmID - SCM ID
+ */
+ public static CertificateServer initializeRootCertificateServer(
+ String clusterID, String scmID, SCMCertStore scmCertStore)
+ throws IOException {
+ String subject = "scm-rootca@" + InetAddress.getLocalHost().getHostName();
+
+ return new DefaultCAServer(subject, clusterID, scmID, scmCertStore,
+ new DefaultCAProfile(),
+ Paths.get(SCM_CA_CERT_STORAGE_DIR, SCM_CA_PATH).toString());
+ }
+
+ /**
+ * Create Server TLS parameters required for Ratis Server.
+ * @param conf
+ * @param caClient
+ * @return
+ */
+ public static Parameters createServerTlsParameters(SecurityConfig conf,
+ CertificateClient caClient) {
+ Parameters parameters = new Parameters();
+
+ if (conf.isSecurityEnabled() && conf.isGrpcTlsEnabled()) {
+ GrpcTlsConfig config = new GrpcTlsConfig(
+ caClient.getPrivateKey(), caClient.getCertificate(),
+ caClient.getCACertificate(), true);
+ GrpcConfigKeys.Server.setTlsConf(parameters, config);
+ GrpcConfigKeys.Admin.setTlsConf(parameters, config);
+ GrpcConfigKeys.Client.setTlsConf(parameters, config);
+ GrpcConfigKeys.TLS.setConf(parameters, config);
+ }
+
+ return parameters;
+ }
+
+ private static PKCS10CertificationRequest generateCSR(
+ CertificateClient client, SCMStorageConfig scmStorageConfig,
+ OzoneConfiguration config, InetSocketAddress scmAddress,
+ String fetchedSCMId) throws IOException {
+ CertificateSignRequest.Builder builder = client.getCSRBuilder();
+ KeyPair keyPair = new KeyPair(client.getPublicKey(),
Review comment:
why do we create a new keypair even non-primary already has a keypair
and the action is to GetCert from Primary SCM?
##########
File path:
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/protocol/SCMSecurityProtocolServerSideTranslatorPB.java
##########
@@ -271,6 +278,7 @@ public SCMGetCertResponseProto getRootCACertificate()
throws IOException {
SCMGetCertResponseProto
.newBuilder()
.setResponseCode(ResponseCode.success)
+ .setX509Certificate(rootCACertificate)
Review comment:
why do we need to set rootCACertificate twice? setX509RootCACertificate
should be sufficient as this is a new API.
##########
File path:
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ha/HASecurityUtils.java
##########
@@ -281,4 +284,26 @@ private static void
persistSubCACertificate(OzoneConfiguration config,
certCodec.writeCertificate(certificateHolder);
}
+ /**
+ * Create Server TLS parameters required for Ratis Server.
+ * @param conf
+ * @param caClient
+ * @return
+ */
+ public static Parameters createServerTlsParameters(SecurityConfig conf,
+ CertificateClient caClient) {
+ Parameters parameters = new Parameters();
+
+ if (conf.isSecurityEnabled() && conf.isGrpcTlsEnabled()) {
+ GrpcTlsConfig config = new GrpcTlsConfig(
+ caClient.getPrivateKey(), caClient.getCertificate(),
+ caClient.getCACertificate(), true);
Review comment:
I think we need to include the RootCA certificate as well.
##########
File path:
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/SCMSecurityProtocolServer.java
##########
@@ -155,18 +155,27 @@ public String getOMCertificate(OzoneManagerDetailsProto
omDetails,
public String getSCMCertificate(ScmNodeDetailsProto scmNodeDetails,
String certSignReq) throws IOException {
Objects.requireNonNull(scmNodeDetails);
- LOGGER.info("Processing CSR for scm {}, nodeId: {}",
- scmNodeDetails.getHostName(), scmNodeDetails.getScmNodeId());
+ String primaryScmId =
+ storageContainerManager.getScmStorageConfig().getPrimaryScmNodeId();
- // Check clusterID
- if (storageContainerManager.getClusterId().equals(
- scmNodeDetails.getClusterId())) {
- throw new IOException("SCM ClusterId mismatch. Peer SCM ClusterId " +
- scmNodeDetails.getClusterId() + ", primary SCM ClusterId "
- + storageContainerManager.getClusterId());
- }
+ if (primaryScmId != null &&
+ primaryScmId.equals(storageContainerManager.getScmId())) {
+ LOGGER.info("Processing CSR for scm {}, nodeId: {}",
+ scmNodeDetails.getHostName(), scmNodeDetails.getScmNodeId());
+
+ // Check clusterID
+ if (!storageContainerManager.getClusterId().equals(
+ scmNodeDetails.getClusterId())) {
+ throw new IOException("SCM ClusterId mismatch. Peer SCM ClusterId " +
+ scmNodeDetails.getClusterId() + ", primary SCM ClusterId "
+ + storageContainerManager.getClusterId());
+ }
- return getEncodedCertToString(certSignReq, NodeType.SCM);
+ return getEncodedCertToString(certSignReq, NodeType.SCM);
+ } else {
+ throw new SCMSecurityException("Get SCM Certificate can be run only " +
+ "primary SCM", PRIMARY_SCM_IS_NOT_LEADER);
Review comment:
This error code is confusing as we don't require the PRIMARY to be the
leader. Just the persist of the issued sub-scm certificate needs to go through
Ratis leader via the Ratis client API.
##########
File path:
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/security/x509/certificate/authority/DefaultApprover.java
##########
@@ -76,12 +77,12 @@ public DefaultApprover(PKIProfile pkiProfile,
SecurityConfig config) {
* @param scmId - SCM id.
* @param clusterId - Cluster id.
* @return Signed Certificate.
- * @throws IOException - On Error
+ * @throws IOException - On Error
Review comment:
NIT: unnecessary change.
##########
File path:
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/StorageContainerManager.java
##########
@@ -860,6 +966,40 @@ public static boolean scmInit(OzoneConfiguration conf,
}
}
+ private static InetSocketAddress getScmAddress(SCMHANodeDetails haDetails,
Review comment:
Can we move this to the HAUtils class?
##########
File path:
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/HAUtils.java
##########
@@ -326,4 +350,123 @@ public static void
checkSecurityAndSCMHAEnabled(OzoneConfiguration conf) {
}
}
}
+
+ /**
+ * Build CA list which need to be passed to client.
+ *
+ * If certificate client is null, obtain the list of CA using SCM security
+ * client, else it uses certificate client.
+ * @param certClient
+ * @param configuration
+ * @return list of CA
+ * @throws IOException
+ */
+ public static List<String> buildCAList(CertificateClient certClient,
+ ConfigurationSource configuration) throws IOException {
+ //TODO: make it configurable.
+ long waitTime = 5 * 60 * 1000L;
+ long retryTime = 10 * 1000L;
+ long currentTime = Time.monotonicNow();
+ List<String> caCertPemList = null;
+ if (certClient != null) {
+ caCertPemList = new ArrayList<>();
+ if (!SCMHAUtils.isSCMHAEnabled(configuration)) {
+ if (certClient.getRootCACertificate() != null) {
+ caCertPemList.add(CertificateCodec.getPEMEncodedString(
Review comment:
Can we bypass the encode as string here and as the caller will need the
X509Certificate format? The encode/decode seems redundant.
##########
File path:
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/SCMSecurityProtocolServer.java
##########
@@ -70,7 +70,7 @@
private static final Logger LOGGER = LoggerFactory
.getLogger(SCMSecurityProtocolServer.class);
- private final CertificateServer certificateServer;
+ private final CertificateServer rootCertificateServer;
Review comment:
NIT: SecurityProtocolServer also serves CSR for OM and DN, rename the
CertificateSever inside as rootCertificateServer is confusing.
##########
File path:
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/SCMSecurityProtocolServer.java
##########
@@ -179,9 +188,15 @@ public String getSCMCertificate(ScmNodeDetailsProto
scmNodeDetails,
*/
private String getEncodedCertToString(String certSignReq, NodeType nodeType)
throws IOException {
- Future<X509CertificateHolder> future =
- certificateServer.requestCertificate(certSignReq,
- KERBEROS_TRUSTED, nodeType);
+
+ Future<X509CertificateHolder> future;
+ if (nodeType == NodeType.SCM) {
+ future = rootCertificateServer.requestCertificate(certSignReq,
+ KERBEROS_TRUSTED, nodeType);
+ } else {
+ future = storageContainerManager.getScmCertificateServer()
Review comment:
Should we move the certificate server from SCM back to
SecurityProtocolServer and keep two certificate server inside the security
server for the primary SCM?
##########
File path:
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/StorageContainerManager.java
##########
@@ -319,7 +331,8 @@ private StorageContainerManager(OzoneConfiguration conf,
// if no Security, we do not create a Certificate Server at all.
// This allows user to boot SCM without security temporarily
// and then come back and enable it without any impact.
- certificateServer = null;
+ rootCertificateServer = null;
Review comment:
can we wrap the cert servers into the security protocol server?
##########
File path:
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/scm/proxy/SCMBlockLocationFailoverProxyProvider.java
##########
@@ -70,11 +70,20 @@
private final int maxRetryCount;
private final long retryInterval;
+ private final UserGroupInformation ugi;
+
public SCMBlockLocationFailoverProxyProvider(ConfigurationSource conf) {
this.conf = conf;
this.scmVersion = RPC.getProtocolVersion(ScmBlockLocationProtocolPB.class);
+ try {
+ this.ugi = UserGroupInformation.getCurrentUser();
Review comment:
Can you enable the Debug log of UGI class to see which UGI is used if we
don't cache the UGI at the time of provider creation?
##########
File path:
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ratis/OzoneManagerRatisServer.java
##########
@@ -651,9 +652,14 @@ private static Parameters
createServerTlsParameters(SecurityConfig conf,
Parameters parameters = new Parameters();
if (conf.isSecurityEnabled() && conf.isGrpcTlsEnabled()) {
+ ArrayList< X509Certificate > listCA = new ArrayList<>();
+ listCA.add(caClient.getCACertificate());
Review comment:
This needs to include the ROOT ca certificate with getCaCertPemList.
##########
File path:
hadoop-hdds/tools/src/main/java/org/apache/hadoop/hdds/scm/cli/ScmOption.java
##########
@@ -61,6 +61,7 @@ public ScmClient createScmClient() {
return new ContainerOperationClient(conf);
} catch (IOException ex) {
+ ex.printStackTrace();
Review comment:
Can we put this into LOG.error instead of print on the console?
##########
File path:
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/StorageContainerManager.java
##########
@@ -558,44 +579,105 @@ private void initializeSystemManagers(OzoneConfiguration
conf,
*/
private void initializeCAnSecurityProtocol(OzoneConfiguration conf,
Review comment:
I feel the logic inside this initializeCAnSecurityProtocol can be
refactored. Let's discuss offline...
##########
File path:
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/StorageContainerManager.java
##########
@@ -400,6 +413,14 @@ private StorageContainerManager(OzoneConfiguration conf,
registerMetricsSource(this);
}
+ private void initializeCertificateClient() {
+ if (primaryScmNodeId != null) {
Review comment:
should we check the local scm node id!=primaryScmNodeId here insead?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
