From cea0366cdf7450e96b663a2ee18ad37c2fdeb5cc Mon Sep 17 00:00:00 2001 From: Peter Dettman Date: Tue, 4 Jun 2024 23:44:11 +0700 Subject: Refactoring in Asn1.Crmf --- crypto/src/asn1/DerUTF8String.cs | 15 +++ crypto/src/asn1/crmf/AttributeTypeAndValue.cs | 60 +++++----- crypto/src/asn1/crmf/CertId.cs | 18 ++- crypto/src/asn1/crmf/CertReqMessages.cs | 41 +++---- crypto/src/asn1/crmf/CertReqMsg.cs | 35 +++--- crypto/src/asn1/crmf/CertRequest.cs | 78 ++++++------- crypto/src/asn1/crmf/CertTemplate.cs | 157 +++++++++----------------- crypto/src/asn1/crmf/Controls.cs | 49 ++++---- crypto/src/asn1/crmf/EncKeyWithID.cs | 90 +++++++-------- crypto/src/asn1/crmf/EncryptedKey.cs | 5 + crypto/src/asn1/crmf/EncryptedValue.cs | 11 +- crypto/src/asn1/crmf/OptionalValidity.cs | 63 ++++++----- crypto/src/asn1/crmf/PKIArchiveOptions.cs | 107 ++++++++++-------- crypto/src/asn1/crmf/PKIPublicationInfo.cs | 40 ++++--- crypto/src/asn1/crmf/PKMacValue.cs | 8 +- crypto/src/asn1/crmf/PopoPrivKey.cs | 94 ++++++++------- crypto/src/asn1/crmf/PopoSigningKey.cs | 27 +++-- crypto/src/asn1/crmf/PopoSigningKeyInput.cs | 18 ++- crypto/src/asn1/crmf/ProofOfPossession.cs | 99 +++++++++------- crypto/src/asn1/crmf/SinglePubInfo.cs | 54 +++++---- 20 files changed, 553 insertions(+), 516 deletions(-) diff --git a/crypto/src/asn1/DerUTF8String.cs b/crypto/src/asn1/DerUTF8String.cs index a9f35abda..965a8ebf8 100644 --- a/crypto/src/asn1/DerUTF8String.cs +++ b/crypto/src/asn1/DerUTF8String.cs @@ -69,6 +69,21 @@ namespace Org.BouncyCastle.Asn1 return (DerUtf8String)Meta.Instance.GetContextInstance(taggedObject, declaredExplicit); } + public static DerUtf8String GetOptional(Asn1Encodable element) + { + if (element == null) + throw new ArgumentNullException(nameof(element)); + if (element is DerUtf8String derUtf8String) + return derUtf8String; + if (element is IAsn1Convertible asn1Convertible && !(element is Asn1Object)) + { + Asn1Object asn1Object = asn1Convertible.ToAsn1Object(); + if (asn1Object is DerUtf8String converted) + return converted; + } + return null; + } + private readonly byte[] m_contents; public DerUtf8String(string str) diff --git a/crypto/src/asn1/crmf/AttributeTypeAndValue.cs b/crypto/src/asn1/crmf/AttributeTypeAndValue.cs index e7587896a..a9899d2f7 100644 --- a/crypto/src/asn1/crmf/AttributeTypeAndValue.cs +++ b/crypto/src/asn1/crmf/AttributeTypeAndValue.cs @@ -1,56 +1,51 @@ using System; -using Org.BouncyCastle.Utilities; - namespace Org.BouncyCastle.Asn1.Crmf { public class AttributeTypeAndValue : Asn1Encodable { - private readonly DerObjectIdentifier type; - private readonly Asn1Encodable value; - - private AttributeTypeAndValue(Asn1Sequence seq) + public static AttributeTypeAndValue GetInstance(object obj) { - type = (DerObjectIdentifier)seq[0]; - value = (Asn1Encodable)seq[1]; + if (obj == null) + return null; + if (obj is AttributeTypeAndValue attributeTypeAndValue) + return attributeTypeAndValue; + return new AttributeTypeAndValue(Asn1Sequence.GetInstance(obj)); } - public static AttributeTypeAndValue GetInstance(object obj) + public static AttributeTypeAndValue GetInstance(Asn1TaggedObject taggedObject, bool declaredExplicit) { - if (obj is AttributeTypeAndValue) - return (AttributeTypeAndValue)obj; + return new AttributeTypeAndValue(Asn1Sequence.GetInstance(taggedObject, declaredExplicit)); + } - if (obj is Asn1Sequence) - return new AttributeTypeAndValue((Asn1Sequence)obj); + private readonly DerObjectIdentifier m_type; + private readonly Asn1Encodable m_value; - throw new ArgumentException("Invalid object: " + Platform.GetTypeName(obj), "obj"); + private AttributeTypeAndValue(Asn1Sequence seq) + { + int count = seq.Count; + if (count != 2) + throw new ArgumentException("Bad sequence size: " + count, nameof(seq)); + + m_type = DerObjectIdentifier.GetInstance(seq[0]); + m_value = seq[1]; } - public AttributeTypeAndValue( - string oid, - Asn1Encodable value) + public AttributeTypeAndValue(string oid, Asn1Encodable value) : this(new DerObjectIdentifier(oid), value) { } - public AttributeTypeAndValue( - DerObjectIdentifier type, - Asn1Encodable value) + public AttributeTypeAndValue(DerObjectIdentifier type, Asn1Encodable value) { - this.type = type; - this.value = value; + m_type = type ?? throw new ArgumentNullException(nameof(type)); + m_value = value ?? throw new ArgumentNullException(nameof(value)); } - public virtual DerObjectIdentifier Type - { - get { return type; } - } + public virtual DerObjectIdentifier Type => m_type; - public virtual Asn1Encodable Value - { - get { return value; } - } + public virtual Asn1Encodable Value => m_value; /** * 
@@ -60,9 +55,6 @@ namespace Org.BouncyCastle.Asn1.Crmf
          *
* @return a basic ASN.1 object representation. */ - public override Asn1Object ToAsn1Object() - { - return new DerSequence(type, value); - } + public override Asn1Object ToAsn1Object() => new DerSequence(m_type, m_value); } } diff --git a/crypto/src/asn1/crmf/CertId.cs b/crypto/src/asn1/crmf/CertId.cs index c63c21ca8..c9f66b065 100644 --- a/crypto/src/asn1/crmf/CertId.cs +++ b/crypto/src/asn1/crmf/CertId.cs @@ -1,4 +1,6 @@ -using Org.BouncyCastle.Asn1.X509; +using System; + +using Org.BouncyCastle.Asn1.X509; namespace Org.BouncyCastle.Asn1.Crmf { @@ -9,8 +11,8 @@ namespace Org.BouncyCastle.Asn1.Crmf { if (obj == null) return null; - if (obj is CertId certID) - return certID; + if (obj is CertId certId) + return certId; return new CertId(Asn1Sequence.GetInstance(obj)); } @@ -24,10 +26,20 @@ namespace Org.BouncyCastle.Asn1.Crmf private CertId(Asn1Sequence seq) { + int count = seq.Count; + if (count != 2) + throw new ArgumentException("Bad sequence size: " + count, nameof(seq)); + m_issuer = GeneralName.GetInstance(seq[0]); m_serialNumber = DerInteger.GetInstance(seq[1]); } + public CertId(GeneralName issuer, DerInteger serialNumber) + { + m_issuer = issuer ?? throw new ArgumentNullException(nameof(issuer)); + m_serialNumber = serialNumber ?? throw new ArgumentNullException(nameof(serialNumber)); + } + public virtual GeneralName Issuer => m_issuer; public virtual DerInteger SerialNumber => m_serialNumber; diff --git a/crypto/src/asn1/crmf/CertReqMessages.cs b/crypto/src/asn1/crmf/CertReqMessages.cs index d49b90fe3..fc01fb99d 100644 --- a/crypto/src/asn1/crmf/CertReqMessages.cs +++ b/crypto/src/asn1/crmf/CertReqMessages.cs @@ -1,49 +1,42 @@ -using System; - -using Org.BouncyCastle.Utilities; - namespace Org.BouncyCastle.Asn1.Crmf { public class CertReqMessages : Asn1Encodable { - private readonly Asn1Sequence content; - - private CertReqMessages(Asn1Sequence seq) + public static CertReqMessages GetInstance(object obj) { - content = seq; + if (obj == null) + return null; + if (obj is CertReqMessages certReqMessages) + return certReqMessages; + return new CertReqMessages(Asn1Sequence.GetInstance(obj)); } - public static CertReqMessages GetInstance(object obj) + public static CertReqMessages GetInstance(Asn1TaggedObject taggedObject, bool declaredExplicit) { - if (obj is CertReqMessages) - return (CertReqMessages)obj; - - if (obj is Asn1Sequence) - return new CertReqMessages((Asn1Sequence)obj); - - throw new ArgumentException("Invalid object: " + Platform.GetTypeName(obj), "obj"); + return new CertReqMessages(Asn1Sequence.GetInstance(taggedObject, declaredExplicit)); } - public CertReqMessages(params CertReqMsg[] msgs) + private readonly Asn1Sequence m_content; + + private CertReqMessages(Asn1Sequence seq) { - content = new DerSequence(msgs); + m_content = seq; } - public virtual CertReqMsg[] ToCertReqMsgArray() + public CertReqMessages(params CertReqMsg[] msgs) { - return content.MapElements(CertReqMsg.GetInstance); + m_content = new DerSequence(msgs); } + public virtual CertReqMsg[] ToCertReqMsgArray() => m_content.MapElements(CertReqMsg.GetInstance); + /** * 
          * CertReqMessages ::= SEQUENCE SIZE (1..MAX) OF CertReqMsg
          *
* @return a basic ASN.1 object representation. */ - public override Asn1Object ToAsn1Object() - { - return content; - } + public override Asn1Object ToAsn1Object() => m_content; } } diff --git a/crypto/src/asn1/crmf/CertReqMsg.cs b/crypto/src/asn1/crmf/CertReqMsg.cs index 1832a34cc..5e56f68c9 100644 --- a/crypto/src/asn1/crmf/CertReqMsg.cs +++ b/crypto/src/asn1/crmf/CertReqMsg.cs @@ -25,21 +25,18 @@ namespace Org.BouncyCastle.Asn1.Crmf private CertReqMsg(Asn1Sequence seq) { - m_certReq = CertRequest.GetInstance(seq[0]); + int count = seq.Count; + if (count < 1 || count > 3) + throw new ArgumentException("Bad sequence size: " + count, nameof(seq)); - for (int pos = 1; pos < seq.Count; ++pos) - { - object o = seq[pos]; + int pos = 0; - if (o is Asn1TaggedObject || o is ProofOfPossession) - { - m_pop = ProofOfPossession.GetInstance(o); - } - else - { - m_regInfo = Asn1Sequence.GetInstance(o); - } - } + m_certReq = CertRequest.GetInstance(seq[pos++]); + m_pop = Asn1Utilities.ReadOptional(seq, ref pos, ProofOfPossession.GetOptional); + m_regInfo = Asn1Utilities.ReadOptional(seq, ref pos, Asn1Sequence.GetOptional); + + if (pos != count) + throw new ArgumentException("Unexpected elements in sequence", nameof(seq)); } /** @@ -50,13 +47,9 @@ namespace Org.BouncyCastle.Asn1.Crmf */ public CertReqMsg(CertRequest certReq, ProofOfPossession popo, AttributeTypeAndValue[] regInfo) { - this.m_certReq = certReq ?? throw new ArgumentNullException(nameof(certReq)); - this.m_pop = popo; - - if (regInfo != null) - { - this.m_regInfo = new DerSequence(regInfo); - } + m_certReq = certReq ?? throw new ArgumentNullException(nameof(certReq)); + m_pop = popo; + m_regInfo = regInfo == null ? null : new DerSequence(regInfo); } public virtual CertRequest CertReq => m_certReq; @@ -81,7 +74,7 @@ namespace Org.BouncyCastle.Asn1.Crmf */ public override Asn1Object ToAsn1Object() { - Asn1EncodableVector v = new Asn1EncodableVector(2); + Asn1EncodableVector v = new Asn1EncodableVector(3); v.Add(m_certReq); v.AddOptional(m_pop, m_regInfo); return new DerSequence(v); diff --git a/crypto/src/asn1/crmf/CertRequest.cs b/crypto/src/asn1/crmf/CertRequest.cs index bf6182f25..dea7ea23c 100644 --- a/crypto/src/asn1/crmf/CertRequest.cs +++ b/crypto/src/asn1/crmf/CertRequest.cs @@ -1,68 +1,61 @@ using System; -using Org.BouncyCastle.Crmf; namespace Org.BouncyCastle.Asn1.Crmf { public class CertRequest : Asn1Encodable { - private readonly DerInteger certReqId; - private readonly CertTemplate certTemplate; - private readonly Controls controls; + public static CertRequest GetInstance(object obj) + { + if (obj == null) + return null; + if (obj is CertRequest certRequest) + return certRequest; + return new CertRequest(Asn1Sequence.GetInstance(obj)); + } - private CertRequest(Asn1Sequence seq) + public static CertRequest GetInstance(Asn1TaggedObject taggedObject, bool declaredExplicit) { - certReqId = DerInteger.GetInstance(seq[0]); - certTemplate = CertTemplate.GetInstance(seq[1]); - if (seq.Count > 2) - { - controls = Controls.GetInstance(seq[2]); - } + return new CertRequest(Asn1Sequence.GetInstance(taggedObject, declaredExplicit)); } - public static CertRequest GetInstance(object obj) + private readonly DerInteger m_certReqId; + private readonly CertTemplate m_certTemplate; + private readonly Controls m_controls; + + private CertRequest(Asn1Sequence seq) { - if (obj is CertRequest) - return (CertRequest)obj; + int count = seq.Count; + if (count < 2 || count > 3) + throw new ArgumentException("Bad sequence size: " + count, nameof(seq)); - if (obj != null) - return new CertRequest(Asn1Sequence.GetInstance(obj)); + int pos = 0; - return null; + m_certReqId = DerInteger.GetInstance(seq[pos++]); + m_certTemplate = CertTemplate.GetInstance(seq[pos++]); + m_controls = Asn1Utilities.ReadOptional(seq, ref pos, Controls.GetOptional); + + if (pos != count) + throw new ArgumentException("Unexpected elements in sequence", nameof(seq)); } - public CertRequest( - int certReqId, - CertTemplate certTemplate, - Controls controls) + public CertRequest(int certReqId, CertTemplate certTemplate, Controls controls) : this(new DerInteger(certReqId), certTemplate, controls) { } - public CertRequest( - DerInteger certReqId, - CertTemplate certTemplate, - Controls controls) + public CertRequest(DerInteger certReqId, CertTemplate certTemplate, Controls controls) { - this.certReqId = certReqId; - this.certTemplate = certTemplate; - this.controls = controls; + m_certReqId = certReqId ?? throw new ArgumentNullException(nameof(certReqId)); + m_certTemplate = certTemplate ?? throw new ArgumentNullException(nameof(certTemplate)); + m_controls = controls; } - public virtual DerInteger CertReqID - { - get { return certReqId; } - } + public virtual DerInteger CertReqID => m_certReqId; - public virtual CertTemplate CertTemplate - { - get { return certTemplate; } - } + public virtual CertTemplate CertTemplate => m_certTemplate; - public virtual Controls Controls - { - get { return controls; } - } + public virtual Controls Controls => m_controls; /** * 
@@ -75,8 +68,9 @@ namespace Org.BouncyCastle.Asn1.Crmf
          */
         public override Asn1Object ToAsn1Object()
         {
-            Asn1EncodableVector v = new Asn1EncodableVector(certReqId, certTemplate);
-            v.AddOptional(controls);
+            Asn1EncodableVector v = new Asn1EncodableVector(3);
+            v.Add(m_certReqId, m_certTemplate);
+            v.AddOptional(m_controls);
             return new DerSequence(v);
         }
     }
diff --git a/crypto/src/asn1/crmf/CertTemplate.cs b/crypto/src/asn1/crmf/CertTemplate.cs
index f731ac12e..62227808c 100644
--- a/crypto/src/asn1/crmf/CertTemplate.cs
+++ b/crypto/src/asn1/crmf/CertTemplate.cs
@@ -7,123 +7,77 @@ namespace Org.BouncyCastle.Asn1.Crmf
     public class CertTemplate
         : Asn1Encodable
     {
-        private readonly Asn1Sequence seq;
-
-        private readonly DerInteger version;
-        private readonly DerInteger serialNumber;
-        private readonly AlgorithmIdentifier signingAlg;
-        private readonly X509Name issuer;
-        private readonly OptionalValidity validity;
-        private readonly X509Name subject;
-        private readonly SubjectPublicKeyInfo publicKey;
-        private readonly DerBitString issuerUID;
-        private readonly DerBitString subjectUID;
-        private readonly X509Extensions extensions;
-
-        private CertTemplate(Asn1Sequence seq)
+        public static CertTemplate GetInstance(object obj)
         {
-            this.seq = seq;
-
-            foreach (Asn1TaggedObject tObj in seq)
-            {
-                switch (tObj.TagNo)
-                {
-                case 0:
-                    version = DerInteger.GetInstance(tObj, false);
-                    break;
-                case 1:
-                    serialNumber = DerInteger.GetInstance(tObj, false);
-                    break;
-                case 2:
-                    signingAlg = AlgorithmIdentifier.GetInstance(tObj, false);
-                    break;
-                case 3:
-                    issuer = X509Name.GetInstance(tObj, true); // CHOICE
-                    break;
-                case 4:
-                    validity = OptionalValidity.GetInstance(Asn1Sequence.GetInstance(tObj, false));
-                    break;
-                case 5:
-                    subject = X509Name.GetInstance(tObj, true); // CHOICE
-                    break;
-                case 6:
-                    publicKey = SubjectPublicKeyInfo.GetInstance(tObj, false);
-                    break;
-                case 7:
-                    issuerUID = DerBitString.GetInstance(tObj, false);
-                    break;
-                case 8:
-                    subjectUID = DerBitString.GetInstance(tObj, false);
-                    break;
-                case 9:
-                    extensions = X509Extensions.GetInstance(tObj, false);
-                    break;
-                default:
-                    throw new ArgumentException("unknown tag: " + tObj.TagNo, "seq");
-                }
-            }
+            if (obj == null)
+                return null;
+            if (obj is CertTemplate certTemplate)
+                return certTemplate;
+            return new CertTemplate(Asn1Sequence.GetInstance(obj));
         }
 
-        public static CertTemplate GetInstance(object obj)
+        public static CertTemplate GetInstance(Asn1TaggedObject taggedObject, bool declaredExplicit)
         {
-            if (obj is CertTemplate)
-                return (CertTemplate)obj;
+            return new CertTemplate(Asn1Sequence.GetInstance(taggedObject, declaredExplicit));
+        }
 
-            if (obj != null)
-                return new CertTemplate(Asn1Sequence.GetInstance(obj));
+        private readonly Asn1Sequence m_seq;
 
-            return null;
-        }
+        private readonly DerInteger m_version;
+        private readonly DerInteger m_serialNumber;
+        private readonly AlgorithmIdentifier m_signingAlg;
+        private readonly X509Name m_issuer;
+        private readonly OptionalValidity m_validity;
+        private readonly X509Name m_subject;
+        private readonly SubjectPublicKeyInfo m_publicKey;
+        private readonly DerBitString m_issuerUID;
+        private readonly DerBitString m_subjectUID;
+        private readonly X509Extensions m_extensions;
 
-        public virtual int Version
+        private CertTemplate(Asn1Sequence seq)
         {
-            get { return version.IntValueExact; }
+            int count = seq.Count;
+            if (count < 0 || count > 10)
+                throw new ArgumentException("Bad sequence size: " + count, nameof(seq));
+
+            int pos = 0;
+
+            m_version = Asn1Utilities.ReadOptionalContextTagged(seq, ref pos, 0, false, DerInteger.GetInstance);
+            m_serialNumber = Asn1Utilities.ReadOptionalContextTagged(seq, ref pos, 1, false, DerInteger.GetInstance);
+            m_signingAlg = Asn1Utilities.ReadOptionalContextTagged(seq, ref pos, 2, false, AlgorithmIdentifier.GetInstance);
+            m_issuer = Asn1Utilities.ReadOptionalContextTagged(seq, ref pos, 3, true, X509Name.GetInstance); // CHOICE Name
+            m_validity = Asn1Utilities.ReadOptionalContextTagged(seq, ref pos, 4, false, OptionalValidity.GetInstance);
+            m_subject = Asn1Utilities.ReadOptionalContextTagged(seq, ref pos, 5, true, X509Name.GetInstance); // CHOICE Name
+            m_publicKey = Asn1Utilities.ReadOptionalContextTagged(seq, ref pos, 6, false, SubjectPublicKeyInfo.GetInstance);
+            m_issuerUID = Asn1Utilities.ReadOptionalContextTagged(seq, ref pos, 7, false, DerBitString.GetInstance);
+            m_subjectUID = Asn1Utilities.ReadOptionalContextTagged(seq, ref pos, 8, false, DerBitString.GetInstance);
+            m_extensions = Asn1Utilities.ReadOptionalContextTagged(seq, ref pos, 9, false, X509Extensions.GetInstance);
+
+            if (pos != count)
+                throw new ArgumentException("Unexpected elements in sequence", nameof(seq));
+
+            m_seq = seq;
         }
 
-        public virtual DerInteger SerialNumber
-        {
-            get { return serialNumber; }
-        }
+        public virtual int Version => m_version.IntValueExact;
 
-        public virtual AlgorithmIdentifier SigningAlg
-        {
-            get { return signingAlg; }
-        }
+        public virtual DerInteger SerialNumber => m_serialNumber;
 
-        public virtual X509Name Issuer
-        {
-            get { return issuer; }
-        }
+        public virtual AlgorithmIdentifier SigningAlg => m_signingAlg;
 
-        public virtual OptionalValidity Validity
-        {
-            get { return validity; }
-        }
+        public virtual X509Name Issuer => m_issuer;
 
-        public virtual X509Name Subject
-        {
-            get { return subject; }
-        }
+        public virtual OptionalValidity Validity => m_validity;
 
-        public virtual SubjectPublicKeyInfo PublicKey
-        {
-            get { return publicKey; }
-        }
+        public virtual X509Name Subject => m_subject;
 
-        public virtual DerBitString IssuerUID
-        {
-            get { return issuerUID; }
-        }
+        public virtual SubjectPublicKeyInfo PublicKey => m_publicKey;
 
-        public virtual DerBitString SubjectUID
-        {
-            get { return subjectUID; }
-        }
+        public virtual DerBitString IssuerUID => m_issuerUID;
 
-        public virtual X509Extensions Extensions
-        {
-            get { return extensions; }
-        }
+        public virtual DerBitString SubjectUID => m_subjectUID;
+
+        public virtual X509Extensions Extensions => m_extensions;
 
         /**
          * 
@@ -141,9 +95,6 @@ namespace Org.BouncyCastle.Asn1.Crmf
          * 

          * @return a basic ASN.1 object representation.
          */
-        public override Asn1Object ToAsn1Object()
-        {
-            return seq;
-        }
+        public override Asn1Object ToAsn1Object() => m_seq;
     }
 }
diff --git a/crypto/src/asn1/crmf/Controls.cs b/crypto/src/asn1/crmf/Controls.cs
index ac568d741..12954f5e8 100644
--- a/crypto/src/asn1/crmf/Controls.cs
+++ b/crypto/src/asn1/crmf/Controls.cs
@@ -1,40 +1,50 @@
 using System;
-using System.Text;
-
-using Org.BouncyCastle.Utilities;
 
 namespace Org.BouncyCastle.Asn1.Crmf
 {
     public class Controls
         : Asn1Encodable
     {
-        private readonly Asn1Sequence content;
+        public static Controls GetInstance(object obj)
+        {
+            if (obj == null)
+                return null;
+            if (obj is Controls controls)
+                return controls;
+            return new Controls(Asn1Sequence.GetInstance(obj));
+        }
 
-        private Controls(Asn1Sequence seq)
+        public static Controls GetInstance(Asn1TaggedObject taggedObject, bool declaredExplicit)
         {
-            content = seq;
+            return new Controls(Asn1Sequence.GetInstance(taggedObject, declaredExplicit));
         }
 
-        public static Controls GetInstance(object obj)
+        public static Controls GetOptional(Asn1Encodable element)
         {
-            if (obj is Controls)
-                return (Controls)obj;
+            if (element == null)
+                throw new ArgumentNullException(nameof(element));
+            if (element is Controls controls)
+                return controls;
+            Asn1Sequence asn1Sequence = Asn1Sequence.GetOptional(element);
+            if (asn1Sequence != null)
+                return new Controls(asn1Sequence);
+            return null;
+        }
 
-            if (obj is Asn1Sequence)
-                return new Controls((Asn1Sequence)obj);
+        private readonly Asn1Sequence m_content;
 
-            throw new ArgumentException("Invalid object: " + Platform.GetTypeName(obj), "obj");
+        private Controls(Asn1Sequence seq)
+        {
+            m_content = seq;
         }
 
         public Controls(params AttributeTypeAndValue[] atvs)
         {
-            content = new DerSequence(atvs);
+            m_content = new DerSequence(atvs);
         }
 
-        public virtual AttributeTypeAndValue[] ToAttributeTypeAndValueArray()
-        {
-            return content.MapElements(AttributeTypeAndValue.GetInstance);
-        }
+        public virtual AttributeTypeAndValue[] ToAttributeTypeAndValueArray() =>
+            m_content.MapElements(AttributeTypeAndValue.GetInstance);
 
         /**
          * 
@@ -42,9 +52,6 @@ namespace Org.BouncyCastle.Asn1.Crmf
          * 

          * @return a basic ASN.1 object representation.
          */
-        public override Asn1Object ToAsn1Object()
-        {
-            return content;
-        }
+        public override Asn1Object ToAsn1Object() => m_content;
     }
 }
diff --git a/crypto/src/asn1/crmf/EncKeyWithID.cs b/crypto/src/asn1/crmf/EncKeyWithID.cs
index 67c16605a..272c38949 100644
--- a/crypto/src/asn1/crmf/EncKeyWithID.cs
+++ b/crypto/src/asn1/crmf/EncKeyWithID.cs
@@ -8,78 +8,74 @@ namespace Org.BouncyCastle.Asn1.Crmf
     public class EncKeyWithID
         : Asn1Encodable
     {
-        private readonly PrivateKeyInfo privKeyInfo;
-        private readonly Asn1Encodable identifier;
-
         public static EncKeyWithID GetInstance(object obj)
         {
-            if (obj is EncKeyWithID)
-                return (EncKeyWithID)obj;
+            if (obj == null)
+                return null;
+            if (obj is EncKeyWithID encKeyWithID)
+                return encKeyWithID;
+            return new EncKeyWithID(Asn1Sequence.GetInstance(obj));
+        }
+
+        public static EncKeyWithID GetInstance(Asn1TaggedObject taggedObject, bool declaredExplicit)
+        {
+            return new EncKeyWithID(Asn1Sequence.GetInstance(taggedObject, declaredExplicit));
+        }
 
-            if (obj != null)
-                return new EncKeyWithID(Asn1Sequence.GetInstance(obj));
+        private static Asn1Encodable GetOptionalChoice(Asn1Encodable element)
+        {
+            var _string = DerUtf8String.GetOptional(element);
+            if (_string != null)
+                return _string;
 
-            return null;
+            return GeneralName.GetInstance(element);
         }
 
+        private readonly PrivateKeyInfo m_privKeyInfo;
+        private readonly Asn1Encodable m_identifier;
+
         private EncKeyWithID(Asn1Sequence seq)
         {
-            this.privKeyInfo = PrivateKeyInfo.GetInstance(seq[0]);
+            int count = seq.Count;
+            if (count < 1 || count > 2)
+                throw new ArgumentException("Bad sequence size: " + count, nameof(seq));
 
-            if (seq.Count > 1)
-            {
-                if (!(seq[1] is DerUtf8String))
-                {
-                    this.identifier = GeneralName.GetInstance(seq[1]);
-                }
-                else
-                {
-                    this.identifier = seq[1];
-                }
-            }
-            else
+            m_privKeyInfo = PrivateKeyInfo.GetInstance(seq[0]);
+
+            if (count > 1)
             {
-                this.identifier = null;
+                m_identifier = GetOptionalChoice(seq[1]);
             }
         }
 
+        private EncKeyWithID(PrivateKeyInfo privKeyInfo, Asn1Encodable identifier)
+        {
+            m_privKeyInfo = privKeyInfo ?? throw new ArgumentNullException(nameof(privKeyInfo));
+            m_identifier = identifier;
+        }
+
         public EncKeyWithID(PrivateKeyInfo privKeyInfo)
+            : this(privKeyInfo, (Asn1Encodable)null)
         {
-            this.privKeyInfo = privKeyInfo;
-            this.identifier = null;
         }
 
         public EncKeyWithID(PrivateKeyInfo privKeyInfo, DerUtf8String str)
+            : this(privKeyInfo, (Asn1Encodable)str)
         {
-            this.privKeyInfo = privKeyInfo;
-            this.identifier = str;
         }
 
         public EncKeyWithID(PrivateKeyInfo privKeyInfo, GeneralName generalName)
+            : this(privKeyInfo, (Asn1Encodable)generalName)
         {
-            this.privKeyInfo = privKeyInfo;
-            this.identifier = generalName;
         }
 
-        public virtual PrivateKeyInfo PrivateKey
-        {
-            get { return privKeyInfo; }
-        }
+        public virtual PrivateKeyInfo PrivateKey => m_privKeyInfo;
 
-        public virtual bool HasIdentifier
-        {
-            get { return identifier != null; }
-        }
+        public virtual bool HasIdentifier => m_identifier != null;
 
-        public virtual bool IsIdentifierUtf8String
-        {
-            get { return identifier is DerUtf8String; }
-        }
+        public virtual bool IsIdentifierUtf8String => m_identifier is DerUtf8String;
 
-        public virtual Asn1Encodable Identifier
-        {
-            get { return identifier; }
-        }
+        public virtual Asn1Encodable Identifier => m_identifier;
 
         /**
          * 
@@ -95,9 +91,9 @@ namespace Org.BouncyCastle.Asn1.Crmf
          */
         public override Asn1Object ToAsn1Object()
         {
-            Asn1EncodableVector v = new Asn1EncodableVector(privKeyInfo);
-            v.AddOptional(identifier);
-            return new DerSequence(v);
+            return m_identifier == null
+                ?  new DerSequence(m_privKeyInfo)
+                :  new DerSequence(m_privKeyInfo, m_identifier);
         }
     }
 }
