1181683 - Mixed-content Ping (TYPE_PING) and Beacon (TYPE_BEACON) requests should be blocked

Status: RESOLVED FIXED

(Core :: DOM: Security, defect)

Description

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

See http://www.w3.org/TR/mixed-content/#category-optionally-blockable. 'ping' and 'beacon' are not listed as optionally-blockable, so they must always be blocked when they would result in mixed content requests.

Comment 1

[Tanvi Vyas[:tanvi]]

When we implemented MCB, ping was disabled.  Now its enabled and we also have beacon.  cc'ing Richard.  Richard, can we block these by default?

I have telemetry on how much mixed passive content is on the web (~1.4% of all page loads) but don't know how much of that is a result of ping vs beacon vs image vs audio/video.

Comment 2

[Tanvi Vyas[:tanvi]]

Attachment: Bug1181683.patch

When we add new features to the browser, we should make them blockable mixed content instead of optionally blockable.  Since its a new feature, there should be no existing web compat issues.

I'm hoping this won't create too much of an issue for beacon and ping.  Both will likely get blocked without a way for the user to override them though, because they happen when the user clicks a link.  Once the link is clicked, the user is navigating to the next page and will miss the override UI.  Beacon and ping requests will just start failing, which will hurt websites that have mixed content but won't hinder usability.

Our telemetry data shows users hardly use the override anyway.

Comment 3

[Tanvi Vyas[:tanvi]]

Pushed to try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=ffbd7f64289b

Comment 4

[Richard Barnes [:rbarnes]]

This seems like a very good idea.  Given that ping/sendBeacon was a replacement for XHR cases and XHR is blocked as mixed content, it follows that ping/sendBeacon should be MCB'ed.

I would be slightly happier if we had direct telemetry on how much MCB is ping/sendBeacon, but given that this isn't user-visible, I say full speed ahead.

Comment 5

[Christoph Kerschbaumer [:ckerschb]]

Comment on attachment 8640093 [details] [diff] [review]
Bug1181683.patch

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

ship it, r=me

Comment 6

[Tanvi Vyas[:tanvi]]

Try shows that there is a ping test that fails with this change (https://mxr.mozilla.org/mozilla-central/source/dom/html/test/test_anchor_ping.html).  The reason is that the test tries to ping http://localhost from a secure origin.

I've tried updating the test to use a different domain than http://localhost, but the problem is that I need an https domain that supports a port over 1024, which we don't have: mxr.mozilla.org/mozilla-central/source/build/pgo/server-locations.txt

Alternatively, I could make the origin http and run the test; that way http://localhost won't get blocked.  Unfortunately, this causes other problems since the test attempts to test what happens when using an encrypted connection:
https://mxr.mozilla.org/mozilla-central/source/dom/html/test/test_anchor_ping.html?force=1#130

As a last resort, I could just disable mixed content blocking on this test since it requires http://localhost.  I'd much rather find a better work around so our mochitest doesn't have mixed content, but if http://localhost is our only option, then I can do this.

needinfo'ing Tim for his advice on how to proceed with this test.  Thanks!

Comment 7

[Tim Taubert [:ttaubert] (inactive)]

It's quite hard to make this work :( To spawn a proper TLS server we'll have to go through some hoops, like the unit tests in security/manager/ssl do. We could ping a .sjs but then there's no shared state and I wouldn't know how to communicate back that we received the ping, other than dirty tricks like writing to a file and waiting for it to appear, or opening a TCPServerSocket waiting for a connection.

Disabling MCB is a poor solution, but also the easiest. Our test suites lack support for simple TLS servers unfortunately.

Comment 8

[Tanvi Vyas[:tanvi]]

Attachment: Bug1181683-test_anchor_ping.patch

Updated patch to disabled mixed content blocking for the test_anchor_ping mochitest.

Original try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=ffbd7f64289b

New try for just the failing oth test - https://treeherder.mozilla.org/#/jobs?repo=try&revision=4ea36d83f99b

Comment 9

[Tim Taubert [:ttaubert] (inactive)]

Comment on attachment 8642569 [details] [diff] [review]
Bug1181683-test_anchor_ping.patch

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

Might be worth doing it only for the one subtest.

Comment 10

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/00613ac2c5aa
https://hg.mozilla.org/integration/mozilla-inbound/rev/fb5980199e21

Comment 11

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/00613ac2c5aa
https://hg.mozilla.org/mozilla-central/rev/fb5980199e21