You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@qpid.apache.org by rg...@apache.org on 2014/02/20 16:46:30 UTC
svn commit: r1570239 - in /qpid/trunk/qpid/java:
broker-core/src/main/java/org/apache/qpid/server/model/adapter/
broker-core/src/main/java/org/apache/qpid/server/security/
broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/acce...
Author: rgodfrey
Date: Thu Feb 20 15:46:29 2014
New Revision: 1570239
URL: http://svn.apache.org/r1570239
Log:
QPID-5567 : Further changes to SecurityMangager
Modified:
qpid/trunk/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/adapter/BrokerAdapter.java
qpid/trunk/qpid/java/broker-core/src/main/java/org/apache/qpid/server/security/AccessControl.java
qpid/trunk/qpid/java/broker-core/src/main/java/org/apache/qpid/server/security/SecurityManager.java
qpid/trunk/qpid/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/plugins/DefaultAccessControl.java
qpid/trunk/qpid/java/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/plugins/DefaultAccessControlTest.java
qpid/trunk/qpid/java/broker-plugins/management-jmx/src/main/java/org/apache/qpid/server/jmx/MBeanInvocationHandlerImpl.java
Modified: qpid/trunk/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/adapter/BrokerAdapter.java
URL: http://svn.apache.org/viewvc/qpid/trunk/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/adapter/BrokerAdapter.java?rev=1570239&r1=1570238&r2=1570239&view=diff
==============================================================================
--- qpid/trunk/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/adapter/BrokerAdapter.java (original)
+++ qpid/trunk/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/adapter/BrokerAdapter.java Thu Feb 20 15:46:29 2014
@@ -24,6 +24,8 @@ import java.lang.reflect.Type;
import java.net.InetSocketAddress;
import java.net.SocketAddress;
import java.security.AccessControlException;
+import java.security.PrivilegedAction;
+import java.security.PrivilegedExceptionAction;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
@@ -73,6 +75,8 @@ import org.apache.qpid.server.util.MapVa
import org.apache.qpid.server.virtualhost.VirtualHostRegistry;
import org.apache.qpid.util.SystemUtils;
+import javax.security.auth.Subject;
+
public class BrokerAdapter extends AbstractAdapter implements Broker, ConfigurationChangeListener
{
private static final Logger LOGGER = Logger.getLogger(BrokerAdapter.class);
@@ -297,15 +301,15 @@ public class BrokerAdapter extends Abstr
// permission has already been granted to create the virtual host
// disable further access check on other operations, e.g. create exchange
- SecurityManager.setAccessChecksDisabled(true);
- try
- {
- virtualHostAdapter.setDesiredState(State.INITIALISING, State.ACTIVE);
- }
- finally
- {
- SecurityManager.setAccessChecksDisabled(false);
- }
+ Subject.doAs(SecurityManager.SYSTEM, new PrivilegedAction<Object>()
+ {
+ @Override
+ public Object run()
+ {
+ virtualHostAdapter.setDesiredState(State.INITIALISING, State.ACTIVE);
+ return null;
+ }
+ });
return virtualHostAdapter;
}
Modified: qpid/trunk/qpid/java/broker-core/src/main/java/org/apache/qpid/server/security/AccessControl.java
URL: http://svn.apache.org/viewvc/qpid/trunk/qpid/java/broker-core/src/main/java/org/apache/qpid/server/security/AccessControl.java?rev=1570239&r1=1570238&r2=1570239&view=diff
==============================================================================
--- qpid/trunk/qpid/java/broker-core/src/main/java/org/apache/qpid/server/security/AccessControl.java (original)
+++ qpid/trunk/qpid/java/broker-core/src/main/java/org/apache/qpid/server/security/AccessControl.java Thu Feb 20 15:46:29 2014
@@ -23,22 +23,17 @@ import org.apache.qpid.server.security.a
import org.apache.qpid.server.security.access.Operation;
/**
- * The two methods, {@link #access(org.apache.qpid.server.security.access.ObjectType)} and {@link #authorise(Operation, ObjectType, ObjectProperties)},
- * return the {@link Result} of the security decision, which may be to {@link Result#ABSTAIN} if no decision is made.
+ * The method {@link #authorise(Operation, ObjectType, ObjectProperties)},
+ * returns the {@link Result} of the security decision, which may be to {@link Result#ABSTAIN} if no decision is made.
*/
public interface AccessControl
{
/**
- * Default result for {@link #access(org.apache.qpid.server.security.access.ObjectType)} or {@link #authorise(Operation, ObjectType, ObjectProperties)}.
+ * Default result for {@link #authorise(Operation, ObjectType, ObjectProperties)}.
*/
Result getDefault();
/**
- * Authorise access granted to an object instance.
- */
- Result access(ObjectType objectType);
-
- /**
* Authorise an operation on an object defined by a set of properties.
*/
Result authorise(Operation operation, ObjectType objectType, ObjectProperties properties);
Modified: qpid/trunk/qpid/java/broker-core/src/main/java/org/apache/qpid/server/security/SecurityManager.java
URL: http://svn.apache.org/viewvc/qpid/trunk/qpid/java/broker-core/src/main/java/org/apache/qpid/server/security/SecurityManager.java?rev=1570239&r1=1570238&r2=1570239&view=diff
==============================================================================
--- qpid/trunk/qpid/java/broker-core/src/main/java/org/apache/qpid/server/security/SecurityManager.java (original)
+++ qpid/trunk/qpid/java/broker-core/src/main/java/org/apache/qpid/server/security/SecurityManager.java Thu Feb 20 15:46:29 2014
@@ -36,6 +36,8 @@ import org.apache.qpid.server.security.a
import org.apache.qpid.server.security.access.Operation;
import org.apache.qpid.server.security.access.OperationLoggingDetails;
+import javax.security.auth.Subject;
+
import static org.apache.qpid.server.security.access.ObjectType.BROKER;
import static org.apache.qpid.server.security.access.ObjectType.EXCHANGE;
import static org.apache.qpid.server.security.access.ObjectType.GROUP;
@@ -54,8 +56,9 @@ import static org.apache.qpid.server.sec
import static org.apache.qpid.server.security.access.Operation.UNBIND;
import static org.apache.qpid.server.security.access.Operation.UPDATE;
-import java.net.SocketAddress;
import java.security.AccessControlException;
+import java.security.AccessController;
+import java.security.Principal;
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
@@ -67,7 +70,11 @@ public class SecurityManager implements
{
private static final Logger _logger = Logger.getLogger(SecurityManager.class);
- public static final ThreadLocal<Boolean> _accessChecksDisabled = new ClearingThreadLocal(false);
+ public static final Subject SYSTEM = new Subject(true,
+ Collections.singleton(new SystemPrincipal()),
+ Collections.emptySet(),
+ Collections.emptySet());
+
private ConcurrentHashMap<String, AccessControl> _globalPlugins = new ConcurrentHashMap<String, AccessControl>();
private ConcurrentHashMap<String, AccessControl> _hostPlugins = new ConcurrentHashMap<String, AccessControl>();
@@ -76,51 +83,6 @@ public class SecurityManager implements
private Broker _broker;
- /**
- * A special ThreadLocal, which calls remove() on itself whenever the value is
- * the default, to avoid leaving a default value set after its use has passed.
- */
- private static final class ClearingThreadLocal extends ThreadLocal<Boolean>
- {
- private Boolean _defaultValue;
-
- public ClearingThreadLocal(Boolean defaultValue)
- {
- super();
- _defaultValue = defaultValue;
- }
-
- @Override
- protected Boolean initialValue()
- {
- return _defaultValue;
- }
-
- @Override
- public void set(Boolean value)
- {
- if (value == _defaultValue)
- {
- super.remove();
- }
- else
- {
- super.set(value);
- }
- }
-
- @Override
- public Boolean get()
- {
- Boolean value = super.get();
- if (value == _defaultValue)
- {
- super.remove();
- }
- return value;
- }
- }
-
/*
* Used by the Broker.
*/
@@ -190,6 +152,19 @@ public class SecurityManager implements
return _logger;
}
+ private static final class SystemPrincipal implements Principal
+ {
+ private SystemPrincipal()
+ {
+ }
+
+ @Override
+ public String getName()
+ {
+ return "SYSTEM";
+ }
+ }
+
private abstract class AccessCheck
{
abstract Result allowed(AccessControl plugin);
@@ -197,7 +172,9 @@ public class SecurityManager implements
private boolean checkAllPlugins(AccessCheck checker)
{
- if(_accessChecksDisabled.get())
+ // If we are running as SYSTEM then no ACL checking
+ final Subject subject = Subject.getSubject(AccessController.getContext());
+ if(subject != null && !subject.getPrincipals(SystemPrincipal.class).isEmpty())
{
return true;
}
@@ -321,7 +298,7 @@ public class SecurityManager implements
{
Result allowed(AccessControl plugin)
{
- return plugin.access(ObjectType.MANAGEMENT);
+ return plugin.authorise(Operation.ACCESS, ObjectType.MANAGEMENT, ObjectProperties.EMPTY);
}
}))
{
@@ -335,7 +312,7 @@ public class SecurityManager implements
{
Result allowed(AccessControl plugin)
{
- return plugin.access(VIRTUALHOST);
+ return plugin.authorise(Operation.ACCESS, VIRTUALHOST, ObjectProperties.EMPTY);
}
}))
{
@@ -548,15 +525,6 @@ public class SecurityManager implements
}
}
- public static boolean setAccessChecksDisabled(final boolean status)
- {
- //remember current value
- boolean current = _accessChecksDisabled.get();
-
- _accessChecksDisabled.set(status);
-
- return current;
- }
private class PublishAccessCheck extends AccessCheck
{
Modified: qpid/trunk/qpid/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/plugins/DefaultAccessControl.java
URL: http://svn.apache.org/viewvc/qpid/trunk/qpid/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/plugins/DefaultAccessControl.java?rev=1570239&r1=1570238&r2=1570239&view=diff
==============================================================================
--- qpid/trunk/qpid/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/plugins/DefaultAccessControl.java (original)
+++ qpid/trunk/qpid/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/plugins/DefaultAccessControl.java Thu Feb 20 15:46:29 2014
@@ -113,49 +113,31 @@ public class DefaultAccessControl implem
}
/**
- * 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<ConnectionPrincipal> 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
* user the plugin will abstain.
*/
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<ConnectionPrincipal> 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, ""));
Modified: qpid/trunk/qpid/java/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/plugins/DefaultAccessControlTest.java
URL: http://svn.apache.org/viewvc/qpid/trunk/qpid/java/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/plugins/DefaultAccessControlTest.java?rev=1570239&r1=1570238&r2=1570239&view=diff
==============================================================================
--- qpid/trunk/qpid/java/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/plugins/DefaultAccessControlTest.java (original)
+++ qpid/trunk/qpid/java/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/plugins/DefaultAccessControlTest.java Thu Feb 20 15:46:29 2014
@@ -246,7 +246,7 @@ public class DefaultAccessControlTest ex
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 ex
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;
Modified: qpid/trunk/qpid/java/broker-plugins/management-jmx/src/main/java/org/apache/qpid/server/jmx/MBeanInvocationHandlerImpl.java
URL: http://svn.apache.org/viewvc/qpid/trunk/qpid/java/broker-plugins/management-jmx/src/main/java/org/apache/qpid/server/jmx/MBeanInvocationHandlerImpl.java?rev=1570239&r1=1570238&r2=1570239&view=diff
==============================================================================
--- qpid/trunk/qpid/java/broker-plugins/management-jmx/src/main/java/org/apache/qpid/server/jmx/MBeanInvocationHandlerImpl.java (original)
+++ qpid/trunk/qpid/java/broker-plugins/management-jmx/src/main/java/org/apache/qpid/server/jmx/MBeanInvocationHandlerImpl.java Thu Feb 20 15:46:29 2014
@@ -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
}
}
- 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
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<Object>()
+ {
+ @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)
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@qpid.apache.org
For additional commands, e-mail: commits-help@qpid.apache.org