diff --git a/crypto/src/asn1/crmf/EncryptedKey.cs b/crypto/src/asn1/crmf/EncryptedKey.cs
index d4ff250c5..62c475f5d 100644
--- a/crypto/src/asn1/crmf/EncryptedKey.cs
+++ b/crypto/src/asn1/crmf/EncryptedKey.cs
@@ -16,6 +16,11 @@ namespace Org.BouncyCastle.Asn1.Crmf
             return new EncryptedKey(EncryptedValue.GetInstance(obj));
         }
 
+        public static EncryptedKey GetInstance(Asn1TaggedObject taggedObject, bool declaredExplicit)
+        {
+            return Asn1Utilities.GetInstanceFromChoice(taggedObject, declaredExplicit, GetInstance);
+        }
+
         private readonly EnvelopedData m_envelopedData;
         private readonly EncryptedValue m_encryptedValue;
 
diff --git a/crypto/src/asn1/crmf/EncryptedValue.cs b/crypto/src/asn1/crmf/EncryptedValue.cs
index 05a96fd37..756c20bd0 100644
--- a/crypto/src/asn1/crmf/EncryptedValue.cs
+++ b/crypto/src/asn1/crmf/EncryptedValue.cs
@@ -9,13 +9,16 @@ namespace Org.BouncyCastle.Asn1.Crmf
     {
         public static EncryptedValue GetInstance(object obj)
         {
+            if (obj == null)
+                return null;
             if (obj is EncryptedValue encryptedValue)
                 return encryptedValue;
+            return new EncryptedValue(Asn1Sequence.GetInstance(obj));
+        }
 
-            if (obj != null)
-                return new EncryptedValue(Asn1Sequence.GetInstance(obj));
-
-            return null;
+        public static EncryptedValue GetInstance(Asn1TaggedObject taggedObject, bool declaredExplicit)
+        {
+            return new EncryptedValue(Asn1Sequence.GetInstance(taggedObject, declaredExplicit));
         }
 
         private readonly AlgorithmIdentifier m_intendedAlg;
diff --git a/crypto/src/asn1/crmf/OptionalValidity.cs b/crypto/src/asn1/crmf/OptionalValidity.cs
index 722705232..290d1905e 100644
--- a/crypto/src/asn1/crmf/OptionalValidity.cs
+++ b/crypto/src/asn1/crmf/OptionalValidity.cs
@@ -7,48 +7,51 @@ namespace Org.BouncyCastle.Asn1.Crmf
     public class OptionalValidity
         : Asn1Encodable
     {
-        private readonly Time notBefore;
-        private readonly Time notAfter;
-
-        private OptionalValidity(Asn1Sequence seq)
-        {
-            foreach (Asn1TaggedObject tObj in seq)
-            {
-                if (tObj.TagNo == 0)
-                {
-                    notBefore = Time.GetInstance(tObj, true);
-                }
-                else
-                {
-                    notAfter = Time.GetInstance(tObj, true);
-                }
-            }
-        }
-
         public static OptionalValidity GetInstance(object obj)
         {
-            if (obj == null || obj is OptionalValidity)
-                return (OptionalValidity)obj;
-
+            if (obj == null)
+                return null;
+            if (obj is OptionalValidity optionalValidity)
+                return optionalValidity;
             return new OptionalValidity(Asn1Sequence.GetInstance(obj));
         }
 
-        public OptionalValidity(Time notBefore, Time notAfter)
+        public static OptionalValidity GetInstance(Asn1TaggedObject taggedObject, bool declaredExplicit)
         {
-            this.notBefore = notBefore;
-            this.notAfter = notAfter;
+            return new OptionalValidity(Asn1Sequence.GetInstance(taggedObject, declaredExplicit));
         }
 
-        public virtual Time NotBefore
+        private readonly Time m_notBefore;
+        private readonly Time m_notAfter;
+
+        private OptionalValidity(Asn1Sequence seq)
         {
-            get { return notBefore; }
+            // TODO Enforce "at least one MUST be present"?
+
+            int count = seq.Count;
+            if (count < 0 || count > 2)
+                throw new ArgumentException("Bad sequence size: " + count, nameof(seq));
+
+            int pos = 0;
+
+            m_notBefore = Asn1Utilities.ReadOptionalContextTagged(seq, ref pos, 0, true, Time.GetInstance);
+            m_notAfter = Asn1Utilities.ReadOptionalContextTagged(seq, ref pos, 1, true, Time.GetInstance);
+
+            if (pos != count)
+                throw new ArgumentException("Unexpected elements in sequence", nameof(seq));
         }
 
-        public virtual Time NotAfter
+        public OptionalValidity(Time notBefore, Time notAfter)
         {
-            get { return notAfter; }
+            // TODO Enforce "at least one MUST be present"?
+            m_notBefore = notBefore;
+            m_notAfter = notAfter;
         }
 
+        public virtual Time NotBefore => m_notBefore;
+
+        public virtual Time NotAfter => m_notAfter;
+
         /**
          * 
          * OptionalValidity ::= SEQUENCE {
@@ -60,8 +63,8 @@ namespace Org.BouncyCastle.Asn1.Crmf
         public override Asn1Object ToAsn1Object()
         {
             Asn1EncodableVector v = new Asn1EncodableVector(2);
-            v.AddOptionalTagged(true, 0, notBefore);
-            v.AddOptionalTagged(true, 1, notAfter);
+            v.AddOptionalTagged(true, 0, m_notBefore);
+            v.AddOptionalTagged(true, 1, m_notAfter);
             return new DerSequence(v);
         }
     }
diff --git a/crypto/src/asn1/crmf/PKIArchiveOptions.cs b/crypto/src/asn1/crmf/PKIArchiveOptions.cs
index 85815c5db..7f756b39a 100644
--- a/crypto/src/asn1/crmf/PKIArchiveOptions.cs
+++ b/crypto/src/asn1/crmf/PKIArchiveOptions.cs
@@ -11,71 +11,87 @@ namespace Org.BouncyCastle.Asn1.Crmf
         public const int keyGenParameters = 1;
         public const int archiveRemGenPrivKey = 2;
 
-        private readonly Asn1Encodable value;
-
         public static PkiArchiveOptions GetInstance(object obj)
         {
-            if (obj is PkiArchiveOptions pkiArchiveOptions)
-                return pkiArchiveOptions;
+            if (obj == null)
+                return null;
+
+            if (obj is Asn1Encodable element)
+            {
+                var result = GetOptional(element);
+                if (result != null)
+                    return result;
+            }
 
-            if (obj is Asn1TaggedObject taggedObject)
-                return new PkiArchiveOptions(Asn1Utilities.CheckContextTagClass(taggedObject));
+            throw new ArgumentException("Invalid object: " + Platform.GetTypeName(obj), nameof(obj));
+        }
 
-            throw new ArgumentException("Invalid object: " + Platform.GetTypeName(obj), "obj");
+        public static PkiArchiveOptions GetInstance(Asn1TaggedObject taggedObject, bool declaredExplicit)
+        {
+            return Asn1Utilities.GetInstanceFromChoice(taggedObject, declaredExplicit, GetInstance);
         }
 
-        private PkiArchiveOptions(Asn1TaggedObject tagged)
+        public static PkiArchiveOptions GetOptional(Asn1Encodable element)
         {
-            switch (tagged.TagNo)
+            if (element == null)
+                throw new ArgumentNullException(nameof(element));
+            if (element is PkiArchiveOptions pkiArchiveOptions)
+                return pkiArchiveOptions;
+            if (element is Asn1TaggedObject taggedObject)
             {
-            case encryptedPrivKey:
-                value = EncryptedKey.GetInstance(tagged.GetExplicitBaseObject());
-                break;
-            case keyGenParameters:
-                value = Asn1OctetString.GetInstance(tagged, false);
-                break;
-            case archiveRemGenPrivKey:
-                value = DerBoolean.GetInstance(tagged, false);
-                break;
-            default:
-                throw new ArgumentException("unknown tag number: " + tagged.TagNo, "tagged");
+                Asn1Encodable baseObject = GetOptionalBaseObject(taggedObject);
+                if (baseObject != null)
+                    return new PkiArchiveOptions(taggedObject.TagNo, baseObject);
             }
+            return null;
         }
 
-        public PkiArchiveOptions(EncryptedKey encKey)
+        private static Asn1Encodable GetOptionalBaseObject(Asn1TaggedObject taggedObject)
         {
-            this.value = encKey;
+            if (taggedObject.HasContextTag())
+            {
+                switch (taggedObject.TagNo)
+                {
+                case encryptedPrivKey:
+                    // CHOICE so explicit
+                    return EncryptedKey.GetInstance(taggedObject, true);
+                case keyGenParameters:
+                    return Asn1OctetString.GetInstance(taggedObject, false);
+                case archiveRemGenPrivKey:
+                    return DerBoolean.GetInstance(taggedObject, false);
+                }
+            }
+            return null;
         }
 
-        public PkiArchiveOptions(Asn1OctetString keyGenParameters)
+        private readonly int m_tagNo;
+        private readonly Asn1Encodable m_obj;
+
+        private PkiArchiveOptions(int tagNo, Asn1Encodable obj)
         {
-            this.value = keyGenParameters;
+            m_tagNo = tagNo;
+            m_obj = obj ?? throw new ArgumentNullException(nameof(obj));
         }
 
-        public PkiArchiveOptions(bool archiveRemGenPrivKey)
+        public PkiArchiveOptions(EncryptedKey encKey)
+            : this(encryptedPrivKey, encKey)
         {
-            this.value = DerBoolean.GetInstance(archiveRemGenPrivKey);
         }
 
-        public virtual int Type
+        public PkiArchiveOptions(Asn1OctetString keyGenParameters)
+            : this(PkiArchiveOptions.keyGenParameters, keyGenParameters)
         {
-            get
-            {
-                if (value is EncryptedKey)
-                    return encryptedPrivKey;
-
-                if (value is Asn1OctetString)
-                    return keyGenParameters;
-
-                return archiveRemGenPrivKey;
-            }
         }
 
-        public virtual Asn1Encodable Value
+        public PkiArchiveOptions(bool archiveRemGenPrivKey)
+            : this(PkiArchiveOptions.archiveRemGenPrivKey, DerBoolean.GetInstance(archiveRemGenPrivKey))
         {
-            get { return value; }
         }
 
+        public virtual int Type => m_tagNo;
+
+        public virtual Asn1Encodable Value => m_obj;
+
         /**
          * 
          *  PkiArchiveOptions ::= CHOICE {
@@ -91,17 +107,8 @@ namespace Org.BouncyCastle.Asn1.Crmf
          */
         public override Asn1Object ToAsn1Object()
         {
-            if (value is EncryptedKey)
-            {
-                return new DerTaggedObject(true, encryptedPrivKey, value);  // choice
-            }
-
-            if (value is Asn1OctetString)
-            {
-                return new DerTaggedObject(false, keyGenParameters, value);
-            }
-
-            return new DerTaggedObject(false, archiveRemGenPrivKey, value);
+            // NOTE: Explicit tagging automatically applied for EncryptedKey (a CHOICE)
+            return new DerTaggedObject(false, m_tagNo, m_obj);
         }
     }
 }
