-
Bug
-
Resolution: Fixed
-
P2
-
1.4.0
-
None
-
merlin
-
generic, sparc
-
generic, solaris_8
SubjectDomainCombiner.combine calls currentDomains[i].getPermissions() for every current domain, even if the result is not needed (it's only needed if there's a cache miss). Since PD.getPermissions() now consults the Policy, the end result is a significant performance regression (AccessController.getContext() slowed down by about .5ms on my system). Another inefficiency in this code is that the subjectPd is always stored back into cachedProtectionDomains, even on a cache hit. Another odd piece of code is that Policy.refresh() is always called if cache.auth.policy is false; there seems to be no justification for that given the new dynamic policy model.
There is also a semantic bug here, because the end result is that the cached protection domains contain the results of dynamic lookups on the Policy, instead of only containing the original permissions that were in the "uncombined" protection domain. Apparently this code does justify the need for a variant of PD.getPermissions that just returns the original permissions.
Another semantic bug is that the CacheEntry holds a strong reference to the classloader, which means that classloaders will never be GC'd. In addition, over the life of a long-running server, the combinations of codesources, subjects, and classloaders is likely to be relatively unbounded. The fact that nothing ever gets deleted from cachedProtectionDomains is likely to pose a problem.
In addittion, the code at the beginning of combine() that decides whether to call combineBackward is flawed. Checking the security property is irrelevant if someone has called javax Policy.setPolicy to set their own policy. What this code should be doing, it seems, is checking whether the current value from javax Policy.getPolicy() is an instance of sun.security.provider.PolicyFile.
There also seems to be a debug statement that can cause stack overflow: the debug statement that prints out the constructed CacheEntry (ce) is not done in a doPrivileged, and CacheEntry.toString calls Subject.toString, which attempts to print the private credentials, which does permission checks. I've seen this stack overflow in action, but I don't have a test case for it.
There is also a semantic bug here, because the end result is that the cached protection domains contain the results of dynamic lookups on the Policy, instead of only containing the original permissions that were in the "uncombined" protection domain. Apparently this code does justify the need for a variant of PD.getPermissions that just returns the original permissions.
Another semantic bug is that the CacheEntry holds a strong reference to the classloader, which means that classloaders will never be GC'd. In addition, over the life of a long-running server, the combinations of codesources, subjects, and classloaders is likely to be relatively unbounded. The fact that nothing ever gets deleted from cachedProtectionDomains is likely to pose a problem.
In addittion, the code at the beginning of combine() that decides whether to call combineBackward is flawed. Checking the security property is irrelevant if someone has called javax Policy.setPolicy to set their own policy. What this code should be doing, it seems, is checking whether the current value from javax Policy.getPolicy() is an instance of sun.security.provider.PolicyFile.
There also seems to be a debug statement that can cause stack overflow: the debug statement that prints out the constructed CacheEntry (ce) is not done in a doPrivileged, and CacheEntry.toString calls Subject.toString, which attempts to print the private credentials, which does permission checks. I've seen this stack overflow in action, but I don't have a test case for it.
- duplicates
-
JDK-4391662 subject doas tests fail even though the required permissions are granted
-
- Closed
-