Refactoring in Asn1.Cmp

28 files changed, 379 insertions, 482 deletions

diff --git a/crypto/src/asn1/cmp/CAKeyUpdAnnContent.cs b/crypto/src/asn1/cmp/CAKeyUpdAnnContent.cs
index 4460c8265..88c3b6e40 100644
--- a/crypto/src/asn1/cmp/CAKeyUpdAnnContent.cs
+++ b/crypto/src/asn1/cmp/CAKeyUpdAnnContent.cs
@@ -1,3 +1,5 @@
+using System;
+
 namespace Org.BouncyCastle.Asn1.Cmp
 {
 	public class CAKeyUpdAnnContent
@@ -23,7 +25,11 @@ namespace Org.BouncyCastle.Asn1.Cmp
 
 		private CAKeyUpdAnnContent(Asn1Sequence seq)
 		{
-			m_oldWithNew = CmpCertificate.GetInstance(seq[0]);
+            int count = seq.Count;
+            if (count != 3)
+                throw new ArgumentException("Bad sequence size: " + count, nameof(seq));
+
+            m_oldWithNew = CmpCertificate.GetInstance(seq[0]);
 			m_newWithOld = CmpCertificate.GetInstance(seq[1]);
 			m_newWithNew = CmpCertificate.GetInstance(seq[2]);
 		}
diff --git a/crypto/src/asn1/cmp/CertOrEncCert.cs b/crypto/src/asn1/cmp/CertOrEncCert.cs
index 94df00006..430c1fd4f 100644
--- a/crypto/src/asn1/cmp/CertOrEncCert.cs
+++ b/crypto/src/asn1/cmp/CertOrEncCert.cs
@@ -46,7 +46,8 @@ namespace Org.BouncyCastle.Asn1.Cmp
 			m_certificate = certificate ?? throw new ArgumentNullException(nameof(certificate));
         }
 
-		public CertOrEncCert(EncryptedValue encryptedValue)
+        [Obsolete("Use constructor with EncryptedKey instead")]
+        public CertOrEncCert(EncryptedValue encryptedValue)
 		{
 			m_encryptedCert = new EncryptedKey(
 				encryptedValue ?? throw new ArgumentNullException(nameof(encryptedValue)));
diff --git a/crypto/src/asn1/cmp/CertRepMessage.cs b/crypto/src/asn1/cmp/CertRepMessage.cs
index 9c260eadb..9e7fae2e2 100644
--- a/crypto/src/asn1/cmp/CertRepMessage.cs
+++ b/crypto/src/asn1/cmp/CertRepMessage.cs
@@ -24,14 +24,15 @@ namespace Org.BouncyCastle.Asn1.Cmp
 		
 		private CertRepMessage(Asn1Sequence seq)
 		{
-			int index = 0;
+            int count = seq.Count, pos = 0;
+            if (count < 1 || count > 2)
+                throw new ArgumentException("Bad sequence size: " + count, nameof(seq));
 
-			if (seq.Count > 1)
-			{
-				m_caPubs = Asn1Sequence.GetInstance((Asn1TaggedObject)seq[index++], true);
-			}
+			m_caPubs = Asn1Utilities.ReadOptionalContextTagged(seq, ref pos, 1, true, Asn1Sequence.GetInstance);
+			m_response = Asn1Sequence.GetInstance(seq[pos++]);
 
-			m_response = Asn1Sequence.GetInstance(seq[index]);
+			if (pos != count)
+                throw new ArgumentException("Unexpected elements in sequence", nameof(seq));
 		}
 
 		public CertRepMessage(CmpCertificate[] caPubs, CertResponse[] response)
diff --git a/crypto/src/asn1/cmp/CertReqTemplateContent.cs b/crypto/src/asn1/cmp/CertReqTemplateContent.cs
index ae35bc5e8..118f23a4c 100644
--- a/crypto/src/asn1/cmp/CertReqTemplateContent.cs
+++ b/crypto/src/asn1/cmp/CertReqTemplateContent.cs
@@ -35,30 +35,33 @@ namespace Org.BouncyCastle.Asn1.Cmp
         }
 
         private readonly CertTemplate m_certTemplate;
-        private readonly Asn1Sequence m_keySpec;
+        private readonly Controls m_keySpec;
 
         private CertReqTemplateContent(Asn1Sequence seq)
         {
-            if (seq.Count != 1 && seq.Count != 2)
-                throw new ArgumentException("expected sequence size of 1 or 2");
+            int count = seq.Count, pos = 0;
+            if (count < 1 || count > 2)
+                throw new ArgumentException("Bad sequence size: " + count, nameof(seq));
 
-            m_certTemplate = CertTemplate.GetInstance(seq[0]);
+            m_certTemplate = CertTemplate.GetInstance(seq[pos++]);
+            m_keySpec = Asn1Utilities.ReadOptional(seq, ref pos, Controls.GetOptional);
 
-            if (seq.Count > 1)
-            {
-                m_keySpec = Asn1Sequence.GetInstance(seq[1]);
-            }
+            if (pos != count)
+                throw new ArgumentException("Unexpected elements in sequence", nameof(seq));
         }
 
         public CertReqTemplateContent(CertTemplate certTemplate, Asn1Sequence keySpec)
         {
-            m_certTemplate = certTemplate;
-            m_keySpec = keySpec;
+            m_certTemplate = certTemplate ?? throw new ArgumentNullException(nameof(certTemplate));
+            m_keySpec = Controls.GetInstance(keySpec);
         }
 
         public virtual CertTemplate CertTemplate => m_certTemplate;
 
-        public virtual Asn1Sequence KeySpec => m_keySpec;
+        [Obsolete("Use 'KeySpecControls' property instead")]
+        public virtual Asn1Sequence KeySpec => Asn1Sequence.GetInstance(m_keySpec?.ToAsn1Object());
+
+        public virtual Controls KeySpecControls => m_keySpec;
 
         public override Asn1Object ToAsn1Object()
         {
diff --git a/crypto/src/asn1/cmp/CertResponse.cs b/crypto/src/asn1/cmp/CertResponse.cs
index ce3355270..0bebb3a01 100644
--- a/crypto/src/asn1/cmp/CertResponse.cs
+++ b/crypto/src/asn1/cmp/CertResponse.cs
@@ -26,32 +26,20 @@ namespace Org.BouncyCastle.Asn1.Cmp
 
 		private CertResponse(Asn1Sequence seq)
 		{
-			m_certReqId = DerInteger.GetInstance(seq[0]);
-			m_status = PkiStatusInfo.GetInstance(seq[1]);
+            int count = seq.Count, pos = 0;
+            if (count < 2 || count > 4)
+                throw new ArgumentException("Bad sequence size: " + count, nameof(seq));
 
-			if (seq.Count >= 3)
-			{
-				if (seq.Count == 3)
-				{
-					Asn1Encodable o = seq[2];
-					if (o is Asn1OctetString octetString)
-					{
-						m_rspInfo = octetString;
-					}
-					else
-					{
-						m_certifiedKeyPair = CertifiedKeyPair.GetInstance(o);
-					}
-				}
-				else
-				{
-					m_certifiedKeyPair = CertifiedKeyPair.GetInstance(seq[2]);
-					m_rspInfo = Asn1OctetString.GetInstance(seq[3]);
-				}
-			}
-		}
+            m_certReqId = DerInteger.GetInstance(seq[pos++]);
+            m_status = PkiStatusInfo.GetInstance(seq[pos++]);
+            m_certifiedKeyPair = Asn1Utilities.ReadOptional(seq, ref pos, CertifiedKeyPair.GetOptional);
+            m_rspInfo = Asn1Utilities.ReadOptional(seq, ref pos, Asn1OctetString.GetOptional);
+
+            if (pos != count)
+                throw new ArgumentException("Unexpected elements in sequence", nameof(seq));
+        }
 
-		public CertResponse(DerInteger certReqId, PkiStatusInfo status)
+        public CertResponse(DerInteger certReqId, PkiStatusInfo status)
 			: this(certReqId, status, null, null)
 		{
 		}