diff --git a/crypto/src/asn1/crmf/PKIPublicationInfo.cs b/crypto/src/asn1/crmf/PKIPublicationInfo.cs
index aac6d9911..5cc1df013 100644
--- a/crypto/src/asn1/crmf/PKIPublicationInfo.cs
+++ b/crypto/src/asn1/crmf/PKIPublicationInfo.cs
@@ -1,4 +1,6 @@
-using Org.BouncyCastle.Math;
+using System;
+
+using Org.BouncyCastle.Math;
 
 namespace Org.BouncyCastle.Asn1.Crmf
 {
@@ -22,13 +24,16 @@ namespace Org.BouncyCastle.Asn1.Crmf
 
         public static PkiPublicationInfo GetInstance(object obj)
         {
+            if (obj == null)
+                return null;
             if (obj is PkiPublicationInfo pkiPublicationInfo)
                 return pkiPublicationInfo;
+            return new PkiPublicationInfo(Asn1Sequence.GetInstance(obj));
+        }
 
-            if (obj != null)
-                return new PkiPublicationInfo(Asn1Sequence.GetInstance(obj));
-
-            return null;
+        public static PkiPublicationInfo GetInstance(Asn1TaggedObject taggedObject, bool declaredExplicit)
+        {
+            return new PkiPublicationInfo(Asn1Sequence.GetInstance(taggedObject, declaredExplicit));
         }
 
         private readonly DerInteger m_action;
@@ -36,11 +41,17 @@ namespace Org.BouncyCastle.Asn1.Crmf
 
         private PkiPublicationInfo(Asn1Sequence seq)
         {
-            m_action = DerInteger.GetInstance(seq[0]);
-            if (seq.Count > 1)
-            {
-                m_pubInfos = Asn1Sequence.GetInstance(seq[1]);
-            }
+            int count = seq.Count;
+            if (count < 1 || count > 2)
+                throw new ArgumentException("Bad sequence size: " + count, nameof(seq));
+
+            int pos = 0;
+
+            m_action = DerInteger.GetInstance(seq[pos++]);
+            m_pubInfos = Asn1Utilities.ReadOptional(seq, ref pos, Asn1Sequence.GetOptional);
+
+            if (pos != count)
+                throw new ArgumentException("Unexpected elements in sequence", nameof(seq));
         }
 
         public PkiPublicationInfo(BigInteger action)
@@ -50,7 +61,7 @@ namespace Org.BouncyCastle.Asn1.Crmf
 
         public PkiPublicationInfo(DerInteger action)
         {
-            m_action = action;
+            m_action = action ?? throw new ArgumentNullException(nameof(action));
         }
 
         /**
@@ -97,10 +108,9 @@ namespace Org.BouncyCastle.Asn1.Crmf
          */
         public override Asn1Object ToAsn1Object()
         {
-            if (m_pubInfos == null)
-                return new DerSequence(m_action);
-
-            return new DerSequence(m_action, m_pubInfos);
+            return m_pubInfos == null
+                ?  new DerSequence(m_action)
+                :  new DerSequence(m_action, m_pubInfos);
         }
     }
 }
