diff --git a/.travis.yml b/.travis.yml index c6403a367..68099e57c 100644 --- a/.travis.yml +++ b/.travis.yml @@ -6,6 +6,10 @@ arch: - ppc64le dist: bionic env: + - POSTGRESQL_VERSION: 13 + JAVA_VERSION: 15 + JVM_IMPL: hotspot + MVN_VERSION: 3.5.2 - POSTGRESQL_VERSION: 12 JAVA_VERSION: 14 JVM_IMPL: hotspot @@ -151,8 +155,13 @@ script: | boolean succeeding = false; // begin pessimistic + import static java.nio.file.Files.createTempFile + import static java.nio.file.Files.write + import java.nio.file.Path import static java.nio.file.Paths.get import java.sql.Connection + import java.sql.PreparedStatement + import java.sql.ResultSet import org.postgresql.pljava.packaging.Node import static org.postgresql.pljava.packaging.Node.q import static org.postgresql.pljava.packaging.Node.stateMachine @@ -251,6 +260,70 @@ script: | (o,p,q) -> null == o ); + /* + * Exercise TrialPolicy some. Need another connection to change + * vmoptions. Uses some example functions, so insert here before the + * test of undeploying the examples. + */ + try ( Connection c2 = n1.connect() ) + { + Path trialPolicy = + createTempFile(n1.data_dir().getParent(), "trial", "policy"); + + write(trialPolicy, List.of( + "grant {", + " permission", + " org.postgresql.pljava.policy.TrialPolicy$Permission;", + "};" + )); + + PreparedStatement setVmOpts = c2.prepareStatement( + "SELECT null::pg_catalog.void" + + " FROM pg_catalog.set_config('pljava.vmoptions', ?, false)" + ); + + setVmOpts.setString(1, vmopts + + " -Dorg.postgresql.pljava.policy.trial=" + trialPolicy.toUri()); + + succeeding &= stateMachine( + "change pljava.vmoptions", + null, + + q(setVmOpts, setVmOpts::execute) + .flatMap(Node::semiFlattenDiagnostics) + .peek(Node::peek), + + (o,p,q) -> isDiagnostic(o, Set.of("error")) ? 1 : -2, + (o,p,q) -> isVoidResultSet(o, 1, 1) ? 3 : false, + (o,p,q) -> null == o + ); + + PreparedStatement tryForbiddenRead = c2.prepareStatement( + "SELECT" + + " CASE WHEN javatest.java_getsystemproperty('java.home')" + + " OPERATOR(pg_catalog.=) ?" + + " THEN javatest.logmessage('INFO', 'trial policy test ok')" + + " ELSE javatest.logmessage('WARNING', 'trial policy test ng')" + + " END" + ); + + tryForbiddenRead.setString(1, System.getProperty("java.home")); + + succeeding &= stateMachine( + "try to read a forbidden property", + null, + + q(tryForbiddenRead, tryForbiddenRead::execute) + .flatMap(Node::semiFlattenDiagnostics) + .peek(Node::peek), + + (o,p,q) -> isDiagnostic(o, Set.of("error", "warning")) ? 1 : -2, + (o,p,q) -> isVoidResultSet(o, 1, 1) ? 3 : false, + (o,p,q) -> null == o + ); + // done with connection c2 + } + /* * Also confirm that the generated undeploy actions work. */ @@ -267,6 +340,41 @@ script: | (o,p,q) -> null == o ); } + + /* + * Get another new connection and make sure the extension can be loaded + * in a non-superuser session. + */ + try ( Connection c = n1.connect() ) + { + succeeding &= stateMachine( + "become non-superuser", + null, + + q(c, + "CREATE ROLE alice;" + + "GRANT USAGE ON SCHEMA sqlj TO alice;" + + "SET SESSION AUTHORIZATION alice") + .flatMap(Node::semiFlattenDiagnostics) + .peek(Node::peek), + + (o,p,q) -> isDiagnostic(o, Set.of("error")) ? 1 : -2, + (o,p,q) -> null == o + ); + + succeeding &= stateMachine( + "load as non-superuser", + null, + + q(c, "SELECT null::pg_catalog.void FROM sqlj.get_classpath('public')") + .flatMap(Node::semiFlattenDiagnostics) + .peek(Node::peek), + + (o,p,q) -> isDiagnostic(o, Set.of("error")) ? 1 : -2, + (o,p,q) -> isVoidResultSet(o, 1, 1) ? 3 : false, + (o,p,q) -> null == o + ); + } } catch ( Throwable t ) { succeeding = false; diff --git a/appveyor.yml b/appveyor.yml index ca13103dd..849223b7f 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -10,16 +10,22 @@ environment: JDK: 10 PG: 12 - SYS: MINGW - JDK: 14 + JDK: 11 + PG: 12 + - SYS: MINGW + JDK: 12 PG: 12 - SYS: MINGW JDK: 13 PG: 12 - SYS: MINGW - JDK: 12 + JDK: 14 PG: 12 - SYS: MINGW - JDK: 11 + JDK: 15 + PG: 12 + - SYS: MSVC + JDK: 15 PG: 12 - SYS: MSVC JDK: 14 @@ -82,8 +88,13 @@ test_script: @' boolean succeeding = false; // begin pessimistic + import static java.nio.file.Files.createTempFile + import static java.nio.file.Files.write + import java.nio.file.Path import static java.nio.file.Paths.get import java.sql.Connection + import java.sql.PreparedStatement + import java.sql.ResultSet import org.postgresql.pljava.packaging.Node import static org.postgresql.pljava.packaging.Node.q import static org.postgresql.pljava.packaging.Node.stateMachine @@ -189,6 +200,70 @@ test_script: (o,p,q) -> null == o ); + /* + * Exercise TrialPolicy some. Need another connection to change + * vmoptions. Uses some example functions, so insert here before the + * test of undeploying the examples. + */ + try ( Connection c2 = n1.connect() ) + { + Path trialPolicy = + createTempFile(n1.data_dir().getParent(), "trial", "policy"); + + write(trialPolicy, List.of( + "grant {", + " permission", + " org.postgresql.pljava.policy.TrialPolicy$Permission;", + "};" + )); + + PreparedStatement setVmOpts = c2.prepareStatement( + "SELECT null::pg_catalog.void" + + " FROM pg_catalog.set_config('pljava.vmoptions', ?, false)" + ); + + setVmOpts.setString(1, vmopts + + " -Dorg.postgresql.pljava.policy.trial=" + trialPolicy.toUri()); + + succeeding &= stateMachine( + "change pljava.vmoptions", + null, + + q(setVmOpts, setVmOpts::execute) + .flatMap(Node::semiFlattenDiagnostics) + .peek(Node::peek), + + (o,p,q) -> isDiagnostic(o, Set.of("error")) ? 1 : -2, + (o,p,q) -> isVoidResultSet(o, 1, 1) ? 3 : false, + (o,p,q) -> null == o + ); + + PreparedStatement tryForbiddenRead = c2.prepareStatement( + "SELECT" + + " CASE WHEN javatest.java_getsystemproperty('java.home')" + + " OPERATOR(pg_catalog.=) ?" + + " THEN javatest.logmessage('INFO', 'trial policy test ok')" + + " ELSE javatest.logmessage('WARNING', 'trial policy test ng')" + + " END" + ); + + tryForbiddenRead.setString(1, System.getProperty("java.home")); + + succeeding &= stateMachine( + "try to read a forbidden property", + null, + + q(tryForbiddenRead, tryForbiddenRead::execute) + .flatMap(Node::semiFlattenDiagnostics) + .peek(Node::peek), + + (o,p,q) -> isDiagnostic(o, Set.of("error", "warning")) ? 1 : -2, + (o,p,q) -> isVoidResultSet(o, 1, 1) ? 3 : false, + (o,p,q) -> null == o + ); + // done with connection c2 + } + /* * Also confirm that the generated undeploy actions work. */ @@ -205,6 +280,42 @@ test_script: (o,p,q) -> null == o ); } + + /* + * Get another new connection and make sure the extension can be loaded + * in a non-superuser session. + */ + try ( Connection c = n1.connect() ) + { + succeeding &= stateMachine( + "become non-superuser", + null, + + q(c, + "CREATE ROLE alice;" + + "GRANT USAGE ON SCHEMA sqlj TO alice;" + + "SET SESSION AUTHORIZATION alice") + .flatMap(Node::semiFlattenDiagnostics) + .peek(Node::peek), + + (o,p,q) -> isDiagnostic(o, Set.of("error")) ? 1 : -2, + (o,p,q) -> null == o + ); + + succeeding &= stateMachine( + "load as non-superuser", + null, + + q(c, + "SELECT null::pg_catalog.void FROM sqlj.get_classpath('public')") + .flatMap(Node::semiFlattenDiagnostics) + .peek(Node::peek), + + (o,p,q) -> isDiagnostic(o, Set.of("error")) ? 1 : -2, + (o,p,q) -> isVoidResultSet(o, 1, 1) ? 3 : false, + (o,p,q) -> null == o + ); + } } catch ( Throwable t ) { succeeding = false; diff --git a/pljava-packaging/src/main/resources/pljava.policy b/pljava-packaging/src/main/resources/pljava.policy index 5fbdd1719..c360dcdb5 100644 --- a/pljava-packaging/src/main/resources/pljava.policy +++ b/pljava-packaging/src/main/resources/pljava.policy @@ -42,6 +42,11 @@ grant { // permission java.util.PropertyPermission "jdk.lang.ref.disableClearBeforeEnqueue", "read"; + + // Something similar happened in Java 14 (not yet fixed in 15). + // + permission java.util.PropertyPermission + "java.util.concurrent.ForkJoinPool.common.maximumSpares", "read"; }; @@ -58,6 +63,8 @@ grant codebase "${org.postgresql.pljava.codesource}" { "charsetProvider"; permission java.lang.RuntimePermission "createClassLoader"; + permission java.lang.RuntimePermission + "getProtectionDomain"; permission java.net.NetPermission "specifyStreamHandler"; permission java.util.logging.LoggingPermission diff --git a/pljava-so/src/main/c/Backend.c b/pljava-so/src/main/c/Backend.c index 89d84125e..4d3b5258d 100644 --- a/pljava-so/src/main/c/Backend.c +++ b/pljava-so/src/main/c/Backend.c @@ -208,6 +208,7 @@ static bool seenVisualVMName; static bool seenModuleMain; static char const visualVMprefix[] = "-Dvisualvm.display.name="; static char const moduleMainPrefix[] = "-Djdk.module.main="; +static char const policyUrlsGUC[] = "pljava.policy_urls"; /* * In a background worker, _PG_init may be called very early, before much of @@ -1648,7 +1649,7 @@ static void registerGUCOptions(void) NULL); /* show hook */ STRING_GUC( - "pljava.policy_urls", + policyUrlsGUC, "URLs to Java security policy file(s) for PL/Java's use", "Quote each URL and separate with commas. Any URL may begin (inside " "the quotes) with n= where n is the index of the Java " @@ -1923,7 +1924,11 @@ JNICALL Java_org_postgresql_pljava_internal_Backend__1getConfigOption(JNIEnv* en { PG_TRY(); { - const char *value = PG_GETCONFIGOPTION(key); + const char *value; + if ( 0 == strcmp(policyUrlsGUC, key) ) + value = policy_urls; + else + value = PG_GETCONFIGOPTION(key); pfree(key); if(value != 0) result = String_createJavaStringFromNTS(value); diff --git a/pljava/src/main/java/module-info.java b/pljava/src/main/java/module-info.java index 610f58612..68923bbe4 100644 --- a/pljava/src/main/java/module-info.java +++ b/pljava/src/main/java/module-info.java @@ -23,6 +23,8 @@ exports org.postgresql.pljava.elog to java.logging; + exports org.postgresql.pljava.policy to java.base; // has custom Permission + provides java.net.spi.URLStreamHandlerProvider with org.postgresql.pljava.sqlj.Handler; diff --git a/pljava/src/main/java/org/postgresql/pljava/internal/InstallHelper.java b/pljava/src/main/java/org/postgresql/pljava/internal/InstallHelper.java index 9e23c30b2..42ed012b4 100644 --- a/pljava/src/main/java/org/postgresql/pljava/internal/InstallHelper.java +++ b/pljava/src/main/java/org/postgresql/pljava/internal/InstallHelper.java @@ -17,6 +17,8 @@ import java.net.URL; import java.net.MalformedURLException; import java.nio.charset.Charset; +import java.security.NoSuchAlgorithmException; +import java.security.Policy; import java.security.Security; import java.sql.Connection; import java.sql.DatabaseMetaData; @@ -36,6 +38,7 @@ import org.postgresql.pljava.jdbc.SQLUtils; import org.postgresql.pljava.management.SQLDeploymentDescriptor; +import org.postgresql.pljava.policy.TrialPolicy; import static org.postgresql.pljava.annotation.processing.DDRWriter.eQuote; import static org.postgresql.pljava.sqlgen.Lexicals.Identifier.Simple; @@ -161,6 +164,8 @@ public static String hello( e); } + setTrialPolicyIfSpecified(); + System.setSecurityManager( new SecurityManager()); StringBuilder sb = new StringBuilder(); @@ -255,6 +260,24 @@ private static void setPolicyURLs() } } + private static void setTrialPolicyIfSpecified() throws SQLException + { + String trialURI = System.getProperty( + "org.postgresql.pljava.policy.trial"); + + if ( null == trialURI ) + return; + + try + { + Policy.setPolicy( new TrialPolicy( trialURI)); + } + catch ( NoSuchAlgorithmException e ) + { + throw new SQLException(e.getMessage(), e); + } + } + public static void groundwork( String module_pathname, String loadpath_tbl, String loadpath_tbl_quoted, boolean asExtension, boolean exNihilo) diff --git a/pljava/src/main/java/org/postgresql/pljava/policy/TrialPolicy.java b/pljava/src/main/java/org/postgresql/pljava/policy/TrialPolicy.java new file mode 100644 index 000000000..320a66c48 --- /dev/null +++ b/pljava/src/main/java/org/postgresql/pljava/policy/TrialPolicy.java @@ -0,0 +1,439 @@ +/* + * Copyright (c) 2020 Tada AB and other contributors, as listed below. + * + * All rights reserved. This program and the accompanying materials + * are made available under the terms of the The BSD 3-Clause License + * which accompanies this distribution, and is available at + * http://opensource.org/licenses/BSD-3-Clause + * + * Contributors: + * Chapman Flack + */ +package org.postgresql.pljava.policy; + +import java.lang.reflect.ReflectPermission; + +import java.net.URI; + +import java.security.CodeSource; +import java.security.NoSuchAlgorithmException; +import java.security.Permission; +import java.security.PermissionCollection; +import java.security.Policy; +import java.security.ProtectionDomain; +import java.security.SecurityPermission; +import java.security.URIParameter; + +import java.util.ArrayList; +import java.util.Arrays; +import static java.util.Collections.emptyEnumeration; +import static java.util.Collections.enumeration; +import java.util.Enumeration; +import java.util.Iterator; +import java.util.List; + +import static org.postgresql.pljava.elog.ELogHandler.LOG_LOG; +import static org.postgresql.pljava.internal.Backend.log; +import static org.postgresql.pljava.internal.Backend.threadMayEnterPG; +import static org.postgresql.pljava.internal.Privilege.doPrivileged; + +/** + * An implementation of {@link Policy} intended for temporary use while + * identifying needed permission grants for existing code. + *