diff --git a/crypto/src/asn1/cmp/CertStatus.cs b/crypto/src/asn1/cmp/CertStatus.cs
index 488f78376..5002b8811 100644
--- a/crypto/src/asn1/cmp/CertStatus.cs
+++ b/crypto/src/asn1/cmp/CertStatus.cs
@@ -29,31 +29,21 @@ namespace Org.BouncyCastle.Asn1.Cmp
 
         private CertStatus(Asn1Sequence seq)
 		{
-			m_certHash = Asn1OctetString.GetInstance(seq[0]);
-			m_certReqID = DerInteger.GetInstance(seq[1]);
-
-			if (seq.Count > 2)
-			{
-				for (int t = 2; t < seq.Count; t++)
-				{
-					Asn1Object p = seq[t].ToAsn1Object();
-					if (p is Asn1Sequence s)
-					{
-						m_statusInfo = PkiStatusInfo.GetInstance(s);
-					}
-					if (p is Asn1TaggedObject dto)
-					{
-						if (!dto.HasContextTag(0))
-							throw new ArgumentException("unknown tag: " + Asn1Utilities.GetTagText(dto));
-
-						m_hashAlg = AlgorithmIdentifier.GetInstance(dto, true);
-					}
-				}
-			}
-		}
+            int count = seq.Count, pos = 0;
+            if (count < 2 || count > 4)
+                throw new ArgumentException("Bad sequence size: " + count, nameof(seq));
+
+            m_certHash = Asn1OctetString.GetInstance(seq[pos++]);
+            m_certReqID = DerInteger.GetInstance(seq[pos++]);
+            m_statusInfo = Asn1Utilities.ReadOptional(seq, ref pos, PkiStatusInfo.GetOptional);
+            m_hashAlg = Asn1Utilities.ReadOptionalContextTagged(seq, ref pos, 0, true, AlgorithmIdentifier.GetInstance);
+
+            if (pos != count)
+                throw new ArgumentException("Unexpected elements in sequence", nameof(seq));
+        }
 
         public CertStatus(byte[] certHash, BigInteger certReqID)
-            : this(certHash, new DerInteger(certReqID))
+            : this(certHash, certReqID, null, null)
         {
         }
 
diff --git a/crypto/src/asn1/cmp/CertifiedKeyPair.cs b/crypto/src/asn1/cmp/CertifiedKeyPair.cs
index 9c7fd8175..61da1d37a 100644
--- a/crypto/src/asn1/cmp/CertifiedKeyPair.cs
+++ b/crypto/src/asn1/cmp/CertifiedKeyPair.cs
@@ -21,36 +21,37 @@ namespace Org.BouncyCastle.Asn1.Cmp
             return new CertifiedKeyPair(Asn1Sequence.GetInstance(taggedObject, declaredExplicit));
         }
 
+        public static CertifiedKeyPair GetOptional(Asn1Encodable element)
+        {
+            if (element == null)
+                throw new ArgumentNullException(nameof(element));
+
+            if (element is CertifiedKeyPair certifiedKeyPair)
+                return certifiedKeyPair;
+
+            Asn1Sequence asn1Sequence = Asn1Sequence.GetOptional(element);
+            if (asn1Sequence != null)
+                return new CertifiedKeyPair(asn1Sequence);
+
+            return null;
+        }
+
         private readonly CertOrEncCert m_certOrEncCert;
 		private readonly EncryptedKey m_privateKey;
 		private readonly PkiPublicationInfo m_publicationInfo;
 
         private CertifiedKeyPair(Asn1Sequence seq)
 		{
-			m_certOrEncCert = CertOrEncCert.GetInstance(seq[0]);
-
-			if (seq.Count >= 2)
-			{
-				if (seq.Count == 2)
-				{
-					Asn1TaggedObject tagged = Asn1TaggedObject.GetInstance(seq[1], Asn1Tags.ContextSpecific);
-					if (tagged.TagNo == 0)
-					{
-						m_privateKey = EncryptedKey.GetInstance(tagged.GetExplicitBaseObject());
-					}
-					else
-					{
-						m_publicationInfo = PkiPublicationInfo.GetInstance(tagged.GetExplicitBaseObject());
-					}
-				}
-				else
-				{
-                    m_privateKey = EncryptedKey.GetInstance(
-						Asn1TaggedObject.GetInstance(seq[1], Asn1Tags.ContextSpecific).GetExplicitBaseObject());
-                    m_publicationInfo = PkiPublicationInfo.GetInstance(
-						Asn1TaggedObject.GetInstance(seq[2], Asn1Tags.ContextSpecific).GetExplicitBaseObject());
-				}
-			}
+            int count = seq.Count, pos = 0;
+            if (count < 1 || count > 3)
+                throw new ArgumentException("Bad sequence size: " + count, nameof(seq));
+
+            m_certOrEncCert = CertOrEncCert.GetInstance(seq[pos++]);
+            m_privateKey = Asn1Utilities.ReadOptionalContextTagged(seq, ref pos, 0, true, EncryptedKey.GetInstance);
+            m_publicationInfo = Asn1Utilities.ReadOptionalContextTagged(seq, ref pos, 1, true, PkiPublicationInfo.GetInstance);
+
+            if (pos != count)
+                throw new ArgumentException("Unexpected elements in sequence", nameof(seq));
 		}
 
 		public CertifiedKeyPair(CertOrEncCert certOrEncCert)
@@ -58,6 +59,7 @@ namespace Org.BouncyCastle.Asn1.Cmp
 		{
 		}
 
+        [Obsolete("Use constructor with EncryptedKey instead")]
         public CertifiedKeyPair(CertOrEncCert certOrEncCert, EncryptedValue privateKey,
             PkiPublicationInfo publicationInfo)
             : this(certOrEncCert, privateKey == null ? null : new EncryptedKey(privateKey), publicationInfo)
@@ -67,10 +69,7 @@ namespace Org.BouncyCastle.Asn1.Cmp
         public CertifiedKeyPair(CertOrEncCert certOrEncCert, EncryptedKey privateKey,
 			PkiPublicationInfo publicationInfo)
         {
-			if (certOrEncCert == null)
-				throw new ArgumentNullException(nameof(certOrEncCert));
-
-            m_certOrEncCert = certOrEncCert;
+            m_certOrEncCert = certOrEncCert ?? throw new ArgumentNullException(nameof(certOrEncCert));
             m_privateKey = privateKey;
             m_publicationInfo = publicationInfo;
         }
@@ -82,13 +81,14 @@ namespace Org.BouncyCastle.Asn1.Cmp
 		public virtual PkiPublicationInfo PublicationInfo => m_publicationInfo;
 
 		/**
+		 * RFC 9480
 		 * <pre>
 		 * CertifiedKeyPair ::= SEQUENCE {
-		 *                                  certOrEncCert       CertOrEncCert,
-		 *                                  privateKey      [0] EncryptedValue      OPTIONAL,
-		 *                                  -- see [CRMF] for comment on encoding
-		 *                                  publicationInfo [1] PKIPublicationInfo  OPTIONAL
-		 *       }
+         *     certOrEncCert       CertOrEncCert,
+         *     privateKey      [0] EncryptedKey        OPTIONAL,
+         *     -- See [RFC4211] for comments on encoding.
+         *     publicationInfo [1] PKIPublicationInfo  OPTIONAL
+         * }
 		 * </pre>
 		 * @return a basic ASN.1 object representation.
 		 */
diff --git a/crypto/src/asn1/cmp/Challenge.cs b/crypto/src/asn1/cmp/Challenge.cs
index 4e30dcb55..0bc1992b0 100644
--- a/crypto/src/asn1/cmp/Challenge.cs
+++ b/crypto/src/asn1/cmp/Challenge.cs
@@ -53,16 +53,17 @@ namespace Org.BouncyCastle.Asn1.Cmp
 
 		private Challenge(Asn1Sequence seq)
 		{
-			int index = 0;
+            int count = seq.Count, pos = 0;
+            if (count < 2 || count > 3)
+                throw new ArgumentException("Bad sequence size: " + count, nameof(seq));
 
-			if (seq.Count == 3)
-			{
-				m_owf = AlgorithmIdentifier.GetInstance(seq[index++]);
-			}
+            m_owf = Asn1Utilities.ReadOptional(seq, ref pos, AlgorithmIdentifier.GetOptional);
+            m_witness = Asn1OctetString.GetInstance(seq[pos++]);
+            m_challenge = Asn1OctetString.GetInstance(seq[pos++]);
 
-			m_witness = Asn1OctetString.GetInstance(seq[index++]);
-			m_challenge = Asn1OctetString.GetInstance(seq[index]);
-		}
+            if (pos != count)
+                throw new ArgumentException("Unexpected elements in sequence", nameof(seq));
+        }
 
         public Challenge(byte[] witness, byte[] challenge)
             : this(null, witness, challenge)
