Derby
  1. Derby
  2. DERBY-5514

SecureServerTest (and others) don't play nice with EMMA: AccessControlException

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 10.9.1.0
    • Fix Version/s: 10.9.1.0
    • Component/s: Test
    • Labels:
      None

      Description

      When running SecureServerTest with jars instrumented with EMMA using the ant emma-all target, I see:

      [junit] (net)derbynet.SecureServerTest.testServerStartup used 8475 ms junit.framework.TestListener: endTest(testServerStartup)
      [junit] START-SPAWNED:SpawnedNetworkServer ERROR OUTPUT:
      [junit] java.security.policy: error adding Entry:
      [junit] java.net.MalformedURLException: no protocol: /export/home/dag/java/sb/sb1/tools/java/emma.jar
      [junit] java.security.AccessControlException: access denied (java.io.FilePermission coverage.ec read)
      [junit] at java.security.AccessControlContext.checkPermission(AccessControlContext.java:374)
      [junit] at java.security.AccessController.checkPermission(AccessController.java:546)
      [junit] at java.lang.SecurityManager.checkPermission(SecurityManager.java:532)
      [junit] at java.lang.SecurityManager.checkRead(SecurityManager.java:871)
      [junit] at java.io.File.exists(File.java:731)
      [junit] at com.vladium.emma.data.DataFactory.persist(DataFactory.java:525)
      [junit] at com.vladium.emma.data.DataFactory.persist(DataFactory.java:86)
      [junit] at com.vladium.emma.rt.RTCoverageDataPersister.dumpCoverageData(RTCoverageDataPersister.java:54)
      [junit] at com.vladium.emma.rt.RTExitHook.run(RTExitHook.java:32)
      [junit] at java.lang.Thread.run(Thread.java:662)
      [junit] Exception in thread "EMMA shutdown handler thread" java.lang.RuntimeException: EMMA failed to dump coverage data: java.security.AccessControlException: access denied (java.io.FilePermission coverage.ec read)
      [junit] at com.vladium.emma.rt.RTCoverageDataPersister.dumpCoverageData(RTCoverageDataPersister.java:71)
      [junit] at com.vladium.emma.rt.RTExitHook.run(RTExitHook.java:32)
      [junit] at java.lang.Thread.run(Thread.java:662)
      [junit] END-SPAWNED :SpawnedNetworkServer ERROR OUTPUT:

      This is presumably because the test spawns a server which runs with the default server policy.

      Another thing is that is seems dangerous to let the spawned process write to EMMA's "coverage.ec", since we don't know when the parent process will write to it. This behavior could be what's been causing our corrutions in the EMMA runs earlier. The missing permissions just highlight what's happening.

      In this case the spawned process was started with this command line (I instrumented the code):

      XXX server startup command = /usr/jdk/instances/jdk1.6.0/jre/bin/java -classpath /<my sandbox>/tools/java/emma.jar:/<my sandbox>/jars/sane/derbyTesting.jar:/<my sandbox>/jars/emma/lib/derbyclient.jar:/<my sandbox>/jars/emma/lib/derbynet.jar:/<my sandbox>/jars/emma/lib/derby.jar:/<my sandbox>/jars/emma/lib/derbytools.jar:/<my sandbox>/jars/emma/lib/derbyrun.jar:/<my sandbox>/tools/java/junit.jar:/usr/share/lib/ant/ant-launcher.jar:/usr/share/lib/ant/ant.jar:/usr/share/lib/ant/ant-junit.jar org.apache.derby.drda.NetworkServerControl start -h localhost -p 1527

      Possible approaches:
      run the spawned VMs with plain jars (downside: we won't get coverage for those)
      run the spawned with a special default policy file when we run with EMMA ++
      run the spawned VM with -noSecurityManager if with EMMA jars

      1. d5514-emma-permissions-to-all.diff
        30 kB
        Knut Anders Hatlen
      2. derby-5514-1.diff
        2 kB
        Dag H. Wanvik
      3. derby-5514-1.stat
        0.2 kB
        Dag H. Wanvik
      4. derby-5514-2.diff
        4 kB
        Dag H. Wanvik
      5. derby-5514-2.stat
        0.3 kB
        Dag H. Wanvik

        Issue Links

          Activity

          Hide
          Dag H. Wanvik added a comment -

          Uploading a patch which implements the third approach. This made the test complete with EMMA instrumented jars.

          Show
          Dag H. Wanvik added a comment - Uploading a patch which implements the third approach. This made the test complete with EMMA instrumented jars.
          Hide
          Dag H. Wanvik added a comment -

          Related: RuntimeInfoTest fails due to asserting on the output from the spawned process: output contains EMMA info.

          Show
          Dag H. Wanvik added a comment - Related: RuntimeInfoTest fails due to asserting on the output from the spawned process: output contains EMMA info.
          Hide
          Knut Anders Hatlen added a comment -

          Running the network server with a special default policy file when we run with EMMA is probably the right thing to do, but running with -noSecurityManager also sounds perfectly acceptable.

          The patch didn't compile in my environment:

          junitcomponents:
          [javac] Compiling 1 source file to /code/derby/trunk/classes
          [javac] /code/derby/trunk/java/testing/org/apache/derbyTesting/junit/NetworkServerTestSetup.java:264: cannot find symbol
          [javac] symbol : method contains(java.lang.String)
          [javac] location: class java.lang.String
          [javac] classpath.contains("emma.jar"))
          [javac] ^
          [javac] 1 error

          Changing contains() to indexOf() made it work, and I could run SecureServerTest without seeing the stack traces getting printed.

          If I added the following line to NetworkServerTestSetup.startSeparateProcess()
          al.add("-Demma.verbosity.level=silent");
          I didn't have to change the assert in SecureServerTest.

          SecureServerTest.runServerCommand() and Driver40UnbootedTest.test_notBooted() also set this property to prevent that EMMA interferes with the canons.

          As to the problem with spawned processes possibly corrupting coverage.ec, perhaps we could run the spawned processes with the emma.coverage.out.file system property set, and use a counter to prevent name clashes? If so, I'd prefer that we do that in BaseTestCase.execJavaCmd() so that it works for most of the tests. NetworkServerTestSetup doesn't use BaseTestCase.execJavaCmd() yet, but making it do so should be trivial. Maybe it would also make sense to push the setting of emma.verbosity.level down to that method too, so that we don't have to do that in every test that happens to spawn a process.

          Show
          Knut Anders Hatlen added a comment - Running the network server with a special default policy file when we run with EMMA is probably the right thing to do, but running with -noSecurityManager also sounds perfectly acceptable. The patch didn't compile in my environment: junitcomponents: [javac] Compiling 1 source file to /code/derby/trunk/classes [javac] /code/derby/trunk/java/testing/org/apache/derbyTesting/junit/NetworkServerTestSetup.java:264: cannot find symbol [javac] symbol : method contains(java.lang.String) [javac] location: class java.lang.String [javac] classpath.contains("emma.jar")) [javac] ^ [javac] 1 error Changing contains() to indexOf() made it work, and I could run SecureServerTest without seeing the stack traces getting printed. If I added the following line to NetworkServerTestSetup.startSeparateProcess() al.add("-Demma.verbosity.level=silent"); I didn't have to change the assert in SecureServerTest. SecureServerTest.runServerCommand() and Driver40UnbootedTest.test_notBooted() also set this property to prevent that EMMA interferes with the canons. As to the problem with spawned processes possibly corrupting coverage.ec, perhaps we could run the spawned processes with the emma.coverage.out.file system property set, and use a counter to prevent name clashes? If so, I'd prefer that we do that in BaseTestCase.execJavaCmd() so that it works for most of the tests. NetworkServerTestSetup doesn't use BaseTestCase.execJavaCmd() yet, but making it do so should be trivial. Maybe it would also make sense to push the setting of emma.verbosity.level down to that method too, so that we don't have to do that in every test that happens to spawn a process.
          Hide
          Knut Anders Hatlen added a comment -

          > If I added the following line to NetworkServerTestSetup.startSeparateProcess()
          > al.add("-Demma.verbosity.level=silent");
          > I didn't have to change the assert in SecureServerTest.

          I spoke too soon. This removed the unexpected line "EMMA: collecting runtime coverage data ..." from the output, but it still didn't show the security manager information expected by the test (because the security manager is not used), so the assert still fails.

          Show
          Knut Anders Hatlen added a comment - > If I added the following line to NetworkServerTestSetup.startSeparateProcess() > al.add("-Demma.verbosity.level=silent"); > I didn't have to change the assert in SecureServerTest. I spoke too soon. This removed the unexpected line "EMMA: collecting runtime coverage data ..." from the output, but it still didn't show the security manager information expected by the test (because the security manager is not used), so the assert still fails.
          Hide
          Dag H. Wanvik added a comment -

          I found that the default test policy has the following entry for EMMA (granted to derby.jar)

          // These permissions are needed when testing code instrumented with EMMA.
          // They will only be used if the emma.active system property property is set,
          // which should be set to "" for the permissions to be correct.
          permission java.util.PropertyPermission "$

          {emma.active}user.dir", "read";
          permission java.io.FilePermission "${emma.active}

          $

          {user.dir}

          $

          {/}

          coverage.ec", "read, write";
          permission java.lang.RuntimePermission "$

          {emma.active}

          writeFileDescriptor";

          I tried approach two above, to craft a special policy file for the server and grant these privileges to emma.jar (for SecureServerTest). This worked in part, except for a case where there was a lacking permission for writeFileDescriptor (same stack trace as below).

          Now, in the tools suite I found a similar issue after running SysinfoLocaleTest:

          [junit] java.security.AccessControlException: access denied (java.lang.RuntimePermission writeFileDescriptor)
          [junit] at java.security.AccessControlContext.checkPermission(AccessControlContext.java:374)
          [junit] at java.security.AccessController.checkPermission(AccessController.java:546)
          [junit] at java.lang.SecurityManager.checkPermission(SecurityManager.java:532)
          [junit] at java.lang.SecurityManager.checkWrite(SecurityManager.java:937)
          [junit] at java.io.FileOutputStream.<init>(FileOutputStream.java:219)
          [junit] at com.vladium.emma.data.DataFactory$UCFileOutputStream.<init>(DataFactory.java:292)
          [junit] at com.vladium.emma.data.DataFactory$RandomAccessFileOutputStream.<init>(DataFactory.java:375)
          [junit] at com.vladium.emma.data.DataFactory.writeEntry(DataFactory.java:715)
          [junit] at com.vladium.emma.data.DataFactory.persist(DataFactory.java:672)
          [junit] at com.vladium.emma.data.DataFactory.persist(DataFactory.java:86)
          [junit] at com.vladium.emma.rt.RTCoverageDataPersister.dumpCoverageData(RTCoverageDataPersister.java:54)
          [junit] at com.vladium.emma.rt.RTExitHook.run(RTExitHook.java:32)
          [junit] at java.lang.Thread.run(Thread.java:662)

          except this time there is no spawned VM, so the code is running under the default test policy, which does have the EMMA permissions shown above. Note that this thread doesn't have any code from derby.jar on the stack, so the fact the derby.jar has the required privileges doesn't help: we still get AccessControlException.

          I downloaded the EMMA sources and wrapped the call to RTCoverageDataPersister#DataFactory.persist (cdataView, outFile, merge) in a doPrivilegedException call. That made it work.

          There is no other use of doPrivileged in EMMA's code, so we may see other similar problems.

          The exception seen above is later followed by this warning:

          [report] processing input file [/export/home/dag/java/sb/sb1/junit_20111124_1520/coverage.ec] ...
          [report] java.io.UTFDataFormatException: malformed input around byte 0

          which is may be the (partial?) reason why the coverage file gets corrupted.

          Show
          Dag H. Wanvik added a comment - I found that the default test policy has the following entry for EMMA (granted to derby.jar) // These permissions are needed when testing code instrumented with EMMA. // They will only be used if the emma.active system property property is set, // which should be set to "" for the permissions to be correct. permission java.util.PropertyPermission "$ {emma.active}user.dir", "read"; permission java.io.FilePermission "${emma.active} $ {user.dir} $ {/} coverage.ec", "read, write"; permission java.lang.RuntimePermission "$ {emma.active} writeFileDescriptor"; I tried approach two above, to craft a special policy file for the server and grant these privileges to emma.jar (for SecureServerTest). This worked in part, except for a case where there was a lacking permission for writeFileDescriptor (same stack trace as below). Now, in the tools suite I found a similar issue after running SysinfoLocaleTest: [junit] java.security.AccessControlException: access denied (java.lang.RuntimePermission writeFileDescriptor) [junit] at java.security.AccessControlContext.checkPermission(AccessControlContext.java:374) [junit] at java.security.AccessController.checkPermission(AccessController.java:546) [junit] at java.lang.SecurityManager.checkPermission(SecurityManager.java:532) [junit] at java.lang.SecurityManager.checkWrite(SecurityManager.java:937) [junit] at java.io.FileOutputStream.<init>(FileOutputStream.java:219) [junit] at com.vladium.emma.data.DataFactory$UCFileOutputStream.<init>(DataFactory.java:292) [junit] at com.vladium.emma.data.DataFactory$RandomAccessFileOutputStream.<init>(DataFactory.java:375) [junit] at com.vladium.emma.data.DataFactory.writeEntry(DataFactory.java:715) [junit] at com.vladium.emma.data.DataFactory.persist(DataFactory.java:672) [junit] at com.vladium.emma.data.DataFactory.persist(DataFactory.java:86) [junit] at com.vladium.emma.rt.RTCoverageDataPersister.dumpCoverageData(RTCoverageDataPersister.java:54) [junit] at com.vladium.emma.rt.RTExitHook.run(RTExitHook.java:32) [junit] at java.lang.Thread.run(Thread.java:662) except this time there is no spawned VM, so the code is running under the default test policy, which does have the EMMA permissions shown above. Note that this thread doesn't have any code from derby.jar on the stack, so the fact the derby.jar has the required privileges doesn't help: we still get AccessControlException. I downloaded the EMMA sources and wrapped the call to RTCoverageDataPersister#DataFactory.persist (cdataView, outFile, merge) in a doPrivilegedException call. That made it work. There is no other use of doPrivileged in EMMA's code, so we may see other similar problems. The exception seen above is later followed by this warning: [report] processing input file [/export/home/dag/java/sb/sb1/junit_20111124_1520/coverage.ec] ... [report] java.io.UTFDataFormatException: malformed input around byte 0 which is may be the (partial?) reason why the coverage file gets corrupted.
          Hide
          Dag H. Wanvik added a comment - - edited

          Small correction to the above: the default policy file for the tests does contain explicit permissions for the emma jar. But, that isn't enough when emma doesn't call doPrivileged, of course. The default server policy does not contain permissions for Emma.

          Show
          Dag H. Wanvik added a comment - - edited Small correction to the above: the default policy file for the tests does contain explicit permissions for the emma jar. But, that isn't enough when emma doesn't call doPrivileged, of course. The default server policy does not contain permissions for Emma.
          Hide
          Dag H. Wanvik added a comment -

          With the patch to Emma, the fix to DERBY-5512 (thanks, Knut) and setting the path the the old releases when calling ant (since i'm behind a firewall), I was able to make all tests in ant target "derby-core" run with emma jars, albeit a few at a time. I'll start all the tests now passing larger heap size etc in via ANT_OPTS, so I'll see how far I get with Emma running the "emma-all" target. It would be nice to have it running again.

          Show
          Dag H. Wanvik added a comment - With the patch to Emma, the fix to DERBY-5512 (thanks, Knut) and setting the path the the old releases when calling ant (since i'm behind a firewall), I was able to make all tests in ant target "derby-core" run with emma jars, albeit a few at a time. I'll start all the tests now passing larger heap size etc in via ANT_OPTS, so I'll see how far I get with Emma running the "emma-all" target. It would be nice to have it running again.
          Hide
          Knut Anders Hatlen added a comment -

          Thanks for that info, Dag! So it seems that the problem is that the JVM's runtime libraries don't have the permissions required by the EMMA code, and the lack of doPrivileged blocks in EMMA requires all code to have those permissions (except if Derby code that contains doPrivileged blocks itself is on the stack and limits the privilege checking).

          Based on that observation, I tried a different approach that didn't require modifying the EMMA source (see d5514-emma-permissions-to-all.diff). Instead, I changed the policy files to grant the permissions required by EMMA to all code bases when running with the instrumented code. That's basically the same as the current policy files do, except they explicitly grant the permissions to all the jar files we know about, whereas my patch grants them to all code bases the JVM knows about. (This also simplifies the policy files a bit, since we now only need one grant rule that covers all code bases, instead of one grant rule per code base.)

          I ran "ant -DderbyTesting.oldReleasePath=... emma-all" with my attached patch and Dag's initial patch for SecureServerTest. I saw two failures in RuntimeInfoTest, but it sounds like Dag already has a fix for that test in his sandbox (by setting the emma.verbosity.level property?).

          I also had to make a small change to RoutineSecurityTest. It contained negative test cases that tried to get the value of various system properties from within a stored procedure. Among these properties were user.dir, on which EMMA requires a read permission. Since EMMA's permissions are now granted to a wider set of code bases, getting that property now unexpectedly succeeded.

          Because of this, the patch skips the negative test case for user.dir. There are negative test cases for many other system properties, so I think RoutineSecurityTest still tests what it intended to test. Also, as a compensation, I enabled the negative test case for the user.home property in the same test. That test case has been disabled since the test was first checked in, but there is no comment saying why it's disabled, and it didn't fail in my environment (tested from both classes and jars, since they have different permissions granted to them, and also both sane and insane).

          Show
          Knut Anders Hatlen added a comment - Thanks for that info, Dag! So it seems that the problem is that the JVM's runtime libraries don't have the permissions required by the EMMA code, and the lack of doPrivileged blocks in EMMA requires all code to have those permissions (except if Derby code that contains doPrivileged blocks itself is on the stack and limits the privilege checking). Based on that observation, I tried a different approach that didn't require modifying the EMMA source (see d5514-emma-permissions-to-all.diff). Instead, I changed the policy files to grant the permissions required by EMMA to all code bases when running with the instrumented code. That's basically the same as the current policy files do, except they explicitly grant the permissions to all the jar files we know about, whereas my patch grants them to all code bases the JVM knows about. (This also simplifies the policy files a bit, since we now only need one grant rule that covers all code bases, instead of one grant rule per code base.) I ran "ant -DderbyTesting.oldReleasePath=... emma-all" with my attached patch and Dag's initial patch for SecureServerTest. I saw two failures in RuntimeInfoTest, but it sounds like Dag already has a fix for that test in his sandbox (by setting the emma.verbosity.level property?). I also had to make a small change to RoutineSecurityTest. It contained negative test cases that tried to get the value of various system properties from within a stored procedure. Among these properties were user.dir, on which EMMA requires a read permission. Since EMMA's permissions are now granted to a wider set of code bases, getting that property now unexpectedly succeeded. Because of this, the patch skips the negative test case for user.dir. There are negative test cases for many other system properties, so I think RoutineSecurityTest still tests what it intended to test. Also, as a compensation, I enabled the negative test case for the user.home property in the same test. That test case has been disabled since the test was first checked in, but there is no comment saying why it's disabled, and it didn't fail in my environment (tested from both classes and jars, since they have different permissions granted to them, and also both sane and insane).
          Hide
          Dag H. Wanvik added a comment -

          Thanks, Knut! Good if we can get away without changing the Emma source. Yes, added "-Demma.verbosity.level=silent" to RuntimeInfoTest#RuntimeinfoCmd and RuntimeinfoLocaleCmd. Seems we are good to code for the emma-all target, then. Btw, the ANT_OPTS may not be required as you said offline, since ant starts each suite separately.

          Show
          Dag H. Wanvik added a comment - Thanks, Knut! Good if we can get away without changing the Emma source. Yes, added "-Demma.verbosity.level=silent" to RuntimeInfoTest#RuntimeinfoCmd and RuntimeinfoLocaleCmd. Seems we are good to code for the emma-all target, then. Btw, the ANT_OPTS may not be required as you said offline, since ant starts each suite separately.
          Hide
          Dag H. Wanvik added a comment -

          Uploading patch derby-5514-2, which

          • fixes the usa of String#contains,
          • adds -Demma.verbosity.level=silent to RuntimeInfoTest
          • adds the convenience method runsWithEmma to BaseTestCase
          • adds fixes to NetworkServerTestSetup to
            a) always use Emma verbosity silent when spawning a server in separate VM, and also
            b) refrains from starting the server with the security manager when running with Emma since the default server policy doesn't contain permissions for Emma, and finally
          • skips the assertion for the security manager being used in
            SecureServerTest (it is not, see preceding item)

          With these fixes and my patch to Emma (it will not be required when Knut's fixes to the policy files are added), "ant emma-all" completed successfully (see https://people.apache.org/~dag/coverage.html).

          I notice emma-all doesn't contain the following tests which we may consider including:

          • LobLimitsLiteTest
          • StressMultiTest
          • i18n._Suite
          • ReplicationSuite

          which are all run as part of PackagesAll in the regressions.

          Show
          Dag H. Wanvik added a comment - Uploading patch derby-5514-2, which fixes the usa of String#contains, adds -Demma.verbosity.level=silent to RuntimeInfoTest adds the convenience method runsWithEmma to BaseTestCase adds fixes to NetworkServerTestSetup to a) always use Emma verbosity silent when spawning a server in separate VM, and also b) refrains from starting the server with the security manager when running with Emma since the default server policy doesn't contain permissions for Emma, and finally skips the assertion for the security manager being used in SecureServerTest (it is not, see preceding item) With these fixes and my patch to Emma (it will not be required when Knut's fixes to the policy files are added), "ant emma-all" completed successfully (see https://people.apache.org/~dag/coverage.html ). I notice emma-all doesn't contain the following tests which we may consider including: LobLimitsLiteTest StressMultiTest i18n._Suite ReplicationSuite which are all run as part of PackagesAll in the regressions.
          Hide
          Knut Anders Hatlen added a comment -

          Committed d5514-emma-permissions-to-all.diff with revision 1206409.

          Show
          Knut Anders Hatlen added a comment - Committed d5514-emma-permissions-to-all.diff with revision 1206409.
          Hide
          Knut Anders Hatlen added a comment -

          I ran suites.All (which includes the above mentioned test suites that are not part of emma-all/junit-all) successfully with derby-5514-2.diff and instrumented jars. +1 to commit.

          Show
          Knut Anders Hatlen added a comment - I ran suites.All (which includes the above mentioned test suites that are not part of emma-all/junit-all) successfully with derby-5514-2.diff and instrumented jars. +1 to commit.
          Hide
          Dag H. Wanvik added a comment -

          Committed derby-5514-2 as svn 1207471, resolving.

          Show
          Dag H. Wanvik added a comment - Committed derby-5514-2 as svn 1207471, resolving.
          Hide
          Knut Anders Hatlen added a comment -

          [bulk update] Close all resolved issues that haven't been updated for more than one year.

          Show
          Knut Anders Hatlen added a comment - [bulk update] Close all resolved issues that haven't been updated for more than one year.

            People

            • Assignee:
              Dag H. Wanvik
              Reporter:
              Dag H. Wanvik
            • Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development