diff --git a/src/share/classes/java/security/Security.java b/src/share/classes/java/security/Security.java index 98699da8149c4e1e3eeadf91553c3071d51d875a..ce99101e71669068b52ac5a07558893a5f8f1039 100644 --- a/src/share/classes/java/security/Security.java +++ b/src/share/classes/java/security/Security.java @@ -326,17 +326,13 @@ public final class Security { * *
A provider cannot be added if it is already installed. * - *
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. + *
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. * - *
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. + *
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. diff --git a/src/share/classes/java/security/SecurityPermission.java b/src/share/classes/java/security/SecurityPermission.java index e0f0f92b40c49d7f7222ccdc4aafd4bf4cd2054a..bbdccaeffe3e40bddbb0aabbeb46aa669d0f8e87 100644 --- a/src/share/classes/java/security/SecurityPermission.java +++ b/src/share/classes/java/security/SecurityPermission.java @@ -130,14 +130,17 @@ import java.util.StringTokenizer; * * *
- * 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. *
* *
insertProvider.{provider name} | + *Addition of a new provider, with the specified name | + *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.
+ * 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. |
+ *
setSystemScope | *Setting of the system identity scope | *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); diff --git a/test/java/security/Security/AddProvider.java b/test/java/security/Security/AddProvider.java new file mode 100644 index 0000000000000000000000000000000000000000..27559ffdbc8fe5897e424a4b8619a9cea9de5337 --- /dev/null +++ b/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!"); + } + } +} diff --git a/test/java/security/Security/AddProvider.policy.1 b/test/java/security/Security/AddProvider.policy.1 new file mode 100644 index 0000000000000000000000000000000000000000..17a49b47b77fb86734aa079a9ae856b9aea039b4 --- /dev/null +++ b/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"; +}; diff --git a/test/java/security/Security/AddProvider.policy.2 b/test/java/security/Security/AddProvider.policy.2 new file mode 100644 index 0000000000000000000000000000000000000000..d5a7ca9444249dac681252de97cde9f1e6d3e114 --- /dev/null +++ b/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"; +}; diff --git a/test/java/security/Security/AddProvider.policy.3 b/test/java/security/Security/AddProvider.policy.3 new file mode 100644 index 0000000000000000000000000000000000000000..930b443d7f683ab28d0111913be85806815bb91e --- /dev/null +++ b/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.*"; +}; |