8196823: jarsigner should not create a signed jar if the signing fails
authorweijun
Thu, 08 Feb 2018 11:44:21 +0800
changeset 48760 25725c11c296
parent 48759 ffa68af7da87
child 48761 74c1fa26435a
8196823: jarsigner should not create a signed jar if the signing fails Reviewed-by: mullan, alanb
src/jdk.jartool/share/classes/jdk/security/jarsigner/JarSigner.java
src/jdk.jartool/share/classes/sun/security/tools/jarsigner/Main.java
test/jdk/sun/security/tools/jarsigner/FailedSigning.java
--- a/src/jdk.jartool/share/classes/jdk/security/jarsigner/JarSigner.java	Wed Feb 07 11:28:23 2018 -0800
+++ b/src/jdk.jartool/share/classes/jdk/security/jarsigner/JarSigner.java	Thu Feb 08 11:44:21 2018 +0800
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2015, 2017, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2015, 2018, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -547,6 +547,11 @@
     /**
      * Signs a file into an {@link OutputStream}. This method will not close
      * {@code file} or {@code os}.
+     * <p>
+     * If an I/O error or signing error occurs during the signing, then it may
+     * do so after some bytes have been written. Consequently, the output
+     * stream may be in an inconsistent state. It is strongly recommended that
+     * it be promptly closed in this case.
      *
      * @param file the file to sign.
      * @param os the output stream.
--- a/src/jdk.jartool/share/classes/sun/security/tools/jarsigner/Main.java	Wed Feb 07 11:28:23 2018 -0800
+++ b/src/jdk.jartool/share/classes/sun/security/tools/jarsigner/Main.java	Thu Feb 08 11:44:21 2018 +0800
@@ -26,6 +26,7 @@
 package sun.security.tools.jarsigner;
 
 import java.io.*;
+import java.net.UnknownHostException;
 import java.security.cert.CertPathValidatorException;
 import java.security.cert.PKIXBuilderParameters;
 import java.util.*;
@@ -1400,13 +1401,6 @@
             error(rb.getString("unable.to.open.jar.file.")+jarName, ioe);
         }
 
-        FileOutputStream fos = null;
-        try {
-            fos = new FileOutputStream(signedJarFile);
-        } catch (IOException ioe) {
-            error(rb.getString("unable.to.create.")+tmpJarName, ioe);
-        }
-
         CertPath cp = CertificateFactory.getInstance("X.509")
                 .generateCertPath(Arrays.asList(certChain));
         JarSigner.Builder builder = new JarSigner.Builder(privateKey, cp);
@@ -1473,24 +1467,42 @@
         builder.setProperty("sectionsOnly", Boolean.toString(!signManifest));
         builder.setProperty("internalSF", Boolean.toString(!externalSF));
 
+        FileOutputStream fos = null;
+        try {
+            fos = new FileOutputStream(signedJarFile);
+        } catch (IOException ioe) {
+            error(rb.getString("unable.to.create.")+tmpJarName, ioe);
+        }
+
+        Throwable failedCause = null;
+        String failedMessage = null;
+
         try {
             builder.build().sign(zipFile, fos);
         } catch (JarSignerException e) {
-            Throwable cause = e.getCause();
-            if (cause != null && cause instanceof SocketTimeoutException) {
+            failedCause = e.getCause();
+            if (failedCause instanceof SocketTimeoutException
+                    || failedCause instanceof UnknownHostException) {
                 // Provide a helpful message when TSA is beyond a firewall
-                error(rb.getString("unable.to.sign.jar.") +
+                failedMessage = rb.getString("unable.to.sign.jar.") +
                         rb.getString("no.response.from.the.Timestamping.Authority.") +
                         "\n  -J-Dhttp.proxyHost=<hostname>" +
                         "\n  -J-Dhttp.proxyPort=<portnumber>\n" +
                         rb.getString("or") +
                         "\n  -J-Dhttps.proxyHost=<hostname> " +
-                        "\n  -J-Dhttps.proxyPort=<portnumber> ", e);
+                        "\n  -J-Dhttps.proxyPort=<portnumber> ";
             } else {
-                error(rb.getString("unable.to.sign.jar.")+e.getCause(), e.getCause());
+                // JarSignerException might have a null cause
+                if (failedCause == null) {
+                    failedCause = e;
+                }
+                failedMessage = rb.getString("unable.to.sign.jar.") + failedCause;
             }
+        } catch (Exception e) {
+            failedCause = e;
+            failedMessage = rb.getString("unable.to.sign.jar.") + failedCause;
         } finally {
-            // close the resouces
+            // close the resources
             if (zipFile != null) {
                 zipFile.close();
                 zipFile = null;
@@ -1499,6 +1511,12 @@
             if (fos != null) {
                 fos.close();
             }
+
+        }
+
+        if (failedCause != null) {
+            signedJarFile.delete();
+            error(failedMessage, failedCause);
         }
 
         // The JarSigner API always accepts the timestamp received.
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/test/jdk/sun/security/tools/jarsigner/FailedSigning.java	Thu Feb 08 11:44:21 2018 +0800
@@ -0,0 +1,64 @@
+/*
+ * Copyright (c) 2018, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+/*
+ * @test
+ * @bug 8196823
+ * @summary jarsigner should not create a signed jar if the signing fails
+ * @library /test/lib
+ */
+
+import jdk.test.lib.Asserts;
+import jdk.test.lib.SecurityTools;
+import jdk.test.lib.util.JarUtils;
+
+import java.nio.file.Files;
+import java.nio.file.Paths;
+
+public class FailedSigning {
+
+    public static void main(String[] args) throws Exception {
+
+        SecurityTools.keytool("-keystore", "ks", "-genkeypair",
+                "-storepass", "changeit", "-keypass", "changeit",
+                "-keyalg", "RSA",
+                "-alias", "x", "-dname", "CN=X")
+            .shouldHaveExitValue(0);
+
+        JarUtils.createJar("x.jar", "ks");
+
+        SecurityTools.jarsigner("-keystore", "ks", "-storepass", "changeit",
+                "-tsa", "ftp://0.0.0.0",
+                "x.jar", "x")
+                .shouldHaveExitValue(1);
+
+        Asserts.assertFalse(Files.exists(Paths.get("x.jar.sig")));
+
+        SecurityTools.jarsigner("-keystore", "ks", "-storepass", "changeit",
+                "-tsa", "ftp://0.0.0.0",
+                "-signedjar", "y.jar", "x.jar", "x")
+                .shouldHaveExitValue(1);
+
+        Asserts.assertFalse(Files.exists(Paths.get("y.jar")));
+    }
+}