From 8f83a2355bc6224a92fb7ba24890ddf3ab317ae6 Mon Sep 17 00:00:00 2001 From: Suresh Srinivas Date: Wed, 11 Jan 2012 21:02:04 +0000 Subject: [PATCH] HADOOP-7964. Deadlock in NetUtils and SecurityUtil class initialization. Contributed by Daryn Sharp. git-svn-id: https://svn.apache.org/repos/asf/hadoop/common/trunk@1230249 13f79535-47bb-0310-9956-ffa450edef68 --- .../hadoop-common/CHANGES.txt | 3 + .../java/org/apache/hadoop/net/NetUtils.java | 175 +--------------- .../apache/hadoop/security/SecurityUtil.java | 188 ++++++++++++++++-- .../fs/TestFileSystemCanonicalization.java | 2 +- .../org/apache/hadoop/net/TestNetUtils.java | 1 + .../NetUtilsTestResolver.java | 23 ++- .../hadoop/security/TestSecurityUtil.java | 2 +- 7 files changed, 196 insertions(+), 198 deletions(-) rename hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/{net => security}/NetUtilsTestResolver.java (80%) diff --git a/hadoop-common-project/hadoop-common/CHANGES.txt b/hadoop-common-project/hadoop-common/CHANGES.txt index ba2bda73b4f..3a190474838 100644 --- a/hadoop-common-project/hadoop-common/CHANGES.txt +++ b/hadoop-common-project/hadoop-common/CHANGES.txt @@ -266,6 +266,9 @@ Release 0.23.1 - Unreleased HADOOP-7963. Fix ViewFS to catch a null canonical service-name and pass tests TestViewFileSystem* (Siddharth Seth via vinodkv) + HADOOP-7964. Deadlock in NetUtils and SecurityUtil class initialization. + (Daryn Sharp via suresh) + Release 0.23.0 - 2011-11-01 INCOMPATIBLE CHANGES diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/net/NetUtils.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/net/NetUtils.java index 752c0be8bc2..f71c1894adc 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/net/NetUtils.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/net/NetUtils.java @@ -51,12 +51,6 @@ import org.apache.hadoop.ipc.VersionedProtocol; import org.apache.hadoop.security.SecurityUtil; import org.apache.hadoop.util.ReflectionUtils; -import com.google.common.annotations.VisibleForTesting; - -//this will need to be replaced someday when there is a suitable replacement -import sun.net.dns.ResolverConfiguration; -import sun.net.util.IPAddressUtil; - @InterfaceAudience.LimitedPrivate({"HDFS", "MapReduce"}) @InterfaceStability.Unstable public class NetUtils { @@ -72,26 +66,6 @@ public class NetUtils { /** Base URL of the Hadoop Wiki: {@value} */ public static final String HADOOP_WIKI = "http://wiki.apache.org/hadoop/"; - private static HostResolver hostResolver; - - static { - // SecurityUtils requires a more secure host resolver if tokens are - // using hostnames - setUseQualifiedHostResolver(!SecurityUtil.getTokenServiceUseIp()); - } - - /** - * This method is intended for use only by SecurityUtils! - * @param flag where the qualified or standard host resolver is used - * to create socket addresses - */ - @InterfaceAudience.Private - public static void setUseQualifiedHostResolver(boolean flag) { - hostResolver = flag - ? new QualifiedHostResolver() - : new StandardHostResolver(); - } - /** * Get the socket factory for the given class according to its * configuration parameter @@ -249,7 +223,7 @@ public class NetUtils { InetSocketAddress addr; try { - InetAddress iaddr = hostResolver.getByName(resolveHost); + InetAddress iaddr = SecurityUtil.getByName(resolveHost); // if there is a static entry for the host, make the returned // address look like the original given host if (staticHost != null) { @@ -261,151 +235,6 @@ public class NetUtils { } return addr; } - - interface HostResolver { - InetAddress getByName(String host) throws UnknownHostException; - } - - /** - * Uses standard java host resolution - */ - static class StandardHostResolver implements HostResolver { - public InetAddress getByName(String host) throws UnknownHostException { - return InetAddress.getByName(host); - } - } - - /** - * This an alternate resolver with important properties that the standard - * java resolver lacks: - * 1) The hostname is fully qualified. This avoids security issues if not - * all hosts in the cluster do not share the same search domains. It - * also prevents other hosts from performing unnecessary dns searches. - * In contrast, InetAddress simply returns the host as given. - * 2) The InetAddress is instantiated with an exact host and IP to prevent - * further unnecessary lookups. InetAddress may perform an unnecessary - * reverse lookup for an IP. - * 3) A call to getHostName() will always return the qualified hostname, or - * more importantly, the IP if instantiated with an IP. This avoids - * unnecessary dns timeouts if the host is not resolvable. - * 4) Point 3 also ensures that if the host is re-resolved, ex. during a - * connection re-attempt, that a reverse lookup to host and forward - * lookup to IP is not performed since the reverse/forward mappings may - * not always return the same IP. If the client initiated a connection - * with an IP, then that IP is all that should ever be contacted. - * - * NOTE: this resolver is only used if: - * hadoop.security.token.service.use_ip=false - */ - protected static class QualifiedHostResolver implements HostResolver { - @SuppressWarnings("unchecked") - private List searchDomains = - ResolverConfiguration.open().searchlist(); - - /** - * Create an InetAddress with a fully qualified hostname of the given - * hostname. InetAddress does not qualify an incomplete hostname that - * is resolved via the domain search list. - * {@link InetAddress#getCanonicalHostName()} will fully qualify the - * hostname, but it always return the A record whereas the given hostname - * may be a CNAME. - * - * @param host a hostname or ip address - * @return InetAddress with the fully qualified hostname or ip - * @throws UnknownHostException if host does not exist - */ - public InetAddress getByName(String host) throws UnknownHostException { - InetAddress addr = null; - - if (IPAddressUtil.isIPv4LiteralAddress(host)) { - // use ipv4 address as-is - byte[] ip = IPAddressUtil.textToNumericFormatV4(host); - addr = InetAddress.getByAddress(host, ip); - } else if (IPAddressUtil.isIPv6LiteralAddress(host)) { - // use ipv6 address as-is - byte[] ip = IPAddressUtil.textToNumericFormatV6(host); - addr = InetAddress.getByAddress(host, ip); - } else if (host.endsWith(".")) { - // a rooted host ends with a dot, ex. "host." - // rooted hosts never use the search path, so only try an exact lookup - addr = getByExactName(host); - } else if (host.contains(".")) { - // the host contains a dot (domain), ex. "host.domain" - // try an exact host lookup, then fallback to search list - addr = getByExactName(host); - if (addr == null) { - addr = getByNameWithSearch(host); - } - } else { - // it's a simple host with no dots, ex. "host" - // try the search list, then fallback to exact host - InetAddress loopback = InetAddress.getByName(null); - if (host.equalsIgnoreCase(loopback.getHostName())) { - addr = InetAddress.getByAddress(host, loopback.getAddress()); - } else { - addr = getByNameWithSearch(host); - if (addr == null) { - addr = getByExactName(host); - } - } - } - // unresolvable! - if (addr == null) { - throw new UnknownHostException(host); - } - return addr; - } - - InetAddress getByExactName(String host) { - InetAddress addr = null; - // InetAddress will use the search list unless the host is rooted - // with a trailing dot. The trailing dot will disable any use of the - // search path in a lower level resolver. See RFC 1535. - String fqHost = host; - if (!fqHost.endsWith(".")) fqHost += "."; - try { - addr = getInetAddressByName(fqHost); - // can't leave the hostname as rooted or other parts of the system - // malfunction, ex. kerberos principals are lacking proper host - // equivalence for rooted/non-rooted hostnames - addr = InetAddress.getByAddress(host, addr.getAddress()); - } catch (UnknownHostException e) { - // ignore, caller will throw if necessary - } - return addr; - } - - InetAddress getByNameWithSearch(String host) { - InetAddress addr = null; - if (host.endsWith(".")) { // already qualified? - addr = getByExactName(host); - } else { - for (String domain : searchDomains) { - String dot = !domain.startsWith(".") ? "." : ""; - addr = getByExactName(host + dot + domain); - if (addr != null) break; - } - } - return addr; - } - - // implemented as a separate method to facilitate unit testing - InetAddress getInetAddressByName(String host) throws UnknownHostException { - return InetAddress.getByName(host); - } - - void setSearchDomains(String ... domains) { - searchDomains = Arrays.asList(domains); - } - } - - /** - * This is for testing only! - */ - @VisibleForTesting - static void setHostResolver(HostResolver newResolver) { - hostResolver = newResolver; - } /** * Resolve the uri's hostname and add the default port if not in the uri @@ -447,7 +276,7 @@ public class NetUtils { String fqHost = canonicalizedHostCache.get(host); if (fqHost == null) { try { - fqHost = hostResolver.getByName(host).getHostName(); + fqHost = SecurityUtil.getByName(host).getHostName(); // slight race condition, but won't hurt canonicalizedHostCache.put(host, fqHost); } catch (UnknownHostException e) { diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/SecurityUtil.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/SecurityUtil.java index a5e8c5d2b55..a72b1f272ca 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/SecurityUtil.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/SecurityUtil.java @@ -23,6 +23,8 @@ import java.net.URI; import java.net.URL; import java.net.UnknownHostException; import java.security.AccessController; +import java.util.Arrays; +import java.util.List; import java.util.ServiceLoader; import java.util.Set; @@ -41,6 +43,11 @@ import org.apache.hadoop.net.NetUtils; import org.apache.hadoop.security.token.Token; import org.apache.hadoop.security.token.TokenInfo; +import com.google.common.annotations.VisibleForTesting; + +//this will need to be replaced someday when there is a suitable replacement +import sun.net.dns.ResolverConfiguration; +import sun.net.util.IPAddressUtil; import sun.security.jgss.krb5.Krb5Util; import sun.security.krb5.Credentials; import sun.security.krb5.PrincipalName; @@ -53,7 +60,10 @@ public class SecurityUtil { // controls whether buildTokenService will use an ip or host/ip as given // by the user - private static boolean useIpForTokenService; + @VisibleForTesting + static boolean useIpForTokenService; + @VisibleForTesting + static HostResolver hostResolver; static { boolean useIp = new Configuration().getBoolean( @@ -68,16 +78,9 @@ public class SecurityUtil { @InterfaceAudience.Private static void setTokenServiceUseIp(boolean flag) { useIpForTokenService = flag; - NetUtils.setUseQualifiedHostResolver(!flag); - } - - /** - * Intended only for temporary use by NetUtils. Do not use. - * @return whether tokens use an IP address - */ - @InterfaceAudience.Private - public static boolean getTokenServiceUseIp() { - return useIpForTokenService; + hostResolver = !useIpForTokenService + ? new QualifiedHostResolver() + : new StandardHostResolver(); } /** @@ -142,7 +145,7 @@ public class SecurityUtil { * it will be removed when the Java behavior is changed. * * @param remoteHost Target URL the krb-https client will access - * @throws IOException + * @throws IOException if the service ticket cannot be retrieved */ public static void fetchServiceTicket(URL remoteHost) throws IOException { if(!UserGroupInformation.isSecurityEnabled()) @@ -179,7 +182,7 @@ public class SecurityUtil { * @param hostname * the fully-qualified domain name used for substitution * @return converted Kerberos principal name - * @throws IOException + * @throws IOException if the client address cannot be determined */ public static String getServerPrincipal(String principalConfig, String hostname) throws IOException { @@ -204,7 +207,7 @@ public class SecurityUtil { * @param addr * InetAddress of the host used for substitution * @return converted Kerberos principal name - * @throws IOException + * @throws IOException if the client address cannot be determined */ public static String getServerPrincipal(String principalConfig, InetAddress addr) throws IOException { @@ -251,7 +254,7 @@ public class SecurityUtil { * the key to look for keytab file in conf * @param userNameKey * the key to look for user's Kerberos principal name in conf - * @throws IOException + * @throws IOException if login fails */ public static void login(final Configuration conf, final String keytabFileKey, final String userNameKey) throws IOException { @@ -271,7 +274,7 @@ public class SecurityUtil { * the key to look for user's Kerberos principal name in conf * @param hostname * hostname to use for substitution - * @throws IOException + * @throws IOException if the config doesn't specify a keytab */ public static void login(final Configuration conf, final String keytabFileKey, final String userNameKey, String hostname) @@ -363,7 +366,7 @@ public class SecurityUtil { * Look up the TokenInfo for a given protocol. It searches all known * SecurityInfo providers. * @param protocol The protocol class to get the information for. - * @conf conf Configuration object + * @param conf Configuration object * @return the TokenInfo or null if it has no KerberosInfo defined */ public static TokenInfo getTokenInfo(Class protocol, Configuration conf) { @@ -442,4 +445,155 @@ public class SecurityUtil { public static Text buildTokenService(URI uri) { return buildTokenService(NetUtils.createSocketAddr(uri.getAuthority())); } + + /** + * Resolves a host subject to the security requirements determined by + * hadoop.security.token.service.use_ip. + * + * @param hostname host or ip to resolve + * @return a resolved host + * @throws UnknownHostException if the host doesn't exist + */ + @InterfaceAudience.Private + public static + InetAddress getByName(String hostname) throws UnknownHostException { + return hostResolver.getByName(hostname); + } + + interface HostResolver { + InetAddress getByName(String host) throws UnknownHostException; + } + + /** + * Uses standard java host resolution + */ + static class StandardHostResolver implements HostResolver { + public InetAddress getByName(String host) throws UnknownHostException { + return InetAddress.getByName(host); + } + } + + /** + * This an alternate resolver with important properties that the standard + * java resolver lacks: + * 1) The hostname is fully qualified. This avoids security issues if not + * all hosts in the cluster do not share the same search domains. It + * also prevents other hosts from performing unnecessary dns searches. + * In contrast, InetAddress simply returns the host as given. + * 2) The InetAddress is instantiated with an exact host and IP to prevent + * further unnecessary lookups. InetAddress may perform an unnecessary + * reverse lookup for an IP. + * 3) A call to getHostName() will always return the qualified hostname, or + * more importantly, the IP if instantiated with an IP. This avoids + * unnecessary dns timeouts if the host is not resolvable. + * 4) Point 3 also ensures that if the host is re-resolved, ex. during a + * connection re-attempt, that a reverse lookup to host and forward + * lookup to IP is not performed since the reverse/forward mappings may + * not always return the same IP. If the client initiated a connection + * with an IP, then that IP is all that should ever be contacted. + * + * NOTE: this resolver is only used if: + * hadoop.security.token.service.use_ip=false + */ + protected static class QualifiedHostResolver implements HostResolver { + @SuppressWarnings("unchecked") + private List searchDomains = + ResolverConfiguration.open().searchlist(); + + /** + * Create an InetAddress with a fully qualified hostname of the given + * hostname. InetAddress does not qualify an incomplete hostname that + * is resolved via the domain search list. + * {@link InetAddress#getCanonicalHostName()} will fully qualify the + * hostname, but it always return the A record whereas the given hostname + * may be a CNAME. + * + * @param host a hostname or ip address + * @return InetAddress with the fully qualified hostname or ip + * @throws UnknownHostException if host does not exist + */ + public InetAddress getByName(String host) throws UnknownHostException { + InetAddress addr = null; + + if (IPAddressUtil.isIPv4LiteralAddress(host)) { + // use ipv4 address as-is + byte[] ip = IPAddressUtil.textToNumericFormatV4(host); + addr = InetAddress.getByAddress(host, ip); + } else if (IPAddressUtil.isIPv6LiteralAddress(host)) { + // use ipv6 address as-is + byte[] ip = IPAddressUtil.textToNumericFormatV6(host); + addr = InetAddress.getByAddress(host, ip); + } else if (host.endsWith(".")) { + // a rooted host ends with a dot, ex. "host." + // rooted hosts never use the search path, so only try an exact lookup + addr = getByExactName(host); + } else if (host.contains(".")) { + // the host contains a dot (domain), ex. "host.domain" + // try an exact host lookup, then fallback to search list + addr = getByExactName(host); + if (addr == null) { + addr = getByNameWithSearch(host); + } + } else { + // it's a simple host with no dots, ex. "host" + // try the search list, then fallback to exact host + InetAddress loopback = InetAddress.getByName(null); + if (host.equalsIgnoreCase(loopback.getHostName())) { + addr = InetAddress.getByAddress(host, loopback.getAddress()); + } else { + addr = getByNameWithSearch(host); + if (addr == null) { + addr = getByExactName(host); + } + } + } + // unresolvable! + if (addr == null) { + throw new UnknownHostException(host); + } + return addr; + } + + InetAddress getByExactName(String host) { + InetAddress addr = null; + // InetAddress will use the search list unless the host is rooted + // with a trailing dot. The trailing dot will disable any use of the + // search path in a lower level resolver. See RFC 1535. + String fqHost = host; + if (!fqHost.endsWith(".")) fqHost += "."; + try { + addr = getInetAddressByName(fqHost); + // can't leave the hostname as rooted or other parts of the system + // malfunction, ex. kerberos principals are lacking proper host + // equivalence for rooted/non-rooted hostnames + addr = InetAddress.getByAddress(host, addr.getAddress()); + } catch (UnknownHostException e) { + // ignore, caller will throw if necessary + } + return addr; + } + + InetAddress getByNameWithSearch(String host) { + InetAddress addr = null; + if (host.endsWith(".")) { // already qualified? + addr = getByExactName(host); + } else { + for (String domain : searchDomains) { + String dot = !domain.startsWith(".") ? "." : ""; + addr = getByExactName(host + dot + domain); + if (addr != null) break; + } + } + return addr; + } + + // implemented as a separate method to facilitate unit testing + InetAddress getInetAddressByName(String host) throws UnknownHostException { + return InetAddress.getByName(host); + } + + void setSearchDomains(String ... domains) { + searchDomains = Arrays.asList(domains); + } + } } diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestFileSystemCanonicalization.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestFileSystemCanonicalization.java index 70c77a51611..8075cd57095 100644 --- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestFileSystemCanonicalization.java +++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestFileSystemCanonicalization.java @@ -25,7 +25,7 @@ import junit.framework.TestCase; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.fs.permission.FsPermission; -import org.apache.hadoop.net.NetUtilsTestResolver; +import org.apache.hadoop.security.NetUtilsTestResolver; import org.apache.hadoop.util.Progressable; import org.junit.Test; diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/net/TestNetUtils.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/net/TestNetUtils.java index 7779980020b..557cdc8c77c 100644 --- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/net/TestNetUtils.java +++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/net/TestNetUtils.java @@ -37,6 +37,7 @@ import org.apache.commons.lang.StringUtils; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.apache.hadoop.conf.Configuration; +import org.apache.hadoop.security.NetUtilsTestResolver; import org.junit.Before; import org.junit.BeforeClass; import org.junit.Test; diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/net/NetUtilsTestResolver.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/NetUtilsTestResolver.java similarity index 80% rename from hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/net/NetUtilsTestResolver.java rename to hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/NetUtilsTestResolver.java index 819264c8b19..087f82429e3 100644 --- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/net/NetUtilsTestResolver.java +++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/NetUtilsTestResolver.java @@ -16,7 +16,7 @@ * limitations under the License. */ -package org.apache.hadoop.net; +package org.apache.hadoop.security; import java.net.InetAddress; import java.net.UnknownHostException; @@ -25,7 +25,7 @@ import java.util.LinkedList; import java.util.List; import java.util.Map; -import org.apache.hadoop.net.NetUtils.QualifiedHostResolver; +import org.apache.hadoop.security.SecurityUtil.QualifiedHostResolver; /** * provides a dummy dns search resolver with a configurable search path @@ -41,7 +41,7 @@ public class NetUtilsTestResolver extends QualifiedHostResolver { resolver.addResolvedHost("host.a.b.", "1.1.1.1"); resolver.addResolvedHost("b-host.b.", "2.2.2.2"); resolver.addResolvedHost("simple.", "3.3.3.3"); - NetUtils.setHostResolver(resolver); + SecurityUtil.hostResolver = resolver; return resolver; } @@ -56,7 +56,8 @@ public class NetUtilsTestResolver extends QualifiedHostResolver { resolvedHosts.put(host, addr); } - InetAddress getInetAddressByName(String host) throws UnknownHostException { + @Override + public InetAddress getInetAddressByName(String host) throws UnknownHostException { hostSearches.add(host); if (!resolvedHosts.containsKey(host)) { throw new UnknownHostException(host); @@ -64,11 +65,21 @@ public class NetUtilsTestResolver extends QualifiedHostResolver { return resolvedHosts.get(host); } - String[] getHostSearches() { + @Override + public InetAddress getByExactName(String host) { + return super.getByExactName(host); + } + + @Override + public InetAddress getByNameWithSearch(String host) { + return super.getByNameWithSearch(host); + } + + public String[] getHostSearches() { return hostSearches.toArray(new String[0]); } - void reset() { + public void reset() { hostSearches.clear(); } } diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/TestSecurityUtil.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/TestSecurityUtil.java index cac0160cbda..84f585b5999 100644 --- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/TestSecurityUtil.java +++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/TestSecurityUtil.java @@ -225,7 +225,7 @@ public class TestSecurityUtil { assertTrue(!addr.isUnresolved()); // don't know what the standard resolver will return for hostname. // should be host for host; host or ip for ip is ambiguous - if (!SecurityUtil.getTokenServiceUseIp()) { + if (!SecurityUtil.useIpForTokenService) { assertEquals(host, addr.getHostName()); assertEquals(host, addr.getAddress().getHostName()); }