[KARAF-4351] Optimize JAAS ACC/Subject access in bulk RBAC calls (cherry picked from commit fe2a82b1bc12c91e8f61e8ec1dc237e09aa0052c)
Project: http://git-wip-us.apache.org/repos/asf/karaf/repo Commit: http://git-wip-us.apache.org/repos/asf/karaf/commit/5b29a446 Tree: http://git-wip-us.apache.org/repos/asf/karaf/tree/5b29a446 Diff: http://git-wip-us.apache.org/repos/asf/karaf/diff/5b29a446 Branch: refs/heads/karaf-2.x Commit: 5b29a4466165b519637b9aa9e0b9c3bf0794b7b9 Parents: e96633f Author: Grzegorz Grzybek <[email protected]> Authored: Mon Feb 22 11:00:09 2016 +0100 Committer: Grzegorz Grzybek <[email protected]> Committed: Mon Feb 22 14:34:29 2016 +0100 ---------------------------------------------------------------------- .../karaf/management/KarafMBeanServerGuard.java | 34 ++++++++++++-------- .../management/internal/BulkRequestContext.java | 32 ++++++++++++++++++ .../management/KarafMBeanServerGuardTest.java | 2 +- .../internal/JMXSecurityMBeanImplTestCase.java | 2 +- 4 files changed, 54 insertions(+), 16 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/karaf/blob/5b29a446/management/server/src/main/java/org/apache/karaf/management/KarafMBeanServerGuard.java ---------------------------------------------------------------------- diff --git a/management/server/src/main/java/org/apache/karaf/management/KarafMBeanServerGuard.java b/management/server/src/main/java/org/apache/karaf/management/KarafMBeanServerGuard.java index e4e0377..d0389a2 100644 --- a/management/server/src/main/java/org/apache/karaf/management/KarafMBeanServerGuard.java +++ b/management/server/src/main/java/org/apache/karaf/management/KarafMBeanServerGuard.java @@ -249,7 +249,7 @@ public class KarafMBeanServerGuard implements InvocationHandler { return true; } for (String role : getRequiredRoles(context, objectName, methodName, signature)) { - if (currentUserHasRole(role)) + if (currentUserHasRole(context.getPrincipals(), role)) return true; } @@ -463,12 +463,28 @@ public class KarafMBeanServerGuard implements InvocationHandler { res.add(JMX_ACL_PID_PREFIX); // this is the top PID (aka jmx.acl) return res; } - static boolean currentUserHasRole(String requestedRole) { if (ROLE_WILDCARD.equals(requestedRole)) { return true; } - + + AccessControlContext acc = AccessController.getContext(); + if (acc == null) { + return false; + } + Subject subject = Subject.getSubject(acc); + if (subject == null) { + return false; + } + + return currentUserHasRole(subject.getPrincipals(), requestedRole); + } + + static boolean currentUserHasRole(Set<Principal> principals, String requestedRole) { + if (ROLE_WILDCARD.equals(requestedRole)) { + return true; + } + String clazz; String role; int index = requestedRole.indexOf(':'); @@ -480,17 +496,7 @@ public class KarafMBeanServerGuard implements InvocationHandler { role = requestedRole; } - AccessControlContext acc = AccessController.getContext(); - if (acc == null) { - return false; - } - Subject subject = Subject.getSubject(acc); - - if (subject == null) { - return false; - } - - for (Principal p : subject.getPrincipals()) { + for (Principal p : principals) { if (clazz.equals(p.getClass().getName()) && role.equals(p.getName())) { return true; } http://git-wip-us.apache.org/repos/asf/karaf/blob/5b29a446/management/server/src/main/java/org/apache/karaf/management/internal/BulkRequestContext.java ---------------------------------------------------------------------- diff --git a/management/server/src/main/java/org/apache/karaf/management/internal/BulkRequestContext.java b/management/server/src/main/java/org/apache/karaf/management/internal/BulkRequestContext.java index 1c72a60..59cb425 100644 --- a/management/server/src/main/java/org/apache/karaf/management/internal/BulkRequestContext.java +++ b/management/server/src/main/java/org/apache/karaf/management/internal/BulkRequestContext.java @@ -17,11 +17,18 @@ package org.apache.karaf.management.internal; import java.io.IOException; +import java.security.AccessControlContext; +import java.security.AccessController; +import java.security.Principal; import java.util.ArrayList; import java.util.Dictionary; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Set; + +import javax.security.auth.Subject; import org.osgi.framework.InvalidSyntaxException; import org.osgi.service.cm.Configuration; @@ -41,6 +48,11 @@ public class BulkRequestContext { private ConfigurationAdmin configAdmin; + // if there's AccessControlContext or subject, we can fail fast + private boolean anonymous = false; + // otherwise we can cache current subject's principals for faster access + private Set<Principal> principals = new HashSet<Principal>(); + // cache with lifecycle bound to BulkRequestContext instance private Map<String, Dictionary<String, Object>> cachedConfigurations = new HashMap<String, Dictionary<String, Object>>(); @@ -50,6 +62,18 @@ public class BulkRequestContext { BulkRequestContext context = new BulkRequestContext(); context.configAdmin = configAdmin; try { + // check JAAS subject here + AccessControlContext acc = AccessController.getContext(); + if (acc == null) { + context.anonymous = true; + } else { + Subject subject = Subject.getSubject(acc); + if (subject == null) { + context.anonymous = true; + } else { + context.principals.addAll(subject.getPrincipals()); + } + } // list available ACL configs - valid for this instance only for (Configuration config : configAdmin.listConfigurations("(service.pid=jmx.acl*)")) { context.allPids.add(config.getPid()); @@ -97,4 +121,12 @@ public class BulkRequestContext { return cachedConfigurations.get(generalPid); } + public boolean isAnonymous() { + return anonymous; + } + + public Set<Principal> getPrincipals() { + return principals; + } + } http://git-wip-us.apache.org/repos/asf/karaf/blob/5b29a446/management/server/src/test/java/org/apache/karaf/management/KarafMBeanServerGuardTest.java ---------------------------------------------------------------------- diff --git a/management/server/src/test/java/org/apache/karaf/management/KarafMBeanServerGuardTest.java b/management/server/src/test/java/org/apache/karaf/management/KarafMBeanServerGuardTest.java index 6a24144..f8d0454 100644 --- a/management/server/src/test/java/org/apache/karaf/management/KarafMBeanServerGuardTest.java +++ b/management/server/src/test/java/org/apache/karaf/management/KarafMBeanServerGuardTest.java @@ -464,7 +464,7 @@ public class KarafMBeanServerGuardTest extends TestCase { ConfigurationAdmin ca = EasyMock.createMock(ConfigurationAdmin.class); for (Configuration c : allConfigs) { - EasyMock.expect(ca.getConfiguration(c.getPid())).andReturn(c).anyTimes(); + EasyMock.expect(ca.getConfiguration(c.getPid(), null)).andReturn(c).anyTimes(); } EasyMock.expect(ca.listConfigurations(EasyMock.eq("(service.pid=jmx.acl*)"))).andReturn( allConfigs.toArray(new Configuration[]{})).anyTimes(); http://git-wip-us.apache.org/repos/asf/karaf/blob/5b29a446/management/server/src/test/java/org/apache/karaf/management/internal/JMXSecurityMBeanImplTestCase.java ---------------------------------------------------------------------- diff --git a/management/server/src/test/java/org/apache/karaf/management/internal/JMXSecurityMBeanImplTestCase.java b/management/server/src/test/java/org/apache/karaf/management/internal/JMXSecurityMBeanImplTestCase.java index d1bd053..3700395 100644 --- a/management/server/src/test/java/org/apache/karaf/management/internal/JMXSecurityMBeanImplTestCase.java +++ b/management/server/src/test/java/org/apache/karaf/management/internal/JMXSecurityMBeanImplTestCase.java @@ -214,7 +214,7 @@ public class JMXSecurityMBeanImplTestCase extends TestCase { EasyMock.expect(fooWildcardTesting.getPid()).andReturn("jmx.acl.foo._.testing").once(); EasyMock.replay(fooWildcardTesting); - Dictionary<String, Object> fooBarProperties = new Hashtable<>(); + Dictionary<String, Object> fooBarProperties = new Hashtable<String, Object>(); // using '*' frees us from mocking JAAS fooBarProperties.put("testMethod(java.lang.String)", "*"); fooBarProperties.put("testMethod(long)", "*");
