[RFC[PATCH]: Patch fix for algorithm that verifies signed JNLP file s
Dr Andrew John Hughes
ahughes at redhat.com
Tue Aug 2 22:53:42 PDT 2011
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
> * 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.
--
Andrew :)
Free Java Software Engineer
Red Hat, Inc. (http://www.redhat.com)
Support Free Java!
Contribute to GNU Classpath and IcedTea
http://www.gnu.org/software/classpath
http://icedtea.classpath.org
PGP Key: F5862A37 (https://keys.indymedia.org/)
Fingerprint = EA30 D855 D50F 90CD F54D 0698 0713 C3ED F586 2A37
More information about the distro-pkg-dev
mailing list