From 5c27ac09c7127a449ad8d9535a0fe420d94829f2 Mon Sep 17 00:00:00 2001
From: Sean Mullan <mullan@openjdk.org>
Date: Mon, 16 May 2016 13:53:46 -0400
Subject: [PATCH] 8150468: ClassCircularityError on error in security policy
file
Reviewed-by: mchung, xuelei
---
.../sun/security/provider/PolicyFile.java | 36 ++++++++++---
.../sun/security/provider/PolicyParser.java | 38 +++++++++-----
.../provider/PolicyFile/BadPolicyFile.java | 51 +++++++++++++++++++
.../provider/PolicyFile/BadPolicyFile.policy | 4 ++
4 files changed, 110 insertions(+), 19 deletions(-)
create mode 100644 jdk/test/sun/security/provider/PolicyFile/BadPolicyFile.java
create mode 100644 jdk/test/sun/security/provider/PolicyFile/BadPolicyFile.policy
diff --git a/jdk/src/java.base/share/classes/sun/security/provider/PolicyFile.java b/jdk/src/java.base/share/classes/sun/security/provider/PolicyFile.java
index 5861b6c7a85..861fa692471 100644
--- a/jdk/src/java.base/share/classes/sun/security/provider/PolicyFile.java
+++ b/jdk/src/java.base/share/classes/sun/security/provider/PolicyFile.java
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 1997, 2015, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1997, 2016, 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
@@ -259,14 +259,10 @@ public class PolicyFile extends java.security.Policy {
private static final Debug debug = Debug.getInstance("policy");
- private static final String NONE = "NONE";
- private static final String P11KEYSTORE = "PKCS11";
-
private static final String SELF = "${{self}}";
private static final String X500PRINCIPAL =
"javax.security.auth.x500.X500Principal";
private static final String POLICY = "java.security.policy";
- private static final String SECURITY_MANAGER = "java.security.manager";
private static final String POLICY_URL = "policy.url.";
private static final String AUTH_POLICY = "java.security.auth.policy";
private static final String AUTH_POLICY_URL = "auth.policy.url.";
@@ -287,6 +283,17 @@ public class PolicyFile extends java.security.Policy {
private static final Class<?>[] PARAMS1 = { String.class };
private static final Class<?>[] PARAMS2 = { String.class, String.class };
+ /**
+ * When a policy file has a syntax error, the exception code may generate
+ * another permission check and this can cause the policy file to be parsed
+ * repeatedly, leading to a StackOverflowError or ClassCircularityError.
+ * To avoid this, this set is populated with policy files that have been
+ * previously parsed and have syntax errors, so that they can be
+ * subsequently ignored.
+ */
+ private static AtomicReference<Set<URL>> badPolicyURLs =
+ new AtomicReference<>(new HashSet<>());
+
/**
* Initializes the Policy object and reads the default policy
* configuration file(s) into the Policy object.
@@ -580,6 +587,16 @@ public class PolicyFile extends java.security.Policy {
* @param policyFile the policy Reader object.
*/
private boolean init(URL policy, PolicyInfo newInfo) {
+
+ // skip parsing policy file if it has been previously parsed and
+ // has syntax errors
+ if (badPolicyURLs.get().contains(policy)) {
+ if (debug != null) {
+ debug.println("skipping bad policy file: " + policy);
+ }
+ return false;
+ }
+
boolean success = false;
PolicyParser pp = new PolicyParser(expandProperties);
InputStreamReader isr = null;
@@ -622,13 +639,18 @@ public class PolicyFile extends java.security.Policy {
addGrantEntry(ge, keyStore, newInfo);
}
} catch (PolicyParser.ParsingException pe) {
+ // record bad policy file to avoid later reparsing it
+ badPolicyURLs.updateAndGet(k -> {
+ k.add(policy);
+ return k;
+ });
MessageFormat form = new MessageFormat(ResourcesMgr.getString
(POLICY + ".error.parsing.policy.message"));
Object[] source = {policy, pe.getLocalizedMessage()};
System.err.println(form.format(source));
- if (debug != null)
+ if (debug != null) {
pe.printStackTrace();
-
+ }
} catch (Exception e) {
if (debug != null) {
debug.println("error parsing "+policy);
diff --git a/jdk/src/java.base/share/classes/sun/security/provider/PolicyParser.java b/jdk/src/java.base/share/classes/sun/security/provider/PolicyParser.java
index ba8ed750254..f8aa620fc77 100644
--- a/jdk/src/java.base/share/classes/sun/security/provider/PolicyParser.java
+++ b/jdk/src/java.base/share/classes/sun/security/provider/PolicyParser.java
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 1997, 2014, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1997, 2016, 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
@@ -213,7 +213,9 @@ public class PolicyParser {
new MessageFormat(ResourcesMgr.getString(
"duplicate.keystore.domain.name"));
Object[] source = {domainName};
- throw new ParsingException(form.format(source));
+ String msg = "duplicate keystore domain name: " +
+ domainName;
+ throw new ParsingException(msg, form, source);
}
}
} else {
@@ -743,7 +745,8 @@ public class PolicyParser {
ResourcesMgr.getString
("expected.expect.read.end.of.file."));
Object[] source = {expect};
- throw new ParsingException(form.format(source));
+ String msg = "expected [" + expect + "], read [end of file]";
+ throw new ParsingException(msg, form, source);
case StreamTokenizer.TT_WORD:
if (expect.equalsIgnoreCase(st.sval)) {
lookahead = st.nextToken();
@@ -1244,7 +1247,8 @@ public class PolicyParser {
MessageFormat form = new MessageFormat(ResourcesMgr.getString(
"duplicate.keystore.name"));
Object[] source = {keystoreName};
- throw new ParsingException(form.format(source));
+ String msg = "duplicate keystore name: " + keystoreName;
+ throw new ParsingException(msg, form, source);
}
}
@@ -1316,6 +1320,8 @@ public class PolicyParser {
private static final long serialVersionUID = -4330692689482574072L;
private String i18nMessage;
+ private MessageFormat form;
+ private Object[] source;
/**
* Constructs a ParsingException with the specified
@@ -1330,26 +1336,34 @@ public class PolicyParser {
i18nMessage = msg;
}
+ public ParsingException(String msg, MessageFormat form,
+ Object[] source) {
+ super(msg);
+ this.form = form;
+ this.source = source;
+ }
+
public ParsingException(int line, String msg) {
super("line " + line + ": " + msg);
- MessageFormat form = new MessageFormat
- (ResourcesMgr.getString("line.number.msg"));
- Object[] source = {line, msg};
- i18nMessage = form.format(source);
+ // don't call form.format unless getLocalizedMessage is called
+ // to avoid unnecessary permission checks
+ form = new MessageFormat(ResourcesMgr.getString("line.number.msg"));
+ source = new Object[] {line, msg};
}
public ParsingException(int line, String expect, String actual) {
super("line " + line + ": expected [" + expect +
"], found [" + actual + "]");
- MessageFormat form = new MessageFormat(ResourcesMgr.getString
+ // don't call form.format unless getLocalizedMessage is called
+ // to avoid unnecessary permission checks
+ form = new MessageFormat(ResourcesMgr.getString
("line.number.expected.expect.found.actual."));
- Object[] source = {line, expect, actual};
- i18nMessage = form.format(source);
+ source = new Object[] {line, expect, actual};
}
@Override
public String getLocalizedMessage() {
- return i18nMessage;
+ return i18nMessage != null ? i18nMessage : form.format(source);
}
}
diff --git a/jdk/test/sun/security/provider/PolicyFile/BadPolicyFile.java b/jdk/test/sun/security/provider/PolicyFile/BadPolicyFile.java
new file mode 100644
index 00000000000..17a4c5d8d71
--- /dev/null
+++ b/jdk/test/sun/security/provider/PolicyFile/BadPolicyFile.java
@@ -0,0 +1,51 @@
+/*
+ * Copyright (c) 2016, 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 8150468
+ * @summary check that a badly formatted policy file is handled correctly
+ * @run main/othervm BadPolicyFile
+ */
+
+import java.io.File;
+import java.net.URI;
+import java.security.AccessControlException;
+import java.security.Policy;
+import java.security.URIParameter;
+
+public class BadPolicyFile {
+
+ public static void main(String[] args) throws Exception {
+ URI uri = new File(System.getProperty("test.src", "."),
+ "BadPolicyFile.policy").toURI();
+ Policy.setPolicy(Policy.getInstance("JavaPolicy", new URIParameter(uri)));
+ System.setSecurityManager(new SecurityManager());
+ try {
+ String javahome = System.getProperty("java.home");
+ throw new Exception("Expected AccessControlException");
+ } catch (AccessControlException ace) {
+ System.out.println("Test PASSED");
+ }
+ }
+}
diff --git a/jdk/test/sun/security/provider/PolicyFile/BadPolicyFile.policy b/jdk/test/sun/security/provider/PolicyFile/BadPolicyFile.policy
new file mode 100644
index 00000000000..b5abfaf8f42
--- /dev/null
+++ b/jdk/test/sun/security/provider/PolicyFile/BadPolicyFile.policy
@@ -0,0 +1,4 @@
+grant {
+ // permission statement is missing trailing semi-colon
+ permission "java.util.PropertyPermission" "java.home", "read"
+};