[WIP] Fixes Issue #1508 providing more information about certificate parsing issues

core/src/main/java/org/bouncycastle/asn1/x509/Certificate.java

@@ -7,6 +7,7 @@
 import org.bouncycastle.asn1.ASN1Sequence;
 import org.bouncycastle.asn1.ASN1TaggedObject;
 import org.bouncycastle.asn1.x500.X500Name;
+import org.bouncycastle.util.IllegalArgumentWarningException;
 
 /**
  * an X509Certificate structure.
@@ -53,12 +54,19 @@ private Certificate(
     {
         this.seq = seq;
 
+        IllegalArgumentWarningException exception = null;
+
         //
         // correct x509 certficate
         //
         if (seq.size() == 3)
         {
-            tbsCert = TBSCertificate.getInstance(seq.getObjectAt(0));
+            try {
+                tbsCert = TBSCertificate.getInstance(seq.getObjectAt(0));
+            } catch (IllegalArgumentWarningException ex) {
+                tbsCert = (TBSCertificate) ex.getObject(TBSCertificate.class);
+                exception = ex;
+            }
             sigAlgId = AlgorithmIdentifier.getInstance(seq.getObjectAt(1));
 
             sig = ASN1BitString.getInstance(seq.getObjectAt(2));
@@ -67,6 +75,10 @@ private Certificate(
         {
             throw new IllegalArgumentException("sequence wrong size for a certificate");
         }
+
+        if (exception != null) {
+            throw new IllegalArgumentWarningException(this, exception);
+        }
     }
 
     public TBSCertificate getTBSCertificate()
@@ -124,6 +136,7 @@ public ASN1BitString getSignature()
         return sig;
     }
 
+    @Override
     public ASN1Primitive toASN1Primitive()
     {
         return seq;

core/src/main/java/org/bouncycastle/asn1/x509/Extensions.java

@@ -12,6 +12,7 @@
 import org.bouncycastle.asn1.ASN1Sequence;
 import org.bouncycastle.asn1.ASN1TaggedObject;
 import org.bouncycastle.asn1.DERSequence;
+import org.bouncycastle.util.IllegalArgumentWarningException;
 import org.bouncycastle.util.Properties;
 
 /**
@@ -77,6 +78,7 @@ private Extensions(
         }
 
         Enumeration e = seq.getObjects();
+        String error = null;
 
         while (e.hasMoreElements())
         {
@@ -86,13 +88,18 @@ private Extensions(
             {
                 if (!Properties.isOverrideSet("org.bouncycastle.x509.ignore_repeated_extensions"))
                 {
-                    throw new IllegalArgumentException("repeated extension found: " + ext.getExtnId());
+                    error = "repeated extension found: " + ext.getExtnId();
+                    continue;
                 }
             }
 
             extensions.put(ext.getExtnId(), ext);
             ordering.addElement(ext.getExtnId());
         }
+
+        if (error != null) {
+            throw new IllegalArgumentWarningException(error, this);
+        }
     }
 
     /**
@@ -177,6 +184,7 @@ public ASN1Encodable getExtensionParsedValue(ASN1ObjectIdentifier oid)
      *        extnValue         OCTET STRING }
      * </pre>
      */
