Marco Trudel wrote:
> Hello list
>
> I ran into some trouble when verifying signed jars:
> 1. it didn't work for classes with long names in jars
> 2. it was extremly slow
>
> 1.
> Some (or all) Manifests seem to break lines longer than 70 characters
> into multiple lines. The break is not always fix at 70 chars, I've seen
> entries where the break is later. The following line(s) then start with
> a space.
> I can see that already in the header of my original
> jface_3.2.0.I20060605-1400.jar (Manifest attached).
> The same applies for Name/Digest pairs (the name only). So they might
> look like this:
>
> Name: org/eclipse/jface/bindings/keys/KeySequenceText$TraversalFilter.
> class
> SHA1-Digest: DxKojrJckHaMCrOyg+hgiOPi1PI=
>
> Because such entries weren't supported, JarFile always told that these
> files aren't signed. My patch adds support for that.
>
>
> 2.
> Parsing the signatures of big Manifests was extremely slow. I assume
> that's because it worked with single bytes and reparsed the whole
> manifest for every file.
> My patch changes that procedure. Now the manifest is parsed once with
> some regexp and the result is reused.
>
> Some little time experience on my computer:
> Sun needs 0.4s to check the validity of the eclipse jface jar.
> JarFile needs:
> - unpatched: 36 seconds when used in a Sun JVM
> - unpatched: 8 seconds when using a GCJ compiled exe
> - patched: 0.7 seconds when using in a Sun JVM
> - patched: 1 second when using in a GCJ compiled exe
>
> It's still really slow compared to Sun. But at least not 9000% slower
> (did I calculate that right?)...
>
Well, I wouldn't call that "really slow" -- that's a pretty huge
improvement. ISTR our regex implementation being slow, and our crypto
algs may not be optimized, either. Raif's native bignum patch may
improve this a bit.
> Another thing I encountered but had no time to fix:
>
> JarFile jarFile = new JarFile("something.jar");
> Enumeration entries = jarFile.entries();
> while(entries.hasMoreElements())
> {
> JarEntry entry = (JarEntry)entries.nextElement();
> InputStream stream = jarFile.getInputStream(entry);
> // read from the stream, that will parse the certificates
>
> // the following works with a sun jvm.
> // with classpath, one has to do the jarFile.entries() and loop
> // again to be able to read the certs (this will always give null)
> Certificate[] certs = entry.getCertificates();
> }
>
It makes sense why this wouldn't work -- the JarEntry created for the
nextElement clause is probably initialized with a null certificates
list, because at that point the entry hasn't been verified. JarEntry
uses a private certificates field to store this, IIRC, not a callback
into its jar file.
I suppose it should be easy enough to change JarEntry to call into its
containing JarFile for that entry's certificates, instead of using a
field in JarEntry.
> I also run into a regular expression bug, related to .* that included
> newlines although the pattern was not in DOTALL mode. But,
> unfortunately, I can't reproduce that anymore...
>
>
> Back to the patch. Comments? Hints?
>
Your approach seems OK, but a hand-coded parser may still be faster (if
only done once for the whole manifest, and if it uses a buffered stream,
instead of reading a byte at a time) than using regexps for it. I'd also
be wary of byte[]->String->byte[] conversions, because you are wrapping
each entry+hash in a String, then using String.getBytes() (what if the
default character set is not what you expect?). This byte representation
must match what's in the file exactly, because it is used to verify the
entry.
It may make sense instead to change the Manifest class to retain the
byte-level representation of each entry, along with the hash value,
instead of re-parsing the manifest like this -- having two parsers for
the same thing is always trouble.
I'm not sure why I didn't try using the existing Manifest class when I
wrote the signed Jar stuff originally.
> If there are no objections: Can someone commit that for me?
> A changelog entry will follow shortly (at the latest by tomorrow
> morning)...
>
>
> thanks
> Marco
>
> PS: What's with the online mail archive? It stopped a couple of days
> (weeks?) ago. It's really usefull having an updated archive online (for
> google and discussion referencce).
>
Maybe it went on vacation along with Mark ;-)
Cheers.
