From 4a768f45a5447cd2f5735895c66527ed89cf461f Mon Sep 17 00:00:00 2001 From: philip Date: Sat, 22 Jul 2023 20:01:58 -0400 Subject: [PATCH 1/3] Add support to parse invalid certs but indicate why they are invalid. Do this in a backwards compatible way. --- .../bouncycastle/asn1/x509/Certificate.java | 13 ++++ .../bouncycastle/asn1/x509/Extensions.java | 18 +++-- .../asn1/x509/TBSCertificate.java | 44 ++++++++++-- .../crypto/params/RSAKeyParameters.java | 69 +++++++++++-------- .../util/IllegalArgumentWarningException.java | 41 +++++++++++ 5 files changed, 147 insertions(+), 38 deletions(-) create mode 100644 core/src/main/java/org/bouncycastle/util/IllegalArgumentWarningException.java diff --git a/core/src/main/java/org/bouncycastle/asn1/x509/Certificate.java b/core/src/main/java/org/bouncycastle/asn1/x509/Certificate.java index e64a197572..cf7e8b8b67 100644 --- a/core/src/main/java/org/bouncycastle/asn1/x509/Certificate.java +++ b/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) { + 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; diff --git a/core/src/main/java/org/bouncycastle/asn1/x509/Extensions.java b/core/src/main/java/org/bouncycastle/asn1/x509/Extensions.java index 0795a08d45..d9417d34e8 100644 --- a/core/src/main/java/org/bouncycastle/asn1/x509/Extensions.java +++ b/core/src/main/java/org/bouncycastle/asn1/x509/Extensions.java @@ -1,9 +1,5 @@ package org.bouncycastle.asn1.x509; -import java.util.Enumeration; -import java.util.Hashtable; -import java.util.Vector; - import org.bouncycastle.asn1.ASN1Encodable; import org.bouncycastle.asn1.ASN1EncodableVector; import org.bouncycastle.asn1.ASN1Object; @@ -12,6 +8,11 @@ import org.bouncycastle.asn1.ASN1Sequence; import org.bouncycastle.asn1.ASN1TaggedObject; import org.bouncycastle.asn1.DERSequence; +import org.bouncycastle.util.IllegalArgumentWarningException; + +import java.util.Enumeration; +import java.util.Hashtable; +import java.util.Vector; /** *
@@ -71,6 +72,7 @@ private Extensions(
         ASN1Sequence seq)
     {
         Enumeration e = seq.getObjects();
+        String error = null;
 
         while (e.hasMoreElements())
         {
@@ -78,12 +80,17 @@ private Extensions(
 
             if (extensions.containsKey(ext.getExtnId()))
             {
-                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);
+        }
     }
 
     /**
@@ -163,6 +170,7 @@ public ASN1Encodable getExtensionParsedValue(ASN1ObjectIdentifier oid)
      *        extnValue         OCTET STRING }
      * 
*/ + @Override public ASN1Primitive toASN1Primitive() { ASN1EncodableVector vec = new ASN1EncodableVector(ordering.size()); diff --git a/core/src/main/java/org/bouncycastle/asn1/x509/TBSCertificate.java b/core/src/main/java/org/bouncycastle/asn1/x509/TBSCertificate.java index 76763291b6..d7429fc33e 100644 --- a/core/src/main/java/org/bouncycastle/asn1/x509/TBSCertificate.java +++ b/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. *
@@ -47,6 +52,7 @@ public class TBSCertificate
     ASN1BitString           issuerUniqueId;
     ASN1BitString           subjectUniqueId;
     Extensions              extensions;
+    List errors;
 
     public static TBSCertificate getInstance(
         ASN1TaggedObject obj,
@@ -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,7 +136,8 @@ 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)
@@ -147,15 +155,42 @@ 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");
+                  return;
+                }
+                try {
+                  extensions = Extensions.getInstance(ASN1Sequence.getInstance(extra, true));
+                } catch (IllegalArgumentWarningException ex) {
+                  extensions = (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());
             }
             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 errors) {
+      for (String error : errors) {
+        addError(error);
+      }
+    }
+
+    public Collection getErrors() {
+      return errors;
     }
 
     public int getVersionNumber()
@@ -218,6 +253,7 @@ public Extensions getExtensions()
         return extensions;
     }
 
