Refactoring around X509

6 files changed, 133 insertions, 235 deletions

diff --git a/crypto/src/x509/AttributeCertificateHolder.cs b/crypto/src/x509/AttributeCertificateHolder.cs
index 9122a809f..1df5b1a56 100644
--- a/crypto/src/x509/AttributeCertificateHolder.cs
+++ b/crypto/src/x509/AttributeCertificateHolder.cs
@@ -1,73 +1,60 @@
 using System;
+using System.Collections.Generic;
 
 using Org.BouncyCastle.Asn1;
 using Org.BouncyCastle.Asn1.X509;
 using Org.BouncyCastle.Crypto;
 using Org.BouncyCastle.Math;
 using Org.BouncyCastle.Security;
-using Org.BouncyCastle.Security.Certificates;
 using Org.BouncyCastle.Utilities;
 using Org.BouncyCastle.Utilities.Collections;
 
 namespace Org.BouncyCastle.X509
 {
-	/// <remarks>
-	/// The Holder object.
-	/// <pre>
- 	/// Holder ::= SEQUENCE {
- 	///		baseCertificateID   [0] IssuerSerial OPTIONAL,
- 	///			-- the issuer and serial number of
- 	///			-- the holder's Public Key Certificate
- 	///		entityName          [1] GeneralNames OPTIONAL,
- 	///			-- the name of the claimant or role
- 	///		objectDigestInfo    [2] ObjectDigestInfo OPTIONAL
- 	///			-- used to directly authenticate the holder,
- 	///			-- for example, an executable
- 	/// }
-	/// </pre>
-	/// </remarks>
-	public class AttributeCertificateHolder
-		//: CertSelector, Selector
-		: ISelector<X509Certificate>
+    /// <remarks>
+    /// The Holder object.
+    /// <pre>
+    /// Holder ::= SEQUENCE {
+    ///		baseCertificateID   [0] IssuerSerial OPTIONAL,
+    ///			-- the issuer and serial number of
+    ///			-- the holder's Public Key Certificate
+    ///		entityName          [1] GeneralNames OPTIONAL,
+    ///			-- the name of the claimant or role
+    ///		objectDigestInfo    [2] ObjectDigestInfo OPTIONAL
+    ///			-- used to directly authenticate the holder,
+    ///			-- for example, an executable
+    /// }
+    /// </pre>
+    /// </remarks>
+    public class AttributeCertificateHolder
+		: IEquatable<AttributeCertificateHolder>, ISelector<X509Certificate>
 	{
-		internal readonly Holder holder;
+		internal readonly Holder m_holder;
 
-		internal AttributeCertificateHolder(
-			Asn1Sequence seq)
+		internal AttributeCertificateHolder(Asn1Sequence seq)
 		{
-			holder = Holder.GetInstance(seq);
+			m_holder = Holder.GetInstance(seq);
 		}
 
-		public AttributeCertificateHolder(
-			X509Name	issuerName,
-			BigInteger	serialNumber)
+		public AttributeCertificateHolder(X509Name issuerName, BigInteger serialNumber)
 		{
-			holder = new Holder(
+			m_holder = new Holder(
 				new IssuerSerial(
 					GenerateGeneralNames(issuerName),
 					new DerInteger(serialNumber)));
 		}
 
-		public AttributeCertificateHolder(
-			X509Certificate	cert)
+		public AttributeCertificateHolder(X509Certificate cert)
 		{
-			X509Name name;
-			try
-			{
-				name = PrincipalUtilities.GetIssuerX509Principal(cert);
-			}
-			catch (Exception e)
-			{
-				throw new CertificateParsingException(e.Message);
-			}
-
-			holder = new Holder(new IssuerSerial(GenerateGeneralNames(name), new DerInteger(cert.SerialNumber)));
+			m_holder = new Holder(
+				new IssuerSerial(
+					GenerateGeneralNames(cert.IssuerDN),
+					new DerInteger(cert.SerialNumber)));
 		}
 
-		public AttributeCertificateHolder(
-			X509Name principal)
+		public AttributeCertificateHolder(X509Name principal)
 		{
-			holder = new Holder(GenerateGeneralNames(principal));
+			m_holder = new Holder(GenerateGeneralNames(principal));
 		}
 
 		/**
@@ -93,16 +80,14 @@ namespace Org.BouncyCastle.X509
 		 *            <code>otherObjectDigest</code>.
 		 * @param objectDigest The hash value.
 		 */
-		public AttributeCertificateHolder(
-			int		digestedObjectType,
-			string	digestAlgorithm,
-			string	otherObjectTypeID,
-			byte[]	objectDigest)
+		public AttributeCertificateHolder(int digestedObjectType, string digestAlgorithm, string otherObjectTypeID,
+			byte[] objectDigest)
 		{
-			// TODO Allow 'objectDigest' to be null?
+			var digestAlgorithmID = new AlgorithmIdentifier(new DerObjectIdentifier(digestAlgorithm));
+			var objectDigestInfo = new ObjectDigestInfo(digestedObjectType, otherObjectTypeID, digestAlgorithmID,
+				Arrays.Clone(objectDigest));
 
-			holder = new Holder(new ObjectDigestInfo(digestedObjectType, otherObjectTypeID,
-				new AlgorithmIdentifier(new DerObjectIdentifier(digestAlgorithm)), Arrays.Clone(objectDigest)));
+            m_holder = new Holder(objectDigestInfo);
 		}
 
 		/**
@@ -124,7 +109,7 @@ namespace Org.BouncyCastle.X509
 		{
 			get
 			{
-				ObjectDigestInfo odi = holder.ObjectDigestInfo;
+				ObjectDigestInfo odi = m_holder.ObjectDigestInfo;
 
 				return odi == null
 					?   -1
@@ -138,31 +123,14 @@ namespace Org.BouncyCastle.X509
 		 * @return The other object type ID or <code>null</code> if no object
 		 *         digest info is set.
 		 */
-		public string DigestAlgorithm
-		{
-			get
-			{
-				ObjectDigestInfo odi = holder.ObjectDigestInfo;
-
-				return odi == null
-					?	null
-					:	odi.DigestAlgorithm.Algorithm.Id;
-			}
-		}
+		public string DigestAlgorithm => m_holder.ObjectDigestInfo?.DigestAlgorithm.Algorithm.Id;
 
 		/**
 		 * Returns the hash if an object digest info is used.
 		 *
 		 * @return The hash or <code>null</code> if no object digest info is set.
 		 */
-		public byte[] GetObjectDigest()
-		{
-			ObjectDigestInfo odi = holder.ObjectDigestInfo;
-
-			return odi == null
-				?	null
-				:	odi.ObjectDigest.GetBytes();
-		}
+		public byte[] GetObjectDigest() => m_holder.ObjectDigestInfo?.ObjectDigest.GetBytes();
 
 		/**
 		 * Returns the digest algorithm ID if an object digest info is used.
@@ -170,43 +138,20 @@ namespace Org.BouncyCastle.X509
 		 * @return The digest algorithm ID or <code>null</code> if no object
 		 *         digest info is set.
 		 */
-		public string OtherObjectTypeID
-		{
-			get
-			{
-				ObjectDigestInfo odi = holder.ObjectDigestInfo;
+		public string OtherObjectTypeID => m_holder.ObjectDigestInfo?.OtherObjectTypeID.Id;
 
-				return odi == null
-					?	null
-					:	odi.OtherObjectTypeID.Id;
-			}
-		}
-
-		private GeneralNames GenerateGeneralNames(
-			X509Name principal)
-		{
-//			return GeneralNames.GetInstance(new DerSequence(new GeneralName(principal)));
-			return new GeneralNames(new GeneralName(principal));
-		}
+		private GeneralNames GenerateGeneralNames(X509Name principal) => new GeneralNames(new GeneralName(principal));
 
-		private bool MatchesDN(
-			X509Name		subject,
-			GeneralNames	targets)
+		private bool MatchesDN(X509Name subject, GeneralNames targets)
 		{
-			GeneralName[] names = targets.GetNames();
-
-			for (int i = 0; i != names.Length; i++)
+			foreach (var gn in targets.GetNames())
 			{
-				GeneralName gn = names[i];
-
 				if (gn.TagNo == GeneralName.DirectoryName)
 				{
 					try
 					{
 						if (X509Name.GetInstance(gn.Name).Equivalent(subject))
-						{
 							return true;
-						}
 					}
 					catch (Exception)
 					{
@@ -217,59 +162,18 @@ namespace Org.BouncyCastle.X509
 			return false;
 		}
 
-		private object[] GetNames(
-			GeneralName[] names)
+		private X509Name[] GetPrincipals(GeneralNames generalNames)
 		{
-            int count = 0;
-            for (int i = 0; i != names.Length; i++)
+			var names = generalNames.GetNames();
+			var result = new List<X509Name>(names.Length);
+			foreach (var name in names)
             {
-                if (names[i].TagNo == GeneralName.DirectoryName)
+                if (GeneralName.DirectoryName == name.TagNo)
                 {
-                    ++count;
+					result.Add(X509Name.GetInstance(name.Name));
                 }
-            }
-
-            object[] result = new object[count];
-
-            int pos = 0;
-            for (int i = 0; i != names.Length; i++)
-            {
-                if (names[i].TagNo == GeneralName.DirectoryName)
-                {
-                    result[pos++] = X509Name.GetInstance(names[i].Name);
-                }
-            }
-
-            return result;
-        }
-
-		private X509Name[] GetPrincipals(
-			GeneralNames names)
-		{
-			object[] p = this.GetNames(names.GetNames());
-
-            int count = 0;
-
-            for (int i = 0; i != p.Length; i++)
-			{
-				if (p[i] is X509Name)
-				{
-                    ++count;
-				}
 			}
-
-            X509Name[] result = new X509Name[count];
-
-            int pos = 0;
-            for (int i = 0; i != p.Length; i++)
-            {
-                if (p[i] is X509Name)
-                {
-                    result[pos++] = (X509Name)p[i];
-                }
-            }
-
-            return result;
+			return result.ToArray();
         }
 
 		/**
@@ -279,12 +183,8 @@ namespace Org.BouncyCastle.X509
 		 */
 		public X509Name[] GetEntityNames()
 		{
-			if (holder.EntityName != null)
-			{
-				return GetPrincipals(holder.EntityName);
-			}
-
-			return null;
+			var entityName = m_holder.EntityName;
+			return entityName == null ? null : GetPrincipals(entityName);
 		}
 
 		/**
@@ -294,12 +194,8 @@ namespace Org.BouncyCastle.X509
 		 */
 		public X509Name[] GetIssuer()
 		{
-			if (holder.BaseCertificateID != null)
-			{
-				return GetPrincipals(holder.BaseCertificateID.Issuer);
-			}
-
-			return null;
+			var baseCertificateID = m_holder.BaseCertificateID;
+			return baseCertificateID == null ? null : GetPrincipals(baseCertificateID.Issuer);
 		}
 
 		/**
@@ -307,23 +203,9 @@ namespace Org.BouncyCastle.X509
 		 *
 		 * @return the certificate serial number, null if no BaseCertificateID is set.
 		 */
-		public BigInteger SerialNumber
-		{
-			get
-			{
-				if (holder.BaseCertificateID != null)
-				{
-					return holder.BaseCertificateID.Serial.Value;
-				}
+		public BigInteger SerialNumber => m_holder.BaseCertificateID?.Serial.Value;
 
-				return null;
-			}
-		}
-
-		public object Clone()
-		{
-			return new AttributeCertificateHolder((Asn1Sequence)holder.ToAsn1Object());
-		}
+		public object Clone() => new AttributeCertificateHolder((Asn1Sequence)m_holder.ToAsn1Object());
 
 		public bool Match(X509Certificate x509Cert)
 		{
@@ -332,23 +214,26 @@ namespace Org.BouncyCastle.X509
 
 			try
 			{
-				if (holder.BaseCertificateID != null)
+                var baseCertificateID = m_holder.BaseCertificateID;
+                if (baseCertificateID != null)
 				{
-					return holder.BaseCertificateID.Serial.HasValue(x509Cert.SerialNumber)
-						&& MatchesDN(PrincipalUtilities.GetIssuerX509Principal(x509Cert), holder.BaseCertificateID.Issuer);
+					return baseCertificateID.Serial.HasValue(x509Cert.SerialNumber)
+						&& MatchesDN(x509Cert.IssuerDN, baseCertificateID.Issuer);
 				}
 
-				if (holder.EntityName != null)
+				var entityName = m_holder.EntityName;
+				if (entityName != null)
 				{
-					if (MatchesDN(PrincipalUtilities.GetSubjectX509Principal(x509Cert), holder.EntityName))
+					if (MatchesDN(x509Cert.SubjectDN, entityName))
 						return true;
 				}
 
-				if (holder.ObjectDigestInfo != null)
+				var objectDigestInfo = m_holder.ObjectDigestInfo;
+				if (objectDigestInfo != null)
 				{
 					IDigest md = DigestUtilities.GetDigest(DigestAlgorithm);
 
-					switch (DigestedObjectType)
+					switch (objectDigestInfo.DigestedObjectType.IntValueExact)
 					{
 					case ObjectDigestInfo.PublicKey:
 					{
@@ -363,15 +248,12 @@ namespace Org.BouncyCastle.X509
 						md.BlockUpdate(b, 0, b.Length);
 						break;
 					}
-
 					// TODO Default handler?
 					}
 
 					// TODO Shouldn't this be the other way around?
-					if (!Arrays.AreEqual(DigestUtilities.DoFinal(md), GetObjectDigest()))
-					{
+					if (!Arrays.AreEqual(GetObjectDigest(), DigestUtilities.DoFinal(md)))
 						return false;
-					}
 				}
 			}
 			catch (Exception)
@@ -381,27 +263,13 @@ namespace Org.BouncyCastle.X509
 			return false;
 		}
 
-		public override bool Equals(
-			object obj)
+		public virtual bool Equals(AttributeCertificateHolder other)
 		{
-			if (obj == this)
-			{
-				return true;
-			}
-
-			if (!(obj is AttributeCertificateHolder))
-			{
-				return false;
-			}
-
-			AttributeCertificateHolder other = (AttributeCertificateHolder)obj;
+            return this == other || m_holder.Equals(other?.m_holder);
+        }
 
-			return this.holder.Equals(other.holder);
-		}
+		public override bool Equals(object obj) => Equals(obj as AttributeCertificateHolder);
 
-		public override int GetHashCode()
-		{
-			return this.holder.GetHashCode();
-		}
+		public override int GetHashCode() => m_holder.GetHashCode();
 	}
 }
diff --git a/crypto/src/x509/PrincipalUtil.cs b/crypto/src/x509/PrincipalUtil.cs
index fb1b01b40..22b0fd76e 100644
--- a/crypto/src/x509/PrincipalUtil.cs
+++ b/crypto/src/x509/PrincipalUtil.cs
@@ -1,3 +1,5 @@
+using System;
+
 using Org.BouncyCastle.Asn1.X509;
 
 namespace Org.BouncyCastle.X509
@@ -10,24 +12,16 @@ namespace Org.BouncyCastle.X509
     /// can be.</p>
     /// </remarks>
     // TODO[api] Make static
+    [Obsolete("Will be removed")]
     public class PrincipalUtilities
 	{
 		/// <summary>Return the issuer of the given cert as an X509Principal.</summary>
-		public static X509Name GetIssuerX509Principal(X509Certificate cert)
-		{
-            return cert.TbsCertificate.Issuer;
-		}
+		public static X509Name GetIssuerX509Principal(X509Certificate cert) => cert.IssuerDN;
 
 		/// <summary>Return the subject of the given cert as an X509Principal.</summary>
-		public static X509Name GetSubjectX509Principal(X509Certificate cert)
-		{
-            return cert.TbsCertificate.Subject;
-		}
+		public static X509Name GetSubjectX509Principal(X509Certificate cert) => cert.SubjectDN;
 
 		/// <summary>Return the issuer of the given CRL as an X509Principal.</summary>
-		public static X509Name GetIssuerX509Principal(X509Crl crl)
-		{
-			return crl.CertificateList.TbsCertList.Issuer;
-		}
+		public static X509Name GetIssuerX509Principal(X509Crl crl) => crl.IssuerDN;
 	}
 }
diff --git a/crypto/src/x509/X509Utilities.cs b/crypto/src/x509/X509Utilities.cs
index e7dfeb2c5..cee664370 100644
--- a/crypto/src/x509/X509Utilities.cs
+++ b/crypto/src/x509/X509Utilities.cs
@@ -137,6 +137,22 @@ namespace Org.BouncyCastle.X509
 			return digestResult.Collect();
         }
 
+        internal static byte[] CalculateDigest(IDigestFactory digestFactory, byte[] buf, int off, int len)
+        {
+            var digestCalculator = digestFactory.CreateCalculator();
+            var digestResult = CalculateResult(digestCalculator, buf, off, len);
+            return digestResult.Collect();
+        }
+
+#if NETCOREAPP2_1_OR_GREATER || NETSTANDARD2_1_OR_GREATER
+        internal static byte[] CalculateDigest(IDigestFactory digestFactory, ReadOnlySpan<byte> buf)
+        {
+            var digestCalculator = digestFactory.CreateCalculator();
+            var digestResult = CalculateResult(digestCalculator, buf);
+            return digestResult.Collect();
+        }
+#endif
+
         internal static byte[] CalculateDigest(IDigestFactory digestFactory,
             Asn1Encodable asn1Encodable)
         {
@@ -145,6 +161,28 @@ namespace Org.BouncyCastle.X509
             return digestResult.Collect();
         }
 
+        internal static TResult CalculateResult<TResult>(IStreamCalculator<TResult> streamCalculator, byte[] buf,
+            int off, int len)
+        {
+            using (var stream = streamCalculator.Stream)
+            {
+                stream.Write(buf, off, len);
+            }
+            return streamCalculator.GetResult();
+        }
+
+#if NETCOREAPP2_1_OR_GREATER || NETSTANDARD2_1_OR_GREATER
+        internal static TResult CalculateResult<TResult>(IStreamCalculator<TResult> streamCalculator,
+            ReadOnlySpan<byte> buf)
+        {
+            using (var stream = streamCalculator.Stream)
+            {
+                stream.Write(buf);
+            }
+            return streamCalculator.GetResult();
+        }
+#endif
+
         internal static TResult CalculateResult<TResult>(IStreamCalculator<TResult> streamCalculator,
             Asn1Encodable asn1Encodable)
         {
diff --git a/crypto/src/x509/X509V2AttributeCertificateGenerator.cs b/crypto/src/x509/X509V2AttributeCertificateGenerator.cs
index bbb246273..6e5343fce 100644
--- a/crypto/src/x509/X509V2AttributeCertificateGenerator.cs
+++ b/crypto/src/x509/X509V2AttributeCertificateGenerator.cs
@@ -34,7 +34,7 @@ namespace Org.BouncyCastle.X509
 		public void SetHolder(
 			AttributeCertificateHolder holder)
 		{
-			acInfoGen.SetHolder(holder.holder);
+			acInfoGen.SetHolder(holder.m_holder);
 		}
 
 		/// <summary>Set the issuer.</summary>
diff --git a/crypto/src/x509/X509V3CertificateGenerator.cs b/crypto/src/x509/X509V3CertificateGenerator.cs
index 2df8d5409..229508e67 100644
--- a/crypto/src/x509/X509V3CertificateGenerator.cs
+++ b/crypto/src/x509/X509V3CertificateGenerator.cs
@@ -136,11 +136,20 @@ namespace Org.BouncyCastle.X509
             tbsGen.SetSubjectPublicKeyInfo(SubjectPublicKeyInfoFactory.CreateSubjectPublicKeyInfo(publicKey));
         }
 
-		/// <summary>
-		/// Set the subject unique ID - note: it is very rare that it is correct to do this.
-		/// </summary>
-		/// <param name="uniqueID"/>
-		public void SetSubjectUniqueID(
+        /// <summary>
+        /// Set the SubjectPublicKeyInfo for the public key that this certificate identifies.
+        /// </summary>
+        /// <param name="subjectPublicKeyInfo"/>
+        public void SetSubjectPublicKeyInfo(SubjectPublicKeyInfo subjectPublicKeyInfo)
+        {
+            tbsGen.SetSubjectPublicKeyInfo(subjectPublicKeyInfo);
+        }
+
+        /// <summary>
+        /// Set the subject unique ID - note: it is very rare that it is correct to do this.
+        /// </summary>
+        /// <param name="uniqueID"/>
+        public void SetSubjectUniqueID(
 			bool[] uniqueID)
 		{
 			tbsGen.SetSubjectUniqueID(BooleanToBitString(uniqueID));
@@ -157,15 +166,6 @@ namespace Org.BouncyCastle.X509
 		}
 
         /// <summary>
-        /// Set the SubjectPublicKeyInfo for the public key that this certificate identifies.
-        /// </summary>
-        /// <param name="subjectPublicKeyInfo"/>
-        public void SetSubjectPublicKeyInfo(SubjectPublicKeyInfo subjectPublicKeyInfo)
-        {
-            tbsGen.SetSubjectPublicKeyInfo(subjectPublicKeyInfo);
-        }
-
-        /// <summary>
         /// Add a given extension field for the standard extensions tag (tag 3).
         /// </summary>
         /// <param name="oid">string containing a dotted decimal Object Identifier.</param>
diff --git a/crypto/src/x509/extension/AuthorityKeyIdentifierStructure.cs b/crypto/src/x509/extension/AuthorityKeyIdentifierStructure.cs
index 006dc009b..abd6bb7cc 100644
--- a/crypto/src/x509/extension/AuthorityKeyIdentifierStructure.cs
+++ b/crypto/src/x509/extension/AuthorityKeyIdentifierStructure.cs
@@ -25,13 +25,11 @@ namespace Org.BouncyCastle.X509.Extension
 		{
 		}
 
-		private static Asn1Sequence FromCertificate(
-			X509Certificate certificate)
+		private static Asn1Sequence FromCertificate(X509Certificate certificate)
 		{
 			try
 			{
-				GeneralName genName = new GeneralName(
-					PrincipalUtilities.GetIssuerX509Principal(certificate));
+				GeneralName genName = new GeneralName(certificate.IssuerDN);
 
 				if (certificate.Version == 3)
 				{