From f57685130d22a0a04dfc71f1b7fdc4a0621d86d0 Mon Sep 17 00:00:00 2001 From: jbachorik Date: Fri, 7 Mar 2014 10:15:36 +0100 Subject: [PATCH] 8029755: Enhance subject class Reviewed-by: sla, dfuchs, hawtin --- .../jmx/remote/security/SubjectDelegator.java | 93 +++++--------- .../com/sun/jmx/remote/util/CacheMap.java | 121 ------------------ .../remote/mandatory/util/CacheMapTest.java | 110 ---------------- 3 files changed, 32 insertions(+), 292 deletions(-) delete mode 100644 src/share/classes/com/sun/jmx/remote/util/CacheMap.java delete mode 100644 test/javax/management/remote/mandatory/util/CacheMapTest.java diff --git a/src/share/classes/com/sun/jmx/remote/security/SubjectDelegator.java b/src/share/classes/com/sun/jmx/remote/security/SubjectDelegator.java index a69c501d2..536e23156 100644 --- a/src/share/classes/com/sun/jmx/remote/security/SubjectDelegator.java +++ b/src/share/classes/com/sun/jmx/remote/security/SubjectDelegator.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2003, 2006, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2003, 2014, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -34,22 +34,14 @@ import javax.security.auth.Subject; import javax.management.remote.SubjectDelegationPermission; -import com.sun.jmx.remote.util.CacheMap; -import java.util.ArrayList; -import java.util.Collection; +import java.util.*; public class SubjectDelegator { - private static final int PRINCIPALS_CACHE_SIZE = 10; - private static final int ACC_CACHE_SIZE = 10; - - private CacheMap principalsCache; - private CacheMap accCache; - /* Return the AccessControlContext appropriate to execute an operation on behalf of the delegatedSubject. If the authenticatedAccessControlContext does not have permission to delegate to that subject, throw SecurityException. */ - public synchronized AccessControlContext + public AccessControlContext delegatedContext(AccessControlContext authenticatedACC, Subject delegatedSubject, boolean removeCallerContext) @@ -58,56 +50,14 @@ public class SubjectDelegator { if (System.getSecurityManager() != null && authenticatedACC == null) { throw new SecurityException("Illegal AccessControlContext: null"); } - if (principalsCache == null || accCache == null) { - principalsCache = - new CacheMap<>(PRINCIPALS_CACHE_SIZE); - accCache = - new CacheMap<>(ACC_CACHE_SIZE); - } - - // Retrieve the principals for the given - // delegated subject from the cache - // - Principal[] delegatedPrincipals = principalsCache.get(delegatedSubject); - - // Convert the set of principals stored in the - // delegated subject into an array of principals - // and store it in the cache - // - if (delegatedPrincipals == null) { - delegatedPrincipals = - delegatedSubject.getPrincipals().toArray(new Principal[0]); - principalsCache.put(delegatedSubject, delegatedPrincipals); - } - - // Retrieve the access control context for the - // given delegated subject from the cache - // - AccessControlContext delegatedACC = accCache.get(delegatedSubject); - - // Build the access control context to be used - // when executing code as the delegated subject - // and store it in the cache - // - if (delegatedACC == null) { - if (removeCallerContext) { - delegatedACC = - JMXSubjectDomainCombiner.getDomainCombinerContext( - delegatedSubject); - } else { - delegatedACC = - JMXSubjectDomainCombiner.getContext(delegatedSubject); - } - accCache.put(delegatedSubject, delegatedACC); - } // Check if the subject delegation permission allows the // authenticated subject to assume the identity of each // principal in the delegated subject // - final Principal[] dp = delegatedPrincipals; - final Collection permissions = new ArrayList<>(dp.length); - for(Principal p : dp) { + Collection ps = getSubjectPrincipals(delegatedSubject); + final Collection permissions = new ArrayList<>(ps.size()); + for(Principal p : ps) { final String pname = p.getClass().getName() + "." + p.getName(); permissions.add(new SubjectDelegationPermission(pname)); } @@ -122,7 +72,15 @@ public class SubjectDelegator { }; AccessController.doPrivileged(action, authenticatedACC); - return delegatedACC; + return getDelegatedAcc(delegatedSubject, removeCallerContext); + } + + private AccessControlContext getDelegatedAcc(Subject delegatedSubject, boolean removeCallerContext) { + if (removeCallerContext) { + return JMXSubjectDomainCombiner.getDomainCombinerContext(delegatedSubject); + } else { + return JMXSubjectDomainCombiner.getContext(delegatedSubject); + } } /** @@ -137,11 +95,9 @@ public class SubjectDelegator { public static synchronized boolean checkRemoveCallerContext(Subject subject) { try { - final Principal[] dp = - subject.getPrincipals().toArray(new Principal[0]); - for (int i = 0 ; i < dp.length ; i++) { + for (Principal p : getSubjectPrincipals(subject)) { final String pname = - dp[i].getClass().getName() + "." + dp[i].getName(); + p.getClass().getName() + "." + p.getName(); final Permission sdp = new SubjectDelegationPermission(pname); AccessController.checkPermission(sdp); @@ -151,4 +107,19 @@ public class SubjectDelegator { } return true; } + + /** + * Retrieves the {@linkplain Subject} principals + * @param subject The subject + * @return If the {@code Subject} is immutable it will return the principals directly. + * If the {@code Subject} is mutable it will create an unmodifiable copy. + */ + private static Collection getSubjectPrincipals(Subject subject) { + if (subject.isReadOnly()) { + return subject.getPrincipals(); + } + + List principals = Arrays.asList(subject.getPrincipals().toArray(new Principal[0])); + return Collections.unmodifiableList(principals); + } } diff --git a/src/share/classes/com/sun/jmx/remote/util/CacheMap.java b/src/share/classes/com/sun/jmx/remote/util/CacheMap.java deleted file mode 100644 index ae21d074c..000000000 --- a/src/share/classes/com/sun/jmx/remote/util/CacheMap.java +++ /dev/null @@ -1,121 +0,0 @@ -/* - * Copyright (c) 2003, 2006, Oracle and/or its affiliates. All rights reserved. - * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. - * - * This code is free software; you can redistribute it and/or modify it - * under the terms of the GNU General Public License version 2 only, as - * published by the Free Software Foundation. Oracle designates this - * particular file as subject to the "Classpath" exception as provided - * by Oracle in the LICENSE file that accompanied this code. - * - * This code is distributed in the hope that it will be useful, but WITHOUT - * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or - * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License - * version 2 for more details (a copy is included in the LICENSE file that - * accompanied this code). - * - * You should have received a copy of the GNU General Public License version - * 2 along with this work; if not, write to the Free Software Foundation, - * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. - * - * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA - * or visit www.oracle.com if you need additional information or have any - * questions. - */ - -package com.sun.jmx.remote.util; - -import java.lang.ref.SoftReference; -import java.util.Iterator; -import java.util.LinkedList; -import java.util.List; -import java.util.WeakHashMap; - -import com.sun.jmx.mbeanserver.Util; - -/** - *

Like WeakHashMap, except that the keys of the n most - * recently-accessed entries are kept as {@link SoftReference soft - * references}. Accessing an element means creating it, or retrieving - * it with {@link #get(Object) get}. Because these entries are kept - * with soft references, they will tend to remain even if their keys - * are not referenced elsewhere. But if memory is short, they will - * be removed.

- */ -public class CacheMap extends WeakHashMap { - /** - *

Create a CacheMap that can keep up to - * nSoftReferences as soft references.

- * - * @param nSoftReferences Maximum number of keys to keep as soft - * references. Access times for {@link #get(Object) get} and - * {@link #put(Object, Object) put} have a component that scales - * linearly with nSoftReferences, so this value - * should not be too great. - * - * @throws IllegalArgumentException if - * nSoftReferences is negative. - */ - public CacheMap(int nSoftReferences) { - if (nSoftReferences < 0) { - throw new IllegalArgumentException("nSoftReferences = " + - nSoftReferences); - } - this.nSoftReferences = nSoftReferences; - } - - public V put(K key, V value) { - cache(key); - return super.put(key, value); - } - - public V get(Object key) { - cache(Util.cast(key)); - return super.get(key); - } - - /* We don't override remove(Object) or try to do something with - the map's iterators to detect removal. So we may keep useless - entries in the soft reference list for keys that have since - been removed. The assumption is that entries are added to the - cache but never removed. But the behavior is not wrong if - they are in fact removed -- the caching is just less - performant. */ - - private void cache(K key) { - Iterator> it = cache.iterator(); - while (it.hasNext()) { - SoftReference sref = it.next(); - K key1 = sref.get(); - if (key1 == null) - it.remove(); - else if (key.equals(key1)) { - // Move this element to the head of the LRU list - it.remove(); - cache.add(0, sref); - return; - } - } - - int size = cache.size(); - if (size == nSoftReferences) { - if (size == 0) - return; // degenerate case, equivalent to WeakHashMap - it.remove(); - } - - cache.add(0, new SoftReference(key)); - } - - /* List of soft references for the most-recently referenced keys. - The list is in most-recently-used order, i.e. the first element - is the most-recently referenced key. There are never more than - nSoftReferences elements of this list. - - If we didn't care about J2SE 1.3 compatibility, we could use - LinkedHashSet in conjunction with a subclass of SoftReference - whose equals and hashCode reflect the referent. */ - private final LinkedList> cache = - new LinkedList>(); - private final int nSoftReferences; -} diff --git a/test/javax/management/remote/mandatory/util/CacheMapTest.java b/test/javax/management/remote/mandatory/util/CacheMapTest.java deleted file mode 100644 index bdd55dbd4..000000000 --- a/test/javax/management/remote/mandatory/util/CacheMapTest.java +++ /dev/null @@ -1,110 +0,0 @@ -/* - * Copyright (c) 2003, Oracle and/or its affiliates. All rights reserved. - * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. - * - * This code is free software; you can redistribute it and/or modify it - * under the terms of the GNU General Public License version 2 only, as - * published by the Free Software Foundation. - * - * This code is distributed in the hope that it will be useful, but WITHOUT - * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or - * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License - * version 2 for more details (a copy is included in the LICENSE file that - * accompanied this code). - * - * You should have received a copy of the GNU General Public License version - * 2 along with this work; if not, write to the Free Software Foundation, - * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. - * - * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA - * or visit www.oracle.com if you need additional information or have any - * questions. - */ - -/* - * @test - * @bug 7654321 - * @summary Tests the CacheMap class. - * @author Eamonn McManus - * @run clean CacheMapTest - * @run build CacheMapTest - * @run main CacheMapTest - */ - -import java.util.Iterator; -import java.util.Map; - -import com.sun.jmx.remote.util.CacheMap; - -public class CacheMapTest { - public static void main(String[] args) { - try { - boolean ok = test(5) && test(100); - if (ok) { - System.out.println("Test completed"); - return; - } else { - System.out.println("Test failed!"); - System.exit(1); - } - } catch (Exception e) { - System.err.println("Unexpected exception: " + e); - e.printStackTrace(); - System.exit(1); - } - } - - private static boolean test(int cacheSize) throws Exception { - System.out.println("CacheMap test with cache size " + cacheSize); - CacheMap map = new CacheMap(cacheSize); - int size = 0; - int maxIterations = cacheSize * 10; - while (map.size() == size && size < maxIterations) { - Integer key = new Integer(size); - Object x = map.put(key, "x"); - if (x != null) { - System.out.println("Map already had entry " + key + "!"); - return false; - } - x = map.get(key); - if (!"x".equals(x)) { - System.out.println("Got back surprising value: " + x); - return false; - } - size++; - } - System.out.println("Map size is " + map.size() + " after inserting " + - size + " elements"); - do { - System.gc(); - Thread.sleep(1); - System.out.println("Map size is " + map.size() + " after GC"); - } while (map.size() > cacheSize); - if (map.size() < cacheSize) { - System.out.println("Map shrank to less than cache size: " + - map.size() + " (surprising but not wrong)"); - } else - System.out.println("Map shrank to cache size as expected"); - int lowest = size - cacheSize; - // lowest value that can still be in cache if LRU is respected - for (Iterator it = map.entrySet().iterator(); it.hasNext(); ) { - Map.Entry entry = (Map.Entry) it.next(); - Integer x = (Integer) entry.getKey(); - int xx = x.intValue(); - if (xx < lowest || xx >= size) { - System.out.println("Old value remained (" + x + "), " + - "expected none earlier than " + lowest); - return false; - } - Object xxx = entry.getValue(); - if (!"x".equals(xxx)) { - System.out.println("Got back surprising value: " + xxx); - return false; - } - } - if (map.size() > 0) - System.out.println("Remaining elements are the most recent ones"); - System.out.println("Test passed"); - return true; - } -} -- GitLab