db-derby-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From d..@apache.org
Subject svn commit: r1617737 - in /db/derby/code/trunk/java: engine/org/apache/derby/iapi/services/monitor/ engine/org/apache/derby/impl/services/monitor/ engine/org/apache/derby/loc/ shared/org/apache/derby/shared/common/reference/ testing/org/apache/derbyTes...
Date Wed, 13 Aug 2014 14:47:31 GMT
Author: dag
Date: Wed Aug 13 14:47:30 2014
New Revision: 1617737

URL: http://svn.apache.org/r1617737
Log:
DERBY-6117 Silently swallowed SecurityExceptions may disable Derby features, including security
features.

Patch derby-6617-3b. It adds reporting of the swallowed
SecurityException in FileMonitor#createDaemonGroup.

To get that printed I also i had to to make sure to dump the temporary
log in a change added to BaseMonitor#runWithState.

Additionally, I discovered that Derby became unbootable if we lack the
modifyThreadGroup permission: the monitor in such an event, lacking a
proper handler, thought it was already initialized so subsequent boot
attempts (from a non-system thread so we wouldn't see the
modifyThreadGroup issue) would also fail. I added a handler to clear
the monitor to fix this.

A new test fixture, testModifyThreadGroup checks that the warning
message being written to the console.

Modified:
    db/derby/code/trunk/java/engine/org/apache/derby/iapi/services/monitor/Monitor.java
    db/derby/code/trunk/java/engine/org/apache/derby/impl/services/monitor/BaseMonitor.java
    db/derby/code/trunk/java/engine/org/apache/derby/impl/services/monitor/FileMonitor.java
    db/derby/code/trunk/java/engine/org/apache/derby/loc/messages.xml
    db/derby/code/trunk/java/shared/org/apache/derby/shared/common/reference/MessageId.java
    db/derby/code/trunk/java/testing/org/apache/derbyTesting/junit/BaseJDBCTestCase.java
    db/derby/code/trunk/java/testing/org/apache/derbyTesting/unitTests/junit/MissingPermissionsTest.java
    db/derby/code/trunk/java/testing/org/apache/derbyTesting/unitTests/junit/MissingPermissionsTest.policy

Modified: db/derby/code/trunk/java/engine/org/apache/derby/iapi/services/monitor/Monitor.java
URL: http://svn.apache.org/viewvc/db/derby/code/trunk/java/engine/org/apache/derby/iapi/services/monitor/Monitor.java?rev=1617737&r1=1617736&r2=1617737&view=diff
==============================================================================
--- db/derby/code/trunk/java/engine/org/apache/derby/iapi/services/monitor/Monitor.java (original)
+++ db/derby/code/trunk/java/engine/org/apache/derby/iapi/services/monitor/Monitor.java Wed
Aug 13 14:47:30 2014
@@ -22,6 +22,7 @@
 package org.apache.derby.iapi.services.monitor;
 
 import java.io.PrintWriter;
+import java.security.AccessControlException;
 import java.util.Locale;
 import java.util.Properties;
 import org.apache.derby.iapi.error.StandardException;
