Skip to content

Commit

Permalink
8277976: Break up SEQUENCE in X509Certificate::getSubjectAlternativeN…
Browse files Browse the repository at this point in the history
…ames and X509Certificate::getIssuerAlternativeNames in otherName

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

Reviewed-by: mullan
  • Loading branch information
wangweij committed Feb 28, 2022
1 parent 4e7fb41 commit 59b3ecc
Show file tree
Hide file tree
Showing 4 changed files with 154 additions and 22 deletions.
@@ -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
Expand Down Expand Up @@ -562,6 +562,10 @@ public List<String> getExtendedKeyUsage() throws CertificateParsingException {
* 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}
Expand All @@ -571,7 +575,7 @@ public List<String> getExtendedKeyUsage() throws CertificateParsingException {
* {@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,
Expand All @@ -581,12 +585,18 @@ public List<String> getExtendedKeyUsage() throws CertificateParsingException {
* 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
* defined for otherNames, X.400 names, EDI party names, or any
* other type of names. They are returned as byte arrays
* containing the ASN.1 DER encoded form of the name.
* RFC 2253</a> string format. No standard string format is defined for
* X.400 names or EDI party names. They are returned as byte arrays
* containing the ASN.1 DER encoded form of the name. otherNames are also
* 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
Expand All @@ -599,6 +609,9 @@ public List<String> getExtendedKeyUsage() throws CertificateParsingException {
* 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
Expand Down Expand Up @@ -627,7 +640,8 @@ public Collection<List<?>> getSubjectAlternativeNames()
* {@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
Expand Down
31 changes: 18 additions & 13 deletions 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
Expand Down Expand Up @@ -48,10 +48,10 @@
*/
public class OtherName implements GeneralNameInterface {

private String name;
private ObjectIdentifier oid;
private byte[] nameValue = null;
private GeneralNameInterface gni = null;
private final ObjectIdentifier oid;
private final String name;
private final byte[] nameValue; // value inside [0]
private final GeneralNameInterface gni;

private static final byte TAG_VALUE = 0;

Expand Down Expand Up @@ -89,8 +89,12 @@ public OtherName(DerValue derValue) throws IOException {
DerInputStream in = derValue.toDerInputStream();

oid = in.getOID();
DerValue val = in.getDerValue();
nameValue = val.toByteArray();
DerValue derValue1 = in.getDerValue();
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();
Expand Down Expand Up @@ -125,12 +129,13 @@ private GeneralNameInterface getGNI(ObjectIdentifier oid, byte[] nameValue)
return null;
}
Class<?>[] params = { Object.class };
Constructor<?> cons = extClass.getConstructor(params);

Object[] passed = new Object[] { nameValue };
GeneralNameInterface gni =
(GeneralNameInterface)cons.newInstance(passed);
return gni;
Constructor<?> cons;
try {
cons = extClass.getConstructor(Object.class);
} catch (NoSuchMethodException e) {
cons = extClass.getConstructor(byte[].class);
}
return (GeneralNameInterface)cons.newInstance(nameValue);
} catch (Exception e) {
throw new IOException("Instantiation error: " + e, e);
}
Expand Down
13 changes: 12 additions & 1 deletion 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
Expand Down Expand Up @@ -1584,6 +1584,17 @@ private static Collection<List<?>> makeAltNames(GeneralNames names) {
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));
Expand Down
102 changes: 102 additions & 0 deletions 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);
}
}

1 comment on commit 59b3ecc

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.