diff --git a/crypto/src/asn1/crmf/PKMacValue.cs b/crypto/src/asn1/crmf/PKMacValue.cs
index 67e5ce6cc..02aeb0547 100644
--- a/crypto/src/asn1/crmf/PKMacValue.cs
+++ b/crypto/src/asn1/crmf/PKMacValue.cs
@@ -1,4 +1,6 @@
-using Org.BouncyCastle.Asn1.Cmp;
+using System;
+
+using Org.BouncyCastle.Asn1.Cmp;
 using Org.BouncyCastle.Asn1.X509;
 
 namespace Org.BouncyCastle.Asn1.Crmf
@@ -28,6 +30,10 @@ namespace Org.BouncyCastle.Asn1.Crmf
 
         private PKMacValue(Asn1Sequence seq)
         {
+            int count = seq.Count;
+            if (count != 2)
+                throw new ArgumentException("Bad sequence size: " + count, nameof(seq));
+
             m_algID = AlgorithmIdentifier.GetInstance(seq[0]);
             m_macValue = DerBitString.GetInstance(seq[1]);
         }
diff --git a/crypto/src/asn1/crmf/PopoPrivKey.cs b/crypto/src/asn1/crmf/PopoPrivKey.cs
index 6e2d695da..13359d5bf 100644
--- a/crypto/src/asn1/crmf/PopoPrivKey.cs
+++ b/crypto/src/asn1/crmf/PopoPrivKey.cs
@@ -1,6 +1,7 @@
 using System;
 
 using Org.BouncyCastle.Asn1.Cms;
