From b06acfbf0b0aa169613985015a8482e5ea91fc15 Mon Sep 17 00:00:00 2001 From: ramkrishna Date: Thu, 19 Nov 2015 11:20:45 +0530 Subject: [PATCH] HBASE-14761 Deletes with and without visibility expression do not delete the matching mutation (Ram) --- .../VisibilityScanDeleteTracker.java | 166 +++++----- .../TestVisibilityLabelsWithDeletes.java | 287 ++++++++++++++++++ 2 files changed, 377 insertions(+), 76 deletions(-) diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/security/visibility/VisibilityScanDeleteTracker.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/security/visibility/VisibilityScanDeleteTracker.java index ea7ce337563..77786ad5ae8 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/security/visibility/VisibilityScanDeleteTracker.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/security/visibility/VisibilityScanDeleteTracker.java @@ -117,21 +117,26 @@ public class VisibilityScanDeleteTracker extends ScanDeleteTracker { // If tag is present in the delete boolean hasVisTag = false; if (delCell.getTagsLength() > 0) { + Byte deleteCellVisTagsFormat = null; switch (type) { case DeleteFamily: List delTags = new ArrayList(); - if (visibilityTagsDeleteFamily != null) { - Byte deleteCellVisTagsFormat = VisibilityUtils.extractVisibilityTags(delCell, delTags); - if (!delTags.isEmpty()) { - visibilityTagsDeleteFamily.add(new Triple, Byte, Long>( - delTags, deleteCellVisTagsFormat, delCell.getTimestamp())); - hasVisTag = true; - } + if (visibilityTagsDeleteFamily == null) { + visibilityTagsDeleteFamily = new ArrayList, Byte, Long>>(); + } + deleteCellVisTagsFormat = VisibilityUtils.extractVisibilityTags(delCell, delTags); + if (!delTags.isEmpty()) { + visibilityTagsDeleteFamily.add(new Triple, Byte, Long>(delTags, + deleteCellVisTagsFormat, delCell.getTimestamp())); + hasVisTag = true; } break; case DeleteFamilyVersion: + if(visibilityTagsDeleteFamilyVersion == null) { + visibilityTagsDeleteFamilyVersion = new ArrayList, Byte, Long>>(); + } delTags = new ArrayList(); - Byte deleteCellVisTagsFormat = VisibilityUtils.extractVisibilityTags(delCell, delTags); + deleteCellVisTagsFormat = VisibilityUtils.extractVisibilityTags(delCell, delTags); if (!delTags.isEmpty()) { visibilityTagsDeleteFamilyVersion.add(new Triple, Byte, Long>(delTags, deleteCellVisTagsFormat, delCell.getTimestamp())); @@ -165,23 +170,6 @@ public class VisibilityScanDeleteTracker extends ScanDeleteTracker { default: throw new IllegalArgumentException("Invalid delete type"); } - } else { - switch (type) { - case DeleteFamily: - visibilityTagsDeleteFamily = null; - break; - case DeleteFamilyVersion: - visibilityTagsDeleteFamilyVersion = null; - break; - case DeleteColumn: - visibilityTagsDeleteColumns = null; - break; - case Delete: - visiblityTagsDeleteColumnVersion = null; - break; - default: - throw new IllegalArgumentException("Invalid delete type"); - } } return hasVisTag; } @@ -194,28 +182,35 @@ public class VisibilityScanDeleteTracker extends ScanDeleteTracker { try { if (hasFamilyStamp) { if (visibilityTagsDeleteFamily != null) { - for (int i = 0; i < visibilityTagsDeleteFamily.size(); i++) { - // visibilityTagsDeleteFamily is ArrayList - Triple, Byte, Long> triple = visibilityTagsDeleteFamily.get(i); - if (timestamp <= triple.getThird()) { - List putVisTags = new ArrayList(); - Byte putCellVisTagsFormat = VisibilityUtils.extractVisibilityTags(cell, putVisTags); - boolean matchFound = VisibilityLabelServiceManager - .getInstance() - .getVisibilityLabelService() - .matchVisibility(putVisTags, putCellVisTagsFormat, triple.getFirst(), - triple.getSecond()); - if (matchFound) { - // A return type of FAMILY_DELETED will cause skip for all remaining cells from this - // family. We would like to match visibility expression on every put cells after - // this and only remove those matching with the family delete visibility. So we are - // returning FAMILY_VERSION_DELETED from here. - return DeleteResult.FAMILY_VERSION_DELETED; + if (!visibilityTagsDeleteFamily.isEmpty()) { + for (int i = 0; i < visibilityTagsDeleteFamily.size(); i++) { + // visibilityTagsDeleteFamily is ArrayList + Triple, Byte, Long> triple = visibilityTagsDeleteFamily.get(i); + if (timestamp <= triple.getThird()) { + List putVisTags = new ArrayList(); + Byte putCellVisTagsFormat = VisibilityUtils.extractVisibilityTags(cell, putVisTags); + boolean matchFound = VisibilityLabelServiceManager.getInstance() + .getVisibilityLabelService().matchVisibility(putVisTags, putCellVisTagsFormat, + triple.getFirst(), triple.getSecond()); + if (matchFound) { + // A return type of FAMILY_DELETED will cause skip for all remaining cells from + // this + // family. We would like to match visibility expression on every put cells after + // this and only remove those matching with the family delete visibility. So we + // are + // returning FAMILY_VERSION_DELETED from here. + return DeleteResult.FAMILY_VERSION_DELETED; + } } } + } else { + if (!VisibilityUtils.isVisibilityTagsPresent(cell) && timestamp <= familyStamp) { + // No tags + return DeleteResult.FAMILY_VERSION_DELETED; + } } } else { - if (!VisibilityUtils.isVisibilityTagsPresent(cell) && timestamp<=familyStamp) { + if (!VisibilityUtils.isVisibilityTagsPresent(cell) && timestamp <= familyStamp) { // No tags return DeleteResult.FAMILY_VERSION_DELETED; } @@ -223,21 +218,26 @@ public class VisibilityScanDeleteTracker extends ScanDeleteTracker { } if (familyVersionStamps.contains(Long.valueOf(timestamp))) { if (visibilityTagsDeleteFamilyVersion != null) { - for (int i = 0; i < visibilityTagsDeleteFamilyVersion.size(); i++) { - // visibilityTagsDeleteFamilyVersion is ArrayList - Triple, Byte, Long> triple = visibilityTagsDeleteFamilyVersion.get(i); - if (timestamp == triple.getThird()) { - List putVisTags = new ArrayList(); - Byte putCellVisTagsFormat = VisibilityUtils.extractVisibilityTags(cell, putVisTags); - boolean matchFound = VisibilityLabelServiceManager - .getInstance() - .getVisibilityLabelService() - .matchVisibility(putVisTags, putCellVisTagsFormat, triple.getFirst(), - triple.getSecond()); - if (matchFound) { - return DeleteResult.FAMILY_VERSION_DELETED; + if (!visibilityTagsDeleteFamilyVersion.isEmpty()) { + for (int i = 0; i < visibilityTagsDeleteFamilyVersion.size(); i++) { + // visibilityTagsDeleteFamilyVersion is ArrayList + Triple, Byte, Long> triple = visibilityTagsDeleteFamilyVersion.get(i); + if (timestamp == triple.getThird()) { + List putVisTags = new ArrayList(); + Byte putCellVisTagsFormat = VisibilityUtils.extractVisibilityTags(cell, putVisTags); + boolean matchFound = VisibilityLabelServiceManager.getInstance() + .getVisibilityLabelService().matchVisibility(putVisTags, putCellVisTagsFormat, + triple.getFirst(), triple.getSecond()); + if (matchFound) { + return DeleteResult.FAMILY_VERSION_DELETED; + } } } + } else { + if (!VisibilityUtils.isVisibilityTagsPresent(cell)) { + // No tags + return DeleteResult.FAMILY_VERSION_DELETED; + } } } else { if (!VisibilityUtils.isVisibilityTagsPresent(cell)) { @@ -253,15 +253,21 @@ public class VisibilityScanDeleteTracker extends ScanDeleteTracker { if (ret == 0) { if (deleteType == KeyValue.Type.DeleteColumn.getCode()) { if (visibilityTagsDeleteColumns != null) { - for (Pair, Byte> tags : visibilityTagsDeleteColumns) { - List putVisTags = new ArrayList(); - Byte putCellVisTagsFormat = VisibilityUtils.extractVisibilityTags(cell, putVisTags); - boolean matchFound = VisibilityLabelServiceManager - .getInstance() - .getVisibilityLabelService() - .matchVisibility(putVisTags, putCellVisTagsFormat, tags.getFirst(), - tags.getSecond()); - if (matchFound) { + if (!visibilityTagsDeleteColumns.isEmpty()) { + for (Pair, Byte> tags : visibilityTagsDeleteColumns) { + List putVisTags = new ArrayList(); + Byte putCellVisTagsFormat = + VisibilityUtils.extractVisibilityTags(cell, putVisTags); + boolean matchFound = VisibilityLabelServiceManager.getInstance() + .getVisibilityLabelService().matchVisibility(putVisTags, putCellVisTagsFormat, + tags.getFirst(), tags.getSecond()); + if (matchFound) { + return DeleteResult.VERSION_DELETED; + } + } + } else { + if (!VisibilityUtils.isVisibilityTagsPresent(cell)) { + // No tags return DeleteResult.VERSION_DELETED; } } @@ -276,15 +282,21 @@ public class VisibilityScanDeleteTracker extends ScanDeleteTracker { // If the timestamp is the same, keep this one if (timestamp == deleteTimestamp) { if (visiblityTagsDeleteColumnVersion != null) { - for (Pair, Byte> tags : visiblityTagsDeleteColumnVersion) { - List putVisTags = new ArrayList(); - Byte putCellVisTagsFormat = VisibilityUtils.extractVisibilityTags(cell, putVisTags); - boolean matchFound = VisibilityLabelServiceManager - .getInstance() - .getVisibilityLabelService() - .matchVisibility(putVisTags, putCellVisTagsFormat, tags.getFirst(), - tags.getSecond()); - if (matchFound) { + if (!visiblityTagsDeleteColumnVersion.isEmpty()) { + for (Pair, Byte> tags : visiblityTagsDeleteColumnVersion) { + List putVisTags = new ArrayList(); + Byte putCellVisTagsFormat = + VisibilityUtils.extractVisibilityTags(cell, putVisTags); + boolean matchFound = VisibilityLabelServiceManager.getInstance() + .getVisibilityLabelService().matchVisibility(putVisTags, putCellVisTagsFormat, + tags.getFirst(), tags.getSecond()); + if (matchFound) { + return DeleteResult.VERSION_DELETED; + } + } + } else { + if (!VisibilityUtils.isVisibilityTagsPresent(cell)) { + // No tags return DeleteResult.VERSION_DELETED; } } @@ -298,6 +310,7 @@ public class VisibilityScanDeleteTracker extends ScanDeleteTracker { } else if (ret < 0) { // Next column case. deleteBuffer = null; + // Can nullify this because we are moving to the next column visibilityTagsDeleteColumns = null; visiblityTagsDeleteColumnVersion = null; } else { @@ -316,9 +329,10 @@ public class VisibilityScanDeleteTracker extends ScanDeleteTracker { @Override public void reset() { super.reset(); + // clear only here visibilityTagsDeleteColumns = null; - visibilityTagsDeleteFamily = new ArrayList, Byte, Long>>(); - visibilityTagsDeleteFamilyVersion = new ArrayList, Byte, Long>>(); + visibilityTagsDeleteFamily = null; + visibilityTagsDeleteFamilyVersion = null; visiblityTagsDeleteColumnVersion = null; } } diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/security/visibility/TestVisibilityLabelsWithDeletes.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/security/visibility/TestVisibilityLabelsWithDeletes.java index 9e8bcd4c534..dd2b5fb3634 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/security/visibility/TestVisibilityLabelsWithDeletes.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/security/visibility/TestVisibilityLabelsWithDeletes.java @@ -469,6 +469,293 @@ public class TestVisibilityLabelsWithDeletes { } } + @Test + public void testDeleteColumnsWithoutAndWithVisibilityLabels() throws Exception { + final TableName tableName = TableName.valueOf(TEST_NAME.getMethodName()); + Admin hBaseAdmin = TEST_UTIL.getHBaseAdmin(); + HColumnDescriptor colDesc = new HColumnDescriptor(fam); + HTableDescriptor desc = new HTableDescriptor(tableName); + desc.addFamily(colDesc); + hBaseAdmin.createTable(desc); + try (Table table = TEST_UTIL.getConnection().getTable(tableName)) { + Put put = new Put(row1); + put.addColumn(fam, qual, value); + put.setCellVisibility(new CellVisibility(CONFIDENTIAL)); + table.put(put); + Delete d = new Delete(row1); + // without visibility + d.addColumns(fam, qual, HConstants.LATEST_TIMESTAMP); + table.delete(d); + PrivilegedExceptionAction scanAction = new PrivilegedExceptionAction() { + @Override + public Void run() throws Exception { + try (Connection connection = ConnectionFactory.createConnection(conf); + Table table = connection.getTable(tableName)) { + Scan s = new Scan(); + ResultScanner scanner = table.getScanner(s); + Result[] next = scanner.next(3); + assertEquals(next.length, 1); + } catch (Throwable t) { + throw new IOException(t); + } + return null; + } + }; + SUPERUSER.runAs(scanAction); + d = new Delete(row1); + // with visibility + d.setCellVisibility(new CellVisibility(CONFIDENTIAL)); + d.addColumns(fam, qual, HConstants.LATEST_TIMESTAMP); + table.delete(d); + scanAction = new PrivilegedExceptionAction() { + @Override + public Void run() throws Exception { + try (Connection connection = ConnectionFactory.createConnection(conf); + Table table = connection.getTable(tableName)) { + Scan s = new Scan(); + ResultScanner scanner = table.getScanner(s); + Result[] next = scanner.next(3); + assertEquals(next.length, 0); + } catch (Throwable t) { + throw new IOException(t); + } + return null; + } + }; + SUPERUSER.runAs(scanAction); + } + } + + @Test + public void testDeleteColumnsWithAndWithoutVisibilityLabels() throws Exception { + final TableName tableName = TableName.valueOf(TEST_NAME.getMethodName()); + Admin hBaseAdmin = TEST_UTIL.getHBaseAdmin(); + HColumnDescriptor colDesc = new HColumnDescriptor(fam); + HTableDescriptor desc = new HTableDescriptor(tableName); + desc.addFamily(colDesc); + hBaseAdmin.createTable(desc); + try (Table table = TEST_UTIL.getConnection().getTable(tableName)) { + Put put = new Put(row1); + put.addColumn(fam, qual, value); + put.setCellVisibility(new CellVisibility(CONFIDENTIAL)); + table.put(put); + Delete d = new Delete(row1); + // with visibility + d.setCellVisibility(new CellVisibility(CONFIDENTIAL)); + d.addColumns(fam, qual, HConstants.LATEST_TIMESTAMP); + table.delete(d); + PrivilegedExceptionAction scanAction = new PrivilegedExceptionAction() { + @Override + public Void run() throws Exception { + try (Connection connection = ConnectionFactory.createConnection(conf); + Table table = connection.getTable(tableName)) { + Scan s = new Scan(); + ResultScanner scanner = table.getScanner(s); + Result[] next = scanner.next(3); + assertEquals(next.length, 0); + } catch (Throwable t) { + throw new IOException(t); + } + return null; + } + }; + SUPERUSER.runAs(scanAction); + d = new Delete(row1); + // without visibility + d.addColumns(fam, qual, HConstants.LATEST_TIMESTAMP); + table.delete(d); + scanAction = new PrivilegedExceptionAction() { + @Override + public Void run() throws Exception { + try (Connection connection = ConnectionFactory.createConnection(conf); + Table table = connection.getTable(tableName)) { + Scan s = new Scan(); + ResultScanner scanner = table.getScanner(s); + Result[] next = scanner.next(3); + assertEquals(next.length, 0); + } catch (Throwable t) { + throw new IOException(t); + } + return null; + } + }; + SUPERUSER.runAs(scanAction); + } + } + + @Test + public void testDeleteFamiliesWithoutAndWithVisibilityLabels() throws Exception { + final TableName tableName = TableName.valueOf(TEST_NAME.getMethodName()); + Admin hBaseAdmin = TEST_UTIL.getHBaseAdmin(); + HColumnDescriptor colDesc = new HColumnDescriptor(fam); + HTableDescriptor desc = new HTableDescriptor(tableName); + desc.addFamily(colDesc); + hBaseAdmin.createTable(desc); + try (Table table = TEST_UTIL.getConnection().getTable(tableName)) { + Put put = new Put(row1); + put.addColumn(fam, qual, value); + put.setCellVisibility(new CellVisibility(CONFIDENTIAL)); + table.put(put); + Delete d = new Delete(row1); + // without visibility + d.addFamily(fam); + table.delete(d); + PrivilegedExceptionAction scanAction = new PrivilegedExceptionAction() { + @Override + public Void run() throws Exception { + try (Connection connection = ConnectionFactory.createConnection(conf); + Table table = connection.getTable(tableName)) { + Scan s = new Scan(); + ResultScanner scanner = table.getScanner(s); + Result[] next = scanner.next(3); + assertEquals(next.length, 1); + } catch (Throwable t) { + throw new IOException(t); + } + return null; + } + }; + SUPERUSER.runAs(scanAction); + d = new Delete(row1); + // with visibility + d.setCellVisibility(new CellVisibility(CONFIDENTIAL)); + d.addFamily(fam); + table.delete(d); + scanAction = new PrivilegedExceptionAction() { + @Override + public Void run() throws Exception { + try (Connection connection = ConnectionFactory.createConnection(conf); + Table table = connection.getTable(tableName)) { + Scan s = new Scan(); + ResultScanner scanner = table.getScanner(s); + Result[] next = scanner.next(3); + assertEquals(next.length, 0); + } catch (Throwable t) { + throw new IOException(t); + } + return null; + } + }; + SUPERUSER.runAs(scanAction); + } + } + + @Test + public void testDeleteFamiliesWithAndWithoutVisibilityLabels() throws Exception { + final TableName tableName = TableName.valueOf(TEST_NAME.getMethodName()); + Admin hBaseAdmin = TEST_UTIL.getHBaseAdmin(); + HColumnDescriptor colDesc = new HColumnDescriptor(fam); + HTableDescriptor desc = new HTableDescriptor(tableName); + desc.addFamily(colDesc); + hBaseAdmin.createTable(desc); + try (Table table = TEST_UTIL.getConnection().getTable(tableName)) { + Put put = new Put(row1); + put.addColumn(fam, qual, value); + put.setCellVisibility(new CellVisibility(CONFIDENTIAL)); + table.put(put); + Delete d = new Delete(row1); + d.setCellVisibility(new CellVisibility(CONFIDENTIAL)); + // with visibility + d.addFamily(fam); + table.delete(d); + PrivilegedExceptionAction scanAction = new PrivilegedExceptionAction() { + @Override + public Void run() throws Exception { + try (Connection connection = ConnectionFactory.createConnection(conf); + Table table = connection.getTable(tableName)) { + Scan s = new Scan(); + ResultScanner scanner = table.getScanner(s); + Result[] next = scanner.next(3); + assertEquals(next.length, 0); + } catch (Throwable t) { + throw new IOException(t); + } + return null; + } + }; + SUPERUSER.runAs(scanAction); + d = new Delete(row1); + // without visibility + d.addFamily(fam); + table.delete(d); + scanAction = new PrivilegedExceptionAction() { + @Override + public Void run() throws Exception { + try (Connection connection = ConnectionFactory.createConnection(conf); + Table table = connection.getTable(tableName)) { + Scan s = new Scan(); + ResultScanner scanner = table.getScanner(s); + Result[] next = scanner.next(3); + assertEquals(next.length, 0); + } catch (Throwable t) { + throw new IOException(t); + } + return null; + } + }; + SUPERUSER.runAs(scanAction); + } + } + + @Test + public void testDeletesWithoutAndWithVisibilityLabels() throws Exception { + final TableName tableName = TableName.valueOf(TEST_NAME.getMethodName()); + Admin hBaseAdmin = TEST_UTIL.getHBaseAdmin(); + HColumnDescriptor colDesc = new HColumnDescriptor(fam); + HTableDescriptor desc = new HTableDescriptor(tableName); + desc.addFamily(colDesc); + hBaseAdmin.createTable(desc); + try (Table table = TEST_UTIL.getConnection().getTable(tableName)) { + Put put = new Put(row1); + put.addColumn(fam, qual, value); + put.setCellVisibility(new CellVisibility(CONFIDENTIAL)); + table.put(put); + Delete d = new Delete(row1); + // without visibility + d.addColumn(fam, qual); + table.delete(d); + PrivilegedExceptionAction scanAction = new PrivilegedExceptionAction() { + @Override + public Void run() throws Exception { + try (Connection connection = ConnectionFactory.createConnection(conf); + Table table = connection.getTable(tableName)) { + Scan s = new Scan(); + ResultScanner scanner = table.getScanner(s); + // The delete would not be able to apply it because of visibility mismatch + Result[] next = scanner.next(3); + assertEquals(next.length, 1); + } catch (Throwable t) { + throw new IOException(t); + } + return null; + } + }; + SUPERUSER.runAs(scanAction); + d = new Delete(row1); + // with visibility + d.setCellVisibility(new CellVisibility(CONFIDENTIAL)); + d.addColumn(fam, qual); + table.delete(d); + scanAction = new PrivilegedExceptionAction() { + @Override + public Void run() throws Exception { + try (Connection connection = ConnectionFactory.createConnection(conf); + Table table = connection.getTable(tableName)) { + Scan s = new Scan(); + ResultScanner scanner = table.getScanner(s); + Result[] next = scanner.next(3); + // this will alone match + assertEquals(next.length, 0); + } catch (Throwable t) { + throw new IOException(t); + } + return null; + } + }; + SUPERUSER.runAs(scanAction); + } + } + @Test public void testVisibilityLabelsWithDeleteFamilyWithPutsReAppearing() throws Exception { final TableName tableName = TableName.valueOf(TEST_NAME.getMethodName());