You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@hbase.apache.org by GitBox <gi...@apache.org> on 2022/09/18 14:54:00 UTC
[GitHub] [hbase] Apache9 commented on a diff in pull request #4724: HBASE-27280 Add mutual authentication support to TLS
Apache9 commented on code in PR #4724:
URL: https://github.com/apache/hbase/pull/4724#discussion_r973733235
##########
hbase-common/src/main/java/org/apache/hadoop/hbase/io/crypto/tls/HBaseTrustManager.java:
##########
@@ -0,0 +1,160 @@
+/*
+ * 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.hbase.io.crypto.tls;
+
+import java.net.InetAddress;
+import java.net.Socket;
+import java.net.UnknownHostException;
+import java.security.cert.CertificateException;
+import java.security.cert.X509Certificate;
+import javax.net.ssl.SSLEngine;
+import javax.net.ssl.SSLException;
+import javax.net.ssl.X509ExtendedTrustManager;
+import org.apache.yetus.audience.InterfaceAudience;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * A custom TrustManager that supports hostname verification We attempt to perform verification
Review Comment:
This is not the default behavior of java's TrustManager implementation?
##########
hbase-common/src/main/java/org/apache/hadoop/hbase/io/crypto/tls/X509Util.java:
##########
@@ -110,6 +122,47 @@ private static String[] getCBCCiphers() {
private static final String[] DEFAULT_CIPHERS_JAVA9 =
ObjectArrays.concat(getGCMCiphers(), getCBCCiphers(), String.class);
+ /**
+ * Enum specifying the client auth requirement of server-side TLS sockets created by this
+ * X509Util.
+ * <ul>
+ * <li>NONE - do not request a client certificate.</li>
+ * <li>WANT - request a client certificate, but allow anonymous clients to connect.</li>
+ * <li>NEED - require a client certificate, disconnect anonymous clients.</li>
+ * </ul>
+ * If the config property is not set, the default value is NEED.
+ */
+ public enum ClientAuth {
Review Comment:
Why not juse use the netty ones directly?
##########
hbase-common/src/main/java/org/apache/hadoop/hbase/io/crypto/tls/HBaseHostnameVerifier.java:
##########
@@ -0,0 +1,368 @@
+/*
+ * 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.hbase.io.crypto.tls;
+
+import java.net.InetAddress;
+import java.net.UnknownHostException;
+import java.security.cert.Certificate;
+import java.security.cert.CertificateParsingException;
+import java.security.cert.X509Certificate;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.List;
+import java.util.Locale;
+import java.util.NoSuchElementException;
+import java.util.Objects;
+import java.util.regex.Pattern;
+import javax.naming.InvalidNameException;
+import javax.naming.NamingException;
+import javax.naming.directory.Attribute;
+import javax.naming.directory.Attributes;
+import javax.naming.ldap.LdapName;
+import javax.naming.ldap.Rdn;
+import javax.net.ssl.HostnameVerifier;
+import javax.net.ssl.SSLException;
+import javax.net.ssl.SSLPeerUnverifiedException;
+import javax.net.ssl.SSLSession;
+import javax.security.auth.x500.X500Principal;
+import org.apache.yetus.audience.InterfaceAudience;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * When enabled in {@link X509Util}, handles verifying that the hostname of a peer matches the
+ * certificate it presents.
+ * <p/>
+ * This file has been copied from the Apache ZooKeeper project.
+ * @see <a href=
+ * "https://github.com/apache/zookeeper/blob/5820d10d9dc58c8e12d2e25386fdf92acb360359/zookeeper-server/src/main/java/org/apache/zookeeper/common/ZKHostnameVerifier.java">Base
+ * revision</a>
+ */
+@InterfaceAudience.Private
+final class HBaseHostnameVerifier implements HostnameVerifier {
Review Comment:
Is it possible to just make use of guava's InetAddresses? It has a isInetAddress method which works for both ipv4 and ipv6, and then we could create a InetAddress instance and will know whether it is ipv4 or ipv6.
##########
hbase-common/src/main/java/org/apache/hadoop/hbase/io/crypto/tls/HBaseHostnameVerifier.java:
##########
@@ -0,0 +1,368 @@
+/*
+ * 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.hbase.io.crypto.tls;
+
+import java.net.InetAddress;
+import java.net.UnknownHostException;
+import java.security.cert.Certificate;
+import java.security.cert.CertificateParsingException;
+import java.security.cert.X509Certificate;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.List;
+import java.util.Locale;
+import java.util.NoSuchElementException;
+import java.util.Objects;
+import java.util.regex.Pattern;
+import javax.naming.InvalidNameException;
+import javax.naming.NamingException;
+import javax.naming.directory.Attribute;
+import javax.naming.directory.Attributes;
+import javax.naming.ldap.LdapName;
+import javax.naming.ldap.Rdn;
+import javax.net.ssl.HostnameVerifier;
+import javax.net.ssl.SSLException;
+import javax.net.ssl.SSLPeerUnverifiedException;
+import javax.net.ssl.SSLSession;
+import javax.security.auth.x500.X500Principal;
+import org.apache.yetus.audience.InterfaceAudience;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * When enabled in {@link X509Util}, handles verifying that the hostname of a peer matches the
+ * certificate it presents.
+ * <p/>
+ * This file has been copied from the Apache ZooKeeper project.
+ * @see <a href=
+ * "https://github.com/apache/zookeeper/blob/5820d10d9dc58c8e12d2e25386fdf92acb360359/zookeeper-server/src/main/java/org/apache/zookeeper/common/ZKHostnameVerifier.java">Base
+ * revision</a>
+ */
+@InterfaceAudience.Private
+final class HBaseHostnameVerifier implements HostnameVerifier {
+
+ private final Logger LOG = LoggerFactory.getLogger(HBaseHostnameVerifier.class);
+
+ /**
+ * Note: copied from Apache httpclient with some minor modifications. We want host verification,
+ * but depending on the httpclient jar caused unexplained performance regressions (even when the
+ * code was not used).
+ */
+ private static final class SubjectName {
+
+ static final int DNS = 2;
+ static final int IP = 7;
+
+ private final String value;
+ private final int type;
+
+ static SubjectName IP(final String value) {
Review Comment:
Use lower case here?
##########
hbase-common/src/main/java/org/apache/hadoop/hbase/io/crypto/tls/HBaseHostnameVerifier.java:
##########
@@ -0,0 +1,368 @@
+/*
+ * 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.hbase.io.crypto.tls;
+
+import java.net.InetAddress;
+import java.net.UnknownHostException;
+import java.security.cert.Certificate;
+import java.security.cert.CertificateParsingException;
+import java.security.cert.X509Certificate;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.List;
+import java.util.Locale;
+import java.util.NoSuchElementException;
+import java.util.Objects;
+import java.util.regex.Pattern;
+import javax.naming.InvalidNameException;
+import javax.naming.NamingException;
+import javax.naming.directory.Attribute;
+import javax.naming.directory.Attributes;
+import javax.naming.ldap.LdapName;
+import javax.naming.ldap.Rdn;
+import javax.net.ssl.HostnameVerifier;
+import javax.net.ssl.SSLException;
+import javax.net.ssl.SSLPeerUnverifiedException;
+import javax.net.ssl.SSLSession;
+import javax.security.auth.x500.X500Principal;
+import org.apache.yetus.audience.InterfaceAudience;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * When enabled in {@link X509Util}, handles verifying that the hostname of a peer matches the
+ * certificate it presents.
+ * <p/>
+ * This file has been copied from the Apache ZooKeeper project.
+ * @see <a href=
+ * "https://github.com/apache/zookeeper/blob/5820d10d9dc58c8e12d2e25386fdf92acb360359/zookeeper-server/src/main/java/org/apache/zookeeper/common/ZKHostnameVerifier.java">Base
+ * revision</a>
+ */
+@InterfaceAudience.Private
+final class HBaseHostnameVerifier implements HostnameVerifier {
+
+ private final Logger LOG = LoggerFactory.getLogger(HBaseHostnameVerifier.class);
+
+ /**
+ * Note: copied from Apache httpclient with some minor modifications. We want host verification,
+ * but depending on the httpclient jar caused unexplained performance regressions (even when the
+ * code was not used).
+ */
+ private static final class SubjectName {
+
+ static final int DNS = 2;
+ static final int IP = 7;
+
+ private final String value;
+ private final int type;
+
+ static SubjectName IP(final String value) {
+ return new SubjectName(value, IP);
+ }
+
+ static SubjectName DNS(final String value) {
+ return new SubjectName(value, DNS);
+ }
+
+ SubjectName(final String value, final int type) {
+ if (type != DNS && type != IP) {
+ throw new IllegalArgumentException("Invalid type: " + type);
+ }
+ this.value = Objects.requireNonNull(value);
+ this.type = type;
+ }
+
+ public int getType() {
+ return type;
+ }
+
+ public String getValue() {
+ return value;
+ }
+
+ @Override
+ public String toString() {
+ return value;
+ }
+
+ }
+
+ /**
+ * Note: copied from Apache httpclient. We want host verification, but depending on the httpclient
+ * jar caused unexplained performance regressions (even when the code was not used).
+ */
+ private static final class InetAddressUtils {
+
+ private InetAddressUtils() {
+ }
+
+ private static final Pattern IPV4_PATTERN = Pattern
+ .compile("^(25[0-5]|2[0-4]\\d|[0-1]?\\d?\\d)(\\.(25[0-5]|2[0-4]\\d|[0-1]?\\d?\\d)){3}$");
+
+ private static final Pattern IPV6_STD_PATTERN =
+ Pattern.compile("^(?:[0-9a-fA-F]{1,4}:){7}[0-9a-fA-F]{1,4}$");
+
+ @SuppressWarnings("checkstyle:linelength")
+ private static final Pattern IPV6_HEX_COMPRESSED_PATTERN = Pattern.compile(
+ "^((?:[0-9A-Fa-f]{1,4}(?::[0-9A-Fa-f]{1,4})*)?)::((?:[0-9A-Fa-f]{1,4}(?::[0-9A-Fa-f]{1,4})*)?)$");
+
+ static boolean isIPv4Address(final String input) {
+ return IPV4_PATTERN.matcher(input).matches();
+ }
+
+ static boolean isIPv6StdAddress(final String input) {
+ return IPV6_STD_PATTERN.matcher(input).matches();
+ }
+
+ static boolean isIPv6HexCompressedAddress(final String input) {
+ return IPV6_HEX_COMPRESSED_PATTERN.matcher(input).matches();
+ }
+
+ static boolean isIPv6Address(final String input) {
+ return isIPv6StdAddress(input) || isIPv6HexCompressedAddress(input);
+ }
+
+ }
+
+ enum HostNameType {
+
+ IPv4(7),
+ IPv6(7),
+ DNS(2);
+
+ final int subjectType;
+
+ HostNameType(final int subjectType) {
+ this.subjectType = subjectType;
+ }
+
+ }
+
+ @Override
+ public boolean verify(final String host, final SSLSession session) {
+ try {
+ final Certificate[] certs = session.getPeerCertificates();
+ final X509Certificate x509 = (X509Certificate) certs[0];
+ verify(host, x509);
+ return true;
+ } catch (final SSLException ex) {
+ LOG.debug("Unexpected exception", ex);
+ return false;
+ }
+ }
+
+ void verify(final String host, final X509Certificate cert) throws SSLException {
+ final HostNameType hostType = determineHostFormat(host);
+ final List<SubjectName> subjectAlts = getSubjectAltNames(cert);
+ if (subjectAlts != null && !subjectAlts.isEmpty()) {
+ switch (hostType) {
+ case IPv4:
+ matchIPAddress(host, subjectAlts);
+ break;
+ case IPv6:
+ matchIPv6Address(host, subjectAlts);
+ break;
+ default:
+ matchDNSName(host, subjectAlts);
+ }
+ } else {
+ // CN matching has been deprecated by rfc2818 and can be used
+ // as fallback only when no subjectAlts are available
+ final X500Principal subjectPrincipal = cert.getSubjectX500Principal();
+ final String cn = extractCN(subjectPrincipal.getName(X500Principal.RFC2253));
+ if (cn == null) {
+ throw new SSLException("Certificate subject for <" + host + "> doesn't contain "
+ + "a common name and does not have alternative names");
+ }
+ matchCN(host, cn);
+ }
+ }
+
+ private static void matchIPAddress(final String host, final List<SubjectName> subjectAlts)
+ throws SSLException {
+ for (int i = 0; i < subjectAlts.size(); i++) {
+ final SubjectName subjectAlt = subjectAlts.get(i);
+ if (subjectAlt.getType() == SubjectName.IP) {
+ if (host.equals(subjectAlt.getValue())) {
+ return;
+ }
+ }
+ }
+ throw new SSLPeerUnverifiedException("Certificate for <" + host + "> doesn't match any "
+ + "of the subject alternative names: " + subjectAlts);
+ }
+
+ private static void matchIPv6Address(final String host, final List<SubjectName> subjectAlts)
+ throws SSLException {
+ final String normalisedHost = normaliseAddress(host);
+ for (int i = 0; i < subjectAlts.size(); i++) {
+ final SubjectName subjectAlt = subjectAlts.get(i);
+ if (subjectAlt.getType() == SubjectName.IP) {
+ final String normalizedSubjectAlt = normaliseAddress(subjectAlt.getValue());
+ if (normalisedHost.equals(normalizedSubjectAlt)) {
+ return;
+ }
+ }
+ }
+ throw new SSLPeerUnverifiedException("Certificate for <" + host + "> doesn't match any "
+ + "of the subject alternative names: " + subjectAlts);
+ }
+
+ private static void matchDNSName(final String host, final List<SubjectName> subjectAlts)
+ throws SSLException {
+ final String normalizedHost = host.toLowerCase(Locale.ROOT);
+ for (int i = 0; i < subjectAlts.size(); i++) {
+ final SubjectName subjectAlt = subjectAlts.get(i);
+ if (subjectAlt.getType() == SubjectName.DNS) {
+ final String normalizedSubjectAlt = subjectAlt.getValue().toLowerCase(Locale.ROOT);
+ if (matchIdentityStrict(normalizedHost, normalizedSubjectAlt)) {
+ return;
+ }
+ }
+ }
+ throw new SSLPeerUnverifiedException("Certificate for <" + host + "> doesn't match any "
+ + "of the subject alternative names: " + subjectAlts);
+ }
+
+ private static void matchCN(final String host, final String cn) throws SSLException {
+ final String normalizedHost = host.toLowerCase(Locale.ROOT);
+ final String normalizedCn = cn.toLowerCase(Locale.ROOT);
+ if (!matchIdentityStrict(normalizedHost, normalizedCn)) {
+ throw new SSLPeerUnverifiedException("Certificate for <" + host + "> doesn't match "
+ + "common name of the certificate subject: " + cn);
+ }
+ }
+
+ private static boolean matchIdentity(final String host, final String identity,
+ final boolean strict) {
+ // RFC 2818, 3.1. Server Identity
+ // "...Names may contain the wildcard
+ // character * which is considered to match any single domain name
+ // component or component fragment..."
+ // Based on this statement presuming only singular wildcard is legal
+ final int asteriskIdx = identity.indexOf('*');
+ if (asteriskIdx != -1) {
+ final String prefix = identity.substring(0, asteriskIdx);
+ final String suffix = identity.substring(asteriskIdx + 1);
+ if (!prefix.isEmpty() && !host.startsWith(prefix)) {
+ return false;
+ }
+ if (!suffix.isEmpty() && !host.endsWith(suffix)) {
+ return false;
+ }
+ // Additional sanity checks on content selected by wildcard can be done here
+ if (strict) {
+ final String remainder = host.substring(prefix.length(), host.length() - suffix.length());
+ return !remainder.contains(".");
+ }
+ return true;
+ }
+ return host.equalsIgnoreCase(identity);
+ }
+
+ private static boolean matchIdentityStrict(final String host, final String identity) {
+ return matchIdentity(host, identity, true);
+ }
+
+ private static String extractCN(final String subjectPrincipal) throws SSLException {
+ if (subjectPrincipal == null) {
+ return null;
+ }
+ try {
+ final LdapName subjectDN = new LdapName(subjectPrincipal);
+ final List<Rdn> rdns = subjectDN.getRdns();
+ for (int i = rdns.size() - 1; i >= 0; i--) {
+ final Rdn rds = rdns.get(i);
+ final Attributes attributes = rds.toAttributes();
+ final Attribute cn = attributes.get("cn");
+ if (cn != null) {
+ try {
+ final Object value = cn.get();
+ if (value != null) {
+ return value.toString();
+ }
+ } catch (final NoSuchElementException ignore) {
+ // ignore exception
+ } catch (final NamingException ignore) {
+ // ignore exception
+ }
+ }
+ }
+ return null;
+ } catch (final InvalidNameException e) {
+ throw new SSLException(subjectPrincipal + " is not a valid X500 distinguished name");
+ }
+ }
+
+ private static HostNameType determineHostFormat(final String host) {
+ if (InetAddressUtils.isIPv4Address(host)) {
+ return HostNameType.IPv4;
+ }
+ String s = host;
+ if (s.startsWith("[") && s.endsWith("]")) {
+ s = host.substring(1, host.length() - 1);
+ }
+ if (InetAddressUtils.isIPv6Address(s)) {
+ return HostNameType.IPv6;
+ }
+ return HostNameType.DNS;
+ }
+
+ @SuppressWarnings("MixedMutabilityReturnType")
+ private static List<SubjectName> getSubjectAltNames(final X509Certificate cert) {
+ try {
+ final Collection<List<?>> entries = cert.getSubjectAlternativeNames();
+ if (entries == null) {
+ return Collections.emptyList();
+ }
+ final List<SubjectName> result = new ArrayList<SubjectName>();
+ for (List<?> entry : entries) {
+ final Integer type = entry.size() >= 2 ? (Integer) entry.get(0) : null;
+ if (type != null) {
+ if (type == SubjectName.DNS || type == SubjectName.IP) {
+ final Object o = entry.get(1);
+ // TODO ASN.1 DER encoded form when instanceof byte[]
+ if (o instanceof String) {
+ result.add(new SubjectName((String) o, type));
+ }
+ }
+ }
+ }
+ return result;
+ } catch (final CertificateParsingException ignore) {
+ return Collections.emptyList();
+ }
+ }
+
+ /*
+ * Normalize IPv6 or DNS name.
+ */
+ private static String normaliseAddress(final String hostname) {
Review Comment:
normalize?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org