1714614 - CSS Visited Styling design results in excessive notifications to content processes

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect)

Description

[Tom Ritter [:tjr] (OOTO until 4/30 at least)]

Content Processes need to know how they should style links (i.e. whether they're visited or not) - so they get the ability to query the Parent Process and ask if any link should be styled as visited or not. (My understanding is the information they receive through this API is limited to that single bit.)

But, to support a page styling links as visited immediately when a user clicks a link, a Content Process B that navigates from b.com to foo.com will result in a NotifyVisited message being delivered to unrelated Content Process A indicating that foo.com should be styled as visited.

Additionally, when Content Process A asks the Parent Process which of a list of links should be styled as visited; the response is sent not only to Content Process A but all other Content Processes as well.

It's clear that absent a re-architecture of how Visited links work on the web (e.g. Bug 1398414 ) Content Processes need to know what links are visited, so the ability to query for links arbitrary cannot be removed.

Instantaneous notifications about navigations are concerning, as it allows a (compromised/Spectre-attacked) content process to watch user behavior cross-origin. There isn't an easy fix for this; however, as this is needed to update the style of links immediately after a navigation! (One fix could let a Content Process 'register' a list of URLs it wants to receive notifications about in the Parent...)

Broadcasting the reply for one content process to all other processes is concerning because it could - in theory - allow a (compromised/Spectre attacked) Content Process to identify what cross-origin pages a user is loading based on their link contents. Seems like an exceptionally hard attack to make work in an open world though. However, the broadcast-the-reply-everywhere behavior does not seem necessary and seems like it potentially could be removed with no adverse affects?

Backing up; while it is in theory possible for a Spectre attacker to monitor incoming IPC messages, in practice it seems unlikely and practically impossible if that IPC message is not intentionally preserved and just processed and discarded. So the impact of these messages/behaviors would be significantly reduced if the Content Process discarded them if they were unneeded vs storing the received data for future use - which does seem to be the case.

Comment 1

[Emilio Cobos Álvarez (:emilio)]

Yeah I can look into making the query reply more targeted.

Comment 2

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1714614 - Remove unused members in GeckoViewHistory. r=#geckoview-reviewers

This is just preliminar cleanup.

Comment 3

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1714614 - Avoid some copies of URI arrays in GeckoViewHistory. r=#geckoview-reviewers

Also preliminar cleanup.

Depends on D117177

Comment 4

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1714614 - Send visited query results only to interested processes. r=mak,#geckoview-reviewers,agi

Depends on D117178

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b7b16e881fcf
Remove unused members in GeckoViewHistory. r=geckoview-reviewers,agi
https://hg.mozilla.org/integration/autoland/rev/c3e9ba0b5d48
Avoid some copies of URI arrays in GeckoViewHistory. r=geckoview-reviewers,agi

Comment 6

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/b7b16e881fcf
https://hg.mozilla.org/mozilla-central/rev/c3e9ba0b5d48

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1e76e5ecdfbe
Send visited query results only to interested processes. r=mak,geckoview-reviewers,agi

Comment 8

[Alexandru Michis [:malexandru]]

Backed out changeset 1e76e5ecdfbe (Bug 1714614) for causing gtest and gv-junit failures.
Backout link: https://hg.mozilla.org/integration/autoland/rev/233861a72bda2f005dd3ce1d66d82d19a6881e96

• Push with failures: gv-junit, gtest.
• Failure logs: gv-junit, gtest.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ff428915f522
Send visited query results only to interested processes. r=mak,geckoview-reviewers,agi

Comment 10

[Andreea Pavel [:apavel]]

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