+using Org.BouncyCastle.Utilities;
 
 namespace Org.BouncyCastle.Asn1.Crmf
 {
@@ -17,9 +18,15 @@ namespace Org.BouncyCastle.Asn1.Crmf
         {
             if (obj == null)
                 return null;
-            if (obj is PopoPrivKey popoPrivKey)
-                return popoPrivKey;
-            return new PopoPrivKey(Asn1TaggedObject.GetInstance(obj, Asn1Tags.ContextSpecific));
+
+            if (obj is Asn1Encodable element)
+            {
+                var result = GetOptional(element);
+                if (result != null)
+                    return result;
+            }
+
+            throw new ArgumentException("Invalid object: " + Platform.GetTypeName(obj), nameof(obj));
         }
 
         public static PopoPrivKey GetInstance(Asn1TaggedObject tagged, bool isExplicit)
@@ -27,57 +34,65 @@ namespace Org.BouncyCastle.Asn1.Crmf
             return Asn1Utilities.GetInstanceFromChoice(tagged, isExplicit, GetInstance);
         }
 
-        private readonly int tagNo;
-        private readonly Asn1Encodable obj;
-
-        private PopoPrivKey(Asn1TaggedObject obj)
+        public static PopoPrivKey GetOptional(Asn1Encodable element)
         {
-            this.tagNo = obj.TagNo;
-
-            switch (tagNo)
+            if (element == null)
+                throw new ArgumentNullException(nameof(element));
+            if (element is PopoPrivKey popoPrivKey)
+                return popoPrivKey;
+            if (element is Asn1TaggedObject taggedObject)
             {
-            case thisMessage:
-                this.obj = DerBitString.GetInstance(obj, false);
-                break;
-            case subsequentMessage:
-                this.obj = SubsequentMessage.ValueOf(DerInteger.GetInstance(obj, false).IntValueExact);
-                break;
-            case dhMAC:
-                this.obj = DerBitString.GetInstance(obj, false);
-                break;
-            case agreeMAC:
-                this.obj = PKMacValue.GetInstance(obj, false);
-                break;
-            case encryptedKey:
-                this.obj = EnvelopedData.GetInstance(obj, false);
-                break;
-            default:
-                throw new ArgumentException("unknown tag in PopoPrivKey", "obj");
+                Asn1Encodable baseObject = GetOptionalBaseObject(taggedObject);
+                if (baseObject != null)
+                    return new PopoPrivKey(taggedObject.TagNo, baseObject);
             }
+            return null;
         }
 
