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

[Omair Majid]

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.

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
>
>