Merge branch '1.9.x'

WHATSNEW

@@ -27,6 +27,16 @@ Fixed bugs:
    tokens, would be replaced by property values
    Bugzilla Report 62147
 
+ * Added a workaround for a bug in the jarsigner tool to <verifyjar>
+   which requires the -storepass command line argument when verifying
+   signatures using -strict together with a PKCS12 keystore. Unlike
+   when signing the jar it will not prompt for the keystore's password
+   and read it from standard input.
+   This means Ant will now pass the keystore's password on the command
+   line when using <verifyjar>, which poses a security risk you should
+   be aware of.
+   Bugzilla Report 62194
+
 Other changes:
 --------------
 

manual/Tasks/signjar.html

@@ -63,7 +63,9 @@ place.</p>
   </tr>
   <tr>
     <td>storepass</td>
-    <td>password for keystore integrity.</td>
+    <td>password for keystore integrity. Ant will not use
+    the <code>-storepass</code> command line argument but send the
+    password to jarsigner when it prompts for it.</td>
     <td>Yes</td>
   </tr>
   <tr>

manual/Tasks/verifyjar.html

@@ -50,8 +50,12 @@ the <var>jar</var> attribute. Nested paths are also supported.</p>
   </tr>
   <tr>
     <td>storepass</td>
-    <td>password for keystore integrity.</td>
-    <td>Yes</td>
+    <td>password for keystore integrity.
+    Note that
+    jarsigner does not read the password from stdin during
+    verification, so the password must be send via a command line
+    interface and may be visible to other users of the system.</td>
+    <td>No</td>
   </tr>
   <tr>
     <td>keystore</td>

src/main/org/apache/tools/ant/taskdefs/VerifyJar.java

@@ -59,6 +59,8 @@ public class VerifyJar extends AbstractJarSignerTask {
     private boolean certificates = false;
     private BufferingOutputFilter outputCache = new BufferingOutputFilter();
 
+    private String savedStorePass = null;
+
     /**
      * Ask for certificate information to be printed
      * @param certificates if true print certificates.
@@ -99,6 +101,42 @@ public class VerifyJar extends AbstractJarSignerTask {
         }
     }
 
+    /**
+     * @since 1.10.3
+     */
+    @Override
+    protected void beginExecution() {
+        // when using a PKCS12 keystore jarsigner -verify will not
+        // prompt for the keystore password but will only properly
+        // verify the jar with -strict enabled if the -storepass
+        // parameter is used. Note that the documentation of jarsigner
+        // says -storepass was never required with -verify - this is
+        // wrong.
+        //
+        // See https://bz.apache.org/bugzilla/show_bug.cgi?id=62194
+        //
+        // So if strict is true then we hide storepass from the base
+        // implementation and instead add the -storepass command line
+        // argument
+        if (mustHideStorePass()) {
+            savedStorePass = storepass;
+            setStorepass(null);
+        }
+        super.beginExecution();
+    }
+
+    /**
+     * @since 1.10.3
+     */
+    @Override
+    protected void endExecution() {
+        if (savedStorePass != null) {
+            setStorepass(savedStorePass);
+            savedStorePass = null;
+        }
+        super.endExecution();
+    }
+
     /**
      * verify a JAR.
      * @param jar the jar to verify.
@@ -112,6 +150,10 @@ public class VerifyJar extends AbstractJarSignerTask {
 
         setCommonOptions(cmd);
         bindToKeystore(cmd);
+        if (savedStorePass != null) {
+            addValue(cmd, "-storepass");
+            addValue(cmd, savedStorePass);
+        }
 
         //verify special operations
         addValue(cmd, "-verify");
@@ -123,6 +165,10 @@ public class VerifyJar extends AbstractJarSignerTask {
         //JAR  is required
         addValue(cmd, jar.getPath());
 
+        if (alias != null) {
+            addValue(cmd, alias);
+        }
+
         log("Verifying JAR: " + jar.getAbsolutePath());
         outputCache.clear();
         BuildException ex = null;
@@ -147,6 +193,10 @@ public class VerifyJar extends AbstractJarSignerTask {
         }
     }
 
+    private boolean mustHideStorePass() {
+        return strict && storepass != null;
+    }
+
     /**
      * we are not thread safe here. Do not use on multiple threads at the same time.
      */

src/tests/antunit/taskdefs/signjar-test.xml

@@ -25,6 +25,7 @@
   <property name="signtest.jar" location="${sign.dir}/signtest.jar" />
   <property name="subdirsigntest.jar" location="${subdir}/signtest.jar" />
   <property name="testkeystore" location="../../../etc/testcases/testkeystore" />
+  <property name="testkeystore.pkcs12" location="${testkeystore}.pkcs12" />
 
   <macrodef name="assertSigned">
     <attribute name="jar" default="${signtest.jar}" />
@@ -43,6 +44,11 @@
     <verifyjar keystore="${testkeystore}" storepass="apacheant" />
   </presetdef>
 
+  <presetdef name="verify-base-pkcs12">
+    <verifyjar keystore="${testkeystore.pkcs12}" storepass="apacheant"
+               storetype="pkcs12" alias="testonly"/>
+  </presetdef>
+
   <presetdef name="sign">
     <sign-base jar="${signtest.jar}" />
   </presetdef>
@@ -60,6 +66,10 @@
     <sign />
   </target>
 
+  <target name="basic-pkcs12" depends="jar">
+    <sign keystore="${testkeystore.pkcs12}" storetype="pkcs12" strict="true"/>
+  </target>
+
   <target name="testBasic" depends="basic">
     <assertSigned />
   </target>
@@ -232,6 +242,10 @@
     <verify-base jar="${signtest.jar}" />
   </target>
 
+  <target name="testVerifyJarPKCS12" depends="basic-pkcs12">
+    <verify-base-pkcs12 jar="${signtest.jar}" />
+  </target>
+
   <target name="testVerifyJarCertificates" depends="basic">
     <verify-base jar="${signtest.jar}" certificates="true" verbose="true" />
   </target>
@@ -268,5 +282,14 @@
     </au:expectfailure>
   </target>
 
+  <target name="testVerifyJarStrict" depends="basic">
+    <verify-base jar="${signtest.jar}" strict="true"/>
+  </target>
+
+  <target name="testVerifyJarStrictPKCS12" depends="basic-pkcs12"
+          description="https://bz.apache.org/bugzilla/show_bug.cgi?id=62194">
+    <verify-base-pkcs12 jar="${signtest.jar}" strict="true"/>
+  </target>
+
 </project>