+    @Override
     public ASN1Primitive toASN1Primitive()
     {
         if (Properties.getPropertyValue("org.bouncycastle.x509.allow_non-der_tbscert") != null)
diff --git a/core/src/main/java/org/bouncycastle/crypto/params/RSAKeyParameters.java b/core/src/main/java/org/bouncycastle/crypto/params/RSAKeyParameters.java
index 65b0b84ee4..c053b76b6c 100644
--- a/core/src/main/java/org/bouncycastle/crypto/params/RSAKeyParameters.java
+++ b/core/src/main/java/org/bouncycastle/crypto/params/RSAKeyParameters.java
@@ -1,12 +1,15 @@
 package org.bouncycastle.crypto.params;
 
-import java.math.BigInteger;
-
 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;
 
+import java.math.BigInteger;
+import java.util.ArrayList;
+import java.util.List;
+
 public class RSAKeyParameters
     extends AsymmetricKeyParameter
 {
@@ -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 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;
     }
diff --git a/core/src/main/java/org/bouncycastle/util/IllegalArgumentWarningException.java b/core/src/main/java/org/bouncycastle/util/IllegalArgumentWarningException.java
new file mode 100644
index 0000000000..5f8fa10ee4
--- /dev/null
+++ b/core/src/main/java/org/bouncycastle/util/IllegalArgumentWarningException.java
@@ -0,0 +1,41 @@
+package org.bouncycastle.util;
+
+import java.util.Collections;
+import java.util.List;
+
+public class IllegalArgumentWarningException extends IllegalArgumentException {
+  
+  private static final long serialVersionUID = 5735291408274180892L;
+  List messages;
+  Object object;
+  
+  public IllegalArgumentWarningException(List messages, Object object, Throwable cause) {
+    super(messages.get(0), cause);
+    this.messages = messages;
+    this.object = object;
+  }
+
+  public IllegalArgumentWarningException(List messages, Object object) {
+    this(messages, object, null);
+  }
+
+  public IllegalArgumentWarningException(String message, Object object) {
+    this(Collections.singletonList(message), object, null);
+  }
+
+  public IllegalArgumentWarningException(Object object, Throwable cause) {
+    this(Collections.singletonList(cause.getMessage()), object, cause);
+  }
+
+  public List getMessages() {
+    return messages;
+  }
+
+  public  T getObject(Class clazz) {
+    if (clazz.isInstance(object)) {
+      return (T) object;
+    }
+    throw new IllegalArgumentException(messages.get(0), this);
+  }
+
+}

From d01e8e523bbf216a4b89a10407aa824266a4b9b3 Mon Sep 17 00:00:00 2001
From: philip 
Date: Wed, 26 Jul 2023 08:38:32 -0400
Subject: [PATCH 2/3] Fixed a bug and made the tests pass

---
 .../java/org/bouncycastle/asn1/x509/TBSCertificate.java    | 7 ++++---
 .../main/java/org/bouncycastle/util/test/SimpleTest.java   | 4 ++--
 .../jce/provider/test/CertPathValidatorTest.java           | 6 +++---
 3 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/core/src/main/java/org/bouncycastle/asn1/x509/TBSCertificate.java b/core/src/main/java/org/bouncycastle/asn1/x509/TBSCertificate.java
index d7429fc33e..69d40879ab 100644
--- a/core/src/main/java/org/bouncycastle/asn1/x509/TBSCertificate.java
+++ b/core/src/main/java/org/bouncycastle/asn1/x509/TBSCertificate.java
@@ -156,17 +156,18 @@ else if (!version.hasValue(2))
                 if (isV2)
                 {
                   addError("version 2 certificate cannot contain extensions");
-                  return;
+                  throw new IllegalArgumentWarningException(errors, this);
                 }
                 try {
                   extensions = Extensions.getInstance(ASN1Sequence.getInstance(extra, true));
                 } catch (IllegalArgumentWarningException ex) {
-                  extensions = (Extensions) ex.getObject(Extensions.class);
+                  extensions = ex.getObject(Extensions.class);
                   addErrors(ex.getMessages());
                 }
                 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--;
         }
