Return-Path: X-Original-To: apmail-hbase-commits-archive@www.apache.org Delivered-To: apmail-hbase-commits-archive@www.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id 43819181F1 for ; Sat, 14 Nov 2015 00:45:03 +0000 (UTC) Received: (qmail 7963 invoked by uid 500); 14 Nov 2015 00:45:03 -0000 Delivered-To: apmail-hbase-commits-archive@hbase.apache.org Received: (qmail 7920 invoked by uid 500); 14 Nov 2015 00:45:03 -0000 Mailing-List: contact commits-help@hbase.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@hbase.apache.org Delivered-To: mailing list commits@hbase.apache.org Received: (qmail 7911 invoked by uid 99); 14 Nov 2015 00:45:03 -0000 Received: from git1-us-west.apache.org (HELO git1-us-west.apache.org) (140.211.11.23) by apache.org (qpsmtpd/0.29) with ESMTP; Sat, 14 Nov 2015 00:45:03 +0000 Received: by git1-us-west.apache.org (ASF Mail Server at git1-us-west.apache.org, from userid 33) id E661BE038A; Sat, 14 Nov 2015 00:45:02 +0000 (UTC) Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: apurtell@apache.org To: commits@hbase.apache.org Date: Sat, 14 Nov 2015 00:45:02 -0000 Message-Id: <81c639b1104f42b4bbbe3c46837de49e@git.apache.org> X-Mailer: ASF-Git Admin Mailer Subject: [1/2] hbase git commit: HBASE-14512 Cache UGI groups Repository: hbase Updated Branches: refs/heads/0.98 0b9602937 -> d29f68dc1 HBASE-14512 Cache UGI groups Summary: Don't create a user for every call. Instead create one user per connection. Then inside of SecureHadoopUser cache the groupNames. This allows HBase to cache empty groups. This is needed since it's much more likely that HBase will be accessed by a user that's not present on the posix /etc/group file. Test Plan: Unit Tests Differential Revision: https://reviews.facebook.net/D47751 Conflicts: hbase-common/src/main/java/org/apache/hadoop/hbase/security/User.java hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/CallRunner.java hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/RpcServer.java Project: http://git-wip-us.apache.org/repos/asf/hbase/repo Commit: http://git-wip-us.apache.org/repos/asf/hbase/commit/8d947730 Tree: http://git-wip-us.apache.org/repos/asf/hbase/tree/8d947730 Diff: http://git-wip-us.apache.org/repos/asf/hbase/diff/8d947730 Branch: refs/heads/0.98 Commit: 8d94773044a68ba601eecd4a08b02c48df5cbaa7 Parents: 0b96029 Author: Elliott Clark Authored: Tue Sep 29 14:17:24 2015 -0700 Committer: Andrew Purtell Committed: Fri Nov 13 14:20:54 2015 -0800 ---------------------------------------------------------------------- .../org/apache/hadoop/hbase/security/User.java | 35 +++++++-- .../hadoop/hbase/security/UserProvider.java | 60 ++++++++++++++- .../org/apache/hadoop/hbase/ipc/CallRunner.java | 7 +- .../org/apache/hadoop/hbase/ipc/RpcServer.java | 79 ++++++++++---------- 4 files changed, 131 insertions(+), 50 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/hbase/blob/8d947730/hbase-common/src/main/java/org/apache/hadoop/hbase/security/User.java ---------------------------------------------------------------------- diff --git a/hbase-common/src/main/java/org/apache/hadoop/hbase/security/User.java b/hbase-common/src/main/java/org/apache/hadoop/hbase/security/User.java index c4d6af4..d017cf3 100644 --- a/hbase-common/src/main/java/org/apache/hadoop/hbase/security/User.java +++ b/hbase-common/src/main/java/org/apache/hadoop/hbase/security/User.java @@ -24,9 +24,10 @@ import java.lang.reflect.UndeclaredThrowableException; import java.security.PrivilegedAction; import java.security.PrivilegedExceptionAction; import java.util.Collection; +import java.util.concurrent.ExecutionException; + +import com.google.common.cache.LoadingCache; -import org.apache.commons.logging.Log; -import org.apache.commons.logging.LogFactory; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.hbase.classification.InterfaceAudience; import org.apache.hadoop.hbase.classification.InterfaceStability; @@ -57,8 +58,6 @@ public abstract class User { public static final String HBASE_SECURITY_AUTHORIZATION_CONF_KEY = "hbase.security.authorization"; - private static Log LOG = LogFactory.getLog(User.class); - protected UserGroupInformation ugi; public UserGroupInformation getUGI() { @@ -285,15 +284,25 @@ public abstract class User { * {@link org.apache.hadoop.security.UserGroupInformation} for secure Hadoop * 0.20 and versions 0.21 and above. */ - private static class SecureHadoopUser extends User { + @InterfaceAudience.Private + public static final class SecureHadoopUser extends User { private String shortName; + private LoadingCache cache; - private SecureHadoopUser() throws IOException { + public SecureHadoopUser() throws IOException { ugi = UserGroupInformation.getCurrentUser(); + this.cache = null; + } + + public SecureHadoopUser(UserGroupInformation ugi) { + this.ugi = ugi; + this.cache = null; } - private SecureHadoopUser(UserGroupInformation ugi) { + public SecureHadoopUser(UserGroupInformation ugi, + LoadingCache cache) { this.ugi = ugi; + this.cache = cache; } @Override @@ -309,6 +318,18 @@ public abstract class User { } @Override + public String[] getGroupNames() { + if (cache != null) { + try { + return this.cache.get(ugi); + } catch (ExecutionException e) { + return new String[0]; + } + } + return ugi.getGroupNames(); + } + + @Override public T runAs(PrivilegedAction action) { return ugi.doAs(action); } http://git-wip-us.apache.org/repos/asf/hbase/blob/8d947730/hbase-common/src/main/java/org/apache/hadoop/hbase/security/UserProvider.java ---------------------------------------------------------------------- diff --git a/hbase-common/src/main/java/org/apache/hadoop/hbase/security/UserProvider.java b/hbase-common/src/main/java/org/apache/hadoop/hbase/security/UserProvider.java index 82f686f..f2897b6 100644 --- a/hbase-common/src/main/java/org/apache/hadoop/hbase/security/UserProvider.java +++ b/hbase-common/src/main/java/org/apache/hadoop/hbase/security/UserProvider.java @@ -18,9 +18,20 @@ package org.apache.hadoop.hbase.security; import java.io.IOException; +import java.util.concurrent.Callable; +import java.util.concurrent.Executors; +import java.util.concurrent.TimeUnit; import org.apache.hadoop.hbase.classification.InterfaceAudience; +import com.google.common.cache.CacheBuilder; +import com.google.common.cache.CacheLoader; +import com.google.common.cache.LoadingCache; +import com.google.common.util.concurrent.ListenableFuture; +import com.google.common.util.concurrent.ListeningExecutorService; +import com.google.common.util.concurrent.MoreExecutors; +import com.google.common.util.concurrent.ThreadFactoryBuilder; import org.apache.hadoop.conf.Configuration; +import org.apache.hadoop.fs.CommonConfigurationKeys; import org.apache.hadoop.hbase.BaseConfigurable; import org.apache.hadoop.security.UserGroupInformation; import org.apache.hadoop.util.ReflectionUtils; @@ -33,6 +44,50 @@ import org.apache.hadoop.util.ReflectionUtils; public class UserProvider extends BaseConfigurable { private static final String USER_PROVIDER_CONF_KEY = "hbase.client.userprovider.class"; + private static final ListeningExecutorService executor = MoreExecutors.listeningDecorator( + Executors.newScheduledThreadPool( + 1, + new ThreadFactoryBuilder().setDaemon(true).setNameFormat("group-cache-%d").build())); + + private LoadingCache groupCache = null; + + + @Override + public void setConf(Configuration conf) { + super.setConf(conf); + long cacheTimeout = + getConf().getLong(CommonConfigurationKeys.HADOOP_SECURITY_GROUPS_CACHE_SECS, + 300) * 1000; + + this.groupCache = CacheBuilder.newBuilder() + // This is the same timeout that hadoop uses. So we'll follow suit. + .refreshAfterWrite(cacheTimeout, TimeUnit.MILLISECONDS) + .expireAfterWrite(10 * cacheTimeout, TimeUnit.MILLISECONDS) + // Set concurrency level equal to the default number of handlers that + // the simple handler spins up. + .concurrencyLevel(20) + // create the loader + // This just delegates to UGI. + .build(new CacheLoader() { + @Override + public String[] load(UserGroupInformation ugi) throws Exception { + return ugi.getGroupNames(); + } + + // Provide the reload function that uses the executor thread. + public ListenableFuture reload(final UserGroupInformation k, + String[] oldValue) throws Exception { + + return executor.submit(new Callable() { + UserGroupInformation userGroupInformation = k; + @Override + public String[] call() throws Exception { + return userGroupInformation.getGroupNames(); + } + }); + } + }); + } /** * Instantiate the {@link UserProvider} specified in the configuration and set the passed @@ -95,7 +150,10 @@ public class UserProvider extends BaseConfigurable { * @return User */ public User create(UserGroupInformation ugi) { - return User.create(ugi); + if (ugi == null) { + return null; + } + return new User.SecureHadoopUser(ugi, groupCache); } /** http://git-wip-us.apache.org/repos/asf/hbase/blob/8d947730/hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/CallRunner.java ---------------------------------------------------------------------- diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/CallRunner.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/CallRunner.java index 49396d7..426ba8d 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/CallRunner.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/CallRunner.java @@ -23,7 +23,6 @@ import org.apache.hadoop.hbase.classification.InterfaceAudience; import org.apache.hadoop.hbase.CellScanner; import org.apache.hadoop.hbase.ipc.RpcServer.Call; import org.apache.hadoop.hbase.monitoring.MonitoredRPCHandler; -import org.apache.hadoop.hbase.monitoring.TaskMonitor; import org.apache.hadoop.hbase.util.Pair; import org.apache.hadoop.security.UserGroupInformation; import org.apache.hadoop.util.StringUtils; @@ -82,9 +81,9 @@ public class CallRunner { } this.status.setStatus("Setting up call"); this.status.setConnection(call.connection.getHostAddress(), call.connection.getRemotePort()); - if (RpcServer.LOG.isDebugEnabled()) { - UserGroupInformation remoteUser = call.connection.user; - RpcServer.LOG.debug(call.toShortString() + " executing as " + + if (RpcServer.LOG.isTraceEnabled()) { + UserGroupInformation remoteUser = call.connection.ugi; + RpcServer.LOG.trace(call.toShortString() + " executing as " + ((remoteUser == null) ? "NULL principal" : remoteUser.getUserName())); } Throwable errorThrowable = null; http://git-wip-us.apache.org/repos/asf/hbase/blob/8d947730/hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/RpcServer.java ---------------------------------------------------------------------- diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/RpcServer.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/RpcServer.java index f637fe4..19ce043 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/RpcServer.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/RpcServer.java @@ -337,11 +337,7 @@ public class RpcServer implements RpcServerInterface { this.isError = false; this.size = size; this.tinfo = tinfo; - if (connection != null && connection.user != null) { - this.user = userProvider.create(connection.user); - } else { - this.user = null; - } + this.user = connection.user; this.remoteAddress = remoteAddress; } @@ -368,22 +364,6 @@ public class RpcServer implements RpcServerInterface { return this.header; } - @Override - public User getRequestUser() { - return user; - } - - @Override - public String getRequestUserName() { - User user = getRequestUser(); - return user == null? null: user.getShortName(); - } - - @Override - public InetAddress getRemoteAddress() { - return remoteAddress; - } - /* * Short string representation without param info because param itself could be huge depends on * the payload of a command @@ -569,6 +549,26 @@ public class RpcServer implements RpcServerInterface { this.responder.doRespond(this); } } + + public UserGroupInformation getRemoteUser() { + return connection.ugi; + } + + @Override + public User getRequestUser() { + return user; + } + + @Override + public String getRequestUserName() { + User user = getRequestUser(); + return user == null? null: user.getShortName(); + } + + @Override + public InetAddress getRemoteAddress() { + return remoteAddress; + } } /** Listens on the socket. Creates jobs for the handler threads*/ @@ -1215,7 +1215,7 @@ public class RpcServer implements RpcServerInterface { */ private CompressionCodec compressionCodec; BlockingService service; - protected UserGroupInformation user = null; + private AuthMethod authMethod; private boolean saslContextEstablished; private boolean skipInitialSaslHandshake; @@ -1241,6 +1241,8 @@ public class RpcServer implements RpcServerInterface { private boolean authenticatedWithFallback; public UserGroupInformation attemptingUser = null; // user name before auth + protected User user = null; + protected UserGroupInformation ugi = null; public Connection(SocketChannel channel, long lastContact) { this.channel = channel; @@ -1422,14 +1424,14 @@ public class RpcServer implements RpcServerInterface { if (saslServer.isComplete()) { String qop = (String) saslServer.getNegotiatedProperty(Sasl.QOP); useWrap = qop != null && !"auth".equalsIgnoreCase(qop); - user = getAuthorizedUgi(saslServer.getAuthorizationID()); + ugi = getAuthorizedUgi(saslServer.getAuthorizationID()); if (LOG.isDebugEnabled()) { LOG.debug("SASL server context established. Authenticated client: " - + user + ". Negotiated QoP is " + + ugi + ". Negotiated QoP is " + saslServer.getNegotiatedProperty(Sasl.QOP)); } metrics.authenticationSuccess(); - AUDITLOG.info(AUTH_SUCCESSFUL_FOR + user); + AUDITLOG.info(AUTH_SUCCESSFUL_FOR + ugi); saslContextEstablished = true; } } @@ -1630,9 +1632,9 @@ public class RpcServer implements RpcServerInterface { setupCellBlockCodecs(this.connectionHeader); UserGroupInformation protocolUser = createUser(connectionHeader); if (!useSasl) { - user = protocolUser; - if (user != null) { - user.setAuthenticationMethod(AuthMethod.SIMPLE.authenticationMethod); + ugi = protocolUser; + if (ugi != null) { + ugi.setAuthenticationMethod(AuthMethod.SIMPLE.authenticationMethod); } // audit logging for SASL authenticated users happens in saslReadAndProcess() if (authenticatedWithFallback) { @@ -1642,26 +1644,26 @@ public class RpcServer implements RpcServerInterface { AUDITLOG.info(AUTH_SUCCESSFUL_FOR + user); } else { // user is authenticated - user.setAuthenticationMethod(authMethod.authenticationMethod); + ugi.setAuthenticationMethod(authMethod.authenticationMethod); //Now we check if this is a proxy user case. If the protocol user is //different from the 'user', it is a proxy user scenario. However, //this is not allowed if user authenticated with DIGEST. if ((protocolUser != null) - && (!protocolUser.getUserName().equals(user.getUserName()))) { + && (!protocolUser.getUserName().equals(ugi.getUserName()))) { if (authMethod == AuthMethod.DIGEST) { // Not allowed to doAs if token authentication is used - throw new AccessDeniedException("Authenticated user (" + user + throw new AccessDeniedException("Authenticated user (" + ugi + ") doesn't match what the client claims to be (" + protocolUser + ")"); } else { // Effective user can be different from authenticated user // for simple auth or kerberos auth // The user is the real user. Now we create a proxy user - UserGroupInformation realUser = user; - user = UserGroupInformation.createProxyUser(protocolUser + UserGroupInformation realUser = ugi; + ugi = UserGroupInformation.createProxyUser(protocolUser .getUserName(), realUser); // Now the user is a proxy user, set Authentication method Proxy. - user.setAuthenticationMethod(AuthenticationMethod.PROXY); + ugi.setAuthenticationMethod(AuthenticationMethod.PROXY); } } } @@ -1748,8 +1750,9 @@ public class RpcServer implements RpcServerInterface { // Throw FatalConnectionException wrapping ACE so client does right thing and closes // down the connection instead of trying to read non-existent retun. throw new AccessDeniedException("Connection from " + this + " for service " + - connectionHeader.getServiceName() + " is unauthorized for user: " + user); + connectionHeader.getServiceName() + " is unauthorized for user: " + ugi); } + this.user = userProvider.create(this.ugi); } } @@ -1855,11 +1858,11 @@ public class RpcServer implements RpcServerInterface { // real user for the effective user, therefore not required to // authorize real user. doAs is allowed only for simple or kerberos // authentication - if (user != null && user.getRealUser() != null + if (ugi != null && ugi.getRealUser() != null && (authMethod != AuthMethod.DIGEST)) { - ProxyUsers.authorize(user, this.getHostAddress(), conf); + ProxyUsers.authorize(ugi, this.getHostAddress(), conf); } - authorize(user, connectionHeader, getHostInetAddress()); + authorize(ugi, connectionHeader, getHostInetAddress()); if (LOG.isDebugEnabled()) { LOG.debug("Authorized " + TextFormat.shortDebugString(connectionHeader)); }