8277976: Break up SEQUENCE in X509Certificate::getSubjectAlternativeNames and X509Certificate::getIssuerAlternativeNames in otherName

6776681: Invalid encoding of an OtherName in X509Certificate.getAlternativeNames()

Reviewed-by: mullan

src/java.base/share/classes/java/security/cert/X509Certificate.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1997, 2020, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1997, 2022, 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
@@ -562,6 +562,10 @@ implements X509Extension {
      *      uniformResourceIdentifier       [6]     IA5String,
      *      iPAddress                       [7]     OCTET STRING,
      *      registeredID                    [8]     OBJECT IDENTIFIER}
+     *
+     * OtherName ::= SEQUENCE {
+     *      type-id    OBJECT IDENTIFIER,
+     *      value      [0] EXPLICIT ANY DEFINED BY type-id }
      * </pre>
      * <p>
      * If this certificate does not contain a {@code SubjectAltName}
@@ -571,7 +575,7 @@ implements X509Extension {
      * {@code List} whose first entry is an {@code Integer}
      * (the name type, 0-8) and whose second entry is a {@code String}
      * or a byte array (the name, in string or ASN.1 DER encoded form,
-     * respectively).
+     * respectively). More entries may exist depending on the name type.
      * <p>
      * <a href="http://www.ietf.org/rfc/rfc822.txt">RFC 822</a>, DNS, and URI
      * names are returned as {@code String}s,
