From df3d4cdb49e08f6711d5821e45391e7de6b2455b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Szabolcs=20G=C3=A1l?= Date: Wed, 5 Jul 2023 19:25:04 +0200 Subject: [PATCH 1/7] HDDS-8592 renameCertificateLifetimeMonitor --- .../x509/certificate/client/DefaultCertificateClient.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) 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 abd2beec506c..e7abfdda3151 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 @@ -1268,7 +1268,7 @@ public synchronized void startCertificateMonitor() { getComponentName() + "-CertificateLifetimeMonitor") .setDaemon(true).build()); } - this.executorService.scheduleAtFixedRate(new CertificateLifetimeMonitor(), + this.executorService.scheduleAtFixedRate(new CertificateRenewerService(), timeBeforeGracePeriod, interval, TimeUnit.MILLISECONDS); getLogger().info("CertificateLifetimeMonitor for {} is started with " + "first delay {} ms and interval {} ms.", component, @@ -1278,9 +1278,9 @@ public synchronized void startCertificateMonitor() { /** * Task to monitor certificate lifetime and renew the certificate if needed. */ - public class CertificateLifetimeMonitor implements Runnable { + public class CertificateRenewerService implements Runnable { - public CertificateLifetimeMonitor() { + public CertificateRenewerService() { } @Override From 5ac8311779ad4e1924005af9e1a7d51f8314f84e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Szabolcs=20G=C3=A1l?= Date: Wed, 5 Jul 2023 19:27:53 +0200 Subject: [PATCH 2/7] HDDS-8592 Add guard cause instead of indenting the entire certificateRenewerService --- .../client/DefaultCertificateClient.java | 57 ++++++++++--------- 1 file changed, 29 insertions(+), 28 deletions(-) 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 e7abfdda3151..7a0893cbe7c5 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 @@ -1295,38 +1295,39 @@ public void run() { synchronized (DefaultCertificateClient.class) { X509Certificate currentCert = getCertificate(); Duration timeLeft = timeBeforeExpiryGracePeriod(currentCert); - if (timeLeft.isZero()) { - String newCertId; - try { - getLogger().info("Current certificate {} has entered the expiry" + - " grace period {}. Starting renew key and certs.", - currentCert.getSerialNumber().toString(), - timeLeft, securityConfig.getRenewalGracePeriod()); - newCertId = renewAndStoreKeyAndCertificate(false); - } catch (CertificateException e) { - if (e.errorCode() == - CertificateException.ErrorCode.ROLLBACK_ERROR) { - if (shutdownCallback != null) { - getLogger().error("Failed to rollback key and cert after an " + - " unsuccessful renew try.", e); - shutdownCallback.run(); - } + if (!timeLeft.isZero()) { + return; + } + String newCertId; + try { + getLogger().info("Current certificate {} has entered the expiry" + + " grace period {}. Starting renew key and certs.", + currentCert.getSerialNumber().toString(), + timeLeft, securityConfig.getRenewalGracePeriod()); + newCertId = renewAndStoreKeyAndCertificate(false); + } catch (CertificateException e) { + if (e.errorCode() == + CertificateException.ErrorCode.ROLLBACK_ERROR) { + if (shutdownCallback != null) { + getLogger().error("Failed to rollback key and cert after an " + + " unsuccessful renew try.", e); + shutdownCallback.run(); } - getLogger().error("Failed to renew and store key and cert." + - " Keep using existing certificates.", e); - return; - } - - // Persist new cert serial id in component VERSION file - if (certIdSaveCallback != null) { - certIdSaveCallback.accept(newCertId); } + getLogger().error("Failed to renew and store key and cert." + + " Keep using existing certificates.", e); + return; + } - // reset and reload all certs - reloadKeyAndCertificate(newCertId); - // cleanup backup directory - cleanBackupDir(); + // Persist new cert serial id in component VERSION file + if (certIdSaveCallback != null) { + certIdSaveCallback.accept(newCertId); } + + // reset and reload all certs + reloadKeyAndCertificate(newCertId); + // cleanup backup directory + cleanBackupDir(); } } } From 1d68f30dc51eccc566c897531cd6637820d4593f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Szabolcs=20G=C3=A1l?= Date: Wed, 5 Jul 2023 19:29:45 +0200 Subject: [PATCH 3/7] HDDS-8592 add force parameter to certificateRenewerService --- .../client/DefaultCertificateClient.java | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) 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 7a0893cbe7c5..57ba832981ed 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 @@ -1265,10 +1265,11 @@ public synchronized void startCertificateMonitor() { if (executorService == null) { executorService = Executors.newScheduledThreadPool(1, new ThreadFactoryBuilder().setNameFormat( - getComponentName() + "-CertificateLifetimeMonitor") + getComponentName() + "-CertificateLifetimeMonitor") .setDaemon(true).build()); } - this.executorService.scheduleAtFixedRate(new CertificateRenewerService(), + this.executorService.scheduleAtFixedRate( + new CertificateRenewerService(false), timeBeforeGracePeriod, interval, TimeUnit.MILLISECONDS); getLogger().info("CertificateLifetimeMonitor for {} is started with " + "first delay {} ms and interval {} ms.", component, @@ -1276,11 +1277,13 @@ public synchronized void startCertificateMonitor() { } /** - * Task to monitor certificate lifetime and renew the certificate if needed. + * Task to monitor certificate lifetime and renew the certificate if needed. */ public class CertificateRenewerService implements Runnable { + private boolean forceRenewal; - public CertificateRenewerService() { + public CertificateRenewerService(boolean forceRenewal) { + this.forceRenewal = forceRenewal; } @Override @@ -1295,7 +1298,7 @@ public void run() { synchronized (DefaultCertificateClient.class) { X509Certificate currentCert = getCertificate(); Duration timeLeft = timeBeforeExpiryGracePeriod(currentCert); - if (!timeLeft.isZero()) { + if (forceRenewal && !timeLeft.isZero()) { return; } String newCertId; @@ -1304,7 +1307,7 @@ public void run() { " grace period {}. Starting renew key and certs.", currentCert.getSerialNumber().toString(), timeLeft, securityConfig.getRenewalGracePeriod()); - newCertId = renewAndStoreKeyAndCertificate(false); + newCertId = renewAndStoreKeyAndCertificate(forceRenewal); } catch (CertificateException e) { if (e.errorCode() == CertificateException.ErrorCode.ROLLBACK_ERROR) { From 5d87b34c1a6a8b6f7e1d019fcde427af2c7c9e7d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Szabolcs=20G=C3=A1l?= Date: Wed, 5 Jul 2023 19:32:32 +0200 Subject: [PATCH 4/7] HDDS-8592 Fix bug regarding certId being inconvertable to long --- .../x509/certificate/client/DefaultCertificateClient.java | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) 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 57ba832981ed..e6dd598fad61 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 @@ -21,6 +21,7 @@ import java.io.File; import java.io.IOException; +import java.math.BigInteger; import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; @@ -228,15 +229,17 @@ private void updateCachedData( } private synchronized void updateCachedRootCAId(String s) { + BigInteger candidateNewId = new BigInteger(s); if (rootCaCertId == null - || Long.parseLong(s) > Long.parseLong(rootCaCertId)) { + || new BigInteger(rootCaCertId).compareTo(candidateNewId) < 0) { rootCaCertId = s; } } private synchronized void updateCachedSubCAId(String s) { + BigInteger candidateNewId = new BigInteger(s); if (caCertId == null - || Long.parseLong(s) > Long.parseLong(caCertId)) { + || new BigInteger(caCertId).compareTo(candidateNewId) < 0) { caCertId = s; } } From 05b1814d50fee5a972532090877fe1beb26a13ed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Szabolcs=20G=C3=A1l?= Date: Wed, 5 Jul 2023 20:20:54 +0200 Subject: [PATCH 5/7] HDDS-8592 Refactor signAndStoreCertificate to be in a centralized place --- .../client/DNCertificateClient.java | 46 ++------------- .../client/DefaultCertificateClient.java | 43 +++++++++++++- .../client/SCMCertificateClient.java | 11 +++- .../client/TestDefaultCertificateClient.java | 27 ++++++--- .../ozone/security/OMCertificateClient.java | 40 ++----------- .../security/ReconCertificateClient.java | 57 +++++-------------- 6 files changed, 93 insertions(+), 131 deletions(-) diff --git a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/security/x509/certificate/client/DNCertificateClient.java b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/security/x509/certificate/client/DNCertificateClient.java index 60853273bd37..27b2da575873 100644 --- a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/security/x509/certificate/client/DNCertificateClient.java +++ b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/security/x509/certificate/client/DNCertificateClient.java @@ -20,11 +20,9 @@ package org.apache.hadoop.hdds.security.x509.certificate.client; import org.apache.hadoop.hdds.protocol.DatanodeDetails; -import org.apache.hadoop.hdds.protocol.proto.SCMSecurityProtocolProtos; +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.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.security.UserGroupInformation; @@ -34,7 +32,6 @@ import java.io.IOException; import java.net.InetAddress; -import java.nio.file.Path; import java.security.KeyPair; import java.util.function.Consumer; @@ -98,43 +95,10 @@ public CertificateSignRequest.Builder getCSRBuilder() } @Override - public String signAndStoreCertificate(PKCS10CertificationRequest csr, - Path certificatePath, boolean renew) throws CertificateException { - try { - // TODO: For SCM CA we should fetch certificate from multiple SCMs. - SCMSecurityProtocolProtos.SCMGetCertResponseProto response = - getScmSecureClient().getDataNodeCertificateChain( - dn.getProtoBufMessage(), getEncodedString(csr)); - - // Persist certificates. - if (response.hasX509CACertificate()) { - String pemEncodedCert = response.getX509Certificate(); - CertificateCodec certCodec = new CertificateCodec( - getSecurityConfig(), certificatePath); - // Certs will be added to cert map after reloadAllCertificate called - storeCertificate(pemEncodedCert, CAType.NONE, - certCodec, false, !renew); - storeCertificate(response.getX509CACertificate(), - CAType.SUBORDINATE, certCodec, false, !renew); - - // Store Root CA certificate. - if (response.hasX509RootCACertificate()) { - storeCertificate(response.getX509RootCACertificate(), - CAType.ROOT, certCodec, false, !renew); - } - // Return the default certificate ID - return CertificateCodec.getX509Certificate(pemEncodedCert) - .getSerialNumber() - .toString(); - } else { - throw new CertificateException("Unable to retrieve datanode " + - "certificate chain."); - } - } catch (IOException | java.security.cert.CertificateException e) { - LOG.error("Error while signing and storing SCM signed certificate.", e); - throw new CertificateException( - "Error while signing and storing SCM signed certificate.", e); - } + public SCMGetCertResponseProto getCertificateSignResponse( + PKCS10CertificationRequest csr) throws IOException { + return getScmSecureClient().getDataNodeCertificateChain( + dn.getProtoBufMessage(), getEncodedString(csr)); } @Override 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 e6dd598fad61..b1bc3774d660 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 @@ -63,6 +63,7 @@ import com.google.common.annotations.VisibleForTesting; import com.google.common.util.concurrent.ThreadFactoryBuilder; import org.apache.commons.io.FileUtils; +import org.apache.hadoop.hdds.protocol.proto.SCMSecurityProtocolProtos.SCMGetCertResponseProto; import org.apache.hadoop.hdds.protocolPB.SCMSecurityProtocolClientSideTranslatorPB; import org.apache.hadoop.hdds.security.SecurityConfig; import org.apache.hadoop.hdds.security.ssl.KeyStoresFactory; @@ -1235,9 +1236,47 @@ protected String signAndStoreCertificate( return signAndStoreCertificate(request, certificatePath, false); } - protected abstract String signAndStoreCertificate( + protected abstract SCMGetCertResponseProto getCertificateSignResponse( + PKCS10CertificationRequest request) throws IOException; + + protected String signAndStoreCertificate( PKCS10CertificationRequest request, Path certificatePath, boolean renew) - throws CertificateException; + throws CertificateException { + try { + SCMGetCertResponseProto response = getCertificateSignResponse(request); + + // Persist certificates. + if (response.hasX509CACertificate()) { + String pemEncodedCert = response.getX509Certificate(); + CertificateCodec certCodec = new CertificateCodec( + getSecurityConfig(), certificatePath); + // Certs will be added to cert map after reloadAllCertificate called + storeCertificate(pemEncodedCert, CAType.NONE, + certCodec, false, !renew); + storeCertificate(response.getX509CACertificate(), + CAType.SUBORDINATE, certCodec, false, !renew); + + // Store Root CA certificate. + if (response.hasX509RootCACertificate()) { + storeCertificate(response.getX509RootCACertificate(), + CAType.ROOT, certCodec, false, !renew); + } + // Return the default certificate ID + return CertificateCodec.getX509Certificate(pemEncodedCert) + .getSerialNumber() + .toString(); + } else { + throw new CertificateException("Unable to retrieve " + + "certificate chain."); + } + } catch (IOException | java.security.cert.CertificateException e) { + logger.error("Error while signing and storing SCM signed certificate.", + e); + throw new CertificateException( + "Error while signing and storing SCM signed certificate.", e); + } + } + public String signAndStoreCertificate( PKCS10CertificationRequest request) throws CertificateException { 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 26305624b490..32a9326e460b 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 @@ -18,10 +18,10 @@ package org.apache.hadoop.hdds.security.x509.certificate.client; +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.protocol.proto.HddsProtos; -import org.apache.hadoop.hdds.protocol.proto.SCMSecurityProtocolProtos; 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; @@ -182,6 +182,13 @@ public Logger getLogger() { return LOG; } + @Override + protected SCMGetCertResponseProto getCertificateSignResponse( + PKCS10CertificationRequest request) { + throw new UnsupportedOperationException("getCertSignResponse of " + + " SCMCertificateClient is not supported currently"); + } + @Override public String signAndStoreCertificate(PKCS10CertificationRequest request, Path certPath, boolean renew) throws CertificateException { @@ -193,7 +200,7 @@ public String signAndStoreCertificate(PKCS10CertificationRequest request, .setScmNodeId(scmId).build(); // Get SCM sub CA cert. - SCMSecurityProtocolProtos.SCMGetCertResponseProto response = + SCMGetCertResponseProto response = getScmSecureClient().getSCMCertChain(scmNodeDetailsProto, getEncodedString(request), true); diff --git a/hadoop-hdds/framework/src/test/java/org/apache/hadoop/hdds/security/x509/certificate/client/TestDefaultCertificateClient.java b/hadoop-hdds/framework/src/test/java/org/apache/hadoop/hdds/security/x509/certificate/client/TestDefaultCertificateClient.java index c0af10a3da65..1725a0b510ea 100644 --- a/hadoop-hdds/framework/src/test/java/org/apache/hadoop/hdds/security/x509/certificate/client/TestDefaultCertificateClient.java +++ b/hadoop-hdds/framework/src/test/java/org/apache/hadoop/hdds/security/x509/certificate/client/TestDefaultCertificateClient.java @@ -20,7 +20,7 @@ import org.apache.hadoop.hdds.HddsConfigKeys; import org.apache.hadoop.hdds.protocol.MockDatanodeDetails; -import org.apache.hadoop.hdds.protocol.proto.SCMSecurityProtocolProtos; +import org.apache.hadoop.hdds.protocol.proto.SCMSecurityProtocolProtos.SCMGetCertResponseProto; import org.apache.hadoop.hdds.protocolPB.SCMSecurityProtocolClientSideTranslatorPB; import org.apache.hadoop.hdds.security.x509.certificate.authority.CAType; import org.apache.hadoop.hdds.security.x509.certificate.client.CertificateClient.InitResponse; @@ -489,10 +489,16 @@ public String signAndStoreCertificate( return null; } + @Override + protected SCMGetCertResponseProto getCertificateSignResponse( + PKCS10CertificationRequest request) { + return null; + } + @Override public String signAndStoreCertificate( PKCS10CertificationRequest request, Path certificatePath, - boolean renew) throws CertificateException { + boolean renew) { return null; } }) { @@ -536,10 +542,11 @@ public void testRenewAndStoreKeyAndCertificate() throws Exception { X509Certificate newCert = generateX509Cert(null); String pemCert = CertificateCodec.getPEMEncodedString(newCert); - SCMSecurityProtocolProtos.SCMGetCertResponseProto responseProto = - SCMSecurityProtocolProtos.SCMGetCertResponseProto - .newBuilder().setResponseCode(SCMSecurityProtocolProtos - .SCMGetCertResponseProto.ResponseCode.success) + SCMGetCertResponseProto responseProto = + SCMGetCertResponseProto + .newBuilder().setResponseCode( + SCMGetCertResponseProto + .ResponseCode.success) .setX509Certificate(pemCert) .setX509CACertificate(pemCert) .build(); @@ -631,10 +638,16 @@ protected String signAndStoreCertificate( return ""; } + @Override + protected SCMGetCertResponseProto getCertificateSignResponse( + PKCS10CertificationRequest request) { + return null; + } + @Override protected String signAndStoreCertificate( PKCS10CertificationRequest request, Path certificatePath, - boolean renew) throws CertificateException { + boolean renew) { return null; } }; diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/security/OMCertificateClient.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/security/OMCertificateClient.java index 1a94d16521af..4b3bbb554557 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/security/OMCertificateClient.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/security/OMCertificateClient.java @@ -24,9 +24,7 @@ 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.x509.certificate.authority.CAType; import org.apache.hadoop.hdds.security.x509.certificate.client.CommonCertificateClient; -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.ozone.om.OMStorage; @@ -36,7 +34,6 @@ import org.slf4j.LoggerFactory; import java.io.IOException; -import java.nio.file.Path; import java.security.KeyPair; import java.util.function.Consumer; @@ -121,39 +118,10 @@ public CertificateSignRequest.Builder getCSRBuilder() } @Override - public String signAndStoreCertificate(PKCS10CertificationRequest request, - Path certificatePath, boolean renew) throws CertificateException { - try { - SCMGetCertResponseProto response = getScmSecureClient() - .getOMCertChain(omInfo, getEncodedString(request)); - - String pemEncodedCert = response.getX509Certificate(); - CertificateCodec certCodec = new CertificateCodec( - getSecurityConfig(), certificatePath); - - // Store SCM CA certificate. - if (response.hasX509CACertificate()) { - String pemEncodedRootCert = response.getX509CACertificate(); - storeCertificate(pemEncodedRootCert, - CAType.SUBORDINATE, certCodec, false, !renew); - storeCertificate(pemEncodedCert, CAType.NONE, certCodec, false, !renew); - - // Store Root CA certificate if available. - if (response.hasX509RootCACertificate()) { - storeCertificate(response.getX509RootCACertificate(), - CAType.ROOT, certCodec, false, !renew); - } - return CertificateCodec.getX509Certificate(pemEncodedCert) - .getSerialNumber().toString(); - } else { - throw new CertificateException("Unable to retrieve OM certificate " + - "chain."); - } - } catch (IOException | java.security.cert.CertificateException e) { - LOG.error("Error while signing and storing SCM signed certificate.", e); - throw new CertificateException( - "Error while signing and storing SCM signed certificate.", e); - } + protected SCMGetCertResponseProto getCertificateSignResponse( + PKCS10CertificationRequest request) throws IOException { + return getScmSecureClient().getOMCertChain( + omInfo, getEncodedString(request)); } @Override diff --git a/hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/security/ReconCertificateClient.java b/hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/security/ReconCertificateClient.java index 5381a6159546..4cffb84e80e7 100644 --- a/hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/security/ReconCertificateClient.java +++ b/hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/security/ReconCertificateClient.java @@ -18,12 +18,10 @@ package org.apache.hadoop.ozone.recon.security; import org.apache.hadoop.hdds.protocol.proto.HddsProtos; -import org.apache.hadoop.hdds.protocol.proto.SCMSecurityProtocolProtos; +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.x509.certificate.authority.CAType; import org.apache.hadoop.hdds.security.x509.certificate.client.CommonCertificateClient; -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.ozone.recon.scm.ReconStorageConfig; @@ -34,11 +32,9 @@ import java.io.IOException; import java.net.InetAddress; -import java.nio.file.Path; import java.security.KeyPair; import java.util.function.Consumer; -import static org.apache.hadoop.hdds.security.x509.certificate.utils.CertificateCodec.getX509Certificate; import static org.apache.hadoop.hdds.security.x509.certificate.utils.CertificateSignRequest.getEncodedString; import static org.apache.hadoop.hdds.security.x509.exception.CertificateException.ErrorCode.CSR_ERROR; @@ -89,44 +85,19 @@ public CertificateSignRequest.Builder getCSRBuilder() } @Override - public String signAndStoreCertificate(PKCS10CertificationRequest csr, - Path certificatePath, boolean renew) throws CertificateException { - try { - SCMSecurityProtocolProtos.SCMGetCertResponseProto response; - HddsProtos.NodeDetailsProto.Builder reconDetailsProtoBuilder = - HddsProtos.NodeDetailsProto.newBuilder() - .setHostName(InetAddress.getLocalHost().getHostName()) - .setClusterId(clusterID) - .setUuid(reconID) - .setNodeType(HddsProtos.NodeType.RECON); - // TODO: For SCM CA we should fetch certificate from multiple SCMs. - response = getScmSecureClient().getCertificateChain( - reconDetailsProtoBuilder.build(), getEncodedString(csr)); - - // Persist certificates. - if (response.hasX509CACertificate()) { - String pemEncodedCert = response.getX509Certificate(); - CertificateCodec certCodec = new CertificateCodec( - getSecurityConfig(), certificatePath); - storeCertificate(pemEncodedCert, CAType.NONE, certCodec, false, !renew); - storeCertificate(response.getX509CACertificate(), - CAType.SUBORDINATE, certCodec, false, !renew); - - // Store Root CA certificate. - if (response.hasX509RootCACertificate()) { - storeCertificate(response.getX509RootCACertificate(), - CAType.ROOT, certCodec, false, !renew); - } - return getX509Certificate(pemEncodedCert).getSerialNumber().toString(); - } else { - throw new CertificateException("Unable to retrieve recon certificate " + - "chain"); - } - } catch (IOException | java.security.cert.CertificateException e) { - LOG.error("Error while signing and storing SCM signed certificate.", e); - throw new CertificateException( - "Error while signing and storing SCM signed certificate.", e); - } + protected SCMGetCertResponseProto getCertificateSignResponse( + PKCS10CertificationRequest request) throws IOException { + SCMGetCertResponseProto response; + HddsProtos.NodeDetailsProto.Builder reconDetailsProtoBuilder = + HddsProtos.NodeDetailsProto.newBuilder() + .setHostName(InetAddress.getLocalHost().getHostName()) + .setClusterId(clusterID) + .setUuid(reconID) + .setNodeType(HddsProtos.NodeType.RECON); + // TODO: For SCM CA we should fetch certificate from multiple SCMs. + response = getScmSecureClient().getCertificateChain( + reconDetailsProtoBuilder.build(), getEncodedString(request)); + return response; } @Override From e670269ffa2b16ca062e2440944aedadfad52bad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Szabolcs=20G=C3=A1l?= Date: Thu, 6 Jul 2023 09:42:42 +0200 Subject: [PATCH 6/7] HDDS-8592 Fix failing unit test, and forcerenewal condition --- .../ozone/TestHddsSecureDatanodeInit.java | 8 +++++++ .../client/DefaultCertificateClient.java | 21 ++++++++++++------- 2 files changed, 21 insertions(+), 8 deletions(-) diff --git a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/TestHddsSecureDatanodeInit.java b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/TestHddsSecureDatanodeInit.java index 67c95ce11152..e9a6e59ff16c 100644 --- a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/TestHddsSecureDatanodeInit.java +++ b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/TestHddsSecureDatanodeInit.java @@ -25,6 +25,8 @@ import java.security.cert.CertificateExpiredException; import java.time.Duration; import java.time.LocalDateTime; +import java.util.ArrayList; +import java.util.List; import java.util.concurrent.Callable; import org.apache.hadoop.fs.FileUtil; @@ -316,6 +318,9 @@ public void testCertificateRotation() throws Exception { when(scmClient.getDataNodeCertificateChain(anyObject(), anyString())) .thenReturn(responseProto); + List rootCaList = new ArrayList<>(); + rootCaList.add(pemCert); + when(scmClient.getAllRootCaCertificates()).thenReturn(rootCaList); // check that new cert ID should not equal to current cert ID String certId = newCertHolder.getSerialNumber().toString(); Assert.assertFalse(certId.equals( @@ -338,6 +343,7 @@ public void testCertificateRotation() throws Exception { // test the second time certificate rotation, generate a new cert newCertHolder = generateX509CertHolder(null, null, Duration.ofSeconds(CERT_LIFETIME)); + rootCaList.remove(pemCert); pemCert = CertificateCodec.getPEMEncodedString(newCertHolder); responseProto = SCMSecurityProtocolProtos.SCMGetCertResponseProto .newBuilder().setResponseCode(SCMSecurityProtocolProtos @@ -348,6 +354,8 @@ public void testCertificateRotation() throws Exception { .build(); when(scmClient.getDataNodeCertificateChain(anyObject(), anyString())) .thenReturn(responseProto); + rootCaList.add(pemCert); + when(scmClient.getAllRootCaCertificates()).thenReturn(rootCaList); String certId2 = newCertHolder.getSerialNumber().toString(); // check after renew, client will have the new cert ID 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 b1bc3774d660..2cea0d8ed7ba 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 @@ -1256,15 +1256,11 @@ protected String signAndStoreCertificate( storeCertificate(response.getX509CACertificate(), CAType.SUBORDINATE, certCodec, false, !renew); - // Store Root CA certificate. - if (response.hasX509RootCACertificate()) { - storeCertificate(response.getX509RootCACertificate(), - CAType.ROOT, certCodec, false, !renew); - } + getAndStoreAllRootCAs(certCodec, renew); // Return the default certificate ID - return CertificateCodec.getX509Certificate(pemEncodedCert) + return updateCertSerialId(CertificateCodec.getX509Certificate(pemEncodedCert) .getSerialNumber() - .toString(); + .toString()); } else { throw new CertificateException("Unable to retrieve " + "certificate chain."); @@ -1277,6 +1273,14 @@ protected String signAndStoreCertificate( } } + private void getAndStoreAllRootCAs(CertificateCodec certCodec, boolean renew) + throws IOException { + List rootCAPems = scmSecurityClient.getAllRootCaCertificates(); + for (String rootCAPem : rootCAPems) { + storeCertificate(rootCAPem, CAType.ROOT, certCodec, + false, !renew); + } + } public String signAndStoreCertificate( PKCS10CertificationRequest request) throws CertificateException { @@ -1340,7 +1344,8 @@ public void run() { synchronized (DefaultCertificateClient.class) { X509Certificate currentCert = getCertificate(); Duration timeLeft = timeBeforeExpiryGracePeriod(currentCert); - if (forceRenewal && !timeLeft.isZero()) { + + if (!forceRenewal && !timeLeft.isZero()) { return; } String newCertId; From 37cd46cf6746ccd0c38fbeb3fa5c09057603aeda Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Szabolcs=20G=C3=A1l?= Date: Thu, 6 Jul 2023 10:55:21 +0200 Subject: [PATCH 7/7] HDDS-8592 Fix line longer than 80 characters --- .../x509/certificate/client/DefaultCertificateClient.java | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) 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 2cea0d8ed7ba..fb7587a838cb 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 @@ -1258,9 +1258,8 @@ protected String signAndStoreCertificate( getAndStoreAllRootCAs(certCodec, renew); // Return the default certificate ID - return updateCertSerialId(CertificateCodec.getX509Certificate(pemEncodedCert) - .getSerialNumber() - .toString()); + return updateCertSerialId(CertificateCodec + .getX509Certificate(pemEncodedCert).getSerialNumber().toString()); } else { throw new CertificateException("Unable to retrieve " + "certificate chain.");