Exploring an alternative AccessController implementation
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
--
- DML
More information about the security-dev
mailing list