HBASE-6851 Fix race condition in TableAuthManager.updateGlobalCache()

git-svn-id: https://svn.apache.org/repos/asf/hbase/trunk@1388894 13f79535-47bb-0310-9956-ffa450edef68
This commit is contained in:
Gary Helmling 2012-09-22 20:35:28 +00:00
parent e30829d8c1
commit 419a8b8f44
2 changed files with 162 additions and 125 deletions

View File

@ -41,20 +41,59 @@ import java.util.concurrent.ConcurrentSkipListMap;
* Performs authorization checks for a given user's assigned permissions * Performs authorization checks for a given user's assigned permissions
*/ */
public class TableAuthManager { public class TableAuthManager {
private static class PermissionCache<T extends Permission> {
/** Cache of user permissions */
private ListMultimap<String,T> userCache = ArrayListMultimap.create();
/** Cache of group permissions */
private ListMultimap<String,T> groupCache = ArrayListMultimap.create();
public List<T> getUser(String user) {
return userCache.get(user);
}
public void putUser(String user, T perm) {
userCache.put(user, perm);
}
public List<T> replaceUser(String user, Iterable<? extends T> perms) {
return userCache.replaceValues(user, perms);
}
public List<T> getGroup(String group) {
return groupCache.get(group);
}
public void putGroup(String group, T perm) {
groupCache.put(group, perm);
}
public List<T> replaceGroup(String group, Iterable<? extends T> perms) {
return groupCache.replaceValues(group, perms);
}
/**
* Returns a combined map of user and group permissions, with group names prefixed by
* {@link AccessControlLists#GROUP_PREFIX}.
*/
public ListMultimap<String,T> getAllPermissions() {
ListMultimap<String,T> tmp = ArrayListMultimap.create();
tmp.putAll(userCache);
for (String group : groupCache.keySet()) {
tmp.putAll(AccessControlLists.GROUP_PREFIX + group, groupCache.get(group));
}
return tmp;
}
}
private static Log LOG = LogFactory.getLog(TableAuthManager.class); private static Log LOG = LogFactory.getLog(TableAuthManager.class);
private static TableAuthManager instance; private static TableAuthManager instance;
/** Cache of global user permissions */ /** Cache of global permissions */
private ListMultimap<String,Permission> USER_CACHE = ArrayListMultimap.create(); private volatile PermissionCache<Permission> globalCache;
/** Cache of global group permissions */
private ListMultimap<String,Permission> GROUP_CACHE = ArrayListMultimap.create();
private ConcurrentSkipListMap<byte[], ListMultimap<String,TablePermission>> TABLE_USER_CACHE = private ConcurrentSkipListMap<byte[], PermissionCache<TablePermission>> tableCache =
new ConcurrentSkipListMap<byte[], ListMultimap<String,TablePermission>>(Bytes.BYTES_COMPARATOR); new ConcurrentSkipListMap<byte[], PermissionCache<TablePermission>>(Bytes.BYTES_COMPARATOR);
private ConcurrentSkipListMap<byte[], ListMultimap<String,TablePermission>> TABLE_GROUP_CACHE =
new ConcurrentSkipListMap<byte[], ListMultimap<String,TablePermission>>(Bytes.BYTES_COMPARATOR);
private Configuration conf; private Configuration conf;
private ZKPermissionWatcher zkperms; private ZKPermissionWatcher zkperms;
@ -70,15 +109,20 @@ public class TableAuthManager {
} }
// initialize global permissions based on configuration // initialize global permissions based on configuration
initGlobal(conf); globalCache = initGlobal(conf);
} }
private void initGlobal(Configuration conf) throws IOException { /**
* Returns a new {@code PermissionCache} initialized with permission assignments
* from the {@code hbase.superuser} configuration key.
*/
private PermissionCache<Permission> initGlobal(Configuration conf) throws IOException {
User user = User.getCurrent(); User user = User.getCurrent();
if (user == null) { if (user == null) {
throw new IOException("Unable to obtain the current user, " + throw new IOException("Unable to obtain the current user, " +
"authorization checks for internal operations will not work correctly!"); "authorization checks for internal operations will not work correctly!");
} }
PermissionCache<Permission> newCache = new PermissionCache<Permission>();
String currentUser = user.getShortName(); String currentUser = user.getShortName();
// the system user is always included // the system user is always included
@ -87,13 +131,14 @@ public class TableAuthManager {
if (superusers != null) { if (superusers != null) {
for (String name : superusers) { for (String name : superusers) {
if (AccessControlLists.isGroupPrincipal(name)) { if (AccessControlLists.isGroupPrincipal(name)) {
GROUP_CACHE.put(AccessControlLists.getGroupName(name), newCache.putGroup(AccessControlLists.getGroupName(name),
new Permission(Permission.Action.values())); new Permission(Permission.Action.values()));
} else { } else {
USER_CACHE.put(name, new Permission(Permission.Action.values())); newCache.putUser(name, new Permission(Permission.Action.values()));
} }
} }
} }
return newCache;
} }
public ZKPermissionWatcher getZKPermissionWatcher() { public ZKPermissionWatcher getZKPermissionWatcher() {
@ -127,22 +172,22 @@ public class TableAuthManager {
* @param userPerms * @param userPerms
*/ */
private void updateGlobalCache(ListMultimap<String,TablePermission> userPerms) { private void updateGlobalCache(ListMultimap<String,TablePermission> userPerms) {
USER_CACHE.clear(); PermissionCache<Permission> newCache = null;
GROUP_CACHE.clear();
try { try {
initGlobal(conf); newCache = initGlobal(conf);
} catch (IOException e) {
// Never happens
LOG.error("Error occured while updating the user cache", e);
}
for (Map.Entry<String,TablePermission> entry : userPerms.entries()) { for (Map.Entry<String,TablePermission> entry : userPerms.entries()) {
if (AccessControlLists.isGroupPrincipal(entry.getKey())) { if (AccessControlLists.isGroupPrincipal(entry.getKey())) {
GROUP_CACHE.put(AccessControlLists.getGroupName(entry.getKey()), newCache.putGroup(AccessControlLists.getGroupName(entry.getKey()),
new Permission(entry.getValue().getActions())); new Permission(entry.getValue().getActions()));
} else { } else {
USER_CACHE.put(entry.getKey(), new Permission(entry.getValue().getActions())); newCache.putUser(entry.getKey(), new Permission(entry.getValue().getActions()));
} }
} }
globalCache = newCache;
} catch (IOException e) {
// Never happens
LOG.error("Error occured while updating the global cache", e);
}
} }
/** /**
@ -154,39 +199,24 @@ public class TableAuthManager {
* @param tablePerms * @param tablePerms
*/ */
private void updateTableCache(byte[] table, ListMultimap<String,TablePermission> tablePerms) { private void updateTableCache(byte[] table, ListMultimap<String,TablePermission> tablePerms) {
// split user from group assignments so we don't have to prepend the group PermissionCache<TablePermission> newTablePerms = new PermissionCache<TablePermission>();
// prefix every time we query for groups
ListMultimap<String,TablePermission> userPerms = ArrayListMultimap.create();
ListMultimap<String,TablePermission> groupPerms = ArrayListMultimap.create();
for (Map.Entry<String,TablePermission> entry : tablePerms.entries()) { for (Map.Entry<String,TablePermission> entry : tablePerms.entries()) {
if (AccessControlLists.isGroupPrincipal(entry.getKey())) { if (AccessControlLists.isGroupPrincipal(entry.getKey())) {
groupPerms.put(AccessControlLists.getGroupName(entry.getKey()), entry.getValue()); newTablePerms.putGroup(AccessControlLists.getGroupName(entry.getKey()), entry.getValue());
} else { } else {
userPerms.put(entry.getKey(), entry.getValue()); newTablePerms.putUser(entry.getKey(), entry.getValue());
} }
} }
TABLE_GROUP_CACHE.put(table, groupPerms); tableCache.put(table, newTablePerms);
TABLE_USER_CACHE.put(table, userPerms);
} }
private List<TablePermission> getUserPermissions(String username, byte[] table) { private PermissionCache<TablePermission> getTablePermissions(byte[] table) {
ListMultimap<String, TablePermission> tablePerms = TABLE_USER_CACHE.get(table); if (!tableCache.containsKey(table)) {
if (tablePerms != null) { tableCache.putIfAbsent(table, new PermissionCache<TablePermission>());
return tablePerms.get(username);
} }
return tableCache.get(table);
return null;
}
private List<TablePermission> getGroupPermissions(String groupName, byte[] table) {
ListMultimap<String, TablePermission> tablePerms = TABLE_GROUP_CACHE.get(table);
if (tablePerms != null) {
return tablePerms.get(groupName);
}
return null;
} }
/** /**
@ -221,14 +251,14 @@ public class TableAuthManager {
return false; return false;
} }
if (authorize(USER_CACHE.get(user.getShortName()), action)) { if (authorize(globalCache.getUser(user.getShortName()), action)) {
return true; return true;
} }
String[] groups = user.getGroupNames(); String[] groups = user.getGroupNames();
if (groups != null) { if (groups != null) {
for (String group : groups) { for (String group : groups) {
if (authorize(GROUP_CACHE.get(group), action)) { if (authorize(globalCache.getGroup(group), action)) {
return true; return true;
} }
} }
@ -257,8 +287,9 @@ public class TableAuthManager {
public boolean authorize(User user, byte[] table, KeyValue kv, public boolean authorize(User user, byte[] table, KeyValue kv,
Permission.Action action) { Permission.Action action) {
List<TablePermission> userPerms = getUserPermissions( PermissionCache<TablePermission> tablePerms = tableCache.get(table);
user.getShortName(), table); if (tablePerms != null) {
List<TablePermission> userPerms = tablePerms.getUser(user.getShortName());
if (authorize(userPerms, table, kv, action)) { if (authorize(userPerms, table, kv, action)) {
return true; return true;
} }
@ -266,13 +297,13 @@ public class TableAuthManager {
String[] groupNames = user.getGroupNames(); String[] groupNames = user.getGroupNames();
if (groupNames != null) { if (groupNames != null) {
for (String group : groupNames) { for (String group : groupNames) {
List<TablePermission> groupPerms = getGroupPermissions(group, table); List<TablePermission> groupPerms = tablePerms.getGroup(group);
if (authorize(groupPerms, table, kv, action)) { if (authorize(groupPerms, table, kv, action)) {
return true; return true;
} }
} }
} }
}
return false; return false;
} }
@ -297,7 +328,7 @@ public class TableAuthManager {
* stored user permissions. * stored user permissions.
*/ */
public boolean authorizeUser(String username, Permission.Action action) { public boolean authorizeUser(String username, Permission.Action action) {
return authorize(USER_CACHE.get(username), action); return authorize(globalCache.getUser(username), action);
} }
/** /**
@ -321,7 +352,7 @@ public class TableAuthManager {
if (authorizeUser(username, action)) { if (authorizeUser(username, action)) {
return true; return true;
} }
return authorize(getUserPermissions(username, table), table, family, return authorize(getTablePermissions(table).getUser(username), table, family,
qualifier, action); qualifier, action);
} }
@ -331,7 +362,7 @@ public class TableAuthManager {
* permissions. * permissions.
*/ */
public boolean authorizeGroup(String groupName, Permission.Action action) { public boolean authorizeGroup(String groupName, Permission.Action action) {
return authorize(GROUP_CACHE.get(groupName), action); return authorize(globalCache.getGroup(groupName), action);
} }
/** /**
@ -349,7 +380,7 @@ public class TableAuthManager {
if (authorizeGroup(groupName, action)) { if (authorizeGroup(groupName, action)) {
return true; return true;
} }
return authorize(getGroupPermissions(groupName, table), table, family, action); return authorize(getTablePermissions(table).getGroup(groupName), table, family, action);
} }
public boolean authorize(User user, byte[] table, byte[] family, public boolean authorize(User user, byte[] table, byte[] family,
@ -382,8 +413,9 @@ public class TableAuthManager {
*/ */
public boolean matchPermission(User user, public boolean matchPermission(User user,
byte[] table, byte[] family, Permission.Action action) { byte[] table, byte[] family, Permission.Action action) {
List<TablePermission> userPerms = getUserPermissions( PermissionCache<TablePermission> tablePerms = tableCache.get(table);
user.getShortName(), table); if (tablePerms != null) {
List<TablePermission> userPerms = tablePerms.getUser(user.getShortName());
if (userPerms != null) { if (userPerms != null) {
for (TablePermission p : userPerms) { for (TablePermission p : userPerms) {
if (p.matchesFamily(table, family, action)) { if (p.matchesFamily(table, family, action)) {
@ -395,7 +427,7 @@ public class TableAuthManager {
String[] groups = user.getGroupNames(); String[] groups = user.getGroupNames();
if (groups != null) { if (groups != null) {
for (String group : groups) { for (String group : groups) {
List<TablePermission> groupPerms = getGroupPermissions(group, table); List<TablePermission> groupPerms = tablePerms.getGroup(group);
if (groupPerms != null) { if (groupPerms != null) {
for (TablePermission p : groupPerms) { for (TablePermission p : groupPerms) {
if (p.matchesFamily(table, family, action)) { if (p.matchesFamily(table, family, action)) {
@ -405,6 +437,7 @@ public class TableAuthManager {
} }
} }
} }
}
return false; return false;
} }
@ -412,8 +445,9 @@ public class TableAuthManager {
public boolean matchPermission(User user, public boolean matchPermission(User user,
byte[] table, byte[] family, byte[] qualifier, byte[] table, byte[] family, byte[] qualifier,
Permission.Action action) { Permission.Action action) {
List<TablePermission> userPerms = getUserPermissions( PermissionCache<TablePermission> tablePerms = tableCache.get(table);
user.getShortName(), table); if (tablePerms != null) {
List<TablePermission> userPerms = tablePerms.getUser(user.getShortName());
if (userPerms != null) { if (userPerms != null) {
for (TablePermission p : userPerms) { for (TablePermission p : userPerms) {
if (p.matchesFamilyQualifier(table, family, qualifier, action)) { if (p.matchesFamilyQualifier(table, family, qualifier, action)) {
@ -425,7 +459,7 @@ public class TableAuthManager {
String[] groups = user.getGroupNames(); String[] groups = user.getGroupNames();
if (groups != null) { if (groups != null) {
for (String group : groups) { for (String group : groups) {
List<TablePermission> groupPerms = getGroupPermissions(group, table); List<TablePermission> groupPerms = tablePerms.getGroup(group);
if (groupPerms != null) { if (groupPerms != null) {
for (TablePermission p : groupPerms) { for (TablePermission p : groupPerms) {
if (p.matchesFamilyQualifier(table, family, qualifier, action)) { if (p.matchesFamilyQualifier(table, family, qualifier, action)) {
@ -435,13 +469,12 @@ public class TableAuthManager {
} }
} }
} }
}
return false; return false;
} }
public void remove(byte[] table) { public void remove(byte[] table) {
TABLE_USER_CACHE.remove(table); tableCache.remove(table);
TABLE_GROUP_CACHE.remove(table);
} }
/** /**
@ -453,13 +486,9 @@ public class TableAuthManager {
*/ */
public void setUserPermissions(String username, byte[] table, public void setUserPermissions(String username, byte[] table,
List<TablePermission> perms) { List<TablePermission> perms) {
ListMultimap<String,TablePermission> tablePerms = TABLE_USER_CACHE.get(table); PermissionCache<TablePermission> tablePerms = getTablePermissions(table);
if (tablePerms == null) { tablePerms.replaceUser(username, perms);
tablePerms = ArrayListMultimap.create(); writeToZooKeeper(table, tablePerms);
TABLE_USER_CACHE.put(table, tablePerms);
}
tablePerms.replaceValues(username, perms);
writeToZooKeeper(table, tablePerms, TABLE_GROUP_CACHE.get(table));
} }
/** /**
@ -471,29 +500,17 @@ public class TableAuthManager {
*/ */
public void setGroupPermissions(String group, byte[] table, public void setGroupPermissions(String group, byte[] table,
List<TablePermission> perms) { List<TablePermission> perms) {
ListMultimap<String,TablePermission> tablePerms = TABLE_GROUP_CACHE.get(table); PermissionCache<TablePermission> tablePerms = getTablePermissions(table);
if (tablePerms == null) { tablePerms.replaceGroup(group, perms);
tablePerms = ArrayListMultimap.create(); writeToZooKeeper(table, tablePerms);
TABLE_GROUP_CACHE.put(table, tablePerms);
}
tablePerms.replaceValues(group, perms);
writeToZooKeeper(table, TABLE_USER_CACHE.get(table), tablePerms);
} }
public void writeToZooKeeper(byte[] table, public void writeToZooKeeper(byte[] table,
ListMultimap<String,TablePermission> userPerms, PermissionCache<TablePermission> tablePerms) {
ListMultimap<String,TablePermission> groupPerms) { byte[] serialized = new byte[0];
ListMultimap<String,TablePermission> tmp = ArrayListMultimap.create(); if (tablePerms != null) {
if (userPerms != null) { serialized = AccessControlLists.writePermissionsAsBytes(tablePerms.getAllPermissions(), conf);
tmp.putAll(userPerms);
} }
if (groupPerms != null) {
for (String group : groupPerms.keySet()) {
tmp.putAll(AccessControlLists.GROUP_PREFIX + group,
groupPerms.get(group));
}
}
byte[] serialized = AccessControlLists.writePermissionsAsBytes(tmp, conf);
zkperms.writeToZookeeper(table, serialized); zkperms.writeToZookeeper(table, serialized);
} }

View File

@ -42,6 +42,7 @@ import org.apache.hadoop.hbase.LargeTests;
import org.apache.hadoop.hbase.client.HBaseAdmin; import org.apache.hadoop.hbase.client.HBaseAdmin;
import org.apache.hadoop.hbase.client.HTable; import org.apache.hadoop.hbase.client.HTable;
import org.apache.hadoop.hbase.client.Put; import org.apache.hadoop.hbase.client.Put;
import org.apache.hadoop.hbase.security.User;
import org.apache.hadoop.hbase.util.Bytes; import org.apache.hadoop.hbase.util.Bytes;
import org.apache.hadoop.hbase.zookeeper.ZooKeeperWatcher; import org.apache.hadoop.hbase.zookeeper.ZooKeeperWatcher;
import org.junit.AfterClass; import org.junit.AfterClass;
@ -357,4 +358,23 @@ public class TestTablePermissions {
}, },
user3Perms.get(0).getActions()); user3Perms.get(0).getActions());
} }
@Test
public void testAuthManager() throws Exception {
Configuration conf = UTIL.getConfiguration();
/* test a race condition causing TableAuthManager to sometimes fail global permissions checks
* when the global cache is being updated
*/
TableAuthManager authManager = TableAuthManager.get(ZKW, conf);
// currently running user is the system user and should have global admin perms
User currentUser = User.getCurrent();
assertTrue(authManager.authorize(currentUser, Permission.Action.ADMIN));
for (int i=1; i<=50; i++) {
AccessControlLists.addUserPermission(conf, new UserPermission(Bytes.toBytes("testauth"+i),
Permission.Action.ADMIN, Permission.Action.READ, Permission.Action.WRITE));
// make sure the system user still shows as authorized
assertTrue("Failed current user auth check on iter "+i,
authManager.authorize(currentUser, Permission.Action.ADMIN));
}
}
} }