Closed Bug 1096197 Opened 10 years ago Closed 10 years ago

SSL Error reporting fails when there is no failed cert chain

Categories

(Core :: Security, defect)

Product:
Core
Component:
Security
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
VERIFIED FIXED
Milestone:
mozilla37
Tracking Flags:
Tracking Status
firefox36 --- verified
firefox37 --- fixed

People

(Reporter: mgoodwin, Assigned: mgoodwin)

References

Details

Attachments

(3 files, 6 obsolete files)

ssler_problem.png
65.07 KB, image/png
Details
bug1096197-fix-no-chain-reports.patch
35.57 KB, patch
mgoodwin
: review+
Details | Diff | Splinter Review
bug1096197-fix-no-chain-reports-beta.patch
35.37 KB, patch
mgoodwin
: review+
Sylvestre
: approval-mozilla-beta+
Details | Diff | Splinter Review 
Issue:
SSL Error reporting attempts to create a report containing information on a failed connection if the failed connection is to a PKP pinned host. Sometimes these errors occur in such a way that there is no certificate chain to include in the report. When this happens, report sending fails.

STR:
I don't have a reliable STR yet; I've sen this twice in the wild; once with a misconfigured server that was offering no certificate and once on an inappropriate fallback (which I can't reproduce).

Resolution:
1) SSL Error Reporting should not fail silently
2) SSL Error Reporting should create a report with no chain if no chain is available.
Attached image ssler_problem.png — 
Example error where this issue occurs
This patch fixes SSL error reporting for cases where there is no certificate chain.

Changes made:
1) in browser.js, we no longer fail if the failedCertChain does not exist
2) tests are modified to
 a) reflect the fact that it's not a single-bug test any more
 b) ensure we still send a report when failed cert chain is not set
Attachment #8519933 - Flags: review?(dkeeler)
I spotted nits
Attachment #8519933 - Attachment is obsolete: true
Attachment #8519933 - Flags: review?(dkeeler)
Attachment #8519954 - Flags: review?(dkeeler)
Comment on attachment 8519954 [details] [diff] [review]
bug1096197-fix-no-chain-reports.patch

Review of attachment 8519954 [details] [diff] [review]:
-----------------------------------------------------------------

LGTM, but you should get sign-off from a firefox/browser peer.

