From 608a0bb1b83fd2920dbc19dc2be399b27c62c1ba Mon Sep 17 00:00:00 2001 From: Robert Godfrey Date: Thu, 20 Feb 2014 15:46:29 +0000 Subject: QPID-5567 : Further changes to SecurityMangager git-svn-id: https://svn.apache.org/repos/asf/qpid/trunk@1570239 13f79535-47bb-0310-9956-ffa450edef68 --- .../access/plugins/DefaultAccessControl.java | 42 +++++++--------------- .../access/plugins/DefaultAccessControlTest.java | 4 +-- .../server/jmx/MBeanInvocationHandlerImpl.java | 35 +++++++++++------- 3 files changed, 36 insertions(+), 45 deletions(-) (limited to 'qpid/java/broker-plugins') diff --git a/qpid/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/plugins/DefaultAccessControl.java b/qpid/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/plugins/DefaultAccessControl.java index 75006ae697..f579ea0ec5 100644 --- a/qpid/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/plugins/DefaultAccessControl.java +++ b/qpid/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/plugins/DefaultAccessControl.java @@ -112,31 +112,6 @@ public class DefaultAccessControl implements AccessControl return _ruleSet.getDefault(); } - /** - * Object instance access authorisation. - * - * Delegate to the {@link #authorise(Operation, ObjectType, ObjectProperties)} method, with - * the operation set to ACCESS and no object properties. - */ - public Result access(ObjectType objectType) - { - InetAddress addressOfClient = null; - final Subject subject = Subject.getSubject(AccessController.getContext()); - if(subject != null) - { - Set principals = subject.getPrincipals(ConnectionPrincipal.class); - if(!principals.isEmpty()) - { - SocketAddress address = principals.iterator().next().getConnection().getRemoteAddress(); - if(address instanceof InetSocketAddress) - { - addressOfClient = ((InetSocketAddress) address).getAddress(); - } - } - } - return authoriseFromAddress(Operation.ACCESS, objectType, ObjectProperties.EMPTY, addressOfClient); - } - /** * Check if an operation is authorised by asking the configuration object about the access * control rules granted to the current thread's {@link Subject}. If there is no current @@ -144,18 +119,25 @@ public class DefaultAccessControl implements AccessControl */ public Result authorise(Operation operation, ObjectType objectType, ObjectProperties properties) { - return authoriseFromAddress(operation, objectType, properties, null); - } - - public Result authoriseFromAddress(Operation operation, ObjectType objectType, ObjectProperties properties, InetAddress addressOfClient) - { + InetAddress addressOfClient = null; final Subject subject = Subject.getSubject(AccessController.getContext()); + // Abstain if there is no subject/principal associated with this thread if (subject == null || subject.getPrincipals().size() == 0) { return Result.ABSTAIN; } + Set principals = subject.getPrincipals(ConnectionPrincipal.class); + if(!principals.isEmpty()) + { + SocketAddress address = principals.iterator().next().getConnection().getRemoteAddress(); + if(address instanceof InetSocketAddress) + { + addressOfClient = ((InetSocketAddress) address).getAddress(); + } + } + if(_logger.isDebugEnabled()) { _logger.debug("Checking " + operation + " " + objectType + " " + ObjectUtils.defaultIfNull(addressOfClient, "")); diff --git a/qpid/java/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/plugins/DefaultAccessControlTest.java b/qpid/java/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/plugins/DefaultAccessControlTest.java index 8ac4e0c424..e907a88001 100644 --- a/qpid/java/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/plugins/DefaultAccessControlTest.java +++ b/qpid/java/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/plugins/DefaultAccessControlTest.java @@ -246,7 +246,7 @@ public class DefaultAccessControlTest extends TestCase DefaultAccessControl accessControl = new DefaultAccessControl(mockRuleSet); - accessControl.access(ObjectType.VIRTUALHOST); + accessControl.authorise(Operation.ACCESS, ObjectType.VIRTUALHOST, ObjectProperties.EMPTY); verify(mockRuleSet).check(subject, Operation.ACCESS, ObjectType.VIRTUALHOST, ObjectProperties.EMPTY, inetAddress); return null; @@ -282,7 +282,7 @@ public class DefaultAccessControlTest extends TestCase inetAddress)).thenThrow(new RuntimeException()); DefaultAccessControl accessControl = new DefaultAccessControl(mockRuleSet); - Result result = accessControl.access(ObjectType.VIRTUALHOST); + Result result = accessControl.authorise(Operation.ACCESS, ObjectType.VIRTUALHOST, ObjectProperties.EMPTY); assertEquals(Result.DENIED, result); return null; diff --git a/qpid/java/broker-plugins/management-jmx/src/main/java/org/apache/qpid/server/jmx/MBeanInvocationHandlerImpl.java b/qpid/java/broker-plugins/management-jmx/src/main/java/org/apache/qpid/server/jmx/MBeanInvocationHandlerImpl.java index e78e59b7fc..f959977d1f 100644 --- a/qpid/java/broker-plugins/management-jmx/src/main/java/org/apache/qpid/server/jmx/MBeanInvocationHandlerImpl.java +++ b/qpid/java/broker-plugins/management-jmx/src/main/java/org/apache/qpid/server/jmx/MBeanInvocationHandlerImpl.java @@ -46,6 +46,9 @@ import java.lang.reflect.Method; import java.lang.reflect.Proxy; import java.security.AccessControlContext; import java.security.AccessController; +import java.security.PrivilegedAction; +import java.security.PrivilegedActionException; +import java.security.PrivilegedExceptionAction; import java.util.Arrays; /** @@ -201,7 +204,7 @@ public class MBeanInvocationHandlerImpl implements InvocationHandler } } - private Object authoriseAndInvoke(Method method, Object[] args) throws IllegalAccessException, InvocationTargetException + private Object authoriseAndInvoke(final Method method, final Object[] args) throws Throwable { String methodName; // Get the component, type and impact, which may be null @@ -229,23 +232,29 @@ public class MBeanInvocationHandlerImpl implements InvocationHandler Operation operation = (isAccessMethod(methodName) || impact == MBeanOperationInfo.INFO) ? Operation.ACCESS : Operation.UPDATE; security.authoriseMethod(operation, type, methodName); - boolean oldAccessChecksDisabled = false; - if(_managementRightsInferAllAccess) + if (_managementRightsInferAllAccess) { - oldAccessChecksDisabled = SecurityManager.setAccessChecksDisabled(true); + try + { + return Subject.doAs(SecurityManager.SYSTEM, new PrivilegedExceptionAction() + { + @Override + public Object run() throws IllegalAccessException, InvocationTargetException + { + return method.invoke(_mbs, args); + } + }); + } + catch (PrivilegedActionException e) + { + throw e.getCause(); + } } - - try + else { return method.invoke(_mbs, args); } - finally - { - if(_managementRightsInferAllAccess) - { - SecurityManager.setAccessChecksDisabled(oldAccessChecksDisabled); - } - } + } private String getType(Method method, Object[] args) -- cgit v1.2.1