Rework SIgnedPublicKeyAndChallenge

4 files changed, 134 insertions, 114 deletions

diff --git a/crypto/src/asn1/mozilla/PublicKeyAndChallenge.cs b/crypto/src/asn1/mozilla/PublicKeyAndChallenge.cs
index eafbfd72b..6b43b1c1f 100644
--- a/crypto/src/asn1/mozilla/PublicKeyAndChallenge.cs
+++ b/crypto/src/asn1/mozilla/PublicKeyAndChallenge.cs
@@ -19,9 +19,6 @@ namespace Org.BouncyCastle.Asn1.Mozilla
     public class PublicKeyAndChallenge
         : Asn1Encodable
     {
-        private readonly SubjectPublicKeyInfo m_spki;
-		private readonly DerIA5String m_challenge;
-
         public static PublicKeyAndChallenge GetInstance(object obj)
         {
             if (obj == null)
@@ -40,6 +37,9 @@ namespace Org.BouncyCastle.Asn1.Mozilla
 #pragma warning restore CS0618 // Type or member is obsolete
         }
 
+        private readonly SubjectPublicKeyInfo m_spki;
+        private readonly DerIA5String m_challenge;
+
         public PublicKeyAndChallenge(SubjectPublicKeyInfo spki, DerIA5String challenge)
         {
             m_spki = spki ?? throw new ArgumentNullException(nameof(spki));
@@ -49,6 +49,11 @@ namespace Org.BouncyCastle.Asn1.Mozilla
         [Obsolete("Use 'GetInstance' instead")]
         public PublicKeyAndChallenge(Asn1Sequence seq)
         {
+            if (seq == null)
+                throw new ArgumentNullException(nameof(seq));
+            if (seq.Count != 2)
+                throw new ArgumentException($"Expected 2 elements, but found {seq.Count}", nameof(seq));
+
             m_spki = SubjectPublicKeyInfo.GetInstance(seq[0]);
             m_challenge = DerIA5String.GetInstance(seq[1]);
         }
diff --git a/crypto/src/asn1/mozilla/SignedPublicKeyAndChallenge.cs b/crypto/src/asn1/mozilla/SignedPublicKeyAndChallenge.cs
index 243cbefd6..76f988c28 100644
--- a/crypto/src/asn1/mozilla/SignedPublicKeyAndChallenge.cs
+++ b/crypto/src/asn1/mozilla/SignedPublicKeyAndChallenge.cs
@@ -1,138 +1,71 @@
 using System;
 
 using Org.BouncyCastle.Asn1.X509;
-using Org.BouncyCastle.Crypto;
-using Org.BouncyCastle.Crypto.Operators;
-using Org.BouncyCastle.Security;
 
 namespace Org.BouncyCastle.Asn1.Mozilla
 {
-    /**
-	 * This is designed to parse
-	 * the SignedPublicKeyAndChallenge created by the KEYGEN tag included by
-	 * Mozilla based browsers.
-     *  <pre>
-     *
-     *  SignedPublicKeyAndChallenge ::= SEQUENCE {
-     *    publicKeyAndChallenge PublicKeyAndChallenge,
-     *    signatureAlgorithm AlgorithmIdentifier,
-     *    signature BIT STRING
-     *  }
-     *  </pre>
-     */
-    internal class SignedPublicKeyAndChallenge : Asn1Encodable
+    /// <summary>
+    /// For parsing the SignedPublicKeyAndChallenge created by the KEYGEN tag included by Mozilla based browsers.
+    /// </summary>
+    /// <remarks>
+    /// <code>
+    /// SignedPublicKeyAndChallenge ::= SEQUENCE
+    /// {
+    ///     publicKeyAndChallenge   PublicKeyAndChallenge,
+    ///     signatureAlgorithm      AlgorithmIdentifier,
+    ///     signature               BIT STRING
+    /// }
+    /// </code>
+    /// </remarks>
+    public class SignedPublicKeyAndChallenge
+        : Asn1Encodable
     {
-        private Asn1Sequence            seq;
-        private PublicKeyAndChallenge   publicKeyAndChallenge;
-        private AlgorithmIdentifier     algorithmIdentifier;
-        private DerBitString            signature;
-
-        public PublicKeyAndChallenge PublicKeyAndChallenge
-        {
-            get { return publicKeyAndChallenge; }
-        }
-
-        public AlgorithmIdentifier AlgorithmIdentifier
-        {
-            get { return algorithmIdentifier; }
-        }
-
-        public DerBitString Signature
-        {
-            get { return signature; }
-        }
-
-        public static SignedPublicKeyAndChallenge GetInstance(Asn1TaggedObject obj, bool explicitly)
-        {
-            return GetInstance(Asn1Sequence.GetInstance(obj, explicitly));
-        }
-
         public static SignedPublicKeyAndChallenge GetInstance(object obj)
         {
-            if (null == obj)
-            {
+            if (obj == null)
                 return null;
-            }
-
-            if (obj is SignedPublicKeyAndChallenge value)
-            {
-                return value;
-            }
-
-            if (obj is Asn1Sequence asn1Sequence)
-            {
-                return new SignedPublicKeyAndChallenge(asn1Sequence);
-            }
-
-            throw new ArgumentException(string.Format("object of unexpected type provided, type=[{0}]", obj.GetType().FullName));
+            if (obj is SignedPublicKeyAndChallenge signedPublicKeyAndChallenge)
+                return signedPublicKeyAndChallenge;
+            return new SignedPublicKeyAndChallenge(Asn1Sequence.GetInstance(obj));
         }
 
-        public SignedPublicKeyAndChallenge(
-            PublicKeyAndChallenge   publicKeyAndChallenge,
-            AlgorithmIdentifier     algorithmIdentifier,
-            DerBitString            signature)
+        public static SignedPublicKeyAndChallenge GetInstance(Asn1TaggedObject taggedObject, bool declaredExplicit)
         {
-            this.publicKeyAndChallenge = publicKeyAndChallenge;
-            this.algorithmIdentifier = algorithmIdentifier;
-            this.signature = signature;
+            return new SignedPublicKeyAndChallenge(Asn1Sequence.GetInstance(taggedObject, declaredExplicit));
         }
 
-        public SignedPublicKeyAndChallenge(
-            Asn1Sequence seq)
-        {
-            this.seq = seq;
+        private readonly PublicKeyAndChallenge m_publicKeyAndChallenge;
+        private readonly AlgorithmIdentifier m_signatureAlgorithm;
+        private readonly DerBitString m_signature;
 
-            if (seq.Count != 3)
-                throw new FormatException($"Sequence contains {seq.Count} elements. Expected 3 elements");
-
-            this.publicKeyAndChallenge = PublicKeyAndChallenge.GetInstance(seq[0]);
-            this.algorithmIdentifier = AlgorithmIdentifier.GetInstance(seq[1]);
-            this.signature = DerBitString.GetInstance(seq[2]);
-        }
-
-        public bool Verify()
+        public SignedPublicKeyAndChallenge(PublicKeyAndChallenge publicKeyAndChallenge,
+            AlgorithmIdentifier signatureAlgorithm, DerBitString signature)
         {
-            AsymmetricKeyParameter publicKey = PublicKeyFactory.CreateKey(this.PublicKeyAndChallenge.SubjectPublicKeyInfo);
-            Asn1VerifierFactoryProvider factory = new Asn1VerifierFactoryProvider(publicKey);
-            IVerifierFactory verifier = factory.CreateVerifierFactory(this.AlgorithmIdentifier);
-
-            try
-            {
-                byte[] derEncoded = this.PublicKeyAndChallenge.GetEncoded();
-                IStreamCalculator<IVerifier> streamCalculator = verifier.CreateCalculator();
-                streamCalculator.Stream.Write(derEncoded, 0, derEncoded.Length);
-                streamCalculator.Stream.Dispose();
-
-                return streamCalculator.GetResult().IsVerified(this.Signature.GetOctets());
-            }
-            catch (Exception exception)
-            {
-                throw new SignatureException("exception encoding SPKAC request", exception);
-            }
+            m_publicKeyAndChallenge = publicKeyAndChallenge
+                ?? throw new ArgumentNullException(nameof(publicKeyAndChallenge));
+            m_signatureAlgorithm = signatureAlgorithm ?? throw new ArgumentNullException(nameof(signatureAlgorithm));
+            m_signature = signature ?? throw new ArgumentNullException(nameof(signature));
         }
 
-        public override Asn1Object ToAsn1Object()
+        private SignedPublicKeyAndChallenge(Asn1Sequence seq)
         {
-            if (null == this.seq)
-            {
-                Asn1EncodableVector v = new Asn1EncodableVector();
+            if (seq == null)
+                throw new ArgumentNullException(nameof(seq));
+            if (seq.Count != 3)
+                throw new ArgumentException($"Expected 3 elements, but found {seq.Count}", nameof(seq));
 
-                if (null == this.PublicKeyAndChallenge)
-                    throw new FormatException($"{nameof(this.PublicKeyAndChallenge)} can not be null");
+            m_publicKeyAndChallenge = PublicKeyAndChallenge.GetInstance(seq[0]);
+            m_signatureAlgorithm = AlgorithmIdentifier.GetInstance(seq[1]);
+            m_signature = DerBitString.GetInstance(seq[2]);
+        }
 
-                if (null == this.AlgorithmIdentifier)
-                    throw new FormatException($"{nameof(this.AlgorithmIdentifier)} can not be null");
+        public PublicKeyAndChallenge PublicKeyAndChallenge => m_publicKeyAndChallenge;
 
-                if (null == this.Signature)
-                    throw new FormatException($"{nameof(this.Signature)} can not be null");
+        public DerBitString Signature => m_signature;
 
-                v.Add(this.PublicKeyAndChallenge);
-                v.Add(this.AlgorithmIdentifier);
-                v.Add(this.Signature);
+        public AlgorithmIdentifier SignatureAlgorithm => m_signatureAlgorithm;
 
-                this.seq = new DerSequence(v);
-            }
-            return this.seq;
-        }
+        public override Asn1Object ToAsn1Object() =>
+            new DerSequence(m_publicKeyAndChallenge, m_signatureAlgorithm, m_signature);
     }
 }
diff --git a/crypto/src/mozilla/SignedPublicKeyAndChallenge.cs b/crypto/src/mozilla/SignedPublicKeyAndChallenge.cs
new file mode 100644
index 000000000..03b8e90ac
--- /dev/null
+++ b/crypto/src/mozilla/SignedPublicKeyAndChallenge.cs
@@ -0,0 +1,48 @@
+using System;
+
+using Org.BouncyCastle.Crypto;
+using Org.BouncyCastle.Crypto.Operators;
+using Org.BouncyCastle.Security;
+
+namespace Org.BouncyCastle.Mozilla
+{
+    public sealed class SignedPublicKeyAndChallenge
+    {
+        private readonly Asn1.Mozilla.SignedPublicKeyAndChallenge m_spkac;
+
+        public SignedPublicKeyAndChallenge(byte[] encoding)
+            : this(Asn1.Mozilla.SignedPublicKeyAndChallenge.GetInstance(encoding))
+        {
+        }
+
+        public SignedPublicKeyAndChallenge(Asn1.Mozilla.SignedPublicKeyAndChallenge spkac)
+        {
+            m_spkac = spkac ?? throw new ArgumentNullException(nameof(spkac));
+        }
+
+        public AsymmetricKeyParameter GetPublicKey() => PublicKeyFactory.CreateKey(m_spkac.PublicKeyAndChallenge.Spki);
+
+        public bool IsSignatureValid(AsymmetricKeyParameter publicKey) =>
+            CheckSignatureValid(new Asn1VerifierFactory(m_spkac.SignatureAlgorithm, publicKey));
+
+        public bool IsSignatureValid(IVerifierFactoryProvider verifierProvider) =>
+            CheckSignatureValid(verifierProvider.CreateVerifierFactory(m_spkac.SignatureAlgorithm));
+
+        public Asn1.Mozilla.SignedPublicKeyAndChallenge ToAsn1Structure() => m_spkac;
+
+        public void Verify(AsymmetricKeyParameter publicKey) =>
+            CheckSignature(new Asn1VerifierFactory(m_spkac.SignatureAlgorithm, publicKey));
+
+        public void Verify(IVerifierFactoryProvider verifierProvider) =>
+            CheckSignature(verifierProvider.CreateVerifierFactory(m_spkac.SignatureAlgorithm));
+
+        private void CheckSignature(IVerifierFactory verifier)
+        {
+            if (!CheckSignatureValid(verifier))
+                throw new InvalidKeyException("Public key presented not for SPKAC signature");
+        }
+
+        private bool CheckSignatureValid(IVerifierFactory verifier) =>
+            X509.X509Utilities.VerifySignature(verifier, m_spkac.PublicKeyAndChallenge, m_spkac.Signature);
+    }
+}
diff --git a/crypto/test/src/mozilla/test/SpkacTest.cs b/crypto/test/src/mozilla/test/SpkacTest.cs
new file mode 100644
index 000000000..3d7533865
--- /dev/null
+++ b/crypto/test/src/mozilla/test/SpkacTest.cs
@@ -0,0 +1,34 @@
+using NUnit.Framework;
+
+using Org.BouncyCastle.Asn1;
+using Org.BouncyCastle.Utilities;
+using Org.BouncyCastle.Utilities.Encoders;
+
+namespace Org.BouncyCastle.Mozilla.Tests
+{
+    [TestFixture]
+    public class SpkacTest
+    {
+        private static readonly byte[] Spkac = Base64.Decode(
+            "MIIBOjCBpDCBnzANBgkqhkiG9w0BAQEFAAOBjQAwgYkCgYEApne7ti0ibPhV8Iht" +
+            "7Pws5iRckM7x4mtZYxEpeX5/IO8tDsBFdY86ewuY2f2KCca0oMWr43kdkZbPyzf4" +
+            "CSV+0fZm9MJyNMywygZjoOCC+rS8kr0Ef31iHChhYsyejJnjw116Jnn96syhdHY6" +
+            "lVD1rK0nn5ZkHjxU74gjoZu6BJMCAwEAARYAMA0GCSqGSIb3DQEBBAUAA4GBAKFL" +
+            "g/luv0C7gMTI8ZKfFoSyi7Q7kiSQcmSj1WJgT56ouIRJO5NdvB/1n4GNik8VOAU0" +
+            "NRztvGy3ZGqgbSav7lrxcNEvXH+dLbtS97s7yiaozpsOcEHqsBribpLOTRzYa8ci" +
+            "CwkPmIiYqcby11diKLpd+W9RFYNme2v0rrbM2CyV");
+
+        [Test]
+        public void TestSpkac()
+        {
+            var spkac = new SignedPublicKeyAndChallenge(Spkac);
+
+            var reencoded = spkac.ToAsn1Structure().GetEncoded(Asn1Encodable.Der);
+            Assert.True(Arrays.AreEqual(Spkac, reencoded));
+
+            var publicKey = spkac.GetPublicKey();
+            var shouldVerify = spkac.IsSignatureValid(publicKey);
+            Assert.True(shouldVerify);
+        }
+    }
+}