@@ -280,12 +281,17 @@ public class Monitor {
 			until an InfoStreams module is successfully started.
 	*/
 
+    @SuppressWarnings("ResultOfObjectAllocationIgnored")
 	public static void startMonitor(Properties bootProperties, PrintWriter logging) {
-
-		new org.apache.derby.impl.services.monitor.FileMonitor(bootProperties, logging);			
+        try {
+            new org.apache.derby.impl.services.monitor.FileMonitor(bootProperties, logging);
+        } catch (AccessControlException e) {
+            clearMonitor();
+            throw e;
+        }
 	}
 	/**
-		Initialise this class, must only be called by an implementation
+        Initialize this class, must only be called by an implementation
 		of the monitor (ModuleFactory).
 	*/
 	public static boolean setMonitor(ModuleFactory theMonitor) {

Modified: db/derby/code/trunk/java/engine/org/apache/derby/impl/services/monitor/BaseMonitor.java
URL: http://svn.apache.org/viewvc/db/derby/code/trunk/java/engine/org/apache/derby/impl/services/monitor/BaseMonitor.java?rev=1617737&r1=1617736&r2=1617737&view=diff
==============================================================================
--- db/derby/code/trunk/java/engine/org/apache/derby/impl/services/monitor/BaseMonitor.java
(original)
+++ db/derby/code/trunk/java/engine/org/apache/derby/impl/services/monitor/BaseMonitor.java
Wed Aug 13 14:47:30 2014
@@ -89,6 +89,7 @@ import java.util.NoSuchElementException;
 import java.lang.reflect.InvocationTargetException;
 
 import java.net.URL;
+import java.security.AccessControlException;
 
 /**
 	Implementation of the monitor that uses the class loader
@@ -364,7 +365,10 @@ abstract class BaseMonitor
 			dumpTempWriter(true);
 
 			return;
-		}
+        } catch (AccessControlException e) {
+            dumpTempWriter(true);
+            throw e;
+        }
 
 		// switch cover to the real error stream and
 		// dump any messages we have been saving ...

Modified: db/derby/code/trunk/java/engine/org/apache/derby/impl/services/monitor/FileMonitor.java
URL: http://svn.apache.org/viewvc/db/derby/code/trunk/java/engine/org/apache/derby/impl/services/monitor/FileMonitor.java?rev=1617737&r1=1617736&r2=1617737&view=diff
==============================================================================
--- db/derby/code/trunk/java/engine/org/apache/derby/impl/services/monitor/FileMonitor.java
(original)
+++ db/derby/code/trunk/java/engine/org/apache/derby/impl/services/monitor/FileMonitor.java
Wed Aug 13 14:47:30 2014
@@ -38,6 +38,7 @@ import org.apache.derby.iapi.services.in
 import org.apache.derby.iapi.services.info.ProductVersionHolder;
 import org.apache.derby.iapi.services.io.FileUtil;
 import org.apache.derby.shared.common.reference.MessageId;
+import org.apache.derby.shared.common.sanity.SanityManager;
 
 /**
 	Implementation of the monitor that uses the class loader
@@ -84,15 +85,21 @@ public final class FileMonitor extends B
      * the group is destroyed and garbage collected when all its
      * members have finished (i.e., either when the driver is
      * unloaded, or when the last database is shut down).
+     *
+     * @return the thread group "derby.daemons" or null if we saw
+     * a SecurityException
      */
-    private static ThreadGroup createDaemonGroup() {
+    private ThreadGroup createDaemonGroup() {
         try {
             ThreadGroup group = new ThreadGroup("derby.daemons");
             group.setDaemon(true);
             return group;
         } catch (SecurityException se) {
-            // In case of a lacking privilege, silently return null and let
-            // the daemon threads be created in the default thread group.
+            // In case of a lacking privilege, issue a warning, return null and
+            // let the daemon threads be created in the default thread group.
+            // This can only happen if the current Derby thread is a part of
+            // the root thread group "system".
+            reportThread(se);
             return null;
         }
     }
@@ -221,6 +228,10 @@ public final class FileMonitor extends B
                 e.toString()));
     }
 
+    private void reportThread(SecurityException e) {
+        report(MessageService.getTextMessage(
+                MessageId.CANNOT_SET_DAEMON, e.toString()));
+    }
 
 	/*
 	** Priv block code, moved out of the old Java2 version.

Modified: db/derby/code/trunk/java/engine/org/apache/derby/loc/messages.xml
URL: http://svn.apache.org/viewvc/db/derby/code/trunk/java/engine/org/apache/derby/loc/messages.xml?rev=1617737&r1=1617736&r2=1617737&view=diff
==============================================================================
--- db/derby/code/trunk/java/engine/org/apache/derby/loc/messages.xml (original)
+++ db/derby/code/trunk/java/engine/org/apache/derby/loc/messages.xml Wed Aug 13 14:47:30
2014
@@ -8882,6 +8882,13 @@ page id:            {0}
             </msg>
 
             <msg>
+                <name>M010</name>
+                <text>WARNING: could not do ThreadGroup#setDaemon on Derby daemons
due to a security exception: {0}. This may impact operation.
+                </text>
+                <arg>error</arg>
+            </msg>
+
+            <msg>
                 <name>N001</name>
                 <text>This error is caused by the following error.</text>
             </msg>

Modified: db/derby/code/trunk/java/shared/org/apache/derby/shared/common/reference/MessageId.java
URL: http://svn.apache.org/viewvc/db/derby/code/trunk/java/shared/org/apache/derby/shared/common/reference/MessageId.java?rev=1617737&r1=1617736&r2=1617737&view=diff
==============================================================================
--- db/derby/code/trunk/java/shared/org/apache/derby/shared/common/reference/MessageId.java
(original)
+++ db/derby/code/trunk/java/shared/org/apache/derby/shared/common/reference/MessageId.java
Wed Aug 13 14:47:30 2014
@@ -231,6 +231,7 @@ public interface MessageId {
      */
     String CANNOT_READ_SECURITY_PROPERTY                     = "M008";
     String CANNOT_CREATE_FILE_OR_DIRECTORY                   = "M009";