diff --git a/crypto/src/asn1/cmp/CrlStatus.cs b/crypto/src/asn1/cmp/CrlStatus.cs
index f5cf091f4..87760b9a2 100644
--- a/crypto/src/asn1/cmp/CrlStatus.cs
+++ b/crypto/src/asn1/cmp/CrlStatus.cs
@@ -29,23 +29,22 @@ namespace Org.BouncyCastle.Asn1.Cmp
         private readonly CrlSource m_source;
         private readonly Time m_thisUpdate;
 
-        private CrlStatus(Asn1Sequence sequence)
+        private CrlStatus(Asn1Sequence seq)
         {
-            int count = sequence.Count;
+            int count = seq.Count, pos = 0;
             if (count < 1 || count > 2)
-                throw new ArgumentException("expected sequence size of 1 or 2, got " + count);
+                throw new ArgumentException("Bad sequence size: " + count, nameof(seq));
 
-            m_source = CrlSource.GetInstance(sequence[0]);
+            m_source = CrlSource.GetInstance(seq[pos++]);
+            m_thisUpdate = Asn1Utilities.ReadOptional(seq, ref pos, Time.GetOptional);
 
-            if (sequence.Count == 2)
-            {
-                m_thisUpdate = Time.GetInstance(sequence[1]);
-            }
+            if (pos != count)
+                throw new ArgumentException("Unexpected elements in sequence", nameof(seq));
         }
 
         public CrlStatus(CrlSource source, Time thisUpdate)
         {
-            m_source = source;
+            m_source = source ?? throw new ArgumentNullException(nameof(source));
             m_thisUpdate = thisUpdate;
         }
 
@@ -55,10 +54,9 @@ namespace Org.BouncyCastle.Asn1.Cmp
 
         public override Asn1Object ToAsn1Object()
         {
-            if (m_thisUpdate == null)
-                return new DerSequence(m_source);
-
-            return new DerSequence(m_source, m_thisUpdate);
+            return m_thisUpdate == null
+                ?  new DerSequence(m_source)
+                :  new DerSequence(m_source, m_thisUpdate);
         }
     }
 }
diff --git a/crypto/src/asn1/cmp/DhbmParameter.cs b/crypto/src/asn1/cmp/DhbmParameter.cs
index 1b44b732e..71773f535 100644
--- a/crypto/src/asn1/cmp/DhbmParameter.cs
+++ b/crypto/src/asn1/cmp/DhbmParameter.cs
@@ -32,19 +32,20 @@ namespace Org.BouncyCastle.Asn1.Cmp
         private readonly AlgorithmIdentifier m_owf;
         private readonly AlgorithmIdentifier m_mac;
 
-        private DhbmParameter(Asn1Sequence sequence)
+        private DhbmParameter(Asn1Sequence seq)
         {
-            if (sequence.Count != 2)
-                throw new ArgumentException("expecting sequence size of 2");
+            int count = seq.Count;
+            if (count != 2)
+                throw new ArgumentException("Bad sequence size: " + count, nameof(seq));
 
-            m_owf = AlgorithmIdentifier.GetInstance(sequence[0]);
-            m_mac = AlgorithmIdentifier.GetInstance(sequence[1]);
+            m_owf = AlgorithmIdentifier.GetInstance(seq[0]);
+            m_mac = AlgorithmIdentifier.GetInstance(seq[1]);
         }
 
         public DhbmParameter(AlgorithmIdentifier owf, AlgorithmIdentifier mac)
         {
-            m_owf = owf;
-            m_mac = mac;
+            m_owf = owf ?? throw new ArgumentNullException(nameof(owf));
+            m_mac = mac ?? throw new ArgumentNullException(nameof(mac));
         }
 
         public virtual AlgorithmIdentifier Owf => m_owf;
diff --git a/crypto/src/asn1/cmp/ErrorMsgContent.cs b/crypto/src/asn1/cmp/ErrorMsgContent.cs
index 44646313e..1af52e65c 100644
--- a/crypto/src/asn1/cmp/ErrorMsgContent.cs
+++ b/crypto/src/asn1/cmp/ErrorMsgContent.cs
@@ -36,20 +36,16 @@ namespace Org.BouncyCastle.Asn1.Cmp
 
 		private ErrorMsgContent(Asn1Sequence seq)
 		{
-			m_pkiStatusInfo = PkiStatusInfo.GetInstance(seq[0]);
+            int count = seq.Count, pos = 0;
+            if (count < 1 || count > 3)
+                throw new ArgumentException("Bad sequence size: " + count, nameof(seq));
 
-			for (int pos = 1; pos < seq.Count; ++pos)
-			{
-				Asn1Encodable ae = seq[pos];
-				if (ae is DerInteger)
-				{
-					m_errorCode = DerInteger.GetInstance(ae);
-				}
-				else
-				{
-					m_errorDetails = PkiFreeText.GetInstance(ae);
-				}
-			}
+			m_pkiStatusInfo = PkiStatusInfo.GetInstance(seq[pos++]);
+			m_errorCode = Asn1Utilities.ReadOptional(seq, ref pos, DerInteger.GetOptional);
+            m_errorDetails = Asn1Utilities.ReadOptional(seq, ref pos, PkiFreeText.GetOptional);
+
+            if (pos != count)
+                throw new ArgumentException("Unexpected elements in sequence", nameof(seq));
 		}
 
 		public ErrorMsgContent(PkiStatusInfo pkiStatusInfo)
diff --git a/crypto/src/asn1/cmp/InfoTypeAndValue.cs b/crypto/src/asn1/cmp/InfoTypeAndValue.cs
index 03e055f10..fbe1293bc 100644
--- a/crypto/src/asn1/cmp/InfoTypeAndValue.cs
+++ b/crypto/src/asn1/cmp/InfoTypeAndValue.cs
@@ -69,9 +69,13 @@ namespace Org.BouncyCastle.Asn1.Cmp
 
         private InfoTypeAndValue(Asn1Sequence seq)
         {
+            int count = seq.Count;
+            if (count < 1 || count > 2)
+                throw new ArgumentException("Bad sequence size: " + count, nameof(seq));
+
             m_infoType = DerObjectIdentifier.GetInstance(seq[0]);
 
-            if (seq.Count > 1)
+            if (count > 1)
             {
                 m_infoValue = seq[1];
             }
@@ -103,10 +107,9 @@ namespace Org.BouncyCastle.Asn1.Cmp
          */
         public override Asn1Object ToAsn1Object()
         {
-            if (m_infoValue == null)
-                return new DerSequence(m_infoType);
-
-            return new DerSequence(m_infoType, m_infoValue);
+            return m_infoValue == null
+                ?  new DerSequence(m_infoType)
+                :  new DerSequence(m_infoType, m_infoValue);
         }
     }
 }