diff --git a/core/src/main/java/org/bouncycastle/util/test/SimpleTest.java b/core/src/main/java/org/bouncycastle/util/test/SimpleTest.java
index 66ce33bd79..c429cb29cc 100644
--- a/core/src/main/java/org/bouncycastle/util/test/SimpleTest.java
+++ b/core/src/main/java/org/bouncycastle/util/test/SimpleTest.java
@@ -47,7 +47,7 @@ protected void isEquals(
     {
         if (!a.equals(b))
         {
-            throw new TestFailedException(SimpleTestResult.failed(this, "no message"));
+            throw new TestFailedException(SimpleTestResult.failed(this, String.format("isEqual fail: %s != %s", a, b)));
         }
     }
 
@@ -57,7 +57,7 @@ protected void isEquals(
     {
         if (a != b)
         {
-            throw new TestFailedException(SimpleTestResult.failed(this, "no message"));
+            throw new TestFailedException(SimpleTestResult.failed(this, String.format("isEqual fail: %s != %s", a, b)));
         }
     }
 
diff --git a/prov/src/test/java/org/bouncycastle/jce/provider/test/CertPathValidatorTest.java b/prov/src/test/java/org/bouncycastle/jce/provider/test/CertPathValidatorTest.java
index 1989c5f2e3..9991a866e7 100644
--- a/prov/src/test/java/org/bouncycastle/jce/provider/test/CertPathValidatorTest.java
+++ b/prov/src/test/java/org/bouncycastle/jce/provider/test/CertPathValidatorTest.java
@@ -582,7 +582,7 @@ private void checkInvalidCertPath()
     {
         checkInvalidPath(extInvTrust, extInvCA, extInvEE, "version 1 certificate contains extra data");
         checkInvalidPath(extInvV2Trust, extInvV2CA, extInvV2EE, "version 2 certificate cannot contain extensions");
-        checkInvalidPath(extInvVersionTrust, extInvVersionCA, extInvVersionEE, "version number not recognised");
+        checkInvalidPath(extInvVersionTrust, extInvVersionCA, extInvVersionEE, "Certificate version number value 5 not 0, 1 or 2");
         checkInvalidPath(extInvExtTrust, extInvExtCA, extInvExtEE, "repeated extension found: 2.5.29.15");
     }
 
@@ -621,7 +621,7 @@ private void checkInvalidPath(byte[] root, byte[] inter, byte[] ee, String expec
         }
         catch (CertPathValidatorException e)
         {
-            isTrue(e.getMessage().equals(expected));
+          isEquals(e.getMessage(), expected);
         }
 
         // check that our cert factory also rejects - the EE is always the invalid one
@@ -633,7 +633,7 @@ private void checkInvalidPath(byte[] root, byte[] inter, byte[] ee, String expec
         }
         catch (CertificateException e)
         {
-            isTrue(e.getMessage().equals("parsing issue: " + expected));
+          isEquals(e.getMessage(), "parsing issue: " + expected);
         }
     }
 

From 00c3dc67a9c0a38122c79d21f06048282c7d91e2 Mon Sep 17 00:00:00 2001
From: philip 
Date: Thu, 19 Oct 2023 10:51:37 -0400
Subject: [PATCH 3/3] Formatting improvements

---
 .../bouncycastle/asn1/x509/Certificate.java   |  14 +--
 .../bouncycastle/asn1/x509/Extensions.java    |  12 +--
 .../asn1/x509/TBSCertificate.java             |  52 ++++-----
 .../crypto/params/RSAKeyParameters.java       |  10 +-
 .../util/IllegalArgumentWarningException.java | 102 ++++++++++++------
 .../provider/test/CertPathValidatorTest.java  |   4 +-
 6 files changed, 116 insertions(+), 78 deletions(-)

diff --git a/core/src/main/java/org/bouncycastle/asn1/x509/Certificate.java b/core/src/main/java/org/bouncycastle/asn1/x509/Certificate.java
index cf7e8b8b67..2f85f2cf0a 100644
--- a/core/src/main/java/org/bouncycastle/asn1/x509/Certificate.java
+++ b/core/src/main/java/org/bouncycastle/asn1/x509/Certificate.java
@@ -61,12 +61,12 @@ private Certificate(
         //
         if (seq.size() == 3)
         {
-          try {
-            tbsCert = TBSCertificate.getInstance(seq.getObjectAt(0));
-          } catch (IllegalArgumentWarningException ex) {
-            tbsCert = (TBSCertificate) ex.getObject(TBSCertificate.class);
-            exception = ex;
-          }
+            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));
@@ -77,7 +77,7 @@ private Certificate(
         }
 
         if (exception != null) {
-          throw new IllegalArgumentWarningException(this, exception);
+            throw new IllegalArgumentWarningException(this, exception);
         }
     }
 
