8257497: Update keytool to create AKID from the SKID of the issuing certificate as specified by RFC 5280

Reviewed-by: coffeys, mullan, weijun

src/java.base/share/classes/sun/security/tools/keytool/Main.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1997, 2020, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1997, 2021, 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
@@ -1476,12 +1476,39 @@ public final class Main {
                 reqex = (CertificateExtensions)attr.getAttributeValue();
             }
         }
+
+        PublicKey subjectPubKey = req.getSubjectPublicKeyInfo();
+        PublicKey issuerPubKey = signerCert.getPublicKey();
+
+        KeyIdentifier signerSubjectKeyId;
+        if (Arrays.equals(subjectPubKey.getEncoded(), issuerPubKey.getEncoded())) {
+            // No AKID for self-signed cert
+            signerSubjectKeyId = null;
+        } else {
+            X509CertImpl certImpl;
+            if (signerCert instanceof X509CertImpl) {
+                certImpl = (X509CertImpl) signerCert;
+            } else {
+                certImpl = new X509CertImpl(signerCert.getEncoded());
+            }
+
+            // To enforce compliance with RFC 5280 section 4.2.1.1: "Where a key
+            // identifier has been previously established, the CA SHOULD use the
+            // previously established identifier."
+            // Use issuer's SKID to establish the AKID in createV3Extensions() method.
+            signerSubjectKeyId = certImpl.getSubjectKeyId();
+
+            if (signerSubjectKeyId == null) {
+                signerSubjectKeyId = new KeyIdentifier(issuerPubKey);
+            }
+        }
+
         CertificateExtensions ext = createV3Extensions(
                 reqex,
                 null,
                 v3ext,
-                req.getSubjectPublicKeyInfo(),
+                subjectPubKey,
-                signerCert.getPublicKey());
+                signerSubjectKeyId);
         info.set(X509CertInfo.EXTENSIONS, ext);
         X509CertImpl cert = new X509CertImpl(info);
         cert.sign(privateKey, sigAlgName);
@@ -4224,6 +4251,7 @@ public final class Main {
      * @param extstrs -ext values, Read keytool doc
      * @param pkey the public key for the certificate
      * @param akey the public key for the authority (issuer)
+     * @param aSubjectKeyId the subject key identifier for the authority (issuer)
      * @return the created CertificateExtensions
      */
     private CertificateExtensions createV3Extensions(
@@ -4231,7 +4259,7 @@ public final class Main {
             CertificateExtensions existingEx,
             List <String> extstrs,
             PublicKey pkey,
-            PublicKey akey) throws Exception {
+            KeyIdentifier aSubjectKeyId) throws Exception {
 
         // By design, inside a CertificateExtensions object, all known
         // extensions uses name (say, "BasicConstraints") as key and
@@ -4256,6 +4284,14 @@ public final class Main {
             }
         }
         try {
+            // always non-critical
+            setExt(result, new SubjectKeyIdentifierExtension(
+                    new KeyIdentifier(pkey).getIdentifier()));
+            if (aSubjectKeyId != null) {
+                setExt(result, new AuthorityKeyIdentifierExtension(aSubjectKeyId,
+                        null, null));
+            }
+
             // name{:critical}{=value}
             // Honoring requested extensions
             if (requestedEx != null) {
@@ -4578,13 +4614,6 @@ public final class Main {
                                 "Unknown.extension.type.") + extstr);
                 }
             }
-            // always non-critical
-            setExt(result, new SubjectKeyIdentifierExtension(
-                    new KeyIdentifier(pkey).getIdentifier()));
-            if (akey != null && !pkey.equals(akey)) {
-                setExt(result, new AuthorityKeyIdentifierExtension(
-                                new KeyIdentifier(akey), null, null));
-            }
         } catch(IOException e) {
             throw new RuntimeException(e);
         }

test/jdk/sun/security/tools/keytool/CheckCertAKID.java

