1296004 - Disable bug 1186948 (via pref)

Status: RESOLVED FIXED

(Core Graveyard :: Plug-ins, defect, P1)

Description

[Benjamin Smedberg]

Bug 1186948 is not ready to ship: we're seeing both false-negatives, bug 1294341, as well as false positives, bug 1295984, and I believe the risk of shipping this is high because it could make it difficult for us to release vulnerable plugin blocks.

We're going to keep evaluating and experimenting, but turn this off by default for now.

Comment 1

[Benjamin Smedberg]

Attachment: Bug 1296004 - Disable bug 1186948 via a new pref,

There were no automated tests for this originally, so no test changes are here, but I've verified the correct behavior manually. Definitely need to add some automated tests before this gets flipped on by default again.

Review commit: https://reviewboard.mozilla.org/r/72360/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/72360/

Comment 2

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 8782107 [details]
Bug 1296004 - Disable bug 1186948 via a new pref,

https://reviewboard.mozilla.org/r/72360/#review69958

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/ee6036f333ed
Disable bug 1186948 via a new pref, r=mconley

Comment 4

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/ee6036f333ed

Comment 5

[Mike Conley (:mconley) (:⚙️)]

I believe we want to uplift this, no? Bug 1186948 landed in 50 and rode the trains up.

Comment 6

[Ton [:Tonnes]]

Bug 1186948 also causes the Plugin Check page to fail detecting Flash when set to click-to-play. I assume this is expected but shouldn’t it be considered a security risk?

Comment 7

[Ton [:Tonnes]]

Oops sorry (cached page).

Comment 8

[Benjamin Smedberg]

Comment on attachment 8782107 [details]
Bug 1296004 - Disable bug 1186948 via a new pref,

Approval Request Comment
[Feature/regressing bug #]: bug 1186948
[User impact if declined]: Inability to confidently deploy plugin blocklist; potentially worse experience for users who have Flash marked ask-to-activate.
[Describe test coverage new/current, TreeHerder]: Landed to m-c, manual testing that we have properly reverted to the old behavior
[Risks and why]: Reversion to previous behavior by adding a pref, not reverting the code altogether. Fairly low risk, but not as low as a straight-up backout.
[String/UUID change made/needed]: None

Comment 9

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8782107 [details]
Bug 1296004 - Disable bug 1186948 via a new pref,

Makes sense, Aurora50+

Comment 10

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Hello Wes, just fyi, this was approved 2 days back and hasn't bee uplift to Aurora yet. Thanks!

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/d6be383e84be

Comment 12

[Eric Shepherd [:sheppy]]

Quick note from docs team: this wasn’t marked dev-doc-needed but I found it anyway, which is good. :)

Please remember to add dev-doc-needed to the bug that enables this by default. Thanks!