You are viewing a plain text version of this content. The canonical link for it is here.
Posted to common-commits@hadoop.apache.org by to...@apache.org on 2011/01/18 22:59:19 UTC
svn commit: r1060594 - in /hadoop/common/branches/branch-0.22: ./
src/java/org/apache/hadoop/ipc/ src/java/org/apache/hadoop/security/
src/java/org/apache/hadoop/security/authorize/
src/test/core/org/apache/hadoop/security/
Author: todd
Date: Tue Jan 18 21:59:18 2011
New Revision: 1060594
URL: http://svn.apache.org/viewvc?rev=1060594&view=rev
Log:
HADOOP-7104. Remove unnecessary DNS reverse lookups from RPC layer. Contributed by Kan Zhang
Modified:
hadoop/common/branches/branch-0.22/CHANGES.txt
hadoop/common/branches/branch-0.22/src/java/org/apache/hadoop/ipc/Client.java
hadoop/common/branches/branch-0.22/src/java/org/apache/hadoop/ipc/Server.java
hadoop/common/branches/branch-0.22/src/java/org/apache/hadoop/security/SecurityUtil.java
hadoop/common/branches/branch-0.22/src/java/org/apache/hadoop/security/authorize/ServiceAuthorizationManager.java
hadoop/common/branches/branch-0.22/src/test/core/org/apache/hadoop/security/TestSecurityUtil.java
Modified: hadoop/common/branches/branch-0.22/CHANGES.txt
URL: http://svn.apache.org/viewvc/hadoop/common/branches/branch-0.22/CHANGES.txt?rev=1060594&r1=1060593&r2=1060594&view=diff
==============================================================================
--- hadoop/common/branches/branch-0.22/CHANGES.txt (original)
+++ hadoop/common/branches/branch-0.22/CHANGES.txt Tue Jan 18 21:59:18 2011
@@ -201,6 +201,9 @@ Release 0.22.0 - Unreleased
HADOOP-7102. Remove "fs.ramfs.impl" field from core-deafult.xml (shv)
+ HADOOP-7104. Remove unnecessary DNS reverse lookups from RPC layer
+ (Kan Zhang via todd)
+
OPTIMIZATIONS
HADOOP-6884. Add LOG.isDebugEnabled() guard for each LOG.debug(..).
Modified: hadoop/common/branches/branch-0.22/src/java/org/apache/hadoop/ipc/Client.java
URL: http://svn.apache.org/viewvc/hadoop/common/branches/branch-0.22/src/java/org/apache/hadoop/ipc/Client.java?rev=1060594&r1=1060593&r2=1060594&view=diff
==============================================================================
--- hadoop/common/branches/branch-0.22/src/java/org/apache/hadoop/ipc/Client.java (original)
+++ hadoop/common/branches/branch-0.22/src/java/org/apache/hadoop/ipc/Client.java Tue Jan 18 21:59:18 2011
@@ -1268,7 +1268,7 @@ public class Client {
+ protocol.getCanonicalName());
}
return SecurityUtil.getServerPrincipal(conf.get(serverKey), address
- .getAddress().getCanonicalHostName());
+ .getAddress());
}
return null;
}
Modified: hadoop/common/branches/branch-0.22/src/java/org/apache/hadoop/ipc/Server.java
URL: http://svn.apache.org/viewvc/hadoop/common/branches/branch-0.22/src/java/org/apache/hadoop/ipc/Server.java?rev=1060594&r1=1060593&r2=1060594&view=diff
==============================================================================
--- hadoop/common/branches/branch-0.22/src/java/org/apache/hadoop/ipc/Server.java (original)
+++ hadoop/common/branches/branch-0.22/src/java/org/apache/hadoop/ipc/Server.java Tue Jan 18 21:59:18 2011
@@ -851,8 +851,8 @@ public abstract class Server {
// Cache the remote host & port info so that even if the socket is
// disconnected, we can say where it used to connect to.
private String hostAddress;
- private String hostName;
private int remotePort;
+ private InetAddress addr;
ConnectionHeader header = new ConnectionHeader();
Class<?> protocol;
@@ -889,12 +889,11 @@ public abstract class Server {
this.unwrappedData = null;
this.unwrappedDataLengthBuffer = ByteBuffer.allocate(4);
this.socket = channel.socket();
- InetAddress addr = socket.getInetAddress();
+ this.addr = socket.getInetAddress();
if (addr == null) {
this.hostAddress = "*Unknown*";
} else {
this.hostAddress = addr.getHostAddress();
- this.hostName = addr.getCanonicalHostName();
}
this.remotePort = socket.getPort();
this.responseQueue = new LinkedList<Call>();
@@ -917,8 +916,8 @@ public abstract class Server {
return hostAddress;
}
- public String getHostName() {
- return hostName;
+ public InetAddress getHostInetAddress() {
+ return addr;
}
public void setLastContact(long lastContact) {
@@ -1326,7 +1325,7 @@ public abstract class Server {
&& (authMethod != AuthMethod.DIGEST)) {
ProxyUsers.authorize(user, this.getHostAddress(), conf);
}
- authorize(user, header, getHostName());
+ authorize(user, header, getHostInetAddress());
if (LOG.isDebugEnabled()) {
LOG.debug("Successfully authorized " + header);
}
@@ -1656,12 +1655,12 @@ public abstract class Server {
*
* @param user client user
* @param connection incoming connection
- * @param hostname fully-qualified domain name of incoming connection
+ * @param addr InetAddress of incoming connection
* @throws AuthorizationException when the client isn't authorized to talk the protocol
*/
public void authorize(UserGroupInformation user,
ConnectionHeader connection,
- String hostname
+ InetAddress addr
) throws AuthorizationException {
if (authorize) {
Class<?> protocol = null;
@@ -1671,7 +1670,7 @@ public abstract class Server {
throw new AuthorizationException("Unknown protocol: " +
connection.getProtocol());
}
- serviceAuthorizationManager.authorize(user, protocol, getConf(), hostname);
+ serviceAuthorizationManager.authorize(user, protocol, getConf(), addr);
}
}
Modified: hadoop/common/branches/branch-0.22/src/java/org/apache/hadoop/security/SecurityUtil.java
URL: http://svn.apache.org/viewvc/hadoop/common/branches/branch-0.22/src/java/org/apache/hadoop/security/SecurityUtil.java?rev=1060594&r1=1060593&r2=1060594&view=diff
==============================================================================
--- hadoop/common/branches/branch-0.22/src/java/org/apache/hadoop/security/SecurityUtil.java (original)
+++ hadoop/common/branches/branch-0.22/src/java/org/apache/hadoop/security/SecurityUtil.java Tue Jan 18 21:59:18 2011
@@ -135,8 +135,8 @@ public class SecurityUtil {
}
/**
- * Convert Kerberos principal name conf values to valid Kerberos principal
- * names. It replaces $host in the conf values with hostname, which should be
+ * Convert Kerberos principal name pattern to valid Kerberos principal
+ * names. It replaces hostname pattern with hostname, which should be
* fully-qualified domain name. If hostname is null or "0.0.0.0", it uses
* dynamically looked-up fqdn of the current host instead.
*
@@ -149,24 +149,57 @@ public class SecurityUtil {
*/
public static String getServerPrincipal(String principalConfig,
String hostname) throws IOException {
- if (principalConfig == null)
- return null;
- String[] components = principalConfig.split("[/@]");
- if (components.length != 3) {
- throw new IOException(
- "Kerberos service principal name isn't configured properly "
- + "(should have 3 parts): " + principalConfig);
- }
-
- if (components[1].equals(HOSTNAME_PATTERN)) {
- String fqdn = hostname;
- if (fqdn == null || fqdn.equals("") || fqdn.equals("0.0.0.0")) {
- fqdn = getLocalHostName();
- }
- return components[0] + "/" + fqdn + "@" + components[2];
+ String[] components = getComponents(principalConfig);
+ if (components == null || components.length != 3
+ || !components[1].equals(HOSTNAME_PATTERN)) {
+ return principalConfig;
} else {
+ return replacePattern(components, hostname);
+ }
+ }
+
+ /**
+ * Convert Kerberos principal name pattern to valid Kerberos principal names.
+ * This method is similar to {@link #getServerPrincipal(String, String)},
+ * except 1) the reverse DNS lookup from addr to hostname is done only when
+ * necessary, 2) param addr can't be null (no default behavior of using local
+ * hostname when addr is null).
+ *
+ * @param principalConfig
+ * Kerberos principal name pattern to convert
+ * @param addr
+ * InetAddress of the host used for substitution
+ * @return converted Kerberos principal name
+ * @throws IOException
+ */
+ public static String getServerPrincipal(String principalConfig,
+ InetAddress addr) throws IOException {
+ String[] components = getComponents(principalConfig);
+ if (components == null || components.length != 3
+ || !components[1].equals(HOSTNAME_PATTERN)) {
return principalConfig;
+ } else {
+ if (addr == null) {
+ throw new IOException("Can't replace " + HOSTNAME_PATTERN
+ + " pattern since client address is null");
+ }
+ return replacePattern(components, addr.getCanonicalHostName());
+ }
+ }
+
+ private static String[] getComponents(String principalConfig) {
+ if (principalConfig == null)
+ return null;
+ return principalConfig.split("[/@]");
+ }
+
+ private static String replacePattern(String[] components, String hostname)
+ throws IOException {
+ String fqdn = hostname;
+ if (fqdn == null || fqdn.equals("") || fqdn.equals("0.0.0.0")) {
+ fqdn = getLocalHostName();
}
+ return components[0] + "/" + fqdn + "@" + components[2];
}
static String getLocalHostName() throws UnknownHostException {
Modified: hadoop/common/branches/branch-0.22/src/java/org/apache/hadoop/security/authorize/ServiceAuthorizationManager.java
URL: http://svn.apache.org/viewvc/hadoop/common/branches/branch-0.22/src/java/org/apache/hadoop/security/authorize/ServiceAuthorizationManager.java?rev=1060594&r1=1060593&r2=1060594&view=diff
==============================================================================
--- hadoop/common/branches/branch-0.22/src/java/org/apache/hadoop/security/authorize/ServiceAuthorizationManager.java (original)
+++ hadoop/common/branches/branch-0.22/src/java/org/apache/hadoop/security/authorize/ServiceAuthorizationManager.java Tue Jan 18 21:59:18 2011
@@ -18,6 +18,7 @@
package org.apache.hadoop.security.authorize;
import java.io.IOException;
+import java.net.InetAddress;
import java.util.IdentityHashMap;
import java.util.Map;
import java.util.Set;
@@ -30,7 +31,6 @@ import org.apache.hadoop.conf.Configurat
import org.apache.hadoop.fs.CommonConfigurationKeys;
import org.apache.hadoop.security.KerberosInfo;
import org.apache.hadoop.security.SecurityUtil;
-import org.apache.hadoop.security.KerberosName;
import org.apache.hadoop.security.UserGroupInformation;
/**
@@ -71,13 +71,13 @@ public class ServiceAuthorizationManager
* @param user user accessing the service
* @param protocol service being accessed
* @param conf configuration to use
- * @param hostname fully qualified domain name of the client
+ * @param addr InetAddress of the client
* @throws AuthorizationException on authorization failure
*/
public void authorize(UserGroupInformation user,
Class<?> protocol,
Configuration conf,
- String hostname
+ InetAddress addr
) throws AuthorizationException {
AccessControlList acl = protocolToAcl.get(protocol);
if (acl == null) {
@@ -91,41 +91,24 @@ public class ServiceAuthorizationManager
if (krbInfo != null) {
String clientKey = krbInfo.clientPrincipal();
if (clientKey != null && !clientKey.equals("")) {
- if (hostname == null) {
- throw new AuthorizationException(
- "Can't authorize client when client hostname is null");
- }
try {
clientPrincipal = SecurityUtil.getServerPrincipal(
- conf.get(clientKey), hostname);
+ conf.get(clientKey), addr);
} catch (IOException e) {
throw (AuthorizationException) new AuthorizationException(
"Can't figure out Kerberos principal name for connection from "
- + hostname + " for user=" + user + " protocol=" + protocol)
+ + addr + " for user=" + user + " protocol=" + protocol)
.initCause(e);
}
}
}
- // when authorizing use the short name only
- String shortName = clientPrincipal;
- if(clientPrincipal != null ) {
- try {
- shortName = new KerberosName(clientPrincipal).getShortName();
- } catch (IOException e) {
- LOG.warn("couldn't get short name from " + clientPrincipal, e);
- // just keep going
- }
- }
- if(LOG.isDebugEnabled()) {
- LOG.debug("for protocol authorization compare (" + clientPrincipal +
- "): " + shortName + " with " + user.getShortUserName());
- }
- if((shortName != null && !shortName.equals(user.getShortUserName())) ||
+ if((clientPrincipal != null && !clientPrincipal.equals(user.getUserName())) ||
!acl.isUserAllowed(user)) {
- AUDITLOG.warn(AUTHZ_FAILED_FOR + user + " for protocol="+protocol);
+ AUDITLOG.warn(AUTHZ_FAILED_FOR + user + " for protocol=" + protocol
+ + ", expected client Kerberos principal is " + clientPrincipal);
throw new AuthorizationException("User " + user +
- " is not authorized for protocol " +
- protocol);
+ " is not authorized for protocol " + protocol +
+ ", expected client Kerberos principal is " + clientPrincipal);
}
AUDITLOG.info(AUTHZ_SUCCESSFULL_FOR + user + " for protocol="+protocol);
}
Modified: hadoop/common/branches/branch-0.22/src/test/core/org/apache/hadoop/security/TestSecurityUtil.java
URL: http://svn.apache.org/viewvc/hadoop/common/branches/branch-0.22/src/test/core/org/apache/hadoop/security/TestSecurityUtil.java?rev=1060594&r1=1060593&r2=1060594&view=diff
==============================================================================
--- hadoop/common/branches/branch-0.22/src/test/core/org/apache/hadoop/security/TestSecurityUtil.java (original)
+++ hadoop/common/branches/branch-0.22/src/test/core/org/apache/hadoop/security/TestSecurityUtil.java Tue Jan 18 21:59:18 2011
@@ -18,14 +18,16 @@ package org.apache.hadoop.security;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.assertEquals;
import java.io.IOException;
+import java.net.InetAddress;
import javax.security.auth.kerberos.KerberosPrincipal;
import org.apache.hadoop.conf.Configuration;
-import org.junit.Assert;
import org.junit.Test;
+import org.mockito.Mockito;
public class TestSecurityUtil {
@Test
@@ -50,28 +52,47 @@ public class TestSecurityUtil {
private void verify(String original, String hostname, String expected)
throws IOException {
- assertTrue(SecurityUtil.getServerPrincipal(original, hostname).equals(
- expected));
- assertTrue(SecurityUtil.getServerPrincipal(original, null).equals(
- expected));
- assertTrue(SecurityUtil.getServerPrincipal(original, "").equals(
- expected));
- assertTrue(SecurityUtil.getServerPrincipal(original, "0.0.0.0").equals(
- expected));
+ assertEquals(expected,
+ SecurityUtil.getServerPrincipal(original, hostname));
+
+ InetAddress addr = mockAddr(hostname);
+ assertEquals(expected,
+ SecurityUtil.getServerPrincipal(original, addr));
}
+ private InetAddress mockAddr(String reverseTo) {
+ InetAddress mock = Mockito.mock(InetAddress.class);
+ Mockito.doReturn(reverseTo).when(mock).getCanonicalHostName();
+ return mock;
+ }
+
@Test
public void testGetServerPrincipal() throws IOException {
String service = "hdfs/";
String realm = "@REALM";
- String hostname = SecurityUtil.getLocalHostName();
+ String hostname = "foohost";
+ String userPrincipal = "foo@FOOREALM";
String shouldReplace = service + SecurityUtil.HOSTNAME_PATTERN + realm;
String replaced = service + hostname + realm;
verify(shouldReplace, hostname, replaced);
String shouldNotReplace = service + SecurityUtil.HOSTNAME_PATTERN + "NAME"
+ realm;
verify(shouldNotReplace, hostname, shouldNotReplace);
- verify(shouldNotReplace, shouldNotReplace, shouldNotReplace);
+ verify(userPrincipal, hostname, userPrincipal);
+ // testing reverse DNS lookup doesn't happen
+ InetAddress notUsed = Mockito.mock(InetAddress.class);
+ assertEquals(shouldNotReplace,
+ SecurityUtil.getServerPrincipal(shouldNotReplace, notUsed));
+ Mockito.verify(notUsed, Mockito.never()).getCanonicalHostName();
+ }
+
+ @Test
+ public void testLocalHostNameForNullOrWild() throws Exception {
+ String local = SecurityUtil.getLocalHostName();
+ assertEquals("hdfs/" + local + "@REALM",
+ SecurityUtil.getServerPrincipal("hdfs/_HOST@REALM", (String)null));
+ assertEquals("hdfs/" + local + "@REALM",
+ SecurityUtil.getServerPrincipal("hdfs/_HOST@REALM", "0.0.0.0"));
}
@Test