diff --git a/core/src/main/java/org/bouncycastle/asn1/x509/Extensions.java b/core/src/main/java/org/bouncycastle/asn1/x509/Extensions.java
index 967386d2fb..e015d475e8 100644
--- a/core/src/main/java/org/bouncycastle/asn1/x509/Extensions.java
+++ b/core/src/main/java/org/bouncycastle/asn1/x509/Extensions.java
@@ -1,5 +1,9 @@
 package org.bouncycastle.asn1.x509;
 
+import java.util.Enumeration;
+import java.util.Hashtable;
+import java.util.Vector;
+
 import org.bouncycastle.asn1.ASN1Encodable;
 import org.bouncycastle.asn1.ASN1EncodableVector;
 import org.bouncycastle.asn1.ASN1Object;
@@ -11,10 +15,6 @@
 import org.bouncycastle.util.IllegalArgumentWarningException;
 import org.bouncycastle.util.Properties;
 
-import java.util.Enumeration;
-import java.util.Hashtable;
-import java.util.Vector;
-
 /**
  * 
  *     Extensions        ::=   SEQUENCE SIZE (1..MAX) OF Extension
@@ -96,9 +96,9 @@ private Extensions(
             extensions.put(ext.getExtnId(), ext);
             ordering.addElement(ext.getExtnId());
         }
-        
+
         if (error != null) {
-          throw new IllegalArgumentWarningException(error, this);
+            throw new IllegalArgumentWarningException(error, this);
         }
     }
 
diff --git a/core/src/main/java/org/bouncycastle/asn1/x509/TBSCertificate.java b/core/src/main/java/org/bouncycastle/asn1/x509/TBSCertificate.java
index 69d40879ab..c97da4240e 100644
--- a/core/src/main/java/org/bouncycastle/asn1/x509/TBSCertificate.java
+++ b/core/src/main/java/org/bouncycastle/asn1/x509/TBSCertificate.java
@@ -52,7 +52,7 @@ public class TBSCertificate
     ASN1BitString           issuerUniqueId;
     ASN1BitString           subjectUniqueId;
     Extensions              extensions;
-    List errors;
+    List            errors;
 
     public static TBSCertificate getInstance(
         ASN1TaggedObject obj,
@@ -98,7 +98,7 @@ private TBSCertificate(
 
         boolean isV1 = false;
         boolean isV2 = false;
- 
+
         if (version.hasValue(0))
         {
             isV1 = true;
@@ -109,8 +109,8 @@ else if (version.hasValue(1))
         }
         else if (!version.hasValue(2))
         {
-          addError(
-              String.format("Certificate version number value %d not 0, 1 or 2", version.getValue()));
+            addError(
+                String.format("Certificate version number value %d not 0, 1 or 2", version.getValue()));
         }
 
         serialNumber = ASN1Integer.getInstance(seq.getObjectAt(seqStart + 1));
@@ -136,10 +136,10 @@ else if (!version.hasValue(2))
         int extras = seq.size() - (seqStart + 6) - 1;
         if (extras != 0 && isV1)
         {
-          addError("version 1 certificate contains extra data");
-          extras = 0; // Ignore the 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);
@@ -155,43 +155,43 @@ else if (!version.hasValue(2))
             case 3:
                 if (isV2)
                 {
-                  addError("version 2 certificate cannot contain extensions");
-                  throw new IllegalArgumentWarningException(errors, this);
+                    addError("version 2 certificate cannot contain extensions");
+                    throw new IllegalArgumentWarningException(errors, this);
                 }
                 try {
-                  extensions = Extensions.getInstance(ASN1Sequence.getInstance(extra, true));
+                    extensions = Extensions.getInstance(ASN1Sequence.getInstance(extra, true));
                 } catch (IllegalArgumentWarningException ex) {
-                  extensions = ex.getObject(Extensions.class);
-                  addErrors(ex.getMessages());
+                    extensions = ex.getObject(Extensions.class);
+                    addErrors(ex.getMessages());
                 }
                 break;
             default:
-              addError("Unknown tag encountered in structure: " + extra.getTagNo());
-              throw new IllegalArgumentWarningException(errors, this);
+                addError("Unknown tag encountered in structure: " + extra.getTagNo());
+                throw new IllegalArgumentWarningException(errors, this);
             }
             extras--;
         }
-        
+
         if (errors != null) {
-          throw new IllegalArgumentWarningException(errors, this);
+            throw new IllegalArgumentWarningException(errors, this);
         }
     }
-    
+
     private void addError(String error) {
-      if (errors == null) {
-        errors = new ArrayList<>();
-      }
-      errors.add(error);
+        if (errors == null) {
+            errors = new ArrayList<>();
+        }
+        errors.add(error);
     }
-    
+
     private void addErrors(List errors) {
-      for (String error : errors) {
-        addError(error);
-      }
+        for (String error : errors) {
+            addError(error);
+        }
     }
 
     public Collection getErrors() {
-      return errors;
+        return errors;
     }
 
     public int getVersionNumber()
diff --git a/core/src/main/java/org/bouncycastle/crypto/params/RSAKeyParameters.java b/core/src/main/java/org/bouncycastle/crypto/params/RSAKeyParameters.java
index c053b76b6c..1e2a3e89c8 100644
--- a/core/src/main/java/org/bouncycastle/crypto/params/RSAKeyParameters.java
+++ b/core/src/main/java/org/bouncycastle/crypto/params/RSAKeyParameters.java
@@ -1,15 +1,15 @@
 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;
 
-import java.math.BigInteger;
-import java.util.ArrayList;
-import java.util.List;
-
 public class RSAKeyParameters
     extends AsymmetricKeyParameter
 {
@@ -106,7 +106,7 @@ private BigInteger validate(BigInteger modulus, boolean isInternal)
             }
 
             if (!issues.isEmpty()) {
-              throw new IllegalArgumentWarningException(issues, this);
+                throw new IllegalArgumentWarningException(issues, this);
             }
 
             validated.add(modulus);
diff --git a/core/src/main/java/org/bouncycastle/util/IllegalArgumentWarningException.java b/core/src/main/java/org/bouncycastle/util/IllegalArgumentWarningException.java
index 5f8fa10ee4..521a5efce0 100644
--- a/core/src/main/java/org/bouncycastle/util/IllegalArgumentWarningException.java
+++ b/core/src/main/java/org/bouncycastle/util/IllegalArgumentWarningException.java
@@ -3,39 +3,77 @@
 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 messages;
-  Object object;
-  
-  public IllegalArgumentWarningException(List messages, Object object, Throwable cause) {
-    super(messages.get(0), cause);
-    this.messages = messages;
-    this.object = object;
-  }
-
-  public IllegalArgumentWarningException(List messages, Object object) {
-    this(messages, object, null);
-  }
-
-  public IllegalArgumentWarningException(String message, Object object) {
-    this(Collections.singletonList(message), object, null);
-  }
-
-  public IllegalArgumentWarningException(Object object, Throwable cause) {
-    this(Collections.singletonList(cause.getMessage()), object, cause);
-  }
-
-  public List getMessages() {
-    return messages;
-  }
-
-  public  T getObject(Class clazz) {
-    if (clazz.isInstance(object)) {
-      return (T) object;
+
+    private static final long serialVersionUID = 5735291408274180892L;
+    List   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 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 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);
     }
-    throw new IllegalArgumentException(messages.get(0), this);
-  }
 
+    /**
+     * 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 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 getObject(Class clazz) {
+        if (clazz.isInstance(object)) {
+            return (T) object;
+        }
+        throw new IllegalArgumentException(messages.get(0), this);
+    }
 }
diff --git a/prov/src/test/java/org/bouncycastle/jce/provider/test/CertPathValidatorTest.java b/prov/src/test/java/org/bouncycastle/jce/provider/test/CertPathValidatorTest.java
index 9991a866e7..2e3dc17da4 100644
--- a/prov/src/test/java/org/bouncycastle/jce/provider/test/CertPathValidatorTest.java
+++ b/prov/src/test/java/org/bouncycastle/jce/provider/test/CertPathValidatorTest.java
@@ -621,7 +621,7 @@ private void checkInvalidPath(byte[] root, byte[] inter, byte[] ee, String expec
         }
         catch (CertPathValidatorException e)
         {
-          isEquals(e.getMessage(), expected);
+            isEquals(e.getMessage(), expected);
         }
 
         // check that our cert factory also rejects - the EE is always the invalid one
@@ -633,7 +633,7 @@ private void checkInvalidPath(byte[] root, byte[] inter, byte[] ee, String expec
         }
         catch (CertificateException e)
         {
-          isEquals(e.getMessage(), "parsing issue: " + expected);
+            isEquals(e.getMessage(), "parsing issue: " + expected);
         }
     }