::: browser/base/content/test/general/browser_ssl_error_reports.js
@@ +8,5 @@
>  function loadFrameScript() {
>    let mm = Cc["@mozilla.org/globalmessagemanager;1"]
>             .getService(Ci.nsIMessageListenerManager);
>    const ROOT = getRootDirectory(gTestPath);
> +  mm.loadFrameScript(ROOT+"browser_ssl_error_reports_content.js", true);

nit: spaces between operators

::: browser/base/content/test/general/pinning_reports.sjs
@@ +38,5 @@
>      // if all is as expected, send the 201 the client expects
>      response.setStatusLine("1.1", 201, "Created");
>      response.write("<html>OK</html>");
> +    break;
> +  case "nocert":

We should probably ensure that the report had an empty failedCertChain in this case, right?
Attachment #8519954 - Flags: review?(dkeeler) → review+
Feedback addressed with one exception...

(In reply to David Keeler (:keeler) [use needinfo?] from comment #4)
> We should probably ensure that the report had an empty failedCertChain in
> this case, right?

Agreed; I've done this now but we have a small problem in that the 'nocert' option is being completely ignored in server-locations.txt - the documentation says I should see no cert (not even the default) - actually I'm getting the usual chain for example.com

Am I doing something obviously wrong? If not, I'll file a bug...

For now, I've commented out the part of the test that the above causes to fail.
Attachment #8519954 - Attachment is obsolete: true
Flags: needinfo?(dkeeler)
Attachment #8522172 - Flags: review?(felipc)
(In reply to Mark Goodwin [:mgoodwin] from comment #5)
> Am I doing something obviously wrong? If not, I'll file a bug...

I was. Turns out that 'nocert' doesn't mean no cert; there's still a certificate message sent (just no SAN matching the server_name).

I need another way of forcing an error with no failedCertChain using mochitest. Ideas?
It doesn't look like NSS supports running a server without a certificate, so we probably can't easily do this with ssltunnel. However, Running `nc -l 8443 < /dev/random | base64` (netcat, listen on port 8443, send random bytes - the pipe to base64 was so I didn't get binary spew on my console) and visiting https://localhost:8443 resulted in ssl_error_rx_unknown_record_type frequently (and, obviously, there was no certificate chain). To actually turn that into a test, we could probably do some websocket stuff in js and deliberately send an unknown TLS record type.
Flags: needinfo?(dkeeler)
(In reply to David Keeler (:keeler) [use needinfo?] from comment #7)
>  However, Running `nc -l
> 8443 < /dev/random | base64` (netcat, listen on port 8443, send random bytes
> - the pipe to base64 was so I didn't get binary spew on my console) and
> visiting https://localhost:8443 resulted in ssl_error_rx_unknown_record_type
> frequently (and, obviously, there was no certificate chain). To actually
> turn that into a test, we could probably do some websocket stuff in js and
> deliberately send an unknown TLS record type.

Another option could be to add an option to specify some renegotiation (or deliberate error condition) in ssltunnel? (since ssltunnel acts as a proxy, we can likely have it do anything we want).
Changed since last patch:
* Added a new 'fail' option to server_locations to allow a TLS handshake to fail.
* Uncommented the section of the test dependent on a no-chain failure.
* renamed the nocert.includes.... host to avoid dumb matching in mochtest runtests.py

I need to find a reviewer for the mochitest changes... If you have names, please do share.
Attachment #8522172 - Attachment is obsolete: true
Attachment #8522172 - Flags: review?(felipc)
Attachment #8530356 - Flags: review?(felipc)
Attachment #8530356 - Flags: feedback?(dkeeler)
Just saw this new feature the first time, tried to send the report, but the button didn't indicate that anything got sent nor did I see any new connection in the dev tools when I clicked it.

The link that leads to the error page: https://myrta.com/myRego/index.jsp
From here: http://www.rms.nsw.gov.au/ clicking on "Renew my vehicle registration"

Was wondering if that's related to this bug or if my example requires a new one!?
(In reply to Albert Scheiner [:alberts] from comment #11)
> Was wondering if that's related to this bug or if my example requires a new
> one!?

This sounds like the same issue. I don't suppose you made a note of the error code you saw, did you?
Flags: needinfo?(albert)
Error code: ssl_error_no_cypher_overlap

BTW, I emailed them (RTA) as well about the issue.
Flags: needinfo?(albert)
(In reply to Albert Scheiner [:alberts] from comment #13)
> Error code: ssl_error_no_cypher_overlap

Yeah, this will certainly be the same issue. Thanks for getting in touch.
Comment on attachment 8530356 [details] [diff] [review]
bug1096197-fix-no-chain-reports.patch

Review of attachment 8530356 [details] [diff] [review]:
-----------------------------------------------------------------

LGTM. I noted some nits.

::: browser/base/content/test/general/browser_ssl_error_reports.js
@@ +3,4 @@
>  var enabledPref = false;
>  var automaticPref = false;
>  var urlPref = "security.ssl.errorReporting.url";
>  var enforcement_level = 1;

Might be nice to update these to not mix let/var (the rest of the test seems to use let).

@@ +137,5 @@
>    let netError = createNetworkErrorMessagePromise(browser);
>    yield netError;
>    netError.then(function(val){
> +    is(val.startsWith("An error occurred during a connection to"), true,
> +      "ensure the correct error message came from about:neterror");

nit: doesn't look like this is aligned properly

::: browser/base/content/test/general/pinning_reports.sjs
@@ +19,5 @@
> +}
> +
> +function handleRequest(request, response) {
> +  switch (request.queryString) {
> +  case "succeed":

I would add another level of indenting here. If you would rather avoid that, using 'if (request.queryString == "succeed") { ... } else if (...) { ... } else { ... }' is fine.

@@ +53,5 @@
> +    // if all is as expected, send the 201 the client expects
> +    response.setStatusLine("1.1", 201, "Created");
> +    response.write("<html>OK</html>");
> +    break;
> +  case "error":

I would also add "default:" here if you stay with the switch statement.

::: build/pgo/server-locations.txt
@@ +19,5 @@
>  # a port number.  The colon and port number must be present even if the port
>  # number is the default for the protocol.
>  #
>  # Unrecognized options are ignored.  Recognized options are "primary" and
> +# "privileged", "nocert", "cert=some_cert_nickname", "redir=hostname", "fail".

Maybe "failHandshake" instead of just "fail"?

@@ +231,5 @@
>  https://bad.include-subdomains.pinning-dynamic.example.com:443    privileged,cert=dynamicPinningBad
>  
>  # Host for static pin tests
> +https://badchain.include-subdomains.pinning.example.com:443       privileged,cert=staticPinningBad
> +https://fail.include-subdomains.pinning.example.com:443           privileged,fail

I don't think this needs to be pinning-specific, right? We can probably name this host "failHandshake.example.com"

::: testing/mochitest/ssltunnel/ssltunnel.cpp
@@ +747,5 @@
>                  PL_HashTableEnumerateEntries(ci->server_info->host_rc4_table, 
>                                               match_hostname, 
>                                               &match);
> +                PL_HashTableEnumerateEntries(ci->server_info->host_fail_table, 
> +                                             match_hostname, 

nit: trailing whitespace on this line and the one above

@@ +1197,5 @@
>          return 1;
>        }
>  
> +      server.host_fail_table = PL_NewHashTable(0, PL_HashString, PL_CompareStrings,
> +                                              PL_CompareStrings, nullptr, nullptr);;

nit: alignment, and this line ends with two ';'

@@ +1199,5 @@
>  
> +      server.host_fail_table = PL_NewHashTable(0, PL_HashString, PL_CompareStrings,
> +                                              PL_CompareStrings, nullptr, nullptr);;
> +      if (!server.host_fail_table)
> +      {

nit: { goes on the previous line
Attachment #8530356 - Flags: feedback?(dkeeler) → feedback+
Comment on attachment 8530356 [details] [diff] [review]
bug1096197-fix-no-chain-reports.patch

Review of attachment 8530356 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good. Make sure to reply to dkeeler's feedback
Attachment #8530356 - Flags: review?(felipc) → review+
Keeler's feedback addressed.

I guess I need some review from a mochitest person for the runtests/ssltunnel changes?

Joel, I've made some changes to allow tests to force handshake failure for certain tests. This is accomplished by adding a 'failHandshake' option to server-locations, upon which ssltunnel will mess up the handshake (by sending a client hello).

Also, I've noticed an occasional try failure that seems to happen that I don't think is the fault of the changes I've made:
https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=73e4b14db67e

It only ever happens on Android - and then, only on opt builds. I can't reproduce the issue at all with mochitest on my android 4.0.4 device. Do you have any suggestions on how to tackle this (or can you suggest someone who might)?

Also, I'd like to run local mochitests against the binary built for the try run on my device - is there a way of doing this (perhaps by overriding the android app bundle name in some way)?
Attachment #8530356 - Attachment is obsolete: true
Flags: needinfo?(jmaher)
Attachment #8531713 - Flags: review?(jmaher)
Comment on attachment 8531713 [details] [diff] [review]
bug1096197-fix-no-chain-reports.patch

Review of attachment 8531713 [details] [diff] [review]:
-----------------------------------------------------------------

::: testing/mochitest/ssltunnel/ssltunnel.cpp
@@ +328,5 @@
>    return true;
>  }
>  
>  bool ConfigureSSLServerSocket(PRFileDesc* socket, server_info_t* si, const string &certificate,
> +                              const client_auth_option clientAuth, bool ssl3, bool rc4, bool failHandshake)

It would be better to change these boolean params to a single flags parameter rather than adding a parameter whenever a new option is added to ssltunnel.
(Yes, I should have done it from the start in bug 1100917. Sorry for the laziness.)
Comment on attachment 8531713 [details] [diff] [review]
bug1096197-fix-no-chain-reports.patch

Review of attachment 8531713 [details] [diff] [review]:
-----------------------------------------------------------------

overall nothing is standing out as a red flag.

::: browser/base/content/test/general/pinning_reports.sjs
@@ +21,5 @@
> +function handleRequest(request, response) {
> +  switch (request.queryString) {
> +    case "succeed":
> +      report = parseReport(request);
> +      certChain = report.failedCertChain;

is there a need for defining report and certChain with let?
Attachment #8531713 - Flags: review?(jmaher) → review+
Looking at your try push there are 2 issues from the log (http://ftp.mozilla.org/pub/mozilla.org/mobile/try-builds/mgoodwin@mozilla.com-73e4b14db67e/try-android/try_ubuntu64_vm_mobile_test-mochitest-1-bm113-tests1-linux64-build445.txt.gz):
23:55:57     INFO -  runtests.py | SSL tunnel pid: 1790
23:55:57     INFO -  Error: keyword "fail" unexpected
23:55:57     INFO -  Error: config file "/tmp/ssltunnellun4jk.cfg" missing or formating incorrect
23:55:57     INFO -  Specify path to the config file as parameter to ssltunnel or
23:55:57     INFO -  create ssltunnel.cfg in the working directory.
23:55:57     INFO -  
23:55:57     INFO -  Example format of the config file:
23:55:57     INFO -  
23:55:57     INFO -         # Enable http/ssl tunneling proxy-like behavior.
23:55:57     INFO -         # If not specified ssltunnel simply does direct forward.
23:55:57     INFO -         httpproxy:1
23:55:57     INFO -  
23:55:57     INFO -         # Specify path to the certification database used.
23:55:57     INFO -         certdbdir:/path/to/certdb
23:55:57     INFO -  
23:55:57     INFO -         # Forward/proxy all requests in raw to 127.0.0.1:8888.
23:55:57     INFO -         forward:127.0.0.1:8888
23:55:57     INFO -  
23:55:57     INFO -         # Accept connections on port 4443 or 5678 resp. and authenticate
23:55:57     INFO -         # to any host ('*') using the 'server cert' or 'server cert 2' resp.
23:55:57     INFO -         listen:*:4443:server cert
23:55:57     INFO -         listen:*:5678:server cert 2
23:55:57     INFO -  
23:55:57     INFO -         # Accept connections on port 4443 and authenticate using
23:55:57     INFO -         # 'a different cert' when target host is 'my.host.name:443'.
23:55:57     INFO -         # This only works in httpproxy mode and has higher priority
23:55:57     INFO -         # than the previous option.
23:55:57     INFO -         listen:my.host.name:443:4443:a different cert
23:55:57     INFO -  
23:55:57     INFO -         # To make a specific host require or just request a client certificate
23:55:57     INFO -         # to authenticate use the following options. This can only be used
23:55:57     INFO -         # in httpproxy mode and only after the 'listen' option has been
23:55:57     INFO -         # specified. You also have to specify the tunnel listen port.
23:55:57     INFO -         clientauth:requesting-client-cert.host.com:443:4443:request
23:55:57     INFO -         clientauth:requiring-client-cert.host.com:443:4443:require
23:55:57     INFO -         # Proxy WebSocket traffic to the server at 127.0.0.1:9999,
23:55:57     INFO -         # instead of the server specified in the 'forward' option.
23:55:57     INFO -         websocketserver:127.0.0.1:9999


This indicates that ssltunnel is not running.  The second error is related to a file not loading:
23:55:57     INFO -  220 INFO TEST-OK | /tests/dom/base/test/csp/test_CSP_bug941404.html | took 4850ms
23:55:57     INFO -  *** error running SJS at /builds/slave/test/build/tests/mochitest/tests/dom/base/test/csp/file_csp_testserver.sjs: [Exception... "Component returned failure code: 0x80520012 (NS_ERROR_FILE_NOT_FOUND) [nsIFileInputStream.init]"  nsresult: "0x80520012 (NS_ERROR_FILE_NOT_FOUND)"  location: "JS frame :: /builds/slave/test/build/hostutils/bin/components/httpd.js :: loadHTMLFromFile :: line 2746"  data: no] on line 20
23:55:57     INFO -  *** error running SJS at /builds/slave/test/build/tests/mochitest/tests/dom/base/test/csp/file_csp_testserver.sjs: [Exception... "Component returned failure code: 0x80520012 (NS_ERROR_FILE_NOT_FOUND) [nsIFileInputStream.init]"  nsresult: "0x80520012 (NS_ERROR_FILE_NOT_FOUND)"  location: "JS frame :: /builds/slave/test/build/hostutils/bin/components/httpd.js :: loadHTMLFromFile :: line 2746"  data: no] on line 20
23:55:57     INFO -  221 INFO TEST-START | /tests/dom/base/test/csp/test_CSP_evalscript.html
23:55:57     INFO -  222 INFO TEST-OK | /tests/dom/base/test/csp/test_CSP_evalscript.html | took 5930ms
23:55:57     INFO -  223 INFO TEST-START | /tests/dom/base/test/csp/test_CSP_inlinescript.html
23:55:57     INFO -  224 INFO TEST-OK | /tests/dom/base/test/csp/test_CSP_inlinescript.html | took 5054ms
23:55:57     INFO -  225 INFO TEST-START | /tests/dom/base/test/csp/test_CSP_inlinestyle.html
23:55:57     INFO -  226 INFO TEST-OK | /tests/dom/base/test/csp/test_CSP_inlinestyle.html | took 6035ms
23:55:57     INFO -  227 INFO TEST-START | /tests/dom/base/test/csp/test_base-uri.html
23:55:57     INFO -  228 INFO TEST-OK | /tests/dom/base/test/csp/test_base-uri.html | took 4861ms
23:55:57     INFO -  229 INFO TEST-START | /tests/dom/base/test/csp/test_bug836922_npolicies.html
23:55:57     INFO -  230 INFO TEST-OK | /tests/dom/base/test/csp/test_bug836922_npolicies.html | took 5541ms
23:55:57     INFO -  231 INFO TEST-START | /tests/dom/base/test/csp/test_bug886164.html
23:55:57     INFO -  232 INFO TEST-OK | /tests/dom/base/test/csp/test_bug886164.html | took 9724ms
23:55:57     INFO -  233 INFO TEST-START | /tests/dom/base/test/csp/test_bug949549.html
23:55:57     INFO -  234 INFO TEST-OK | /tests/dom/base/test/csp/test_bug949549.html | took 5572ms
23:55:57     INFO -  235 INFO TEST-START | /tests/dom/base/test/csp/test_connect-src.html
23:55:57     INFO -  236 INFO TEST-OK | /tests/dom/base/test/csp/test_connect-src.html | took 5166ms
23:55:57     INFO -  237 INFO TEST-START | /tests/dom/base/test/csp/test_csp_allow_https_schemes.html
23:55:57     INFO -  238 INFO TEST-UNEXPECTED-FAIL | /tests/dom/base/test/csp/test_csp_allow_https_schemes.html | should be allowed in test 0! - got blocked, expected allowed
23:55:57     INFO -  239 INFO TEST-UNEXPECTED-FAIL | /tests/dom/base/test/csp/test_csp_allow_https_schemes.html | should be allowed in test 1! - got blocked, expected allowed
23:55:57     INFO -  240 INFO TEST-OK | /tests/dom/base/test/csp/test_csp_allow_https_schemes.html | took 6611ms
23:55:57     INFO -  241 INFO TEST-START | /tests/dom/base/test/csp/test_csp_invalid_source_expression.html

the failing test depends on the file that appears to not be loading.  I did verify in the tests.zip from the try push that the file does exist in the right place!

The question then becomes- why is this failing on android2.3.  I have included gbrown to maybe help shed some light on what might be different about android2.3 vs android 4.0.
Flags: needinfo?(jmaher) → needinfo?(gbrown)
(In reply to Joel Maher (:jmaher) from comment #20)
> Looking at your try push there are 2 issues from the log
<snip/>
> 23:55:57     INFO -  Error: keyword "fail" unexpected

http://dxr.mozilla.org/mozilla-central/source/testing/mochitest/ssltunnel/ssltunnel.cpp#1308 <- this indicates the wrong version of the ssltunnel binary is sometimes being used.

> the failing test depends on the file that appears to not be loading.  I did
> verify in the tests.zip from the try push that the file does exist in the
> right place!
> 
> The question then becomes- why is this failing on android2.3.  I have
> included gbrown to maybe help shed some light on what might be different
> about android2.3 vs android 4.0.

Perhaps this is the same issue; in both cases there is a mismatch between what should be there and what's seen on the filesystem at runtime?

Note, I have seen this same issue on a different run on 4.0. I have only seen it for opt builds though, which seems weird.

(In reply to Masatoshi Kimura [:emk] from comment #18)
> It would be better to change these boolean params to a single flags
> parameter rather than adding a parameter whenever a new option is added to
> ssltunnel.
> (Yes, I should have done it from the start in bug 1100917. Sorry for the
> laziness.)

OK, I'll do this.
(In reply to Mark Goodwin [:mgoodwin] from comment #21)
> http://dxr.mozilla.org/mozilla-central/source/testing/mochitest/ssltunnel/
> ssltunnel.cpp#1308 <- this indicates the wrong version of the ssltunnel
> binary is sometimes being used.

I wonder if the ssltunnel binary in the tests zip is being confused with the ssltunnel binary in tegra-host-utils. We download both the tests zip and tegra-host-utils on both Android 2.3 and Android 4.0.

I downloaded the tests zip from comment 20. I could not download tegra-host-utils, but I have an old copy on my laptop:

gbrown@mozpad:~/Downloads$ unzip -l fennec-37.0a1.en-US.android-arm.tests.zip | grep ssltunnel
   215732  2014-12-01 04:07   bin/ssltunnel
gbrown@mozpad:~/Downloads$ unzip -l ../arch/tegra-host-utils.Linux.742597.zip | grep ssltunnel
   705481  2012-07-17 04:45   bin/ssltunnel

I don't know how these are getting confused exactly.

I can't think of why this would be different (or sometimes different) between 2.3 and 4.0.
Flags: needinfo?(gbrown)
The archives are unzipped to different locations.

23:07:47     INFO - Running command: ['unzip', '-q', '-o', '/builds/slave/test/build/fennec-37.0a1.en-US.android-arm.tests.zip'] in /builds/slave/test/build/tests

23:08:35     INFO - Running command: ['unzip', '-q', '-o', '/builds/slave/test/build/tegra-host-utils.Linux.742597.zip'] in /builds/slave/test/build/hostutils

runtests.py launches ssltunnel from --utility-path, which differs slightly between 2.3 and 4.0: 

--utility-path=/builds/slave/test/build/hostutils/bin (2.3)

--utility-path=../hostutils/bin (4.0)  (in /builds/panda-0042/test/build/tests)
(In reply to Joel Maher (:jmaher) from comment #19)
> Comment on attachment 8531713 [details] [diff] [review]
> is there a need for defining report and certChain with let?

Addressed...

I've also changed the boolean params to a flags param as per comment 18 (thus my requesting review again).

From the information you pointed out in comment 20, I'm confident the try failure is not something I'm causing; what's the correct course of action? File a new bug to sort out the file sync issue?
Attachment #8531713 - Attachment is obsolete: true
Attachment #8532184 - Flags: review?(jmaher)
Comment on attachment 8532184 [details] [diff] [review]
bug1096197-fix-no-chain-reports.patch

Review of attachment 8532184 [details] [diff] [review]:
-----------------------------------------------------------------

this looks great.  As for the hostutils, please file a bug in release engineering :: automation, and cc :Callek, :gbrown, and myself.

I am not sure how to solve this as it is a chicken and egg problem, although I recall solving it before.  I think we pulled the binaries from a try push and then created the hostutils.zip.  Finally once that is tested in deployment, then we landed the change.  Beware if this change is not backwards compatible as this will affect multiple branches.
Attachment #8532184 - Flags: review?(jmaher) → review+
A try to a) demonstrate lack of breakage and b) obtain a binary for hostutils.zip

https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=d32dce6b7840
Depends on: 1109310
No longer depends on: 1109310
Mark, now you should be able to land without waiting for bug 1109310 because of my workaround.
Updated so patch applies cleanly.

Try run: https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=e13cc8d20ffe
Attachment #8532184 - Attachment is obsolete: true
Attachment #8545172 - Flags: review+
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/4f547cd4ce3d
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla37
Blocks: 1112399
Mark, could you request uplift to Firefox 36? This is needed to uplift bug 1112399.
Flags: needinfo?(mgoodwin)
(In reply to Masatoshi Kimura [:emk] from comment #31)
> Mark, could you request uplift to Firefox 36? This is needed to uplift bug
> 1112399.

Sure
Flags: needinfo?(mgoodwin)
Attachment #8551960 - Flags: review+
Comment on attachment 8551960 [details] [diff] [review]
bug1096197-fix-no-chain-reports-beta.patch

Approval Request Comment
[Feature/regressing bug #]: bug 1096197
[User impact if declined]: ssl error reports will fail in some cases (the user will click the 'report' button, nothing will happen).
[Describe test coverage new/current, TreeHerder]: Updates to the relevant mochitest included
[Risks and why]:
Very small risk to functionality (only affecting about:neterror) - defects here should only affect the area fixed. There's a slightly bigger risk to tests due to a change to ssltunnel needed for the tests to work. 
[String/UUID change made/needed]: No
Attachment #8551960 - Flags: approval-mozilla-beta?
Attachment #8551960 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Keywords: checkin-needed
Removing "qawanted" and setting "qe-verify-" since this seems to be covered automatically. Please set it back if you think manual QA is needed, and provide testing details.
Flags: qe-verify-
Keywords: qawanted
Using David's steps in comment 7, I was able to see the proper SSL error page, with reporting. Marking verified based on this.
Status: RESOLVED → VERIFIED
status-firefox36: fixed → verified
Depends on: 1135339
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: