Exploring an alternative AccessController implementation (original) (raw)

David Lloyd david.lloyd at redhat.com
Mon Dec 11 23:35:54 UTC 2017

This past weekend I took some time to try out an idea I've been kicking around for a couple of weeks: a pure-Java (no native) AccessController implementation [1].

•Rationale•

I have a number of reasons for attempting this enhancement.

• I think it would be beneficial (security-wise) if access control was purely Java, because the set of users who can understand, enhance, and troubleshoot the implementation would be expanded from those with a relatively specific intersection of knowledge (hotspot, C++, Java, security) to a significantly larger set of people. • I suspect that there are multiple performance gains to be had by eliminating the jump to JNI and dropping the array-centric current design. • It is interesting!

•Goals•

The things I primarily wish to accomplish were:

• Test the hypotheses that a pure Java AccessController is even possible • Eliminate, to the maximum extent possible, the cost of plain doPrivileged() • Reduce the cost of getContext() and other doPrivileged() methods

Secondary goals:

• Discover the limitations of Java-side access checking • Explore some optimization ideas • Have fun!

•Principal concepts•

Of main interest is that AccessControlContext is now an immutable linked list with each instance containing a ProtectionDomain and a "next" pointer. Any given AccessControlContext permits the intersection of its ProtectionDomain and whatever the next AccessControlContext permits.

All AccessControlContext instances are ultimately linked to a singleton ROOT_CONTEXT (or in certain less-likely circumstances, a clone thereof). Any two given AccessControlContext instances may share an arbitrary number of tail nodes. Any given ProtectionDomain will appear in an ACC list only once (by equals/hashCode contract). The means that, assuming well-written ClassLoaders, the depth of the ACC stack will usually be shallow compared to the depth of the call stack, as the ACC stack will usually maximally contain a number of entries equal to the total number of JARs or modules present in a given application.

Because ACC is a linked list, using a DomainCombiner will likely impose a significant penalty (because it operates on ProtectionDomain[], thus it entials the unwinding of the linked list to an array and back again), though there's nothing in the design here that would prohibit them from actually working. There might be some optimization(s) possible that I didn't think of on my first pass through the code. Nevertheless, I marked DomainCombiner and its use sites as @Deprecated in this implementation, because it violates the principle of only narrowing access (as well as, in my opinion, any other sane security principle, given that a DC can arbitrarily muck up an ACC before any access check). Coming up with an alternative for Subject association (and similar use cases) is something that could be discussed separately.

Methods were added to ACC to get new ACC instances that consist of the intersection of the existing ACC and additional protection domains (see AccessControlContext#with(*)). Because these instance methods always yield a less-privileged ACC, I determined that these methods do not need additional permission checks. Calling with() with a protection domain that is null or is already included in the ACC will return the same ACC instance.

Making ACC act like a stack has a similar algorithmic complexity to the old array-based code in many cases, though it's possible that in a few cases, N×M iteration was able to be optimized away. There is a potential disadvantage in that linked lists can have worse cache locality compared to arrays.

However, using a linked list/stack also allows for a potentially interesting optimization: the caching of ACC instances into the StackFrame of calling classes. I made a placeholder API method (for illustrative purposes) in JavaLangAccess which is meant to allow retrieval and update of an AccessControlContext field on a StackFrame. This would allow (for example) consecutive or nearby calls to AccessController.getContext() to share instances. Of course this would require a potentially complex modification to the JVM itself to reserve space for an extra reference field on every stack frame, and it is not certain that there would be a net benefit, at least not unless it could be tested.

If a cached AccessControllerContext instance is found on a stack frame, then it would also allow checkPermission() to be optimized as well: if a frame is encountered with a cached ACC, then traversal can end and the checkPermission() method on ACC can be used, which is sure to be optimized as the ACC stack will contain no duplicate items.

•Implementation•

So far, my first rough implementation (which can be found at [1]) consists of 822 additions and 1709 deletions, so that's what I could call a move in the right direction (especially for security-related code). It seems to "work", in that regular applications seem to function. It is not thoroughly tested yet; I wanted to gauge interest/reactions before I actually get serious about it.

Some of the important or interesting aspects of the implementation:

• All the native code relating to AccessControlContext and AccessController was deleted. • The plain AccessController.doPrivileged() method now has no overhead, other than setting (and clearing) a thread-local value around calling the action run() method. • I was able to eliminate two different SharedSecrets interfaces, which seems like a good thing. • In particular, since "intersection" contexts are now an integral part of the AccessControlContext API, I was able to delete JavaSecurityAccess.doIntersectionPrivilege() method and replace its usages with public API. • The constructor for AccessControlContext which accepts an array of ProtectionDomains is deprecated (and, still requires a permission check) but still functions. All usages of this constructor in the JDK should be gone. • Because StackWalker is used, AccessController behaves differently before VM.initLevel(1) as before that point StackWalker cannot function. Some analysis would be necessary to double-check that the few calls to AccessController.doPrivileged and AccessController.getContext before this point are OK. • The doPrivileged methods which accept a permissions array are implemented by adding in an extra ProtectionDomain into the ACC chain, which contains exactly the given permissions, with the code source of the caller of doPrivileged. The codeSource part might not be necessary, but it seems like a good idea from a diagnostic perspective. • The actual permission checker in AccessController uses StackWalker to search the stack for the first ProtectionDomain which does not grant the given permission, stopping at the first frame after a doPrivileged-type call. It is likely that this code might test the same protection domain multiple times; but, there are definitely more ideas to play with here in terms of performance optimization. It's possible that, with the cache described above, using AccessController.getContext().checkPermission() instead might yield a net gain for some applications or libraries which do frequent permission checks, because the traversed stack would be optimal in many cases. • I made an effort to preserve the comparison behavior of ProtectionDomain, such that they are considered equal only if they are of the same class and the equals() method returns true. • I took the liberty of making a few API additions: ◦ AccessController.getPrivilegedContext() - like getContext(), except the resultant ACC only contains the immediate caller's protection domain; requires no special privileges. ◦ AccessController.getPrivilegedContext(Permission... perms) - same, except it also adds another ProtectionDomain restricted to the given permissions. This replaces many utility methods found throughout the JDK. ◦ AccessControlContext.with(ProtectionDomain) and a vararg variant - easily add protection domains to the given ACC, returning a more-restricted ACC. • AccessControlContext.equals() is still terrible, though perhaps slightly less so than it was, as I presently cache and compare hashCode values, so it should still be avoided. The specification appears to give not much leeway here.

I had an unexpected amount of fun writing this, and I hope you all find it interesting.

[1] https://github.com/dmlloyd/openjdk-secmgr/commit/secmgr-v1?w=1

More information about the security-dev mailing list