search

jsr107 / jsr107spec

JSR107 Cache Specification
Apache License 2.0
415 stars 164 forks source link

Caching.getcache() requires SecurityManager access to all system properties #398

Closed aguibert closed 6 years ago

aguibert commented 6 years ago

When attempting to obtain a cache from an application, the JCache spec API attempts to read all system properties and then check for the one it is looking for. This requires the application developer to grant access to all system properties.

Example stack from JCache 1.1.0 API:

[WARNING ] CWWKE0921W: Current Java 2 Security policy reported a potential violation of Java 2 Security Permission. The application needs to have permissions addedPermission: 
("java.util.PropertyPermission" "*" "read,write")
Stack: 
java.security.AccessControlException: access denied ("java.util.PropertyPermission" "*" "read,write")java.security.AccessControlContext.checkPermission(AccessControlContext.java:472)
java.security.AccessController.checkPermission(AccessController.java:884)
java.lang.SecurityManager.checkPermission(SecurityManager.java:549)
com.ibm.ws.kernel.launch.internal.MissingDoPrivDetectionSecurityManager.checkPermission(MissingDoPrivDetectionSecurityManager.java:45)
java.lang.SecurityManager.checkPropertiesAccess(SecurityManager.java:1262)
java.lang.System.getProperties(System.java:630)
javax.cache.Caching$CachingProviderRegistry.getCachingProviders(Caching.java:436)
javax.cache.Caching$CachingProviderRegistry.getCachingProvider(Caching.java:380)
javax.cache.Caching$CachingProviderRegistry.getCachingProvider(Caching.java:361)
javax.cache.Caching.getCachingProvider(Caching.java:151)
javax.cache.Caching.getCache(Caching.java:289)
session.cache.web.SessionCacheTestServlet.testSessionPropertyCache(SessionCacheTestServlet.java:332)
gregrluck commented 6 years ago

Can you give me your Java version, OS etc. I have not encountered this issue and I don't think anyone else on the EG did either.

aguibert commented 6 years ago

Hi Greg, I'm using Oracle JDK (running on MacOS). Here is the full version info:

java version "1.8.0_151"
Java(TM) SE Runtime Environment (build 1.8.0_151-b12)
Java HotSpot(TM) 64-Bit Server VM (build 25.151-b12, mixed mode)

According to javadoc System.getProperties() can throw a SecurityException.

gregrluck commented 6 years ago

What OS?

aguibert commented 6 years ago

Running on MacOS, I don't think OS matters here though. I have a simple fix proposed in PR #399 by the way.

tristantarrant commented 6 years ago

@aguibert you should include your SecurityManager configuration (i.e. your policy file)

cruftex commented 6 years ago

The PR from @aguibert looks good to me. We should not use System.getProperties().

Its an obvious fix in the code that ships in the API jar and no change in the spec itself. Maybe we can fix that in the jar without the MR procedure?

aguibert commented 6 years ago

@tristantarrant here is my policy file (from $JAVA_HOME/jre/lib/security/java.policy). It is just the out-of-the-box policy file and I have not modified it at all. As you can see, it only permits reading the builtin java.* system properties.

// Standard extensions get all permissions by default

grant codeBase "file:${{java.ext.dirs}}/*" {
        permission java.security.AllPermission;
};

// default permissions granted to all domains

grant {
        // Allows any thread to stop itself using the java.lang.Thread.stop()
        // method that takes no argument.
        // Note that this permission is granted by default only to remain
        // backwards compatible.
        // It is strongly recommended that you either remove this permission
        // from this policy file or further restrict it to code sources
        // that you specify, because Thread.stop() is potentially unsafe.
        // See the API specification of java.lang.Thread.stop() for more
        // information.
        permission java.lang.RuntimePermission "stopThread";

        // allows anyone to listen on dynamic ports
        permission java.net.SocketPermission "localhost:0", "listen";

        // "standard" properies that can be read by anyone

        permission java.util.PropertyPermission "java.version", "read";
        permission java.util.PropertyPermission "java.vendor", "read";
        permission java.util.PropertyPermission "java.vendor.url", "read";
        permission java.util.PropertyPermission "java.class.version", "read";
        permission java.util.PropertyPermission "os.name", "read";
        permission java.util.PropertyPermission "os.version", "read";
        permission java.util.PropertyPermission "os.arch", "read";
        permission java.util.PropertyPermission "file.separator", "read";
        permission java.util.PropertyPermission "path.separator", "read";
        permission java.util.PropertyPermission "line.separator", "read";

        permission java.util.PropertyPermission "java.specification.version", "read";
        permission java.util.PropertyPermission "java.specification.vendor", "read";
        permission java.util.PropertyPermission "java.specification.name", "read";

        permission java.util.PropertyPermission "java.vm.specification.version", "read";
        permission java.util.PropertyPermission "java.vm.specification.vendor", "read";
        permission java.util.PropertyPermission "java.vm.specification.name", "read";
        permission java.util.PropertyPermission "java.vm.version", "read";
        permission java.util.PropertyPermission "java.vm.vendor", "read";
        permission java.util.PropertyPermission "java.vm.name", "read";
};

@cruftex It's a binary compatible change so typically bumping the micro version would be sufficient (e.g. creating version 1.1.1). Maven Central doesn't allow overwriting existing artifacts though (which is a good thing).

vbekiaris commented 6 years ago

Running with security manager enabled and the default security policy gives the following results:

I think this is a clear improvement, +1 for the PR.

aguibert commented 6 years ago

I had to cancel my PR #399 because I am not allowed to sign the OCA at this time. If someone else could contribute this simple change on my behalf, it would be much appreciated.

vbekiaris commented 6 years ago

@aguibert thanks for raising this issue, I prepared PR #400 (substituting #399)

gregrluck commented 6 years ago

To reproduce this issue, turn on the SecurityManager. It is not turned on by default.

-Djava.security.manager=default

See https://www.ibm.com/support/knowledgecenter/en/SSGMCP_4.1.0/com.ibm.cics.ts.java.doc/topics/dfhpj5u.html.

aguibert commented 6 years ago

thanks for the quick resolution everyone!