Validate ServerKeyExchange signature algorithm (TLS 1.2+)

- check the algorithm is in signature_algorithms (or the implicit
defaults if that extension was not sent)
- add (D)TLS test scenarios to cover these checks

9 files changed, 114 insertions, 6 deletions

diff --git a/crypto/src/crypto/tls/AbstractTlsKeyExchange.cs b/crypto/src/crypto/tls/AbstractTlsKeyExchange.cs
index c9ec06107..09fb8782d 100644
--- a/crypto/src/crypto/tls/AbstractTlsKeyExchange.cs
+++ b/crypto/src/crypto/tls/AbstractTlsKeyExchange.cs
@@ -18,6 +18,17 @@ namespace Org.BouncyCastle.Crypto.Tls
             this.mSupportedSignatureAlgorithms = supportedSignatureAlgorithms;
         }
 
+        protected virtual DigitallySigned ParseSignature(Stream input)
+        {
+            DigitallySigned signature = DigitallySigned.Parse(mContext, input);
+            SignatureAndHashAlgorithm signatureAlgorithm = signature.Algorithm;
+            if (signatureAlgorithm != null)
+            {
+                TlsUtilities.VerifySupportedSignatureAlgorithm(mSupportedSignatureAlgorithms, signatureAlgorithm);
+            }
+            return signature;
+        }
+
         public virtual void Init(TlsContext context)
         {
             this.mContext = context;
diff --git a/crypto/src/crypto/tls/TlsDheKeyExchange.cs b/crypto/src/crypto/tls/TlsDheKeyExchange.cs
index 9831e8cd7..cdd629247 100644
--- a/crypto/src/crypto/tls/TlsDheKeyExchange.cs
+++ b/crypto/src/crypto/tls/TlsDheKeyExchange.cs
@@ -71,7 +71,7 @@ namespace Org.BouncyCastle.Crypto.Tls
 
             ServerDHParams dhParams = ServerDHParams.Parse(teeIn);
 
-            DigitallySigned signed_params = DigitallySigned.Parse(mContext, input);
+            DigitallySigned signed_params = ParseSignature(input);
 
             ISigner signer = InitVerifyer(mTlsSigner, signed_params.Algorithm, securityParameters);
             buf.UpdateSigner(signer);
diff --git a/crypto/src/crypto/tls/TlsECDheKeyExchange.cs b/crypto/src/crypto/tls/TlsECDheKeyExchange.cs
index b681aada3..e0553b3f0 100644
--- a/crypto/src/crypto/tls/TlsECDheKeyExchange.cs
+++ b/crypto/src/crypto/tls/TlsECDheKeyExchange.cs
@@ -73,7 +73,7 @@ namespace Org.BouncyCastle.Crypto.Tls
 
             byte[] point = TlsUtilities.ReadOpaque8(teeIn);
 
-            DigitallySigned signed_params = DigitallySigned.Parse(mContext, input);
+            DigitallySigned signed_params = ParseSignature(input);
 
             ISigner signer = InitVerifyer(mTlsSigner, signed_params.Algorithm, securityParameters);
             buf.UpdateSigner(signer);
diff --git a/crypto/src/crypto/tls/TlsSrpKeyExchange.cs b/crypto/src/crypto/tls/TlsSrpKeyExchange.cs
index ce8e4834a..09fa72348 100644
--- a/crypto/src/crypto/tls/TlsSrpKeyExchange.cs
+++ b/crypto/src/crypto/tls/TlsSrpKeyExchange.cs
@@ -189,7 +189,7 @@ namespace Org.BouncyCastle.Crypto.Tls
 
             if (buf != null)
             {
-                DigitallySigned signed_params = DigitallySigned.Parse(mContext, input);
+                DigitallySigned signed_params = ParseSignature(input);
 
                 ISigner signer = InitVerifyer(mTlsSigner, signed_params.Algorithm, securityParameters);
                 buf.UpdateSigner(signer);
diff --git a/crypto/test/src/crypto/tls/test/DtlsTestSuite.cs b/crypto/test/src/crypto/tls/test/DtlsTestSuite.cs
index 69ce36071..e9e4411af 100644
--- a/crypto/test/src/crypto/tls/test/DtlsTestSuite.cs
+++ b/crypto/test/src/crypto/tls/test/DtlsTestSuite.cs
@@ -149,6 +149,35 @@ namespace Org.BouncyCastle.Crypto.Tls.Tests
 
                 AddTestCase(testSuite, c, prefix + "BadMandatoryCertReqDeclined");
             }
+
+            /*
+             * Server selects MD5/RSA for ServerKeyExchange signature, which is not in the default
+             * supported signature algorithms that the client sent. We expect fatal alert from the
+             * client when it verifies the selected algorithm against the supported algorithms.
+             */
+            if (TlsUtilities.IsTlsV12(version))
+            {
+                TlsTestConfig c = CreateDtlsTestConfig(version);
+                c.serverAuthSigAlg = new SignatureAndHashAlgorithm(HashAlgorithm.md5, SignatureAlgorithm.rsa);
+                c.ExpectClientFatalAlert(AlertDescription.illegal_parameter);
+
+                AddTestCase(testSuite, c, prefix + "BadServerKeyExchangeSigAlg");
+            }
+
+            /*
+             * Server selects MD5/RSA for ServerKeyExchange signature, which is not the default {sha1,rsa}
+             * implied by the absent signature_algorithms extension. We expect fatal alert from the
+             * client when it verifies the selected algorithm against the implicit default.
+             */
+            if (TlsUtilities.IsTlsV12(version))
+            {
+                TlsTestConfig c = CreateDtlsTestConfig(version);
+                c.clientSendSignatureAlgorithms = false;
+                c.serverAuthSigAlg = new SignatureAndHashAlgorithm(HashAlgorithm.md5, SignatureAlgorithm.rsa);
+                c.ExpectClientFatalAlert(AlertDescription.illegal_parameter);
+
+                AddTestCaseDebug(testSuite, c, prefix + "BadServerKeyExchangeSigAlg2");
+            }
 #endif
 
             {
@@ -174,6 +203,11 @@ namespace Org.BouncyCastle.Crypto.Tls.Tests
 
         private static void AddTestCase(IList testSuite, TlsTestConfig config, String name)
         {
+            //testSuite.Add(new TestCaseData(config).SetName(name));
+        }
+
+        private static void AddTestCaseDebug(IList testSuite, TlsTestConfig config, String name)
+        {
             testSuite.Add(new TestCaseData(config).SetName(name));
         }
 
diff --git a/crypto/test/src/crypto/tls/test/TlsTestClientImpl.cs b/crypto/test/src/crypto/tls/test/TlsTestClientImpl.cs
index f313f75fe..864a0a5cc 100644
--- a/crypto/test/src/crypto/tls/test/TlsTestClientImpl.cs
+++ b/crypto/test/src/crypto/tls/test/TlsTestClientImpl.cs
@@ -57,6 +57,17 @@ namespace Org.BouncyCastle.Crypto.Tls.Tests
 	        }
         }
 