+    String CANNOT_SET_DAEMON                                 = "M010";
     /*
      * Misc
      */

Modified: db/derby/code/trunk/java/testing/org/apache/derbyTesting/junit/BaseJDBCTestCase.java
URL: http://svn.apache.org/viewvc/db/derby/code/trunk/java/testing/org/apache/derbyTesting/junit/BaseJDBCTestCase.java?rev=1617737&r1=1617736&r2=1617737&view=diff
==============================================================================
--- db/derby/code/trunk/java/testing/org/apache/derbyTesting/junit/BaseJDBCTestCase.java (original)
+++ db/derby/code/trunk/java/testing/org/apache/derbyTesting/junit/BaseJDBCTestCase.java Wed
Aug 13 14:47:30 2014
@@ -538,7 +538,7 @@ public abstract class BaseJDBCTestCase
      * specified user name and password.
      * <BR>
      * This connection is not
-     * automaticaly closed on tearDown, the test fixture must
+     * automatically closed on tearDown, the test fixture must
      * ensure the connection is closed.
      * 
      * The connection will be initialized by calling initializeConnection.

Modified: db/derby/code/trunk/java/testing/org/apache/derbyTesting/unitTests/junit/MissingPermissionsTest.java
URL: http://svn.apache.org/viewvc/db/derby/code/trunk/java/testing/org/apache/derbyTesting/unitTests/junit/MissingPermissionsTest.java?rev=1617737&r1=1617736&r2=1617737&view=diff
==============================================================================
--- db/derby/code/trunk/java/testing/org/apache/derbyTesting/unitTests/junit/MissingPermissionsTest.java
(original)
+++ db/derby/code/trunk/java/testing/org/apache/derbyTesting/unitTests/junit/MissingPermissionsTest.java
Wed Aug 13 14:47:30 2014
@@ -25,7 +25,9 @@ import java.io.BufferedReader;
 import java.io.FileNotFoundException;
 import java.io.FileReader;
 import java.io.IOException;
+import java.security.AccessControlException;
 import java.security.AccessController;
+import java.security.PrivilegedAction;
 import java.security.PrivilegedActionException;
 import java.security.PrivilegedExceptionAction;
 import java.sql.Connection;
@@ -67,6 +69,8 @@ public class MissingPermissionsTest exte
             "MissingPermissionsTest.policy";
     private final static String OK_POLICY_T =
             testPrefix + OK_POLICY;
+    private final static String OK_POLICY_R =
+            resourcePrefix + OK_POLICY;
 
     private final static String POLICY_MINUS_PROPERTYPERMISSION =
             "MissingPermissionsTest1.policy";
@@ -86,6 +90,11 @@ public class MissingPermissionsTest exte
     private final int KIND_EXPECT_ERROR_MSG_PRESENT = 0;
     private final int KIND_EXPECT_ERROR_MSG_ABSENT = 1;
 
+    /**
+     * Used for running #testModifyThreadGroup
+     */
+    private static boolean inSubProcess = false;
+
     public MissingPermissionsTest(String name) {
         super(name);
     }
@@ -107,27 +116,37 @@ public class MissingPermissionsTest exte
     }
 
     public static Test suite() {
+        inSubProcess = Boolean.getBoolean("inSubProcess");
+
         final BaseTestSuite suite =
                 new BaseTestSuite("SystemPrivilegesPermissionTest");
 
-        if (!TestConfiguration.loadingFromJars()) {
-            // This test only works with jar files.
+        if (!inSubProcess && !TestConfiguration.loadingFromJars()) {
+            // This test only works with jar files. Only check at top
+            // level
             return suite;
         }
 
-        suite.addTest(
-                new SupportFilesSetup(
-                        makeTest("testMissingFilePermission",
-                                POLICY_MINUS_FILEPERMISSION_T),
-                        new String[] {
-                            POLICY_MINUS_FILEPERMISSION_R}));
+        if (!inSubProcess) {
+            suite.addTest(
+                    new SupportFilesSetup(
+                            makeTest("testMissingFilePermission",
+                                    POLICY_MINUS_FILEPERMISSION_T),
+                            new String[] {
+                                POLICY_MINUS_FILEPERMISSION_R}));
 
-        suite.addTest(makeTest("testPresentPropertiesPermission",
-                OK_POLICY_T));
+            suite.addTest(makeTest("testPresentPropertiesPermission",
+                    OK_POLICY_T));
 
-        suite.addTest(makeTest("testMissingPropertiesPermission",
-                POLICY_MINUS_PROPERTYPERMISSION_T));
+            suite.addTest(makeTest("testMissingPropertiesPermission",
+                    POLICY_MINUS_PROPERTYPERMISSION_T));
+        }
 
+        // This test runs in both the top process and a subprocess since it has
+        // two parts:
+        suite.addTest(new SupportFilesSetup(makeTest("testModifyThreadGroup",
+                OK_POLICY_T),
+                new String[] {OK_POLICY_R}));
         return suite;
     }
 
@@ -338,4 +357,132 @@ public class MissingPermissionsTest exte
                 return new BufferedReader(new FileReader(file));
             }});
     }
+
+
+    public void testModifyThreadGroup() throws Throwable {
+        if (!inSubProcess) {
+            // Set up run of this test in a sub process, so we can catch its
+            // standard err/standard out.
+            final List<String> args = new ArrayList<String>();
+            args.add("-DinSubProcess=true");
+            args.add("-Djava.security.manager");
+            args.add(
+                "-Djava.security.policy=extin/MissingPermissionsTest.policy");
+            args.add("-DderbyTesting.codejar="
+                    + getSystemProperty("derbyTesting.codejar"));
+            args.add("-DderbyTesting.testjar="
+                    + getSystemProperty("derbyTesting.testjar"));
+            args.add("-DderbyTesting.junit="
+                    + getSystemProperty("derbyTesting.junit"));
+            String antjunit = getSystemProperty("derbyTesting.antjunit");
+            if (antjunit != null) {
+                // This property is only available when the test is started
+                // by Ant's JUnit task.
+                args.add("-DderbyTesting.antjunit=" + antjunit);
+            }
+            args.add("-Dderby.system.home=system/nested_tMTG");
+            args.add("-Dderby.system.durability=" +
+                     getSystemProperty("derby.system.durability"));
+            args.add("-Dderby.tests.trace=" +
+                     getSystemProperty("derby.tests.trace"));
+            args.add("-Dderby.system.debug=" +
+                     getSystemProperty("derby.tests.debug"));
+            args.add("junit.textui.TestRunner");
+            args.add(this.getClass().getName());
+
+            final String[] argArray = args.toArray(new String[0]);
+            final Process p = execJavaCmd(argArray);
+            SpawnedProcess spawned = new SpawnedProcess(p, "MPT");
+            spawned.suppressOutputOnComplete(); // we want to read it ourselves
+
+            // The started process is an interactive ij session that will wait
+            // for user input. Close stdin of the process so that it stops
+            // waiting and exits.
+            p.getOutputStream().close();
+
+            final int exitCode = spawned.complete(120000L); // 2 minutes
+
+            assertTrue(spawned.getFailMessage("subprocess run failed: "),
+                    exitCode == 0);
+
+            final String expectedMessageOnConsole =
+                    "WARNING: could not do ThreadGroup#setDaemon on Derby " +
+                    "daemons due to a security exception";
+
+            final String output = spawned.getFullServerOutput(); // ignore
+            final String err    = spawned.getFullServerError();
+
+            assertTrue(err, err.contains(expectedMessageOnConsole));
+
+            // Print sub process' output if this test specifies any such
+            if (Boolean.parseBoolean(
+                        getSystemProperty("derby.tests.trace")) ||
+                Boolean.parseBoolean(
+                    getSystemProperty("derby.tests.debug"))) {
+
+                System.out.println("\n[ (subprocess) " +
+                        output.replace("\n", "\n  (subprocess) ") + "]\n");
+            }
+
+        } else {
+            final SystemThreadRun mst = new SystemThreadRun(this);
+
+            Thread t = AccessController.doPrivileged(
+                new PrivilegedAction<Thread>() {
+                    @Override
+                    public Thread run() {
+                        return new Thread(
+                            Thread.currentThread().getThreadGroup().getParent(),
+                            mst);
+                    }});
+
+
+            t.start();
+            t.join();
+
+            // The boot will fail since operation that require
+            // modifyThreadGroup lead to boot failure. So the fact that the
+            // same permission is missing in FileMonitor#createDaemonGroup
+            // isn't an issue: it will not go undetected. It fails at this line
+            // in BaseMonitor#runWithState:
+            //
+            //  timerFactory = (TimerFactory)Monitor.startSystemModule(
+            //         "org.apache.derby.iapi.services.timer.TimerFactory");
+            //
+            // and the AccessControlException isn't caught and percolates all
+            // the way out.
+            assertTrue(mst.f instanceof AccessControlException);
+
+            // This patch also fixes the fact that previously, the monitor in such
+            // an event, thought it was already initialized so subsequent boot
+            // attempt (from a non-system thread) would also fail. We now clean up,
+            // so a boot here should work.
+            openDefaultConnection("APP", "APPPW").close();
+        }
+    }
+
+    private class SystemThreadRun implements Runnable {
+        public Throwable f;
+        private final BaseJDBCTestCase test;
+
+        public SystemThreadRun(BaseJDBCTestCase test) {
+            super();
+            this.test = test;
+        }
+
+        @SuppressWarnings({"BroadCatchBlock", "TooBroadCatch"})
+        @Override
+        public void run() {
+            try {
+                assertEquals(
+                    Thread.currentThread().getThreadGroup().getName(),
+                    "system");
+                // Expect this to fail with AccessControlException
+                test.openDefaultConnection("APP", "APPPW").close();
+                fail();
+            } catch (Throwable e) {
+                this.f = e;
+            }
+        }
+    }
 }