+    @Override
     public ASN1Primitive toASN1Primitive()
     {
         ASN1EncodableVector vec = new ASN1EncodableVector(ordering.size());

core/src/main/java/org/bouncycastle/asn1/x509/TBSCertificate.java

@@ -10,8 +10,13 @@
 import org.bouncycastle.asn1.DERSequence;
 import org.bouncycastle.asn1.DERTaggedObject;
 import org.bouncycastle.asn1.x500.X500Name;
+import org.bouncycastle.util.IllegalArgumentWarningException;
 import org.bouncycastle.util.Properties;
 
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.List;
+
 /**
  * The TBSCertificate object.
  * <pre>
@@ -47,6 +52,7 @@ public class TBSCertificate
     ASN1BitString           issuerUniqueId;
     ASN1BitString           subjectUniqueId;
     Extensions              extensions;
+    List<String>            errors;
 
     public static TBSCertificate getInstance(
         ASN1TaggedObject obj,
@@ -92,7 +98,7 @@ private TBSCertificate(
 
         boolean isV1 = false;
         boolean isV2 = false;
- 
+
         if (version.hasValue(0))
         {
             isV1 = true;
@@ -103,7 +109,8 @@ else if (version.hasValue(1))
         }
         else if (!version.hasValue(2))
         {
-            throw new IllegalArgumentException("version number not recognised");
+            addError(
+                String.format("Certificate version number value %d not 0, 1 or 2", version.getValue()));
         }
 
         serialNumber = ASN1Integer.getInstance(seq.getObjectAt(seqStart + 1));
@@ -129,9 +136,10 @@ else if (!version.hasValue(2))
         int extras = seq.size() - (seqStart + 6) - 1;
         if (extras != 0 && isV1)
         {
-            throw new IllegalArgumentException("version 1 certificate contains extra data");
+            addError("version 1 certificate contains extra data");
+            extras = 0; // Ignore the extra data
         }
-        
+
         while (extras > 0)
         {
             ASN1TaggedObject extra = (ASN1TaggedObject)seq.getObjectAt(seqStart + 6 + extras);
@@ -147,15 +155,43 @@ else if (!version.hasValue(2))
             case 3:
                 if (isV2)
                 {
-                    throw new IllegalArgumentException("version 2 certificate cannot contain extensions");
+                    addError("version 2 certificate cannot contain extensions");
+                    throw new IllegalArgumentWarningException(errors, this);
+                }
+                try {
+                    extensions = Extensions.getInstance(ASN1Sequence.getInstance(extra, true));
+                } catch (IllegalArgumentWarningException ex) {
+                    extensions = ex.getObject(Extensions.class);
+                    addErrors(ex.getMessages());
                 }
-                extensions = Extensions.getInstance(ASN1Sequence.getInstance(extra, true));
                 break;
             default:
-                throw new IllegalArgumentException("Unknown tag encountered in structure: " + extra.getTagNo());
+                addError("Unknown tag encountered in structure: " + extra.getTagNo());
+                throw new IllegalArgumentWarningException(errors, this);
             }
             extras--;
         }
+
+        if (errors != null) {
+            throw new IllegalArgumentWarningException(errors, this);
+        }
+    }
+
+    private void addError(String error) {
+        if (errors == null) {
+            errors = new ArrayList<>();
+        }
+        errors.add(error);
+    }
+
+    private void addErrors(List<String> errors) {
+        for (String error : errors) {
+            addError(error);
+        }
+    }
+
+    public Collection<String> getErrors() {
+        return errors;
     }
 
     public int getVersionNumber()
@@ -218,6 +254,7 @@ public Extensions getExtensions()
         return extensions;
     }
 
+    @Override
     public ASN1Primitive toASN1Primitive()
     {
         if (Properties.getPropertyValue("org.bouncycastle.x509.allow_non-der_tbscert") != null)

core/src/main/java/org/bouncycastle/crypto/params/RSAKeyParameters.java

@@ -1,10 +1,13 @@
 package org.bouncycastle.crypto.params;
 
 import java.math.BigInteger;
+import java.util.ArrayList;
+import java.util.List;
 
 import org.bouncycastle.crypto.CryptoServicesRegistrar;
 import org.bouncycastle.math.Primes;
 import org.bouncycastle.util.BigIntegers;
+import org.bouncycastle.util.IllegalArgumentWarningException;
 import org.bouncycastle.util.Properties;
 
 public class RSAKeyParameters
@@ -41,16 +44,20 @@ public RSAKeyParameters(
     {
         super(isPrivate);
 
+        this.modulus = modulus;
+        this.exponent = exponent;
+
         if (!isPrivate)
         {
             if ((exponent.intValue() & 1) == 0)
             {
-                throw new IllegalArgumentException("RSA publicExponent is even");
+                throw new IllegalArgumentWarningException("RSA publicExponent is even", this);
             }
         }
 
-        this.modulus = validated.contains(modulus) ? modulus : validate(modulus, isInternal);
-        this.exponent = exponent;
+        if (!validated.contains(modulus)) {
+            validate(modulus, isInternal);
+        }
     }
 
     private BigInteger validate(BigInteger modulus, boolean isInternal)
@@ -62,44 +69,48 @@ private BigInteger validate(BigInteger modulus, boolean isInternal)
             return modulus;
         }
 
+        List<String> issues = new ArrayList<>();
+
         if ((modulus.intValue() & 1) == 0)
         {
-            throw new IllegalArgumentException("RSA modulus is even");
+            issues.add("RSA modulus is even");
         }
 
         // If you need to set this you need to have a serious word to whoever is generating
         // your keys.
-        if (Properties.isOverrideSet("org.bouncycastle.rsa.allow_unsafe_mod"))
+        if (!Properties.isOverrideSet("org.bouncycastle.rsa.allow_unsafe_mod"))
         {
-            return modulus;
-        }
-
-        int maxBitLength = Properties.asInteger("org.bouncycastle.rsa.max_size", 15360);
+            int maxBitLength = Properties.asInteger("org.bouncycastle.rsa.max_size", 15360);
 
-        int modBitLength = modulus.bitLength();
-        if (maxBitLength < modBitLength)
-        {
-            throw new IllegalArgumentException("modulus value out of range");
-        }
+            int modBitLength = modulus.bitLength();
+            if (maxBitLength < modBitLength)
+            {
+                issues.add("modulus value out of range");
+            }
 
-        if (!modulus.gcd(SMALL_PRIMES_PRODUCT).equals(ONE))
-        {
-            throw new IllegalArgumentException("RSA modulus has a small prime factor");
-        }
+            if (!modulus.gcd(SMALL_PRIMES_PRODUCT).equals(ONE))
+            {
+                issues.add("RSA modulus has a small prime factor");
+            }
 
-        int bits = modulus.bitLength() / 2;
-        int iterations = Properties.asInteger("org.bouncycastle.rsa.max_mr_tests", getMRIterations(bits));
+            int bits = modulus.bitLength() / 2;
+            int iterations = Properties.asInteger("org.bouncycastle.rsa.max_mr_tests", getMRIterations(bits));
 
-        if (iterations > 0)
-        {
-            Primes.MROutput mr = Primes.enhancedMRProbablePrimeTest(modulus, CryptoServicesRegistrar.getSecureRandom(), iterations);
-            if (!mr.isProvablyComposite())
+            if (iterations > 0)
             {
-                throw new IllegalArgumentException("RSA modulus is not composite");
+                Primes.MROutput mr = Primes.enhancedMRProbablePrimeTest(modulus, CryptoServicesRegistrar.getSecureRandom(), iterations);
+                if (!mr.isProvablyComposite())
+                {
+                    issues.add("RSA modulus is not composite");
+                }
+            }
+
+            if (!issues.isEmpty()) {
+                throw new IllegalArgumentWarningException(issues, this);
             }
-        }
 
-        validated.add(modulus);
+            validated.add(modulus);
+        }
 
         return modulus;
     }

core/src/main/java/org/bouncycastle/util/IllegalArgumentWarningException.java

@@ -0,0 +1,79 @@
+package org.bouncycastle.util;
+
+import java.util.Collections;
+import java.util.List;
+
+/**
+ * Exception which is thrown when parsing a certificate when errors in the 
+ * parsing are detected. This provides a list of the errors that were
+ * detected along with the (mostly) parsed object.
+ */
+public class IllegalArgumentWarningException extends IllegalArgumentException {
+
+    private static final long serialVersionUID = 5735291408274180892L;
+    List<String>   messages;
+    Object         object;
+
+    /**
+     * Basic constructor.
+     *
+     * @param messages Non empty list of messages to associate with this Exception.
+     * @param object The partially parsed object.
+     * @param cause The underlying exception (if any).
+     */
+    public IllegalArgumentWarningException(List<String> messages, Object object, Throwable cause) {
+        super(messages.get(0), cause);
+        this.messages = messages;
+        this.object = object;
+    }
+
+    /**
+     * Basic constructor.
+     *
+     * @param messages Non empty list of messages to associate with this Exception.
+     * @param object The partially parsed object.
+     */
+    public IllegalArgumentWarningException(List<String> messages, Object object) {
+        this(messages, object, null);
+    }
+
+    /**
+     * Basic constructor.
+     *
+     * @param message Single message to be associated with this Exception.
+     * @param object The partially parsed object.
+     */
+    public IllegalArgumentWarningException(String message, Object object) {
+        this(Collections.singletonList(message), object, null);
+    }
+
+    /**
+     * Basic constructor.
+     *
+     * @param object The partially parsed object.
+     * @param cause The underlying exception.
+     */
+    public IllegalArgumentWarningException(Object object, Throwable cause) {
+        this(Collections.singletonList(cause.getMessage()), object, cause);
+    }
+
+    /**
+     * Gets the list of error messages.
+     */
+    public List<String> getMessages() {
+        return messages;
+    }
+
+    /**
+     * This gets the partially parsed object -- but only if you provide the correct
+     * class!
+     *
+     * @param clazz The class of the object that you are expecting.
+     */
+    public <T> T getObject(Class<T> clazz) {
+        if (clazz.isInstance(object)) {
+            return (T) object;
+        }
+        throw new IllegalArgumentException(messages.get(0), this);
+    }
+}