8255536: Remove the directsign property and option

Reviewed-by: mullan

src/java.base/share/classes/sun/security/pkcs/PKCS7.java

@@ -28,9 +28,6 @@ package sun.security.pkcs;
 import java.io.*;
 import java.math.BigInteger;
 import java.net.URI;
-import java.security.interfaces.EdECPrivateKey;
-import java.security.spec.InvalidParameterSpecException;
-import java.security.spec.PSSParameterSpec;
 import java.util.*;
 import java.security.cert.X509Certificate;
 import java.security.cert.CertificateException;

src/jdk.jartool/share/classes/jdk/security/jarsigner/JarSigner.java

@@ -34,7 +34,6 @@ import sun.security.pkcs.PKCS9Attribute;
 import sun.security.pkcs.PKCS9Attributes;
 import sun.security.timestamp.HttpTimestamper;
 import sun.security.tools.PathList;
-import sun.security.tools.jarsigner.TimestampedSigner;
 import sun.security.util.Event;
 import sun.security.util.ManifestDigester;
 import sun.security.util.SignatureFileVerifier;
@@ -122,7 +121,6 @@ public final class JarSigner {
         String tSADigestAlg;
         boolean sectionsonly = false;
         boolean internalsf = false;
-        boolean directsign = false;
         String altSignerPath;
         String altSigner;
 
@@ -358,10 +356,6 @@ public final class JarSigner {
          * <li>"sectionsonly": "true" if the .SF file only contains the hash
          * value for each section of the manifest and not for the whole
          * manifest, "false" otherwise. Default "false".
-         * <li>"directsign": "true" if the signature is calculated on the
-         * content directly, "false" if it's calculated on signed attributes
-         * which itself is calculated from the content and stored in the
-         * signer's SignerInfo. Default "false".
          * </ul>
          * All property names are case-insensitive.
          *
@@ -395,9 +389,6 @@ public final class JarSigner {
                 case "sectionsonly":
                     this.sectionsonly = parseBoolean("sectionsonly", value);
                     break;
-                case "directsign":
-                    this.directsign = parseBoolean("directsign", value);
-                    break;
                 case "altsignerpath":
                     altSignerPath = value;
                     break;
@@ -510,7 +501,6 @@ public final class JarSigner {
     private final String tSADigestAlg;
     private final boolean sectionsonly; // do not "sign" the whole manifest
     private final boolean internalsf; // include the .SF inside the PKCS7 block
-    private final boolean directsign;
 
     @Deprecated(since="16", forRemoval=true)
     private final String altSignerPath;
@@ -561,9 +551,12 @@ public final class JarSigner {
         this.altSigner = builder.altSigner;
         this.altSignerPath = builder.altSignerPath;
 
-        this.directsign = this.altSigner != null
+        // altSigner cannot support modern algorithms like RSASSA-PSS and EdDSA
-                ? true
+        if (altSigner != null
-                : builder.directsign;
+                && !sigalg.toUpperCase(Locale.ENGLISH).contains("WITH")) {
+            throw new IllegalArgumentException(
+                    "Customized ContentSigner is not supported for " + sigalg);
+        }
     }
 
     /**
@@ -666,8 +659,6 @@ public final class JarSigner {
                 return Boolean.toString(sectionsonly);
             case "altsignerpath":
                 return altSignerPath;
-            case "directsign":
-                return Boolean.toString(directsign);
             case "altsigner":
                 return altSigner;
             default:
@@ -855,20 +846,7 @@ public final class JarSigner {
         sf.write(baos);
         byte[] content = baos.toByteArray();
 
-        // Use new method if directSign is false or it's a modern
+        if (altSigner == null) {
-        // algorithm not supported by existing ContentSigner.
-        // Make this always true after we remove ContentSigner.
-        boolean useNewMethod = !directsign
-                || !sigalg.toUpperCase(Locale.ENGLISH).contains("WITH");
-
-        // For newer sigalg without "with", always use the new PKCS7
-        // generateToken method. Otherwise, use deprecated ContentSigner.
-        if (useNewMethod) {
-            if (altSigner != null) {
-                throw new IllegalArgumentException(directsign
-                        ? ("Customized ContentSigner is not supported for " + sigalg)
-                        : "Customized ContentSigner does not support authenticated attributes");
-            }
             Function<byte[], PKCS9Attributes> timestamper = null;
             if (tsaUrl != null) {
                 timestamper = s -> {
@@ -889,7 +867,7 @@ public final class JarSigner {
             }
             // We now create authAttrs in block data, so "direct == false".
             block = PKCS7.generateNewSignedData(sigalg, sigProvider, privateKey, certChain,
-                    content, internalsf, directsign, timestamper);
+                    content, internalsf, false, timestamper);
         } else {
             Signature signer = SignatureUtil.fromKey(sigalg, privateKey, sigProvider);
             signer.update(content);
@@ -901,9 +879,7 @@ public final class JarSigner {
                             tSADigestAlg, signature,
                             signer.getAlgorithm(), certChain, content, zipFile);
             @SuppressWarnings("removal")
-            ContentSigner signingMechanism = (altSigner != null)
+            ContentSigner signingMechanism = loadSigningMechanism(altSigner, altSignerPath);
-                    ? loadSigningMechanism(altSigner, altSignerPath)
-                    : new TimestampedSigner();
             block = signingMechanism.generateSignedData(
                     params,
                     !internalsf,

src/jdk.jartool/share/classes/sun/security/tools/jarsigner/Main.java

@@ -163,7 +163,6 @@ public class Main {
     boolean debug = false; // debug
     boolean signManifest = true; // "sign" the whole manifest
     boolean externalSF = true; // leave the .SF out of the PKCS7 block
-    boolean directSign = false; // sign SF directly or thru signedAttrs
     boolean strict = false;  // treat warnings as error
     boolean revocationCheck = false; // Revocation check flag
 
@@ -473,8 +472,6 @@ public class Main {
                 signManifest = false;
             } else if (collator.compare(flags, "-internalsf") ==0) {
                 externalSF = false;
-            } else if (collator.compare(flags, "-directsign") ==0) {
-                directSign = true;
             } else if (collator.compare(flags, "-verify") ==0) {
                 verify = true;
             } else if (collator.compare(flags, "-verbose") ==0) {
@@ -663,9 +660,6 @@ public class Main {
         System.out.println(rb.getString
                 (".internalsf.include.the.SF.file.inside.the.signature.block"));
         System.out.println();
-        System.out.println(rb.getString
-                (".directsign.sign.the.SF.file.directly.no.signerinfo.signedattributes"));
-        System.out.println();
         System.out.println(rb.getString
                 (".sectionsonly.don.t.compute.hash.of.entire.manifest"));
         System.out.println();
@@ -1773,7 +1767,6 @@ public class Main {
 
         builder.setProperty("sectionsOnly", Boolean.toString(!signManifest));
         builder.setProperty("internalSF", Boolean.toString(!externalSF));
-        builder.setProperty("directsign", Boolean.toString(directSign));
 
         FileOutputStream fos = null;
         try {

src/jdk.jartool/share/classes/sun/security/tools/jarsigner/Resources.java

@@ -101,8 +101,6 @@ public class Resources extends java.util.ListResourceBundle {
                 "                            (This option is deprecated and will be removed in a future release.)"},
         {".internalsf.include.the.SF.file.inside.the.signature.block",
                 "[-internalsf]               include the .SF file inside the signature block"},
-        {".directsign.sign.the.SF.file.directly.no.signerinfo.signedattributes",
-                "[-directsign]               sign the .SF file directly (no SignerInfo signedAttributes)"},
         {".sectionsonly.don.t.compute.hash.of.entire.manifest",
                 "[-sectionsonly]             don't compute hash of entire manifest"},
         {".protected.keystore.has.protected.authentication.path",

src/jdk.jartool/share/classes/sun/security/tools/jarsigner/TimestampedSigner.java

@@ -1,113 +0,0 @@
-/*
- * Copyright (c) 2007, 2020, 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.  Oracle designates this
- * particular file as subject to the "Classpath" exception as provided
- * by Oracle in the LICENSE file that accompanied this code.
- *
- * 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.
- */
-
-package sun.security.tools.jarsigner;
-
-import java.io.IOException;
-import java.net.URI;
-import java.security.NoSuchAlgorithmException;
-import java.security.cert.CertificateException;
-import java.security.cert.X509Certificate;
-
-import com.sun.jarsigner.*;
-import sun.security.pkcs.PKCS7;
-
-/**
- * This class implements a content signing service.
- * It generates a timestamped signature for a given content according to
- * <a href="http://www.ietf.org/rfc/rfc3161.txt">RFC 3161</a>.
- * The signature along with a trusted timestamp and the signer's certificate
- * are all packaged into a standard PKCS #7 Signed Data message.
- *
- * @author Vincent Ryan
- */
-@Deprecated(since="16", forRemoval=true)
-@SuppressWarnings("removal")
-public final class TimestampedSigner extends ContentSigner {
-
-    /**
-     * Instantiates a content signer that supports timestamped signatures.
-     */
-    public TimestampedSigner() {
-    }
-
-    /**
-     * Generates a PKCS #7 signed data message that includes a signature
-     * timestamp.
-     * This method is used when a signature has already been generated.
-     * The signature, a signature timestamp, the signer's certificate chain,
-     * and optionally the content that was signed, are packaged into a PKCS #7
-     * signed data message.
-     *
-     * @param params The non-null input parameters.
-     * @param omitContent true if the content should be omitted from the
-     *        signed data message. Otherwise the content is included.
-     * @param applyTimestamp true if the signature should be timestamped.
-     *        Otherwise timestamping is not performed.
-     * @return A PKCS #7 signed data message including a signature timestamp.
-     * @throws NoSuchAlgorithmException The exception is thrown if the signature
-     *         algorithm is unrecognised.
-     * @throws CertificateException The exception is thrown if an error occurs
-     *         while processing the signer's certificate or the TSA's
-     *         certificate.
-     * @throws IOException The exception is thrown if an error occurs while
-     *         generating the signature timestamp or while generating the signed
-     *         data message.
-     * @throws NullPointerException The exception is thrown if parameters is
-     *         null.
-     */
-    public byte[] generateSignedData(ContentSignerParameters params,
-        boolean omitContent, boolean applyTimestamp)
-            throws NoSuchAlgorithmException, CertificateException, IOException {
-
-        if (params == null) {
-            throw new NullPointerException();
-        }
-
-        X509Certificate[] signerChain = params.getSignerCertificateChain();
-        byte[] signature = params.getSignature();
-
-        // Include or exclude content
-        byte[] content = (omitContent == true) ? null : params.getContent();
-
-        URI tsaURI = null;
-        if (applyTimestamp) {
-            tsaURI = params.getTimestampingAuthority();
-            if (tsaURI == null) {
-                // Examine TSA cert
-                tsaURI = PKCS7.getTimestampingURI(
-                    params.getTimestampingAuthorityCertificate());
-                if (tsaURI == null) {
-                    throw new CertificateException(
-                        "Subject Information Access extension not found");
-                }
-            }
-        }
-        return PKCS7.generateSignedData(signature, signerChain, content,
-                                        params.getSignatureAlgorithm(), tsaURI,
-                                        params.getTSAPolicyID(),
-                                        params.getTSADigestAlg());
-    }
-}

test/jdk/jdk/security/jarsigner/Properties.java

@@ -75,15 +75,10 @@ public class Properties {
         Asserts.assertTrue(sf.startsWith("Signature-Version"));
 
         // There is a SignedAttributes
-        byte[] d0 = sign(jsb.setProperty("directsign", "false"));
+        byte[] d0 = sign(jsb);
         Asserts.assertTrue(DerUtils.innerDerValue(d0, "10403")
                 .isContextSpecific((byte)0));
 
-        // There is no SignedAttributes
-        byte[] d1 = sign(jsb.setProperty("directsign", "true"));
-        Asserts.assertFalse(DerUtils.innerDerValue(d1, "10403")
-                .isContextSpecific((byte)0));
-
         // Has a hash for the whole manifest
         byte[] s0 = sign(jsb.setProperty("sectionsonly", "false"));
         sf = new String(DerUtils.innerDerValue(s0, "10210").getOctetString());

test/jdk/jdk/security/jarsigner/Spec.java

@@ -23,7 +23,7 @@
 
 /**
  * @test
- * @bug 8056174 8242068
+ * @bug 8056174 8242068 8255536
  * @summary Make sure JarSigner impl conforms to spec
  * @library /test/lib
  * @modules java.base/sun.security.tools.keytool
@@ -70,6 +70,9 @@ public class Spec {
         sun.security.tools.keytool.Main.main(
                 ("-keystore ks -storepass changeit -keypass changeit -dname" +
                         " CN=DSA -alias d -genkeypair -keyalg dsa").split(" "));
+        sun.security.tools.keytool.Main.main(
+                ("-keystore ks -storepass changeit -keypass changeit -dname" +
+                        " CN=Ed25519 -alias e -genkeypair -keyalg Ed25519").split(" "));
 
         char[] pass = "changeit".toCharArray();
 
@@ -127,8 +130,6 @@ public class Spec {
         iae(()->b1.setProperty("sectionsonly", "OK"));
         npe(()->b1.setProperty("sectionsonly", null));
         npe(()->b1.setProperty("altsigner", null));
-        iae(()->b1.setProperty("directsign", "OK"));
-        npe(()->b1.setProperty("directsign", null));
         npe(()->b1.eventHandler(null));
 
         // default values
@@ -146,7 +147,6 @@ public class Spec {
         assertTrue(js2.getProperty("tsapolicyid") == null);
         assertTrue(js2.getProperty("internalsf").equals("false"));
         assertTrue(js2.getProperty("sectionsonly").equals("false"));
-        assertTrue(js2.getProperty("directsign").equals("false"));
         assertTrue(js2.getProperty("altsigner") == null);
         uoe(()->js2.getProperty("invalid"));
 
@@ -163,7 +163,6 @@ public class Spec {
                 .setProperty("tsapolicyid", "1.2.3.4")
                 .setProperty("internalsf", "true")
                 .setProperty("sectionsonly", "true")
-                .setProperty("directsign", "true")
                 .setProperty("altsigner", "MyContentSigner")
                 .eventHandler(myeh);
         JarSigner js3 = b3.build();
@@ -176,7 +175,6 @@ public class Spec {
         assertTrue(js3.getProperty("tsapolicyid").equals("1.2.3.4"));
         assertTrue(js3.getProperty("internalsf").equals("true"));
         assertTrue(js3.getProperty("sectionsonly").equals("true"));
-        assertTrue(js3.getProperty("directsign").equals("true"));
         assertTrue(js3.getProperty("altsigner").equals("MyContentSigner"));
         assertTrue(js3.getProperty("altsignerpath") == null);
 
@@ -208,6 +206,14 @@ public class Spec {
         assertTrue(JarSigner.Builder
                 .getDefaultSignatureAlgorithm(kpg.generateKeyPair().getPrivate())
                 .equals("SHA512withECDSA"));
+
+        // altsigner does not support modern algorithms
+        JarSigner.Builder b4 = new JarSigner.Builder(
+                (PrivateKey)ks.getKey("e", pass),
+                CertificateFactory.getInstance("X.509")
+                        .generateCertPath(Arrays.asList(ks.getCertificateChain("e"))));
+        b4.setProperty("altsigner", "MyContentSigner");
+        iae(() -> b4.build());
     }
 
     interface RunnableWithException {