HDDS-16034. Certificates omit DNS SANs on clusters with non-public hostname suffixes

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;

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<InetAddress> 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<InetAddress> addresses,
-        DomainValidator validator) {
+        List<InetAddress> addresses) {
       // Add all valid ips.
       addresses.forEach(
           ip -> {
             this.addIpAddress(ip.getHostAddress());
-            if (validator.isValid(ip.getCanonicalHostName())) {
-              this.addDnsName(ip.getCanonicalHostName());
+            Optional<String> 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<String> 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;
     }
 

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<String> 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;
+  }
+}

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,31 +220,73 @@ public Builder makeCA(BigInteger serialId) {
 
     public Builder addInetAddresses() throws CertificateException {
       try {
-        DomainValidator validator = DomainValidator.getInstance();
         // Add all valid ips.
         List<InetAddress> 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);
       }
       return this;
     }
 
-    public Builder addInetAddresses(List<InetAddress> addresses,
-        DomainValidator validator) {
+    public Builder addInetAddresses(List<InetAddress> addresses) {
       addresses.forEach(
           ip -> {
             this.addIpAddress(ip.getHostAddress());
-            if (validator.isValid(ip.getCanonicalHostName())) {
-              this.addDnsName(ip.getCanonicalHostName());
+            Optional<String> 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<String> 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) {

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<CertPath> holder = testCA.requestCertificate(
+        csr, CertificateApprover.ApprovalType.TESTING_AUTOMATIC, SCM,
+        String.valueOf(System.nanoTime()));
+    assertTrue(holder.isDone());
+    X509Certificate signedCert =
+        CertificateCodec.firstCertificateFrom(holder.get());
+
+    Collection<List<?>> 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