@@ -581,12 +585,18 @@ implements X509Extension {
      * in the form "a1:a2:...:a8", where a1-a8 are hexadecimal values
      * representing the eight 16-bit pieces of the address. OID names are
      * returned as {@code String}s represented as a series of nonnegative
-     * integers separated by periods. And directory names (distinguished names)
+     * integers separated by periods. Directory names (distinguished names)
      * are returned in <a href="http://www.ietf.org/rfc/rfc2253.txt">
-     * RFC 2253</a> string format. No standard string format is
+     * RFC 2253</a> string format. No standard string format is defined for
-     * defined for otherNames, X.400 names, EDI party names, or any
+     * X.400 names or EDI party names. They are returned as byte arrays
-     * other type of names. They are returned as byte arrays
+     * containing the ASN.1 DER encoded form of the name. otherNames are also
-     * containing the ASN.1 DER encoded form of the name.
+     * returned as byte arrays containing the ASN.1 DER encoded form of the
+     * name. A third entry may also be present in the list containing the
+     * {@code type-id} of the otherName in string form, and a fourth entry
+     * containing its {@code value} as either a string (if the value is
+     * a valid supported character string) or a byte array containing the
+     * ASN.1 DER encoded form of the value without the context-specific
+     * constructed tag with number 0.
      * <p>
      * Note that the {@code Collection} returned may contain more
      * than one name of the same type. Also, note that the returned
@@ -599,6 +609,9 @@ implements X509Extension {
      * and it provides a default implementation. Subclasses
      * should override this method with a correct implementation.
      *
+     * @implNote The JDK SUN provider supports the third and fourth
+     * otherName entries.
+     *
      * @return an immutable {@code Collection} of subject alternative
      * names (or {@code null})
      * @throws CertificateParsingException if the extension cannot be decoded
@@ -627,7 +640,8 @@ implements X509Extension {
      * {@code List} whose first entry is an {@code Integer}
      * (the name type, 0-8) and whose second entry is a {@code String}
      * or a byte array (the name, in string or ASN.1 DER encoded form,
-     * respectively). For more details about the formats used for each
+     * respectively).  More entries may exist depending on the name type.
+     * For more details about the formats used for each
      * name type, see the {@code getSubjectAlternativeNames} method.
      * <p>
      * Note that the {@code Collection} returned may contain more

src/java.base/share/classes/sun/security/x509/OtherName.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1998, 2015, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1998, 2022, 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
@@ -48,10 +48,10 @@ import sun.security.util.*;
  */
 public class OtherName implements GeneralNameInterface {
 
-    private String name;
+    private final ObjectIdentifier oid;
-    private ObjectIdentifier oid;
+    private final String name;
-    private byte[] nameValue = null;
+    private final byte[] nameValue; // value inside [0]
-    private GeneralNameInterface gni = null;
+    private final GeneralNameInterface gni;
 
     private static final byte TAG_VALUE = 0;
 
@@ -89,8 +89,12 @@ public class OtherName implements GeneralNameInterface {
         DerInputStream in = derValue.toDerInputStream();
 
         oid = in.getOID();
-        DerValue val = in.getDerValue();
+        DerValue derValue1 = in.getDerValue();
-        nameValue = val.toByteArray();
+        if (derValue1.isContextSpecific((byte) 0) && derValue1.isConstructed()) {
+            nameValue = derValue1.data.toByteArray();
+        } else {
+            throw new IOException("value is not EXPLICTly tagged [0]");
+        }
         gni = getGNI(oid, nameValue);
         if (gni != null) {
             name = gni.toString();
@@ -125,12 +129,13 @@ public class OtherName implements GeneralNameInterface {
                 return null;
             }
             Class<?>[] params = { Object.class };
-            Constructor<?> cons = extClass.getConstructor(params);
+            Constructor<?> cons;
-
+            try {
-            Object[] passed = new Object[] { nameValue };
+                cons = extClass.getConstructor(Object.class);
-            GeneralNameInterface gni =
+            } catch (NoSuchMethodException e) {
-                       (GeneralNameInterface)cons.newInstance(passed);
+                cons = extClass.getConstructor(byte[].class);
-            return gni;
+            }
+            return (GeneralNameInterface)cons.newInstance(nameValue);
         } catch (Exception e) {
             throw new IOException("Instantiation error: " + e, e);
         }

src/java.base/share/classes/sun/security/x509/X509CertImpl.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1996, 2021, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1996, 2022, 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
@@ -1584,6 +1584,17 @@ public class X509CertImpl extends X509Certificate implements DerEncoder {
                     throw new RuntimeException("name cannot be encoded", ioe);
                 }
                 nameEntry.add(derOut.toByteArray());
+                if (name.getType() == GeneralNameInterface.NAME_ANY
+                        && name instanceof OtherName oname) {
+                    nameEntry.add(oname.getOID().toString());
+                    byte[] nameValue = oname.getNameValue();
+                    try {
+                        String v = new DerValue(nameValue).getAsString();
+                        nameEntry.add(v == null ? nameValue : v);
+                    } catch (IOException ioe) {
+                        nameEntry.add(nameValue);
+                    }
+                }
                 break;
             }
             newNames.add(Collections.unmodifiableList(nameEntry));

test/jdk/sun/security/x509/OtherName/Parse.java

@@ -0,0 +1,102 @@
+/*
+ * Copyright (c) 2022, 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 8277976
+ * @summary Break up SEQUENCE in X509Certificate::getSubjectAlternativeNames
+ *          and X509Certificate::getIssuerAlternativeNames in otherName
+ * @modules java.base/sun.security.util
+ *          java.base/sun.security.x509
+ *          java.base/sun.security.tools.keytool
+ * @library /test/lib
+ */
+
+import jdk.test.lib.Asserts;
+import sun.security.tools.keytool.CertAndKeyGen;
+import sun.security.util.DerValue;
+import sun.security.util.ObjectIdentifier;
+import sun.security.x509.CertificateExtensions;
+import sun.security.x509.DNSName;
+import sun.security.x509.GeneralName;
+import sun.security.x509.GeneralNames;
+import sun.security.x509.OIDMap;
+import sun.security.x509.OtherName;
+import sun.security.x509.SubjectAlternativeNameExtension;
+import sun.security.x509.X500Name;
+
+import java.io.IOException;
+import java.nio.charset.StandardCharsets;
+import java.security.cert.X509Certificate;
+import java.util.Arrays;
+import java.util.Date;
+
+public class Parse {
+
+    public static class MyDNSName extends DNSName {
+        public MyDNSName(byte[] in) throws IOException {
+            super(new String(Arrays.copyOfRange(in, 2, in.length),
+                    StandardCharsets.US_ASCII));
+        }
+    }
+
+    public static void main(String[] args) throws Exception {
+        OIDMap.addAttribute("n1", "1.2.3.6", MyDNSName.class);
+
+        CertificateExtensions exts = new CertificateExtensions();
+        GeneralNames names = new GeneralNames();
+
+        byte[] d1 = new byte[] {
+                DerValue.tag_OctetString, 5, 'a', '.', 'c', 'o', 'm' };
+        names.add(new GeneralName(
+                new OtherName(ObjectIdentifier.of("1.2.3.5"), d1)));
+
+        byte[] d2 = new byte[] {
+                DerValue.tag_UTF8String, 5, 'a', '.', 'c', 'o', 'm' };
+        names.add(new GeneralName(
+                new OtherName(ObjectIdentifier.of("1.2.3.6"), d2)));
+
+        exts.set("x", new SubjectAlternativeNameExtension(names));
+        CertAndKeyGen g = new CertAndKeyGen("Ed25519", "Ed25519");
+        g.generate(-1);
+        X509Certificate x = g.getSelfCertificate(new X500Name("CN=ME"),
+                new Date(),
+                100000,
+                exts);
+
+        int found = 0;
+        for (var san : x.getSubjectAlternativeNames()) {
+            if (san.size() >= 4 && san.get(0).equals(0)) {
+                if (san.get(2).equals("1.2.3.5")) {
+                    Asserts.assertTrue(Arrays.equals((byte[]) san.get(3), d1));
+                    found++;
+                } else if (san.get(2).equals("1.2.3.6")) {
+                    Asserts.assertEQ(san.get(3), "a.com");
+                    found++;
+                }
+            }
+        }
+        Asserts.assertEQ(found, 2);
+    }
+}
+