[RFC[PATCH]: Patch fix for algorithm that verifies signed JNLP file s
Saad Mohammad
smohammad at redhat.com
Wed Aug 3 07:29:17 PDT 2011
On 08/03/2011 01:53 AM, Dr Andrew John Hughes wrote:
> On 17:06 Tue 02 Aug , Saad Mohammad wrote:
>> I have read all the comments and reviews on my previous patch that added the algorithm of checking signed JNLP files. I want to thank you all for your input, it was a big help to me.
>>
>> Since I have committed one of the patches, I have attached a new patch that addresses the issues we had with the previous patch.
>>
>> CHANGELOG:
>>
>> 2011-08-02 Saad Mohammad<smohammad at redhat.com>
>>
>> * netx/net/sourceforge/jnlp/JNLPMatcher.java:
>> (JNLPMatcher): Removed NullPointerException from being thrown, caught and
>> then thrown again via JNLPMatcherException. This was replaced by throwing
>> a checked exception [JNLPMatcherException] directly.
>> (JNLPMatcher): Removed unused code [getters]
>> (JNLPMatcher): Closed Input/Output streams that were opened.
>> (isMatch): Removed caching of return value
>> (closeInputStream): Added this method to close input streams
>> (closeOutputStream): Added this method to close output streama
Oops, I made a little typo there ^
s/streama/streams
>> * netx/net/sourceforge/jnlp/Node.java:
>> Removed getAttributeNames() method from the commented section
>>
>> FYI,
>> I have not attached the implementation of verifying signed JNLP file when launching the application
>> (Patch 2 from previous emails with subject: [RFC][PATCH][icedtea-web]: Added support for signed JNLP file- Updated Patch]).
>>
>> I have discovered some new changes that should be implemented:
>>
>> - The main jar file is ONLY checked for a signed JNLP file (It should not check other jar resource; just the jar with the main class)
>>
>> - As Omair pointed out, we have to handle "lazy" jars differently. At the moment, there is a bug that I will need to fix before I can continue: all 'lazy' jars are automatically considered unsigned by
>> IcedTea-Web (even ones with valid signatures)
>> [http://icedtea.classpath.org/bugzilla/show_bug.cgi?id=765]
>>
>> - Applications with a valid signed JNLP file have special security privileges and also allows special arguments to be passed though using "java-vm-args" attribute within the js2e element. I have also read
>> that special properties can be used with a signed JNLP file application. I am uncertain if there are any properties or vm arguments that IcedTea-Web has restricted unless the application has certain
>> permissions.
>> [http://forums.oracle.com/forums/thread.jspa?threadID=1359245&tstart=0]
>> diff -r 7668bf410571 netx/net/sourceforge/jnlp/JNLPMatcher.java
>> --- a/netx/net/sourceforge/jnlp/JNLPMatcher.java Tue Aug 02 11:05:47 2011 +0200
>> +++ b/netx/net/sourceforge/jnlp/JNLPMatcher.java Tue Aug 02 15:11:51 2011 -0400
>> @@ -38,10 +38,11 @@
>> package net.sourceforge.jnlp;
>>
>> import java.util.List;
>> +import java.io.InputStream;
>> import java.io.InputStreamReader;
>> +import java.io.OutputStream;
>> import java.io.PipedInputStream;
>> import java.io.PipedOutputStream;
>> -import java.util.ArrayList;
>> import java.util.Arrays;
>> import java.util.Collections;
>> import java.util.LinkedList;
>> @@ -59,7 +60,6 @@
>> private final Node appTemplateNode;
>> private final Node launchJNLPNode;
>> private final boolean isTemplate;
>> - private Boolean match;
>>
>> /**
>> * Public constructor
>> @@ -78,26 +78,33 @@
>> public JNLPMatcher(InputStreamReader appTemplate, InputStreamReader launchJNLP,
>> boolean isTemplate) throws JNLPMatcherException {
>>
>> + if (appTemplate == null&& launchJNLP == null)
>> + throw new JNLPMatcherException(
>> + "Template JNLP file and Launching JNLP file are both null.");
>> + else if (appTemplate == null)
>> + throw new JNLPMatcherException("Template JNLP file is null.");
>> + else if (launchJNLP == null)
>> + throw new JNLPMatcherException("Launching JNLP file is null.");
>> +
>> + //Declare variables for signed JNLP file
>> + PipedInputStream pinTemplate= null;
>> + PipedOutputStream poutTemplate= null;
>> +
>> + //Declare variables for launching JNLP file
>> + PipedInputStream pinJNLPFile = null;
>> + PipedOutputStream poutJNLPFile = null;
>> +
>> try {
>> -
>> - if (appTemplate == null&& launchJNLP == null)
>> - throw new NullPointerException(
>> - "Template JNLP file and Launching JNLP file are both null.");
>> - else if (appTemplate == null)
>> - throw new NullPointerException("Template JNLP file is null.");
>> - else if (launchJNLP == null)
>> - throw new NullPointerException("Launching JNLP file is null.");
>> -
>> XMLElement appTemplateXML = new XMLElement();
>> XMLElement launchJNLPXML = new XMLElement();
>>
>> // Remove the comments and CDATA from the JNLP file
>> - final PipedInputStream pinTemplate = new PipedInputStream();
>> - final PipedOutputStream poutTemplate = new PipedOutputStream(pinTemplate);
>> + pinTemplate = new PipedInputStream();
>> + poutTemplate = new PipedOutputStream(pinTemplate);
>> appTemplateXML.sanitizeInput(appTemplate, poutTemplate);
>>
>> - final PipedInputStream pinJNLPFile = new PipedInputStream();
>> - final PipedOutputStream poutJNLPFile = new PipedOutputStream(pinJNLPFile);
>> + pinJNLPFile = new PipedInputStream();
>> + poutJNLPFile = new PipedOutputStream(pinJNLPFile);
>> launchJNLPXML.sanitizeInput(launchJNLP, poutJNLPFile);
>>
>> // Parse both files
>> @@ -113,6 +120,14 @@
>> throw new JNLPMatcherException(
>> "Failed to create an instance of JNLPVerify with specified InputStreamReader",
>> e);
>> + } finally {
>> + // Close all stream
>> + closeInputStream(pinTemplate);
>> + closeOutputStream(poutTemplate);
>> +
>> + closeInputStream(pinJNLPFile);
>> + closeOutputStream(poutJNLPFile);
>> +
>> }
>> }
>>
>> @@ -122,11 +137,9 @@
>> * @return true if both JNLP files are 'matched', otherwise false
>> */
>> public boolean isMatch() {
>> -
>> - if (match == null)
>> - match = matchNodes(appTemplateNode, launchJNLPNode);
>> -
>> - return match;
>> +
>> + return matchNodes(appTemplateNode, launchJNLPNode);
>> +
>> }
>>
>> /**
>> @@ -241,32 +254,34 @@
>> }
>> return false;
>> }
>> -
>> - /**
>> - * Getter for application/template Node
>> +
>> + /***
>> + * Closes an input stream
>> *
>> - * @return the Node of the signed application/template file
>> + * @param stream
>> + * The input stream that will be closed
>> */
>> - public Node getAppTemplateNode() {
>> - return appTemplateNode;
>> + private void closeInputStream(InputStream stream) {
>> + if (stream != null)
>> + try {
>> + stream.close();
>> + } catch (Exception e) {
>> + e.printStackTrace(System.err);
>> + }
>> }
>>
>> - /**
>> - * Getter for launching application Node
>> + /***
>> + * Closes an output stream
>> *
>> - * @return the Node of the launching JNLP file
>> + * @param stream
>> + * The output stream that will be closed
>> */
>> - public Node getLaunchJNLPNode() {
>> - return launchJNLPNode;
>> - }
>> -
>> - /**
>> - * Getter for isTemplate
>> - *
>> - * @return true if a signed template is being used for matching; otherwise
>> - * false.
>> - */
>> - public boolean isTemplate() {
>> - return isTemplate;
>> + private void closeOutputStream(OutputStream stream) {
>> + if (stream != null)
>> + try {
>> + stream.close();
>> + } catch (Exception e) {
>> + e.printStackTrace(System.err);
>> + }
>> }
>> }
>> diff -r 7668bf410571 netx/net/sourceforge/jnlp/Node.java
>> --- a/netx/net/sourceforge/jnlp/Node.java Tue Aug 02 11:05:47 2011 +0200
>> +++ b/netx/net/sourceforge/jnlp/Node.java Tue Aug 02 15:11:51 2011 -0400
>> @@ -145,19 +145,6 @@
>> return children;
>> }
>>
>> - String[] getAttributeNames() {
>> - if (attributeNames == null) {
>> - List<String> list = new ArrayList<String>();
>> -
>> - for (Enumeration e = xml.enumerateAttributeNames(); e.hasMoreElements();)
>> - list.add(new String((String) e.nextElement()));
>> -
>> - attributeNames = list.toArray(new String[list.size()]);
>> -
>> - }
>> - return attributeNames;
>> - }
>> -
>> String getAttribute(String name) {
>> return tinyNode.getAttribute(name);
>> }
>>
> This seems to deal with all the issues I mentioned.
Excellent, thanks again for the feedback.
Does this patch look good to be pushed? Deepak and Jiri?
--
Cheers,
Saad Mohammad
More information about the distro-pkg-dev
mailing list