1140638 - Disable <meta referrer> on 37 due to outstanding middle click bug and risk to uplifting its fix

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Sid Stamm [:geekboy or :sstamm]]

The fix for bug 1113431 (flaw in <meta referrer>) hasn't landed yet, and it will be too risky to uplift to 37 this late in the cycle if it does land soon.  We need to disable <meta referrer> handling and ALSO the CSP referrer directive (since it uses the same logic for enforcing referrer policies).

Comment 1

[Sid Stamm [:geekboy or :sstamm]]

bz: would you prefer we take the same "disable it" patch you wrote for bug 1134606 to disable this again, or should I put it behind a pref (or some other way) to disable the feature?

Comment 2

[Boris Zbarsky [:bzbarsky]]

Whatever is simpler, though if we think we'll need it again for 38 a pref might be nice...

Comment 3

[Sid Stamm [:geekboy or :sstamm]]

Attachment: disable_meta

The patch from bug 1134606 applied to beta 37, but required a small (trivial) merge.  This is it after the merge.

Comment 4

[Sid Stamm [:geekboy or :sstamm]]

Attachment: disable csp referrer

Chris, we need to disable the referrer directive in beta 37 (as we discussed).  Does this patch look appropriate?

Comment 5

[Christoph Kerschbaumer [:ckerschb]]

Comment on attachment 8574312 [details] [diff] [review]
disable csp referrer

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

Hey Sid, while your patch definitely serves the purpose of disabling the CSP referrer directive, I would also comment out:
a) referrerDirectiveValue() in the *.h and *.cpp file of the Parser, as well as
b) the only callsite of that method here:
  http://mxr.mozilla.org/mozilla-central/source/dom/security/nsCSPParser.cpp#931

It's up to you, but in case someone browses the code it's not immediately obvious why our CSP implementation does not accept the referrer directive in our CSP implementation.

Comment 6

[Lawrence Mandel [:lmandel] (use needinfo)]

Tracking 37 as we need to disable this feature.

Sid - With the r+ I think we're ready to land for Beta 4 (gtb today). If that's correct, can you please add a Beta approval request to this bug?

Comment 7

[Sid Stamm [:geekboy or :sstamm]]

Attachment: disable csp referrer

Added early return for the csp referrer parsing.

Approval Request Comment
[Feature/regressing bug #]: See comment 0, this is disabling an incomplete feature before it merges to release (removal from beta 37 only).
[User impact if declined]: users will have an incomplete (and possibly problematic) implementation of a feature that should be held back.
[Describe test coverage new/current, TreeHerder]:  https://treeherder.mozilla.org/#/jobs?repo=try&revision=7b342bd33a7b
[Risks and why]: riskier to let the feature merge to release than to disable it like this.
[String/UUID change made/needed]: None.

Comment 8

[Sid Stamm [:geekboy or :sstamm]]

Comment on attachment 8574311 [details] [diff] [review]
disable_meta

Approval Request Comment
[Feature/regressing bug #]: see bug 1113431 and comment 0 (incomplete feature, fix didn't land quick enough)
[User impact if declined]: improper and problematic feature implementation will ship
[Describe test coverage new/current, TreeHerder]: https://treeherder.mozilla.org/#/jobs?repo=try&revision=7b342bd33a7b
[Risks and why]: none significant (this was held back from 36 already)
[String/UUID change made/needed]: none

Note: this patch is a carry over from bug 1134606 and only has minor bitrot, so carrying over r+.

Comment 9

[Sid Stamm [:geekboy or :sstamm]]

Lawrence: please flag checkin-needed or find ckerschb if you want to check this in today; I'm not likely available to do the checkin for the rest of the day.

Comment 10

[Lawrence Mandel [:lmandel] (use needinfo)]

Comment on attachment 8574311 [details] [diff] [review]
disable_meta

As previously discussed with the sec team, we are disabling this feature in 37. Beta+

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/173e1bbcd0b6
https://hg.mozilla.org/releases/mozilla-beta/rev/a982b8eabc42