From 535ebbfde9e424fab27f79bfa6b40964c4918fc6 Mon Sep 17 00:00:00 2001 From: Matteo Bertozzi Date: Thu, 19 Mar 2015 07:01:21 +0000 Subject: [PATCH] HBASE-13235 Revisit the security auditing semantics (Srikanth Srungarapu) --- .../org/apache/hadoop/hbase/TableName.java | 13 +++ .../security/access/AccessController.java | 88 +++++++++++++++++-- .../hbase/security/access/AuthResult.java | 77 ++++++++++++++-- 3 files changed, 166 insertions(+), 12 deletions(-) diff --git a/hbase-common/src/main/java/org/apache/hadoop/hbase/TableName.java b/hbase-common/src/main/java/org/apache/hadoop/hbase/TableName.java index 245bef569eb..17fd3b7e68b 100644 --- a/hbase-common/src/main/java/org/apache/hadoop/hbase/TableName.java +++ b/hbase-common/src/main/java/org/apache/hadoop/hbase/TableName.java @@ -241,6 +241,19 @@ public final class TableName implements Comparable { return namespaceAsString; } + /** + * Ideally, getNameAsString should contain namespace within it, + * but if the namespace is default, it just returns the name. This method + * takes care of this corner case. + */ + public String getNameWithNamespaceInclAsString() { + if(getNamespaceAsString().equals(NamespaceDescriptor.DEFAULT_NAMESPACE_NAME_STR)) { + return NamespaceDescriptor.DEFAULT_NAMESPACE_NAME_STR + + TableName.NAMESPACE_DELIM + getNameAsString(); + } + return getNameAsString(); + } + public byte[] getQualifier() { return qualifier; } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessController.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessController.java index 30294131883..140534df099 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessController.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessController.java @@ -431,6 +431,39 @@ public class AccessController extends BaseMasterAndRegionObserver } } + /** + * Authorizes that the current user has any of the given permissions for the + * given table, column family and column qualifier. + * @param tableName Table requested + * @param family Column family param + * @param qualifier Column qualifier param + * @throws IOException if obtaining the current user fails + * @throws AccessDeniedException if user has no authorization + */ + private void requireTablePermission(String request, TableName tableName, byte[] family, + byte[] qualifier, Action... permissions) throws IOException { + User user = getActiveUser(); + AuthResult result = null; + + for (Action permission : permissions) { + if (authManager.authorize(user, tableName, null, null, permission)) { + result = AuthResult.allow(request, "Table permission granted", user, + permission, tableName, null, null); + result.getParams().setFamily(family).setQualifier(qualifier); + break; + } else { + // rest of the world + result = AuthResult.deny(request, "Insufficient permissions", user, + permission, tableName, family, qualifier); + result.getParams().setFamily(family).setQualifier(qualifier); + } + } + logResult(result); + if (!result.isAllowed()) { + throw new AccessDeniedException("Insufficient permissions " + result.toContextString()); + } + } + /** * Authorizes that the current user has any of the given permissions to access the table. * @@ -507,10 +540,15 @@ public class AccessController extends BaseMasterAndRegionObserver private void requireGlobalPermission(String request, Action perm, TableName tableName, Map> familyMap) throws IOException { User user = getActiveUser(); + AuthResult result = null; if (authManager.authorize(user, perm)) { - logResult(AuthResult.allow(request, "Global check allowed", user, perm, tableName, familyMap)); + result = AuthResult.allow(request, "Global check allowed", user, perm, tableName, familyMap); + result.getParams().setTableName(tableName).setFamilies(familyMap); + logResult(result); } else { - logResult(AuthResult.deny(request, "Global check failed", user, perm, tableName, familyMap)); + result = AuthResult.deny(request, "Global check failed", user, perm, tableName, familyMap); + result.getParams().setTableName(tableName).setFamilies(familyMap); + logResult(result); throw new AccessDeniedException("Insufficient permissions for user '" + (user != null ? user.getShortName() : "null") +"' (global, action=" + perm.toString() + ")"); @@ -527,10 +565,15 @@ public class AccessController extends BaseMasterAndRegionObserver private void requireGlobalPermission(String request, Action perm, String namespace) throws IOException { User user = getActiveUser(); + AuthResult authResult = null; if (authManager.authorize(user, perm)) { - logResult(AuthResult.allow(request, "Global check allowed", user, perm, namespace)); + authResult = AuthResult.allow(request, "Global check allowed", user, perm, null); + authResult.getParams().setNamespace(namespace); + logResult(authResult); } else { - logResult(AuthResult.deny(request, "Global check failed", user, perm, namespace)); + authResult = AuthResult.deny(request, "Global check failed", user, perm, null); + authResult.getParams().setNamespace(namespace); + logResult(authResult); throw new AccessDeniedException("Insufficient permissions for user '" + (user != null ? user.getShortName() : "null") +"' (global, action=" + perm.toString() + ")"); @@ -565,6 +608,37 @@ public class AccessController extends BaseMasterAndRegionObserver } } + /** + * Checks that the user has the given global or namespace permission. + * @param namespace + * @param permissions Actions being requested + */ + public void requireNamespacePermission(String request, String namespace, TableName tableName, + Map> familyMap, Action... permissions) + throws IOException { + User user = getActiveUser(); + AuthResult result = null; + + for (Action permission : permissions) { + if (authManager.authorize(user, namespace, permission)) { + result = AuthResult.allow(request, "Namespace permission granted", + user, permission, namespace); + result.getParams().setTableName(tableName).setFamilies(familyMap); + break; + } else { + // rest of the world + result = AuthResult.deny(request, "Insufficient permissions", user, + permission, namespace); + result.getParams().setTableName(tableName).setFamilies(familyMap); + } + } + logResult(result); + if (!result.isAllowed()) { + throw new AccessDeniedException("Insufficient permissions " + + result.toContextString()); + } + } + /** * Returns true if the current user is allowed the given action * over at least one of the column qualifiers in the given column families. @@ -919,7 +993,8 @@ public class AccessController extends BaseMasterAndRegionObserver for (byte[] family: families) { familyMap.put(family, null); } - requireNamespacePermission("createTable", desc.getTableName().getNamespaceAsString(), Action.CREATE); + requireNamespacePermission("createTable", desc.getTableName().getNamespaceAsString(), + desc.getTableName(), familyMap, Action.CREATE); } @Override @@ -1050,7 +1125,8 @@ public class AccessController extends BaseMasterAndRegionObserver @Override public void preAddColumn(ObserverContext c, TableName tableName, HColumnDescriptor column) throws IOException { - requirePermission("addColumn", tableName, null, null, Action.ADMIN, Action.CREATE); + requireTablePermission("addColumn", tableName, column.getName(), null, Action.ADMIN, + Action.CREATE); } @Override diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AuthResult.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AuthResult.java index c4bc35ed8f9..bf05dc10b89 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AuthResult.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AuthResult.java @@ -27,6 +27,8 @@ import org.apache.hadoop.hbase.TableName; import org.apache.hadoop.hbase.security.User; import org.apache.hadoop.hbase.util.Bytes; +import com.google.common.base.Joiner; + /** * Represents the result of an authorization check for logging and error * reporting. @@ -40,6 +42,7 @@ public class AuthResult { private final String request; private String reason; private final User user; + private AuthResult.Params params; // "family" and "qualifier" should only be used if "families" is null. private final byte[] family; @@ -58,6 +61,7 @@ public class AuthResult { this.action = action; this.families = null; this.namespace = null; + this.params = new Params().setTableName(table).setFamily(family).setQualifier(qualifier); } public AuthResult(boolean allowed, String request, String reason, User user, @@ -73,6 +77,7 @@ public class AuthResult { this.action = action; this.families = families; this.namespace = null; + this.params = new Params().setTableName(table).setFamilies(families); } public AuthResult(boolean allowed, String request, String reason, User user, @@ -87,6 +92,7 @@ public class AuthResult { this.family = null; this.qualifier = null; this.families = null; + this.params = new Params().setNamespace(namespace); } public boolean isAllowed() { @@ -121,6 +127,8 @@ public class AuthResult { return request; } + public Params getParams() { return this.params;} + public void setAllowed(boolean allowed) { this.allowed = allowed; } @@ -129,7 +137,8 @@ public class AuthResult { this.reason = reason; } - String toFamilyString() { + private static String toFamiliesString(Map> families, + byte[] family, byte[] qual) { StringBuilder sb = new StringBuilder(); if (families != null) { boolean first = true; @@ -164,8 +173,8 @@ public class AuthResult { } } else if (family != null) { sb.append(Bytes.toString(family)); - if (qualifier != null) { - sb.append(":").append(Bytes.toString(qualifier)); + if (qual != null) { + sb.append(":").append(Bytes.toString(qual)); } } return sb.toString(); @@ -173,17 +182,25 @@ public class AuthResult { public String toContextString() { StringBuilder sb = new StringBuilder(); + String familiesString = toFamiliesString(families, family, qualifier); sb.append("(user=") .append(user != null ? user.getName() : "UNKNOWN") .append(", "); sb.append("scope=") - .append(namespace != null ? namespace : table == null ? "GLOBAL" : table) + .append(namespace != null ? namespace : + table == null ? "GLOBAL" : table.getNameWithNamespaceInclAsString()) .append(", "); - if(namespace == null) { + if(namespace == null && familiesString.length() > 0) { sb.append("family=") - .append(toFamilyString()) + .append(familiesString) .append(", "); } + String paramsString = params.toString(); + if(paramsString.length() > 0) { + sb.append("params=[") + .append(paramsString) + .append("],"); + } sb.append("action=") .append(action != null ? action.toString() : "") .append(")"); @@ -225,4 +242,52 @@ public class AuthResult { Map> families) { return new AuthResult(false, request, reason, user, action, table, families); } + + public String toFamilyString() { + return toFamiliesString(families, family, qualifier); + } + + public static class Params { + private String namespace = null; + private TableName tableName = null; + private Map> families = null; + byte[] family = null; + byte[] qualifier = null; + + public Params setNamespace(String namespace) { + this.namespace = namespace; + return this; + } + + public Params setTableName(TableName table) { + this.tableName = table; + return this; + } + + public Params setFamilies(Map> families) { + this.families = families; + return this; + } + + public Params setFamily(byte[] family) { + this.family = family; + return this; + } + + public Params setQualifier(byte[] qualifier) { + this.qualifier = qualifier; + return this; + } + + public String toString() { + String familiesString = toFamiliesString(families, family, qualifier); + String[] params = new String[] { + namespace != null ? "namespace=" + namespace : null, + tableName != null ? "table=" + tableName.getNameWithNamespaceInclAsString() : null, + familiesString.length() > 0 ? "family=" + familiesString : null + }; + return Joiner.on(",").skipNulls().join(params); + } + + } }