@@ -0,0 +1,102 @@
+/*
+ * Copyright (c) 2021, 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 8257497
+ * @summary Check if issuer's SKID is used to establish the AKID for the subject cert
+ * @library /test/lib
+ * @modules java.base/sun.security.util
+ */
+
+import jdk.test.lib.SecurityTools;
+import jdk.test.lib.process.OutputAnalyzer;
+
+import java.io.*;
+import java.security.KeyStore;
+import java.security.cert.X509Certificate;
+import java.util.Arrays;
+import sun.security.util.DerValue;
+import sun.security.util.KnownOIDs;
+import static sun.security.util.KnownOIDs.*;
+
+public class CheckCertAKID {
+
+    static OutputAnalyzer kt(String cmd, String ks) throws Exception {
+        return SecurityTools.keytool("-storepass changeit " + cmd +
+                " -keystore " + ks);
+    }
+
+    public static void main(String[] args) throws Exception {
+
+        System.out.println("Generating a root cert with SubjectKeyIdentifier extension");
+        kt("-genkeypair -keyalg rsa -alias ca -dname CN=CA -ext bc:c " +
+                "-ext 2.5.29.14=04:14:00:01:02:03:04:05:06:07:08:09:10:11:12:13:14:15:16:17:18:19",
+                "ks");
+
+        kt("-exportcert -alias ca -rfc -file root.cert", "ks");
+
+        SecurityTools.keytool("-keystore ks -storepass changeit " +
+                "-printcert -file root.cert")
+                .shouldNotContain("AuthorityKeyIdentifier")
+                .shouldContain("SubjectKeyIdentifier")
+                .shouldContain("0000: 00 01 02 03 04 05 06 07   08 09 10 11 12 13 14 15")
+                .shouldContain("0010: 16 17 18 19")
+                .shouldHaveExitValue(0);
+
+        System.out.println("Generating an end entity cert using issuer CA's SKID as its AKID");
+        kt("-genkeypair -keyalg rsa -alias e1 -dname CN=E1", "ks");
+        kt("-certreq -alias e1 -file tmp.req", "ks");
+        kt("-gencert -alias ca -ext san=dns:e1 -infile tmp.req -outfile tmp.cert ",
+                "ks");
+        kt("-importcert -alias e1 -file tmp.cert", "ks");
+
+        byte[] expectedId = {0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07,
+                0x08, 0x09, 0x10, 0x11, 0x12, 0x13, 0x14, 0x15, 0x16, 0x17, 0x18, 0x19};
+
+        KeyStore kstore = KeyStore.getInstance(new File("ks"),
+                "changeit".toCharArray());
+        X509Certificate cert = (X509Certificate)kstore.getCertificate("e1");
+        byte[] authorityKeyIdExt = cert.getExtensionValue(
+                KnownOIDs.AuthorityKeyID.value());
+
+        byte[] authorityKeyId = null;
+        if (authorityKeyIdExt == null) {
+            System.out.println("Failed to get AKID extension from the cert");
+            System.exit(1);
+        } else {
+            try {
+                authorityKeyId = new DerValue(authorityKeyIdExt).getOctetString();
+            } catch (IOException e) {
+                System.out.println("Failed to get AKID encoded OctetString in the cert");
+                System.exit(1);
+            }
+        }
+
+        authorityKeyId = Arrays.copyOfRange(authorityKeyId, 4, authorityKeyId.length);
+        if (!Arrays.equals(authorityKeyId, expectedId)) {
+            System.out.println("Failed due to AKID mismatch");
+            System.exit(1);
+        }
+    }
+}

test/jdk/sun/security/tools/keytool/ExtOptionCamelCase.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2019, 2020, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2019, 2021, 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
@@ -23,7 +23,7 @@
 
 /*
  * @test
- * @bug 8231950
+ * @bug 8231950 8257497
  * @summary keytool -ext camel-case shorthand not working
  * @modules java.base/sun.security.tools.keytool
  *          java.base/sun.security.tools.keytool:open
@@ -38,6 +38,7 @@ import sun.security.util.DerValue;
 import sun.security.x509.BasicConstraintsExtension;
 import sun.security.x509.CertificateExtensions;
 import sun.security.x509.Extension;
+import sun.security.x509.KeyIdentifier;
 import sun.security.x509.KeyUsageExtension;
 
 import java.io.ByteArrayOutputStream;
@@ -154,7 +155,7 @@ public class ExtOptionCamelCase {
                 CertificateExtensions.class,
                 List.class,
                 PublicKey.class,
-                PublicKey.class);
+                KeyIdentifier.class);
         createV3Extensions.setAccessible(true);
         ctor = Main.class.getDeclaredConstructor();
         ctor.setAccessible(true);