From 8c766f8354d876f112664d225e449a6d827e469e Mon Sep 17 00:00:00 2001 From: Vivek Ratnavel Subramanian Date: Sat, 17 Aug 2019 15:29:45 -0700 Subject: [PATCH 1/4] HDDS-1946. CertificateClient should not persist keys/certs to ozone.metadata.dir --- .../hadoop/hdds/security/x509/SecurityConfig.java | 5 +++-- .../certificate/client/DNCertificateClient.java | 10 ++++++++-- .../client/DefaultCertificateClient.java | 13 ++++++++----- .../certificate/client/OMCertificateClient.java | 8 ++++++-- .../client/TestCertificateClientInit.java | 6 ++++-- .../client/TestDefaultCertificateClient.java | 13 ++++++++----- .../hadoop/ozone/TestHddsSecureDatanodeInit.java | 5 +++-- 7 files changed, 40 insertions(+), 20 deletions(-) diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/security/x509/SecurityConfig.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/security/x509/SecurityConfig.java index 0e4204f9c44a1..3d3c5c73300d8 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/security/x509/SecurityConfig.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/security/x509/SecurityConfig.java @@ -271,7 +271,7 @@ public Path getKeyLocation(String component) { } /** - * Returns the File path to where keys are stored. + * Returns the File path to where certificates are stored. * * @return path Key location. */ @@ -282,7 +282,8 @@ public Path getCertificateLocation() { } /** - * Returns the File path to where keys are stored with an addition component + * Returns the File path to where certificates are stored with an addition + * component * name inserted in between. * * @param component - Component Name - String. diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/security/x509/certificate/client/DNCertificateClient.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/security/x509/certificate/client/DNCertificateClient.java index 7790d0429197b..c8e1da3b98b73 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/security/x509/certificate/client/DNCertificateClient.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/security/x509/certificate/client/DNCertificateClient.java @@ -25,6 +25,9 @@ import org.slf4j.LoggerFactory; import org.apache.hadoop.hdds.security.x509.SecurityConfig; + +import java.nio.file.Paths; + /** * Certificate client for DataNodes. */ @@ -32,13 +35,16 @@ public class DNCertificateClient extends DefaultCertificateClient { private static final Logger LOG = LoggerFactory.getLogger(DNCertificateClient.class); + + public static final String COMPONENT_NAME = Paths.get("dn").toString(); + public DNCertificateClient(SecurityConfig securityConfig, String certSerialId) { - super(securityConfig, LOG, certSerialId); + super(securityConfig, LOG, certSerialId, COMPONENT_NAME); } public DNCertificateClient(SecurityConfig securityConfig) { - super(securityConfig, LOG, null); + super(securityConfig, LOG, null, COMPONENT_NAME); } /** diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/security/x509/certificate/client/DefaultCertificateClient.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/security/x509/certificate/client/DefaultCertificateClient.java index 8f135745ab7ab..1bc2eedf76374 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/security/x509/certificate/client/DefaultCertificateClient.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/security/x509/certificate/client/DefaultCertificateClient.java @@ -89,16 +89,18 @@ public abstract class DefaultCertificateClient implements CertificateClient { private X509Certificate x509Certificate; private Map certificateMap; private String certSerialId; + private String component; DefaultCertificateClient(SecurityConfig securityConfig, Logger log, - String certSerialId) { + String certSerialId, String component) { Objects.requireNonNull(securityConfig); this.securityConfig = securityConfig; keyCodec = new KeyCodec(securityConfig); this.logger = log; this.certificateMap = new ConcurrentHashMap<>(); this.certSerialId = certSerialId; + this.component = component; loadAllCertificates(); } @@ -108,7 +110,7 @@ public abstract class DefaultCertificateClient implements CertificateClient { * */ private void loadAllCertificates() { // See if certs directory exists in file system. - Path certPath = securityConfig.getCertificateLocation(); + Path certPath = securityConfig.getCertificateLocation(component); if (Files.exists(certPath) && Files.isDirectory(certPath)) { getLogger().info("Loading certificate from location:{}.", certPath); @@ -116,7 +118,7 @@ private void loadAllCertificates() { if (certFiles != null) { CertificateCodec certificateCodec = - new CertificateCodec(securityConfig); + new CertificateCodec(securityConfig, component); for (File file : certFiles) { if (file.isFile()) { try { @@ -477,9 +479,10 @@ public void storeCertificate(String pemEncodedCert, boolean force) @Override public void storeCertificate(String pemEncodedCert, boolean force, boolean caCert) throws CertificateException { - CertificateCodec certificateCodec = new CertificateCodec(securityConfig); + CertificateCodec certificateCodec = new CertificateCodec(securityConfig, + component); try { - Path basePath = securityConfig.getCertificateLocation(); + Path basePath = securityConfig.getCertificateLocation(component); X509Certificate cert = CertificateCodec.getX509Certificate(pemEncodedCert); diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/security/x509/certificate/client/OMCertificateClient.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/security/x509/certificate/client/OMCertificateClient.java index b1f7504026648..a5b24617279d2 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/security/x509/certificate/client/OMCertificateClient.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/security/x509/certificate/client/OMCertificateClient.java @@ -26,6 +26,8 @@ import org.apache.hadoop.hdds.security.x509.SecurityConfig; import org.apache.hadoop.hdds.security.x509.exceptions.CertificateException; +import java.nio.file.Paths; + import static org.apache.hadoop.hdds.security.x509.certificate.client.CertificateClient.InitResponse.FAILURE; import static org.apache.hadoop.hdds.security.x509.certificate.client.CertificateClient.InitResponse.GETCERT; import static org.apache.hadoop.hdds.security.x509.certificate.client.CertificateClient.InitResponse.RECOVER; @@ -39,13 +41,15 @@ public class OMCertificateClient extends DefaultCertificateClient { private static final Logger LOG = LoggerFactory.getLogger(OMCertificateClient.class); + public static final String COMPONENT_NAME = Paths.get("om").toString(); + public OMCertificateClient(SecurityConfig securityConfig, String certSerialId) { - super(securityConfig, LOG, certSerialId); + super(securityConfig, LOG, certSerialId, COMPONENT_NAME); } public OMCertificateClient(SecurityConfig securityConfig) { - super(securityConfig, LOG, null); + super(securityConfig, LOG, null, COMPONENT_NAME); } protected InitResponse handleCase(InitCase init) throws diff --git a/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/security/x509/certificate/client/TestCertificateClientInit.java b/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/security/x509/certificate/client/TestCertificateClientInit.java index 61bcf2155491d..df08edd31c041 100644 --- a/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/security/x509/certificate/client/TestCertificateClientInit.java +++ b/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/security/x509/certificate/client/TestCertificateClientInit.java @@ -139,7 +139,8 @@ public void testInitDatanode() throws Exception { } if (certPresent) { - CertificateCodec codec = new CertificateCodec(securityConfig); + CertificateCodec codec = new CertificateCodec(securityConfig, + DNCertificateClient.COMPONENT_NAME); codec.writeCertificate(new X509CertificateHolder( x509Certificate.getEncoded())); } else { @@ -179,7 +180,8 @@ public void testInitOzoneManager() throws Exception { } if (certPresent) { - CertificateCodec codec = new CertificateCodec(securityConfig); + CertificateCodec codec = new CertificateCodec(securityConfig, + OMCertificateClient.COMPONENT_NAME); codec.writeCertificate(new X509CertificateHolder( x509Certificate.getEncoded())); } else { diff --git a/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/security/x509/certificate/client/TestDefaultCertificateClient.java b/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/security/x509/certificate/client/TestDefaultCertificateClient.java index 11be0ded560e8..fe409e649507c 100644 --- a/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/security/x509/certificate/client/TestDefaultCertificateClient.java +++ b/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/security/x509/certificate/client/TestDefaultCertificateClient.java @@ -284,9 +284,10 @@ public void testCertificateLoadingOnInit() throws Exception { X509Certificate cert1 = generateX509Cert(keyPair); X509Certificate cert2 = generateX509Cert(keyPair); X509Certificate cert3 = generateX509Cert(keyPair); + String component = DNCertificateClient.COMPONENT_NAME; - Path certPath = dnSecurityConfig.getCertificateLocation(); - CertificateCodec codec = new CertificateCodec(dnSecurityConfig); + Path certPath = dnSecurityConfig.getCertificateLocation(component); + CertificateCodec codec = new CertificateCodec(dnSecurityConfig, component); // Certificate not found. LambdaTestUtils.intercept(CertificateException.class, "Error while" + @@ -308,7 +309,7 @@ public void testCertificateLoadingOnInit() throws Exception { codec.writeCertificate(certPath, "3.crt", getPEMEncodedString(cert3), true); - // Re instentiate DN client which will load certificates from filesystem. + // Re instantiate DN client which will load certificates from filesystem. dnCertClient = new DNCertificateClient(dnSecurityConfig, certSerialId); assertNotNull(dnCertClient.getCertificate(cert1.getSerialNumber() @@ -392,11 +393,13 @@ public void testInitCertAndKeypairValidationFailures() throws Exception { FileUtils.deleteQuietly(Paths.get(dnSecurityConfig.getKeyLocation() .toString(), dnSecurityConfig.getCertificateFileName()).toFile()); - CertificateCodec omCertCodec = new CertificateCodec(omSecurityConfig); + CertificateCodec omCertCodec = new CertificateCodec(omSecurityConfig, + OMCertificateClient.COMPONENT_NAME); omCertCodec.writeCertificate(new X509CertificateHolder( x509Certificate.getEncoded())); - CertificateCodec dnCertCodec = new CertificateCodec(dnSecurityConfig); + CertificateCodec dnCertCodec = new CertificateCodec(dnSecurityConfig, + DNCertificateClient.COMPONENT_NAME); dnCertCodec.writeCertificate(new X509CertificateHolder( x509Certificate.getEncoded())); // Check for DN. 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 20d5eef67fe4f..b0563ad9781c6 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 @@ -93,7 +93,8 @@ public static void setUp() throws Exception { service.initializeCertificateClient(conf); return null; }); - certCodec = new CertificateCodec(securityConfig); + certCodec = new CertificateCodec(securityConfig, + DNCertificateClient.COMPONENT_NAME); keyCodec = new KeyCodec(securityConfig); dnLogs.clearOutput(); privateKey = service.getCertificateClient().getPrivateKey(); @@ -120,7 +121,7 @@ public void setUpDNCertClient(){ FileUtils.deleteQuietly(Paths.get(securityConfig.getKeyLocation() .toString(), securityConfig.getPublicKeyFileName()).toFile()); FileUtils.deleteQuietly(Paths.get(securityConfig - .getCertificateLocation().toString(), + .getCertificateLocation(DNCertificateClient.COMPONENT_NAME).toString(), securityConfig.getCertificateFileName()).toFile()); dnLogs.clearOutput(); client = new DNCertificateClient(securityConfig, From 6fd5f9dabecbb67a558c617c0e286d08c855bbb0 Mon Sep 17 00:00:00 2001 From: Vivek Ratnavel Subramanian Date: Sat, 17 Aug 2019 15:39:35 -0700 Subject: [PATCH 2/4] Fixed typo in javadoc --- .../org/apache/hadoop/hdds/security/x509/SecurityConfig.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/security/x509/SecurityConfig.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/security/x509/SecurityConfig.java index 3d3c5c73300d8..71eb633cd8a06 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/security/x509/SecurityConfig.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/security/x509/SecurityConfig.java @@ -273,7 +273,7 @@ public Path getKeyLocation(String component) { /** * Returns the File path to where certificates are stored. * - * @return path Key location. + * @return path Certificate location. */ public Path getCertificateLocation() { Preconditions.checkNotNull(this.metadatDir, "Metadata directory can't be" From 1ad5211ba42f61378ae46ec2257f01a0fbcf070a Mon Sep 17 00:00:00 2001 From: Vivek Ratnavel Subramanian Date: Sun, 18 Aug 2019 01:13:17 -0700 Subject: [PATCH 3/4] Add component name to key locations in a secure cluster --- .../client/DNCertificateClient.java | 4 +- .../client/DefaultCertificateClient.java | 8 +- .../client/OMCertificateClient.java | 4 +- .../client/TestCertificateClientInit.java | 61 ++++++----- .../client/TestDefaultCertificateClient.java | 100 +++++++++++------- .../ozone/TestHddsSecureDatanodeInit.java | 18 ++-- 6 files changed, 111 insertions(+), 84 deletions(-) diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/security/x509/certificate/client/DNCertificateClient.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/security/x509/certificate/client/DNCertificateClient.java index c8e1da3b98b73..76986586d344c 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/security/x509/certificate/client/DNCertificateClient.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/security/x509/certificate/client/DNCertificateClient.java @@ -26,8 +26,6 @@ import org.apache.hadoop.hdds.security.x509.SecurityConfig; -import java.nio.file.Paths; - /** * Certificate client for DataNodes. */ @@ -36,7 +34,7 @@ public class DNCertificateClient extends DefaultCertificateClient { private static final Logger LOG = LoggerFactory.getLogger(DNCertificateClient.class); - public static final String COMPONENT_NAME = Paths.get("dn").toString(); + public static final String COMPONENT_NAME = "dn"; public DNCertificateClient(SecurityConfig securityConfig, String certSerialId) { diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/security/x509/certificate/client/DefaultCertificateClient.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/security/x509/certificate/client/DefaultCertificateClient.java index 1bc2eedf76374..388c5bc099e50 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/security/x509/certificate/client/DefaultCertificateClient.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/security/x509/certificate/client/DefaultCertificateClient.java @@ -96,7 +96,7 @@ public abstract class DefaultCertificateClient implements CertificateClient { String certSerialId, String component) { Objects.requireNonNull(securityConfig); this.securityConfig = securityConfig; - keyCodec = new KeyCodec(securityConfig); + keyCodec = new KeyCodec(securityConfig, component); this.logger = log; this.certificateMap = new ConcurrentHashMap<>(); this.certSerialId = certSerialId; @@ -160,7 +160,7 @@ public PrivateKey getPrivateKey() { return privateKey; } - Path keyPath = securityConfig.getKeyLocation(); + Path keyPath = securityConfig.getKeyLocation(component); if (OzoneSecurityUtil.checkIfFileExist(keyPath, securityConfig.getPrivateKeyFileName())) { try { @@ -184,7 +184,7 @@ public PublicKey getPublicKey() { return publicKey; } - Path keyPath = securityConfig.getKeyLocation(); + Path keyPath = securityConfig.getKeyLocation(component); if (OzoneSecurityUtil.checkIfFileExist(keyPath, securityConfig.getPublicKeyFileName())) { try { @@ -741,7 +741,7 @@ protected boolean validateKeyPair(PublicKey pubKey) * location. * */ protected void bootstrapClientKeys() throws CertificateException { - Path keyPath = securityConfig.getKeyLocation(); + Path keyPath = securityConfig.getKeyLocation(component); if (Files.notExists(keyPath)) { try { Files.createDirectories(keyPath); diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/security/x509/certificate/client/OMCertificateClient.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/security/x509/certificate/client/OMCertificateClient.java index a5b24617279d2..cb3ce7536e1ef 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/security/x509/certificate/client/OMCertificateClient.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/security/x509/certificate/client/OMCertificateClient.java @@ -26,8 +26,6 @@ import org.apache.hadoop.hdds.security.x509.SecurityConfig; import org.apache.hadoop.hdds.security.x509.exceptions.CertificateException; -import java.nio.file.Paths; - import static org.apache.hadoop.hdds.security.x509.certificate.client.CertificateClient.InitResponse.FAILURE; import static org.apache.hadoop.hdds.security.x509.certificate.client.CertificateClient.InitResponse.GETCERT; import static org.apache.hadoop.hdds.security.x509.certificate.client.CertificateClient.InitResponse.RECOVER; @@ -41,7 +39,7 @@ public class OMCertificateClient extends DefaultCertificateClient { private static final Logger LOG = LoggerFactory.getLogger(OMCertificateClient.class); - public static final String COMPONENT_NAME = Paths.get("om").toString(); + public static final String COMPONENT_NAME = "om"; public OMCertificateClient(SecurityConfig securityConfig, String certSerialId) { diff --git a/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/security/x509/certificate/client/TestCertificateClientInit.java b/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/security/x509/certificate/client/TestCertificateClientInit.java index df08edd31c041..dcd9898cbeee0 100644 --- a/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/security/x509/certificate/client/TestCertificateClientInit.java +++ b/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/security/x509/certificate/client/TestCertificateClientInit.java @@ -66,8 +66,11 @@ public class TestCertificateClientInit { private HDDSKeyGenerator keyGenerator; private Path metaDirPath; private SecurityConfig securityConfig; - private KeyCodec keyCodec; + private KeyCodec dnKeyCodec; + private KeyCodec omKeyCodec; private X509Certificate x509Certificate; + private final static String DN_COMPONENT = DNCertificateClient.COMPONENT_NAME; + private final static String OM_COMPONENT = OMCertificateClient.COMPONENT_NAME; @Parameter public boolean pvtKeyPresent; @@ -107,9 +110,11 @@ public void setUp() throws Exception { certSerialId); omCertificateClient = new OMCertificateClient(securityConfig, certSerialId); - keyCodec = new KeyCodec(securityConfig); + dnKeyCodec = new KeyCodec(securityConfig, DN_COMPONENT); + omKeyCodec = new KeyCodec(securityConfig, OM_COMPONENT); - Files.createDirectories(securityConfig.getKeyLocation()); + Files.createDirectories(securityConfig.getKeyLocation(DN_COMPONENT)); + Files.createDirectories(securityConfig.getKeyLocation(OM_COMPONENT)); } @After @@ -123,29 +128,32 @@ public void tearDown() { @Test public void testInitDatanode() throws Exception { if (pvtKeyPresent) { - keyCodec.writePrivateKey(keyPair.getPrivate()); + dnKeyCodec.writePrivateKey(keyPair.getPrivate()); } else { - FileUtils.deleteQuietly(Paths.get(securityConfig.getKeyLocation() - .toString(), securityConfig.getPrivateKeyFileName()).toFile()); + FileUtils.deleteQuietly(Paths.get( + securityConfig.getKeyLocation(DN_COMPONENT).toString(), + securityConfig.getPrivateKeyFileName()).toFile()); } if (pubKeyPresent) { if (dnCertificateClient.getPublicKey() == null) { - keyCodec.writePublicKey(keyPair.getPublic()); + dnKeyCodec.writePublicKey(keyPair.getPublic()); } } else { - FileUtils.deleteQuietly(Paths.get(securityConfig.getKeyLocation() - .toString(), securityConfig.getPublicKeyFileName()).toFile()); + FileUtils.deleteQuietly( + Paths.get(securityConfig.getKeyLocation(DN_COMPONENT).toString(), + securityConfig.getPublicKeyFileName()).toFile()); } if (certPresent) { CertificateCodec codec = new CertificateCodec(securityConfig, - DNCertificateClient.COMPONENT_NAME); + DN_COMPONENT); codec.writeCertificate(new X509CertificateHolder( x509Certificate.getEncoded())); } else { - FileUtils.deleteQuietly(Paths.get(securityConfig.getKeyLocation() - .toString(), securityConfig.getCertificateFileName()).toFile()); + FileUtils.deleteQuietly(Paths.get( + securityConfig.getKeyLocation(DN_COMPONENT).toString(), + securityConfig.getCertificateFileName()).toFile()); } InitResponse response = dnCertificateClient.init(); @@ -153,10 +161,10 @@ public void testInitDatanode() throws Exception { if (!response.equals(FAILURE)) { assertTrue(OzoneSecurityUtil.checkIfFileExist( - securityConfig.getKeyLocation(), + securityConfig.getKeyLocation(DN_COMPONENT), securityConfig.getPrivateKeyFileName())); assertTrue(OzoneSecurityUtil.checkIfFileExist( - securityConfig.getKeyLocation(), + securityConfig.getKeyLocation(DN_COMPONENT), securityConfig.getPublicKeyFileName())); } } @@ -164,29 +172,32 @@ public void testInitDatanode() throws Exception { @Test public void testInitOzoneManager() throws Exception { if (pvtKeyPresent) { - keyCodec.writePrivateKey(keyPair.getPrivate()); + omKeyCodec.writePrivateKey(keyPair.getPrivate()); } else { - FileUtils.deleteQuietly(Paths.get(securityConfig.getKeyLocation() - .toString(), securityConfig.getPrivateKeyFileName()).toFile()); + FileUtils.deleteQuietly(Paths.get( + securityConfig.getKeyLocation(OM_COMPONENT).toString(), + securityConfig.getPrivateKeyFileName()).toFile()); } if (pubKeyPresent) { if (omCertificateClient.getPublicKey() == null) { - keyCodec.writePublicKey(keyPair.getPublic()); + omKeyCodec.writePublicKey(keyPair.getPublic()); } } else { - FileUtils.deleteQuietly(Paths.get(securityConfig.getKeyLocation() - .toString(), securityConfig.getPublicKeyFileName()).toFile()); + FileUtils.deleteQuietly(Paths.get( + securityConfig.getKeyLocation(OM_COMPONENT).toString(), + securityConfig.getPublicKeyFileName()).toFile()); } if (certPresent) { CertificateCodec codec = new CertificateCodec(securityConfig, - OMCertificateClient.COMPONENT_NAME); + OM_COMPONENT); codec.writeCertificate(new X509CertificateHolder( x509Certificate.getEncoded())); } else { - FileUtils.deleteQuietly(Paths.get(securityConfig.getKeyLocation() - .toString(), securityConfig.getCertificateFileName()).toFile()); + FileUtils.deleteQuietly(Paths.get( + securityConfig.getKeyLocation(OM_COMPONENT).toString(), + securityConfig.getCertificateFileName()).toFile()); } InitResponse response = omCertificateClient.init(); @@ -198,10 +209,10 @@ public void testInitOzoneManager() throws Exception { if (!response.equals(FAILURE)) { assertTrue(OzoneSecurityUtil.checkIfFileExist( - securityConfig.getKeyLocation(), + securityConfig.getKeyLocation(OM_COMPONENT), securityConfig.getPrivateKeyFileName())); assertTrue(OzoneSecurityUtil.checkIfFileExist( - securityConfig.getKeyLocation(), + securityConfig.getKeyLocation(OM_COMPONENT), securityConfig.getPublicKeyFileName())); } } diff --git a/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/security/x509/certificate/client/TestDefaultCertificateClient.java b/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/security/x509/certificate/client/TestDefaultCertificateClient.java index fe409e649507c..f389cdb6d22ea 100644 --- a/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/security/x509/certificate/client/TestDefaultCertificateClient.java +++ b/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/security/x509/certificate/client/TestDefaultCertificateClient.java @@ -76,6 +76,8 @@ public class TestDefaultCertificateClient { private SecurityConfig omSecurityConfig; private SecurityConfig dnSecurityConfig; private final static String UTF = "UTF-8"; + private final static String DN_COMPONENT = DNCertificateClient.COMPONENT_NAME; + private final static String OM_COMPONENT = OMCertificateClient.COMPONENT_NAME; private KeyCodec omKeyCodec; private KeyCodec dnKeyCodec; @@ -99,11 +101,11 @@ public void setUp() throws Exception { keyGenerator = new HDDSKeyGenerator(omSecurityConfig); - omKeyCodec = new KeyCodec(omSecurityConfig); - dnKeyCodec = new KeyCodec(dnSecurityConfig); + omKeyCodec = new KeyCodec(omSecurityConfig, OM_COMPONENT); + dnKeyCodec = new KeyCodec(dnSecurityConfig, DN_COMPONENT); - Files.createDirectories(omSecurityConfig.getKeyLocation()); - Files.createDirectories(dnSecurityConfig.getKeyLocation()); + Files.createDirectories(omSecurityConfig.getKeyLocation(OM_COMPONENT)); + Files.createDirectories(dnSecurityConfig.getKeyLocation(DN_COMPONENT)); x509Certificate = generateX509Cert(null); certSerialId = x509Certificate.getSerialNumber().toString(); getCertClient(); @@ -156,14 +158,18 @@ private KeyPair generateKeyPairFiles() throws Exception { } private void cleanupOldKeyPair() { - FileUtils.deleteQuietly(Paths.get(omSecurityConfig.getKeyLocation() - .toString(), omSecurityConfig.getPrivateKeyFileName()).toFile()); - FileUtils.deleteQuietly(Paths.get(omSecurityConfig.getKeyLocation() - .toString(), omSecurityConfig.getPublicKeyFileName()).toFile()); - FileUtils.deleteQuietly(Paths.get(dnSecurityConfig.getKeyLocation() - .toString(), dnSecurityConfig.getPrivateKeyFileName()).toFile()); - FileUtils.deleteQuietly(Paths.get(dnSecurityConfig.getKeyLocation() - .toString(), dnSecurityConfig.getPublicKeyFileName()).toFile()); + FileUtils.deleteQuietly(Paths.get( + omSecurityConfig.getKeyLocation(OM_COMPONENT).toString(), + omSecurityConfig.getPrivateKeyFileName()).toFile()); + FileUtils.deleteQuietly(Paths.get( + omSecurityConfig.getKeyLocation(OM_COMPONENT).toString(), + omSecurityConfig.getPublicKeyFileName()).toFile()); + FileUtils.deleteQuietly(Paths.get( + dnSecurityConfig.getKeyLocation(DN_COMPONENT).toString(), + dnSecurityConfig.getPrivateKeyFileName()).toFile()); + FileUtils.deleteQuietly(Paths.get( + dnSecurityConfig.getKeyLocation(DN_COMPONENT).toString(), + dnSecurityConfig.getPublicKeyFileName()).toFile()); } /** @@ -196,10 +202,12 @@ private X509Certificate generateX509Cert(KeyPair keyPair) throws Exception { @Test public void testSignDataStream() throws Exception { String data = RandomStringUtils.random(100, UTF); - FileUtils.deleteQuietly(Paths.get(omSecurityConfig.getKeyLocation() - .toString(), omSecurityConfig.getPrivateKeyFileName()).toFile()); - FileUtils.deleteQuietly(Paths.get(omSecurityConfig.getKeyLocation() - .toString(), omSecurityConfig.getPublicKeyFileName()).toFile()); + FileUtils.deleteQuietly(Paths.get( + omSecurityConfig.getKeyLocation(OM_COMPONENT).toString(), + omSecurityConfig.getPrivateKeyFileName()).toFile()); + FileUtils.deleteQuietly(Paths.get( + omSecurityConfig.getKeyLocation(OM_COMPONENT).toString(), + omSecurityConfig.getPublicKeyFileName()).toFile()); // Expect error when there is no private key to sign. LambdaTestUtils.intercept(IOException.class, "Error while " + @@ -284,10 +292,10 @@ public void testCertificateLoadingOnInit() throws Exception { X509Certificate cert1 = generateX509Cert(keyPair); X509Certificate cert2 = generateX509Cert(keyPair); X509Certificate cert3 = generateX509Cert(keyPair); - String component = DNCertificateClient.COMPONENT_NAME; - Path certPath = dnSecurityConfig.getCertificateLocation(component); - CertificateCodec codec = new CertificateCodec(dnSecurityConfig, component); + Path certPath = dnSecurityConfig.getCertificateLocation(DN_COMPONENT); + CertificateCodec codec = new CertificateCodec(dnSecurityConfig, + DN_COMPONENT); // Certificate not found. LambdaTestUtils.intercept(CertificateException.class, "Error while" + @@ -353,16 +361,20 @@ public void testInitCertAndKeypairValidationFailures() throws Exception { omClientLog.clearOutput(); // Case 1. Expect failure when keypair validation fails. - FileUtils.deleteQuietly(Paths.get(omSecurityConfig.getKeyLocation() - .toString(), omSecurityConfig.getPrivateKeyFileName()).toFile()); - FileUtils.deleteQuietly(Paths.get(omSecurityConfig.getKeyLocation() - .toString(), omSecurityConfig.getPublicKeyFileName()).toFile()); + FileUtils.deleteQuietly(Paths.get( + omSecurityConfig.getKeyLocation(OM_COMPONENT).toString(), + omSecurityConfig.getPrivateKeyFileName()).toFile()); + FileUtils.deleteQuietly(Paths.get( + omSecurityConfig.getKeyLocation(OM_COMPONENT).toString(), + omSecurityConfig.getPublicKeyFileName()).toFile()); - FileUtils.deleteQuietly(Paths.get(dnSecurityConfig.getKeyLocation() - .toString(), dnSecurityConfig.getPrivateKeyFileName()).toFile()); - FileUtils.deleteQuietly(Paths.get(dnSecurityConfig.getKeyLocation() - .toString(), dnSecurityConfig.getPublicKeyFileName()).toFile()); + FileUtils.deleteQuietly(Paths.get( + dnSecurityConfig.getKeyLocation(DN_COMPONENT).toString(), + dnSecurityConfig.getPrivateKeyFileName()).toFile()); + FileUtils.deleteQuietly(Paths.get( + dnSecurityConfig.getKeyLocation(DN_COMPONENT).toString(), + dnSecurityConfig.getPublicKeyFileName()).toFile()); omKeyCodec.writePrivateKey(keyPair.getPrivate()); omKeyCodec.writePublicKey(keyPair2.getPublic()); @@ -388,18 +400,20 @@ public void testInitCertAndKeypairValidationFailures() throws Exception { // Case 2. Expect failure when certificate is generated from different // private key and keypair validation fails. getCertClient(); - FileUtils.deleteQuietly(Paths.get(omSecurityConfig.getKeyLocation() - .toString(), omSecurityConfig.getCertificateFileName()).toFile()); - FileUtils.deleteQuietly(Paths.get(dnSecurityConfig.getKeyLocation() - .toString(), dnSecurityConfig.getCertificateFileName()).toFile()); + FileUtils.deleteQuietly(Paths.get( + omSecurityConfig.getKeyLocation(OM_COMPONENT).toString(), + omSecurityConfig.getCertificateFileName()).toFile()); + FileUtils.deleteQuietly(Paths.get( + dnSecurityConfig.getKeyLocation(DN_COMPONENT).toString(), + dnSecurityConfig.getCertificateFileName()).toFile()); CertificateCodec omCertCodec = new CertificateCodec(omSecurityConfig, - OMCertificateClient.COMPONENT_NAME); + OM_COMPONENT); omCertCodec.writeCertificate(new X509CertificateHolder( x509Certificate.getEncoded())); CertificateCodec dnCertCodec = new CertificateCodec(dnSecurityConfig, - DNCertificateClient.COMPONENT_NAME); + DN_COMPONENT); dnCertCodec.writeCertificate(new X509CertificateHolder( x509Certificate.getEncoded())); // Check for DN. @@ -419,10 +433,12 @@ public void testInitCertAndKeypairValidationFailures() throws Exception { // private key and certificate validation fails. // Re write the correct public key. - FileUtils.deleteQuietly(Paths.get(omSecurityConfig.getKeyLocation() - .toString(), omSecurityConfig.getPublicKeyFileName()).toFile()); - FileUtils.deleteQuietly(Paths.get(dnSecurityConfig.getKeyLocation() - .toString(), dnSecurityConfig.getPublicKeyFileName()).toFile()); + FileUtils.deleteQuietly(Paths.get( + omSecurityConfig.getKeyLocation(OM_COMPONENT).toString(), + omSecurityConfig.getPublicKeyFileName()).toFile()); + FileUtils.deleteQuietly(Paths.get( + dnSecurityConfig.getKeyLocation(DN_COMPONENT).toString(), + dnSecurityConfig.getPublicKeyFileName()).toFile()); getCertClient(); omKeyCodec.writePublicKey(keyPair.getPublic()); dnKeyCodec.writePublicKey(keyPair.getPublic()); @@ -443,10 +459,12 @@ public void testInitCertAndKeypairValidationFailures() throws Exception { // Case 4. Failure when public key recovery fails. getCertClient(); - FileUtils.deleteQuietly(Paths.get(omSecurityConfig.getKeyLocation() - .toString(), omSecurityConfig.getPublicKeyFileName()).toFile()); - FileUtils.deleteQuietly(Paths.get(dnSecurityConfig.getKeyLocation() - .toString(), dnSecurityConfig.getPublicKeyFileName()).toFile()); + FileUtils.deleteQuietly(Paths.get( + omSecurityConfig.getKeyLocation(OM_COMPONENT).toString(), + omSecurityConfig.getPublicKeyFileName()).toFile()); + FileUtils.deleteQuietly(Paths.get( + dnSecurityConfig.getKeyLocation(DN_COMPONENT).toString(), + dnSecurityConfig.getPublicKeyFileName()).toFile()); // Check for DN. assertEquals(dnCertClient.init(), FAILURE); 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 b0563ad9781c6..04fd3a499aa0a 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 @@ -66,6 +66,7 @@ public class TestHddsSecureDatanodeInit { private static KeyCodec keyCodec; private static CertificateCodec certCodec; private static X509CertificateHolder certHolder; + private final static String DN_COMPONENT = DNCertificateClient.COMPONENT_NAME; @BeforeClass public static void setUp() throws Exception { @@ -93,9 +94,8 @@ public static void setUp() throws Exception { service.initializeCertificateClient(conf); return null; }); - certCodec = new CertificateCodec(securityConfig, - DNCertificateClient.COMPONENT_NAME); - keyCodec = new KeyCodec(securityConfig); + certCodec = new CertificateCodec(securityConfig, DN_COMPONENT); + keyCodec = new KeyCodec(securityConfig, DN_COMPONENT); dnLogs.clearOutput(); privateKey = service.getCertificateClient().getPrivateKey(); publicKey = service.getCertificateClient().getPublicKey(); @@ -116,12 +116,14 @@ public static void tearDown() { @Before public void setUpDNCertClient(){ - FileUtils.deleteQuietly(Paths.get(securityConfig.getKeyLocation() - .toString(), securityConfig.getPrivateKeyFileName()).toFile()); - FileUtils.deleteQuietly(Paths.get(securityConfig.getKeyLocation() - .toString(), securityConfig.getPublicKeyFileName()).toFile()); + FileUtils.deleteQuietly(Paths.get( + securityConfig.getKeyLocation(DN_COMPONENT).toString(), + securityConfig.getPrivateKeyFileName()).toFile()); + FileUtils.deleteQuietly(Paths.get( + securityConfig.getKeyLocation(DN_COMPONENT).toString(), + securityConfig.getPublicKeyFileName()).toFile()); FileUtils.deleteQuietly(Paths.get(securityConfig - .getCertificateLocation(DNCertificateClient.COMPONENT_NAME).toString(), + .getCertificateLocation(DN_COMPONENT).toString(), securityConfig.getCertificateFileName()).toFile()); dnLogs.clearOutput(); client = new DNCertificateClient(securityConfig, From 54a26383ffcef04061eb1aef1bc1cd28db16b14c Mon Sep 17 00:00:00 2001 From: Vivek Ratnavel Subramanian Date: Thu, 22 Aug 2019 14:58:29 -0700 Subject: [PATCH 4/4] Remove getKeyLocation() with no arguments --- .../hadoop/hdds/scm/XceiverClientGrpc.java | 7 +- .../hdds/security/x509/SecurityConfig.java | 99 ++++++++++++------- .../certificate/utils/CertificateCodec.java | 25 +---- .../hdds/security/x509/keys/KeyCodec.java | 26 +---- .../utils/TestCertificateCodec.java | 10 +- .../hdds/security/x509/keys/TestKeyCodec.java | 18 ++-- .../transport/server/XceiverServerGrpc.java | 8 +- .../hadoop/ozone/TestSecureOzoneCluster.java | 4 +- .../ozone/om/TestSecureOzoneManager.java | 14 +-- 9 files changed, 106 insertions(+), 105 deletions(-) diff --git a/hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/XceiverClientGrpc.java b/hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/XceiverClientGrpc.java index b51b221f4d163..d8daaa7497fbd 100644 --- a/hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/XceiverClientGrpc.java +++ b/hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/XceiverClientGrpc.java @@ -70,6 +70,7 @@ */ public class XceiverClientGrpc extends XceiverClientSpi { static final Logger LOG = LoggerFactory.getLogger(XceiverClientGrpc.class); + private static final String COMPONENT = "dn"; private final Pipeline pipeline; private final Configuration config; private Map asyncStubs; @@ -150,9 +151,9 @@ private void connectToDatanode(DatanodeDetails dn, String encodedToken) .intercept(new ClientCredentialInterceptor(userName, encodedToken), new GrpcClientInterceptor()); if (secConfig.isGrpcTlsEnabled()) { - File trustCertCollectionFile = secConfig.getTrustStoreFile(); - File privateKeyFile = secConfig.getClientPrivateKeyFile(); - File clientCertChainFile = secConfig.getClientCertChainFile(); + File trustCertCollectionFile = secConfig.getTrustStoreFile(COMPONENT); + File privateKeyFile = secConfig.getClientPrivateKeyFile(COMPONENT); + File clientCertChainFile = secConfig.getClientCertChainFile(COMPONENT); SslContextBuilder sslContextBuilder = GrpcSslContexts.forClient(); if (trustCertCollectionFile != null) { diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/security/x509/SecurityConfig.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/security/x509/SecurityConfig.java index 71eb633cd8a06..969f7bb819702 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/security/x509/SecurityConfig.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/security/x509/SecurityConfig.java @@ -20,6 +20,7 @@ package org.apache.hadoop.hdds.security.x509; import com.google.common.base.Preconditions; +import org.apache.commons.lang3.StringUtils; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.ozone.OzoneConfigKeys; import org.apache.ratis.thirdparty.io.netty.handler.ssl.SslProvider; @@ -246,23 +247,12 @@ public String getPrivateKeyFileName() { return privateKeyFileName; } - /** - * Returns the File path to where keys are stored. - * - * @return path Key location. - */ - public Path getKeyLocation() { - Preconditions.checkNotNull(this.metadatDir, "Metadata directory can't be" - + " null. Please check configs."); - return Paths.get(metadatDir, keyDir); - } - /** * Returns the File path to where keys are stored with an additional component * name inserted in between. * * @param component - Component Name - String. - * @return Path location. + * @return Path Key location. */ public Path getKeyLocation(String component) { Preconditions.checkNotNull(this.metadatDir, "Metadata directory can't be" @@ -270,17 +260,6 @@ public Path getKeyLocation(String component) { return Paths.get(metadatDir, component, keyDir); } - /** - * Returns the File path to where certificates are stored. - * - * @return path Certificate location. - */ - public Path getCertificateLocation() { - Preconditions.checkNotNull(this.metadatDir, "Metadata directory can't be" - + " null. Please check configs."); - return Paths.get(metadatDir, certificateDir); - } - /** * Returns the File path to where certificates are stored with an addition * component @@ -380,14 +359,35 @@ public boolean isGrpcMutualTlsRequired() { return this.grpcMutualTlsRequired; } + /** + * Returns the TLS-enabled gRPC client private key file(Only needed for mutual + * authentication) for the given component. + * @param component name of the component. + * @return the TLS-enabled gRPC client private key file. + */ + public File getClientPrivateKeyFile(String component) { + return Paths.get(getKeyLocation(component).toString(), + "client." + privateKeyFileName).toFile(); + } + /** * Returns the TLS-enabled gRPC client private key file(Only needed for mutual * authentication). * @return the TLS-enabled gRPC client private key file. */ public File getClientPrivateKeyFile() { - return Paths.get(getKeyLocation().toString(), - "client." + privateKeyFileName).toFile(); + return getClientPrivateKeyFile(StringUtils.EMPTY); + } + + /** + * Returns the TLS-enabled gRPC server private key file for the given + * component. + * @param component name of the component. + * @return the TLS-enabled gRPC server private key file. + */ + public File getServerPrivateKeyFile(String component) { + return Paths.get(getKeyLocation(component).toString(), + "server." + privateKeyFileName).toFile(); } /** @@ -395,8 +395,19 @@ public File getClientPrivateKeyFile() { * @return the TLS-enabled gRPC server private key file. */ public File getServerPrivateKeyFile() { - return Paths.get(getKeyLocation().toString(), - "server." + privateKeyFileName).toFile(); + return getServerPrivateKeyFile(StringUtils.EMPTY); + } + + /** + * Get the trusted CA certificate file for the given component. (CA + * certificate) + * @param component name of the component. + * @return the trusted CA certificate. + */ + public File getTrustStoreFile(String component) { + return Paths.get(getKeyLocation(component).toString(), + trustStoreFileName). + toFile(); } /** @@ -404,7 +415,19 @@ public File getServerPrivateKeyFile() { * @return the trusted CA certificate. */ public File getTrustStoreFile() { - return Paths.get(getKeyLocation().toString(), trustStoreFileName). + return getTrustStoreFile(StringUtils.EMPTY); + } + + /** + * Get the TLS-enabled gRPC Client certificate chain file for the given + * component (only needed for + * mutual authentication). + * @param component name of the component. + * @return the TLS-enabled gRPC Server certificate chain file. + */ + public File getClientCertChainFile(String component) { + return Paths.get(getKeyLocation(component).toString(), + clientCertChainFileName). toFile(); } @@ -414,7 +437,18 @@ public File getTrustStoreFile() { * @return the TLS-enabled gRPC Server certificate chain file. */ public File getClientCertChainFile() { - return Paths.get(getKeyLocation().toString(), clientCertChainFileName). + return getClientCertChainFile(StringUtils.EMPTY); + } + + /** + * Get the TLS-enabled gRPC Server certificate chain file for the given + * component. + * @param component name of the component. + * @return the TLS-enabled gRPC Server certificate chain file. + */ + public File getServerCertChainFile(String component) { + return Paths.get(getKeyLocation(component).toString(), + serverCertChainFileName). toFile(); } @@ -423,8 +457,7 @@ public File getClientCertChainFile() { * @return the TLS-enabled gRPC Server certificate chain file. */ public File getServerCertChainFile() { - return Paths.get(getKeyLocation().toString(), serverCertChainFileName). - toFile(); + return getServerCertChainFile(StringUtils.EMPTY); } /** @@ -438,7 +471,7 @@ public SslProvider getGrpcSslProvider() { /** * Return true if using test certificates with authority as localhost. - * This should be used only for unit test where certifiates are generated + * This should be used only for unit test where certificates are generated * by openssl with localhost as DN and should never use for production as it * will bypass the hostname/ip matching verification. * @return true if using test certificates. @@ -465,7 +498,7 @@ private Provider initSecurityProvider(String providerName) { /** * Returns max date for which S3 tokens will be valid. - * */ + */ public long getS3TokenMaxDate() { return getConfiguration().getTimeDuration( OzoneConfigKeys.OZONE_S3_TOKEN_MAX_LIFETIME_KEY, diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/security/x509/certificate/utils/CertificateCodec.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/security/x509/certificate/utils/CertificateCodec.java index 90d5325fb5f93..2c8721b199bd7 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/security/x509/certificate/utils/CertificateCodec.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/security/x509/certificate/utils/CertificateCodec.java @@ -19,9 +19,7 @@ package org.apache.hadoop.hdds.security.x509.certificate.utils; -import com.google.common.base.Preconditions; import org.apache.commons.io.IOUtils; -import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.hdds.security.exception.SCMSecurityException; import org.apache.hadoop.hdds.security.x509.SecurityConfig; import org.bouncycastle.cert.X509CertificateHolder; @@ -70,7 +68,7 @@ public class CertificateCodec { Stream.of(OWNER_READ, OWNER_WRITE, OWNER_EXECUTE) .collect(Collectors.toSet()); /** - * Creates an CertificateCodec. + * Creates a CertificateCodec with component name. * * @param config - Security Config. * @param component - Component String. @@ -80,27 +78,6 @@ public CertificateCodec(SecurityConfig config, String component) { this.location = securityConfig.getCertificateLocation(component); } - /** - * Creates an CertificateCodec. - * - * @param config - Security Config. - */ - public CertificateCodec(SecurityConfig config) { - this.securityConfig = config; - this.location = securityConfig.getCertificateLocation(); - } - - /** - * Creates an CertificateCodec. - * - * @param configuration - Configuration - */ - public CertificateCodec(Configuration configuration) { - Preconditions.checkNotNull(configuration, "Config cannot be null"); - this.securityConfig = new SecurityConfig(configuration); - this.location = securityConfig.getCertificateLocation(); - } - /** * Returns a X509 Certificate from the Certificate Holder. * diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/security/x509/keys/KeyCodec.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/security/x509/keys/KeyCodec.java index a5ebdaefffc05..82873b06c7146 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/security/x509/keys/KeyCodec.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/security/x509/keys/KeyCodec.java @@ -22,7 +22,6 @@ import com.google.common.base.Preconditions; import org.apache.commons.io.FileUtils; import org.apache.commons.io.output.FileWriterWithEncoding; -import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.hdds.security.x509.SecurityConfig; import org.bouncycastle.util.io.pem.PemObject; import org.bouncycastle.util.io.pem.PemReader; @@ -76,7 +75,7 @@ public class KeyCodec { private Supplier isPosixFileSystem; /** - * Creates an KeyCodec. + * Creates a KeyCodec with component name. * * @param config - Security Config. * @param component - Component String. @@ -87,29 +86,6 @@ public KeyCodec(SecurityConfig config, String component) { this.location = securityConfig.getKeyLocation(component); } - /** - * Creates an KeyCodec. - * - * @param config - Security Config. - */ - public KeyCodec(SecurityConfig config) { - this.securityConfig = config; - isPosixFileSystem = KeyCodec::isPosix; - this.location = securityConfig.getKeyLocation(); - } - - /** - * Creates an HDDS Key Writer. - * - * @param configuration - Configuration - */ - public KeyCodec(Configuration configuration) { - Preconditions.checkNotNull(configuration, "Config cannot be null"); - this.securityConfig = new SecurityConfig(configuration); - isPosixFileSystem = KeyCodec::isPosix; - this.location = securityConfig.getKeyLocation(); - } - /** * Checks if File System supports posix style security permissions. * diff --git a/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/security/x509/certificate/utils/TestCertificateCodec.java b/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/security/x509/certificate/utils/TestCertificateCodec.java index 9ac956ffe82b8..ded52068395ff 100644 --- a/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/security/x509/certificate/utils/TestCertificateCodec.java +++ b/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/security/x509/certificate/utils/TestCertificateCodec.java @@ -22,6 +22,7 @@ import org.apache.commons.lang3.RandomStringUtils; import org.apache.hadoop.hdds.conf.OzoneConfiguration; import org.apache.hadoop.hdds.security.exception.SCMSecurityException; +import org.apache.hadoop.hdds.security.x509.SecurityConfig; import org.apache.hadoop.hdds.security.x509.certificates.utils.SelfSignedCertificate; import org.apache.hadoop.hdds.security.x509.keys.HDDSKeyGenerator; import org.bouncycastle.cert.X509CertificateHolder; @@ -50,12 +51,15 @@ */ public class TestCertificateCodec { private static OzoneConfiguration conf = new OzoneConfiguration(); + private static final String COMPONENT = "test"; + private SecurityConfig securityConfig; @Rule public TemporaryFolder temporaryFolder = new TemporaryFolder(); @Before public void init() throws IOException { conf.set(OZONE_METADATA_DIRS, temporaryFolder.newFolder().toString()); + securityConfig = new SecurityConfig(conf); } /** @@ -88,7 +92,7 @@ public void testGetPEMEncodedString() .setKey(keyGenerator.generateKey()) .makeCA() .build(); - CertificateCodec codec = new CertificateCodec(conf); + CertificateCodec codec = new CertificateCodec(securityConfig, COMPONENT); String pemString = codec.getPEMEncodedString(cert); assertTrue(pemString.startsWith(CertificateCodec.BEGIN_CERT)); assertTrue(pemString.endsWith(CertificateCodec.END_CERT + "\n")); @@ -131,7 +135,7 @@ public void testwriteCertificate() throws NoSuchProviderException, .setKey(keyGenerator.generateKey()) .makeCA() .build(); - CertificateCodec codec = new CertificateCodec(conf); + CertificateCodec codec = new CertificateCodec(securityConfig, COMPONENT); String pemString = codec.getPEMEncodedString(cert); File basePath = temporaryFolder.newFolder(); if (!basePath.exists()) { @@ -172,7 +176,7 @@ public void testwriteCertificateDefault() .setKey(keyGenerator.generateKey()) .makeCA() .build(); - CertificateCodec codec = new CertificateCodec(conf); + CertificateCodec codec = new CertificateCodec(securityConfig, COMPONENT); codec.writeCertificate(cert); X509CertificateHolder certHolder = codec.readCertificate(); assertNotNull(certHolder); diff --git a/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/security/x509/keys/TestKeyCodec.java b/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/security/x509/keys/TestKeyCodec.java index d3e13d2383c61..d82b02f43c5dc 100644 --- a/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/security/x509/keys/TestKeyCodec.java +++ b/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/security/x509/keys/TestKeyCodec.java @@ -57,6 +57,8 @@ public class TestKeyCodec { @Rule public TemporaryFolder temporaryFolder = new TemporaryFolder(); private OzoneConfiguration configuration; + private SecurityConfig securityConfig; + private String component; private HDDSKeyGenerator keyGenerator; private String prefix; @@ -66,6 +68,8 @@ public void init() throws IOException { prefix = temporaryFolder.newFolder().toString(); configuration.set(HDDS_METADATA_DIR_NAME, prefix); keyGenerator = new HDDSKeyGenerator(configuration); + securityConfig = new SecurityConfig(configuration); + component = "test_component"; } /** @@ -83,11 +87,11 @@ public void testWriteKey() throws NoSuchProviderException, NoSuchAlgorithmException, IOException, InvalidKeySpecException { KeyPair keys = keyGenerator.generateKey(); - KeyCodec pemWriter = new KeyCodec(configuration); + KeyCodec pemWriter = new KeyCodec(securityConfig, component); pemWriter.writeKey(keys); // Assert that locations have been created. - Path keyLocation = pemWriter.getSecurityConfig().getKeyLocation(); + Path keyLocation = pemWriter.getSecurityConfig().getKeyLocation(component); Assert.assertTrue(keyLocation.toFile().exists()); // Assert that locations are created in the locations that we specified @@ -172,7 +176,7 @@ public void testWriteKey() public void testReWriteKey() throws Exception { KeyPair kp = keyGenerator.generateKey(); - KeyCodec pemWriter = new KeyCodec(configuration); + KeyCodec pemWriter = new KeyCodec(securityConfig, component); SecurityConfig secConfig = pemWriter.getSecurityConfig(); pemWriter.writeKey(kp); @@ -181,13 +185,13 @@ public void testReWriteKey() .intercept(IOException.class, "Private Key file already exists.", () -> pemWriter.writeKey(kp)); FileUtils.deleteQuietly(Paths.get( - secConfig.getKeyLocation().toString() + "/" + secConfig + secConfig.getKeyLocation(component).toString() + "/" + secConfig .getPrivateKeyFileName()).toFile()); LambdaTestUtils .intercept(IOException.class, "Public Key file already exists.", () -> pemWriter.writeKey(kp)); FileUtils.deleteQuietly(Paths.get( - secConfig.getKeyLocation().toString() + "/" + secConfig + secConfig.getKeyLocation(component).toString() + "/" + secConfig .getPublicKeyFileName()).toFile()); // Should succeed now as both public and private key are deleted. @@ -206,7 +210,7 @@ public void testReWriteKey() public void testWriteKeyInNonPosixFS() throws Exception { KeyPair kp = keyGenerator.generateKey(); - KeyCodec pemWriter = new KeyCodec(configuration); + KeyCodec pemWriter = new KeyCodec(securityConfig, component); pemWriter.setIsPosixFileSystem(() -> false); // Assert key rewrite fails in non Posix file system. @@ -221,7 +225,7 @@ public void testReadWritePublicKeywithoutArgs() InvalidKeySpecException { KeyPair kp = keyGenerator.generateKey(); - KeyCodec keycodec = new KeyCodec(configuration); + KeyCodec keycodec = new KeyCodec(securityConfig, component); keycodec.writeKey(kp); PublicKey pubKey = keycodec.readPublicKey(); diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/transport/server/XceiverServerGrpc.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/transport/server/XceiverServerGrpc.java index 78c941e4bca05..23fa2d059ae4c 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/transport/server/XceiverServerGrpc.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/transport/server/XceiverServerGrpc.java @@ -64,6 +64,7 @@ public final class XceiverServerGrpc extends XceiverServer { private static final Logger LOG = LoggerFactory.getLogger(XceiverServerGrpc.class); + private static final String COMPONENT = "dn"; private int port; private UUID id; private Server server; @@ -111,11 +112,12 @@ public XceiverServerGrpc(DatanodeDetails datanodeDetails, Configuration conf, } if (getSecConfig().isGrpcTlsEnabled()) { - File privateKeyFilePath = getSecurityConfig().getServerPrivateKeyFile(); + File privateKeyFilePath = + getSecurityConfig().getServerPrivateKeyFile(COMPONENT); File serverCertChainFilePath = - getSecurityConfig().getServerCertChainFile(); + getSecurityConfig().getServerCertChainFile(COMPONENT); File clientCertChainFilePath = - getSecurityConfig().getClientCertChainFile(); + getSecurityConfig().getClientCertChainFile(COMPONENT); try { SslContextBuilder sslClientContextBuilder = SslContextBuilder.forServer( serverCertChainFilePath, privateKeyFilePath); 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 853b6a2c309cd..207f0ff73197b 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 @@ -41,6 +41,7 @@ import org.apache.hadoop.hdds.scm.client.HddsClientUtils; import org.apache.hadoop.hdds.scm.server.SCMStorageConfig; import org.apache.hadoop.hdds.scm.server.StorageContainerManager; +import org.apache.hadoop.hdds.security.x509.SecurityConfig; import org.apache.hadoop.hdds.security.x509.certificate.utils.CertificateCodec; import org.apache.hadoop.hdds.security.x509.keys.HDDSKeyGenerator; import org.apache.hadoop.hdds.security.x509.keys.KeyCodec; @@ -113,6 +114,7 @@ public final class TestSecureOzoneCluster { private static final String TEST_USER = "testUgiUser@EXAMPLE.COM"; + private static final String COMPONENT = "test"; private static final int CLIENT_TIMEOUT = 2 * 1000; private Logger logger = LoggerFactory .getLogger(TestSecureOzoneCluster.class); @@ -558,7 +560,7 @@ public Void run() throws Exception { private void generateKeyPair(OzoneConfiguration config) throws Exception { HDDSKeyGenerator keyGenerator = new HDDSKeyGenerator(conf); keyPair = keyGenerator.generateKey(); - KeyCodec pemWriter = new KeyCodec(config); + KeyCodec pemWriter = new KeyCodec(new SecurityConfig(config), COMPONENT); pemWriter.writeKey(keyPair, true); } diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestSecureOzoneManager.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestSecureOzoneManager.java index 888a650773e70..728d170cd7faa 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestSecureOzoneManager.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestSecureOzoneManager.java @@ -60,6 +60,7 @@ */ public class TestSecureOzoneManager { + private static final String COMPONENT = "om"; private MiniOzoneCluster cluster = null; private OzoneConfiguration conf; private String clusterId; @@ -151,7 +152,7 @@ public void testSecureOmInitFailures() throws Exception { // Case 3: When public key as well as certificate is missing. client = new OMCertificateClient(securityConfig); - FileUtils.deleteQuietly(Paths.get(securityConfig.getKeyLocation() + FileUtils.deleteQuietly(Paths.get(securityConfig.getKeyLocation(COMPONENT) .toString(), securityConfig.getPublicKeyFileName()).toFile()); LambdaTestUtils.intercept(RuntimeException.class, " OM security" + " initialization failed", @@ -164,9 +165,9 @@ public void testSecureOmInitFailures() throws Exception { // Case 4: When private key and certificate is missing. client = new OMCertificateClient(securityConfig); - FileUtils.deleteQuietly(Paths.get(securityConfig.getKeyLocation() + FileUtils.deleteQuietly(Paths.get(securityConfig.getKeyLocation(COMPONENT) .toString(), securityConfig.getPrivateKeyFileName()).toFile()); - KeyCodec keyCodec = new KeyCodec(securityConfig); + KeyCodec keyCodec = new KeyCodec(securityConfig, COMPONENT); keyCodec.writePublicKey(publicKey); LambdaTestUtils.intercept(RuntimeException.class, " OM security" + " initialization failed", @@ -178,9 +179,10 @@ public void testSecureOmInitFailures() throws Exception { omLogs.clearOutput(); // Case 5: When only certificate is present. - FileUtils.deleteQuietly(Paths.get(securityConfig.getKeyLocation() + FileUtils.deleteQuietly(Paths.get(securityConfig.getKeyLocation(COMPONENT) .toString(), securityConfig.getPublicKeyFileName()).toFile()); - CertificateCodec certCodec = new CertificateCodec(securityConfig); + CertificateCodec certCodec = + new CertificateCodec(securityConfig, COMPONENT); X509Certificate x509Certificate = KeyStoreTestUtil.generateCertificate( "CN=Test", new KeyPair(publicKey, privateKey), 10, securityConfig.getSignatureAlgo()); @@ -201,7 +203,7 @@ public void testSecureOmInitFailures() throws Exception { // Case 6: When private key and certificate is present. client = new OMCertificateClient(securityConfig, x509Certificate.getSerialNumber().toString()); - FileUtils.deleteQuietly(Paths.get(securityConfig.getKeyLocation() + FileUtils.deleteQuietly(Paths.get(securityConfig.getKeyLocation(COMPONENT) .toString(), securityConfig.getPublicKeyFileName()).toFile()); keyCodec.writePrivateKey(privateKey); OzoneManager.initializeSecurity(conf, omStorage);