+ * This policy is meant to operate as a fallback in conjunction with the normal + * PL/Java policy specified with the {@code pljava.policy_urls} configuration + * setting. This policy is activated by specifying an additional policy file + * URL with {@code -Dorg.postgresql.pljava.policy.trial=}url in the + * {@code pljava.vmoptions} setting. + *

+ * Permission checks that are allowed by the normal policy in + * {@code pljava.policy_urls} are allowed with no further checking. Permissions + * denied by that policy are checked in this one. If denied in this policy, that + * is the end of the matter. A permission check that is denied by the normal + * policy but allowed by this one is allowed, with a message to the server log. + *

+ * The log message begins with {@code POLICY DENIES/TRIAL POLICY ALLOWS:} + * and the requested permission, followed by an abbreviated stack trace. + * To minimize log volume, the stack trace includes a frame above and below + * each crossing of a module or protection domain boundary; a single {@code ...} + * replaces intermediate frames within the same module and domain. + * At the position in the trace of the protection domain that failed the policy + * check, a line is inserted with the domain's code source and principals, + * such as {@code >> sqlj:examples [PLPrincipal.Sandboxed: java] <<}. This + * abbreviated trace should be well suited to the purpose of determining where + * any additional permission grants ought to be made. + *

+ * Because each check that is logged is then allowed, it can be possible to see + * multiple log entries for the same permission check, one for each domain in + * the call stack that is not granted the permission in the normal policy. + *

