You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hbase.apache.org by ap...@apache.org on 2014/07/04 02:40:45 UTC
[1/3] git commit: HBASE-11434 [AccessController] Disallow inbound
cells with reserved tags
Repository: hbase
Updated Branches:
refs/heads/0.98 5bd3ad3dd -> 54c186b42
refs/heads/branch-1 75ace7ed2 -> a8cbf2988
refs/heads/master 8f6e04c44 -> 80f1271ee
HBASE-11434 [AccessController] Disallow inbound cells with reserved tags
Project: http://git-wip-us.apache.org/repos/asf/hbase/repo
Commit: http://git-wip-us.apache.org/repos/asf/hbase/commit/80f1271e
Tree: http://git-wip-us.apache.org/repos/asf/hbase/tree/80f1271e
Diff: http://git-wip-us.apache.org/repos/asf/hbase/diff/80f1271e
Branch: refs/heads/master
Commit: 80f1271ee5eb8957ff67040e95b2880257bda822
Parents: 8f6e04c
Author: Andrew Purtell <ap...@apache.org>
Authored: Thu Jul 3 17:23:00 2014 -0700
Committer: Andrew Purtell <ap...@apache.org>
Committed: Thu Jul 3 17:23:00 2014 -0700
----------------------------------------------------------------------
.../hbase/security/access/AccessController.java | 66 ++++++++++++++++----
.../security/access/TestAccessController.java | 29 ++++++++-
2 files changed, 82 insertions(+), 13 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/hbase/blob/80f1271e/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessController.java
----------------------------------------------------------------------
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 6ab696b..77734a7 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
@@ -31,6 +31,7 @@ import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.hbase.Cell;
+import org.apache.hadoop.hbase.CellScanner;
import org.apache.hadoop.hbase.CellUtil;
import org.apache.hadoop.hbase.CompoundConfiguration;
import org.apache.hadoop.hbase.CoprocessorEnvironment;
@@ -152,6 +153,7 @@ public class AccessController extends BaseRegionObserver
private static final Log AUDITLOG =
LogFactory.getLog("SecurityLogger."+AccessController.class.getName());
private static final String CHECK_COVERING_PERM = "check_covering_perm";
+ private static final String TAG_CHECK_PASSED = "tag_check_passed";
private static final byte[] TRUE = Bytes.toBytes(true);
TableAuthManager authManager = null;
@@ -167,8 +169,12 @@ public class AccessController extends BaseRegionObserver
private Map<InternalScanner,String> scannerOwners =
new MapMaker().weakKeys().makeMap();
+ // Provider for mapping principal names to Users
private UserProvider userProvider;
+ // The list of users with superuser authority
+ private List<String> superusers;
+
// if we are able to support cell ACLs
boolean cellFeaturesEnabled;
@@ -745,7 +751,7 @@ public class AccessController extends BaseRegionObserver
return cellGrants > 0;
}
- private void addCellPermissions(final byte[] perms, Map<byte[], List<Cell>> familyMap) {
+ private static void addCellPermissions(final byte[] perms, Map<byte[], List<Cell>> familyMap) {
// Iterate over the entries in the familyMap, replacing the cells therein
// with new cells including the ACL data
for (Map.Entry<byte[], List<Cell>> e: familyMap.entrySet()) {
@@ -772,6 +778,33 @@ public class AccessController extends BaseRegionObserver
e.setValue(newCells);
}
}
+
+ // Checks whether incoming cells contain any tag with type as ACL_TAG_TYPE. This tag
+ // type is reserved and should not be explicitly set by user.
+ private void checkForReservedTagPresence(User user, Mutation m) throws IOException {
+ // Superusers are allowed to store cells unconditionally.
+ if (superusers.contains(user.getShortName())) {
+ return;
+ }
+ // We already checked (prePut vs preBatchMutation)
+ if (m.getAttribute(TAG_CHECK_PASSED) != null) {
+ return;
+ }
+ for (CellScanner cellScanner = m.cellScanner(); cellScanner.advance();) {
+ Cell cell = cellScanner.current();
+ if (cell.getTagsLength() > 0) {
+ Iterator<Tag> tagsItr = CellUtil.tagsIterator(cell.getTagsArray(), cell.getTagsOffset(),
+ cell.getTagsLength());
+ while (tagsItr.hasNext()) {
+ if (tagsItr.next().getType() == AccessControlLists.ACL_TAG_TYPE) {
+ throw new AccessDeniedException("Mutation contains cell with reserved type tag");
+ }
+ }
+ }
+ }
+ m.setAttribute(TAG_CHECK_PASSED, TRUE);
+ }
+
/* ---- MasterObserver implementation ---- */
public void start(CoprocessorEnvironment env) throws IOException {
@@ -808,6 +841,11 @@ public class AccessController extends BaseRegionObserver
// set the user-provider.
this.userProvider = UserProvider.instantiate(env.getConfiguration());
+ // set up the list of users with superuser privilege
+ User user = userProvider.getCurrent();
+ superusers = Lists.asList(user.getShortName(),
+ conf.getStrings(AccessControlLists.SUPERUSER_CONF_KEY, new String[0]));
+
// If zk is null or IOException while obtaining auth manager,
// throw RuntimeException so that the coprocessor is unloaded.
if (zk != null) {
@@ -1451,9 +1489,10 @@ public class AccessController extends BaseRegionObserver
// HBase value. A new ACL in a new Put applies to that Put. It doesn't
// change the ACL of any previous Put. This allows simple evolution of
// security policy over time without requiring expensive updates.
+ User user = getActiveUser();
+ checkForReservedTagPresence(user, put);
RegionCoprocessorEnvironment env = c.getEnvironment();
Map<byte[],? extends Collection<Cell>> families = put.getFamilyCellMap();
- User user = getActiveUser();
AuthResult authResult = permissionGranted(OpType.PUT, user, env, families, Action.WRITE);
logResult(authResult);
if (!authResult.isAllowed()) {
@@ -1463,6 +1502,7 @@ public class AccessController extends BaseRegionObserver
throw new AccessDeniedException("Insufficient permissions " + authResult.toContextString());
}
}
+ // Add cell ACLs from the operation to the cells themselves
byte[] bytes = put.getAttribute(AccessControlConstants.OP_ATTRIBUTE_ACL);
if (bytes != null) {
if (cellFeaturesEnabled) {
@@ -1518,7 +1558,13 @@ public class AccessController extends BaseRegionObserver
if (m.getAttribute(CHECK_COVERING_PERM) != null) {
// We have a failure with table, cf and q perm checks and now giving a chance for cell
// perm check
- OpType opType = (m instanceof Put) ? OpType.PUT : OpType.DELETE;
+ OpType opType;
+ if (m instanceof Put) {
+ checkForReservedTagPresence(getActiveUser(), m);
+ opType = OpType.PUT;
+ } else {
+ opType = OpType.DELETE;
+ }
AuthResult authResult = null;
if (checkCoveringPermission(opType, c.getEnvironment(), m.getRow(), m.getFamilyCellMap(),
m.getTimeStamp(), Action.WRITE)) {
@@ -1554,9 +1600,10 @@ public class AccessController extends BaseRegionObserver
final ByteArrayComparable comparator, final Put put,
final boolean result) throws IOException {
// Require READ and WRITE permissions on the table, CF, and KV to update
+ User user = getActiveUser();
+ checkForReservedTagPresence(user, put);
RegionCoprocessorEnvironment env = c.getEnvironment();
Map<byte[],? extends Collection<byte[]>> families = makeFamilyMap(family, qualifier);
- User user = getActiveUser();
AuthResult authResult = permissionGranted(OpType.CHECK_AND_PUT, user, env, families,
Action.READ, Action.WRITE);
logResult(authResult);
@@ -1690,9 +1737,10 @@ public class AccessController extends BaseRegionObserver
public Result preAppend(ObserverContext<RegionCoprocessorEnvironment> c, Append append)
throws IOException {
// Require WRITE permission to the table, CF, and the KV to be appended
+ User user = getActiveUser();
+ checkForReservedTagPresence(user, append);
RegionCoprocessorEnvironment env = c.getEnvironment();
Map<byte[],? extends Collection<Cell>> families = append.getFamilyCellMap();
- User user = getActiveUser();
AuthResult authResult = permissionGranted(OpType.APPEND, user, env, families, Action.WRITE);
logResult(authResult);
if (!authResult.isAllowed()) {
@@ -1743,9 +1791,10 @@ public class AccessController extends BaseRegionObserver
throws IOException {
// Require WRITE permission to the table, CF, and the KV to be replaced by
// the incremented value
+ User user = getActiveUser();
+ checkForReservedTagPresence(user, increment);
RegionCoprocessorEnvironment env = c.getEnvironment();
Map<byte[],? extends Collection<Cell>> families = increment.getFamilyCellMap();
- User user = getActiveUser();
AuthResult authResult = permissionGranted(OpType.INCREMENT, user, env, families,
Action.WRITE);
logResult(authResult);
@@ -2219,11 +2268,6 @@ public class AccessController extends BaseRegionObserver
throw new IOException("Unable to obtain the current user, " +
"authorization checks for internal operations will not work correctly!");
}
-
- String currentUser = user.getShortName();
- List<String> superusers = Lists.asList(currentUser, conf.getStrings(
- AccessControlLists.SUPERUSER_CONF_KEY, new String[0]));
-
User activeUser = getActiveUser();
if (!(superusers.contains(activeUser.getShortName()))) {
throw new AccessDeniedException("User '" + (user != null ? user.getShortName() : "null") +
http://git-wip-us.apache.org/repos/asf/hbase/blob/80f1271e/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestAccessController.java
----------------------------------------------------------------------
diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestAccessController.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestAccessController.java
index 6e79124..a0ccf2e 100644
--- a/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestAccessController.java
+++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestAccessController.java
@@ -48,6 +48,7 @@ import org.apache.hadoop.hbase.MiniHBaseCluster;
import org.apache.hadoop.hbase.ServerName;
import org.apache.hadoop.hbase.TableName;
import org.apache.hadoop.hbase.TableNotFoundException;
+import org.apache.hadoop.hbase.Tag;
import org.apache.hadoop.hbase.client.Append;
import org.apache.hadoop.hbase.client.Delete;
import org.apache.hadoop.hbase.client.Get;
@@ -94,10 +95,8 @@ import org.apache.hadoop.hbase.security.access.Permission.Action;
import org.apache.hadoop.hbase.util.Bytes;
import org.apache.hadoop.hbase.util.JVMClusterUtil;
import org.apache.hadoop.hbase.util.TestTableName;
-
import org.apache.log4j.Level;
import org.apache.log4j.Logger;
-
import org.junit.After;
import org.junit.AfterClass;
import org.junit.Before;
@@ -2126,4 +2125,30 @@ public class TestAccessController extends SecureTestUtil {
verifyAllowed(execEndpointAction, userA, userB);
}
+ @Test
+ public void testReservedCellTags() throws Exception {
+ AccessTestAction putWithReservedTag = new AccessTestAction() {
+ @Override
+ public Object run() throws Exception {
+ HTable t = new HTable(conf, TEST_TABLE.getTableName());
+ try {
+ KeyValue kv = new KeyValue(TEST_ROW, TEST_FAMILY, TEST_QUALIFIER,
+ HConstants.LATEST_TIMESTAMP, HConstants.EMPTY_BYTE_ARRAY,
+ new Tag[] { new Tag(AccessControlLists.ACL_TAG_TYPE,
+ ProtobufUtil.toUsersAndPermissions(USER_OWNER.getShortName(),
+ new Permission(Permission.Action.READ)).toByteArray()) });
+ t.put(new Put(TEST_ROW).add(kv));
+ } finally {
+ t.close();
+ }
+ return null;
+ }
+ };
+
+ // Current user is superuser
+ verifyAllowed(putWithReservedTag, User.getCurrent());
+ // No other user should be allowed
+ verifyDenied(putWithReservedTag, USER_OWNER, USER_ADMIN, USER_CREATE, USER_RW, USER_RO);
+ }
+
}
[2/3] git commit: HBASE-11434 [AccessController] Disallow inbound
cells with reserved tags
Posted by ap...@apache.org.
HBASE-11434 [AccessController] Disallow inbound cells with reserved tags
Project: http://git-wip-us.apache.org/repos/asf/hbase/repo
Commit: http://git-wip-us.apache.org/repos/asf/hbase/commit/54c186b4
Tree: http://git-wip-us.apache.org/repos/asf/hbase/tree/54c186b4
Diff: http://git-wip-us.apache.org/repos/asf/hbase/diff/54c186b4
Branch: refs/heads/0.98
Commit: 54c186b42c3c77248783d2c3d9181c12e7b06802
Parents: 5bd3ad3
Author: Andrew Purtell <ap...@apache.org>
Authored: Thu Jul 3 17:23:00 2014 -0700
Committer: Andrew Purtell <ap...@apache.org>
Committed: Thu Jul 3 17:24:25 2014 -0700
----------------------------------------------------------------------
.../hbase/security/access/AccessController.java | 65 ++++++++++++++++----
.../security/access/TestAccessController.java | 29 ++++++++-
2 files changed, 81 insertions(+), 13 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/hbase/blob/54c186b4/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessController.java
----------------------------------------------------------------------
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 588a216..21f8892 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
@@ -31,6 +31,7 @@ import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.hbase.Cell;
+import org.apache.hadoop.hbase.CellScanner;
import org.apache.hadoop.hbase.CellUtil;
import org.apache.hadoop.hbase.CompoundConfiguration;
import org.apache.hadoop.hbase.CoprocessorEnvironment;
@@ -152,6 +153,7 @@ public class AccessController extends BaseRegionObserver
private static final Log AUDITLOG =
LogFactory.getLog("SecurityLogger."+AccessController.class.getName());
private static final String CHECK_COVERING_PERM = "check_covering_perm";
+ private static final String TAG_CHECK_PASSED = "tag_check_passed";
private static final byte[] TRUE = Bytes.toBytes(true);
TableAuthManager authManager = null;
@@ -167,8 +169,12 @@ public class AccessController extends BaseRegionObserver
private Map<InternalScanner,String> scannerOwners =
new MapMaker().weakKeys().makeMap();
+ // Provider for mapping principal names to Users
private UserProvider userProvider;
+ // The list of users with superuser authority
+ private List<String> superusers;
+
// if we are able to support cell ACLs
boolean cellFeaturesEnabled;
@@ -745,7 +751,7 @@ public class AccessController extends BaseRegionObserver
return cellGrants > 0;
}
- private void addCellPermissions(final byte[] perms, Map<byte[], List<Cell>> familyMap) {
+ private static void addCellPermissions(final byte[] perms, Map<byte[], List<Cell>> familyMap) {
// Iterate over the entries in the familyMap, replacing the cells therein
// with new cells including the ACL data
for (Map.Entry<byte[], List<Cell>> e: familyMap.entrySet()) {
@@ -774,6 +780,32 @@ public class AccessController extends BaseRegionObserver
}
}
+ // Checks whether incoming cells contain any tag with type as ACL_TAG_TYPE. This tag
+ // type is reserved and should not be explicitly set by user.
+ private void checkForReservedTagPresence(User user, Mutation m) throws IOException {
+ // Superusers are allowed to store cells unconditionally.
+ if (superusers.contains(user.getShortName())) {
+ return;
+ }
+ // We already checked (prePut vs preBatchMutation)
+ if (m.getAttribute(TAG_CHECK_PASSED) != null) {
+ return;
+ }
+ for (CellScanner cellScanner = m.cellScanner(); cellScanner.advance();) {
+ Cell cell = cellScanner.current();
+ if (cell.getTagsLength() > 0) {
+ Iterator<Tag> tagsItr = CellUtil.tagsIterator(cell.getTagsArray(), cell.getTagsOffset(),
+ cell.getTagsLength());
+ while (tagsItr.hasNext()) {
+ if (tagsItr.next().getType() == AccessControlLists.ACL_TAG_TYPE) {
+ throw new AccessDeniedException("Mutation contains cell with reserved type tag");
+ }
+ }
+ }
+ }
+ m.setAttribute(TAG_CHECK_PASSED, TRUE);
+ }
+
/* ---- MasterObserver implementation ---- */
public void start(CoprocessorEnvironment env) throws IOException {
@@ -810,6 +842,11 @@ public class AccessController extends BaseRegionObserver
// set the user-provider.
this.userProvider = UserProvider.instantiate(env.getConfiguration());
+ // set up the list of users with superuser privilege
+ User user = userProvider.getCurrent();
+ superusers = Lists.asList(user.getShortName(),
+ conf.getStrings(AccessControlLists.SUPERUSER_CONF_KEY, new String[0]));
+
// If zk is null or IOException while obtaining auth manager,
// throw RuntimeException so that the coprocessor is unloaded.
if (zk != null) {
@@ -1426,9 +1463,10 @@ public class AccessController extends BaseRegionObserver
// HBase value. A new ACL in a new Put applies to that Put. It doesn't
// change the ACL of any previous Put. This allows simple evolution of
// security policy over time without requiring expensive updates.
+ User user = getActiveUser();
+ checkForReservedTagPresence(user, put);
RegionCoprocessorEnvironment env = c.getEnvironment();
Map<byte[],? extends Collection<Cell>> families = put.getFamilyCellMap();
- User user = getActiveUser();
AuthResult authResult = permissionGranted(OpType.PUT, user, env, families, Action.WRITE);
logResult(authResult);
if (!authResult.isAllowed()) {
@@ -1438,6 +1476,7 @@ public class AccessController extends BaseRegionObserver
throw new AccessDeniedException("Insufficient permissions " + authResult.toContextString());
}
}
+ // Add cell ACLs from the operation to the cells themselves
byte[] bytes = put.getAttribute(AccessControlConstants.OP_ATTRIBUTE_ACL);
if (bytes != null) {
if (cellFeaturesEnabled) {
@@ -1493,7 +1532,13 @@ public class AccessController extends BaseRegionObserver
if (m.getAttribute(CHECK_COVERING_PERM) != null) {
// We have a failure with table, cf and q perm checks and now giving a chance for cell
// perm check
- OpType opType = (m instanceof Put) ? OpType.PUT : OpType.DELETE;
+ OpType opType;
+ if (m instanceof Put) {
+ checkForReservedTagPresence(getActiveUser(), m);
+ opType = OpType.PUT;
+ } else {
+ opType = OpType.DELETE;
+ }
AuthResult authResult = null;
if (checkCoveringPermission(opType, c.getEnvironment(), m.getRow(), m.getFamilyCellMap(),
m.getTimeStamp(), Action.WRITE)) {
@@ -1529,9 +1574,10 @@ public class AccessController extends BaseRegionObserver
final ByteArrayComparable comparator, final Put put,
final boolean result) throws IOException {
// Require READ and WRITE permissions on the table, CF, and KV to update
+ User user = getActiveUser();
+ checkForReservedTagPresence(user, put);
RegionCoprocessorEnvironment env = c.getEnvironment();
Map<byte[],? extends Collection<byte[]>> families = makeFamilyMap(family, qualifier);
- User user = getActiveUser();
AuthResult authResult = permissionGranted(OpType.CHECK_AND_PUT, user, env, families,
Action.READ, Action.WRITE);
logResult(authResult);
@@ -1665,9 +1711,10 @@ public class AccessController extends BaseRegionObserver
public Result preAppend(ObserverContext<RegionCoprocessorEnvironment> c, Append append)
throws IOException {
// Require WRITE permission to the table, CF, and the KV to be appended
+ User user = getActiveUser();
+ checkForReservedTagPresence(user, append);
RegionCoprocessorEnvironment env = c.getEnvironment();
Map<byte[],? extends Collection<Cell>> families = append.getFamilyCellMap();
- User user = getActiveUser();
AuthResult authResult = permissionGranted(OpType.APPEND, user, env, families, Action.WRITE);
logResult(authResult);
if (!authResult.isAllowed()) {
@@ -1718,9 +1765,10 @@ public class AccessController extends BaseRegionObserver
throws IOException {
// Require WRITE permission to the table, CF, and the KV to be replaced by
// the incremented value
+ User user = getActiveUser();
+ checkForReservedTagPresence(user, increment);
RegionCoprocessorEnvironment env = c.getEnvironment();
Map<byte[],? extends Collection<Cell>> families = increment.getFamilyCellMap();
- User user = getActiveUser();
AuthResult authResult = permissionGranted(OpType.INCREMENT, user, env, families,
Action.WRITE);
logResult(authResult);
@@ -2195,11 +2243,6 @@ public class AccessController extends BaseRegionObserver
throw new IOException("Unable to obtain the current user, " +
"authorization checks for internal operations will not work correctly!");
}
-
- String currentUser = user.getShortName();
- List<String> superusers = Lists.asList(currentUser, conf.getStrings(
- AccessControlLists.SUPERUSER_CONF_KEY, new String[0]));
-
User activeUser = getActiveUser();
if (!(superusers.contains(activeUser.getShortName()))) {
throw new AccessDeniedException("User '" + (user != null ? user.getShortName() : "null") +
http://git-wip-us.apache.org/repos/asf/hbase/blob/54c186b4/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestAccessController.java
----------------------------------------------------------------------
diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestAccessController.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestAccessController.java
index 2d8374f..df1544f 100644
--- a/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestAccessController.java
+++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestAccessController.java
@@ -48,6 +48,7 @@ import org.apache.hadoop.hbase.MiniHBaseCluster;
import org.apache.hadoop.hbase.ServerName;
import org.apache.hadoop.hbase.TableName;
import org.apache.hadoop.hbase.TableNotFoundException;
+import org.apache.hadoop.hbase.Tag;
import org.apache.hadoop.hbase.client.Append;
import org.apache.hadoop.hbase.client.Delete;
import org.apache.hadoop.hbase.client.Get;
@@ -94,10 +95,8 @@ import org.apache.hadoop.hbase.security.access.Permission.Action;
import org.apache.hadoop.hbase.util.Bytes;
import org.apache.hadoop.hbase.util.JVMClusterUtil;
import org.apache.hadoop.hbase.util.TestTableName;
-
import org.apache.log4j.Level;
import org.apache.log4j.Logger;
-
import org.junit.After;
import org.junit.AfterClass;
import org.junit.Before;
@@ -2109,4 +2108,30 @@ public class TestAccessController extends SecureTestUtil {
verifyAllowed(execEndpointAction, userA, userB);
}
+ @Test
+ public void testReservedCellTags() throws Exception {
+ AccessTestAction putWithReservedTag = new AccessTestAction() {
+ @Override
+ public Object run() throws Exception {
+ HTable t = new HTable(conf, TEST_TABLE.getTableName());
+ try {
+ KeyValue kv = new KeyValue(TEST_ROW, TEST_FAMILY, TEST_QUALIFIER,
+ HConstants.LATEST_TIMESTAMP, HConstants.EMPTY_BYTE_ARRAY,
+ new Tag[] { new Tag(AccessControlLists.ACL_TAG_TYPE,
+ ProtobufUtil.toUsersAndPermissions(USER_OWNER.getShortName(),
+ new Permission(Permission.Action.READ)).toByteArray()) });
+ t.put(new Put(TEST_ROW).add(kv));
+ } finally {
+ t.close();
+ }
+ return null;
+ }
+ };
+
+ // Current user is superuser
+ verifyAllowed(putWithReservedTag, User.getCurrent());
+ // No other user should be allowed
+ verifyDenied(putWithReservedTag, USER_OWNER, USER_ADMIN, USER_CREATE, USER_RW, USER_RO);
+ }
+
}
[3/3] git commit: HBASE-11434 [AccessController] Disallow inbound
cells with reserved tags
Posted by ap...@apache.org.
HBASE-11434 [AccessController] Disallow inbound cells with reserved tags
Project: http://git-wip-us.apache.org/repos/asf/hbase/repo
Commit: http://git-wip-us.apache.org/repos/asf/hbase/commit/a8cbf298
Tree: http://git-wip-us.apache.org/repos/asf/hbase/tree/a8cbf298
Diff: http://git-wip-us.apache.org/repos/asf/hbase/diff/a8cbf298
Branch: refs/heads/branch-1
Commit: a8cbf2988b9153ade4d4ebf8b5eefe5420c5b41a
Parents: 75ace7e
Author: Andrew Purtell <ap...@apache.org>
Authored: Thu Jul 3 17:23:00 2014 -0700
Committer: Andrew Purtell <ap...@apache.org>
Committed: Thu Jul 3 17:37:07 2014 -0700
----------------------------------------------------------------------
.../hbase/security/access/AccessController.java | 66 ++++++++++++++++----
.../security/access/TestAccessController.java | 29 ++++++++-
2 files changed, 82 insertions(+), 13 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/hbase/blob/a8cbf298/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessController.java
----------------------------------------------------------------------
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 6ab696b..77734a7 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
@@ -31,6 +31,7 @@ import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.hbase.Cell;
+import org.apache.hadoop.hbase.CellScanner;
import org.apache.hadoop.hbase.CellUtil;
import org.apache.hadoop.hbase.CompoundConfiguration;
import org.apache.hadoop.hbase.CoprocessorEnvironment;
@@ -152,6 +153,7 @@ public class AccessController extends BaseRegionObserver
private static final Log AUDITLOG =
LogFactory.getLog("SecurityLogger."+AccessController.class.getName());
private static final String CHECK_COVERING_PERM = "check_covering_perm";
+ private static final String TAG_CHECK_PASSED = "tag_check_passed";
private static final byte[] TRUE = Bytes.toBytes(true);
TableAuthManager authManager = null;
@@ -167,8 +169,12 @@ public class AccessController extends BaseRegionObserver
private Map<InternalScanner,String> scannerOwners =
new MapMaker().weakKeys().makeMap();
+ // Provider for mapping principal names to Users
private UserProvider userProvider;
+ // The list of users with superuser authority
+ private List<String> superusers;
+
// if we are able to support cell ACLs
boolean cellFeaturesEnabled;
@@ -745,7 +751,7 @@ public class AccessController extends BaseRegionObserver
return cellGrants > 0;
}
- private void addCellPermissions(final byte[] perms, Map<byte[], List<Cell>> familyMap) {
+ private static void addCellPermissions(final byte[] perms, Map<byte[], List<Cell>> familyMap) {
// Iterate over the entries in the familyMap, replacing the cells therein
// with new cells including the ACL data
for (Map.Entry<byte[], List<Cell>> e: familyMap.entrySet()) {
@@ -772,6 +778,33 @@ public class AccessController extends BaseRegionObserver
e.setValue(newCells);
}
}
+
+ // Checks whether incoming cells contain any tag with type as ACL_TAG_TYPE. This tag
+ // type is reserved and should not be explicitly set by user.
+ private void checkForReservedTagPresence(User user, Mutation m) throws IOException {
+ // Superusers are allowed to store cells unconditionally.
+ if (superusers.contains(user.getShortName())) {
+ return;
+ }
+ // We already checked (prePut vs preBatchMutation)
+ if (m.getAttribute(TAG_CHECK_PASSED) != null) {
+ return;
+ }
+ for (CellScanner cellScanner = m.cellScanner(); cellScanner.advance();) {
+ Cell cell = cellScanner.current();
+ if (cell.getTagsLength() > 0) {
+ Iterator<Tag> tagsItr = CellUtil.tagsIterator(cell.getTagsArray(), cell.getTagsOffset(),
+ cell.getTagsLength());
+ while (tagsItr.hasNext()) {
+ if (tagsItr.next().getType() == AccessControlLists.ACL_TAG_TYPE) {
+ throw new AccessDeniedException("Mutation contains cell with reserved type tag");
+ }
+ }
+ }
+ }
+ m.setAttribute(TAG_CHECK_PASSED, TRUE);
+ }
+
/* ---- MasterObserver implementation ---- */
public void start(CoprocessorEnvironment env) throws IOException {
@@ -808,6 +841,11 @@ public class AccessController extends BaseRegionObserver
// set the user-provider.
this.userProvider = UserProvider.instantiate(env.getConfiguration());
+ // set up the list of users with superuser privilege
+ User user = userProvider.getCurrent();
+ superusers = Lists.asList(user.getShortName(),
+ conf.getStrings(AccessControlLists.SUPERUSER_CONF_KEY, new String[0]));
+
// If zk is null or IOException while obtaining auth manager,
// throw RuntimeException so that the coprocessor is unloaded.
if (zk != null) {
@@ -1451,9 +1489,10 @@ public class AccessController extends BaseRegionObserver
// HBase value. A new ACL in a new Put applies to that Put. It doesn't
// change the ACL of any previous Put. This allows simple evolution of
// security policy over time without requiring expensive updates.
+ User user = getActiveUser();
+ checkForReservedTagPresence(user, put);
RegionCoprocessorEnvironment env = c.getEnvironment();
Map<byte[],? extends Collection<Cell>> families = put.getFamilyCellMap();
- User user = getActiveUser();
AuthResult authResult = permissionGranted(OpType.PUT, user, env, families, Action.WRITE);
logResult(authResult);
if (!authResult.isAllowed()) {
@@ -1463,6 +1502,7 @@ public class AccessController extends BaseRegionObserver
throw new AccessDeniedException("Insufficient permissions " + authResult.toContextString());
}
}
+ // Add cell ACLs from the operation to the cells themselves
byte[] bytes = put.getAttribute(AccessControlConstants.OP_ATTRIBUTE_ACL);
if (bytes != null) {
if (cellFeaturesEnabled) {
@@ -1518,7 +1558,13 @@ public class AccessController extends BaseRegionObserver
if (m.getAttribute(CHECK_COVERING_PERM) != null) {
// We have a failure with table, cf and q perm checks and now giving a chance for cell
// perm check
- OpType opType = (m instanceof Put) ? OpType.PUT : OpType.DELETE;
+ OpType opType;
+ if (m instanceof Put) {
+ checkForReservedTagPresence(getActiveUser(), m);
+ opType = OpType.PUT;
+ } else {
+ opType = OpType.DELETE;
+ }
AuthResult authResult = null;
if (checkCoveringPermission(opType, c.getEnvironment(), m.getRow(), m.getFamilyCellMap(),
m.getTimeStamp(), Action.WRITE)) {
@@ -1554,9 +1600,10 @@ public class AccessController extends BaseRegionObserver
final ByteArrayComparable comparator, final Put put,
final boolean result) throws IOException {
// Require READ and WRITE permissions on the table, CF, and KV to update
+ User user = getActiveUser();
+ checkForReservedTagPresence(user, put);
RegionCoprocessorEnvironment env = c.getEnvironment();
Map<byte[],? extends Collection<byte[]>> families = makeFamilyMap(family, qualifier);
- User user = getActiveUser();
AuthResult authResult = permissionGranted(OpType.CHECK_AND_PUT, user, env, families,
Action.READ, Action.WRITE);
logResult(authResult);
@@ -1690,9 +1737,10 @@ public class AccessController extends BaseRegionObserver
public Result preAppend(ObserverContext<RegionCoprocessorEnvironment> c, Append append)
throws IOException {
// Require WRITE permission to the table, CF, and the KV to be appended
+ User user = getActiveUser();
+ checkForReservedTagPresence(user, append);
RegionCoprocessorEnvironment env = c.getEnvironment();
Map<byte[],? extends Collection<Cell>> families = append.getFamilyCellMap();
- User user = getActiveUser();
AuthResult authResult = permissionGranted(OpType.APPEND, user, env, families, Action.WRITE);
logResult(authResult);
if (!authResult.isAllowed()) {
@@ -1743,9 +1791,10 @@ public class AccessController extends BaseRegionObserver
throws IOException {
// Require WRITE permission to the table, CF, and the KV to be replaced by
// the incremented value
+ User user = getActiveUser();
+ checkForReservedTagPresence(user, increment);
RegionCoprocessorEnvironment env = c.getEnvironment();
Map<byte[],? extends Collection<Cell>> families = increment.getFamilyCellMap();
- User user = getActiveUser();
AuthResult authResult = permissionGranted(OpType.INCREMENT, user, env, families,
Action.WRITE);
logResult(authResult);
@@ -2219,11 +2268,6 @@ public class AccessController extends BaseRegionObserver
throw new IOException("Unable to obtain the current user, " +
"authorization checks for internal operations will not work correctly!");
}
-
- String currentUser = user.getShortName();
- List<String> superusers = Lists.asList(currentUser, conf.getStrings(
- AccessControlLists.SUPERUSER_CONF_KEY, new String[0]));
-
User activeUser = getActiveUser();
if (!(superusers.contains(activeUser.getShortName()))) {
throw new AccessDeniedException("User '" + (user != null ? user.getShortName() : "null") +
http://git-wip-us.apache.org/repos/asf/hbase/blob/a8cbf298/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestAccessController.java
----------------------------------------------------------------------
diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestAccessController.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestAccessController.java
index 6e79124..a0ccf2e 100644
--- a/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestAccessController.java
+++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestAccessController.java
@@ -48,6 +48,7 @@ import org.apache.hadoop.hbase.MiniHBaseCluster;
import org.apache.hadoop.hbase.ServerName;
import org.apache.hadoop.hbase.TableName;
import org.apache.hadoop.hbase.TableNotFoundException;
+import org.apache.hadoop.hbase.Tag;
import org.apache.hadoop.hbase.client.Append;
import org.apache.hadoop.hbase.client.Delete;
import org.apache.hadoop.hbase.client.Get;
@@ -94,10 +95,8 @@ import org.apache.hadoop.hbase.security.access.Permission.Action;
import org.apache.hadoop.hbase.util.Bytes;
import org.apache.hadoop.hbase.util.JVMClusterUtil;
import org.apache.hadoop.hbase.util.TestTableName;
-
import org.apache.log4j.Level;
import org.apache.log4j.Logger;
-
import org.junit.After;
import org.junit.AfterClass;
import org.junit.Before;
@@ -2126,4 +2125,30 @@ public class TestAccessController extends SecureTestUtil {
verifyAllowed(execEndpointAction, userA, userB);
}
+ @Test
+ public void testReservedCellTags() throws Exception {
+ AccessTestAction putWithReservedTag = new AccessTestAction() {
+ @Override
+ public Object run() throws Exception {
+ HTable t = new HTable(conf, TEST_TABLE.getTableName());
+ try {
+ KeyValue kv = new KeyValue(TEST_ROW, TEST_FAMILY, TEST_QUALIFIER,
+ HConstants.LATEST_TIMESTAMP, HConstants.EMPTY_BYTE_ARRAY,
+ new Tag[] { new Tag(AccessControlLists.ACL_TAG_TYPE,
+ ProtobufUtil.toUsersAndPermissions(USER_OWNER.getShortName(),
+ new Permission(Permission.Action.READ)).toByteArray()) });
+ t.put(new Put(TEST_ROW).add(kv));
+ } finally {
+ t.close();
+ }
+ return null;
+ }
+ };
+
+ // Current user is superuser
+ verifyAllowed(putWithReservedTag, User.getCurrent());
+ // No other user should be allowed
+ verifyDenied(putWithReservedTag, USER_OWNER, USER_ADMIN, USER_CREATE, USER_RW, USER_RO);
+ }
+
}