diff --git a/crypto/src/asn1/cmp/KemBMParameter.cs b/crypto/src/asn1/cmp/KemBMParameter.cs
index 846233054..a49dc4181 100644
--- a/crypto/src/asn1/cmp/KemBMParameter.cs
+++ b/crypto/src/asn1/cmp/KemBMParameter.cs
@@ -34,8 +34,9 @@ namespace Org.BouncyCastle.Asn1.Cmp
 
         private KemBMParameter(Asn1Sequence seq)
         {
-            if (seq.Count != 3)
-                throw new ArgumentException("sequence size should 3", nameof(seq));
+            int count = seq.Count;
+            if (count != 3)
+                throw new ArgumentException("Bad sequence size: " + count, nameof(seq));
 
             m_kdf = AlgorithmIdentifier.GetInstance(seq[0]);
             m_len = DerInteger.GetInstance(seq[1]);
@@ -44,9 +45,9 @@ namespace Org.BouncyCastle.Asn1.Cmp
 
         public KemBMParameter(AlgorithmIdentifier kdf, DerInteger len, AlgorithmIdentifier mac)
         {
-            m_kdf = kdf;
-            m_len = len;
-            m_mac = mac;
+            m_kdf = kdf ?? throw new ArgumentNullException(nameof(kdf));
+            m_len = len ?? throw new ArgumentNullException(nameof(len));
+            m_mac = mac ?? throw new ArgumentNullException(nameof(mac));
         }
 
         public KemBMParameter(AlgorithmIdentifier kdf, long len, AlgorithmIdentifier mac)
diff --git a/crypto/src/asn1/cmp/KemCiphertextInfo.cs b/crypto/src/asn1/cmp/KemCiphertextInfo.cs
index 7a6c3b25e..0fa68aa59 100644
--- a/crypto/src/asn1/cmp/KemCiphertextInfo.cs
+++ b/crypto/src/asn1/cmp/KemCiphertextInfo.cs
@@ -32,8 +32,9 @@ namespace Org.BouncyCastle.Asn1.Cmp
 
         private KemCiphertextInfo(Asn1Sequence seq)
         {
-            if (seq.Count != 2)
-                throw new ArgumentException("sequence size should 2", nameof(seq));
+            int count = seq.Count;
+            if (count != 2)
+                throw new ArgumentException("Bad sequence size: " + count, nameof(seq));
 
             m_kem = AlgorithmIdentifier.GetInstance(seq[0]);
             m_ct = Asn1OctetString.GetInstance(seq[1]);
@@ -41,8 +42,8 @@ namespace Org.BouncyCastle.Asn1.Cmp
 
         public KemCiphertextInfo(AlgorithmIdentifier kem, Asn1OctetString ct)
         {
-            m_kem = kem;
-            m_ct = ct;
+            m_kem = kem ?? throw new ArgumentNullException(nameof(kem));
+            m_ct = ct ?? throw new ArgumentNullException(nameof(ct));
         }
 
         public virtual AlgorithmIdentifier Kem => m_kem;
diff --git a/crypto/src/asn1/cmp/KemOtherInfo.cs b/crypto/src/asn1/cmp/KemOtherInfo.cs
index 3185495fc..7b46dd398 100644
--- a/crypto/src/asn1/cmp/KemOtherInfo.cs
+++ b/crypto/src/asn1/cmp/KemOtherInfo.cs
@@ -62,47 +62,23 @@ namespace Org.BouncyCastle.Asn1.Cmp
 
         private KemOtherInfo(Asn1Sequence seq)
         {
-            if (seq.Count < 4 || seq.Count > 7)
-                throw new ArgumentException("sequence size should be between 4 and 7 inclusive", nameof(seq));
+            int count = seq.Count, pos = 0;
+            if (count < 4 || count > 7)
+                throw new ArgumentException("Bad sequence size: " + count, nameof(seq));
 
-            int seqPos = 0;
-
-            m_staticString = PkiFreeText.GetInstance(seq[seqPos]);
+            m_staticString = PkiFreeText.GetInstance(seq[pos++]);
             if (!DEFAULT_staticString.Equals(m_staticString))
                 throw new ArgumentException("staticString field should be " + DEFAULT_staticString);
 
-            Asn1TaggedObject tagged = seq[++seqPos] as Asn1TaggedObject;
-
-            if (tagged != null &&
-                Asn1Utilities.TryGetContextBaseUniversal(tagged, 0, true, Asn1Tags.OctetString, out var transactionID))
-            {
-                m_transactionID = (Asn1OctetString)transactionID;
-                tagged = seq[++seqPos] as Asn1TaggedObject;
-            }
-
-            if (tagged != null &&
-                Asn1Utilities.TryGetContextBaseUniversal(tagged, 1, true, Asn1Tags.OctetString, out var senderNonce))
-            {
-                m_senderNonce = (Asn1OctetString)senderNonce;
-                tagged = seq[++seqPos] as Asn1TaggedObject;
-            }
-
-            if (tagged != null &&
-                Asn1Utilities.TryGetContextBaseUniversal(tagged, 2, true, Asn1Tags.OctetString, out var recipNonce))
-            {
-                m_recipNonce = (Asn1OctetString)recipNonce;
-                tagged = seq[++seqPos] as Asn1TaggedObject;
-            }
-
-            if (tagged != null)
-                throw new ArgumentException("unknown tag: " + Asn1Utilities.GetTagText(tagged));
-
-            m_len = DerInteger.GetInstance(seq[seqPos]);
-            m_mac = AlgorithmIdentifier.GetInstance(seq[++seqPos]);
-            m_ct = Asn1OctetString.GetInstance(seq[++seqPos]);
-
-            if (++seqPos != seq.Count)
-                throw new ArgumentException("unexpected data at end of sequence", nameof(seq));
+            m_transactionID = Asn1Utilities.ReadOptionalContextTagged(seq, ref pos, 0, true, Asn1OctetString.GetInstance);
+            m_senderNonce = Asn1Utilities.ReadOptionalContextTagged(seq, ref pos, 1, true, Asn1OctetString.GetInstance);
+            m_recipNonce = Asn1Utilities.ReadOptionalContextTagged(seq, ref pos, 2, true, Asn1OctetString.GetInstance);
+            m_len = DerInteger.GetInstance(seq[pos++]);
+            m_mac = AlgorithmIdentifier.GetInstance(seq[pos++]);
+            m_ct = Asn1OctetString.GetInstance(seq[pos++]);
+
+            if (pos != count)
+                throw new ArgumentException("Unexpected elements in sequence", nameof(seq));
         }
 
         public virtual Asn1OctetString TransactionID => m_transactionID;
diff --git a/crypto/src/asn1/cmp/KeyRecRepContent.cs b/crypto/src/asn1/cmp/KeyRecRepContent.cs
index e465346eb..ec604f644 100644
--- a/crypto/src/asn1/cmp/KeyRecRepContent.cs
+++ b/crypto/src/asn1/cmp/KeyRecRepContent.cs
@@ -26,28 +26,18 @@ namespace Org.BouncyCastle.Asn1.Cmp
 
 		private KeyRecRepContent(Asn1Sequence seq)
 		{
-			m_status = PkiStatusInfo.GetInstance(seq[0]);
+            int count = seq.Count, pos = 0;
+            if (count < 1 || count > 4)
+                throw new ArgumentException("Bad sequence size: " + count, nameof(seq));
 
-			for (int pos = 1; pos < seq.Count; ++pos)
-			{
-				Asn1TaggedObject tObj = Asn1TaggedObject.GetInstance(seq[pos], Asn1Tags.ContextSpecific);
+            m_status = PkiStatusInfo.GetInstance(seq[pos++]);
+            m_newSigCert = Asn1Utilities.ReadOptionalContextTagged(seq, ref pos, 0, true, CmpCertificate.GetInstance);
+            m_caCerts = Asn1Utilities.ReadOptionalContextTagged(seq, ref pos, 1, true, Asn1Sequence.GetInstance);
+            m_keyPairHist = Asn1Utilities.ReadOptionalContextTagged(seq, ref pos, 2, true, Asn1Sequence.GetInstance);
 
-				switch (tObj.TagNo)
-				{
-				case 0:
-					m_newSigCert = CmpCertificate.GetInstance(tObj.GetExplicitBaseObject());
-					break;
-				case 1:
-					m_caCerts = Asn1Sequence.GetInstance(tObj.GetExplicitBaseObject());
-					break;
-				case 2:
-					m_keyPairHist = Asn1Sequence.GetInstance(tObj.GetExplicitBaseObject());
-					break;
-				default:
-					throw new ArgumentException("unknown tag number: " + tObj.TagNo);
-				}
-			}
-		}
+            if (pos != count)
+                throw new ArgumentException("Unexpected elements in sequence", nameof(seq));
+        }
 
 		public virtual PkiStatusInfo Status => m_status;
 
diff --git a/crypto/src/asn1/cmp/OobCertHash.cs b/crypto/src/asn1/cmp/OobCertHash.cs
index 209113167..f1531f309 100644
--- a/crypto/src/asn1/cmp/OobCertHash.cs
+++ b/crypto/src/asn1/cmp/OobCertHash.cs
@@ -40,29 +40,25 @@ namespace Org.BouncyCastle.Asn1.Cmp
 
 		private OobCertHash(Asn1Sequence seq)
 		{
-			int index = seq.Count - 1;
+            int count = seq.Count, pos = 0;
+            if (count < 1 || count > 3)
+                throw new ArgumentException("Bad sequence size: " + count, nameof(seq));
 
-			m_hashVal = DerBitString.GetInstance(seq[index--]);
+			m_hashAlg = Asn1Utilities.ReadOptionalContextTagged(seq, ref pos, 0, true, AlgorithmIdentifier.GetInstance);
+            m_certId = Asn1Utilities.ReadOptionalContextTagged(seq, ref pos, 1, true, CertId.GetInstance);
+			m_hashVal = DerBitString.GetInstance(seq[pos++]);
 
-			for (int i = index; i >= 0; i--)
-			{
-				Asn1TaggedObject tObj = (Asn1TaggedObject)seq[i];
-
-				if (tObj.HasContextTag(0))
-				{
-					m_hashAlg = AlgorithmIdentifier.GetInstance(tObj, true);
-				}
-				else if (tObj.HasContextTag(1))
-				{
-					m_certId = CertId.GetInstance(tObj, true);
-				}
-				else
-				{
-                    throw new ArgumentException("unknown tag: " + Asn1Utilities.GetTagText(tObj));
-                }
-            }
+			if (pos != count)
+                throw new ArgumentException("Unexpected elements in sequence", nameof(seq));
 		}
 
+        public OobCertHash(AlgorithmIdentifier hashAlg, CertId certId, DerBitString hashVal)
+        {
+            m_hashAlg = hashAlg;
+            m_certId = certId;
+            m_hashVal = hashVal ?? throw new ArgumentNullException(nameof(hashVal));
+        }
+
 		public virtual CertId CertID => m_certId;
 
         public virtual AlgorithmIdentifier HashAlg => m_hashAlg;
diff --git a/crypto/src/asn1/cmp/PKIFreeText.cs b/crypto/src/asn1/cmp/PKIFreeText.cs
index a567852e4..e2845c529 100644
--- a/crypto/src/asn1/cmp/PKIFreeText.cs
+++ b/crypto/src/asn1/cmp/PKIFreeText.cs
@@ -19,6 +19,21 @@ namespace Org.BouncyCastle.Asn1.Cmp
             return new PkiFreeText(Asn1Sequence.GetInstance(taggedObject, declaredExplicit));
         }
 
