Bug 1116428 - Part 1: Expose SSLv3 and RC4 warnings in the webconsole actor. r=past
authorSami Jaktholm <sjakthol@outlook.com>
Sat, 24 Jan 2015 14:40:10 +0200
changeset 225996 046c7d482f36f80aed6c992b25a9d1462b74675e
parent 225995 0e5635270d320e2f5a409be7f170267d6d93d3f4
child 225997 06e5cde2c6fc82a5a76bc8eb136df34b1387208b
push id11008
push userryanvm@gmail.com
push dateTue, 27 Jan 2015 15:12:41 +0000
treeherderfx-team@06e5cde2c6fc [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerspast
bugs1116428
milestone38.0a1
Bug 1116428 - Part 1: Expose SSLv3 and RC4 warnings in the webconsole actor. r=past
toolkit/devtools/webconsole/network-helper.js
toolkit/devtools/webconsole/test/unit/test_security-info-state.js
toolkit/devtools/webconsole/test/unit/test_security-info-weakness-reasons.js
toolkit/devtools/webconsole/test/unit/xpcshell.ini
--- a/toolkit/devtools/webconsole/network-helper.js
+++ b/toolkit/devtools/webconsole/network-helper.js
@@ -495,27 +495,31 @@ let NetworkHelper = {
    *        The httpActivity object for the request with at least members
    *        { private, hostname }.
    *
    * @return object
    *         Returns an object containing following members:
    *          - state: The security of the connection used to fetch this
    *                   request. Has one of following string values:
    *                    * "insecure": the connection was not secure (only http)
+   *                    * "weak": the connection has minor security issues
    *                    * "broken": secure connection failed (e.g. expired cert)
    *                    * "secure": the connection was properly secured.
    *          If state == broken:
    *            - errorMessage: full error message from nsITransportSecurityInfo.
    *          If state == secure:
    *            - protocolVersion: one of SSLv3, TLSv1, TLSv1.1, TLSv1.2.
    *            - cipherSuite: the cipher suite used in this connection.
    *            - cert: information about certificate used in this connection.
    *                    See parseCertificateInfo for the contents.
    *            - hsts: true if host uses Strict Transport Security, false otherwise
    *            - hpkp: true if host uses Public Key Pinning, false otherwise
+   *          If state == weak: Same as state == secure and
+   *            - weaknessReasons: list of reasons that cause the request to be
+   *                               considered weak. See getReasonsForWeakness.
    */
   parseSecurityInfo: function NH_parseSecurityInfo(securityInfo, httpActivity) {
     const info = {
       state: "insecure",
     };
 
     // The request did not contain any security info.
     if (!securityInfo) {
@@ -546,30 +550,46 @@ let NetworkHelper = {
      *
      * - request is HTTPS but it uses a weak cipher or old protocol, see
      *   http://hg.mozilla.org/mozilla-central/annotate/def6ed9d1c1a/
      *   security/manager/ssl/src/nsNSSCallbacks.cpp#l1233
      * - request is mixed content (which makes no sense whatsoever)
      *   => .securityState has STATE_IS_BROKEN flag
      *   => .errorCode is NOT an NSS error code
      *   => .errorMessage is not available
-     *      => state === "insecure"
+     *      => state === "weak"
      */
 
     securityInfo.QueryInterface(Ci.nsITransportSecurityInfo);
     securityInfo.QueryInterface(Ci.nsISSLStatusProvider);
 
     const wpl = Ci.nsIWebProgressListener;
     const NSSErrorsService = Cc['@mozilla.org/nss_errors_service;1']
                                .getService(Ci.nsINSSErrorsService);
     const SSLStatus = securityInfo.SSLStatus;
+    if (!NSSErrorsService.isNSSErrorCode(securityInfo.errorCode)) {
+      const state = securityInfo.securityState;
 
-    if (securityInfo.securityState & wpl.STATE_IS_SECURE) {
-      // The connection is secure.
-      info.state = "secure";
+      if (state & wpl.STATE_IS_SECURE) {
+        // The connection is secure.
+        info.state = "secure";
+      } else if (state & wpl.STATE_IS_BROKEN) {
+        // The connection is not secure, there was no error but there's some
+        // minor security issues.
+        info.state = "weak";
+        info.weaknessReasons = this.getReasonsForWeakness(state);
+      } else if (state & wpl.STATE_IS_INSECURE) {
+        // This was most likely an https request that was aborted before
+        // validation. Return info as info.state = insecure.
+        return info;
+      } else {
+        DevToolsUtils.reportException("NetworkHelper.parseSecurityInfo",
+          "Security state " + state + " has no known STATE_IS_* flags.");
+        return info;
+      }
 
       // Cipher suite.
       info.cipherSuite = SSLStatus.cipherName;
 
       // Protocol version.
       info.protocolVersion = this.formatSecurityProtocol(SSLStatus.protocolVersion);
 
       // Certificate.
@@ -593,24 +613,20 @@ let NetworkHelper = {
         info.hpkp = sss.isSecureHost(sss.HEADER_HPKP, host, flags);
       } else {
         DevToolsUtils.reportException("NetworkHelper.parseSecurityInfo",
           "Could not get HSTS/HPKP status as hostname is not available.");
         info.hsts = false;
         info.hpkp = false;
       }
 
-    } else if (NSSErrorsService.isNSSErrorCode(securityInfo.errorCode)) {
+    } else {
       // The connection failed.
       info.state = "broken";
       info.errorMessage = securityInfo.errorMessage;
-    } else {
-      // Connection has securityInfo, it is not secure and there's no problems
-      // to report. Mark the request as insecure.
-      return info;
     }
 
     return info;
   },
 
   /**
    * Takes an nsIX509Cert and returns an object with certificate information.
    *
@@ -678,13 +694,53 @@ let NetworkHelper = {
       case Ci.nsISSLStatus.TLS_VERSION_1_2:
         return "TLSv1.2";
       default:
         DevToolsUtils.reportException("NetworkHelper.formatSecurityProtocol",
           "protocolVersion " + version + " is unknown.");
         return "Unknown";
     }
   },
+
+  /**
+   * Takes the securityState bitfield and returns reasons for weak connection
+   * as an array of strings.
+   *
+   * @param Number state
+   *        nsITransportSecurityInfo.securityState.
+   *
+   * @return Array[String]
+   *         List of weakness reasons. A subset of { cipher, sslv3 } where
+   *         * cipher: The cipher suite is consireded to be weak (RC4).
+   *         * sslv3: The protocol, SSLv3, is weak.
+   */
+  getReasonsForWeakness: function NH_getReasonsForWeakness(state) {
+    const wpl = Ci.nsIWebProgressListener;
+
+    // If there's non-fatal security issues the request has STATE_IS_BROKEN
+    // flag set. See http://hg.mozilla.org/mozilla-central/file/44344099d119
+    // /security/manager/ssl/src/nsNSSCallbacks.cpp#l1233
+    let reasons = [];
+
+    if (state & wpl.STATE_IS_BROKEN) {
+      let isSSLV3 = state & wpl.STATE_USES_SSL_3;
+      let isCipher = state & wpl.STATE_USES_WEAK_CRYPTO;
+      if (isSSLV3) {
+        reasons.push("sslv3");
+      }
+
+      if (isCipher) {
+        reasons.push("cipher");
+      }
+
+      if (!isCipher && !isSSLV3) {
+        DevToolsUtils.reportException("NetworkHelper.getReasonsForWeakness",
+          "STATE_IS_BROKEN without a known reason. Full state was: " + state);
+      }
+    }
+
+    return reasons;
+  },
 };
 
 for (let prop of Object.getOwnPropertyNames(NetworkHelper)) {
   exports[prop] = NetworkHelper[prop];
 }
--- a/toolkit/devtools/webconsole/test/unit/test_security-info-state.js
+++ b/toolkit/devtools/webconsole/test/unit/test_security-info-state.js
@@ -84,17 +84,17 @@ function test_secureSecurityInfo() {
   MockSecurityInfo.securityState = wpl.STATE_IS_SECURE;
 
   let result = NetworkHelper.parseSecurityInfo(MockSecurityInfo, {});
   equal(result.state, "secure",
     "state == 'secure' if securityState contains STATE_IS_SECURE flag");
 }
 
 /**
- * Test that STATE_IS_BROKEN returns "insecure"
+ * Test that STATE_IS_BROKEN returns "weak"
  */
 function test_brokenSecurityInfo() {
   MockSecurityInfo.securityState = wpl.STATE_IS_BROKEN;
 
   let result = NetworkHelper.parseSecurityInfo(MockSecurityInfo, {});
-  equal(result.state, "insecure",
-    "state == 'insecure' if securityState contains STATE_IS_BROKEN flag");
+  equal(result.state, "weak",
+    "state == 'weak' if securityState contains STATE_IS_BROKEN flag");
 }
new file mode 100644
--- /dev/null
+++ b/toolkit/devtools/webconsole/test/unit/test_security-info-weakness-reasons.js
@@ -0,0 +1,55 @@
+/* vim: set ft=javascript ts=2 et sw=2 tw=80: */
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+"use strict";
+
+// Tests that NetworkHelper.getReasonsForWeakness returns correct reasons for
+// weak requests.
+
+const { devtools } = Components.utils.import("resource://gre/modules/devtools/Loader.jsm", {});
+
+Object.defineProperty(this, "NetworkHelper", {
+  get: function() {
+    return devtools.require("devtools/toolkit/webconsole/network-helper");
+  },
+  configurable: true,
+  writeable: false,
+  enumerable: true
+});
+
+const Ci = Components.interfaces;
+const wpl = Ci.nsIWebProgressListener;
+const TEST_CASES = [
+  {
+    description: "weak cipher",
+    input: wpl.STATE_IS_BROKEN | wpl.STATE_USES_WEAK_CRYPTO,
+    expected: ["cipher"]
+  }, {
+    description: "weak sslv3 protocol",
+    input: wpl.STATE_IS_BROKEN | wpl.STATE_USES_SSL_3,
+    expected: ["sslv3"]
+  }, {
+    description: "weak cipher + sslv3",
+    input: wpl.STATE_IS_BROKEN | wpl.STATE_USES_WEAK_CRYPTO | wpl.STATE_USES_SSL_3,
+    expected: ["sslv3", "cipher"] // order matters for deepEqual
+  }, {
+    description: "only STATE_IS_BROKEN flag",
+    input: wpl.STATE_IS_BROKEN,
+    expected: []
+  }, {
+    description: "only STATE_IS_SECURE flag",
+    input: wpl.STATE_IS_SECURE,
+    expected: []
+  },
+];
+
+function run_test() {
+  do_print("Testing NetworkHelper.getReasonsForWeakness.");
+
+  for (let {description, input, expected} of TEST_CASES) {
+    do_print("Testing " + description);
+
+    deepEqual(NetworkHelper.getReasonsForWeakness(input), expected,
+      "Got the expected reasons for weakness.");
+  }
+}
--- a/toolkit/devtools/webconsole/test/unit/xpcshell.ini
+++ b/toolkit/devtools/webconsole/test/unit/xpcshell.ini
@@ -6,8 +6,9 @@ support-files =
 
 [test_js_property_provider.js]
 [test_network_helper.js]
 [test_security-info-certificate.js]
 [test_security-info-parser.js]
 [test_security-info-protocol-version.js]
 [test_security-info-state.js]
 [test_security-info-static-hpkp.js]
+[test_security-info-weakness-reasons.js]