-        public PopoPrivKey(PKMacValue pkMacValue)
+        private static Asn1Encodable GetOptionalBaseObject(Asn1TaggedObject taggedObject)
         {
-            this.tagNo = agreeMAC;
-            this.obj = pkMacValue;
+            if (taggedObject.HasContextTag())
+            {
+                switch (taggedObject.TagNo)
+                {
+                case thisMessage:
+                case dhMAC:
+                    return DerBitString.GetInstance(taggedObject, false);
+                case subsequentMessage:
+                    return SubsequentMessage.ValueOf(DerInteger.GetInstance(taggedObject, false).IntValueExact);
+                case agreeMAC:
+                    return PKMacValue.GetInstance(taggedObject, false);
+                case encryptedKey:
+                    return EnvelopedData.GetInstance(taggedObject, false);
+
+                }
+            }
+            return null;
         }
 
-        public PopoPrivKey(SubsequentMessage msg)
+        private readonly int m_tagNo;
+        private readonly Asn1Encodable m_obj;
+
+        private PopoPrivKey(int tagNo, Asn1Encodable obj)
         {
-            this.tagNo = subsequentMessage;
-            this.obj = msg;
+            m_tagNo = tagNo;
+            m_obj = obj ?? throw new ArgumentNullException(nameof(obj));
         }
 
