Repository: hbase Updated Branches: refs/heads/master 29c233e6e -> b24518562
HBASE-12644 Visibility Labels: issue with storing super users in labels table (Jerry He) Project: http://git-wip-us.apache.org/repos/asf/hbase/repo Commit: http://git-wip-us.apache.org/repos/asf/hbase/commit/b2451856 Tree: http://git-wip-us.apache.org/repos/asf/hbase/tree/b2451856 Diff: http://git-wip-us.apache.org/repos/asf/hbase/diff/b2451856 Branch: refs/heads/master Commit: b24518562adb5deb41a8780cead268cb163864d1 Parents: 29c233e Author: tedyu <[email protected]> Authored: Sat Dec 13 09:10:50 2014 -0800 Committer: tedyu <[email protected]> Committed: Sat Dec 13 09:10:50 2014 -0800 ---------------------------------------------------------------------- .../DefaultVisibilityLabelServiceImpl.java | 29 ++++++------- .../visibility/ZKVisibilityLabelWatcher.java | 19 ++++----- .../ExpAsStringVisibilityLabelServiceImpl.java | 24 +++++------ .../TestDefaultScanLabelGeneratorStack.java | 43 +++++++++++++++++++- .../TestEnforcingScanLabelGenerator.java | 22 ++++++++++ 5 files changed, 96 insertions(+), 41 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/hbase/blob/b2451856/hbase-server/src/main/java/org/apache/hadoop/hbase/security/visibility/DefaultVisibilityLabelServiceImpl.java ---------------------------------------------------------------------- diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/security/visibility/DefaultVisibilityLabelServiceImpl.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/security/visibility/DefaultVisibilityLabelServiceImpl.java index 2747d51..2d0e446 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/security/visibility/DefaultVisibilityLabelServiceImpl.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/security/visibility/DefaultVisibilityLabelServiceImpl.java @@ -80,6 +80,7 @@ public class DefaultVisibilityLabelServiceImpl implements VisibilityLabelService private HRegion labelsRegion; private VisibilityLabelsCache labelsCache; private List<ScanLabelGenerator> scanLabelGenerators; + private List<String> superUsers; static { ByteArrayOutputStream baos = new ByteArrayOutputStream(); @@ -116,6 +117,7 @@ public class DefaultVisibilityLabelServiceImpl implements VisibilityLabelService throw ioe; } this.scanLabelGenerators = VisibilityUtils.getScanLabelGenerators(this.conf); + this.superUsers = getSystemAndSuperUsers(); if (e.getRegion().getRegionInfo().getTable().equals(LABELS_TABLE_NAME)) { this.labelsRegion = e.getRegion(); Pair<Map<String, Integer>, Map<String, List<Integer>>> labelsAndUserAuths = @@ -196,22 +198,8 @@ public class DefaultVisibilityLabelServiceImpl implements VisibilityLabelService if (!labels.containsKey(SYSTEM_LABEL)) { Put p = new Put(Bytes.toBytes(SYSTEM_LABEL_ORDINAL)); p.addImmutable(LABELS_TABLE_FAMILY, LABEL_QUALIFIER, Bytes.toBytes(SYSTEM_LABEL)); - // Set auth for "system" label for all super users. - List<String> superUsers = getSystemAndSuperUsers(); - for (String superUser : superUsers) { - p.addImmutable(LABELS_TABLE_FAMILY, Bytes.toBytes(superUser), DUMMY_VALUE, - LABELS_TABLE_TAGS); - } region.put(p); labels.put(SYSTEM_LABEL, SYSTEM_LABEL_ORDINAL); - for (String superUser : superUsers) { - List<Integer> auths = userAuths.get(superUser); - if (auths == null) { - auths = new ArrayList<Integer>(1); - userAuths.put(superUser, auths); - } - auths.add(SYSTEM_LABEL_ORDINAL); - } } } @@ -426,7 +414,7 @@ public class DefaultVisibilityLabelServiceImpl implements VisibilityLabelService throws IOException { // If a super user issues a get/scan, he should be able to scan the cells // irrespective of the Visibility labels - if (isReadFromSuperUser()) { + if (isReadFromSystemAuthUser()) { return new VisibilityExpEvaluator() { @Override public boolean evaluate(Cell cell) throws IOException { @@ -505,13 +493,18 @@ public class DefaultVisibilityLabelServiceImpl implements VisibilityLabelService }; } - protected boolean isReadFromSuperUser() throws IOException { + protected boolean isReadFromSystemAuthUser() throws IOException { byte[] user = Bytes.toBytes(VisibilityUtils.getActiveUser().getShortName()); return havingSystemAuth(user); } @Override public boolean havingSystemAuth(byte[] user) throws IOException { + // A super user has 'system' auth. + if (isSystemOrSuperUser(user)) { + return true; + } + // A user can also be explicitly granted 'system' auth. List<String> auths = this.getAuths(user, true); if (LOG.isTraceEnabled()) { LOG.trace("The auths for user " + Bytes.toString(user) + " are " + auths); @@ -519,6 +512,10 @@ public class DefaultVisibilityLabelServiceImpl implements VisibilityLabelService return auths.contains(SYSTEM_LABEL); } + protected boolean isSystemOrSuperUser(byte[] user) throws IOException { + return this.superUsers.contains(Bytes.toString(user)); + } + @Override public boolean matchVisibility(List<Tag> putVisTags, Byte putTagsFormat, List<Tag> deleteVisTags, Byte deleteTagsFormat) throws IOException { http://git-wip-us.apache.org/repos/asf/hbase/blob/b2451856/hbase-server/src/main/java/org/apache/hadoop/hbase/security/visibility/ZKVisibilityLabelWatcher.java ---------------------------------------------------------------------- diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/security/visibility/ZKVisibilityLabelWatcher.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/security/visibility/ZKVisibilityLabelWatcher.java index 5c12d86..4941a54 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/security/visibility/ZKVisibilityLabelWatcher.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/security/visibility/ZKVisibilityLabelWatcher.java @@ -59,17 +59,15 @@ public class ZKVisibilityLabelWatcher extends ZooKeeperListener { public void start() throws KeeperException { watcher.registerListener(this); - if (ZKUtil.watchAndCheckExists(watcher, labelZnode)) { - byte[] data = ZKUtil.getDataAndWatch(watcher, labelZnode); - if (data != null) { - refreshVisibilityLabelsCache(data); - } + ZKUtil.createWithParents(watcher, labelZnode); + ZKUtil.createWithParents(watcher, userAuthsZnode); + byte[] data = ZKUtil.getDataAndWatch(watcher, labelZnode); + if (data != null && data.length > 0) { + refreshVisibilityLabelsCache(data); } - if (ZKUtil.watchAndCheckExists(watcher, userAuthsZnode)) { - byte[] data = ZKUtil.getDataAndWatch(watcher, userAuthsZnode); - if (data != null) { - refreshUserAuthsCache(data); - } + data = ZKUtil.getDataAndWatch(watcher, userAuthsZnode); + if (data != null && data.length > 0) { + refreshUserAuthsCache(data); } } @@ -143,7 +141,6 @@ public class ZKVisibilityLabelWatcher extends ZooKeeperListener { znode = this.userAuthsZnode; } try { - ZKUtil.createWithParents(watcher, znode); ZKUtil.updateExistingNodeData(watcher, znode, data, -1); } catch (KeeperException e) { LOG.error("Failed writing to " + znode, e); http://git-wip-us.apache.org/repos/asf/hbase/blob/b2451856/hbase-server/src/test/java/org/apache/hadoop/hbase/security/visibility/ExpAsStringVisibilityLabelServiceImpl.java ---------------------------------------------------------------------- diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/security/visibility/ExpAsStringVisibilityLabelServiceImpl.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/security/visibility/ExpAsStringVisibilityLabelServiceImpl.java index ae61dfd..cc615ed 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/security/visibility/ExpAsStringVisibilityLabelServiceImpl.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/security/visibility/ExpAsStringVisibilityLabelServiceImpl.java @@ -79,6 +79,7 @@ public class ExpAsStringVisibilityLabelServiceImpl implements VisibilityLabelSer private Configuration conf; private HRegion labelsRegion; private List<ScanLabelGenerator> scanLabelGenerators; + private List<String> superUsers; @Override public OperationStatus[] addLabels(List<byte[]> labels) throws IOException { @@ -203,7 +204,7 @@ public class ExpAsStringVisibilityLabelServiceImpl implements VisibilityLabelSer throws IOException { // If a super user issues a get/scan, he should be able to scan the cells // irrespective of the Visibility labels - if (isReadFromSuperUser()) { + if (isReadFromSystemAuthUser()) { return new VisibilityExpEvaluator() { @Override public boolean evaluate(Cell cell) throws IOException { @@ -273,7 +274,7 @@ public class ExpAsStringVisibilityLabelServiceImpl implements VisibilityLabelSer }; } - protected boolean isReadFromSuperUser() throws IOException { + protected boolean isReadFromSystemAuthUser() throws IOException { byte[] user = Bytes.toBytes(VisibilityUtils.getActiveUser().getShortName()); return havingSystemAuth(user); } @@ -339,19 +340,9 @@ public class ExpAsStringVisibilityLabelServiceImpl implements VisibilityLabelSer @Override public void init(RegionCoprocessorEnvironment e) throws IOException { this.scanLabelGenerators = VisibilityUtils.getScanLabelGenerators(this.conf); + this.superUsers = getSystemAndSuperUsers(); if (e.getRegion().getRegionInfo().getTable().equals(LABELS_TABLE_NAME)) { this.labelsRegion = e.getRegion(); - // Set auth for "system" label for all super users. - List<String> superUsers = getSystemAndSuperUsers(); - for (String superUser : superUsers) { - byte[] user = Bytes.toBytes(superUser); - List<String> auths = this.getAuths(user, true); - if (auths == null || auths.isEmpty()) { - Put p = new Put(user); - p.addImmutable(LABELS_TABLE_FAMILY, Bytes.toBytes(SYSTEM_LABEL), DUMMY_VALUE); - labelsRegion.put(p); - } - } } } @@ -370,8 +361,15 @@ public class ExpAsStringVisibilityLabelServiceImpl implements VisibilityLabelSer return superUsers; } + protected boolean isSystemOrSuperUser(byte[] user) throws IOException { + return this.superUsers.contains(Bytes.toString(user)); + } + @Override public boolean havingSystemAuth(byte[] user) throws IOException { + if (isSystemOrSuperUser(user)) { + return true; + } List<String> auths = this.getAuths(user, true); return auths.contains(SYSTEM_LABEL); } http://git-wip-us.apache.org/repos/asf/hbase/blob/b2451856/hbase-server/src/test/java/org/apache/hadoop/hbase/security/visibility/TestDefaultScanLabelGeneratorStack.java ---------------------------------------------------------------------- diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/security/visibility/TestDefaultScanLabelGeneratorStack.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/security/visibility/TestDefaultScanLabelGeneratorStack.java index df2a61c..2897048 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/security/visibility/TestDefaultScanLabelGeneratorStack.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/security/visibility/TestDefaultScanLabelGeneratorStack.java @@ -31,6 +31,8 @@ import org.apache.hadoop.hbase.CellScanner; import org.apache.hadoop.hbase.HBaseTestingUtility; import org.apache.hadoop.hbase.HConstants; import org.apache.hadoop.hbase.TableName; +import org.apache.hadoop.hbase.client.Connection; +import org.apache.hadoop.hbase.client.ConnectionFactory; import org.apache.hadoop.hbase.client.Get; import org.apache.hadoop.hbase.client.HTable; import org.apache.hadoop.hbase.client.Put; @@ -124,6 +126,46 @@ public class TestDefaultScanLabelGeneratorStack { } }); + // Test that super user can see all the cells. + SUPERUSER.runAs(new PrivilegedExceptionAction<Void>() { + public Void run() throws Exception { + Connection connection = ConnectionFactory.createConnection(conf); + Table table = connection.getTable(tableName); + try { + Scan s = new Scan(); + ResultScanner scanner = table.getScanner(s); + Result[] next = scanner.next(1); + + // Test that super user can see all the cells. + assertTrue(next.length == 1); + CellScanner cellScanner = next[0].cellScanner(); + cellScanner.advance(); + Cell current = cellScanner.current(); + assertTrue(Bytes.equals(current.getRowArray(), current.getRowOffset(), + current.getRowLength(), ROW_1, 0, ROW_1.length)); + assertTrue(Bytes.equals(current.getQualifier(), Q1)); + assertTrue(Bytes.equals(current.getValue(), value1)); + cellScanner.advance(); + current = cellScanner.current(); + assertTrue(Bytes.equals(current.getRowArray(), current.getRowOffset(), + current.getRowLength(), ROW_1, 0, ROW_1.length)); + assertTrue(Bytes.equals(current.getQualifier(), Q2)); + assertTrue(Bytes.equals(current.getValue(), value2)); + cellScanner.advance(); + current = cellScanner.current(); + assertTrue(Bytes.equals(current.getRowArray(), current.getRowOffset(), + current.getRowLength(), ROW_1, 0, ROW_1.length)); + assertTrue(Bytes.equals(current.getQualifier(), Q3)); + assertTrue(Bytes.equals(current.getValue(), value3)); + + return null; + } finally { + table.close(); + connection.close(); + } + } + }); + TESTUSER.runAs(new PrivilegedExceptionAction<Void>() { public Void run() throws Exception { Table table = new HTable(conf, tableName); @@ -193,7 +235,6 @@ public class TestDefaultScanLabelGeneratorStack { assertFalse(cellScanner2.advance()); - return null; } finally { table.close(); http://git-wip-us.apache.org/repos/asf/hbase/blob/b2451856/hbase-server/src/test/java/org/apache/hadoop/hbase/security/visibility/TestEnforcingScanLabelGenerator.java ---------------------------------------------------------------------- diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/security/visibility/TestEnforcingScanLabelGenerator.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/security/visibility/TestEnforcingScanLabelGenerator.java index 916f730..a06f03d 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/security/visibility/TestEnforcingScanLabelGenerator.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/security/visibility/TestEnforcingScanLabelGenerator.java @@ -28,6 +28,8 @@ import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.hbase.HBaseTestingUtility; import org.apache.hadoop.hbase.HConstants; import org.apache.hadoop.hbase.TableName; +import org.apache.hadoop.hbase.client.Connection; +import org.apache.hadoop.hbase.client.ConnectionFactory; import org.apache.hadoop.hbase.client.Get; import org.apache.hadoop.hbase.client.HTable; import org.apache.hadoop.hbase.client.Put; @@ -119,6 +121,26 @@ public class TestEnforcingScanLabelGenerator { } }); + // Test that super user can see all the cells. + SUPERUSER.runAs(new PrivilegedExceptionAction<Void>() { + public Void run() throws Exception { + Connection connection = ConnectionFactory.createConnection(conf); + Table table = connection.getTable(tableName); + try { + // Test that super user can see all the cells. + Get get = new Get(ROW_1); + Result result = table.get(get); + assertTrue("Missing authorization", result.containsColumn(CF, Q1)); + assertTrue("Missing authorization", result.containsColumn(CF, Q2)); + assertTrue("Missing authorization", result.containsColumn(CF, Q3)); + return null; + } finally { + table.close(); + connection.close(); + } + } + }); + TESTUSER.runAs(new PrivilegedExceptionAction<Void>() { public Void run() throws Exception { Table table = new HTable(conf, tableName);
