From cf58b9d8a74dfb517da77dd997334bbb0c814de1 Mon Sep 17 00:00:00 2001 From: Roland Praml Date: Wed, 29 Sep 2021 08:39:22 +0200 Subject: [PATCH] Classcache must honor current security context. (#1019) With this PR, the cache takes the current security context (in detail, the java.security.Permissions) into account when caching classes. Co-authored-by: Roland Praml Co-authored-by: Roland Praml --- src/org/mozilla/javascript/ClassCache.java | 37 ++++- src/org/mozilla/javascript/JavaMembers.java | 34 ++++- .../com/example/securitytest/SomeFactory.java | 27 ++++ .../example/securitytest/SomeInterface.java | 18 +++ .../example/securitytest/impl/SomeClass.java | 33 +++++ .../tests/SecurityControllerTest.java | 126 ++++++++++++++++++ .../javascript/tests/grant-all-java.policy | 5 + .../javascript/tests/javascript.policy | 20 +++ 8 files changed, 292 insertions(+), 8 deletions(-) create mode 100644 testsrc/com/example/securitytest/SomeFactory.java create mode 100644 testsrc/com/example/securitytest/SomeInterface.java create mode 100644 testsrc/com/example/securitytest/impl/SomeClass.java create mode 100644 testsrc/org/mozilla/javascript/tests/SecurityControllerTest.java create mode 100644 testsrc/org/mozilla/javascript/tests/grant-all-java.policy create mode 100644 testsrc/org/mozilla/javascript/tests/javascript.policy diff --git a/src/org/mozilla/javascript/ClassCache.java b/src/org/mozilla/javascript/ClassCache.java index 11abba8a6..19672cde4 100644 --- a/src/org/mozilla/javascript/ClassCache.java +++ b/src/org/mozilla/javascript/ClassCache.java @@ -8,6 +8,7 @@ package org.mozilla.javascript; import java.io.Serializable; import java.util.Map; +import java.util.Objects; import java.util.concurrent.ConcurrentHashMap; /** @@ -21,12 +22,42 @@ public class ClassCache implements Serializable { private static final long serialVersionUID = -8866246036237312215L; private static final Object AKEY = "ClassCache"; private volatile boolean cachingIsEnabled = true; - private transient Map, JavaMembers> classTable; + private transient Map classTable; private transient Map> classAdapterCache; private transient Map, Object> interfaceAdapterCache; private int generatedClassSerial; private Scriptable associatedScope; + /** + * CacheKey is a combination of class and securityContext. This is required when classes are + * loaded from different security contexts + */ + static class CacheKey { + final Class cls; + final Object sec; + /** Constructor. */ + public CacheKey(Class cls, Object securityContext) { + this.cls = cls; + this.sec = securityContext; + } + + @Override + public int hashCode() { + int result = cls.hashCode(); + if (sec != null) { + result = sec.hashCode() * 31; + } + return result; + } + + @Override + public boolean equals(Object obj) { + return (obj instanceof CacheKey) + && Objects.equals(this.cls, ((CacheKey) obj).cls) + && Objects.equals(this.sec, ((CacheKey) obj).sec); + } + } + /** * Search for ClassCache object in the given scope. The method first calls {@link * ScriptableObject#getTopLevelScope(Scriptable scope)} to get the top most scope and then tries @@ -101,11 +132,11 @@ public class ClassCache implements Serializable { } /** @return a map from classes to associated JavaMembers objects */ - Map, JavaMembers> getClassCacheMap() { + Map getClassCacheMap() { if (classTable == null) { // Use 1 as concurrency level here and for other concurrent hash maps // as we don't expect high levels of sustained concurrent writes. - classTable = new ConcurrentHashMap, JavaMembers>(16, 0.75f, 1); + classTable = new ConcurrentHashMap(16, 0.75f, 1); } return classTable; } diff --git a/src/org/mozilla/javascript/JavaMembers.java b/src/org/mozilla/javascript/JavaMembers.java index e9e564e71..3486293bd 100644 --- a/src/org/mozilla/javascript/JavaMembers.java +++ b/src/org/mozilla/javascript/JavaMembers.java @@ -15,6 +15,9 @@ import java.lang.reflect.Field; import java.lang.reflect.Member; import java.lang.reflect.Method; import java.lang.reflect.Modifier; +import java.security.AccessControlContext; +import java.security.AllPermission; +import java.security.Permission; import java.util.ArrayList; import java.util.Arrays; import java.util.HashMap; @@ -28,6 +31,8 @@ import java.util.Map; * @see NativeJavaClass */ class JavaMembers { + private static final Permission allPermission = new AllPermission(); + JavaMembers(Scriptable scope, Class cl) { this(scope, cl, false); } @@ -749,16 +754,17 @@ class JavaMembers { Scriptable scope, Class dynamicType, Class staticType, boolean includeProtected) { JavaMembers members; ClassCache cache = ClassCache.get(scope); - Map, JavaMembers> ct = cache.getClassCacheMap(); + Map ct = cache.getClassCacheMap(); Class cl = dynamicType; + Object secCtx = getSecurityContext(); for (; ; ) { - members = ct.get(cl); + members = ct.get(new ClassCache.CacheKey(cl, secCtx)); if (members != null) { if (cl != dynamicType) { // member lookup for the original class failed because of // missing privileges, cache the result so we don't try again - ct.put(dynamicType, members); + ct.put(new ClassCache.CacheKey(dynamicType, secCtx), members); } return members; } @@ -789,16 +795,34 @@ class JavaMembers { } if (cache.isCachingEnabled()) { - ct.put(cl, members); + ct.put(new ClassCache.CacheKey(cl, secCtx), members); if (cl != dynamicType) { // member lookup for the original class failed because of // missing privileges, cache the result so we don't try again - ct.put(dynamicType, members); + ct.put(new ClassCache.CacheKey(dynamicType, secCtx), members); } } return members; } + private static Object getSecurityContext() { + Object sec = null; + SecurityManager sm = System.getSecurityManager(); + if (sm != null) { + sec = sm.getSecurityContext(); + if (sec instanceof AccessControlContext) { + try { + ((AccessControlContext) sec).checkPermission(allPermission); + // if we have allPermission, we do not need to store the + // security object in the cache key + return null; + } catch (SecurityException e) { + } + } + } + return sec; + } + RuntimeException reportMemberNotFound(String memberName) { return Context.reportRuntimeErrorById( "msg.java.member.not.found", cl.getName(), memberName); diff --git a/testsrc/com/example/securitytest/SomeFactory.java b/testsrc/com/example/securitytest/SomeFactory.java new file mode 100644 index 000000000..2cbb01fe1 --- /dev/null +++ b/testsrc/com/example/securitytest/SomeFactory.java @@ -0,0 +1,27 @@ +/* -*- Mode: java; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 4 -*- + * + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. + */ + +package com.example.securitytest; + +/** + * Class for SecurityControllerTest. + * + * @author Roland Praml, FOCONIS AG + */ +public class SomeFactory { + + public static int TEST = 42; + + public SomeInterface create() { + try { + return (SomeInterface) + Class.forName("com.example.securitytest.impl.SomeClass").newInstance(); + } catch (InstantiationException | IllegalAccessException | ClassNotFoundException e) { + throw new RuntimeException("Could not create impl", e); + } + } +} diff --git a/testsrc/com/example/securitytest/SomeInterface.java b/testsrc/com/example/securitytest/SomeInterface.java new file mode 100644 index 000000000..f01a7ae27 --- /dev/null +++ b/testsrc/com/example/securitytest/SomeInterface.java @@ -0,0 +1,18 @@ +/* -*- Mode: java; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 4 -*- + * + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. + */ + +package com.example.securitytest; + +/** + * Class for SecurityControllerTest. + * + * @author Roland Praml, FOCONIS AG + */ +public interface SomeInterface { + + String foo(); +} diff --git a/testsrc/com/example/securitytest/impl/SomeClass.java b/testsrc/com/example/securitytest/impl/SomeClass.java new file mode 100644 index 000000000..50c48a4db --- /dev/null +++ b/testsrc/com/example/securitytest/impl/SomeClass.java @@ -0,0 +1,33 @@ +/* -*- Mode: java; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 4 -*- + * + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. + */ + +package com.example.securitytest.impl; + +import com.example.securitytest.SomeInterface; +import java.util.ArrayList; + +/** + * Provides an implementation for SomeInterface. Defines two methods: foo overridden + * (defined by interface) and bar defined at this class. + * + *

If this class is excluded by the shutter, the method bar should not be accessible + * in scripts. + * + * @author Roland Praml, FOCONIS AG + */ +public class SomeClass extends ArrayList implements SomeInterface { + private static final long serialVersionUID = 1L; + + @Override + public String foo() { + return "FOO"; + } + + public String bar() { + return "BAR"; + } +} diff --git a/testsrc/org/mozilla/javascript/tests/SecurityControllerTest.java b/testsrc/org/mozilla/javascript/tests/SecurityControllerTest.java new file mode 100644 index 000000000..3d143e13d --- /dev/null +++ b/testsrc/org/mozilla/javascript/tests/SecurityControllerTest.java @@ -0,0 +1,126 @@ +package org.mozilla.javascript.tests; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.fail; + +import java.io.File; +import java.net.MalformedURLException; +import java.net.URL; +import java.security.CodeSource; +import java.security.Permission; +import java.security.Permissions; +import java.security.Policy; +import java.security.ProtectionDomain; +import java.security.URIParameter; +import java.util.Enumeration; +import org.junit.BeforeClass; +import org.junit.Test; +import org.mozilla.javascript.ClassShutter; +import org.mozilla.javascript.EcmaError; +import org.mozilla.javascript.Scriptable; +import org.mozilla.javascript.SecurityController; +import org.mozilla.javascript.tools.shell.Global; +import org.mozilla.javascript.tools.shell.JavaPolicySecurity; + +/** Perform some tests when we have a securityController in place. */ +public class SecurityControllerTest { + + private static ProtectionDomain UNTRUSTED_JAVASCRIPT; + private static ProtectionDomain ALLOW_IMPL_ACCESS; + private static ProtectionDomain RESTRICT_IMPL_ACCESS; + protected final Global global = new Global(); + + /** Sets up the security manager and loads the "grant-all-java.policy". */ + static void setupSecurityManager() {} + /** Setup the security */ + @BeforeClass + public static void setup() throws Exception { + URL url = SecurityControllerTest.class.getResource("grant-all-java.policy"); + if (url != null) { + System.setProperty("java.security.policy", url.toString()); + Policy.getPolicy().refresh(); + System.setSecurityManager(new SecurityManager()); + } + SecurityController.initGlobal(new JavaPolicySecurity()); + + url = SecurityControllerTest.class.getResource("javascript.policy"); + Policy policy = Policy.getInstance("JavaPolicy", new URIParameter(url.toURI())); + RESTRICT_IMPL_ACCESS = createProtectionDomain(policy, "RESTRICT_IMPL_ACCESS"); + ALLOW_IMPL_ACCESS = createProtectionDomain(policy, "ALLOW_IMPL_ACCESS"); + } + + /** Creates a new protectionDomain with the given Code-Source Suffix. */ + private static ProtectionDomain createProtectionDomain(Policy policy, String csSuffix) + throws MalformedURLException { + File file = new File(System.getProperty("user.dir")); + file = new File(file, "javascript"); + file = new File(file, csSuffix); + URL url = file.toURI().toURL(); + CodeSource cs = new CodeSource(url, (java.security.cert.Certificate[]) null); + Permissions perms = new Permissions(); + Enumeration elems = policy.getPermissions(cs).elements(); + while (elems.hasMoreElements()) { + perms.add(elems.nextElement()); + } + perms.setReadOnly(); + return new ProtectionDomain(cs, perms, null, null); + } + + @Test + public void testBarAccess() { + // f.create produces "SomeClass extends ArrayList implements + // SomeInterface" + // we may access array methods, like 'size' defined by ArrayList, + // but not methods like 'bar' defined by SomeClass, because it is in a restricted package + String script = + "f = new com.example.securitytest.SomeFactory();\n" + + "var i = f.create();\n" + + "i.size();\n" + + "i.bar();"; + + // try in allowed scope + runScript(script, ALLOW_IMPL_ACCESS); + + try { + // in restricted scope, we expect an EcmaError + runScript(script, RESTRICT_IMPL_ACCESS); + fail("EcmaError expected"); + } catch (EcmaError ee) { + assertEquals("TypeError: Cannot find function bar in object []. (#4)", ee.getMessage()); + } + + // try in allowed scope again + runScript(script, ALLOW_IMPL_ACCESS); + } + + /** + * This classShutter checks the "rhino.visible.{pkg}" runtime property, which can be defined in + * a policy file. Note: Every other code in your stack-chain will need this permission also. + */ + private static class PolicyClassShutter implements ClassShutter { + + @Override + public boolean visibleToScripts(String fullClassName) { + SecurityManager sm = System.getSecurityManager(); + if (sm != null) { + int idx = fullClassName.lastIndexOf('.'); + if (idx != -1) { + String pkg = fullClassName.substring(0, idx); + sm.checkPermission(new RuntimePermission("rhino.visible." + pkg)); + } + } + return true; + } + } + + /** Compiles and runs the script with the given protection domain. */ + private void runScript(String scriptSourceText, ProtectionDomain pd) { + Utils.runWithAllOptimizationLevels( + context -> { + context.setClassShutter(new PolicyClassShutter()); + Scriptable scope = context.initStandardObjects(global); + + return context.evaluateString(scope, scriptSourceText, "", 1, pd); + }); + } +} diff --git a/testsrc/org/mozilla/javascript/tests/grant-all-java.policy b/testsrc/org/mozilla/javascript/tests/grant-all-java.policy new file mode 100644 index 000000000..c755fd786 --- /dev/null +++ b/testsrc/org/mozilla/javascript/tests/grant-all-java.policy @@ -0,0 +1,5 @@ +// Grant everyone the following permission: (required for SecurityControllerTest) +grant { + // permission all; + permission java.security.AllPermission "", ""; +}; diff --git a/testsrc/org/mozilla/javascript/tests/javascript.policy b/testsrc/org/mozilla/javascript/tests/javascript.policy new file mode 100644 index 000000000..cf21b4a42 --- /dev/null +++ b/testsrc/org/mozilla/javascript/tests/javascript.policy @@ -0,0 +1,20 @@ +// Sample script, how to define different security codebases for javaScript. + +grant codebase "file:${user.dir}/javascript/ALLOW_IMPL_ACCESS" { + permission java.lang.RuntimePermission "rhino.visible.com"; + permission java.lang.RuntimePermission "rhino.visible.com.example"; + permission java.lang.RuntimePermission "rhino.visible.com.example.securitytest"; + permission java.lang.RuntimePermission "rhino.visible.com.example.securitytest.*"; +}; + +grant codebase "file:${user.dir}/javascript/RESTRICT_IMPL_ACCESS" { + permission java.lang.RuntimePermission "rhino.visible.com"; + permission java.lang.RuntimePermission "rhino.visible.com.example"; + permission java.lang.RuntimePermission "rhino.visible.com.example.securitytest"; +}; + +grant { + // grant every script access to java.lang and java.util (but not to java.util.*) + permission java.lang.RuntimePermission "rhino.visible.java"; + permission java.lang.RuntimePermission "rhino.visible.java.*"; +};