Some security messages in the web console only show for top level channels

RESOLVED FIXED in Firefox 38

Status

()

RESOLVED FIXED
4 years ago
2 years ago

People

(Reporter: mgoodwin, Assigned: mgoodwin)

Tracking

unspecified
mozilla38
x86_64
Linux
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox38 fixed)

Details

Attachments

(2 attachments, 4 obsolete attachments)

(Assignee)

Description

4 years ago
HPKP and HSTS related security messages appear for the top level loads but not for any other resources.
(Assignee)

Comment 1

4 years ago
STR:
1) Create a resource which sets an invalid HPKP or HSTS header (e.g. an image)
2) Create a document which uses this resource
3) Open the web console
4) Visit the document from step #2
(Assignee)

Comment 2

4 years ago
Created attachment 8514853 [details] [diff] [review]
bug1092055_test.patch

A mochitest to test for successful inclusion of a non-top-level security message (HSTS, in this case)

Updated

4 years ago
Depends on: 1038756

Updated

4 years ago
Component: Developer Tools: Console → General
Product: Firefox → Core
(Assignee)

Updated

4 years ago
Assignee: mgoodwin → nobody
(Assignee)

Updated

4 years ago
Assignee: nobody → mgoodwin
(Assignee)

Comment 3

4 years ago
Created attachment 8561392 [details] [diff] [review]
Bug1092055.patch

Send messages directly to the console so that subresources' security error messages end up in the devtools webconsole for the correct tab.
Attachment #8561392 - Flags: review?(mcmanus)
(Assignee)

Comment 4

4 years ago
Created attachment 8561393 [details] [diff] [review]
bug1092055_test.patch

Devtools mochitest to ensure that security messages are sent to the web console for subresources
Attachment #8514853 - Attachment is obsolete: true
Attachment #8561393 - Flags: review?(past)
Attachment #8561392 - Flags: review?(mcmanus) → review+
Comment on attachment 8561393 [details] [diff] [review]
bug1092055_test.patch

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

::: browser/devtools/webconsole/test/browser_webconsole_show_subresource_security_errors.js
@@ +2,5 @@
> +/* ***** BEGIN LICENSE BLOCK *****
> + * Any copyright is dedicated to the Public Domain.
> + * http://creativecommons.org/publicdomain/zero/1.0/
> + *
> + * ***** END LICENSE BLOCK ***** */

Nit: no need for the BEGIN/END lines.

::: browser/devtools/webconsole/test/test_bug1092055_shouldwarn.html
@@ +3,5 @@
> +<head>
> +  <meta charset="UTF-8">
> +  <title>Bug 1092055 - Log console messages for non-top-level security errors</title>
> +  <script src="test_bug1092055_shouldwarn.js"></script>
> +</head>

Nit: we put license comments even in html files (in the <head>), like this:
<!-- Any copyright is dedicated to the Public Domain.
     http://creativecommons.org/publicdomain/zero/1.0/ -->
Attachment #8561393 - Flags: review?(past) → review+
(Assignee)

Comment 6

4 years ago
Created attachment 8561959 [details] [diff] [review]
bug1092055_test.patch

Nits addressed
Attachment #8561393 - Attachment is obsolete: true
Attachment #8561959 - Flags: review+
(Assignee)

Comment 7

4 years ago
https://treeherder.mozilla.org/#/jobs?repo=try&revision=3e88d62a7bf4 

The Win7 failures seem to be as a result of Bug 1087726: bobowen reports that tests pass with the patches for this bug, but fail with the patches for Bug 1087726 applied.
(Assignee)

Updated

4 years ago
Keywords: checkin-needed
So yeah, let's run this through Try next time to verify that the failures are actually fixed rather than just assuming so.
(Assignee)

Comment 11

4 years ago
Created attachment 8563322 [details] [diff] [review]
Bug1092055_test.patch

Modified browser/devtools/webconsole/test/browser_webconsole_certificate_messages.js to deal with webconsole message leakage on Windows 7.
Attachment #8561959 - Attachment is obsolete: true
(Assignee)

Comment 12

4 years ago
Created attachment 8563337 [details] [diff] [review]
Bug1092055_test.patch

... and trying again, this time with no subresources (waiting for messages-cleared isn't enough to win the race)
Attachment #8563322 - Attachment is obsolete: true
(Assignee)

Comment 13

4 years ago
Comment on attachment 8563337 [details] [diff] [review]
Bug1092055_test.patch

Panos, this is changed only to add a data: favicon in test-certificate-messages.html (to prevent sub-resource loads as a workaround for the warning message leakage discussed earlier).
Attachment #8563337 - Flags: review?(past)
Comment on attachment 8563337 [details] [diff] [review]
Bug1092055_test.patch

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

Yup.
Attachment #8563337 - Flags: review?(past) → review+
(Assignee)

Updated

4 years ago
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/dac58ad1d7f2
https://hg.mozilla.org/mozilla-central/rev/d393df30168d
Status: NEW → RESOLVED
Last Resolved: 4 years ago
status-firefox38: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla38

Updated

2 years ago
Depends on: 1319118
You need to log in before you can comment on or make changes to this bug.