Return-Path: X-Original-To: apmail-incubator-accumulo-commits-archive@minotaur.apache.org Delivered-To: apmail-incubator-accumulo-commits-archive@minotaur.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id CABF49369 for ; Thu, 19 Jan 2012 17:15:33 +0000 (UTC) Received: (qmail 94607 invoked by uid 500); 19 Jan 2012 17:15:33 -0000 Delivered-To: apmail-incubator-accumulo-commits-archive@incubator.apache.org Received: (qmail 94566 invoked by uid 500); 19 Jan 2012 17:15:33 -0000 Mailing-List: contact accumulo-commits-help@incubator.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: accumulo-dev@incubator.apache.org Delivered-To: mailing list accumulo-commits@incubator.apache.org Received: (qmail 94559 invoked by uid 99); 19 Jan 2012 17:15:32 -0000 Received: from nike.apache.org (HELO nike.apache.org) (192.87.106.230) by apache.org (qpsmtpd/0.29) with ESMTP; Thu, 19 Jan 2012 17:15:32 +0000 X-ASF-Spam-Status: No, hits=-2000.0 required=5.0 tests=ALL_TRUSTED X-Spam-Check-By: apache.org Received: from [140.211.11.4] (HELO eris.apache.org) (140.211.11.4) by apache.org (qpsmtpd/0.29) with ESMTP; Thu, 19 Jan 2012 17:15:29 +0000 Received: from eris.apache.org (localhost [127.0.0.1]) by eris.apache.org (Postfix) with ESMTP id 1715B238890B; Thu, 19 Jan 2012 17:15:08 +0000 (UTC) Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: svn commit: r1233475 - /incubator/accumulo/branches/1.4/src/server/src/main/java/org/apache/accumulo/server/security/ZKAuthenticator.java Date: Thu, 19 Jan 2012 17:15:08 -0000 To: accumulo-commits@incubator.apache.org From: vines@apache.org X-Mailer: svnmailer-1.0.8-patched Message-Id: <20120119171508.1715B238890B@eris.apache.org> X-Virus-Checked: Checked by ClamAV on apache.org Author: vines Date: Thu Jan 19 17:15:07 2012 New Revision: 1233475 URL: http://svn.apache.org/viewvc?rev=1233475&view=rev Log: ACCUMULO-238 - we try harder to catch potential errors between checking users and pulling their permissions. With the 1.5 changes, for the ZKAuthenticator, it should probably just pull permissions and interpret a lack of existance as user deleted, instead of our current setup which is succeptable to race conditions. We should also take into account ZKStat objects for tracking node information Modified: incubator/accumulo/branches/1.4/src/server/src/main/java/org/apache/accumulo/server/security/ZKAuthenticator.java Modified: incubator/accumulo/branches/1.4/src/server/src/main/java/org/apache/accumulo/server/security/ZKAuthenticator.java URL: http://svn.apache.org/viewvc/incubator/accumulo/branches/1.4/src/server/src/main/java/org/apache/accumulo/server/security/ZKAuthenticator.java?rev=1233475&r1=1233474&r2=1233475&view=diff ============================================================================== --- incubator/accumulo/branches/1.4/src/server/src/main/java/org/apache/accumulo/server/security/ZKAuthenticator.java (original) +++ incubator/accumulo/branches/1.4/src/server/src/main/java/org/apache/accumulo/server/security/ZKAuthenticator.java Thu Jan 19 17:15:07 2012 @@ -338,7 +338,11 @@ public final class ZKAuthenticator imple return Constants.NO_AUTHS; if (userExists(user)) - return Tool.convertAuthorizations(zooCache.get(ZKUserPath + "/" + user + ZKUserAuths)); + try { + return Tool.convertAuthorizations(zooCache.get(ZKUserPath + "/" + user + ZKUserAuths)); + } catch (IllegalArgumentException iae) { + // User was deleted between checking existance and grabbing auths. + } throw new AccumuloSecurityException(user, SecurityErrorCode.USER_DOESNT_EXIST); // user doesn't exist } @@ -400,14 +404,18 @@ public final class ZKAuthenticator imple // Don't let nonexistant users scan if (!userExists(user)) throw new AccumuloSecurityException(user, SecurityErrorCode.USER_DOESNT_EXIST); // user doesn't exist - + // allow anybody to read the METADATA table if (table.equals(Constants.METADATA_TABLE_ID) && permission.equals(TablePermission.READ)) return true; byte[] serializedPerms = zooCache.get(ZKUserPath + "/" + user + ZKUserTablePerms + "/" + table); if (serializedPerms != null) { - return Tool.convertTablePermissions(serializedPerms).contains(permission); + try { + return Tool.convertTablePermissions(serializedPerms).contains(permission); + } catch (IllegalArgumentException iae) { + throw new AccumuloSecurityException(user, SecurityErrorCode.USER_DOESNT_EXIST); // user doesn't exist + } } return false; } @@ -425,8 +433,8 @@ public final class ZKAuthenticator imple throw new AccumuloSecurityException(credentials.user, SecurityErrorCode.GRANT_INVALID); if (userExists(user)) { - Set perms = Tool.convertSystemPermissions(zooCache.get(ZKUserPath + "/" + user + ZKUserSysPerms)); try { + Set perms = Tool.convertSystemPermissions(zooCache.get(ZKUserPath + "/" + user + ZKUserSysPerms)); if (perms.add(permission)) { synchronized (zooCache) { zooCache.clear(); @@ -435,6 +443,10 @@ public final class ZKAuthenticator imple } } log.info("Granted system permission " + permission + " for user " + user + " at the request of user " + credentials.user); + return; + } catch (IllegalArgumentException iae) { + // User was deleted between checking existance and grabbing auths. + // Exception at end handles this } catch (KeeperException e) { log.error(e, e); throw new AccumuloSecurityException(user, SecurityErrorCode.CONNECTION_ERROR, e); @@ -442,10 +454,10 @@ public final class ZKAuthenticator imple log.error(e, e); throw new RuntimeException(e); } - } else - throw new AccumuloSecurityException(credentials.user, SecurityErrorCode.USER_DOESNT_EXIST); // user doesn't exist + } + throw new AccumuloSecurityException(credentials.user, SecurityErrorCode.USER_DOESNT_EXIST); // user doesn't exist } - + @Override public void grantTablePermission(AuthInfo credentials, String user, String table, TablePermission permission) throws AccumuloSecurityException { if (!hasSystemPermission(credentials, credentials.user, SystemPermission.ALTER_USER) @@ -483,7 +495,7 @@ public final class ZKAuthenticator imple } else throw new AccumuloSecurityException(credentials.user, SecurityErrorCode.USER_DOESNT_EXIST); // user doesn't exist } - + @Override public void revokeSystemPermission(AuthInfo credentials, String user, SystemPermission permission) throws AccumuloSecurityException { if (!hasSystemPermission(credentials, credentials.user, SystemPermission.GRANT)) @@ -495,10 +507,10 @@ public final class ZKAuthenticator imple if (permission.equals(SystemPermission.GRANT)) throw new AccumuloSecurityException(credentials.user, SecurityErrorCode.GRANT_INVALID); - + if (userExists(user)) { - Set sysPerms = Tool.convertSystemPermissions(zooCache.get(ZKUserPath + "/" + user + ZKUserSysPerms)); try { + Set sysPerms = Tool.convertSystemPermissions(zooCache.get(ZKUserPath + "/" + user + ZKUserSysPerms)); if (sysPerms.remove(permission)) { synchronized (zooCache) { zooCache.clear(); @@ -507,6 +519,10 @@ public final class ZKAuthenticator imple } } log.info("Revoked system permission " + permission + " for user " + user + " at the request of user " + credentials.user); + } catch (IllegalArgumentException iae) { + // User was deleted between checking and pulling from the zooCache + throw new AccumuloSecurityException(credentials.user, SecurityErrorCode.USER_DOESNT_EXIST); + } catch (KeeperException e) { log.error(e, e); throw new AccumuloSecurityException(user, SecurityErrorCode.CONNECTION_ERROR, e); @@ -517,7 +533,7 @@ public final class ZKAuthenticator imple } else throw new AccumuloSecurityException(credentials.user, SecurityErrorCode.USER_DOESNT_EXIST); } - + @Override public void revokeTablePermission(AuthInfo credentials, String user, String table, TablePermission permission) throws AccumuloSecurityException { if (!hasSystemPermission(credentials, credentials.user, SystemPermission.ALTER_USER) @@ -555,7 +571,7 @@ public final class ZKAuthenticator imple } else throw new AccumuloSecurityException(credentials.user, SecurityErrorCode.USER_DOESNT_EXIST); } - + @Override public void deleteTable(AuthInfo credentials, String table) throws AccumuloSecurityException { if (!hasSystemPermission(credentials, credentials.user, SystemPermission.DROP_TABLE) @@ -577,7 +593,7 @@ public final class ZKAuthenticator imple throw new RuntimeException(e); } } - + /** * All the static too methods used for this class, so that we can separate out stuff that isn't using ZooKeeper. That way, we can check the synchronization * model more easily, as we only need to check to make sure zooCache is cleared when things are written to ZooKeeper in methods that might use it. These @@ -644,7 +660,7 @@ public final class ZKAuthenticator imple public static byte[] convertAuthorizations(Authorizations authorizations) { return authorizations.getAuthorizationsArray(); } - + public static byte[] convertSystemPermissions(Set systempermissions) { ByteArrayOutputStream bytes = new ByteArrayOutputStream(systempermissions.size()); DataOutputStream out = new DataOutputStream(bytes); @@ -692,12 +708,12 @@ public final class ZKAuthenticator imple return toReturn; } } - + @Override public void clearCache(String user) { zooCache.clear(ZKUserPath + "/" + user); } - + @Override public void clearCache(String user, String tableId) { zooCache.clear(ZKUserPath + "/" + user + ZKUserTablePerms + "/" + tableId);