+        public override IDictionary GetClientExtensions()
+        {
+            IDictionary clientExtensions = base.GetClientExtensions();
+            if (clientExtensions != null && !mConfig.clientSendSignatureAlgorithms)
+            {
+                clientExtensions.Remove(ExtensionType.signature_algorithms);
+                this.mSupportedSignatureAlgorithms = null;
+            }
+            return clientExtensions;
+        }
+
         public override bool IsFallback
         {
             get { return mConfig.clientFallback; }
diff --git a/crypto/test/src/crypto/tls/test/TlsTestConfig.cs b/crypto/test/src/crypto/tls/test/TlsTestConfig.cs
index 212d2b63c..ccbb919d2 100644
--- a/crypto/test/src/crypto/tls/test/TlsTestConfig.cs
+++ b/crypto/test/src/crypto/tls/test/TlsTestConfig.cs
@@ -75,6 +75,17 @@ namespace Org.BouncyCastle.Crypto.Tls.Tests
         public bool clientFallback = false;
 
         /**
+         * Configures whether a (TLS 1.2+) client will send the signature_algorithms extension in ClientHello.
+         */
+        public bool clientSendSignatureAlgorithms = true;
+
+        /**
+         * If not null, and TLS 1.2 or higher is negotiated, selects a fixed signature/hash algorithm to
+         * be used for the ServerKeyExchange signature (if one is sent).
+         */
+        public SignatureAndHashAlgorithm serverAuthSigAlg = null;
+
+        /**
          * Configures whether the test server will send a certificate request.
          */
         public int serverCertReq = SERVER_CERT_REQ_OPTIONAL;
diff --git a/crypto/test/src/crypto/tls/test/TlsTestServerImpl.cs b/crypto/test/src/crypto/tls/test/TlsTestServerImpl.cs
index 6f9d35b5c..9edd2e524 100644
--- a/crypto/test/src/crypto/tls/test/TlsTestServerImpl.cs
+++ b/crypto/test/src/crypto/tls/test/TlsTestServerImpl.cs
@@ -172,15 +172,27 @@ namespace Org.BouncyCastle.Crypto.Tls.Tests
             }
         }
 