-        public virtual int Type
+        public PopoPrivKey(PKMacValue pkMacValue)
+            : this(agreeMAC, pkMacValue)
         {
-            get { return tagNo; }
         }
 
-        public virtual Asn1Encodable Value
+        public PopoPrivKey(SubsequentMessage msg)
+            : this(subsequentMessage, msg)
         {
-            get { return obj; }
         }
 
+        public virtual int Type => m_tagNo;
+
+        public virtual Asn1Encodable Value => m_obj;
+
         /**
          * 
          * PopoPrivKey ::= CHOICE {
@@ -91,9 +106,6 @@ namespace Org.BouncyCastle.Asn1.Crmf
          *        encryptedKey      [4] EnvelopedData }
          * 

          */
-        public override Asn1Object ToAsn1Object()
-        {
-            return new DerTaggedObject(false, tagNo, obj);
-        }
+        public override Asn1Object ToAsn1Object() => new DerTaggedObject(false, m_tagNo, m_obj);
     }
 }
diff --git a/crypto/src/asn1/crmf/PopoSigningKey.cs b/crypto/src/asn1/crmf/PopoSigningKey.cs
index 2d30e1a67..00138ddb3 100644
--- a/crypto/src/asn1/crmf/PopoSigningKey.cs
+++ b/crypto/src/asn1/crmf/PopoSigningKey.cs
@@ -1,4 +1,6 @@
-using Org.BouncyCastle.Asn1.X509;
+using System;
+
+using Org.BouncyCastle.Asn1.X509;
 
 namespace Org.BouncyCastle.Asn1.Crmf
 {
@@ -25,17 +27,18 @@ namespace Org.BouncyCastle.Asn1.Crmf
 
         private PopoSigningKey(Asn1Sequence seq)
         {
-            int index = 0;
+            int count = seq.Count;
+            if (count < 2 || count > 3)
+                throw new ArgumentException("Bad sequence size: " + count, nameof(seq));
+
+            int pos = 0;
 
-            if (seq[index] is Asn1TaggedObject tagObj)
-            {
-                index++;
+            m_poposkInput = Asn1Utilities.ReadOptionalContextTagged(seq, ref pos, 0, false, PopoSigningKeyInput.GetInstance);
+            m_algorithmIdentifier = AlgorithmIdentifier.GetInstance(seq[pos++]);
+            m_signature = DerBitString.GetInstance(seq[pos++]);
 
-                m_poposkInput = PopoSigningKeyInput.GetInstance(
-                    Asn1Utilities.GetContextBaseUniversal(tagObj, 0, false, Asn1Tags.Sequence));
-            }
-            m_algorithmIdentifier = AlgorithmIdentifier.GetInstance(seq[index++]);
-            m_signature = DerBitString.GetInstance(seq[index]);
+            if (pos != count)
+                throw new ArgumentException("Unexpected elements in sequence", nameof(seq));
         }
 
         /**
@@ -49,8 +52,8 @@ namespace Org.BouncyCastle.Asn1.Crmf
         public PopoSigningKey(PopoSigningKeyInput poposkIn, AlgorithmIdentifier aid, DerBitString signature)
         {
             m_poposkInput = poposkIn;
-            m_algorithmIdentifier = aid;
-            m_signature = signature;
+            m_algorithmIdentifier = aid ?? throw new ArgumentNullException(nameof(aid));
+            m_signature = signature ?? throw new ArgumentNullException(nameof(signature));
         }
 
         public virtual PopoSigningKeyInput PoposkInput => m_poposkInput;
diff --git a/crypto/src/asn1/crmf/PopoSigningKeyInput.cs b/crypto/src/asn1/crmf/PopoSigningKeyInput.cs
index 865ed669d..c62159051 100644
--- a/crypto/src/asn1/crmf/PopoSigningKeyInput.cs
+++ b/crypto/src/asn1/crmf/PopoSigningKeyInput.cs
@@ -1,4 +1,6 @@
-using Org.BouncyCastle.Asn1.X509;
+using System;
+
+using Org.BouncyCastle.Asn1.X509;
 
 namespace Org.BouncyCastle.Asn1.Crmf
 {
@@ -25,6 +27,10 @@ namespace Org.BouncyCastle.Asn1.Crmf
 
         private PopoSigningKeyInput(Asn1Sequence seq)
         {
+            int count = seq.Count;
+            if (count != 2)
+                throw new ArgumentException("Bad sequence size: " + count, nameof(seq));
+
             Asn1Encodable authInfo = (Asn1Encodable)seq[0];
 
             if (authInfo is Asn1TaggedObject tagObj)
@@ -42,15 +48,15 @@ namespace Org.BouncyCastle.Asn1.Crmf
         /** Creates a new PopoSigningKeyInput with sender name as authInfo. */
         public PopoSigningKeyInput(GeneralName sender, SubjectPublicKeyInfo spki)
         {
-            m_sender = sender;
-            m_publicKey = spki;
+            m_sender = sender ?? throw new ArgumentNullException(nameof(sender));
+            m_publicKey = spki ?? throw new ArgumentNullException(nameof(spki));
         }
 
         /** Creates a new PopoSigningKeyInput using password-based MAC. */
         public PopoSigningKeyInput(PKMacValue pkmac, SubjectPublicKeyInfo spki)
         {
-            m_publicKeyMac = pkmac;
-            m_publicKey = spki;
+            m_publicKeyMac = pkmac ?? throw new ArgumentNullException(nameof(pkmac));
+            m_publicKey = spki ?? throw new ArgumentNullException(nameof(spki));
         }
 
         /** Returns the sender field, or null if authInfo is publicKeyMac */
@@ -83,7 +89,7 @@ namespace Org.BouncyCastle.Asn1.Crmf
 
             if (m_sender != null)
             {
-                v.Add(new DerTaggedObject(false, 0, m_sender));
+                v.Add(new DerTaggedObject(true, 0, m_sender));
             }
             else
             {
diff --git a/crypto/src/asn1/crmf/ProofOfPossession.cs b/crypto/src/asn1/crmf/ProofOfPossession.cs
index 6ef62efda..0e6a64dac 100644
--- a/crypto/src/asn1/crmf/ProofOfPossession.cs
+++ b/crypto/src/asn1/crmf/ProofOfPossession.cs
@@ -1,5 +1,6 @@
 using System;
 
+using Org.BouncyCastle.Tls;
 using Org.BouncyCastle.Utilities;
 
 namespace Org.BouncyCastle.Asn1.Crmf
@@ -12,53 +13,79 @@ namespace Org.BouncyCastle.Asn1.Crmf
         public const int TYPE_KEY_ENCIPHERMENT = 2;
         public const int TYPE_KEY_AGREEMENT = 3;
 
-        private readonly int tagNo;
-        private readonly Asn1Encodable obj;
-
-        private ProofOfPossession(Asn1TaggedObject tagged)
+        public static ProofOfPossession GetInstance(object obj)
         {
-            tagNo = tagged.TagNo;
-            switch (tagNo)
+            if (obj == null)
+                return null;
+
+            if (obj is Asn1Encodable element)
             {
-            case 0:
-                obj = DerNull.Instance;
-                break;
-            case 1:
-                obj = PopoSigningKey.GetInstance(tagged, false);
-                break;
-            case 2:
-            case 3:
-                // CHOICE so explicit
-                obj = PopoPrivKey.GetInstance(tagged, true);
-                break;
-            default:
-                throw new ArgumentException("unknown tag: " + tagNo, "tagged");
+                var result = GetOptional(element);
+                if (result != null)
+                    return result;
             }
+
+            throw new ArgumentException("Invalid object: " + Platform.GetTypeName(obj), nameof(obj));
         }
 
-        public static ProofOfPossession GetInstance(object obj)
+        public static ProofOfPossession GetInstance(Asn1TaggedObject taggedObject, bool declaredExplicit)
         {
-            if (obj is ProofOfPossession proofOfPossession)
+            return Asn1Utilities.GetInstanceFromChoice(taggedObject, declaredExplicit, GetInstance);
+        }
+
+        public static ProofOfPossession GetOptional(Asn1Encodable element)
+        {
+            if (element == null)
+                throw new ArgumentNullException(nameof(element));
+            if (element is ProofOfPossession proofOfPossession)
                 return proofOfPossession;
+            if (element is Asn1TaggedObject taggedObject)
+            {
+                Asn1Encodable baseObject = GetOptionalBaseObject(taggedObject);
+                if (baseObject != null)
+                    return new ProofOfPossession(taggedObject.TagNo, baseObject);
+            }
+            return null;
+        }
 
-            if (obj is Asn1TaggedObject taggedObject)
-                return new ProofOfPossession(taggedObject);
+        private static Asn1Encodable GetOptionalBaseObject(Asn1TaggedObject taggedObject)
+        {
+            if (taggedObject.HasContextTag())
+            {
+                switch (taggedObject.TagNo)
+                {
+                case 0:
+                    return DerNull.GetInstance(taggedObject, false);
+                case 1:
+                    return PopoSigningKey.GetInstance(taggedObject, false);
+                case 2:
+                case 3:
+                    // CHOICE so explicit
+                    return PopoPrivKey.GetInstance(taggedObject, true);
+                }
+            }
+            return null;
+        }
+
+        private readonly int m_tagNo;
+        private readonly Asn1Encodable m_obj;
 
-            throw new ArgumentException("Invalid object: " + Platform.GetTypeName(obj), "obj");
+        private ProofOfPossession(int tagNo, Asn1Encodable obj)
+        {
+            m_tagNo = tagNo;
+            m_obj = obj ?? throw new ArgumentNullException(nameof(obj));
         }
 
         /** Creates a ProofOfPossession with type raVerified. */
         public ProofOfPossession()
+            : this(TYPE_RA_VERIFIED, DerNull.Instance)
         {
-            tagNo = TYPE_RA_VERIFIED;
-            obj = DerNull.Instance;
         }
 
         /** Creates a ProofOfPossession for a signing key. */
         public ProofOfPossession(PopoSigningKey Poposk)
+            : this(TYPE_SIGNING_KEY, Poposk)
         {
-            tagNo = TYPE_SIGNING_KEY;
-            obj = Poposk;
         }
 
         /**
@@ -66,20 +93,13 @@ namespace Org.BouncyCastle.Asn1.Crmf
          * @param type one of TYPE_KEY_ENCIPHERMENT or TYPE_KEY_AGREEMENT
          */
         public ProofOfPossession(int type, PopoPrivKey privkey)
+            : this(type, (Asn1Encodable)privkey)
         {
-            tagNo = type;
-            obj = privkey;
         }
 
-        public virtual int Type
-        {
-            get { return tagNo; }
-        }
+        public virtual int Type => m_tagNo;
 
-        public virtual Asn1Encodable Object
-        {
-            get { return obj; }
-        }
+        public virtual Asn1Encodable Object => m_obj;
 
         /**
          * 
@@ -95,7 +115,8 @@ namespace Org.BouncyCastle.Asn1.Crmf
          */
         public override Asn1Object ToAsn1Object()
         {
-            return new DerTaggedObject(false, tagNo, obj);
+            // NOTE: Explicit tagging automatically applied for PopoPrivKey (a CHOICE)
+            return new DerTaggedObject(false, m_tagNo, m_obj);
         }
     }
 }
diff --git a/crypto/src/asn1/crmf/SinglePubInfo.cs b/crypto/src/asn1/crmf/SinglePubInfo.cs
index 5205ce366..7d5d742fd 100644
--- a/crypto/src/asn1/crmf/SinglePubInfo.cs
+++ b/crypto/src/asn1/crmf/SinglePubInfo.cs
@@ -1,42 +1,50 @@
 using System;
 
 using Org.BouncyCastle.Asn1.X509;
-using Org.BouncyCastle.Utilities;
 
 namespace Org.BouncyCastle.Asn1.Crmf
 {
     public class SinglePubInfo
         : Asn1Encodable
     {
-        private readonly DerInteger pubMethod;
-        private readonly GeneralName pubLocation;
-
-        private SinglePubInfo(Asn1Sequence seq)
+        public static SinglePubInfo GetInstance(object obj)
         {
-            pubMethod = DerInteger.GetInstance(seq[0]);
+            if (obj == null)
+                return null;
+            if (obj is SinglePubInfo singlePubInfo)
+                return singlePubInfo;
+            return new SinglePubInfo(Asn1Sequence.GetInstance(obj));
+        }
 
-            if (seq.Count == 2)
-            {
-                pubLocation = GeneralName.GetInstance(seq[1]);
-            }
+        public static SinglePubInfo GetInstance(Asn1TaggedObject taggedObject, bool declaredExplicit)
+        {
+            return new SinglePubInfo(Asn1Sequence.GetInstance(taggedObject, declaredExplicit));
         }
 
-        public static SinglePubInfo GetInstance(object obj)
+        private readonly DerInteger m_pubMethod;
+        private readonly GeneralName m_pubLocation;
+
+        private SinglePubInfo(Asn1Sequence seq)
         {
-            if (obj is SinglePubInfo)
-                return (SinglePubInfo)obj;
+            int count = seq.Count;
+            if (count < 1 || count > 2)
+                throw new ArgumentException("Bad sequence size: " + count, nameof(seq));
 
-            if (obj is Asn1Sequence)
-                return new SinglePubInfo((Asn1Sequence)obj);
+            int pos = 0;
 
-            throw new ArgumentException("Invalid object: " + Platform.GetTypeName(obj), "obj");
-        }
+            m_pubMethod = DerInteger.GetInstance(seq[pos++]);
 
-        public virtual GeneralName PubLocation
-        {
-            get { return pubLocation; }
+            if (pos < count)
+            {
+                m_pubLocation = GeneralName.GetInstance(seq[pos++]);
+            }
+
+            if (pos != count)
+                throw new ArgumentException("Unexpected elements in sequence", nameof(seq));
         }
 
+        public virtual GeneralName PubLocation => m_pubLocation;
+
         /**
          * 
          * SinglePubInfo ::= SEQUENCE {
@@ -51,9 +59,9 @@ namespace Org.BouncyCastle.Asn1.Crmf
          */
         public override Asn1Object ToAsn1Object()
         {
-            Asn1EncodableVector v = new Asn1EncodableVector(pubMethod);
-            v.AddOptional(pubLocation);
-            return new DerSequence(v);
+            return m_pubLocation == null
+                ?  new DerSequence(m_pubMethod)
+                :  new DerSequence(m_pubMethod, m_pubLocation);
         }
     }
 }
-- 
cgit 1.5.1