+        public static PkiFreeText GetOptional(Asn1Encodable element)
+        {
+            if (element == null)
+                throw new ArgumentNullException(nameof(element));
+
+            if (element is PkiFreeText pkiFreeText)
+                return pkiFreeText;
+
+            Asn1Sequence asn1Sequence = Asn1Sequence.GetOptional(element);
+            if (asn1Sequence != null)
+                return new PkiFreeText(asn1Sequence);
+
+            return null;
+        }
+
         private readonly Asn1Sequence m_strings;
 
         internal PkiFreeText(Asn1Sequence seq)
diff --git a/crypto/src/asn1/cmp/PKIHeader.cs b/crypto/src/asn1/cmp/PKIHeader.cs
index c000c8b98..71dc18e7d 100644
--- a/crypto/src/asn1/cmp/PKIHeader.cs
+++ b/crypto/src/asn1/cmp/PKIHeader.cs
@@ -10,7 +10,7 @@ namespace Org.BouncyCastle.Asn1.Cmp
         /**
          * Value for a "null" recipient or sender.
          */
-        public static readonly GeneralName NULL_NAME = new GeneralName(X509Name.GetInstance(new DerSequence()));
+        public static readonly GeneralName NULL_NAME = new GeneralName(X509Name.GetInstance(DerSequence.Empty));
 
         public static readonly int CMP_1999 = 1;
         public static readonly int CMP_2000 = 2;
@@ -29,143 +29,77 @@ namespace Org.BouncyCastle.Asn1.Cmp
             return new PkiHeader(Asn1Sequence.GetInstance(taggedObject, declaredExplicit));
         }
 
-        private readonly DerInteger pvno;
-        private readonly GeneralName sender;
-        private readonly GeneralName recipient;
-        private readonly Asn1GeneralizedTime messageTime;
-        private readonly AlgorithmIdentifier protectionAlg;
-        private readonly Asn1OctetString senderKID;       // KeyIdentifier
-        private readonly Asn1OctetString recipKID;        // KeyIdentifier
-        private readonly Asn1OctetString transactionID;
-        private readonly Asn1OctetString senderNonce;
-        private readonly Asn1OctetString recipNonce;
-        private readonly PkiFreeText freeText;
-        private readonly Asn1Sequence generalInfo;
+        private readonly DerInteger m_pvno;
+        private readonly GeneralName m_sender;
+        private readonly GeneralName m_recipient;
+        private readonly Asn1GeneralizedTime m_messageTime;
+        private readonly AlgorithmIdentifier m_protectionAlg;
+        private readonly Asn1OctetString m_senderKID;       // KeyIdentifier
+        private readonly Asn1OctetString m_recipKID;        // KeyIdentifier
+        private readonly Asn1OctetString m_transactionID;
+        private readonly Asn1OctetString m_senderNonce;
+        private readonly Asn1OctetString m_recipNonce;
+        private readonly PkiFreeText m_freeText;
+        private readonly Asn1Sequence m_generalInfo;
 
         private PkiHeader(Asn1Sequence seq)
         {
-            pvno = DerInteger.GetInstance(seq[0]);
-            sender = GeneralName.GetInstance(seq[1]);
-            recipient = GeneralName.GetInstance(seq[2]);
-
-            for (int pos = 3; pos < seq.Count; ++pos)
-            {
-                Asn1TaggedObject tObj = Asn1TaggedObject.GetInstance(seq[pos]);
-                if (!tObj.HasContextTag())
-                    throw new ArgumentException("unknown tag: " + Asn1Utilities.GetTagText(tObj));
-
-                switch (tObj.TagNo)
-                {
-                case 0:
-                    messageTime = Asn1GeneralizedTime.GetInstance(tObj, true);
-                    break;
-                case 1:
-                    protectionAlg = AlgorithmIdentifier.GetInstance(tObj, true);
-                    break;
-                case 2:
-                    senderKID = Asn1OctetString.GetInstance(tObj, true);
-                    break;
-                case 3:
-                    recipKID = Asn1OctetString.GetInstance(tObj, true);
-                    break;
-                case 4:
-                    transactionID = Asn1OctetString.GetInstance(tObj, true);
-                    break;
-                case 5:
-                    senderNonce = Asn1OctetString.GetInstance(tObj, true);
-                    break;
-                case 6:
-                    recipNonce = Asn1OctetString.GetInstance(tObj, true);
-                    break;
-                case 7:
-                    freeText = PkiFreeText.GetInstance(tObj, true);
-                    break;
-                case 8:
-                    generalInfo = Asn1Sequence.GetInstance(tObj, true);
-                    break;
-                default:
-                    throw new ArgumentException("unknown tag number: " + tObj.TagNo);
-                }
-            }
-        }
-
-        public PkiHeader(
-            int pvno,
-            GeneralName sender,
-            GeneralName recipient)
+            int count = seq.Count, pos = 0;
+            if (count < 3 || count > 12)
+                throw new ArgumentException("Bad sequence size: " + count, nameof(seq));
+
+            m_pvno = DerInteger.GetInstance(seq[pos++]);
+            m_sender = GeneralName.GetInstance(seq[pos++]);
+            m_recipient = GeneralName.GetInstance(seq[pos++]);
+            m_messageTime = Asn1Utilities.ReadOptionalContextTagged(seq, ref pos, 0, true, Asn1GeneralizedTime.GetInstance);
+            m_protectionAlg = Asn1Utilities.ReadOptionalContextTagged(seq, ref pos, 1, true, AlgorithmIdentifier.GetInstance);
+            m_senderKID = Asn1Utilities.ReadOptionalContextTagged(seq, ref pos, 2, true, Asn1OctetString.GetInstance);
+            m_recipKID = Asn1Utilities.ReadOptionalContextTagged(seq, ref pos, 3, true, Asn1OctetString.GetInstance);
+            m_transactionID = Asn1Utilities.ReadOptionalContextTagged(seq, ref pos, 4, true, Asn1OctetString.GetInstance);
+            m_senderNonce = Asn1Utilities.ReadOptionalContextTagged(seq, ref pos, 5, true, Asn1OctetString.GetInstance);
+            m_recipNonce = Asn1Utilities.ReadOptionalContextTagged(seq, ref pos, 6, true, Asn1OctetString.GetInstance);
+            m_freeText = Asn1Utilities.ReadOptionalContextTagged(seq, ref pos, 7, true, PkiFreeText.GetInstance);
+            m_generalInfo = Asn1Utilities.ReadOptionalContextTagged(seq, ref pos, 8, true, Asn1Sequence.GetInstance);
+
+            if (pos != count)
+                throw new ArgumentException("Unexpected elements in sequence", nameof(seq));
+        }
+
+        public PkiHeader(int pvno, GeneralName sender, GeneralName recipient)
             : this(new DerInteger(pvno), sender, recipient)
         {
         }
 
