From 56142a89952533fef922fa86739a879c073e7c2a Mon Sep 17 00:00:00 2001 From: Jeffrey Manno Date: Wed, 9 Dec 2020 10:26:20 -0500 Subject: [PATCH] Throw sercurity exceptions when permissions checks fail. Backport to 1.10 (#1830) * backport sercurity exceptions to 1.10 * fix AuditMessageIT --- .../master/MasterClientServiceHandler.java | 21 ++++++++++++------- .../apache/accumulo/test/AuditMessageIT.java | 2 +- 2 files changed, 15 insertions(+), 8 deletions(-) diff --git a/server/master/src/main/java/org/apache/accumulo/master/MasterClientServiceHandler.java b/server/master/src/main/java/org/apache/accumulo/master/MasterClientServiceHandler.java index ab45a49eecd..608a81951de 100644 --- a/server/master/src/main/java/org/apache/accumulo/master/MasterClientServiceHandler.java +++ b/server/master/src/main/java/org/apache/accumulo/master/MasterClientServiceHandler.java @@ -118,7 +118,8 @@ protected MasterClientServiceHandler(Master master) { public long initiateFlush(TInfo tinfo, TCredentials c, String tableId) throws ThriftSecurityException, ThriftTableOperationException { String namespaceId = getNamespaceIdFromTableId(TableOperation.FLUSH, tableId); - master.security.canFlush(c, tableId, namespaceId); + if (!master.security.canFlush(c, tableId, namespaceId)) + throw new ThriftSecurityException(c.getPrincipal(), SecurityErrorCode.PERMISSION_DENIED); String zTablePath = Constants.ZROOT + "/" + master.getInstance().getInstanceID() + Constants.ZTABLES + "/" + tableId + Constants.ZTABLE_FLUSH_ID; @@ -150,7 +151,8 @@ public void waitForFlush(TInfo tinfo, TCredentials c, String tableId, ByteBuffer ByteBuffer endRow, long flushID, long maxLoops) throws ThriftSecurityException, ThriftTableOperationException { String namespaceId = getNamespaceIdFromTableId(TableOperation.FLUSH, tableId); - master.security.canFlush(c, tableId, namespaceId); + if (!master.security.canFlush(c, tableId, namespaceId)) + throw new ThriftSecurityException(c.getPrincipal(), SecurityErrorCode.PERMISSION_DENIED); if (endRow != null && startRow != null && ByteBufferUtil.toText(startRow).compareTo(ByteBufferUtil.toText(endRow)) >= 0) @@ -305,7 +307,8 @@ public void setTableProperty(TInfo info, TCredentials credentials, String tableN @Override public void shutdown(TInfo info, TCredentials c, boolean stopTabletServers) throws ThriftSecurityException { - master.security.canPerformSystemActions(c); + if (!master.security.canPerformSystemActions(c)) + throw new ThriftSecurityException(c.getPrincipal(), SecurityErrorCode.PERMISSION_DENIED); if (stopTabletServers) { master.setMasterGoalState(MasterGoalState.CLEAN_STOP); EventCoordinator.Listener eventListener = master.nextEvent.getListener(); @@ -319,7 +322,8 @@ public void shutdown(TInfo info, TCredentials c, boolean stopTabletServers) @Override public void shutdownTabletServer(TInfo info, TCredentials c, String tabletServer, boolean force) throws ThriftSecurityException { - master.security.canPerformSystemActions(c); + if (!master.security.canPerformSystemActions(c)) + throw new ThriftSecurityException(c.getPrincipal(), SecurityErrorCode.PERMISSION_DENIED); final TServerInstance doomed = master.tserverSet.find(tabletServer); if (!force) { @@ -391,7 +395,8 @@ public void reportTabletStatus(TInfo info, TCredentials credentials, String serv @Override public void setMasterGoalState(TInfo info, TCredentials c, MasterGoalState state) throws ThriftSecurityException { - master.security.canPerformSystemActions(c); + if (!master.security.canPerformSystemActions(c)) + throw new ThriftSecurityException(c.getPrincipal(), SecurityErrorCode.PERMISSION_DENIED); master.setMasterGoalState(state); } @@ -399,7 +404,8 @@ public void setMasterGoalState(TInfo info, TCredentials c, MasterGoalState state @Override public void removeSystemProperty(TInfo info, TCredentials c, String property) throws ThriftSecurityException { - master.security.canPerformSystemActions(c); + if (!master.security.canPerformSystemActions(c)) + throw new ThriftSecurityException(c.getPrincipal(), SecurityErrorCode.PERMISSION_DENIED); try { SystemPropUtil.removeSystemProperty(property); @@ -413,7 +419,8 @@ public void removeSystemProperty(TInfo info, TCredentials c, String property) @Override public void setSystemProperty(TInfo info, TCredentials c, String property, String value) throws ThriftSecurityException, TException { - master.security.canPerformSystemActions(c); + if (!master.security.canPerformSystemActions(c)) + throw new ThriftSecurityException(c.getPrincipal(), SecurityErrorCode.PERMISSION_DENIED); try { SystemPropUtil.setSystemProperty(property, value); diff --git a/test/src/main/java/org/apache/accumulo/test/AuditMessageIT.java b/test/src/main/java/org/apache/accumulo/test/AuditMessageIT.java index abaed738b27..69e294bdc1d 100644 --- a/test/src/main/java/org/apache/accumulo/test/AuditMessageIT.java +++ b/test/src/main/java/org/apache/accumulo/test/AuditMessageIT.java @@ -465,7 +465,7 @@ public void testDeniedAudits() throws AccumuloSecurityException, AccumuloExcepti auditConnector.tableOperations().rename(OLD_TEST_TABLE_NAME, NEW_TEST_TABLE_NAME); } catch (AccumuloSecurityException ex) {} try { - auditConnector.tableOperations().clone(OLD_TEST_TABLE_NAME, NEW_TEST_TABLE_NAME, true, + auditConnector.tableOperations().clone(OLD_TEST_TABLE_NAME, NEW_TEST_TABLE_NAME, false, Collections.emptyMap(), Collections.emptySet()); } catch (AccumuloSecurityException ex) {} try {