Re: RFR: JDK-8210274: Source Launcher should work with a security manager
On 9/11/18 12:34 PM, Alan Bateman wrote: What are the implications for uses of javax.tools and com.sun.tools.javac.Main in code running with a security manager? Maybe that is a separate project but I would have expected to see privileged blocks in places that need permissions. The intent was to stay clear (in this changeset) of all other ways of invoking javac, meaning javax.tools, com.sun.tools.javac.Main and spi.ToolProvider. While there are relatively few ways to invoke javac, these ways all permit the use of annotation processors and other callbacks, and so we would need privileged blocks in all places where callbacks could re-enter javac. That is what I was wondering about it. I don't see a queue at the door of developers looking to run javac with a security manager but at the same time it is possible to configure many app servers with JSP support to run with a security manager. I assume they must have historically granted at least some permissions to "tools.jar" for this to work. Probably. Also they might also wrap their call to javac with doPrivileged. If we were to drop the support for a security manager from the source launcher, would you still consider it worthwhile to update the policy file to enumerate the privileges required to run javac? Setting aside the updates for the Source Launcher tests, I think the work to improve all the other tests to function better when a security manager is involved is also worth while. It took a lot of work in JDK 9 to identify the minimum permissions needed by several modules. The java.xml.bind and java.xml.ws modules took a lot of effort because of the callbacks and missing privileged blocks. It does take a lot of effort and testing to be confident. So my personal view (and not my call) is that it's probably not high priority to do the same for jdk.compiler at this time. I see this changeset is to get the source launcher to work with security manager so that launching in class file mode and source mode can specify the same runtime options. You are right that it'd be a lot of effort to get jdk.compiler to work with callbacks and missing privileged blocks. The minimum permissions are good for source launcher support. If we were to drop the support for a security manager from the source launcher, I think jdk.compiler would need AllPermission due to callback. Mandy
Re: RFR: JDK-8210274: Source Launcher should work with a security manager
On 09/11/2018 12:53 PM, Sean Mullan wrote: I have looked over the changes and they look reasonable, though I am not very familiar with this code. I was wondering, when running with the PermissiveTestSecurityManager did you also have to enable security debugging (ex: java.security.debug=access) so that you log the permissions that were required? If so, it might be helpful to add that to the comments in the test. If not, how did you figure that out? - it's not immediately apparent when looking at the code. --Sean Sean, Thanks for looking at this. I did not need to enable any security debugging when using the PermissiveTestSecurityManager. For the most part, the basic security infrastructure was good enough by itself, since it reported enough information in the SecurityExceptions to be able to easily determine the missing but required permissions. It helped to have a sense of what permissions might be required, such file access, system properties, and permissions for class loaders and reflections in some limited parts of javac, and the corresponding tests in the test suite. The most "tedious" part was just running the tests until all the issues had been found and fixed, but that being said, the overall process converged pretty quickly. I will note that PermissiveTestSecurityManager arrived late in the game for this work. For the most part, I was using the plain standard security manager, and was adding permissions for tests as needed in a custom policy file that I also specified on the jtreg command line. That work could never have been checked in, since it involved lots of host-specific paths in the additional policy file. It was only later on that I came up with the idea of using first a custom security manager, and from there, the idea of using a custom policy in the custom security manager. The use of PermissiveTestSecurityManager made it much faster to find and fix all remaining issues and enabled me to achieve the goal of getting all javac tests to work, instead of settling for most tests. (I had previously been prepared to set aside and ignore the main block of annotation processing tests.) -- Jon
Re: RFR: JDK-8210274: Source Launcher should work with a security manager
I have looked over the changes and they look reasonable, though I am not very familiar with this code. I was wondering, when running with the PermissiveTestSecurityManager did you also have to enable security debugging (ex: java.security.debug=access) so that you log the permissions that were required? If so, it might be helpful to add that to the comments in the test. If not, how did you figure that out? - it's not immediately apparent when looking at the code. --Sean On 9/10/18 4:37 PM, Jonathan Gibbons wrote: Please review a patch to have the Source Launcher be able to work when a security manager is enabled. There are 4 parts to the work: an update to the default policy file, an update to the source launcher itself, updates to tests, and a custom security manager to help with testing. 1. src/java.base/share/lib/security/default.policy Add reasonably fine-grain permissions to give javac the permissions it needs to run when a security manager is enabled. These permissions were determined by running all javac tests under with a special security manager installed: more on that later. There is one anomalous property which was detected, which is a property to control some internal javac behavior. Arguably, it should eventually be renamed with at least a "javac.*" prefix. 2. src/jdk.compiler/share/classes/com/sun/tools/javac/launcher/Main.java Update the source launcher to use doPrivileged in a couple of places: most notably around the main invocation of javac. Generally, it continues to be the policy that command-line tools like javac, javadoc do not use fine-grained doPrivileged blocks throughout the tool, and this update does not change that. It is worth noting that when javac is invoked from the Source Launcher, it does not support callbacks like annotation processors, task listeners, and other plugin code, so that eliminates any issues relating to using callbacks from code in a doPrivileged block. 3. test/langtools/tools/lib/security/PermissiveTestSecurityManager.java This is a custom new security manager for manual testing purposes in conjunction with jtreg, so that we can run a large subset of tests under the influence of a security manager. . It is a design characteristic of jtreg that almost all tests have their own distinct codebase, and so it is impractical to use a modified policy file to grant all necessary permissions to all necessary tests, so this security manager installs a custom Policy object that grants permissions to test code while deferring to the system policy for everything else (and for javac in particular.) Using this security manager, it is possible to run all the javac tests to detect with high probability all the permissions that it requires. Using a custom security manager to install a custom Policy object allows the feature to be easily enabled on the jtreg command line. There is one workaround in this security manager: a bug was uncovered such that the jdk.zipfs module needs permission to access the user.dir system property. ( https://bugs.openjdk.java.net/browse/JDK-8210469 ) A temporary workaround is used to get a clean test run. Note: this code is not directly used in any normal/automated test runs for javac; nevertheless, it is deemed worthwhile to save the code for use in similar situations in the future. 4. Minor updates to tests. Some new test cases are added to Source Launcher tests. Although most tests are not directly affected by the use of a security manager, there are some exceptions. Some tests manipulate the security manager and/or expect the security manager to be unset. These tests are modified to "pass by default" if the runtime conditions are not suitable (i.e. a security manager has been installed.) Although it is not a requirement to be able to run annotation processing tests when a security manager is enabled (because that condition cannot arise with the Source Launcher), most annotation processing tests do run successfully. There were 3 exceptions, where the test required more permissions than granted to javac in code being called from javac. These permissions were to access test.* system properties and the setIO runtime permission. Rather than grant unnecessary properties to javac, it was enough to use doPrivileged in a couple of places in the javac "toolbox" test library, and to refactor those tests to better use toolbox. In conjunction with these changes,ignoring any tests on the ProblemList, all javac tests pass, with and without a security manager being present. JBS: https://bugs.openjdk.java.net/browse/JDK-8210274 Webrev: http://cr.openjdk.java.net/~jjg/8210274/webrev.00/ -- Jon
Re: RFR: JDK-8210274: Source Launcher should work with a security manager
Alan, Thanks for all the feedback. I'll add the extra test case you suggest. -- Jon On 09/11/2018 12:34 PM, Alan Bateman wrote: On 11/09/2018 19:42, Jonathan Gibbons wrote: : As regards the interaction between Source Launcher and the use of a security manager, I see 3 possibilities: 1. Specifically support it, as provided in this webrev 2. No code change, but update JEP 330 to specify the behavior 3. Explicitly reject the combination of Source Launcher and the security manager. Since you've started into this then it's probably best to just continue with #1 and get it done. My main point here was the test in the webrev sets the security manager on the command line and I think you also need another test that sets in the test main method. Is there any way (spi.ToolProvider or some means) for untrusted code to indirectly run the source launcher? This question is important because the updated source launcher could be abused to probe anywhere on the file system. Untrusted code can only run the source launcher by breaking encapsulation on the command line. The source launcher is a combination of native code in the launcher itself and a class in an unexported package of jdk.compiler. Even if you could access the source launcher, the compilation command line (i.e. the args for the internal call to javac) is highly constrained, and so it is difficult to see how the source launcher could be abused. Thanks, the question had to be asked because the privileged block in the source launcher main means it can access any file. What are the implications for uses of javax.tools and com.sun.tools.javac.Main in code running with a security manager? Maybe that is a separate project but I would have expected to see privileged blocks in places that need permissions. The intent was to stay clear (in this changeset) of all other ways of invoking javac, meaning javax.tools, com.sun.tools.javac.Main and spi.ToolProvider. While there are relatively few ways to invoke javac, these ways all permit the use of annotation processors and other callbacks, and so we would need privileged blocks in all places where callbacks could re-enter javac. That is what I was wondering about it. I don't see a queue at the door of developers looking to run javac with a security manager but at the same time it is possible to configure many app servers with JSP support to run with a security manager. I assume they must have historically granted at least some permissions to "tools.jar" for this to work. If we were to drop the support for a security manager from the source launcher, would you still consider it worthwhile to update the policy file to enumerate the privileges required to run javac? Setting aside the updates for the Source Launcher tests, I think the work to improve all the other tests to function better when a security manager is involved is also worth while. It took a lot of work in JDK 9 to identify the minimum permissions needed by several modules. The java.xml.bind and java.xml.ws modules took a lot of effort because of the callbacks and missing privileged blocks. It does take a lot of effort and testing to be confident. So my personal view (and not my call) is that it's probably not high priority to do the same for jdk.compiler at this time. -Alan
Re: RFR: JDK-8210274: Source Launcher should work with a security manager
On 11/09/2018 19:42, Jonathan Gibbons wrote: : As regards the interaction between Source Launcher and the use of a security manager, I see 3 possibilities: 1. Specifically support it, as provided in this webrev 2. No code change, but update JEP 330 to specify the behavior 3. Explicitly reject the combination of Source Launcher and the security manager. Since you've started into this then it's probably best to just continue with #1 and get it done. My main point here was the test in the webrev sets the security manager on the command line and I think you also need another test that sets in the test main method. Is there any way (spi.ToolProvider or some means) for untrusted code to indirectly run the source launcher? This question is important because the updated source launcher could be abused to probe anywhere on the file system. Untrusted code can only run the source launcher by breaking encapsulation on the command line. The source launcher is a combination of native code in the launcher itself and a class in an unexported package of jdk.compiler. Even if you could access the source launcher, the compilation command line (i.e. the args for the internal call to javac) is highly constrained, and so it is difficult to see how the source launcher could be abused. Thanks, the question had to be asked because the privileged block in the source launcher main means it can access any file. What are the implications for uses of javax.tools and com.sun.tools.javac.Main in code running with a security manager? Maybe that is a separate project but I would have expected to see privileged blocks in places that need permissions. The intent was to stay clear (in this changeset) of all other ways of invoking javac, meaning javax.tools, com.sun.tools.javac.Main and spi.ToolProvider. While there are relatively few ways to invoke javac, these ways all permit the use of annotation processors and other callbacks, and so we would need privileged blocks in all places where callbacks could re-enter javac. That is what I was wondering about it. I don't see a queue at the door of developers looking to run javac with a security manager but at the same time it is possible to configure many app servers with JSP support to run with a security manager. I assume they must have historically granted at least some permissions to "tools.jar" for this to work. If we were to drop the support for a security manager from the source launcher, would you still consider it worthwhile to update the policy file to enumerate the privileges required to run javac? Setting aside the updates for the Source Launcher tests, I think the work to improve all the other tests to function better when a security manager is involved is also worth while. It took a lot of work in JDK 9 to identify the minimum permissions needed by several modules. The java.xml.bind and java.xml.ws modules took a lot of effort because of the callbacks and missing privileged blocks. It does take a lot of effort and testing to be confident. So my personal view (and not my call) is that it's probably not high priority to do the same for jdk.compiler at this time. -Alan
Re: RFR: JDK-8210274: Source Launcher should work with a security manager
On 9/11/18 12:58 AM, Alan Bateman wrote: On 10/09/2018 21:37, Jonathan Gibbons wrote: Please review a patch to have the Source Launcher be able to work when a security manager is enabled. It's not clear to me that this is an interesting use-case but in any case I think you've got two scenarios to test. One is setting java.security.manager on the command line, the other is the launched code's main method calling System.setSecurityManager which amounts to setting a security manager in a running VM. You might want to add a test case for the latter. I agree that this may not be a very interesting use case, and I have no strong feelings about whether or not to support it, except to say that I think we should specify the interaction between Source Launcher and the use of a security manager. I note that this piece of work was triggered by the recent change to support the getResource* family of methods in the classloader used to run the user classes. At that time, it was noted that creating the URL involved directly calling a method that needed a privilege to be available (NetPermission specifyStreamHandler) (as compared to all the permissions indirectly required when invoking the compiler.) As regards the interaction between Source Launcher and the use of a security manager, I see 3 possibilities: 1. Specifically support it, as provided in this webrev 2. No code change, but update JEP 330 to specify the behavior 3. Explicitly reject the combination of Source Launcher and the security manager. Is there any way (spi.ToolProvider or some means) for untrusted code to indirectly run the source launcher? This question is important because the updated source launcher could be abused to probe anywhere on the file system. Untrusted code can only run the source launcher by breaking encapsulation on the command line. The source launcher is a combination of native code in the launcher itself and a class in an unexported package of jdk.compiler. Even if you could access the source launcher, the compilation command line (i.e. the args for the internal call to javac) is highly constrained, and so it is difficult to see how the source launcher could be abused. What are the implications for uses of javax.tools and com.sun.tools.javac.Main in code running with a security manager? Maybe that is a separate project but I would have expected to see privileged blocks in places that need permissions. The intent was to stay clear (in this changeset) of all other ways of invoking javac, meaning javax.tools, com.sun.tools.javac.Main and spi.ToolProvider. While there are relatively few ways to invoke javac, these ways all permit the use of annotation processors and other callbacks, and so we would need privileged blocks in all places where callbacks could re-enter javac. -Alan If we were to drop the support for a security manager from the source launcher, would you still consider it worthwhile to update the policy file to enumerate the privileges required to run javac? Setting aside the updates for the Source Launcher tests, I think the work to improve all the other tests to function better when a security manager is involved is also worth while. -- Jon
Re: RFR: JDK-8210274: Source Launcher should work with a security manager
On 10/09/2018 21:37, Jonathan Gibbons wrote: Please review a patch to have the Source Launcher be able to work when a security manager is enabled. It's not clear to me that this is an interesting use-case but in any case I think you've got two scenarios to test. One is setting java.security.manager on the command line, the other is the launched code's main method calling System.setSecurityManager which amounts to setting a security manager in a running VM. You might want to add a test case for the latter. Is there any way (spi.ToolProvider or some means) for untrusted code to indirectly run the source launcher? This question is important because the updated source launcher could be abused to probe anywhere on the file system. What are the implications for uses of javax.tools and com.sun.tools.javac.Main in code running with a security manager? Maybe that is a separate project but I would have expected to see privileged blocks in places that need permissions. -Alan
