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?)...

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();
}

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?

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).
Index: classpath/java/util/jar/JarFile.java
===================================================================
--- classpath/java/util/jar/JarFile.java        (revision 117867)
+++ classpath/java/util/jar/JarFile.java        (working copy)
@@ -68,6 +68,8 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
 import java.util.zip.ZipEntry;
 import java.util.zip.ZipException;
 import java.util.zip.ZipFile;
@@ -599,6 +601,30 @@
         validCerts.clear();
       }
 
+    // Read the manifest into a HashMap (String fileName, String entry)
+    // The fileName might be split into multiple lines in the manifest.
+    // Such additional lines will start with a space.
+    InputStream in = super.getInputStream(super.getEntry(MANIFEST_NAME));
+    ByteArrayOutputStream baStream = new ByteArrayOutputStream();
+    byte[] ba = new byte[1024];
+    while(true)
+      {
+        int len = in.read(ba);
+        if(len < 0) break;
+        baStream.write(ba, 0, len);
+      }
+    in.close();
+
+    HashMap hmManifestEntries = new HashMap();
+    Pattern p = Pattern.compile("Name: (.+?\r?\n(?: .+?\r?\n)*)" +
+      ".+?-Digest: .+?\r?\n\r?\n");
+    Matcher m = p.matcher(baStream.toString());
+    while(m.find())
+      {
+        String fileName = m.group(1).replaceAll("\r?\n ?", "");
+        hmManifestEntries.put(fileName, m.group());
+      }
+
     // Phase 3: verify the signature file signatures against the manifest,
     // mapping the entry name to the target certificates.
     this.entryCerts = new HashMap();
@@ -614,7 +640,7 @@
             Map.Entry e2 = (Map.Entry) it2.next();
             String entryname = String.valueOf(e2.getKey());
             Attributes attr = (Attributes) e2.getValue();
-            if (verifyHashes(entryname, attr))
+            if (verifyHashes(entryname, attr, hmManifestEntries))
               {
                 if (DEBUG)
                   debug("entry " + entryname + " has certificates " + 
certificates);
@@ -727,34 +753,23 @@
    * @param entry The entry name.
    * @param attr The attributes from the signature file to verify.
    */
-  private boolean verifyHashes(String entry, Attributes attr)
+  private boolean verifyHashes(String entry, Attributes attr,
+    HashMap hmManifestEntries)
   {
     int verified = 0;
 
-    // The bytes for ENTRY's manifest entry, which are signed in the
-    // signature file.
-    byte[] entryBytes = null;
-    try
+    String stringEntry = (String)hmManifestEntries.get(entry);
+    if(stringEntry == null)
       {
-       ZipEntry e = super.getEntry(entry);
-       if (e == null)
-         {
-           if (DEBUG)
-             debug("verifyHashes: no entry '" + entry + "'");
-           return false;
-         }
-        entryBytes = readManifestEntry(e);
-      }
-    catch (IOException ioe)
-      {
-        if (DEBUG)
-          {
-            debug(ioe);
-            ioe.printStackTrace();
-          }
+        if (DEBUG) debug("could not find " + entry + " in manifest");
         return false;
       }
 
+    // The bytes for ENTRY's manifest entry, which are signed in the
+    // signature file.
+    byte[] entryBytes = stringEntry.getBytes();
+
+
     for (Iterator it = attr.entrySet().iterator(); it.hasNext(); )
       {
         Map.Entry e = (Map.Entry) it.next();
@@ -801,100 +816,6 @@
   }
 
   /**
-   * Read the raw bytes that comprise a manifest entry. We can't use the
-   * Manifest object itself, because that loses information (such as line
-   * endings, and order of entries).
-   */
-  private byte[] readManifestEntry(ZipEntry entry) throws IOException
-  {
-    InputStream in = super.getInputStream(super.getEntry(MANIFEST_NAME));
-    ByteArrayOutputStream out = new ByteArrayOutputStream();
-    byte[] target = ("Name: " + entry.getName()).getBytes();
-    int t = 0, c, prev = -1, state = 0, l = -1;
-
-    while ((c = in.read()) != -1)
-      {
-//         if (DEBUG)
-//           debug("read "
-//                 + (c == '\n' ? "\\n" : (c == '\r' ? "\\r" : 
String.valueOf((char) c)))
-//                 + " state=" + state + " prev="
-//                 + (prev == '\n' ? "\\n" : (prev == '\r' ? "\\r" : 
String.valueOf((char) prev)))
-//                 + " t=" + t + (t < target.length ? (" target[t]=" + (char) 
target[t]) : "")
-//                 + " l=" + l);
-        switch (state)
-          {
-
-          // Step 1: read until we find the "target" bytes: the start
-          // of the entry we need to read.
-          case 0:
-            if (((byte) c) != target[t])
-              t = 0;
-            else
-              {
-                t++;
-                if (t == target.length)
-                  {
-                    out.write(target);
-                    state = 1;
-                  }
-              }
-            break;
-
-          // Step 2: assert that there is a newline character after
-          // the "target" bytes.
-          case 1:
-            if (c != '\n' && c != '\r')
-              {
-                out.reset();
-                t = 0;
-                state = 0;
-              }
-            else
-              {
-                out.write(c);
-                state = 2;
-              }
-            break;
-
-          // Step 3: read this whole entry, until we reach an empty
-          // line.
-          case 2:
-            if (c == '\n')
-              {
-                out.write(c);
-                // NL always terminates a line.
-                if (l == 0 || (l == 1 && prev == '\r'))
-                  return out.toByteArray();
-                l = 0;
-              }
-            else
-              {
-                // Here we see a blank line terminated by a CR,
-                // followed by the next entry. Technically, `c' should
-                // always be 'N' at this point.
-                if (l == 1 && prev == '\r')
-                  return out.toByteArray();
-                out.write(c);
-                l++;
-              }
-            prev = c;
-            break;
-
-          default:
-            throw new RuntimeException("this statement should be unreachable");
-          }
-      }
-
-    // The last entry, with a single CR terminating the line.
-    if (state == 2 && prev == '\r' && l == 0)
-      return out.toByteArray();
-
-    // We should not reach this point, we didn't find the entry (or, possibly,
-    // it is the last entry and is malformed).
-    throw new IOException("could not find " + entry + " in manifest");
-  }
-
-  /**
    * A utility class that verifies jar entries as they are read.
    */
   private static class EntryInputStream extends FilterInputStream