-        private PkiHeader(
-            DerInteger pvno,
-            GeneralName sender,
-            GeneralName recipient)
+        private PkiHeader(DerInteger pvno, GeneralName sender, GeneralName recipient)
         {
-            this.pvno = pvno;
-            this.sender = sender;
-            this.recipient = recipient;
+            m_pvno = pvno ?? throw new ArgumentNullException(nameof(pvno));
+            m_sender = sender ?? throw new ArgumentNullException(nameof(sender));
+            m_recipient = recipient ?? throw new ArgumentNullException(nameof(recipient));
         }
 
-        public virtual DerInteger Pvno
-        {
-            get { return pvno; }
-        }
+        public virtual DerInteger Pvno => m_pvno;
 
-        public virtual GeneralName Sender
-        {
-            get { return sender; }
-        }
+        public virtual GeneralName Sender => m_sender;
 
-        public virtual GeneralName Recipient
-        {
-            get { return recipient; }
-        }
+        public virtual GeneralName Recipient => m_recipient;
 
-        public virtual Asn1GeneralizedTime MessageTime
-        {
-            get { return messageTime; }
-        }
+        public virtual Asn1GeneralizedTime MessageTime => m_messageTime;
 
-        public virtual AlgorithmIdentifier ProtectionAlg
-        {
-            get { return protectionAlg; }
-        }
+        public virtual AlgorithmIdentifier ProtectionAlg => m_protectionAlg;
 
-        public virtual Asn1OctetString SenderKID
-        {   
-            get { return senderKID; }
-        }
+        public virtual Asn1OctetString SenderKID => m_senderKID;
 
-        public virtual Asn1OctetString RecipKID
-        {   
-            get { return recipKID; }
-        }
+        public virtual Asn1OctetString RecipKID => m_recipKID;
 
-        public virtual Asn1OctetString TransactionID
-        {   
-            get { return transactionID; }
-        }
+        public virtual Asn1OctetString TransactionID => m_transactionID;
 
-        public virtual Asn1OctetString SenderNonce
-        {   
-            get { return senderNonce; }
-        }
+        public virtual Asn1OctetString SenderNonce => m_senderNonce;
 
-        public virtual Asn1OctetString RecipNonce
-        {   
-            get { return recipNonce; }
-        }
+        public virtual Asn1OctetString RecipNonce => m_recipNonce;
 
-        public virtual PkiFreeText FreeText
-        {
-            get { return freeText; }
-        }
+        public virtual PkiFreeText FreeText => m_freeText;
 
-        public virtual InfoTypeAndValue[] GetGeneralInfo()
-        {
-            return generalInfo?.MapElements(InfoTypeAndValue.GetInstance);
-        }
+        public virtual InfoTypeAndValue[] GetGeneralInfo() => m_generalInfo?.MapElements(InfoTypeAndValue.GetInstance);
 
         /**
          * <pre>
@@ -207,16 +141,17 @@ namespace Org.BouncyCastle.Asn1.Cmp
          */
         public override Asn1Object ToAsn1Object()
         {
-            Asn1EncodableVector v = new Asn1EncodableVector(pvno, sender, recipient);
-            v.AddOptionalTagged(true, 0, messageTime);
-            v.AddOptionalTagged(true, 1, protectionAlg);
-            v.AddOptionalTagged(true, 2, senderKID);
-            v.AddOptionalTagged(true, 3, recipKID);
-            v.AddOptionalTagged(true, 4, transactionID);
-            v.AddOptionalTagged(true, 5, senderNonce);
-            v.AddOptionalTagged(true, 6, recipNonce);
-            v.AddOptionalTagged(true, 7, freeText);
-            v.AddOptionalTagged(true, 8, generalInfo);
+            Asn1EncodableVector v = new Asn1EncodableVector(12);
+            v.Add(m_pvno, m_sender, m_recipient);
+            v.AddOptionalTagged(true, 0, m_messageTime);
+            v.AddOptionalTagged(true, 1, m_protectionAlg);
+            v.AddOptionalTagged(true, 2, m_senderKID);
+            v.AddOptionalTagged(true, 3, m_recipKID);
+            v.AddOptionalTagged(true, 4, m_transactionID);
+            v.AddOptionalTagged(true, 5, m_senderNonce);
+            v.AddOptionalTagged(true, 6, m_recipNonce);
+            v.AddOptionalTagged(true, 7, m_freeText);
+            v.AddOptionalTagged(true, 8, m_generalInfo);
             return new DerSequence(v);
         }
     }
