Bug 52344 - Add -sigalg and -digestalg support to the signjar task
Summary: Add -sigalg and -digestalg support to the signjar task
Status: RESOLVED FIXED
Alias: None
Product: Ant
Classification: Unclassified
Component: Core tasks (show other bugs)
Version: unspecified
Hardware: All All
: P3 enhancement with 1 vote (vote)
Target Milestone: 1.8.3
Assignee: Ant Notifications List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-12-16 03:44 UTC by Wang Weijun
Modified: 2011-12-21 22:09 UTC (History)
0 users



Attachments
Patch for sigalg and digestalg support (3.40 KB, patch)
2011-12-19 07:23 UTC, Wang Weijun
Details | Diff
new patch (3.28 KB, patch)
2011-12-19 07:28 UTC, Wang Weijun
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Wang Weijun 2011-12-16 03:44:33 UTC
The jarsigner command supports the -digestalg and -sigalg options to specify the algorithms used for digesting (the lines in MANIFEST.MF and *.SF) and signing (*.RSA). It would be nice if the signjar task can support it.

In fact, in JDK 7, the default algorithms have been bumped to SHA-256 and SHA256withRSA respectively, and they are not supported on Android.

See http://stackoverflow.com/questions/8036422/android-signing-with-ant.
Comment 1 Wang Weijun 2011-12-16 03:50:48 UTC
My proposed patch is included below. Sorry I am new to both the ant tool and the source codes, and I don't know how to add a test.

I did manually check the result:

1. Specify a digestalg parameter and you can see changes in digest lines in META-INF/MANIFEST.MF and META-INF/ALIAS.MF.

2. Specify a sigalg and feed the META-INF/ALIAS.RSA file to "openssl asn1parse -inform DER" and you can see the PKCS #7 SignedData struct now uses a new signature algorithm (look at the last 6 lines of output).


$ svn di
Index: src/main/org/apache/tools/ant/taskdefs/SignJar.java
===================================================================
--- src/main/org/apache/tools/ant/taskdefs/SignJar.java	(revision 1215029)
+++ src/main/org/apache/tools/ant/taskdefs/SignJar.java	(working copy)
@@ -110,6 +110,16 @@
     private boolean force = false;
 
     /**
+     * signature algorithm
+     */
+    protected String sigAlg;
+
+    /**
+     * digest algorithm
+     */
+    protected String digestAlg;
+
+    /**
      * error string for unit test verification: {@value}
      */
     public static final String ERROR_TODIR_AND_SIGNEDJAR
