JDK-8091308 : Define fine-grained permissions for JavaFX
  • Type: Enhancement
  • Component: javafx
  • Sub-Component: other
  • Priority: P3
  • Status: Resolved
  • Resolution: Fixed
  • Submitted: 2014-11-13
  • Updated: 2016-05-05
  • Resolved: 2016-05-05
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
Related Reports
Blocks :  
Relates :  
Relates :  
Relates :  
Relates :  
We should consider defining fine-grained permissions for JavaFX for various operations which currently require an application to request AllPermissions:

Creating a transparent window

Various full-screen capabilities (enter FS from arbitrary code, change/disable the exit key, get arbitrary keyboard input)

Load an embedded font or a web font
Changeset: db7746323247 Author: kcr Date: 2015-12-10 08:23 -0800 URL: http://hg.openjdk.java.net/openjfx/9-dev/rt/rev/db7746323247

I filed JDK-8156078 to track the issue of failing to reset the property to false when access is denied.



New webrev: http://cr.openjdk.java.net/~kcr/8091308/webrev.03/ Delta between .02 and .03 (only FXPermission changed): http://cr.openjdk.java.net/~kcr/8091308/delta-02-03/ New docs: http://cr.openjdk.java.net/~kcr/8091308/docs.03/javafx/util/FXPermission.html

Thanks for your comments. 1) I checked the JDK code, looked at the docs, and also ran some tests. The two-arg constructor is present in almost all sub-classes of BasicPermission (e.g., AWTPermission, AudioPermission, AuthPermission, DelegationPermission, LinkPermission, PropertyPermission, and RuntimePermission), although most indicate that the args paramter is unused and should be set to null. Notably JAXBPermission only has a one-arg version so there is precedent for not including the two-arg constructor. The code that constructs a Permission object (in sun.security.provider.PolicyFile), based on entries in a security policy file, does so using reflection; as long as the "actions" argument is null (not present in the file), it first looks for a one-arg constructor and then a two-arg constructor. If "actions" is non-null then it only looks for the two-arg constructor. In testing this, the only way I can see the two-arg version being called is if someone were to create an entry in the policy file with a (bogus) second argument. permission javafx.util.FXPermission "accessClipboard"; // will call the one-arg version permission javafx.util.FXPermission "accessClipboard", "the action string"; // will call the two-arg version I don't have a problem leaving it out. It just means that the latter, bogus entry will be ignored (and will cause an error message to be printed to the console). We can add it later if there is a need. 2) Good point. I'll change to mention the system clipboard as you suggest 3) This is really a question about the existing implementation, since other than changing which permission is required, the logic itself is unchanged. To answer your question, though, this appears to be a bug in the initial implementation of this back in 8u40. There is code in GlassWindowEventHandler to notify the Stage via the StagePeerListener whenever the state of the property changes, but this isn't being called when access is denied. Similar logic for the fullScreen property works correctly (as of a recent bug fix), so this is a simple matter of calling the listener when access is denied. I will file a new bug for this, since it is independent of fine-grained permissions. A new webrev will be forthcoming after I finish testing the changes in #1 to ensure that there are no issues.

Nit-picky comments: 1) I agree with Sergey, the second constructor seems redundant at this point, and if so, it is probably best to not include it for now. 2) For 'accessClipboard', the description talks about the 'JavaFX clipboard' - perhaps this should instead say the 'system clipboard' or at least allude to the fact that this isn't some special JavaFX construct, so it has system-wide implications. 3) Is there any concern over the possibility of the Stage.alwaysOnTopProperty being in an inconsistent state with the actual WindowStage state? It looks like Stage will always accept the boolean value, but WindowStage will only change state if the SM allows it. The JavaDoc says it will be returned to false, but I don't see where that happens.

A good question. I added this constructor for symmetry with AWTPermission, but also because BasicPermission has it and I didn't check whether the code that parses the security policy files might expect it to be there.

I always wonder why we need "FXPermission(String name, String actions)" when we specify that actions should be null?

Webrev for review: http://cr.openjdk.java.net/~kcr/8091308/webrev.02/ The only new API is the new FXPermission class, and the most interesting part of that is the table of permission strings. Since this is easier to review by looking at the generated javadoc API doc page, I have uploaded that as well: http://cr.openjdk.java.net/~kcr/8091308/docs.02/javafx/util/FXPermission.html The main changes are to check the new specific permission rather than AllPermission in each place where we have a security check. I also updated the docs for methods that do a security check to indicate the specific permission that is needed. [~jgiles] the docs have not changed since version .01, which you reviewed back in December, but it's been long enough that a once over would be helpful. I have tested this with standalone apps and other internal tests, have filed JDK-8156039 as a follow-on bug to add new unit tests for the specific permissions.

Here is an incomplete, preliminary webrev. http://cr.openjdk.java.net/~kcr/8091308/webrev.00/ Here are the docs for the new class: http://cr.openjdk.java.net/~kcr/8091308/docs.00/javafx/util/FXPermission.html Notes: * API itself is complete -- there is only one new public class * javadoc -- new class is mostly done (need to replace "MIGHT BE DANGEROUS" with real text) * javadoc -- still need to add docs to existing methods that require a permission to indicate *what* permission is required (in lieu of AllPermission) * Implementation in graphics module is mostly done and can be looked at as an example of the changes.

To implement this I propose to define a new FXPermission class in javafx.base, similar to RuntimePermission (in java.base) or AWTPermission (in java.desktop). This class will document the permission strings used by any JavaFX module that needs them (currently only javafx.graphics and javafx.fxml, but later we could add others). public final class javafx.util.FXPermission extends BasicPermission Here is the preliminary list of proposed permissions: modifyFXMLClassLoader -- allow to set the default FXML class loader and access fields of system classes createRobot modifyPlatformDeviceRegistry -- not sure whether we need this one createTransparentWindow loadFont setWindowAlwaysOnTop fullScreenControl accessWindowList accessClipboard (for compat with 8u72, we also need to check the equivalent AWTPermssion)

We will look into implementing this for JDK 9.

In the absence of this an application is required to assert more privileges than they need, which violates the best practice of asserting only the needed privileges.