8001319: Add SecurityPermission "insertProvider" target name

Reviewed-by: vinnie

jdk/src/share/classes/java/security/Security.java

@@ -326,17 +326,13 @@ public final class Security {
      *
      * <p>A provider cannot be added if it is already installed.
      *
-     * <p>First, if there is a security manager, its
-     * {@code checkSecurityAccess}
-     * method is called with the string
-     * {@code "insertProvider."+provider.getName()}
-     * to see if it's ok to add a new provider.
-     * If the default implementation of {@code checkSecurityAccess}
-     * is used (i.e., that method is not overriden), then this will result in
-     * a call to the security manager's {@code checkPermission} method
-     * with a
-     * {@code SecurityPermission("insertProvider."+provider.getName())}
-     * permission.
+     * <p>If there is a security manager, the
+     * {@link java.lang.SecurityManager#checkSecurityAccess} method is called
+     * with the {@code "insertProvider"} permission target name to see if
+     * it's ok to add a new provider. If this permission check is denied,
+     * {@code checkSecurityAccess} is called again with the
+     * {@code "insertProvider."+provider.getName()} permission target name. If
+     * both checks are denied, a {@code SecurityException} is thrown.
      *
      * @param provider the provider to be added.
      *
@@ -360,7 +356,7 @@ public final class Security {
     public static synchronized int insertProviderAt(Provider provider,
             int position) {
         String providerName = provider.getName();
-        check("insertProvider." + providerName);
+        checkInsertProvider(providerName);
         ProviderList list = Providers.getFullProviderList();
         ProviderList newList = ProviderList.insertAt(list, provider, position - 1);
         if (list == newList) {
@@ -373,17 +369,13 @@ public final class Security {
     /**
      * Adds a provider to the next position available.
      *
-     * <p>First, if there is a security manager, its
-     * {@code checkSecurityAccess}
-     * method is called with the string
-     * {@code "insertProvider."+provider.getName()}
-     * to see if it's ok to add a new provider.
-     * If the default implementation of {@code checkSecurityAccess}
-     * is used (i.e., that method is not overriden), then this will result in
-     * a call to the security manager's {@code checkPermission} method
-     * with a
-     * {@code SecurityPermission("insertProvider."+provider.getName())}
-     * permission.
+     * <p>If there is a security manager, the
+     * {@link java.lang.SecurityManager#checkSecurityAccess} method is called
+     * with the {@code "insertProvider"} permission target name to see if
+     * it's ok to add a new provider. If this permission check is denied,
+     * {@code checkSecurityAccess} is called again with the
+     * {@code "insertProvider."+provider.getName()} permission target name. If
+     * both checks are denied, a {@code SecurityException} is thrown.
      *
      * @param provider the provider to be added.
      *
@@ -863,6 +855,23 @@ public final class Security {
         }
     }
 
+    private static void checkInsertProvider(String name) {
+        SecurityManager security = System.getSecurityManager();
+        if (security != null) {
+            try {
+                security.checkSecurityAccess("insertProvider");
+            } catch (SecurityException se1) {
+                try {
+                    security.checkSecurityAccess("insertProvider." + name);
+                } catch (SecurityException se2) {
+                    // throw first exception, but add second to suppressed
+                    se1.addSuppressed(se2);
+                    throw se1;
+                }
+            }
+        }
+    }
+
     /*
     * Returns all providers who satisfy the specified
     * criterion.

jdk/src/share/classes/java/security/SecurityPermission.java

@@ -130,14 +130,17 @@ import java.util.StringTokenizer;
  * </tr>
  *
  * <tr>
- *   <td>insertProvider.{provider name}</td>
- *   <td>Addition of a new provider, with the specified name</td>
+ *   <td>insertProvider</td>
+ *   <td>Addition of a new provider</td>
  *   <td>This would allow somebody to introduce a possibly
  * malicious provider (e.g., one that discloses the private keys passed
  * to it) as the highest-priority provider. This would be possible
  * because the Security object (which manages the installed providers)
  * currently does not check the integrity or authenticity of a provider
- * before attaching it.</td>
+ * before attaching it. The "insertProvider" permission subsumes the
+ * "insertProvider.{provider name}" permission (see the section below for
+ * more information).
+ * </td>
  * </tr>
  *
  * <tr>
@@ -186,9 +189,10 @@ import java.util.StringTokenizer;
  * </table>
  *
  * <P>
- * The following permissions are associated with classes that have been
- * deprecated: {@link Identity}, {@link IdentityScope}, {@link Signer}. Use of
- * them is discouraged. See the applicable classes for more information.
+ * The following permissions have been superseded by newer permissions or are
+ * associated with classes that have been deprecated: {@link Identity},
+ * {@link IdentityScope}, {@link Signer}. Use of them is discouraged. See the
+ * applicable classes for more information.
  * <P>
  *
  * <table border=1 cellpadding=5 summary="target name,what the permission allows, and associated risks">
@@ -199,6 +203,23 @@ import java.util.StringTokenizer;
  * </tr>
  *
  * <tr>
+ *   <td>insertProvider.{provider name}</td>
+ *   <td>Addition of a new provider, with the specified name</td>
+ *   <td>Use of this permission is discouraged from further use because it is
+ * possible to circumvent the name restrictions by overriding the
+ * {@link java.security.Provider#getName} method. Also, there is an equivalent
+ * level of risk associated with granting code permission to insert a provider
+ * with a specific name, or any name it chooses. Users should use the
+ * "insertProvider" permission instead.
+ * <p>This would allow somebody to introduce a possibly
+ * malicious provider (e.g., one that discloses the private keys passed
+ * to it) as the highest-priority provider. This would be possible
+ * because the Security object (which manages the installed providers)
+ * currently does not check the integrity or authenticity of a provider
+ * before attaching it.</td>
+ * </tr>
+ *
+ * <tr>
  *   <td>setSystemScope</td>
  *   <td>Setting of the system identity scope</td>
  *   <td>This would allow an attacker to configure the system identity scope with
@@ -306,7 +327,6 @@ public final class SecurityPermission extends BasicPermission {
      * @throws NullPointerException if {@code name} is {@code null}.
      * @throws IllegalArgumentException if {@code name} is empty.
      */
-
     public SecurityPermission(String name)
     {
         super(name);
@@ -323,7 +343,6 @@ public final class SecurityPermission extends BasicPermission {
      * @throws NullPointerException if {@code name} is {@code null}.
      * @throws IllegalArgumentException if {@code name} is empty.
      */
-
     public SecurityPermission(String name, String actions)
     {
         super(name, actions);

jdk/test/java/security/Security/AddProvider.java

@@ -0,0 +1,59 @@
+/*
+ * Copyright (c) 2013, 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.
+ */
+
+/*
+ * @test
+ * @bug 8001319
+ * @summary check that SecurityPermission insertProvider permission is enforced
+ *          correctly
+ * @run main/othervm/policy=AddProvider.policy.1 AddProvider 1
+ * @run main/othervm/policy=AddProvider.policy.2 AddProvider 2
+ * @run main/othervm/policy=AddProvider.policy.3 AddProvider 3
+ */
+import java.security.Provider;
+import java.security.Security;
+
+public class AddProvider {
+
+    public static void main(String[] args) throws Exception {
+        boolean legacy = args[0].equals("2");
+        Security.addProvider(new TestProvider("Test1"));
+        Security.insertProviderAt(new TestProvider("Test2"), 1);
+        try {
+            Security.addProvider(new TestProvider("Test3"));
+            if (legacy) {
+                throw new Exception("Expected SecurityException");
+            }
+        } catch (SecurityException se) {
+            if (!legacy) {
+                throw se;
+            }
+        }
+    }
+
+    private static class TestProvider extends Provider {
+        TestProvider(String name) {
+            super(name, 0.0, "Not for use in production systems!");
+        }
+    }
+}

jdk/test/java/security/Security/AddProvider.policy.1

@@ -0,0 +1,7 @@
+grant codeBase "file:${{java.ext.dirs}}/*" {
+	permission java.security.AllPermission;
+};
+
+grant {
+    permission java.security.SecurityPermission "insertProvider";
+};

jdk/test/java/security/Security/AddProvider.policy.2

@@ -0,0 +1,8 @@
+grant codeBase "file:${{java.ext.dirs}}/*" {
+	permission java.security.AllPermission;
+};
+
+grant {
+    permission java.security.SecurityPermission "insertProvider.Test1";
+    permission java.security.SecurityPermission "insertProvider.Test2";
+};

jdk/test/java/security/Security/AddProvider.policy.3

@@ -0,0 +1,7 @@
+grant codeBase "file:${{java.ext.dirs}}/*" {
+	permission java.security.AllPermission;
+};
+
+grant {
+    permission java.security.SecurityPermission "insertProvider.*";
+};