[icedtea-web] RFC: do not check INDEX.LIST for being signed

[Deepak Bhole]

* Omair Majid <omajid at redhat.com> [2011-03-24 10:36]:
> On 03/23/2011 06:00 PM, Dr Andrew John Hughes wrote:
> >On 19:28 Fri 11 Feb     , Omair Majid wrote:
> >>Hi,
> >>
> >>The attached patch modifies JarSigner so that we do not verify the jar
> >>index.
> >>
> >>There are some applications that contain jars with all entries except
> >>the jar index signed. See
> >>https://bugzilla.redhat.com/show_bug.cgi?id=675271 for an example.
> >>
> >>The jar index contains a list of jars and packages inside them. Our
> >>classloader uses it to look up where (in the same domain) it might look
> >>for additional jars if some classes can not be found. The jar index does
> >>not say anything about those particular jars being signed, nor does it
> >>contain any signatures for those classes. The effect of the jar index
> >>being modified should be the same as the archive tag in an applet tag
> >>being modified (or the jar element in a jnlp file being modified) - and
> >>we dont verify jnlp files or web pages as being signed.
> >>
> >>More information about the jar index can be found at [1].
> >>
> >>All in all, I dont think not verifying signatures on jar index will have
> >>any security impact. If no one has issues with the patch, I would like
> >>to add it to icedtea-web HEAD.
> >>
> >>Thoughts? Comments?
> >>
> >>Cheers,
> >>Omair
> >>
> >>[1]
> >>http://download.oracle.com/javase/6/docs/technotes/guides/jar/jar.html#JARIndex
> >
> >Technically this looks ok.  Again, I would simplify the if test to just return
> >the boolean value directly.  However, I'm not sure about the security implications
> >which is why I was hoping someone else was going to comment.
> >
> 
> Deepak, what are your thoughts on this? As explained above, it
> should not have any security impact: INDEX.LIST contains a list of
> jars which will be downloaded and verified, just like all other
> jars. On the other hand, I can certainly see your point if you think
> this is an ugly hack and the application should be fixed instead. I
> have not seen any other jars that have all entries other than
> INDEX.LIST signed, so this could be just a one-time human error.
>

Looking at the current code, the jars that INDEX.LIST contains seem to
be downloaded via loadClass() in JNLPClassLoader. When do we do
verification on these?

Cheers,
Deepak
 
> Thanks,
> Omair
> 
> >>diff -r c0d4bd69b8f7 netx/net/sourceforge/jnlp/tools/JarSigner.java
> >>--- a/netx/net/sourceforge/jnlp/tools/JarSigner.java	Tue Feb 08 16:51:56 2011 -0500
> >>+++ b/netx/net/sourceforge/jnlp/tools/JarSigner.java	Tue Feb 08 17:18:23 2011 -0500
> >>@@ -277,7 +277,8 @@
> >>                      anySigned |= isSigned;
> >>
> >>                      boolean shouldHaveSignature = !je.isDirectory()
> >>-&&  !signatureRelated(name);
> >>+&&  !signatureRelated(name)
> >>+&&  !isIndex(name);
> >>
> >>                      hasUnsignedEntry |= shouldHaveSignature&&   !isSigned;
> >>
> >>@@ -457,6 +458,14 @@
> >>          return false;
> >>      }
> >>
> >>+    private boolean isIndex(String name) {
> >>+        String ucName = name.toUpperCase();
> >>+        if (ucName.equals(META_INF + "INDEX.LIST")) {
> >>+            return true;
> >>+        }
> >>+        return false;
> >>+    }
> >>+
> >>      /**
> >>       * Check if userCert is designed to be a code signer
> >>       * @param userCert the certificate to be examined
> >
> >
>