Mercurial > hg > icedtea9-forest > jdk
changeset 5743:8516c5b4521b
7143872: Improve certificate extension processing
Reviewed-by: mullan
author | weijun |
---|---|
date | Wed, 29 Feb 2012 14:06:00 +0800 |
parents | 9e6e535a6769 |
children | 3640f1a043f8 |
files | src/share/classes/sun/security/x509/CRLExtensions.java src/share/classes/sun/security/x509/CertificateExtensions.java src/share/classes/sun/security/x509/X509CRLEntryImpl.java src/share/classes/sun/security/x509/X509CRLImpl.java src/share/classes/sun/security/x509/X509CertImpl.java test/sun/security/x509/X509CRLImpl/OrderAndDup.java |
diffstat | 6 files changed, 201 insertions(+), 50 deletions(-) [+] |
line wrap: on
line diff
--- a/src/share/classes/sun/security/x509/CRLExtensions.java Tue Feb 28 16:09:15 2012 +0200 +++ b/src/share/classes/sun/security/x509/CRLExtensions.java Wed Feb 29 14:06:00 2012 +0800 @@ -32,8 +32,10 @@ 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 @@ */ public class CRLExtensions { - private Hashtable<String,Extension> map = new Hashtable<String,Extension>(); + private Map<String,Extension> map = Collections.synchronizedMap( + new TreeMap<String,Extension>()); private boolean unsupportedCritExt = false; /** @@ -215,7 +218,7 @@ * @return an enumeration of the extensions in this CRL. */ public Enumeration<Extension> getElements() { - return map.elements(); + return Collections.enumeration(map.values()); } /**
--- a/src/share/classes/sun/security/x509/CertificateExtensions.java Tue Feb 28 16:09:15 2012 +0200 +++ b/src/share/classes/sun/security/x509/CertificateExtensions.java Wed Feb 29 14:06:00 2012 +0800 @@ -57,7 +57,8 @@ private static final Debug debug = Debug.getInstance("x509"); - private Hashtable<String,Extension> map = new Hashtable<String,Extension>(); + private Map<String,Extension> map = Collections.synchronizedMap( + new TreeMap<String,Extension>()); private boolean unsupportedCritExt = false; private Map<String,Extension> unparseableExtensions; @@ -117,7 +118,7 @@ if (ext.isCritical() == false) { // ignore errors parsing non-critical extensions if (unparseableExtensions == null) { - unparseableExtensions = new HashMap<String,Extension>(); + unparseableExtensions = new TreeMap<String,Extension>(); } unparseableExtensions.put(ext.getExtensionId().toString(), new UnparseableExtension(ext, e)); @@ -218,6 +219,12 @@ 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 @@ * attribute. */ public Enumeration<Extension> getElements() { - return map.elements(); + return Collections.enumeration(map.values()); } /**
--- a/src/share/classes/sun/security/x509/X509CRLEntryImpl.java Tue Feb 28 16:09:15 2012 +0200 +++ b/src/share/classes/sun/security/x509/X509CRLEntryImpl.java Wed Feb 29 14:06:00 2012 +0800 @@ -32,13 +32,7 @@ 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 @@ * @author Hemma Prafullchandra */ -public class X509CRLEntryImpl extends X509CRLEntry { +public class X509CRLEntryImpl extends X509CRLEntry + implements Comparable<X509CRLEntryImpl> { private SerialNumber serialNumber = null; private Date revocationDate = null; @@ -196,9 +191,14 @@ * @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 @@ if (extensions == null) { return null; } - Set<String> extSet = new HashSet<String>(); + Set<String> extSet = new TreeSet<>(); for (Extension ex : extensions.getAllExtensions()) { if (ex.isCritical()) { extSet.add(ex.getExtensionId().toString()); @@ -373,7 +373,7 @@ if (extensions == null) { return null; } - Set<String> extSet = new HashSet<String>(); + Set<String> extSet = new TreeSet<>(); for (Extension ex : extensions.getAllExtensions()) { if (!ex.isCritical()) { extSet.add(ex.getExtensionId().toString()); @@ -501,13 +501,39 @@ 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<String, java.security.cert.Extension> getExtensions() { + if (extensions == null) { + return Collections.emptyMap(); + } Collection<Extension> exts = extensions.getAllExtensions(); - HashMap<String, java.security.cert.Extension> map = - new HashMap<String, java.security.cert.Extension>(exts.size()); + Map<String, java.security.cert.Extension> 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<thisEncoded.length && i<thatEncoded.length; i++) { + int a = thisEncoded[i] & 0xff; + int b = thatEncoded[i] & 0xff; + if (a != b) return a-b; + } + return thisEncoded.length -thatEncoded.length; + } catch (CRLException ce) { + return -1; + } + } }
--- a/src/share/classes/sun/security/x509/X509CRLImpl.java Tue Feb 28 16:09:15 2012 +0200 +++ b/src/share/classes/sun/security/x509/X509CRLImpl.java Wed Feb 29 14:06:00 2012 +0800 @@ -53,7 +53,7 @@ /** * <p> - * An implmentation for X509 CRL (Certificate Revocation List). + * An implementation for X509 CRL (Certificate Revocation List). * <p> * The X.509 v2 CRL format is described below in ASN.1: * <pre> @@ -104,7 +104,8 @@ private X500Principal issuerPrincipal = null; private Date thisUpdate = null; private Date nextUpdate = null; - private Map<X509IssuerSerial,X509CRLEntry> revokedCerts = new LinkedHashMap<X509IssuerSerial,X509CRLEntry>(); + private Map<X509IssuerSerial,X509CRLEntry> revokedMap = new TreeMap<>(); + private List<X509CRLEntry> revokedList = new LinkedList<>(); private CRLExtensions extensions = null; private final static boolean isExplicit = true; private static final long YR_2050 = 2524636800000L; @@ -223,7 +224,8 @@ 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 @@ 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 @@ 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<X509CRLEntry> 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<Extension> allExts = extensions.getAllExtensions(); @@ -543,12 +545,12 @@ * 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 @@ * @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 @@ * @see X509CRLEntry */ public Set<X509CRLEntry> getRevokedCertificates() { - if (revokedCerts.isEmpty()) { + if (revokedList.isEmpty()) { return null; } else { - return new HashSet<X509CRLEntry>(revokedCerts.values()); + return new TreeSet<X509CRLEntry>(revokedList); } } @@ -905,7 +907,7 @@ if (extensions == null) { return null; } - Set<String> extSet = new HashSet<String>(); + Set<String> extSet = new TreeSet<>(); for (Extension ex : extensions.getAllExtensions()) { if (ex.isCritical()) { extSet.add(ex.getExtensionId().toString()); @@ -926,7 +928,7 @@ if (extensions == null) { return null; } - Set<String> extSet = new HashSet<String>(); + Set<String> extSet = new TreeSet<>(); for (Extension ex : extensions.getAllExtensions()) { if (!ex.isCritical()) { extSet.add(ex.getExtensionId().toString()); @@ -1103,7 +1105,8 @@ 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 @@ /** * Immutable X.509 Certificate Issuer DN and serial number pair */ - private final static class X509IssuerSerial { + private final static class X509IssuerSerial + implements Comparable<X509IssuerSerial> { final X500Principal issuer; final BigInteger serial; volatile int hashcode = 0; @@ -1287,5 +1291,13 @@ } 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); + } } }
--- a/src/share/classes/sun/security/x509/X509CertImpl.java Tue Feb 28 16:09:15 2012 +0200 +++ b/src/share/classes/sun/security/x509/X509CertImpl.java Wed Feb 29 14:06:00 2012 +0800 @@ -1214,7 +1214,7 @@ if (exts == null) { return null; } - Set<String> extSet = new HashSet<String>(); + Set<String> extSet = new TreeSet<>(); for (Extension ex : exts.getAllExtensions()) { if (ex.isCritical()) { extSet.add(ex.getExtensionId().toString()); @@ -1244,7 +1244,7 @@ if (exts == null) { return null; } - Set<String> extSet = new HashSet<String>(); + Set<String> extSet = new TreeSet<>(); for (Extension ex : exts.getAllExtensions()) { if (!ex.isCritical()) { extSet.add(ex.getExtensionId().toString()); @@ -1278,10 +1278,14 @@ 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 @@ if (names.isEmpty()) { return Collections.<List<?>>emptySet(); } - Set<List<?>> newNames = new HashSet<List<?>>(); + List<List<?>> newNames = new ArrayList<>(); for (GeneralName gname : names.names()) { GeneralNameInterface name = gname.getName(); - List<Object> nameEntry = new ArrayList<Object>(2); + List<Object> nameEntry = new ArrayList<>(2); nameEntry.add(Integer.valueOf(name.getType())); switch (name.getType()) { case GeneralNameInterface.NAME_RFC822: @@ -1541,12 +1545,12 @@ } } if (mustClone) { - Set<List<?>> namesCopy = new HashSet<List<?>>(); + List<List<?>> namesCopy = new ArrayList<>(); for (List<?> nameEntry : altNames) { Object nameObject = nameEntry.get(1); if (nameObject instanceof byte[]) { List<Object> nameEntryCopy = - new ArrayList<Object>(nameEntry); + new ArrayList<>(nameEntry); nameEntryCopy.set(1, ((byte[])nameObject).clone()); namesCopy.add(Collections.unmodifiableList(nameEntryCopy)); } else {
--- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/test/sun/security/x509/X509CRLImpl/OrderAndDup.java Wed Feb 29 14:06:00 2012 +0800 @@ -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<count; i++) { + serials[i] = BigInteger.valueOf(i*7%10); + } + + // Generates a CRL + X509CRLEntry[] badCerts = new X509CRLEntry[count]; + for (int i=0; i<count; i++) { + badCerts[i] = new X509CRLEntryImpl(serials[i], + new Date(System.currentTimeMillis()+i*1000)); + } + X500Name owner = new X500Name("CN=CA"); + X509CRLImpl crl = new X509CRLImpl(owner, new Date(), new Date(), badCerts); + KeyPairGenerator kpg = KeyPairGenerator.getInstance("RSA"); + crl.sign(kpg.genKeyPair().getPrivate(), "SHA1withRSA"); + byte[] data = crl.getEncodedInternal(); + + // Check the encoding + checkData(crl, data, serials); + + // Load a CRL from raw data + CertificateFactory cf = CertificateFactory.getInstance("X.509"); + X509CRLImpl crl2 = (X509CRLImpl)cf.generateCRL(new ByteArrayInputStream(data)); + + // Check the encoding again + data = crl2.getEncodedInternal(); + checkData(crl2, data, serials); + } + + // Check the raw data's ASN.1 structure to see if the revoked certs + // have the same number and correct order as inserted + static void checkData(X509CRLImpl c, byte[] data, BigInteger[] expected) + throws Exception { + if (c.getRevokedCertificates().size() != expected.length) { + throw new Exception("Wrong count in CRL object, now " + + c.getRevokedCertificates().size()); + } + DerValue d1 = new DerValue(data); + // revokedCertificates at 5th place of TBSCertList + DerValue[] d2 = new DerInputStream( + d1.data.getSequence(0)[4].toByteArray()) + .getSequence(0); + if (d2.length != expected.length) { + throw new Exception("Wrong count in raw data, now " + d2.length); + } + for (int i=0; i<d2.length; i++) { + // Serial is first in revokedCertificates entry + BigInteger bi = d2[i].data.getBigInteger(); + if (!bi.equals(expected[i])) { + throw new Exception("Entry at #" + i + " is " + bi + + ", should be " + expected[i]); + } + } + } +} +