Manifest-Version: 1.0
Bundle-Name: %pluginName
Bundle-ClassPath: .
Import-Package: javax.xml.parsers,org.w3c.dom,org.xml.sax
Bundle-RequiredExecutionEnvironment: J2SE-1.4,CDC-1.0/Foundation-1.0,J
 2SE-1.3
Bundle-Vendor: %providerName
Bundle-ManifestVersion: 2
Bundle-Localization: plugin
Bundle-SymbolicName: org.eclipse.jface
Require-Bundle: org.eclipse.swt;bundle-version="[3.2.0,4.0.0)";visibil
 ity:=reexport,org.eclipse.core.commands;bundle-version="[3.2.0,4.0.0)
 ";visibility:=reexport,org.eclipse.equinox.common;bundle-version="[3.
 2.0,4.0.0)"
Export-Package: org.eclipse.jface,org.eclipse.jface.action,org.eclipse
 .jface.action.images,org.eclipse.jface.bindings,org.eclipse.jface.bin
 dings.keys,org.eclipse.jface.bindings.keys.formatting,org.eclipse.jfa
 ce.commands,org.eclipse.jface.contexts,org.eclipse.jface.dialogs,org.
 eclipse.jface.dialogs.images,org.eclipse.jface.fieldassist,org.eclips
 e.jface.fieldassist.images,org.eclipse.jface.images,org.eclipse.jface
 .internal.provisional.action;x-friends:="org.eclipse.ui.workbench",or
 g.eclipse.jface.layout,org.eclipse.jface.menus,org.eclipse.jface.oper
 ation,org.eclipse.jface.preference,org.eclipse.jface.preference.image
 s,org.eclipse.jface.resource,org.eclipse.jface.util,org.eclipse.jface
 .viewers,org.eclipse.jface.viewers.deferred,org.eclipse.jface.window,
 org.eclipse.jface.wizard,org.eclipse.jface.wizard.images
Bundle-Version: 3.2.0.I20060605-1400

Reply via email to