Fix regression with trivially signed (eg, just META-INF/) jars

3 files changed, 24 insertions, 12 deletions

diff --git a/ChangeLog b/ChangeLog
index 4d60869..2f1a145 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -30,6 +30,17 @@
 
 2012-10-19  Adam Domurad  <[email protected]>
 
+	Fixes JCV#isTriviallySigned(). Reproducer 'EmptySignedJar' passes 
+	again.
+	* netx/net/sourceforge/jnlp/tools/JarCertVerifier.java: Remove 
+	problematic 'triviallySigned' variable and instead determine 
+	whether triviallySigned on the fly. Consider jars with 0 signable 
+	entries as SIGNED_OK.
+	* tests/netx/unit/net/sourceforge/jnlp/tools/JarCertVerifierTest.java:
+	Update no-signers unit test.
+
+2012-10-19  Adam Domurad  <[email protected]>
+
 	* netx/net/sourceforge/jnlp/security/AppVerifier.java: Use interface 
 	types for declared types where applicable.
 	* netx/net/sourceforge/jnlp/security/PluginAppVerifier.java: Same.
diff --git a/netx/net/sourceforge/jnlp/tools/JarCertVerifier.java b/netx/net/sourceforge/jnlp/tools/JarCertVerifier.java
index 6b828c6..17c2a93 100644
--- a/netx/net/sourceforge/jnlp/tools/JarCertVerifier.java
+++ b/netx/net/sourceforge/jnlp/tools/JarCertVerifier.java
@@ -105,14 +105,13 @@ public class JarCertVerifier implements CertVerifier {
         appVerifier = verifier;
     }
 
-    /** Whether a signable entry was found within jars (jars with content more than just META-INF/*) */
-    private boolean triviallySigned = false;
-
     /**
-     * Return true if there are signable entries in the jars, otherwise false
+     * Return true if there are no signable entries in the jar.
+     * This will return false if any of verified jars have content more than just META-INF/.
      */
     public boolean isTriviallySigned() {
-        return triviallySigned;
+        return getTotalJarEntries(jarSignableEntries) <= 0
+                && certs.size() <= 0;
     }
 
     public boolean getAlreadyTrustPublisher() {
@@ -178,7 +177,7 @@ public class JarCertVerifier implements CertVerifier {
      */
     // FIXME: Change javadoc once applets do not need entire jars signed.
     public boolean isFullySigned() {
-        if (triviallySigned)
+        if (isTriviallySigned())
             return true;
         boolean fullySigned = appVerifier.isFullySigned(certs,
                 jarSignableEntries);
@@ -236,7 +235,6 @@ public class JarCertVerifier implements CertVerifier {
                 }
 
                 VerifyResult result = verifyJar(localFile);
-                triviallySigned = false;
 
                 if (result == VerifyResult.UNSIGNED) {
                     unverifiedJars.add(localFile);
@@ -244,8 +242,6 @@ public class JarCertVerifier implements CertVerifier {
                     verifiedJars.add(localFile);
                 } else if (result == VerifyResult.SIGNED_OK) {
                     verifiedJars.add(localFile);
-                    triviallySigned = getTotalJarEntries(jarSignableEntries) <= 0
-                            && certs.size() <= 0;
                 }
             } catch (Exception e) {
                 // We may catch exceptions from using verifyJar()
@@ -399,7 +395,12 @@ public class JarCertVerifier implements CertVerifier {
         // Every signable entry of this jar needs to be signed by at least
         // one signer for the jar to be considered successfully signed.
         VerifyResult result = null;
-        if (allEntriesSignedBySingleCert) {
+
+        if (numSignableEntriesInJar == 0) {
+            // Allow jars with no signable entries to simply be considered signed.
+            // There should be no security risk in doing so.
+            result = VerifyResult.SIGNED_OK;
+        } else if (allEntriesSignedBySingleCert) {
 
             // We need to find at least one signer without any issues.
             for (CertPath entryCertPath : jarSignCount.keySet()) {
diff --git a/tests/netx/unit/net/sourceforge/jnlp/tools/JarCertVerifierTest.java b/tests/netx/unit/net/sourceforge/jnlp/tools/JarCertVerifierTest.java
index 88054ab..78e41e4 100644
--- a/tests/netx/unit/net/sourceforge/jnlp/tools/JarCertVerifierTest.java
+++ b/tests/netx/unit/net/sourceforge/jnlp/tools/JarCertVerifierTest.java
@@ -121,8 +121,8 @@ public class JarCertVerifierTest {
         entries.add(new JarCertVerifierEntry("META-INF/MANIFEST.MF"));
         VerifyResult result = jcv.verifyJarEntryCerts("", true, entries);
 
-        Assert.assertEquals("No signable entry (only dirs/manifests) should be considered unsigned.",
-                VerifyResult.UNSIGNED, result);
+        Assert.assertEquals("No signable entry (only dirs/manifests) should be considered trivially signed.",
+                VerifyResult.SIGNED_OK, result);
         Assert.assertEquals("No signable entry (only dirs/manifests) means no signers in the verifier.",
                 0, jcv.getCertsList().size());
     }