About false positives

+ * It is not uncommon to have software that checks in normal operation for + * certain permissions, catches exceptions, and proceeds to function normally. + * Use of this policy, if it is configured to grant the permissions being + * checked, will produce log entries for those 'hidden' checks and may create + * the appearance that permissions need to be granted when, in fact, the + * software would show no functional impairment without them. It is difficult + * to distinguish such false positives from other log entries for permissions + * that do need to be granted for the software to properly function. + *

+ * One approach would be to try to determine, from the log entries, which + * functions of the software led to the permission checks that were logged, and + * specifically test those functions in a database session that has been set up + * with a different policy file that does not grant those permissions. If the + * software then functions without incident, it may be concluded that those + * log entries were false positives. + */ +public class TrialPolicy extends Policy +{ + private static final String TYPE = "JavaPolicy"; + private static final RuntimePermission GET_PROTECTION_DOMAIN = + new RuntimePermission("getProtectionDomain"); + private final Policy realPolicy; + private final Policy limitPolicy; + private final StackWalker walker = + StackWalker.getInstance(StackWalker.Option.RETAIN_CLASS_REFERENCE); + + public TrialPolicy(String limitURI) throws NoSuchAlgorithmException + { + URIParameter lim = new URIParameter(URI.create(limitURI)); + realPolicy = Policy.getInstance(TYPE, null); + limitPolicy = Policy.getInstance(TYPE, lim); + } + + @Override + public PermissionCollection getPermissions(CodeSource codesource) + { + return realPolicy.getPermissions(codesource); + } + + @Override + public PermissionCollection getPermissions(ProtectionDomain domain) + { + return realPolicy.getPermissions(domain); + } + + @Override + public boolean implies( + ProtectionDomain domain, java.security.Permission permission) + { + if ( realPolicy.implies(domain, permission) ) + return true; + + if ( ! limitPolicy.implies(domain, permission) ) + { + /* + * The TrialPolicy.Permission below is an unusual one: like Java's + * own AllPermission, its implies() can be true for permissions of + * other classes than its own. Java's AllPermission is handled + * magically, and this one must be also, because deep down, the + * built-in Policy implementation keeps its PermissionCollections + * segregated by permission class. It would not notice on its own + * that 'permission' might be implied by a permission that is held + * but is of some other class. + */ + if ( ! limitPolicy.implies(domain, Permission.INSTANCE) + || ! Permission.INSTANCE.implies(permission) ) + return false; + } + + /* + * Construct a (with any luck, useful) abbreviated stack trace, using + * the first frame encountered at each change of protection domain while + * walking up the stack, saving the index of the first entry for the + * domain being checked. + */ + List stack = new ArrayList<>(); + int matchingDomainIndex = doPrivileged(() -> walker.walk(s -> + { + ProtectionDomain lastDomain = null; + StackWalker.StackFrame lastFrame = null; + Module lastModule = null; + Module thisModule = getClass().getModule(); + int matchIndex = -1; + int walkIndex = 0; + int newDomainIndex = 0; // walkIndex of first frame in a new domain + for ( StackWalker.StackFrame f : + (Iterable)s.skip(5)::iterator ) + { + ++ walkIndex; + Class frameClass = f.getDeclaringClass(); + Module frameModule = frameClass.getModule(); + ProtectionDomain frameDomain = frameClass.getProtectionDomain(); + if ( ! equals(lastDomain, frameDomain) + || null != lastModule && ! lastModule.equals(frameModule) ) + { + if ( null != lastFrame && walkIndex > 1 + newDomainIndex ) + { + if ( walkIndex > 2 + newDomainIndex ) + stack.add(null); // will be rendered as ... + stack.add(lastFrame.toStackTraceElement()); + } + if ( -1 == matchIndex && equals(domain, frameDomain) ) + matchIndex = stack.size(); + stack.add(f.toStackTraceElement()); + lastModule = frameModule; + lastDomain = frameDomain; + newDomainIndex = walkIndex; + } + + /* + * Exit the walk early, skip boring EntryPoints. + */ + if ( frameModule.equals(thisModule) + && "org.postgresql.pljava.internal.EntryPoints" + .equals(frameClass.getName()) ) + { + if ( newDomainIndex == walkIndex ) + stack.remove(stack.size() - 1); + -- walkIndex; + break; + } + + lastFrame = f; + } + + if ( null != lastFrame && walkIndex > 1 + newDomainIndex ) + stack.add(lastFrame.toStackTraceElement()); + + if ( -1 == matchIndex ) + matchIndex = stack.size(); + return matchIndex; + }), null, GET_PROTECTION_DOMAIN); + + /* + * Construct a string representation of the trace. + */ + StringBuilder sb = new StringBuilder( + "POLICY DENIES/TRIAL POLICY ALLOWS: " + permission + '\n'); + Iterator it = stack.iterator(); + int i = 0; + for ( ;; ) + { + if ( matchingDomainIndex == i ++ ) + sb.append(">> ") + .append(domain.getCodeSource().getLocation()) + .append(' ') + .append(Arrays.toString(domain.getPrincipals())) + .append(" <<\n"); + if ( ! it.hasNext() ) + break; + StackTraceElement e = it.next(); + sb.append(null == e ? "..." : e.toString()); + if ( it.hasNext() || matchingDomainIndex == i ) + sb.append('\n'); + } + + /* + * This is not the best way to avoid blocking on log(); in some flavors + * of pljava.java_thread_pg_entry, threadMayEnterPG can return false + * simply because it's not /known/ that PG could be entered right now, + * and this could send the message off to System.err at times even if + * log() would have completed with no blocking. But the always accurate + * "could I enter PG right now without blocking?" method isn't provided + * yet. + */ + if ( threadMayEnterPG() ) + log(LOG_LOG, sb.toString()); + else + System.err.println(sb); + + return true; + } + + @Override + public void refresh() + { + realPolicy.refresh(); + limitPolicy.refresh(); + } + + /* + * Compare two protection domains, only by their code source for now. + * It appears that StackWalker doesn't invoke domain combiners, so the + * frames seen in the walk won't match the principals of the argument + * to implies(). + */ + private boolean equals(ProtectionDomain a, ProtectionDomain b) + { + if ( null == a || null == b) + return a == b; + + CodeSource csa = a.getCodeSource(); + CodeSource csb = b.getCodeSource(); + + if ( null == csa || null == csb ) + return csa == csb; + + return csa.equals(csb); + } + + /** + * A permission like {@code java.security.AllPermission}, but without + * any {@code FilePermission} (the real policy's sandboxed/unsandboxed + * grants should handle those), nor a couple dozen varieties of + * {@code RuntimePermission}, {@code SecurityPermission}, and + * {@code ReflectPermission} that would typically not be granted without + * clear intent. + *

+ * This permission can be granted in a {@code TrialPolicy} while identifying + * any straggling permissions needed by some existing code, without quite + * the excitement of granting {@code AllPermission}. Any of the permissions + * excluded from this one can also be granted in the {@code TrialPolicy}, + * of course, if there is reason to believe the code might need them. + *

+ * The proper spelling in a policy file is + * {@code org.postgresql.pljava.policy.TrialPolicy$Permission}. + *

+ * This permission will probably only work right in a {@code TrialPolicy}. + * Any permission whose {@code implies} method can return true for + * permissions of other classes than its own may be ineffective in a stock + * Java policy, where permission collections are kept segregated by the + * class of the permission to be checked. Java's {@code AllPermission} gets + * special-case treatment in the stock implementation, and this permission + * likewise has to be treated specially in {@code TrialPolicy}. The only + * kind of custom permission that can genuinely drop in and work is one + * whose {@code implies} method only imposes semantics on the names/actions + * of different instances of that permission class. + *

+ * A permission that does not live on the boot classpath is initially read + * from a policy file as an instance of {@code UnresolvedPermission}, and + * only gets resolved when a permission check is made, checking for an + * instance of its actual class. That is another complication when + * implementing a permission that may imply permissions of other classes. + *

+ * A permission implemented in a different named module must be in a package + * that is exported to {@code java.base}. + */ + public static final class Permission extends java.security.Permission + { + private static final long serialVersionUID = 6401893677037633706L; + + /** + * An instance of this permission (not a singleton, merely one among + * possible others). + */ + static final Permission INSTANCE = new Permission(); + + public Permission() + { + super(""); + } + + public Permission(String name, String actions) + { + super(""); + } + + @Override + public boolean equals(Object other) + { + return other instanceof Permission; + } + + @Override + public int hashCode() + { + return 131113; + } + + @Override + public String getActions() + { + return null; + } + + @Override + public PermissionCollection newPermissionCollection() + { + return new Collection(); + } + + @Override + public boolean implies(java.security.Permission p) + { + if ( p instanceof Permission ) + return true; + + if ( p instanceof java.io.FilePermission ) + return false; + + if ( Holder.EXCLUDERHS.stream().anyMatch(r -> p.implies(r)) ) + return false; + + if ( Holder.EXCLUDELHS.stream().anyMatch(l -> l.implies(p)) ) + return false; + + return true; + } + + static class Collection extends PermissionCollection + { + private static final long serialVersionUID = 917249873714843122L; + + Permission the_permission = null; + + @Override + public void add(java.security.Permission p) + { + if ( isReadOnly() ) + throw new SecurityException( + "attempt to add a Permission to a readonly " + + "PermissionCollection"); + + if ( ! (p instanceof Permission) ) + throw new IllegalArgumentException( + "invalid in homogeneous PermissionCollection: " + p); + + if ( null == the_permission ) + the_permission = (Permission) p; + } + + @Override + public boolean implies(java.security.Permission p) + { + if ( null == the_permission ) + return false; + return the_permission.implies(p); + } + + @Override + public Enumeration elements() + { + if ( null == the_permission ) + return emptyEnumeration(); + return enumeration(List.of(the_permission)); + } + } + + static class Holder + { + static final List EXCLUDERHS = List.of( + new RuntimePermission("createClassLoader"), + new RuntimePermission("getClassLoader"), + new RuntimePermission("setContextClassLoader"), + new RuntimePermission("enableContextClassLoaderOverride"), + new RuntimePermission("setSecurityManager"), + new RuntimePermission("createSecurityManager"), + new RuntimePermission("shutdownHooks"), + new RuntimePermission("exitVM"), + new RuntimePermission("setFactory"), + new RuntimePermission("setIO"), + new RuntimePermission("getStackWalkerWithClassReference"), + new RuntimePermission("setDefaultUncaughtExceptionHandler"), + new RuntimePermission("manageProcess"), + new ReflectPermission("suppressAccessChecks"), + new SecurityPermission("createAccessControlContext"), + new SecurityPermission("setPolicy"), + new SecurityPermission("createPolicy.JavaPolicy") + ); + + static final List EXCLUDELHS = List.of( + new RuntimePermission("exitVM.*"), + new RuntimePermission("defineClassInPackage.*"), + new ReflectPermission("newProxyInPackage.*"), + new SecurityPermission("setProperty.*") + ); + } + } +} diff --git a/pljava/src/main/java/org/postgresql/pljava/policy/package-info.java b/pljava/src/main/java/org/postgresql/pljava/policy/package-info.java new file mode 100644 index 000000000..dc411b58b --- /dev/null +++ b/pljava/src/main/java/org/postgresql/pljava/policy/package-info.java @@ -0,0 +1,22 @@ +/* + * Copyright (c) 2020 Tada AB and other contributors, as listed below. + * + * All rights reserved. This program and the accompanying materials + * are made available under the terms of the The BSD 3-Clause License + * which accompanies this distribution, and is available at + * http://opensource.org/licenses/BSD-3-Clause + * + * Contributors: + * Tada AB + * Purdue University + */ +/** + * Package implementing custom Java security policy useful while migrating + * existing code to policy-based PL/Java; allows permission checks denied by the + * main policy to succeed, while logging them so any needed permission grants + * can be identified and added to the main policy. + *

+ * This package is exported to {@code java.base} to provide a custom + * {@code Permission} that can be granted in policy. + */ +package org.postgresql.pljava.policy; diff --git a/pljava/src/main/java/org/postgresql/pljava/sqlj/Handler.java b/pljava/src/main/java/org/postgresql/pljava/sqlj/Handler.java index a0bd626b5..268c6be03 100644 --- a/pljava/src/main/java/org/postgresql/pljava/sqlj/Handler.java +++ b/pljava/src/main/java/org/postgresql/pljava/sqlj/Handler.java @@ -29,7 +29,7 @@ public class Handler extends URLStreamHandlerProvider { private static final Handler INSTANCE = new Handler(); - public URLStreamHandlerProvider provider() + public static URLStreamHandlerProvider provider() { return INSTANCE; } diff --git a/src/site/markdown/use/policy.md b/src/site/markdown/use/policy.md index 02cdf8b08..8ce129486 100644 --- a/src/site/markdown/use/policy.md +++ b/src/site/markdown/use/policy.md @@ -318,8 +318,21 @@ such as `java.version` or `org.postgresql.pljava.version`._ ## Troubleshooting -When in doubt what permissions are needed to get some existing PL/Java code -working again, it may be helpful to add `-Djava.security.debug=access` in +When in doubt what permissions may need to be granted in `pljava.policy` to run +some existing PL/Java code, these techniques may be helpful. + +### Running PL/Java with a 'trial' policy + +To simplify the job of finding the permissions needed by some existing code, +it is possible to run PL/Java at first with a 'trial' policy, allowing code to +run while logging permissions that `pljava.policy` has not granted. The log +entries have a condensed format meant to be convenient for this use. +Trial policy configuration is described [here][trial]. + +### Using policy debug features provided by Java + +Java itself offers a number of debugging switches to reveal details of +permission decisions. It may be useful to add `-Djava.security.debug=access` in the setting of `pljava.vmoptions`, and observe the messages on the PostgreSQL backend's standard error (which should be included in the log file, if `logging_collector` is `on`). It is not necessary to change the @@ -330,6 +343,9 @@ Other options for `java.security.debug` can be found in [Troubleshooting Security][tssec]. Some can be used to filter the logging down to requests for specific permissions or from a specific codebase. +The log output produced by Java's debug options can be voluminous compared to +the condensed output of PL/Java's trial policy. + ## Forward compatibility The current implementation makes use of the Java classes @@ -344,3 +360,4 @@ release, so relying on it is not recommended. [dopriv]: https://docs.oracle.com/en/java/javase/14/security/java-se-platform-security-architecture.html#GUID-E8898CB5-65BB-4D1A-A574-8F7112FC353F [sqljajl]: ../pljava/apidocs/org.postgresql.pljava.internal/org/postgresql/pljava/management/Commands.html#alias_java_language [tssec]: https://docs.oracle.com/en/java/javase/14/security/troubleshooting-security.html +[trial]: trial.html diff --git a/src/site/markdown/use/trial.md b/src/site/markdown/use/trial.md new file mode 100644 index 000000000..79d4bffda --- /dev/null +++ b/src/site/markdown/use/trial.md @@ -0,0 +1,186 @@ +# Migrating to policy-based permissions from an earlier PL/Java release + +When migrating existing code from a PL/Java 1.5 or earlier release to 1.6, +it may be necessary to add permission grants in the new `pljava.policy` file, +which grants few permissions by default. PL/Java's security policy configuration +is described [here][policy]. + +To simplify migration, it is possible to run with a 'trial' policy initially, +allowing code to run but logging permissions that may need to be added in +`pljava.policy`. + +## Configuring a trial policy + +Even when running with a trial policy, the [configuration variable][vbls] +`pljava.policy_urls` should point to the normal policy file(s), as usual. +That is where the ultimate policy for production will be developed. + +The trial policy is configured by creating another policy file somewhere, using +the same policy file syntax, and pointing to it with +`-Dorg.postgresql.pljava.policy.trial=`_url_ added to the configuration variable +`pljava.vmoptions`. + +Anything _this_ policy allows will be allowed, but will be logged if the regular +policy would have denied it. So you can make this one more generous than the +regular policy, and use the log entries to identify grants that might belong in +the regular policy. As you add the missing ones to the real policy, they stop +getting logged by this one, and the log gets quieter. You can make this one as +generous as you are comfortable making it during the period of testing and +tuning. + +At the very extreme of generosity it could be this: + +``` +grant { + permission java.security.AllPermission; +}; +``` + +and it would happily allow the code under test to do _anything at all_, while +logging whatever permissions aren't in the regular policy. (A side effect of +this would be to erase any distinction between `java` and `javaU` for as long as +the trial policy is in place.) Such a setting would be difficult to recommend in +general, but it might suffice if the only code being tested has already been in +use for years under PL/Java 1.5 and is well trusted, users of the database have +not been granted permission to install more PL/Java functions, and if +the purpose of testing is only to learn what permissions the code uses that +may need to be granted in the 1.6 policy. + +### Granting `TrialPolicy$Permission` + +When `AllPermission` is too broad, there is the difficulty that Java's +permission model does not have a subtractive mode; it is not simple to say +"grant `AllPermission` except for this list of the ones I'd really rather not." +Therefore, PL/Java offers a custom "meta-permission" with roughly that meaning: + +``` +grant { + permission org.postgresql.pljava.policy.TrialPolicy$Permission; +}; +``` + +`TrialPolicy$Permission` is effectively `AllPermission` but excluding any +`FilePermission` (so that `java`/`javaU` distinction stays meaningful) as well +as a couple dozen other various +`SecurityPermission`/`ReflectPermission`/`RuntimePermission` instances in the +"really rather not" category. If its hard-coded exclusion list excludes +any permissions that some unusual code under test might legitimately need, +those can be explicitly added to the trial policy too. + +Configuring a trial policy can be a bit of a balancing act: if it is very +generous, that minimizes the chance of breaking the code under test because of +a denied permission, but increases potential exposure if that code misbehaves. +A more limited trial policy decreases exposure but increase the risk of +service interruption if the code under test really does need some permission +that you weren't comfortable putting in the trial policy. Somewhere near +the sweet spot is where `TrialPolicy$Permission` is aimed. + +All other normal policy features also work in the trial policy. If your +code is installed in several different jars, you can use `grant codebase` +separately to put different outer limits around different jars, and completely +remove the grants for one jar after another as you are satisfied you have added +the right things for each one in the regular policy. You could also set +different limits for `java` and `javaU` by granting to the `PLPrincipal`, +just as you can in the regular policy. + +## About false positives + +One thing to be aware of is that the trial policy can give false alarms. It is +not uncommon for software to include configuration-dependent bits that +tentatively try certain actions, catch exceptions, and then proceed normally, +having discovered what the configuration allows. The trial policy can log +permission denials that happen in the course of such checks, even if the denial +has no functional impact on the code. + +There may be no perfect way to tell which denials being logged by the trial +policy are false alarms. One approach would be to collect a sampling of log +entries, figure out what user-visible functions of the code they were coming +from, and then start a dedicated session without the +`-Dorg.postgresql.pljava.policy.trial` setting (or with it pointing to a +different, more restrictive version of the policy, not granting the permissions +you're curious about), then exercise those functions of the code and see if +anything breaks. Other users could still have the more generous trial setting in +their sessions, so as not to be affected by your experiments. + +False positives, of course, are also affected by the choice of how generous to +make the trial policy. Log entries are only produced for permissions that the +regular policy denies but the trial policy allows. If the permissions being +silently checked by benign code are not granted in the trial policy, they will +be silently denied, just as they would in normal operation, and produce no +log entries. + +## Format of the log entries + +To avoid bloating logs too much, `TrialPolicy` emits an abbreviated form of +stack trace for each entry. The approach is to keep one stack frame above and +one below each crossing of a module or protection-domain boundary, with `...` +replacing intermediate frames within the same module/domain, and the code +source/principals of the denied domain shown wrapped in `>> <<`at +the appropriate position in the trace. For the purpose of identifying the +source of a permission request and the appropriate domain(s) to be granted +the permission, this is probably more usable than the very long full traces +available with `java.security.debug`. + +The messages are sent through the PostgreSQL log if the thread making the +permission check knows it can do so without blocking; otherwise they just go to +standard error, which should wind up in the PostgreSQL log anyway, if +`logging_collector` is on; otherwise it may be system-dependent where they go. + +There isn't really a reliable "can I do so without blocking?" check for every +setting of the `pljava.java_thread_pg_entry` configuration variable. +If it is set to `throw` (and that is a workable setting for the code under +test), the logging behavior will be more predictable; entries from the main +thread will go through PostgreSQL's log facility always, and those from any +other thread will go to standard error. + +Here is an example of two log entries, generated by the same permission check: + +``` +POLICY DENIES/TRIAL POLICY ALLOWS: ("java.net.SocketPermission" "127.0.0.1:5432" "connect,resolve") +java.base/java.security.ProtectionDomain.implies(ProtectionDomain.java:321) +... +java.base/java.net.Socket.(Socket.java:294) +>> null [PLPrincipal.Sandboxed: java] << +jdk.translet/die.verwandlung.GregorSamsa.template$dot$0() +... +jdk.translet/die.verwandlung.GregorSamsa.transform() +java.xml/com.sun.org.apache.xalan.internal.xsltc.runtime.AbstractTranslet.transform(AbstractTranslet.java:624) +... +java.xml/com.sun.org.apache.xalan.internal.xsltc.trax.TransformerImpl.transform(TransformerImpl.java:383) +org.postgresql.pljava.example.annotation.PassXML.transformXML(PassXML.java:561) + +POLICY DENIES/TRIAL POLICY ALLOWS: ("java.net.SocketPermission" "127.0.0.1:5432" "connect,resolve") +java.base/java.security.ProtectionDomain.implies(ProtectionDomain.java:321) +... +java.base/java.net.Socket.(Socket.java:294) +jdk.translet/die.verwandlung.GregorSamsa.template$dot$0() +... +jdk.translet/die.verwandlung.GregorSamsa.transform() +java.xml/com.sun.org.apache.xalan.internal.xsltc.runtime.AbstractTranslet.transform(AbstractTranslet.java:624) +... +java.xml/com.sun.org.apache.xalan.internal.xsltc.trax.TransformerImpl.transform(TransformerImpl.java:383) +>> sqlj:examples [PLPrincipal.Sandboxed: java] << +org.postgresql.pljava.example.annotation.PassXML.transformXML(PassXML.java:561) +``` + +The example shows the use of an XSLT 1.0 transform that appears to +make use of the Java XSLT ability to call out to arbitrary Java, and is trying +to make a network connection back to PostgreSQL on `localhost`. Java's XSLTC +implementation compiles the transform to a class in `jdk.translet` with null +as its codebase, and the first log entry shows permission is denied at that +level (the protection domain shown as +`>> null [PLPrincipal.Sandboxed: java] <<`). + +A second log entry results because `TrialPolicy` turns the first failure to +success, allowing the permission check to continue, and it next fails at +the PL/Java function being called, in the `sqlj:examples` jar. Under the trial +policy, that also is logged and then allowed to succeed. + +The simplest way to allow this connection in the production policy would be +to grant the needed `java.net.SocketPermission` to `PLPrincipal$Sandboxed`, +as that is present in both denied domains. It would be possible to grant +the permission by codebase to `sqlj:examples` instead, but not to +the nameless codebase of the compiled XSLT transform. + +[policy]: policy.html +[vbls]: variables.html diff --git a/src/site/markdown/use/use.md b/src/site/markdown/use/use.md index 5ed1cec51..27b16e47a 100644 --- a/src/site/markdown/use/use.md +++ b/src/site/markdown/use/use.md @@ -42,6 +42,15 @@ The permissions in effect for PL/Java functions can be tailored, independently for functions declared to the `TRUSTED` or untrusted language, as described [here](policy.html). +#### Tailoring permissions for code migrated from PL/Java pre-1.6 + +When migrating existing code from a PL/Java 1.5 or earlier release to 1.6, +it may be necessary to add permission grants in the new `pljava.policy` file, +which grants few permissions by default. To simplify migration, it is possible +to run with a 'trial' policy initially, allowing code to run but logging +permissions that may need to be added in `pljava.policy`. How to do that is +described [here](trial.html). + ### Choices when mapping data types #### Date and time types