Modified: db/derby/code/trunk/java/testing/org/apache/derbyTesting/unitTests/junit/MissingPermissionsTest.policy
URL: http://svn.apache.org/viewvc/db/derby/code/trunk/java/testing/org/apache/derbyTesting/unitTests/junit/MissingPermissionsTest.policy?rev=1617737&r1=1617736&r2=1617737&view=diff
==============================================================================
--- db/derby/code/trunk/java/testing/org/apache/derbyTesting/unitTests/junit/MissingPermissionsTest.policy
(original)
+++ db/derby/code/trunk/java/testing/org/apache/derbyTesting/unitTests/junit/MissingPermissionsTest.policy
Wed Aug 13 14:47:30 2014
@@ -115,6 +115,8 @@ grant codeBase "${derbyTesting.testjar}d
   // to refresh the policy
   permission java.util.PropertyPermission "java.security.policy", "read,write";
   permission java.lang.RuntimePermission "setSecurityManager";
+  permission java.lang.RuntimePermission "modifyThreadGroup";
+  permission java.lang.RuntimePermission "modifyThread";
   permission java.security.SecurityPermission "getPolicy";
 
   // derbyTesting.junit.TestConfiguration... modifies System properties
@@ -124,7 +126,7 @@ grant codeBase "${derbyTesting.testjar}d
   permission javax.security.auth.AuthPermission "doAsPrivileged";
 
   // **** Needed by this test
-  permission java.io.FilePermission "<<ALL FILES>>", "read,write,delete";
+  permission java.io.FilePermission "<<ALL FILES>>", "read,write,delete,execute";
 };
 
 // JUnit jar file tries to read junit.properties in the user's
@@ -133,6 +135,8 @@ grant codeBase "${derbyTesting.testjar}d
 // junit.swingui.TestRunner writes to .junitsession on exit.
 grant codeBase "${derbyTesting.junit}" {
     permission java.util.PropertyPermission "user.home", "read";
+    permission java.util.PropertyPermission "user.dir", "read";
+    permission java.util.PropertyPermission "inSubProcess", "read";
     permission java.io.FilePermission "${user.home}${/}junit.properties", "read";
     permission java.io.FilePermission "${user.home}${/}.junitsession", "write";
 



Mime
View raw message