6826789: SecureClassLoader should not use CodeSource URLs as HashMap keys

Reviewed-by: weijun

jdk/src/java.base/share/classes/java/security/CodeSource.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1997, 2013, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1997, 2015, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -34,6 +34,7 @@ import java.util.Hashtable;
 import java.io.ByteArrayInputStream;
 import java.io.IOException;
 import java.security.cert.*;
+import sun.net.util.URLUtil;
 
 /**
  *
@@ -72,6 +73,15 @@ public class CodeSource implements java.io.Serializable {
     // for generating cert paths
     private transient CertificateFactory factory = null;
 
+    /**
+     * A String form of the URL for use as a key in HashMaps/Sets. The String
+     * form should be behave in the same manner as the URL when compared for
+     * equality in a HashMap/Set, except that no nameservice lookup is done
+     * on the hostname (only string comparison), and the fragment is not
+     * considered.
+     */
+    private transient String locationNoFragString;
+
     /**
      * Constructs a CodeSource and associates it with the specified
      * location and set of certificates.
@@ -83,6 +93,9 @@ public class CodeSource implements java.io.Serializable {
      */
     public CodeSource(URL url, java.security.cert.Certificate certs[]) {
         this.location = url;
+        if (url != null) {
+            this.locationNoFragString = URLUtil.urlNoFragString(url);
+        }
 
         // Copy the supplied certs
         if (certs != null) {
@@ -102,6 +115,9 @@ public class CodeSource implements java.io.Serializable {
      */
     public CodeSource(URL url, CodeSigner[] signers) {
         this.location = url;
+        if (url != null) {
+            this.locationNoFragString = URLUtil.urlNoFragString(url);
+        }
 
         // Copy the supplied signers
         if (signers != null) {
@@ -168,6 +184,13 @@ public class CodeSource implements java.io.Serializable {
         return this.location;
     }
 
+    /**
+     * Returns a String form of the URL for use as a key in HashMaps/Sets.
+     */
+    String getLocationNoFragString() {
+        return locationNoFragString;
+    }
+
     /**
      * Returns the certificates associated with this CodeSource.
      * <p>
@@ -588,6 +611,10 @@ public class CodeSource implements java.io.Serializable {
         } catch (IOException ioe) {
             // no signers present
         }
+
+        if (location != null) {
+            locationNoFragString = URLUtil.urlNoFragString(location);
+        }
     }
 
     /*

jdk/src/java.base/share/classes/java/security/SecureClassLoader.java

@@ -47,10 +47,16 @@ public class SecureClassLoader extends ClassLoader {
      */
     private final boolean initialized;
 
-    // HashMap that maps CodeSource to ProtectionDomain
+    /*
-    // @GuardedBy("pdcache")
+     * HashMap that maps the CodeSource URL (as a String) to ProtectionDomain.
-    private final HashMap<CodeSource, ProtectionDomain> pdcache =
+     * We use a String instead of a CodeSource/URL as the key to avoid
-                        new HashMap<>(11);
+     * potential expensive name service lookups. This does mean that URLs that
+     * are equivalent after nameservice lookup will be placed in separate
+     * ProtectionDomains; however during policy enforcement these URLs will be
+     * canonicalized and resolved resulting in a consistent set of granted
+     * permissions.
+     */
+    private final HashMap<String, ProtectionDomain> pdcache = new HashMap<>(11);
 
     private static final Debug debug = Debug.getInstance("scl");
 
@@ -196,16 +202,22 @@ public class SecureClassLoader extends ClassLoader {
      * Returned cached ProtectionDomain for the specified CodeSource.
      */
     private ProtectionDomain getProtectionDomain(CodeSource cs) {
-        if (cs == null)
+        if (cs == null) {
             return null;
+        }
 
         ProtectionDomain pd = null;
         synchronized (pdcache) {
-            pd = pdcache.get(cs);
+            // Use a String form of the URL as the key. It should behave in the
+            // same manner as the URL when compared for equality except that no
+            // nameservice lookup is done on the hostname (String comparison
+            // only), and the fragment is not considered.
+            String key = cs.getLocationNoFragString();
+            pd = pdcache.get(key);
             if (pd == null) {
                 PermissionCollection perms = getPermissions(cs);
                 pd = new ProtectionDomain(cs, perms, this, null);
-                pdcache.put(cs, pd);
+                pdcache.put(key, pd);
                 if (debug != null) {
                     debug.println(" getPermissions "+ pd);
                     debug.println("");

jdk/test/java/security/SecureClassLoader/DefineClass.java

@@ -0,0 +1,114 @@
+/*
+ * Copyright (c) 2015, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+import java.io.IOException;
+import java.net.URL;
+import java.security.CodeSource;
+import java.security.Permission;
+import java.security.Policy;
+import java.security.ProtectionDomain;
+import java.security.SecureClassLoader;
+import java.security.cert.Certificate;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Base64;
+import java.util.Collections;
+import java.util.List;
+import java.util.PropertyPermission;
+
+/*
+ * @test
+ * @bug 6826789
+ * @summary Make sure equivalent ProtectionDomains are granted the same
+ *          permissions when the CodeSource URLs are different but resolve
+ *          to the same ip address after name service resolution.
+ * @run main/othervm/java.security.policy=DefineClass.policy DefineClass
+ */
+
+public class DefineClass {
+
+    // permissions that are expected to be granted by the policy file
+    private final static Permission[] GRANTED_PERMS = new Permission[] {
+        new PropertyPermission("user.home", "read"),
+        new PropertyPermission("user.name", "read")
+    };
+
+    // Base64 encoded bytes of a simple class: "public class Foo {}"
+    private final static String FOO_CLASS =
+        "yv66vgAAADQADQoAAwAKBwALBwAMAQAGPGluaXQ+AQADKClWAQAEQ29kZQEA" +
+        "D0xpbmVOdW1iZXJUYWJsZQEAClNvdXJjZUZpbGUBAAhGb28uamF2YQwABAAF" +
+        "AQADRm9vAQAQamF2YS9sYW5nL09iamVjdAAhAAIAAwAAAAAAAQABAAQABQAB" +
+        "AAYAAAAdAAEAAQAAAAUqtwABsQAAAAEABwAAAAYAAQAAAAEAAQAIAAAAAgAJ";
+
+    // Base64 encoded bytes of a simple class: "public class Bar {}"
+    private final static String BAR_CLASS =
+        "yv66vgAAADQADQoAAwAKBwALBwAMAQAGPGluaXQ+AQADKClWAQAEQ29kZQEA" +
+        "D0xpbmVOdW1iZXJUYWJsZQEAClNvdXJjZUZpbGUBAAhCYXIuamF2YQwABAAF" +
+        "AQADQmFyAQAQamF2YS9sYW5nL09iamVjdAAhAAIAAwAAAAAAAQABAAQABQAB" +
+        "AAYAAAAdAAEAAQAAAAUqtwABsQAAAAEABwAAAAYAAQAAAAEAAQAIAAAAAgAJ";
+
+    public static void main(String[] args) throws Exception {
+
+        MySecureClassLoader scl = new MySecureClassLoader();
+        Policy p = Policy.getPolicy();
+        ArrayList<Permission> perms1 = getPermissions(scl, p,
+                                                      "http://localhost/",
+                                                      "Foo", FOO_CLASS);
+        checkPerms(perms1, GRANTED_PERMS);
+        ArrayList<Permission> perms2 = getPermissions(scl, p,
+                                                      "http://127.0.0.1/",
+                                                      "Bar", BAR_CLASS);
+        checkPerms(perms2, GRANTED_PERMS);
+        assert(perms1.equals(perms2));
+    }
+
+    // returns the permissions granted to the codebase URL
+    private static ArrayList<Permission> getPermissions(MySecureClassLoader scl,
+                                                        Policy p, String url,
+                                                        String className,
+                                                        String classBytes)
+                                                        throws IOException {
+        CodeSource cs = new CodeSource(new URL(url), (Certificate[])null);
+        Base64.Decoder bd = Base64.getDecoder();
+        byte[] bytes = bd.decode(classBytes);
+        Class<?> c = scl.defineMyClass(className, bytes, cs);
+        ProtectionDomain pd = c.getProtectionDomain();
+        return Collections.list(p.getPermissions(pd).elements());
+    }
+
+    private static void checkPerms(List<Permission> perms,
+                                   Permission... grantedPerms)
+        throws Exception
+    {
+        if (!perms.containsAll(Arrays.asList(grantedPerms))) {
+            throw new Exception("Granted permissions not correct");
+        }
+    }
+
+    // A SecureClassLoader that allows the test to define its own classes
+    private static class MySecureClassLoader extends SecureClassLoader {
+        Class<?> defineMyClass(String name, byte[] b, CodeSource cs) {
+            return super.defineClass(name, b, 0, b.length, cs);
+        }
+    }
+}

jdk/test/java/security/SecureClassLoader/DefineClass.policy

@@ -0,0 +1,11 @@
+grant {
+    permission java.lang.RuntimePermission "createClassLoader";
+    permission java.lang.RuntimePermission "getProtectionDomain";
+    permission java.security.SecurityPermission "getPolicy";
+};
+grant codebase "http://localhost/" {
+    permission java.util.PropertyPermission "user.home", "read";
+};
+grant codebase "http://127.0.0.1/" {
+    permission java.util.PropertyPermission "user.name", "read";
+};