[rfc][icedtea-web] policytool in itweb-settings

[Jiri Vanek]

On 01/15/2014 12:34 AM, Jacob Wisor wrote:
> On 01/14/2014 07:16 PM, Andrew Azores wrote:
>> On 01/14/2014 10:34 AM, Jiri Vanek wrote:
>>> On 01/14/2014 03:16 PM, Jacob Wisor wrote:
>>>> On 01/14/2014 01:42 PM, Jiri Vanek wrote:
>>>>> On 01/14/2014 12:33 AM, Jacob Wisor wrote:
>>>>>> Hello there!
>>>>>>
>>>>>> On 01/13/2014 11:20 PM, Andrew Azores wrote:
>>>>>>> Hi,
>>>>>>>
>>>>>>> This small patch hooks the JDK policytool into itweb-settings. It can then be
>>>>>>> used to set up a custom user-level JNLP policy - this, in combination with
>>>>>>> the
>>>>>>> Run in Sandbox patch, will allow for quite a lot more flexibility in how
>>>>>>> permissions are handled with signed applets/applications.
>>>>>>>
>>>>>>> A nicer, more user-friendly editor to replace the policytool will
>>>>>>> hopefully come
>>>>>>> later on.
>>>>>>
>>>>>> Oooooooh yes, please! This would be awesome! :-)
>>>>>
>>>>> Yes this would be :))
>>>>> But it is different task. And Quite complex. Especially it must pass upstream
>>>>> (openjdk). And that is *the* task!
>>>>
>>>> Well, it does not need to replace or displace OpenJDK's policytool. It should
>>>> be probably enough
>>>> that it complements it. ;-) You know, it's neither forbidden nor against any
>>>> spec to build
>>>> augmenting tools around OpenJDK.
>>>
>>> But still contribute to Openjdk itself is the right thing to do.
>>>
>>> ...
>>>>
>>>> At first, I thought that the problem would rather be that some system
>>>> configurations may be missing
>>>> a PATH environment variable entry to policytool and thus launching it may
>>>> fail. But, Jiri is right.
>>>> The best approach here would probably be to call directly into policytool's
>>>> main() method with
>>>> user.home as its current working directory. policytool is part of rt.jar, not
>>>> tools.jar, so it
>>>> should already be on bootclasspath. But, you may have to investigate deeper
>>>> into it because the
>>>
>>> Great!
>>>
>>> But I doubt "with  user.home as its current working directory" is possible.
>>> Or do we misunderstand each other?
>>>
>>> I had in my mind simple PolicyTool.main(arg,aerg,arg) call in itw-settings.
>>>
>>>
>>>> package names of some tools have changed from OpenJDK 6 over to OpenJDK 7.
>>>
>>> I doubt policy tool was touched in last 10 years ;)
>>>
>>> J.
>>>
>>
>> [...]
>>
>> The actual PolicyPanel is now launching the PolicyTool by invoking its main
>> method, rather than exec'ing a new process.
>
> After I have hit the send button for my last e-mail, I have realized that it may be desirable to run
> policytool in a separate process for security reasons, although calling directly into
> PolicyTool.main(). Maybe it's just me being paranoid, but authoring policies is a security sensitive
> task. In the current patch policytool gets loaded into the same JVM as itw-settings. If the current
> JVM or itw-settings has been compromised for the current user, he/she may be editing a heck of
> secure policy but could end up with a policy file on disk, granting AllPermissions to some malicious
> code bearing URL. Sure, policy files are simple human readable text files that can be checked before
> applying. But, who really does that?
> Of course, I do realize that if one's system JRE has been compromised then probably all other JVM
> instances and the applications running on them are going to get compromised sooner or later on that
> system. I don't know, I just wanted to share some wired thoughts on policy authoring security. :-D

Ugh. I hope this is not true. But Ido not know.
Btw - by specifying policies - the itw-settings will be still unaffected correct?
>
>> I've put this into a new thread, but tbh I'm not sure if that was entirely necessary.
>
> In this case, I had rather something like SwingUtilities.invokeLater() or EventQueue.invokeLater()
> in mind. In general, only activities related to manipulating and displaying UI elements should be
> placed on the AWT thread. Work code should either be put into invokeLater() or custom working
> threads (like SwingWorker for example), although the later has some more caveats in most cases.
> invokeLater() actually does indeed create a thread too but it also neatly ties it into the UI
> toolkit and takes care of most of those caveats, so developers are really advised to use it. You may
> want to read the tiny discussion on "Swing's Threading Policy" at the bottom of javax.swing's
> package API documentation regarding this topic. ;-)

+1!

>
>> The "View Policy" button has a tooltip now that indicates the location of the file that will be
>> opened,
>> although this is also displayed in the policytool itself if you launch it.
>
> Right, that's actually why I have remained silent on that matter. It's a good move to indicate the
> policy file's location before launching policytool.
>
>> The View Button's action listener is no longer an anonymous inner class, since I
>> don't really mind the style either way personally.
>
> Yey, I love inner classes! :-D Declaring a member variable holding the ActionListener's reference
> would have been enough too, but hey, going all the way through is even cooler. 8-)
>
>> Its implementation does have an anonymous inner Runnable inside its Thread, though.
>
> An anonymous Runnable implementation without keeping a reference to it is okay here because in any
> case, there can only be one Runnable object per Thread object, hence a reference to that anonymous
> Runnable object is always unambiguously retrievable. Nevertheless, you may want to prefer
> invokeLater() over that custom thread.

as above
>
>> Oh, and there's also now an error dialog if for some reason the policy file can't be opened (eg it
>> exists
>> but you don't have read permission, or it exists but is a directory, or
>> whatever). If the file doesn't already exist then we attempt to create a blank
>> one there first, since the PolicyTool itself doesn't seem to do this.
>
> Although all JOptionPane.showXxxDialog() dialogs are modal, please pass it a reference to
> itw-settings' JFrame for the sake of completeness, before it grabs some undesired /default/
> Compenent - what ever this might be - or perhaps even the desktop window.
+1

>
> I hope you are sure about what you have been probably intending to accomplish with
> canOpenPolicyFile() actually does what you had in mind. File.canRead() and File.canWrite() are not
> platform agnostic and they are only checking file system access permissions, not physical read/write
> capability. Those methods are so misleading in their semantics and should not have found their way
> into the Java API in the first place. The JCP board would be well advised to deprecate these methods
> as soon as possible for being highly platform dependent and having misleading semantics. I have
> already covered this in an earlier post to this mailing list. ;-)
> Besides File.canRead() and File.canWrite() also test for existence so you may want to simplify and
> drop some redundant tests in you current code. I may be mistaken, but there may already be code
> present in net.sourceforge.jnlp.util.FileUtils for your intended purpose.
> IMHO, you should probably drop these tests anyway because it's on the policytool to do those tests
> (from a software engineering point of view).

This have already appeared, Jacob, have not? :o)

+    public static boolean canOpenPolicyFile(final File policyFile) throws IOException {
+        FileUtils.createParentDir(policyFile);
+        if (!policyFile.exists()) {
+            policyFile.createNewFile();
+        }
+        return policyFile.isFile() && policyFile.canRead() && policyFile.canWrite();
+    }


For a directory you can use DirectroyValidator class in itw. It do what you wont to do in full scale.

imho the policyFile.canWrite(); is not necessary (the message "can not write" is worthy anyway )  . 
The toll is useful also for read only viewing..
The messages "is not file" or "can not be read" or "cannot be written" are better then one generic one.

Thanx both of you for contributions!


J.