Bug 1454321 - Add tests for invalid ECDH ServerKeyExchange.Signature.algorithm values r=mtNSS_3_37_BETA2

Reviewers: mt

Reviewed By: mt

Bug #: 1454321

Differential Revision: https://phabricator.services.mozilla.com/D947

2 files changed, 48 insertions, 0 deletions

diff --git a/gtests/ssl_gtest/ssl_ecdh_unittest.cc b/gtests/ssl_gtest/ssl_ecdh_unittest.cc
index 480888fc7..12c6e8516 100644
--- a/gtests/ssl_gtest/ssl_ecdh_unittest.cc
+++ b/gtests/ssl_gtest/ssl_ecdh_unittest.cc
@@ -620,6 +620,52 @@ TEST_P(TlsConnectGenericPre13, ConnectUnsupportedPointFormat) {
   client_->CheckErrorCode(SEC_ERROR_UNSUPPORTED_EC_POINT_FORM);
 }
 
+// Replace SignatureAndHashAlgorithm of a SKE.
+class ECCServerKEXSigAlgReplacer : public TlsHandshakeFilter {
+ public:
+  ECCServerKEXSigAlgReplacer(const std::shared_ptr<TlsAgent> &server,
+                             SSLSignatureScheme sig_scheme)
+      : TlsHandshakeFilter(server, {kTlsHandshakeServerKeyExchange}),
+        sig_scheme_(sig_scheme) {}
+
+ protected:
+  virtual PacketFilter::Action FilterHandshake(const HandshakeHeader &header,
+                                               const DataBuffer &input,
+                                               DataBuffer *output) {
+    *output = input;
+
+    uint32_t point_len;
+    EXPECT_TRUE(output->Read(3, 1, &point_len));
+    output->Write(4 + point_len, sig_scheme_, 2);
+
+    return CHANGE;
+  }
+
+ private:
+  SSLSignatureScheme sig_scheme_;
+};
+
+TEST_P(TlsConnectTls12, ConnectUnsupportedSigAlg) {
+  EnsureTlsSetup();
+  client_->DisableAllCiphers();
+  client_->EnableCiphersByKeyExchange(ssl_kea_ecdh);
+
+  MakeTlsFilter<ECCServerKEXSigAlgReplacer>(server_, ssl_sig_none);
+  ConnectExpectAlert(client_, kTlsAlertIllegalParameter);
+  client_->CheckErrorCode(SSL_ERROR_UNSUPPORTED_SIGNATURE_ALGORITHM);
+}
+
+TEST_P(TlsConnectTls12, ConnectIncorrectSigAlg) {
+  EnsureTlsSetup();
+  client_->DisableAllCiphers();
+  client_->EnableCiphersByKeyExchange(ssl_kea_ecdh);
+
+  MakeTlsFilter<ECCServerKEXSigAlgReplacer>(server_,
+                                            ssl_sig_ecdsa_secp256r1_sha256);
+  ConnectExpectAlert(client_, kTlsAlertIllegalParameter);
+  client_->CheckErrorCode(SSL_ERROR_INCORRECT_SIGNATURE_ALGORITHM);
+}
+
 INSTANTIATE_TEST_CASE_P(KeyExchangeTest, TlsKeyExchangeTest,
                         ::testing::Combine(TlsConnectTestBase::kTlsVariantsAll,
                                            TlsConnectTestBase::kTlsV11Plus));
diff --git a/lib/ssl/ssl3ecc.c b/lib/ssl/ssl3ecc.c
index 071ef7d9a..f8b9a9400 100644
--- a/lib/ssl/ssl3ecc.c
+++ b/lib/ssl/ssl3ecc.c
@@ -548,11 +548,13 @@ ssl3_HandleECDHServerKeyExchange(sslSocket *ss, PRUint8 *b, PRUint32 length)
     if (ss->ssl3.prSpec->version == SSL_LIBRARY_VERSION_TLS_1_2) {
         rv = ssl_ConsumeSignatureScheme(ss, &b, &length, &sigScheme);
         if (rv != SECSuccess) {
+            errCode = PORT_GetError();
             goto alert_loser; /* malformed or unsupported. */
         }
         rv = ssl_CheckSignatureSchemeConsistency(ss, sigScheme,
                                                  ss->sec.peerCert);
         if (rv != SECSuccess) {
+            errCode = PORT_GetError();
             goto alert_loser;
         }
         hashAlg = ssl_SignatureSchemeToHashType(sigScheme);