diff --git a/crypto/src/asn1/cmp/PKIHeaderBuilder.cs b/crypto/src/asn1/cmp/PKIHeaderBuilder.cs
index 914c8a8fa..3426c8f0e 100644
--- a/crypto/src/asn1/cmp/PKIHeaderBuilder.cs
+++ b/crypto/src/asn1/cmp/PKIHeaderBuilder.cs
@@ -6,9 +6,9 @@ namespace Org.BouncyCastle.Asn1.Cmp
 {
 	public class PkiHeaderBuilder
 	{
-		private DerInteger pvno;
-		private GeneralName sender;
-		private GeneralName recipient;
+		private readonly DerInteger pvno;
+		private readonly GeneralName sender;
+		private readonly GeneralName recipient;
 		private Asn1GeneralizedTime messageTime;
 		private AlgorithmIdentifier protectionAlg;
 		private Asn1OctetString senderKID;       // KeyIdentifier
@@ -19,22 +19,16 @@ namespace Org.BouncyCastle.Asn1.Cmp
 		private PkiFreeText     freeText;
 		private Asn1Sequence    generalInfo;
 
-		public PkiHeaderBuilder(
-			int			pvno,
-			GeneralName	sender,
-			GeneralName	recipient)
-			: this(new DerInteger(pvno), sender, recipient)
-		{
-		}
+        public PkiHeaderBuilder(int pvno, GeneralName sender, GeneralName recipient)
+            : this(new DerInteger(pvno), sender, recipient)
+        {
+        }
 
-		private PkiHeaderBuilder(
-			DerInteger	pvno,
-			GeneralName	sender,
-			GeneralName	recipient)
-		{
-			this.pvno = pvno;
-			this.sender = sender;
-			this.recipient = recipient;
+        private PkiHeaderBuilder(DerInteger pvno, GeneralName sender, GeneralName recipient)
+        {
+            this.pvno = pvno ?? throw new ArgumentNullException(nameof(pvno));
+			this.sender = sender ?? throw new ArgumentNullException(nameof(sender));
+			this.recipient = recipient ?? throw new ArgumentNullException(nameof(recipient));
 		}
 
 		public virtual PkiHeaderBuilder SetMessageTime(Asn1GeneralizedTime time)
@@ -133,7 +127,7 @@ namespace Org.BouncyCastle.Asn1.Cmp
 
 		private static Asn1Sequence MakeGeneralInfoSeq(InfoTypeAndValue[] generalInfos)
 		{
-			return generalInfos == null ? null : new DerSequence(generalInfos);
+			return generalInfos == null ? null : DerSequence.FromElements(generalInfos);
 		}
 
 		/**
diff --git a/crypto/src/asn1/cmp/PKIMessage.cs b/crypto/src/asn1/cmp/PKIMessage.cs
index e835b6f16..4b5acad77 100644
--- a/crypto/src/asn1/cmp/PKIMessage.cs
+++ b/crypto/src/asn1/cmp/PKIMessage.cs
@@ -26,22 +26,17 @@ namespace Org.BouncyCastle.Asn1.Cmp
 
         private PkiMessage(Asn1Sequence seq)
         {
-            m_header = PkiHeader.GetInstance(seq[0]);
-            m_body = PkiBody.GetInstance(seq[1]);
+            int count = seq.Count, pos = 0;
+            if (count < 2 || count > 4)
+                throw new ArgumentException("Bad sequence size: " + count, nameof(seq));
 
-            for (int pos = 2; pos < seq.Count; ++pos)
-            {
-                Asn1TaggedObject tObj = Asn1TaggedObject.GetInstance(seq[pos]);
+            m_header = PkiHeader.GetInstance(seq[pos++]);
+            m_body = PkiBody.GetInstance(seq[pos++]);
+            m_protection = Asn1Utilities.ReadOptionalContextTagged(seq, ref pos, 0, true, DerBitString.GetInstance);
+            m_extraCerts = Asn1Utilities.ReadOptionalContextTagged(seq, ref pos, 1, true, Asn1Sequence.GetInstance);
 
-                if (tObj.HasContextTag(0))
-                {
-                    m_protection = DerBitString.GetInstance(tObj, true);
-                }
-                else if (tObj.HasContextTag(1))
-                {
-                    m_extraCerts = Asn1Sequence.GetInstance(tObj, true);
-                }
-            }
+            if (pos != count)
+                throw new ArgumentException("Unexpected elements in sequence", nameof(seq));
         }
 
         /**
@@ -54,13 +49,10 @@ namespace Org.BouncyCastle.Asn1.Cmp
          */
         public PkiMessage(PkiHeader header, PkiBody body, DerBitString protection, CmpCertificate[] extraCerts)
         {
-            m_header = header;
-            m_body = body;
+            m_header = header ?? throw new ArgumentNullException(nameof(header));
+            m_body = body ?? throw new ArgumentNullException(nameof(body));
             m_protection = protection;
-            if (extraCerts != null)
-            {
-                m_extraCerts = new DerSequence(extraCerts);
-            }
+            m_extraCerts = extraCerts == null ? null : DerSequence.FromElements(extraCerts);
         }
 
         public PkiMessage(PkiHeader header, PkiBody body, DerBitString protection)
diff --git a/crypto/src/asn1/cmp/PKIStatusInfo.cs b/crypto/src/asn1/cmp/PKIStatusInfo.cs
index fbd180c6d..f88651008 100644
--- a/crypto/src/asn1/cmp/PKIStatusInfo.cs
+++ b/crypto/src/asn1/cmp/PKIStatusInfo.cs
@@ -25,6 +25,23 @@ namespace Org.BouncyCastle.Asn1.Cmp
 #pragma warning restore CS0618 // Type or member is obsolete
         }
 
+        public static PkiStatusInfo GetOptional(Asn1Encodable element)
+        {
+            if (element == null)
+                throw new ArgumentNullException(nameof(element));
+
+            if (element is PkiStatusInfo pkiStatusInfo)
+                return pkiStatusInfo;
+
+            Asn1Sequence asn1Sequence = Asn1Sequence.GetOptional(element);
+            if (asn1Sequence != null)
+#pragma warning disable CS0618 // Type or member is obsolete
+                return new PkiStatusInfo(asn1Sequence);
+#pragma warning restore CS0618 // Type or member is obsolete
+
+            return null;
+        }
+
         private readonly DerInteger m_status;
 		private readonly PkiFreeText m_statusString;
 		private readonly DerBitString m_failInfo;
@@ -32,28 +49,16 @@ namespace Org.BouncyCastle.Asn1.Cmp
         [Obsolete("Use 'GetInstance' instead")]
         public PkiStatusInfo(Asn1Sequence seq)
 		{
-			m_status = DerInteger.GetInstance(seq[0]);
+            int count = seq.Count, pos = 0;
+            if (count < 1 || count > 3)
+                throw new ArgumentException("Bad sequence size: " + count, nameof(seq));
 
-			m_statusString = null;
-			m_failInfo = null;
+			m_status = DerInteger.GetInstance(seq[pos++]);
+			m_statusString = Asn1Utilities.ReadOptional(seq, ref pos, PkiFreeText.GetOptional);
+            m_failInfo = Asn1Utilities.ReadOptional(seq, ref pos, DerBitString.GetOptional);
 
-			if (seq.Count > 2)
-			{
-				m_statusString = PkiFreeText.GetInstance(seq[1]);
-				m_failInfo = DerBitString.GetInstance(seq[2]);
-			}
-			else if (seq.Count > 1)
-			{
-				object obj = seq[1];
-				if (obj is DerBitString)
-				{
-					m_failInfo = DerBitString.GetInstance(obj);
-				}
-				else
-				{
-					m_statusString = PkiFreeText.GetInstance(obj);
-				}
-			}
+            if (pos != count)
+                throw new ArgumentException("Unexpected elements in sequence", nameof(seq));
 		}
 
 		public PkiStatusInfo(int status)
diff --git a/crypto/src/asn1/cmp/PbmParameter.cs b/crypto/src/asn1/cmp/PbmParameter.cs
index aeb91d7d0..8119e4faf 100644
--- a/crypto/src/asn1/cmp/PbmParameter.cs
+++ b/crypto/src/asn1/cmp/PbmParameter.cs
@@ -1,4 +1,6 @@
-using Org.BouncyCastle.Asn1.X509;
+using System;
+
+using Org.BouncyCastle.Asn1.X509;
 
 namespace Org.BouncyCastle.Asn1.Cmp
 {
@@ -43,6 +45,10 @@ namespace Org.BouncyCastle.Asn1.Cmp
 
         private PbmParameter(Asn1Sequence seq)
         {
+            int count = seq.Count;
+            if (count != 4)
+                throw new ArgumentException("Bad sequence size: " + count, nameof(seq));
+
             m_salt = Asn1OctetString.GetInstance(seq[0]);
             m_owf = AlgorithmIdentifier.GetInstance(seq[1]);
             m_iterationCount = DerInteger.GetInstance(seq[2]);
@@ -57,10 +63,10 @@ namespace Org.BouncyCastle.Asn1.Cmp
         public PbmParameter(Asn1OctetString salt, AlgorithmIdentifier owf, DerInteger iterationCount,
             AlgorithmIdentifier mac)
         {
-            m_salt = salt;
-            m_owf = owf;
-            m_iterationCount = iterationCount;
-            m_mac = mac;
+            m_salt = salt ?? throw new ArgumentNullException(nameof(salt));
+            m_owf = owf ?? throw new ArgumentNullException(nameof(owf));
+            m_iterationCount = iterationCount ?? throw new ArgumentNullException(nameof(iterationCount));
+            m_mac = mac ?? throw new ArgumentNullException(nameof(mac));
         }
 
         public virtual DerInteger IterationCount => m_iterationCount;
diff --git a/crypto/src/asn1/cmp/ProtectedPart.cs b/crypto/src/asn1/cmp/ProtectedPart.cs
index e6b6311df..7c7110517 100644
--- a/crypto/src/asn1/cmp/ProtectedPart.cs
+++ b/crypto/src/asn1/cmp/ProtectedPart.cs
@@ -1,3 +1,5 @@
+using System;
+
 namespace Org.BouncyCastle.Asn1.Cmp
 {
 	public class ProtectedPart
@@ -22,14 +24,18 @@ namespace Org.BouncyCastle.Asn1.Cmp
 
 		private ProtectedPart(Asn1Sequence seq)
 		{
-			m_header = PkiHeader.GetInstance(seq[0]);
+            int count = seq.Count;
+            if (count != 2)
+                throw new ArgumentException("Bad sequence size: " + count, nameof(seq));
+
+            m_header = PkiHeader.GetInstance(seq[0]);
 			m_body = PkiBody.GetInstance(seq[1]);
 		}
 
 		public ProtectedPart(PkiHeader header, PkiBody body)
 		{
-			m_header = header;
-			m_body = body;
+			m_header = header ?? throw new ArgumentNullException(nameof(header));
+			m_body = body ?? throw new ArgumentNullException(nameof(body));
 		}
 
 		public virtual PkiHeader Header => m_header;
diff --git a/crypto/src/asn1/cmp/RevAnnContent.cs b/crypto/src/asn1/cmp/RevAnnContent.cs
index 5ac731fb8..e94beb0df 100644
--- a/crypto/src/asn1/cmp/RevAnnContent.cs
+++ b/crypto/src/asn1/cmp/RevAnnContent.cs
@@ -1,3 +1,5 @@
+using System;
+
 using Org.BouncyCastle.Asn1.Crmf;
 using Org.BouncyCastle.Asn1.X509;
 
@@ -35,24 +37,27 @@ namespace Org.BouncyCastle.Asn1.Cmp
         public RevAnnContent(PkiStatusEncodable status, CertId certID, Asn1GeneralizedTime willBeRevokedAt,
             Asn1GeneralizedTime badSinceDate, X509Extensions crlDetails)
         {
-            m_status = status;
-            m_certID = certID;
-            m_willBeRevokedAt = willBeRevokedAt;
-            m_badSinceDate = badSinceDate;
+            m_status = status ?? throw new ArgumentNullException(nameof(status));
+            m_certID = certID ?? throw new ArgumentNullException(nameof(certID));
+            m_willBeRevokedAt = willBeRevokedAt ?? throw new ArgumentNullException(nameof(willBeRevokedAt));
+            m_badSinceDate = badSinceDate ?? throw new ArgumentNullException(nameof(badSinceDate));
             m_crlDetails = crlDetails;
         }
 
         private RevAnnContent(Asn1Sequence seq)
 		{
-			m_status = PkiStatusEncodable.GetInstance(seq[0]);
-			m_certID = CertId.GetInstance(seq[1]);
-			m_willBeRevokedAt = Asn1GeneralizedTime.GetInstance(seq[2]);
-			m_badSinceDate = Asn1GeneralizedTime.GetInstance(seq[3]);
+            int count = seq.Count, pos = 0;
+            if (count < 4 || count > 5)
+                throw new ArgumentException("Bad sequence size: " + count, nameof(seq));
+
+            m_status = PkiStatusEncodable.GetInstance(seq[pos++]);
+            m_certID = CertId.GetInstance(seq[pos++]);
+            m_willBeRevokedAt = Asn1GeneralizedTime.GetInstance(seq[pos++]);
+            m_badSinceDate = Asn1GeneralizedTime.GetInstance(seq[pos++]);
+            m_crlDetails = Asn1Utilities.ReadOptional(seq, ref pos, X509Extensions.GetOptional);
 
-			if (seq.Count > 4)
-			{
-				m_crlDetails = X509Extensions.GetInstance(seq[4]);
-			}
+            if (pos != count)
+                throw new ArgumentException("Unexpected elements in sequence", nameof(seq));
 		}
 
 		public virtual PkiStatusEncodable Status => m_status;
diff --git a/crypto/src/asn1/cmp/RevDetails.cs b/crypto/src/asn1/cmp/RevDetails.cs
index 718a9ef71..b1b5981b1 100644
--- a/crypto/src/asn1/cmp/RevDetails.cs
+++ b/crypto/src/asn1/cmp/RevDetails.cs
@@ -1,3 +1,5 @@
+using System;
+
 using Org.BouncyCastle.Asn1.Crmf;
 using Org.BouncyCastle.Asn1.X509;
 
@@ -37,12 +39,15 @@ namespace Org.BouncyCastle.Asn1.Cmp
 
         private RevDetails(Asn1Sequence seq)
 		{
-			m_certDetails = CertTemplate.GetInstance(seq[0]);
+            int count = seq.Count, pos = 0;
+            if (count < 1 || count > 2)
+                throw new ArgumentException("Bad sequence size: " + count, nameof(seq));
+
+            m_certDetails = CertTemplate.GetInstance(seq[pos++]);
+            m_crlEntryDetails = Asn1Utilities.ReadOptional(seq, ref pos, X509Extensions.GetOptional);
 
-            if (seq.Count > 1)
-            {
-                m_crlEntryDetails = X509Extensions.GetInstance(seq[1]);
-            }
+            if (pos != count)
+                throw new ArgumentException("Unexpected elements in sequence", nameof(seq));
 		}
 
 		public RevDetails(CertTemplate certDetails)
@@ -52,7 +57,7 @@ namespace Org.BouncyCastle.Asn1.Cmp
 
         public RevDetails(CertTemplate certDetails, X509Extensions crlEntryDetails)
 		{
-            m_certDetails = certDetails;
+            m_certDetails = certDetails ?? throw new ArgumentNullException(nameof(certDetails));
             m_crlEntryDetails = crlEntryDetails;
 		}
 
@@ -75,10 +80,9 @@ namespace Org.BouncyCastle.Asn1.Cmp
 		*/
 		public override Asn1Object ToAsn1Object()
 		{
-            Asn1EncodableVector v = new Asn1EncodableVector(2);
-            v.Add(m_certDetails);
-			v.AddOptional(m_crlEntryDetails);
-			return new DerSequence(v);
+            return m_crlEntryDetails == null
+                ?  new DerSequence(m_certDetails)
+                :  new DerSequence(m_certDetails, m_crlEntryDetails);
 		}
 	}
 }
diff --git a/crypto/src/asn1/cmp/RevRepContent.cs b/crypto/src/asn1/cmp/RevRepContent.cs
index ae1d0baed..e9b3230c8 100644
--- a/crypto/src/asn1/cmp/RevRepContent.cs
+++ b/crypto/src/asn1/cmp/RevRepContent.cs
@@ -1,3 +1,5 @@
+using System;
+
 using Org.BouncyCastle.Asn1.Crmf;
 using Org.BouncyCastle.Asn1.X509;
 
@@ -40,24 +42,19 @@ namespace Org.BouncyCastle.Asn1.Cmp
 
 		private RevRepContent(Asn1Sequence seq)
 		{
-			m_status = Asn1Sequence.GetInstance(seq[0]);
+            int count = seq.Count, pos = 0;
+            if (count < 1 || count > 3)
+                throw new ArgumentException("Bad sequence size: " + count, nameof(seq));
 
-			for (int pos = 1; pos < seq.Count; ++pos)
-			{
-				Asn1TaggedObject tObj = Asn1TaggedObject.GetInstance(seq[pos]);
+            m_status = Asn1Sequence.GetInstance(seq[pos++]);
+			m_revCerts = Asn1Utilities.ReadOptionalContextTagged(seq, ref pos, 0, true, Asn1Sequence.GetInstance);
+            m_crls = Asn1Utilities.ReadOptionalContextTagged(seq, ref pos, 1, true, Asn1Sequence.GetInstance);
 
-				if (tObj.HasContextTag(0))
-				{
-					m_revCerts = Asn1Sequence.GetInstance(tObj, true);
-				}
-				else if (tObj.HasContextTag(1))
-				{
-					m_crls = Asn1Sequence.GetInstance(tObj, true);
-				}
-			}
+            if (pos != count)
+                throw new ArgumentException("Unexpected elements in sequence", nameof(seq));
 		}
 
-		public virtual PkiStatusInfo[] GetStatus() => m_status.MapElements(PkiStatusInfo.GetInstance);
+        public virtual PkiStatusInfo[] GetStatus() => m_status.MapElements(PkiStatusInfo.GetInstance);
 
 		public virtual CertId[] GetRevCerts() => m_revCerts?.MapElements(CertId.GetInstance);
 
diff --git a/crypto/src/asn1/cmp/RootCaKeyUpdateContent.cs b/crypto/src/asn1/cmp/RootCaKeyUpdateContent.cs
index f00090f23..c9782bfd9 100644
--- a/crypto/src/asn1/cmp/RootCaKeyUpdateContent.cs
+++ b/crypto/src/asn1/cmp/RootCaKeyUpdateContent.cs
@@ -47,30 +47,16 @@ namespace Org.BouncyCastle.Asn1.Cmp
 
         private RootCaKeyUpdateContent(Asn1Sequence seq)
         {
-            if (seq.Count < 1 || seq.Count > 3)
-                throw new ArgumentException("expected sequence of 1 to 3 elements only");
+            int count = seq.Count, pos = 0;
+            if (count < 1 || count > 3)
+                throw new ArgumentException("Bad sequence size: " + count, nameof(seq));
 
-            CmpCertificate newWithNew = CmpCertificate.GetInstance(seq[0]);
-            CmpCertificate newWithOld = null;
-            CmpCertificate oldWithNew = null;
+            m_newWithNew = CmpCertificate.GetInstance(seq[pos++]);
+            m_newWithOld = Asn1Utilities.ReadOptionalContextTagged(seq, ref pos, 0, true, CmpCertificate.GetInstance);
+            m_oldWithNew = Asn1Utilities.ReadOptionalContextTagged(seq, ref pos, 1, true, CmpCertificate.GetInstance);
 
-            for (int pos = 1; pos < seq.Count; ++pos)
-            {
-                Asn1TaggedObject ato = Asn1TaggedObject.GetInstance(seq[pos]);
-
-                if (ato.HasContextTag(0))
-                {
-                    newWithOld = CmpCertificate.GetInstance(ato, true);
-                }
-                else if (ato.HasContextTag(1))
-                {
-                    oldWithNew = CmpCertificate.GetInstance(ato, true);
-                }
-            }
-
-            m_newWithNew = newWithNew;
-            m_newWithOld = newWithOld;
-            m_oldWithNew = oldWithNew;
+            if (pos != count)
+                throw new ArgumentException("Unexpected elements in sequence", nameof(seq));
         }
 
         public virtual CmpCertificate NewWithNew => m_newWithNew;