diff --git a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/security/x509/certificate/authority/profile/DefaultProfile.java b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/security/x509/certificate/authority/profile/DefaultProfile.java index b4c7ad5d4e16..7749ac99dd8c 100644 --- a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/security/x509/certificate/authority/profile/DefaultProfile.java +++ b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/security/x509/certificate/authority/profile/DefaultProfile.java @@ -34,7 +34,7 @@ import java.util.stream.Stream; import org.apache.commons.codec.DecoderException; import org.apache.commons.codec.binary.Hex; -import org.apache.commons.validator.routines.DomainValidator; +import org.apache.hadoop.hdds.security.x509.certificate.utils.DnsNames; import org.bouncycastle.asn1.ASN1ObjectIdentifier; import org.bouncycastle.asn1.x500.RDN; import org.bouncycastle.asn1.x509.ExtendedKeyUsage; @@ -233,7 +233,7 @@ public boolean validateGeneralName(int type, String value) { return false; } case GeneralName.dNSName: - return DomainValidator.getInstance().isValid(value); + return DnsNames.isValidDnsName(value); case GeneralName.otherName: // for other name it's a general string, nothing to validate return true; diff --git a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/security/x509/certificate/utils/CertificateSignRequest.java b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/security/x509/certificate/utils/CertificateSignRequest.java index a3933e22df49..74206f716003 100644 --- a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/security/x509/certificate/utils/CertificateSignRequest.java +++ b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/security/x509/certificate/utils/CertificateSignRequest.java @@ -26,13 +26,13 @@ import java.io.StringReader; import java.io.StringWriter; import java.net.InetAddress; +import java.net.UnknownHostException; import java.security.KeyPair; import java.util.ArrayList; import java.util.List; import java.util.Objects; import java.util.Optional; import org.apache.commons.lang3.StringUtils; -import org.apache.commons.validator.routines.DomainValidator; import org.apache.hadoop.hdds.security.SecurityConfig; import org.apache.hadoop.hdds.security.exception.SCMSecurityException; import org.apache.hadoop.hdds.security.x509.exception.CertificateException; @@ -282,6 +282,19 @@ public boolean hasDnsName() { return false; } + private boolean hasDnsName(String candidate) { + if (altNames == null) { + return false; + } + for (GeneralName name : altNames) { + if (name.getTagNo() == GeneralName.dNSName + && name.getName().toString().equalsIgnoreCase(candidate)) { + return true; + } + } + return false; + } + // IP address is subject to change which is optional for now. public CertificateSignRequest.Builder addIpAddress(String ip) { Objects.requireNonNull(ip, "Ip address cannot be null"); @@ -292,10 +305,9 @@ public CertificateSignRequest.Builder addIpAddress(String ip) { public CertificateSignRequest.Builder addInetAddresses() throws CertificateException { try { - DomainValidator validator = DomainValidator.getInstance(); // Add all valid ips. List inetAddresses = getValidInetsForCurrentHost(); - this.addInetAddresses(inetAddresses, validator); + this.addInetAddresses(inetAddresses); } catch (IOException e) { throw new CertificateException("Error while getting Inet addresses " + "for the CSR builder", e, CSR_ERROR); @@ -304,18 +316,36 @@ public CertificateSignRequest.Builder addInetAddresses() } public CertificateSignRequest.Builder addInetAddresses( - List addresses, - DomainValidator validator) { + List addresses) { // Add all valid ips. addresses.forEach( ip -> { this.addIpAddress(ip.getHostAddress()); - if (validator.isValid(ip.getCanonicalHostName())) { - this.addDnsName(ip.getCanonicalHostName()); + Optional dnsName = DnsNames.toDnsSanValue(ip.getCanonicalHostName()); + if (dnsName.isPresent()) { + if (!hasDnsName(dnsName.get())) { + this.addDnsName(dnsName.get()); + } } else { - LOG.error("Invalid domain {}", ip.getCanonicalHostName()); + LOG.warn("Rejected DNS SAN candidate '{}': not a valid RFC 1123 DNS name", + ip.getCanonicalHostName()); } }); + + if (!hasDnsName()) { + Optional dnsName; + try { + dnsName = DnsNames.toDnsSanValue(InetAddress.getLocalHost().getCanonicalHostName()); + } catch (UnknownHostException e) { + dnsName = Optional.empty(); + } + if (dnsName.isPresent()) { + this.addDnsName(dnsName.get()); + } else { + LOG.warn("Certificate will have no DNS SAN; by-name TLS connections " + + "to this node will fail"); + } + } return this; } diff --git a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/security/x509/certificate/utils/DnsNames.java b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/security/x509/certificate/utils/DnsNames.java new file mode 100644 index 000000000000..f216af27c5a0 --- /dev/null +++ b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/security/x509/certificate/utils/DnsNames.java @@ -0,0 +1,107 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.hadoop.hdds.security.x509.certificate.utils; + +import java.net.IDN; +import java.util.Optional; +import org.apache.commons.validator.routines.InetAddressValidator; + +/** + * Shared helper for validating and normalizing RFC 1123 DNS names used as + * certificate Subject Alternative Names. + */ +public final class DnsNames { + + private static final int MAX_NAME_LENGTH = 253; + private static final int MAX_LABEL_LENGTH = 63; + + private DnsNames() { + } + + /** + * Normalizes a candidate DNS name for use as a certificate SAN value. + * Strips at most one trailing '.', converts it to its ASCII/A-label form + * via IDN, and validates the result with {@link #isValidDnsName(String)}. + * + * @param candidate the raw candidate DNS name + * @return the normalized DNS name, or {@link Optional#empty()} if the + * candidate is null, empty, or not a valid DNS name + */ + public static Optional toDnsSanValue(String candidate) { + if (candidate == null || candidate.isEmpty()) { + return Optional.empty(); + } + + String stripped = candidate.endsWith(".") + ? candidate.substring(0, candidate.length() - 1) + : candidate; + + String ascii; + try { + ascii = IDN.toASCII(stripped, IDN.ALLOW_UNASSIGNED); + } catch (IllegalArgumentException e) { + return Optional.empty(); + } + + return isValidDnsName(ascii) ? Optional.of(ascii) : Optional.empty(); + } + + /** + * Validates that the given value is a syntactically valid RFC 1123 DNS + * name for use as a certificate Subject Alternative Name. Does not perform + * IDN conversion or trailing-dot stripping. + * + * @param value the DNS name to validate + * @return true iff the value is a valid RFC 1123 DNS name + */ + public static boolean isValidDnsName(String value) { + if (value == null || value.isEmpty() || value.length() > MAX_NAME_LENGTH) { + return false; + } + + if (InetAddressValidator.getInstance().isValid(value)) { + return false; + } + + String[] labels = value.split("\\.", -1); + for (String label : labels) { + if (!isValidLabel(label)) { + return false; + } + } + return true; + } + + private static boolean isValidLabel(String label) { + int length = label.length(); + if (length < 1 || length > MAX_LABEL_LENGTH) { + return false; + } + if (label.charAt(0) == '-' || label.charAt(length - 1) == '-') { + return false; + } + for (int i = 0; i < length; i++) { + char c = label.charAt(i); + boolean isAlphaNumeric = (c >= 'A' && c <= 'Z') || (c >= 'a' && c <= 'z') || (c >= '0' && c <= '9'); + if (!isAlphaNumeric && c != '-') { + return false; + } + } + return true; + } +} diff --git a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/security/x509/certificate/utils/SelfSignedCertificate.java b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/security/x509/certificate/utils/SelfSignedCertificate.java index 1d9cd7d58477..e56b4466e9e7 100644 --- a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/security/x509/certificate/utils/SelfSignedCertificate.java +++ b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/security/x509/certificate/utils/SelfSignedCertificate.java @@ -26,6 +26,7 @@ import java.io.IOException; import java.math.BigInteger; import java.net.InetAddress; +import java.net.UnknownHostException; import java.security.KeyPair; import java.security.cert.X509Certificate; import java.time.Duration; @@ -34,8 +35,8 @@ import java.util.Date; import java.util.List; import java.util.Objects; +import java.util.Optional; import org.apache.commons.lang3.StringUtils; -import org.apache.commons.validator.routines.DomainValidator; import org.apache.hadoop.hdds.security.SecurityConfig; import org.apache.hadoop.hdds.security.exception.SCMSecurityException; import org.apache.hadoop.hdds.security.x509.exception.CertificateException; @@ -219,10 +220,9 @@ public Builder makeCA(BigInteger serialId) { public Builder addInetAddresses() throws CertificateException { try { - DomainValidator validator = DomainValidator.getInstance(); // Add all valid ips. List inetAddresses = getValidInetsForCurrentHost(); - this.addInetAddresses(inetAddresses, validator); + this.addInetAddresses(inetAddresses); } catch (IOException e) { throw new CertificateException("Error while getting Inet addresses " + "for the CSR builder", e, CSR_ERROR); @@ -230,20 +230,63 @@ public Builder addInetAddresses() throws CertificateException { return this; } - public Builder addInetAddresses(List addresses, - DomainValidator validator) { + public Builder addInetAddresses(List addresses) { addresses.forEach( ip -> { this.addIpAddress(ip.getHostAddress()); - if (validator.isValid(ip.getCanonicalHostName())) { - this.addDnsName(ip.getCanonicalHostName()); + Optional dnsName = DnsNames.toDnsSanValue(ip.getCanonicalHostName()); + if (dnsName.isPresent()) { + if (!hasDnsName(dnsName.get())) { + this.addDnsName(dnsName.get()); + } } else { - LOG.error("Invalid domain {}", ip.getCanonicalHostName()); + LOG.warn("Rejected DNS SAN candidate '{}': not a valid RFC 1123 DNS name", + ip.getCanonicalHostName()); } }); + + if (!hasDnsName()) { + Optional dnsName; + try { + dnsName = DnsNames.toDnsSanValue(InetAddress.getLocalHost().getCanonicalHostName()); + } catch (UnknownHostException e) { + dnsName = Optional.empty(); + } + if (dnsName.isPresent()) { + this.addDnsName(dnsName.get()); + } else { + LOG.warn("Certificate will have no DNS SAN; by-name TLS connections " + + "to this node will fail"); + } + } return this; } + private boolean hasDnsName() { + if (altNames == null) { + return false; + } + for (GeneralName name : altNames) { + if (name.getTagNo() == GeneralName.dNSName) { + return true; + } + } + return false; + } + + private boolean hasDnsName(String candidate) { + if (altNames == null) { + return false; + } + for (GeneralName name : altNames) { + if (name.getTagNo() == GeneralName.dNSName + && name.getName().toString().equalsIgnoreCase(candidate)) { + return true; + } + } + return false; + } + // Support SAN extension with DNS and RFC822 Name // other name type will be added as needed. public Builder addDnsName(String dnsName) { diff --git a/hadoop-hdds/framework/src/test/java/org/apache/hadoop/hdds/security/x509/certificate/authority/TestDefaultCAServer.java b/hadoop-hdds/framework/src/test/java/org/apache/hadoop/hdds/security/x509/certificate/authority/TestDefaultCAServer.java index 5f8b72dc3a10..7e435bf9f603 100644 --- a/hadoop-hdds/framework/src/test/java/org/apache/hadoop/hdds/security/x509/certificate/authority/TestDefaultCAServer.java +++ b/hadoop-hdds/framework/src/test/java/org/apache/hadoop/hdds/security/x509/certificate/authority/TestDefaultCAServer.java @@ -46,6 +46,7 @@ import java.time.LocalDate; import java.time.ZoneId; import java.time.ZonedDateTime; +import java.util.Collection; import java.util.Date; import java.util.List; import java.util.TimeZone; @@ -68,6 +69,7 @@ import org.apache.hadoop.hdds.security.x509.keys.HDDSKeyGenerator; import org.apache.hadoop.hdds.security.x509.keys.KeyStorage; import org.apache.hadoop.security.ssl.KeyStoreTestUtil; +import org.bouncycastle.asn1.x509.GeneralName; import org.bouncycastle.pkcs.PKCS10CertificationRequest; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -196,6 +198,49 @@ public void testRequestCertificate() throws Exception { } + /** + * Tests that an internal-suffix DNS name in the CSR is retained as a + * dNSName Subject Alternative Name in the issued certificate. + * @throws Exception - on ERROR. + */ + @Test + public void testRequestCertificateRetainsInternalDnsName() throws Exception { + String scmId = RandomStringUtils.secure().nextAlphabetic(4); + String clusterId = RandomStringUtils.secure().nextAlphabetic(4); + KeyPair keyPair = + new HDDSKeyGenerator(securityConfig).generateKey(); + PKCS10CertificationRequest csr = new CertificateSignRequest.Builder() + .addDnsName("scm1.lxd") + .setCA(false) + .setClusterID(clusterId) + .setScmID(scmId) + .setSubject("Ozone Cluster") + .setConfiguration(securityConfig) + .setKey(keyPair) + .build() + .generateCSR(); + + CertificateServer testCA = new DefaultCAServer("testCA", + clusterId, scmId, caStore, + new DefaultProfile(), + Paths.get(SCM_CA_CERT_STORAGE_DIR, SCM_CA_PATH).toString()); + testCA.init(securityConfig, CAType.ROOT); + + Future holder = testCA.requestCertificate( + csr, CertificateApprover.ApprovalType.TESTING_AUTOMATIC, SCM, + String.valueOf(System.nanoTime())); + assertTrue(holder.isDone()); + X509Certificate signedCert = + CertificateCodec.firstCertificateFrom(holder.get()); + + Collection> subjectAlternativeNames = + signedCert.getSubjectAlternativeNames(); + assertNotNull(subjectAlternativeNames); + assertTrue(subjectAlternativeNames.stream().anyMatch( + san -> ((Integer) san.get(0)) == GeneralName.dNSName + && "scm1.lxd".equals(san.get(1)))); + } + /** * Tests that we are able * to create a Test CA, creates it own self-Signed CA and then issue a diff --git a/hadoop-hdds/framework/src/test/java/org/apache/hadoop/hdds/security/x509/certificate/authority/TestDefaultProfile.java b/hadoop-hdds/framework/src/test/java/org/apache/hadoop/hdds/security/x509/certificate/authority/TestDefaultProfile.java index 5f09b347b937..e8a5ebfdf231 100644 --- a/hadoop-hdds/framework/src/test/java/org/apache/hadoop/hdds/security/x509/certificate/authority/TestDefaultProfile.java +++ b/hadoop-hdds/framework/src/test/java/org/apache/hadoop/hdds/security/x509/certificate/authority/TestDefaultProfile.java @@ -155,6 +155,48 @@ public void testExtensions() throws Exception { assertTrue(approver.verfiyExtensions(csr)); } + /** + * Tests that internal-suffix and single-label DNS names, which are not + * accepted by a public-suffix based validator, are accepted by + * the RFC 1123 based DnsNames validation. + */ + @Test + public void testExtensionsWithInternalDnsNames() throws Exception { + PKCS10CertificationRequest csr = new CertificateSignRequest.Builder() + .addDnsName("scm1.lxd") + .addDnsName("datanode1") + .setCA(false) + .setClusterID("ClusterID") + .setScmID("SCMID") + .setSubject("Ozone Cluster") + .setConfiguration(securityConfig) + .setKey(keyPair) + .build() + .generateCSR(); + assertTrue(approver.verfiyExtensions(csr)); + } + + /** + * Tests that a wildcard, an IP literal, or an empty dNSName are still + * rejected by the DnsNames validation. + */ + @Test + public void testInvalidExtensionsWithDnsName() throws IOException, + OperatorCreationException { + Extensions dnsExtension = getSANExtension(GeneralName.dNSName, + "*.example.com", false); + PKCS10CertificationRequest csr = getInvalidCSR(keyPair, dnsExtension); + assertFalse(approver.verfiyExtensions(csr)); + + dnsExtension = getSANExtension(GeneralName.dNSName, "10.0.0.5", false); + csr = getInvalidCSR(keyPair, dnsExtension); + assertFalse(approver.verfiyExtensions(csr)); + + dnsExtension = getSANExtension(GeneralName.dNSName, "", false); + csr = getInvalidCSR(keyPair, dnsExtension); + assertFalse(approver.verfiyExtensions(csr)); + } + /** * Tests that invalid extensions cause a failure in validation. We will fail * if CA extension is enabled. diff --git a/hadoop-hdds/framework/src/test/java/org/apache/hadoop/hdds/security/x509/certificate/utils/TestCertificateSignRequest.java b/hadoop-hdds/framework/src/test/java/org/apache/hadoop/hdds/security/x509/certificate/utils/TestCertificateSignRequest.java index 051d28593c95..a73b2173bbf7 100644 --- a/hadoop-hdds/framework/src/test/java/org/apache/hadoop/hdds/security/x509/certificate/utils/TestCertificateSignRequest.java +++ b/hadoop-hdds/framework/src/test/java/org/apache/hadoop/hdds/security/x509/certificate/utils/TestCertificateSignRequest.java @@ -25,10 +25,20 @@ import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assertions.fail; +import static org.mockito.Mockito.CALLS_REAL_METHODS; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.mockStatic; +import static org.mockito.Mockito.when; import java.io.IOException; +import java.net.InetAddress; +import java.net.UnknownHostException; import java.nio.file.Path; import java.security.KeyPair; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; +import java.util.List; import java.util.UUID; import org.apache.hadoop.hdds.conf.OzoneConfiguration; import org.apache.hadoop.hdds.security.SecurityConfig; @@ -49,6 +59,7 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.io.TempDir; +import org.mockito.MockedStatic; /** * Certificate Signing Request. @@ -259,6 +270,120 @@ public void testCsrSerialization() throws Exception { assertEquals(csr, dsCsr); } + @Test + public void testAddInetAddressesAddsDnsNameFromCanonicalHostName() throws Exception { + InetAddress address = mock(InetAddress.class); + when(address.getHostAddress()).thenReturn("192.0.2.10"); + when(address.getCanonicalHostName()).thenReturn("scm1.lxd"); + + CertificateSignRequest.Builder builder = newBuilder(); + builder.addInetAddresses(Collections.singletonList(address)); + + PKCS10CertificationRequest csr = builder.build().generateCSR(); + List sanNames = getSanNames(csr); + assertEquals(1, countByTag(sanNames, GeneralName.iPAddress)); + assertEquals(1, countByTag(sanNames, GeneralName.dNSName)); + assertTrue(dnsNameValues(sanNames).contains("scm1.lxd")); + } + + @Test + public void testAddInetAddressesSkipsIpLiteralAndFallbackFailure() throws Exception { + InetAddress address = mock(InetAddress.class); + when(address.getHostAddress()).thenReturn("192.0.2.11"); + when(address.getCanonicalHostName()).thenReturn("10.0.0.5"); + + CertificateSignRequest.Builder builder = newBuilder(); + try (MockedStatic mockedInetAddress = mockStatic(InetAddress.class, CALLS_REAL_METHODS)) { + mockedInetAddress.when(InetAddress::getLocalHost).thenThrow(new UnknownHostException("no localhost")); + builder.addInetAddresses(Collections.singletonList(address)); + } + + PKCS10CertificationRequest csr = builder.build().generateCSR(); + List sanNames = getSanNames(csr); + assertEquals(1, countByTag(sanNames, GeneralName.iPAddress)); + assertEquals(0, countByTag(sanNames, GeneralName.dNSName)); + } + + @Test + public void testAddInetAddressesFallsBackToLocalHostCanonicalName() throws Exception { + InetAddress address = mock(InetAddress.class); + when(address.getHostAddress()).thenReturn("192.0.2.12"); + when(address.getCanonicalHostName()).thenReturn("10.0.0.5"); + + InetAddress localHost = mock(InetAddress.class); + when(localHost.getCanonicalHostName()).thenReturn("fallback1.lxd"); + + CertificateSignRequest.Builder builder = newBuilder(); + try (MockedStatic mockedInetAddress = mockStatic(InetAddress.class, CALLS_REAL_METHODS)) { + mockedInetAddress.when(InetAddress::getLocalHost).thenReturn(localHost); + builder.addInetAddresses(Collections.singletonList(address)); + } + + PKCS10CertificationRequest csr = builder.build().generateCSR(); + List sanNames = getSanNames(csr); + assertEquals(1, countByTag(sanNames, GeneralName.dNSName)); + assertEquals(Collections.singletonList("fallback1.lxd"), dnsNameValues(sanNames)); + } + + @Test + public void testAddInetAddressesDeduplicatesDnsNamesCaseInsensitively() throws Exception { + InetAddress address1 = mock(InetAddress.class); + when(address1.getHostAddress()).thenReturn("192.0.2.13"); + when(address1.getCanonicalHostName()).thenReturn("SCM1.LXD"); + + InetAddress address2 = mock(InetAddress.class); + when(address2.getHostAddress()).thenReturn("192.0.2.14"); + when(address2.getCanonicalHostName()).thenReturn("scm1.lxd"); + + CertificateSignRequest.Builder builder = newBuilder(); + builder.addInetAddresses(Arrays.asList(address1, address2)); + + PKCS10CertificationRequest csr = builder.build().generateCSR(); + List sanNames = getSanNames(csr); + assertEquals(2, countByTag(sanNames, GeneralName.iPAddress)); + assertEquals(1, countByTag(sanNames, GeneralName.dNSName)); + } + + private CertificateSignRequest.Builder newBuilder() throws Exception { + String clusterID = UUID.randomUUID().toString(); + String scmID = UUID.randomUUID().toString(); + String subject = "DN001"; + HDDSKeyGenerator keyGen = new HDDSKeyGenerator(securityConfig); + KeyPair keyPair = keyGen.generateKey(); + return new CertificateSignRequest.Builder() + .setSubject(subject) + .setScmID(scmID) + .setClusterID(clusterID) + .setKey(keyPair) + .setConfiguration(securityConfig); + } + + private List getSanNames(PKCS10CertificationRequest csr) throws Exception { + Extensions extensions = getPkcs9Extensions(csr); + Extension ext = extensions.getExtension(Extension.subjectAlternativeName); + return Arrays.asList(GeneralNames.getInstance(ext.getParsedValue()).getNames()); + } + + private long countByTag(List names, int tag) { + long count = 0; + for (GeneralName name : names) { + if (name.getTagNo() == tag) { + count++; + } + } + return count; + } + + private List dnsNameValues(List names) { + List values = new ArrayList<>(); + for (GeneralName name : names) { + if (name.getTagNo() == GeneralName.dNSName) { + values.add(name.getName().toString()); + } + } + return values; + } + private void verifyServiceId(Extensions extensions) { GeneralNames gns = GeneralNames.fromExtensions( diff --git a/hadoop-hdds/framework/src/test/java/org/apache/hadoop/hdds/security/x509/certificate/utils/TestDnsNames.java b/hadoop-hdds/framework/src/test/java/org/apache/hadoop/hdds/security/x509/certificate/utils/TestDnsNames.java new file mode 100644 index 000000000000..d65c92b6f03b --- /dev/null +++ b/hadoop-hdds/framework/src/test/java/org/apache/hadoop/hdds/security/x509/certificate/utils/TestDnsNames.java @@ -0,0 +1,93 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.hadoop.hdds.security.x509.certificate.utils; + +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.util.Optional; +import org.apache.commons.lang3.StringUtils; +import org.junit.jupiter.api.Test; + +/** + * Tests for {@link DnsNames}. + */ +public class TestDnsNames { + + @Test + public void acceptsValidDnsNames() { + String[] valid = { + "scm1.lxd", + "datanode1", + "om.internal", + "dn3.local", + "a-b.c-d.example.com", + "hadoop.apache.org", + StringUtils.repeat('a', 63), // 63-character label + }; + for (String name : valid) { + assertTrue(DnsNames.isValidDnsName(name), name); + assertTrue(DnsNames.toDnsSanValue(name).isPresent(), name); + } + } + + @Test + public void rejectsInvalidDnsNames() { + String longLabel = StringUtils.repeat('a', 64); // 64-character label + String longName = StringUtils.repeat("a234567890.", 24) + "example.com"; // 275 chars, > 253 + String[] invalid = { + "", + " ", + "*.example.com", + "10.0.0.5", + "2001:db8::1", + "-lead.example.com", + "trail-.example.com", + "host_name.lxd", + longLabel, + longName, + }; + for (String name : invalid) { + assertFalse(DnsNames.isValidDnsName(name), name); + assertFalse(DnsNames.toDnsSanValue(name).isPresent(), name); + } + } + + @Test + public void doesNotThrowForNull() { + assertDoesNotThrow(() -> DnsNames.isValidDnsName(null)); + assertDoesNotThrow(() -> DnsNames.toDnsSanValue(null)); + assertFalse(DnsNames.isValidDnsName(null)); + assertFalse(DnsNames.toDnsSanValue(null).isPresent()); + } + + @Test + public void stripsAtMostOneTrailingDot() { + assertEquals(Optional.of("scm1.lxd"), DnsNames.toDnsSanValue("scm1.lxd.")); + assertFalse(DnsNames.isValidDnsName("scm1.lxd.")); + assertFalse(DnsNames.toDnsSanValue("scm1.lxd..").isPresent()); + } + + @Test + public void handlesIdnConversion() { + assertEquals(Optional.of("xn--bcher-kva.lxd"), DnsNames.toDnsSanValue("bücher.lxd")); + assertFalse(DnsNames.isValidDnsName("bücher.lxd")); + } +} diff --git a/hadoop-hdds/framework/src/test/java/org/apache/hadoop/hdds/security/x509/certificate/utils/TestRootCertificate.java b/hadoop-hdds/framework/src/test/java/org/apache/hadoop/hdds/security/x509/certificate/utils/TestRootCertificate.java index 7f71b6515c8b..549f2c8aaa8d 100644 --- a/hadoop-hdds/framework/src/test/java/org/apache/hadoop/hdds/security/x509/certificate/utils/TestRootCertificate.java +++ b/hadoop-hdds/framework/src/test/java/org/apache/hadoop/hdds/security/x509/certificate/utils/TestRootCertificate.java @@ -25,8 +25,11 @@ import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assertions.fail; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; import java.math.BigInteger; +import java.net.InetAddress; import java.nio.file.Path; import java.security.InvalidKeyException; import java.security.KeyPair; @@ -34,7 +37,11 @@ import java.security.cert.CertificateException; import java.security.cert.X509Certificate; import java.time.ZonedDateTime; +import java.util.ArrayList; +import java.util.Collection; +import java.util.Collections; import java.util.Date; +import java.util.List; import java.util.UUID; import org.apache.hadoop.hdds.conf.OzoneConfiguration; import org.apache.hadoop.hdds.security.SecurityConfig; @@ -151,6 +158,46 @@ public void testCACert(@TempDir Path basePath) throws Exception { loadedCert.getSerialNumber()); } + @Test + public void testCACertWithMockedInetAddressAddsDnsName() throws Exception { + ZonedDateTime notBefore = ZonedDateTime.now(); + ZonedDateTime notAfter = notBefore.plusYears(1); + String clusterID = UUID.randomUUID().toString(); + String scmID = UUID.randomUUID().toString(); + String subject = "testRootCert"; + HDDSKeyGenerator keyGen = + new HDDSKeyGenerator(securityConfig); + KeyPair keyPair = keyGen.generateKey(); + + InetAddress address = mock(InetAddress.class); + when(address.getHostAddress()).thenReturn("192.0.2.20"); + when(address.getCanonicalHostName()).thenReturn("scm1.lxd"); + + X509Certificate certificate = + SelfSignedCertificate.newBuilder() + .setBeginDate(notBefore) + .setEndDate(notAfter) + .setClusterID(clusterID) + .setScmID(scmID) + .setSubject(subject) + .setKey(keyPair) + .setConfiguration(securityConfig) + .makeCA() + .addInetAddresses(Collections.singletonList(address)) + .build(); + + Collection> subjectAlternativeNames = certificate.getSubjectAlternativeNames(); + assertNotNull(subjectAlternativeNames); + List dnsNames = new ArrayList<>(); + for (List san : subjectAlternativeNames) { + // GeneralName type 2 is dNSName, see RFC 5280 4.2.1.6. + if (((Number) san.get(0)).intValue() == 2) { + dnsNames.add((String) san.get(1)); + } + } + assertTrue(dnsNames.contains("scm1.lxd")); + } + @Test public void testInvalidParamFails() throws Exception { ZonedDateTime notBefore = ZonedDateTime.now(); diff --git a/hadoop-ozone/integration-test/pom.xml b/hadoop-ozone/integration-test/pom.xml index 6c56728b9380..0e67eeb811ba 100644 --- a/hadoop-ozone/integration-test/pom.xml +++ b/hadoop-ozone/integration-test/pom.xml @@ -71,11 +71,6 @@ commons-io test - - commons-validator - commons-validator - test - info.picocli picocli 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 bb9fb9f61807..e6178ee380cf 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 @@ -92,7 +92,6 @@ import org.apache.commons.io.IOUtils; import org.apache.commons.lang3.RandomStringUtils; import org.apache.commons.lang3.StringUtils; -import org.apache.commons.validator.routines.DomainValidator; import org.apache.hadoop.hdds.HddsConfigKeys; import org.apache.hadoop.hdds.conf.OzoneConfiguration; import org.apache.hadoop.hdds.protocol.proto.HddsProtos; @@ -1268,9 +1267,8 @@ private static X509Certificate signX509Cert( private static void addIpAndDnsDataToBuilder( CertificateSignRequest.Builder csrBuilder) throws IOException { - DomainValidator validator = DomainValidator.getInstance(); // Add all valid ips. List inetAddresses = getValidInetsForCurrentHost(); - csrBuilder.addInetAddresses(inetAddresses, validator); + csrBuilder.addInetAddresses(inetAddresses); } }