Bug 1443136 - Add support for signature scheme preferences in BoGo r=franziskus,ekr

Reviewers: franziskus, ekr

Reviewed By: franziskus

Bug #: 1443136

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

5 files changed, 76 insertions, 20 deletions

diff --git a/gtests/nss_bogo_shim/config.cc b/gtests/nss_bogo_shim/config.cc
index 2e6f7f775..603bb6029 100644
--- a/gtests/nss_bogo_shim/config.cc
+++ b/gtests/nss_bogo_shim/config.cc
@@ -9,26 +9,37 @@
 #include <queue>
 #include <string>
 
-bool ConfigEntryBase::ParseInternal(std::queue<const char *> *args,
-                                    std::string *out) {
-  if (args->empty()) return false;
-  *out = args->front();
-  args->pop();
+bool ConfigEntryBase::ParseInternal(std::queue<const char *> &args,
+                                    std::vector<int> &out) {
+  if (args.empty()) return false;
+
+  char *endptr;
+  out.push_back(strtol(args.front(), &endptr, 10));
+  args.pop();
+
+  return !*endptr;
+}
+
+bool ConfigEntryBase::ParseInternal(std::queue<const char *> &args,
+                                    std::string &out) {
+  if (args.empty()) return false;
+  out = args.front();
+  args.pop();
   return true;
 }
 
-bool ConfigEntryBase::ParseInternal(std::queue<const char *> *args, int *out) {
-  if (args->empty()) return false;
+bool ConfigEntryBase::ParseInternal(std::queue<const char *> &args, int &out) {
+  if (args.empty()) return false;
 
   char *endptr;
-  *out = strtol(args->front(), &endptr, 10);
-  args->pop();
+  out = strtol(args.front(), &endptr, 10);
+  args.pop();
 
   return !*endptr;
 }
 
-bool ConfigEntryBase::ParseInternal(std::queue<const char *> *args, bool *out) {
-  *out = true;
+bool ConfigEntryBase::ParseInternal(std::queue<const char *> &args, bool &out) {
+  out = true;
   return true;
 }
 
@@ -51,7 +62,7 @@ Config::Status Config::ParseArgs(int argc, char **argv) {
     if (e == entries_.end()) {
       return kUnknownFlag;
     }
-    if (!e->second->Parse(&args)) return kMalformedArgument;
+    if (!e->second->Parse(args)) return kMalformedArgument;
   }
 
   return kOK;
diff --git a/gtests/nss_bogo_shim/config.h b/gtests/nss_bogo_shim/config.h
index 4f4eec403..07a5df2a7 100644
--- a/gtests/nss_bogo_shim/config.h
+++ b/gtests/nss_bogo_shim/config.h
@@ -29,12 +29,13 @@ class ConfigEntryBase {
   virtual ~ConfigEntryBase() {}
 
   const std::string& type() const { return type_; }
-  virtual bool Parse(std::queue<const char*>* args) = 0;
+  virtual bool Parse(std::queue<const char*>& args) = 0;
 
  protected:
-  bool ParseInternal(std::queue<const char*>* args, std::string* out);
-  bool ParseInternal(std::queue<const char*>* args, int* out);
-  bool ParseInternal(std::queue<const char*>* args, bool* out);
+  bool ParseInternal(std::queue<const char*> &args, std::vector<int> &out);
+  bool ParseInternal(std::queue<const char*> &args, std::string &out);
+  bool ParseInternal(std::queue<const char*> &args, int &out);
+  bool ParseInternal(std::queue<const char*> &args, bool &out);
 
   const std::string name_;
   const std::string type_;
@@ -48,8 +49,8 @@ class ConfigEntry : public ConfigEntryBase {
       : ConfigEntryBase(name, typeid(T).name()), value_(init) {}
   T get() const { return value_; }
 
-  bool Parse(std::queue<const char*>* args) {
-    return ParseInternal(args, &value_);
+  bool Parse(std::queue<const char*> &args) {
+    return ParseInternal(args, value_);
   }
 
  private:
diff --git a/gtests/nss_bogo_shim/config.json b/gtests/nss_bogo_shim/config.json
index 667201545..5804f6657 100644
--- a/gtests/nss_bogo_shim/config.json
+++ b/gtests/nss_bogo_shim/config.json
@@ -1,7 +1,9 @@
 {
     "DisabledTests": {
         "### These tests break whenever we rev versions, so just leave them here for easy uncommenting":"",
+        "FilterExtraAlgorithms":"NSS doesn't allow sending unsupported signature algorithms",
         "SendBogusAlertType":"Unexpected TLS alerts should abort connections (Bug 1438263)",
+        "VerifyPreferences-Ed25519":"Add Ed25519 support (Bug 1325335)",
         "Ed25519DefaultDisable*":"Add Ed25519 support (Bug 1325335)",
         "ServerCipherFilter*":"Add Ed25519 support (Bug 1325335)",
         "GarbageCertificate*":"Send bad_certificate alert when certificate parsing fails (Bug 1441565)",
diff --git a/gtests/nss_bogo_shim/nss_bogo_shim.cc b/gtests/nss_bogo_shim/nss_bogo_shim.cc
index 62a2d258e..1e76c5449 100644
--- a/gtests/nss_bogo_shim/nss_bogo_shim.cc
+++ b/gtests/nss_bogo_shim/nss_bogo_shim.cc
@@ -292,6 +292,27 @@ class TestAgent {
       if (rv != SECSuccess) return false;
     }
 
+    // Set supported signature schemes.
+    auto sign_prefs = cfg_.get<std::vector<int>>("signing-prefs");
+    auto verify_prefs = cfg_.get<std::vector<int>>("verify-prefs");
+    if (sign_prefs.empty()) {
+      sign_prefs = verify_prefs;
+    } else if (!verify_prefs.empty()) {
+      return false; // Both shouldn't be set.
+    }
+    if (!sign_prefs.empty()) {
+      std::vector<SSLSignatureScheme> sig_schemes;
+      std::transform(verify_prefs.begin(), verify_prefs.end(),
+                     std::back_inserter(sig_schemes), [](int scheme) {
+                       return static_cast<SSLSignatureScheme>(scheme);
+                     });
+
+      rv = SSL_SignatureSchemePrefSet(
+          ssl_fd_, sig_schemes.data(),
+          static_cast<unsigned int>(sig_schemes.size()));
+      if (rv != SECSuccess) return false;
+    }
+
     if (cfg_.get<bool>("fallback-scsv")) {
       rv = SSL_OptionSet(ssl_fd_, SSL_ENABLE_FALLBACK_SCSV, PR_TRUE);
       if (rv != SECSuccess) return false;
@@ -486,6 +507,24 @@ class TestAgent {
       }
     }
 
+    auto sig_alg = cfg_.get<int>("expect-peer-signature-algorithm");
+    if (sig_alg) {
+      SSLChannelInfo info;
+      rv = SSL_GetChannelInfo(ssl_fd_, &info, sizeof(info));
+      if (rv != SECSuccess) {
+        PRErrorCode err = PR_GetError();
+        std::cerr << "SSL_GetChannelInfo failed with error=" << FormatError(err)
+                  << std::endl;
+        return SECFailure;
+      }
+
+      auto expected = static_cast<SSLSignatureScheme>(sig_alg);
+      if (info.signatureScheme != expected) {
+        std::cerr << "Unexpected signature scheme" << std::endl;
+        return SECFailure;
+      }
+    }
+
     return SECSuccess;
   }
 
@@ -518,6 +557,9 @@ std::unique_ptr<const Config> ReadConfig(int argc, char** argv) {
   cfg->AddEntry<bool>("verify-peer", false);
   cfg->AddEntry<std::string>("advertise-alpn", "");
   cfg->AddEntry<std::string>("expect-alpn", "");
+  cfg->AddEntry<std::vector<int>>("signing-prefs", std::vector<int>());
+  cfg->AddEntry<std::vector<int>>("verify-prefs", std::vector<int>());
+  cfg->AddEntry<int>("expect-peer-signature-algorithm", 0);
 
   auto rv = cfg->ParseArgs(argc, argv);
   switch (rv) {
diff --git a/lib/ssl/ssl3ecc.c b/lib/ssl/ssl3ecc.c
index 913a14f63..071ef7d9a 100644
--- a/lib/ssl/ssl3ecc.c
+++ b/lib/ssl/ssl3ecc.c
@@ -548,12 +548,12 @@ 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) {
-            goto loser; /* malformed or unsupported. */
+            goto alert_loser; /* malformed or unsupported. */
         }
         rv = ssl_CheckSignatureSchemeConsistency(ss, sigScheme,
                                                  ss->sec.peerCert);
         if (rv != SECSuccess) {
-            goto loser;
+            goto alert_loser;
         }
         hashAlg = ssl_SignatureSchemeToHashType(sigScheme);
     } else {