diff --git a/src/share/classes/sun/security/x509/CRLExtensions.java b/src/share/classes/sun/security/x509/CRLExtensions.java index 7b6c34b54a045c6ac11ecc25f326a87d9b7a05a6..dc1fc71968c792a7296f2e1c37d350a565ef4dd8 100644 --- a/src/share/classes/sun/security/x509/CRLExtensions.java +++ b/src/share/classes/sun/security/x509/CRLExtensions.java @@ -32,8 +32,10 @@ import java.lang.reflect.InvocationTargetException; import java.security.cert.CRLException; import java.security.cert.CertificateException; import java.util.Collection; +import java.util.Collections; import java.util.Enumeration; -import java.util.Hashtable; +import java.util.Map; +import java.util.TreeMap; import sun.security.util.*; import sun.misc.HexDumpEncoder; @@ -62,7 +64,8 @@ import sun.misc.HexDumpEncoder; */ public class CRLExtensions { - private Hashtable map = new Hashtable(); + private Map map = Collections.synchronizedMap( + new TreeMap()); private boolean unsupportedCritExt = false; /** @@ -215,7 +218,7 @@ public class CRLExtensions { * @return an enumeration of the extensions in this CRL. */ public Enumeration getElements() { - return map.elements(); + return Collections.enumeration(map.values()); } /** diff --git a/src/share/classes/sun/security/x509/CertificateExtensions.java b/src/share/classes/sun/security/x509/CertificateExtensions.java index b1dc93675120018510b4e25750d927465387d8fa..65dcff1a3e170d4a4caad3cfdba5b802d16dca1c 100644 --- a/src/share/classes/sun/security/x509/CertificateExtensions.java +++ b/src/share/classes/sun/security/x509/CertificateExtensions.java @@ -57,7 +57,8 @@ public class CertificateExtensions implements CertAttrSet { private static final Debug debug = Debug.getInstance("x509"); - private Hashtable map = new Hashtable(); + private Map map = Collections.synchronizedMap( + new TreeMap()); private boolean unsupportedCritExt = false; private Map unparseableExtensions; @@ -117,7 +118,7 @@ public class CertificateExtensions implements CertAttrSet { if (ext.isCritical() == false) { // ignore errors parsing non-critical extensions if (unparseableExtensions == null) { - unparseableExtensions = new HashMap(); + unparseableExtensions = new TreeMap(); } unparseableExtensions.put(ext.getExtensionId().toString(), new UnparseableExtension(ext, e)); @@ -218,6 +219,12 @@ public class CertificateExtensions implements CertAttrSet { return (obj); } + // Similar to get(String), but throw no exception, might return null. + // Used in X509CertImpl::getExtension(OID). + Extension getExtension(String name) { + return map.get(name); + } + /** * Delete the attribute value. * @param name the extension name used in the lookup. @@ -245,7 +252,7 @@ public class CertificateExtensions implements CertAttrSet { * attribute. */ public Enumeration getElements() { - return map.elements(); + return Collections.enumeration(map.values()); } /** diff --git a/src/share/classes/sun/security/x509/X509CRLEntryImpl.java b/src/share/classes/sun/security/x509/X509CRLEntryImpl.java index 495cc06e5be7bdbf4631ecc080ed355a9c9b2b8e..a074c38720baa4a1b653f3b888bdbb8bca1d2388 100644 --- a/src/share/classes/sun/security/x509/X509CRLEntryImpl.java +++ b/src/share/classes/sun/security/x509/X509CRLEntryImpl.java @@ -32,13 +32,7 @@ import java.security.cert.CRLReason; import java.security.cert.CertificateException; import java.security.cert.X509CRLEntry; import java.math.BigInteger; -import java.util.Collection; -import java.util.Date; -import java.util.Enumeration; -import java.util.HashMap; -import java.util.Map; -import java.util.Set; -import java.util.HashSet; +import java.util.*; import javax.security.auth.x500.X500Principal; @@ -75,7 +69,8 @@ import sun.misc.HexDumpEncoder; * @author Hemma Prafullchandra */ -public class X509CRLEntryImpl extends X509CRLEntry { +public class X509CRLEntryImpl extends X509CRLEntry + implements Comparable { private SerialNumber serialNumber = null; private Date revocationDate = null; @@ -196,9 +191,14 @@ public class X509CRLEntryImpl extends X509CRLEntry { * @exception CRLException if an encoding error occurs. */ public byte[] getEncoded() throws CRLException { + return getEncoded0().clone(); + } + + // Called internally to avoid clone + private byte[] getEncoded0() throws CRLException { if (revokedCert == null) this.encode(new DerOutputStream()); - return revokedCert.clone(); + return revokedCert; } @Override @@ -352,7 +352,7 @@ public class X509CRLEntryImpl extends X509CRLEntry { if (extensions == null) { return null; } - Set extSet = new HashSet(); + Set extSet = new TreeSet<>(); for (Extension ex : extensions.getAllExtensions()) { if (ex.isCritical()) { extSet.add(ex.getExtensionId().toString()); @@ -373,7 +373,7 @@ public class X509CRLEntryImpl extends X509CRLEntry { if (extensions == null) { return null; } - Set extSet = new HashSet(); + Set extSet = new TreeSet<>(); for (Extension ex : extensions.getAllExtensions()) { if (!ex.isCritical()) { extSet.add(ex.getExtensionId().toString()); @@ -501,13 +501,39 @@ public class X509CRLEntryImpl extends X509CRLEntry { getExtension(PKIXExtensions.CertificateIssuer_Id); } + /** + * Returns all extensions for this entry in a map + * @return the extension map, can be empty, but not null + */ public Map getExtensions() { + if (extensions == null) { + return Collections.emptyMap(); + } Collection exts = extensions.getAllExtensions(); - HashMap map = - new HashMap(exts.size()); + Map map = new TreeMap<>(); for (Extension ext : exts) { map.put(ext.getId(), ext); } return map; } + + @Override + public int compareTo(X509CRLEntryImpl that) { + int compSerial = getSerialNumber().compareTo(that.getSerialNumber()); + if (compSerial != 0) { + return compSerial; + } + try { + byte[] thisEncoded = this.getEncoded0(); + byte[] thatEncoded = that.getEncoded0(); + for (int i=0; i - * An implmentation for X509 CRL (Certificate Revocation List). + * An implementation for X509 CRL (Certificate Revocation List). *

* The X.509 v2 CRL format is described below in ASN.1: *

@@ -104,7 +104,8 @@ public class X509CRLImpl extends X509CRL implements DerEncoder {
     private X500Principal    issuerPrincipal = null;
     private Date             thisUpdate = null;
     private Date             nextUpdate = null;
-    private Map revokedCerts = new LinkedHashMap();
+    private Map revokedMap = new TreeMap<>();
+    private List revokedList = new LinkedList<>();
     private CRLExtensions    extensions = null;
     private final static boolean isExplicit = true;
     private static final long YR_2050 = 2524636800000L;
@@ -223,7 +224,8 @@ public class X509CRLImpl extends X509CRL implements DerEncoder {
                 badCert.setCertificateIssuer(crlIssuer, badCertIssuer);
                 X509IssuerSerial issuerSerial = new X509IssuerSerial
                     (badCertIssuer, badCert.getSerialNumber());
-                this.revokedCerts.put(issuerSerial, badCert);
+                this.revokedMap.put(issuerSerial, badCert);
+                this.revokedList.add(badCert);
                 if (badCert.hasExtensions()) {
                     this.version = 1;
                 }
@@ -305,8 +307,8 @@ public class X509CRLImpl extends X509CRL implements DerEncoder {
                     tmp.putGeneralizedTime(nextUpdate);
             }
 
-            if (!revokedCerts.isEmpty()) {
-                for (X509CRLEntry entry : revokedCerts.values()) {
+            if (!revokedList.isEmpty()) {
+                for (X509CRLEntry entry : revokedList) {
                     ((X509CRLEntryImpl)entry).encode(rCerts);
                 }
                 tmp.write(DerValue.tag_Sequence, rCerts);
@@ -490,14 +492,14 @@ public class X509CRLImpl extends X509CRL implements DerEncoder {
             sb.append("\nThis Update: " + thisUpdate.toString() + "\n");
         if (nextUpdate != null)
             sb.append("Next Update: " + nextUpdate.toString() + "\n");
-        if (revokedCerts.isEmpty())
+        if (revokedList.isEmpty())
             sb.append("\nNO certificates have been revoked\n");
         else {
-            sb.append("\nRevoked Certificates: " + revokedCerts.size());
+            sb.append("\nRevoked Certificates: " + revokedList.size());
             int i = 1;
-            for (Iterator iter = revokedCerts.values().iterator();
-                                             iter.hasNext(); i++)
-                sb.append("\n[" + i + "] " + iter.next().toString());
+            for (X509CRLEntry entry: revokedList) {
+                sb.append("\n[" + i++ + "] " + entry.toString());
+            }
         }
         if (extensions != null) {
             Collection allExts = extensions.getAllExtensions();
@@ -543,12 +545,12 @@ public class X509CRLImpl extends X509CRL implements DerEncoder {
      * false otherwise.
      */
     public boolean isRevoked(Certificate cert) {
-        if (revokedCerts.isEmpty() || (!(cert instanceof X509Certificate))) {
+        if (revokedMap.isEmpty() || (!(cert instanceof X509Certificate))) {
             return false;
         }
         X509Certificate xcert = (X509Certificate) cert;
         X509IssuerSerial issuerSerial = new X509IssuerSerial(xcert);
-        return revokedCerts.containsKey(issuerSerial);
+        return revokedMap.containsKey(issuerSerial);
     }
 
     /**
@@ -638,24 +640,24 @@ public class X509CRLImpl extends X509CRL implements DerEncoder {
      * @see X509CRLEntry
      */
     public X509CRLEntry getRevokedCertificate(BigInteger serialNumber) {
-        if (revokedCerts.isEmpty()) {
+        if (revokedMap.isEmpty()) {
             return null;
         }
         // assume this is a direct CRL entry (cert and CRL issuer are the same)
         X509IssuerSerial issuerSerial = new X509IssuerSerial
             (getIssuerX500Principal(), serialNumber);
-        return revokedCerts.get(issuerSerial);
+        return revokedMap.get(issuerSerial);
     }
 
     /**
      * Gets the CRL entry for the given certificate.
      */
     public X509CRLEntry getRevokedCertificate(X509Certificate cert) {
-        if (revokedCerts.isEmpty()) {
+        if (revokedMap.isEmpty()) {
             return null;
         }
         X509IssuerSerial issuerSerial = new X509IssuerSerial(cert);
-        return revokedCerts.get(issuerSerial);
+        return revokedMap.get(issuerSerial);
     }
 
     /**
@@ -667,10 +669,10 @@ public class X509CRLImpl extends X509CRL implements DerEncoder {
      * @see X509CRLEntry
      */
     public Set getRevokedCertificates() {
-        if (revokedCerts.isEmpty()) {
+        if (revokedList.isEmpty()) {
             return null;
         } else {
-            return new HashSet(revokedCerts.values());
+            return new TreeSet(revokedList);
         }
     }
 
@@ -905,7 +907,7 @@ public class X509CRLImpl extends X509CRL implements DerEncoder {
         if (extensions == null) {
             return null;
         }
-        Set extSet = new HashSet();
+        Set extSet = new TreeSet<>();
         for (Extension ex : extensions.getAllExtensions()) {
             if (ex.isCritical()) {
                 extSet.add(ex.getExtensionId().toString());
@@ -926,7 +928,7 @@ public class X509CRLImpl extends X509CRL implements DerEncoder {
         if (extensions == null) {
             return null;
         }
-        Set extSet = new HashSet();
+        Set extSet = new TreeSet<>();
         for (Extension ex : extensions.getAllExtensions()) {
             if (!ex.isCritical()) {
                 extSet.add(ex.getExtensionId().toString());
@@ -1103,7 +1105,8 @@ public class X509CRLImpl extends X509CRL implements DerEncoder {
                 entry.setCertificateIssuer(crlIssuer, badCertIssuer);
                 X509IssuerSerial issuerSerial = new X509IssuerSerial
                     (badCertIssuer, entry.getSerialNumber());
-                revokedCerts.put(issuerSerial, entry);
+                revokedMap.put(issuerSerial, entry);
+                revokedList.add(entry);
             }
         }
 
@@ -1208,7 +1211,8 @@ public class X509CRLImpl extends X509CRL implements DerEncoder {
     /**
      * Immutable X.509 Certificate Issuer DN and serial number pair
      */
-    private final static class X509IssuerSerial {
+    private final static class X509IssuerSerial
+            implements Comparable {
         final X500Principal issuer;
         final BigInteger serial;
         volatile int hashcode = 0;
@@ -1287,5 +1291,13 @@ public class X509CRLImpl extends X509CRL implements DerEncoder {
             }
             return hashcode;
         }
+
+        @Override
+        public int compareTo(X509IssuerSerial another) {
+            int cissuer = issuer.toString()
+                    .compareTo(another.issuer.toString());
+            if (cissuer != 0) return cissuer;
+            return this.serial.compareTo(another.serial);
+        }
     }
 }
diff --git a/src/share/classes/sun/security/x509/X509CertImpl.java b/src/share/classes/sun/security/x509/X509CertImpl.java
index eb7dc39c0bf8610cf9cbe8fe5303993642f58c47..64ac1c3617632131926edaba222603d0aa3cd805 100644
--- a/src/share/classes/sun/security/x509/X509CertImpl.java
+++ b/src/share/classes/sun/security/x509/X509CertImpl.java
@@ -1214,7 +1214,7 @@ public class X509CertImpl extends X509Certificate implements DerEncoder {
             if (exts == null) {
                 return null;
             }
-            Set extSet = new HashSet();
+            Set extSet = new TreeSet<>();
             for (Extension ex : exts.getAllExtensions()) {
                 if (ex.isCritical()) {
                     extSet.add(ex.getExtensionId().toString());
@@ -1244,7 +1244,7 @@ public class X509CertImpl extends X509Certificate implements DerEncoder {
             if (exts == null) {
                 return null;
             }
-            Set extSet = new HashSet();
+            Set extSet = new TreeSet<>();
             for (Extension ex : exts.getAllExtensions()) {
                 if (!ex.isCritical()) {
                     extSet.add(ex.getExtensionId().toString());
@@ -1278,10 +1278,14 @@ public class X509CertImpl extends X509Certificate implements DerEncoder {
             if (extensions == null) {
                 return null;
             } else {
-                for (Extension ex : extensions.getAllExtensions()) {
-                    if (ex.getExtensionId().equals(oid)) {
+                Extension ex = extensions.getExtension(oid.toString());
+                if (ex != null) {
+                    return ex;
+                }
+                for (Extension ex2: extensions.getAllExtensions()) {
+                    if (ex2.getExtensionId().equals((Object)oid)) {
                         //XXXX May want to consider cloning this
-                        return ex;
+                        return ex2;
                     }
                 }
                 /* no such extension in this certificate */
@@ -1480,10 +1484,10 @@ public class X509CertImpl extends X509Certificate implements DerEncoder {
         if (names.isEmpty()) {
             return Collections.>emptySet();
         }
-        Set> newNames = new HashSet>();
+        List> newNames = new ArrayList<>();
         for (GeneralName gname : names.names()) {
             GeneralNameInterface name = gname.getName();
-            List nameEntry = new ArrayList(2);
+            List nameEntry = new ArrayList<>(2);
             nameEntry.add(Integer.valueOf(name.getType()));
             switch (name.getType()) {
             case GeneralNameInterface.NAME_RFC822:
@@ -1541,12 +1545,12 @@ public class X509CertImpl extends X509Certificate implements DerEncoder {
             }
         }
         if (mustClone) {
-            Set> namesCopy = new HashSet>();
+            List> namesCopy = new ArrayList<>();
             for (List nameEntry : altNames) {
                 Object nameObject = nameEntry.get(1);
                 if (nameObject instanceof byte[]) {
                     List nameEntryCopy =
-                                        new ArrayList(nameEntry);
+                                        new ArrayList<>(nameEntry);
                     nameEntryCopy.set(1, ((byte[])nameObject).clone());
                     namesCopy.add(Collections.unmodifiableList(nameEntryCopy));
                 } else {
diff --git a/test/sun/security/x509/X509CRLImpl/OrderAndDup.java b/test/sun/security/x509/X509CRLImpl/OrderAndDup.java
new file mode 100644
index 0000000000000000000000000000000000000000..51610e61c7f85953d415706f3b8462423c0e97ea
--- /dev/null
+++ b/test/sun/security/x509/X509CRLImpl/OrderAndDup.java
@@ -0,0 +1,99 @@
+/*
+ * Copyright (c) 2012, 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 7143872
+ * @summary Improve certificate extension processing
+ */
+import java.io.ByteArrayInputStream;
+import java.math.BigInteger;
+import java.security.KeyPairGenerator;
+import java.security.cert.CertificateFactory;
+import java.security.cert.X509CRLEntry;
+import java.util.Date;
+import sun.security.util.DerInputStream;
+import sun.security.util.DerValue;
+import sun.security.x509.*;
+
+public class OrderAndDup {
+    public static void main(String[] args) throws Exception {
+
+        // Generate 20 serial numbers with dup and a special order
+        int count = 20;
+        BigInteger[] serials = new BigInteger[count];
+        for (int i=0; i