@@ -276,6 +286,38 @@
     }
 
     /**
+     * Signature Algorithm; optional
+     *
+     * @param sigAlg the signature algorithm
+     */
+    public void setSigAlg(String sigAlg) {
+        this.sigAlg = sigAlg;
+    }
+
+    /**
+     * Signature Algorithm; optional
+     */
+    public String getSigAlg() {
+        return sigAlg;
+    }
+
+    /**
+     * Digest Algorithm; optional
+     *
+     * @param digestAlg the digest algorithm
+     */
+    public void setDigestAlg(String digestAlg) {
+        this.digestAlg = digestAlg;
+    }
+
+    /**
+     * Digest Algorithm; optional
+     */
+    public String getDigestAlg() {
+        return digestAlg;
+    }
+
+    /**
      * sign the jar(s)
      *
      * @throws BuildException on errors
@@ -420,6 +462,16 @@
             addValue(cmd, "-sectionsonly");
         }
 
+        if (sigAlg != null) {
+            addValue(cmd, "-sigalg");
+            addValue(cmd, sigAlg);
+        }
+
+        if (digestAlg != null) {
+            addValue(cmd, "-digestalg");
+            addValue(cmd, digestAlg);
+        }
+
         //add -tsa operations if declared
         addTimestampAuthorityCommands(cmd);
Comment 2 Joe Bowbeer 2011-12-16 20:25:26 UTC
This is an important fix:

The default behavior of jarsigner changed in jdk7 in a way that is incompatible with Android, among others, and the signjar task in Ant doesn't even support the arguments that are needed to restore compatibility.

With this fix, Android 'release' builds can be made to work correctly on jdk7.
Comment 3 Stefan Bodewig 2011-12-19 06:37:58 UTC
Patch looks reasonable except that I'd keep the field private.  It may be better to move this to AbstractJarSigner (even though VerifyJar is and probably has ever been dead).

Do we need to adapt the IsSigned condition as well?

Tests for signjar are AntUnit based in http://svn.apache.org/repos/asf/ant/core/trunk/src/tests/antunit/taskdefs/signjar-test.xml

It would be nice if you could provide a blurb for the task's manual page (in particular one that described the problems with Android/Java7) as well.  It is in http://svn.apache.org/repos/asf/ant/core/trunk/manual/Tasks/signjar.html
Comment 4 Wang Weijun 2011-12-19 06:58:49 UTC
1. Sure, the field can be made private.

2. The two options are only for the signing side, they are not provided on the verify side, so has better stay inside SignJar.

3. What would "isSigned" be used for? The jarsigner does not care if a jar was signed or not.

4. Sorry, I'm not familiar with AntUnit tests. Besides the XML file, are there codes behind? For this patch, you need to check if the output signed jar does use the new algorithms. As far as I know, digestalg can be checked by looking into the content of JarFile::getManifest::getEntries(). I cannot think of a good way to check for sigalg except for checking the .RSA file in raw bytes.

5. Sure, from the jarsigner --help output, we have

   sigalg: name of signature algorithm
   digestalg: name of digest algorithm

and an example can be

<signjar destDir="signed"
    alias="testonly" keystore="testkeystore"
    storepass="apacheant"
    sigalg="MD5withRSA">
    digestalg="SHA1"
  <path>
    <fileset dir="dist" includes="**/*.jar" />
  </path>
  <flattenmapper />
</signjar>

Sign all the JAR files in dist/**/*.jar using the digest algorithm SHA1 and the signature algorithm MD5withRSA. This is especially useful when you want to use the JDK 7 jarsigner (which uses SHA256 and SHA256withRSA as default) to create signed jars that will be deployed on platforms not supporting SHA256 and SHA256withRSA.
Comment 5 Stefan Bodewig 2011-12-19 07:10:38 UTC
IsSigned checks whether a .SF file with a name matching that archive is located in META-INF and is used inside the task if the lazy attribute is true to skip signing the jar.

I didn't check but if the different algos don't affect how the name of the signature file is constructed then we won't need to change this part.

Oh, and please us an attachment for the patch rather than pasting it in.

Thanks!
Comment 6 Wang Weijun 2011-12-19 07:23:03 UTC
Created attachment 28085 [details]
Patch for sigalg and digestalg support

Two comments:

1. fields now private, although I have no idea why some others (say, tsaurl, tsacert) are protected.

2. the "Required" value for the two new attributes is a simple "No". They do have default values, but are vendor/release-dependent.
Comment 7 Wang Weijun 2011-12-19 07:24:29 UTC
(In reply to comment #5)
> IsSigned checks whether a .SF file with a name matching that archive is located
> in META-INF and is used inside the task if the lazy attribute is true to skip
> signing the jar.
> 
> I didn't check but if the different algos don't affect how the name of the
> signature file is constructed then we won't need to change this part.

As long as you still use the same alias name, the old .SF and .RSA files will be overwritten.

> 
> Oh, and please us an attachment for the patch rather than pasting it in.
> 
> Thanks!
Comment 8 Wang Weijun 2011-12-19 07:28:36 UTC
Created attachment 28086 [details]
new patch

Sorry, the previous has a dup <h3> line.
Comment 9 Stefan Bodewig 2011-12-21 22:09:06 UTC
Thanks!

Committed as svn revision 1221901