+        protected virtual IList GetSupportedSignatureAlgorithms()
+        {
+            if (TlsUtilities.IsTlsV12(mContext) && mConfig.serverAuthSigAlg != null)
+            {
+                IList signatureAlgorithms = new ArrayList(1);
+                signatureAlgorithms.Add(mConfig.serverAuthSigAlg);
+                return signatureAlgorithms;
+            }
+
+            return mSupportedSignatureAlgorithms;
+        }
+
         protected override TlsSignerCredentials GetDsaSignerCredentials()
         {
-            return TlsTestUtilities.LoadSignerCredentials(mContext, mSupportedSignatureAlgorithms, SignatureAlgorithm.dsa,
+            return TlsTestUtilities.LoadSignerCredentials(mContext, GetSupportedSignatureAlgorithms(), SignatureAlgorithm.dsa,
                 "x509-server-dsa.pem", "x509-server-key-dsa.pem");
         }
 
         protected override TlsSignerCredentials GetECDsaSignerCredentials()
         {
-            return TlsTestUtilities.LoadSignerCredentials(mContext, mSupportedSignatureAlgorithms, SignatureAlgorithm.ecdsa,
+            return TlsTestUtilities.LoadSignerCredentials(mContext, GetSupportedSignatureAlgorithms(), SignatureAlgorithm.ecdsa,
                 "x509-server-ecdsa.pem", "x509-server-key-ecdsa.pem");
         }
 
@@ -192,7 +204,7 @@ namespace Org.BouncyCastle.Crypto.Tls.Tests
 
         protected override TlsSignerCredentials GetRsaSignerCredentials()
         {
-            return TlsTestUtilities.LoadSignerCredentials(mContext, mSupportedSignatureAlgorithms, SignatureAlgorithm.rsa,
+            return TlsTestUtilities.LoadSignerCredentials(mContext, GetSupportedSignatureAlgorithms(), SignatureAlgorithm.rsa,
                 "x509-server.pem", "x509-server-key.pem");
         }
 
diff --git a/crypto/test/src/crypto/tls/test/TlsTestSuite.cs b/crypto/test/src/crypto/tls/test/TlsTestSuite.cs
index 0770187d5..77cebe0a6 100644
--- a/crypto/test/src/crypto/tls/test/TlsTestSuite.cs
+++ b/crypto/test/src/crypto/tls/test/TlsTestSuite.cs
@@ -142,6 +142,35 @@ namespace Org.BouncyCastle.Crypto.Tls.Tests
                 AddTestCase(testSuite, c, prefix + "BadMandatoryCertReqDeclined");
             }
 
+            /*
+             * Server selects MD5/RSA for ServerKeyExchange signature, which is not in the default
+             * supported signature algorithms that the client sent. We expect fatal alert from the
+             * client when it verifies the selected algorithm against the supported algorithms.
+             */
+            if (TlsUtilities.IsTlsV12(version))
+            {
+                TlsTestConfig c = CreateTlsTestConfig(version);
+                c.serverAuthSigAlg = new SignatureAndHashAlgorithm(HashAlgorithm.md5, SignatureAlgorithm.rsa);
+                c.ExpectClientFatalAlert(AlertDescription.illegal_parameter);
+
+                AddTestCase(testSuite, c, prefix + "BadServerKeyExchangeSigAlg");
+            }
+
+            /*
+             * Server selects MD5/RSA for ServerKeyExchange signature, which is not the default {sha1,rsa}
+             * implied by the absent signature_algorithms extension. We expect fatal alert from the
+             * client when it verifies the selected algorithm against the implicit default.
+             */
+            if (TlsUtilities.IsTlsV12(version))
+            {
+                TlsTestConfig c = CreateTlsTestConfig(version);
+                c.clientSendSignatureAlgorithms = false;
+                c.serverAuthSigAlg = new SignatureAndHashAlgorithm(HashAlgorithm.md5, SignatureAlgorithm.rsa);
+                c.ExpectClientFatalAlert(AlertDescription.illegal_parameter);
+
+                AddTestCase(testSuite, c, prefix + "BadServerKeyExchangeSigAlg2");
+            }
+
             {
                 TlsTestConfig c = CreateTlsTestConfig(version);
                 c.serverCertReq = C.SERVER_CERT_REQ_NONE;