In Bug 1723797, we added a pref to only isolate High-Value sites in the mobile platform.
This bug is used to track how we can identify High-Value sites with password-related heuristics.

Right now, the plan is identifying a High-Value site when the site meets any of the following conditions:

1. The site has a saved login in the password manager
2. The user has logged in to the site

For condition (2), it will be done by detecting when users filled-in their username/password in a site and then submit the login form.

Additional notes:

• Login via OAuth is not covered in this bug.

One issue we have to solve in the bug is that geckoview only supports fetching logins async, which means we can’t check whether a site has a saved login synchronously.

As discussed offline with Andrea, if synchronously checking whether a site has a saved login is not possible, one possible approach is checking saved logins BEFORE starting the load and then writing the permission to the permission manager when the result is returned. So when the load happens (ShouldIsolate func in Bug 1723797), the permission might be there. But I guess that also means we can’t guarantee if the result can come back in time.

If we want to support checking logins synchronously, I think we might have to maintain logins either in storage-geckoview.js or in another component that uses geckoview’s API to retrieve the logins (Add a LoginDetectionService, for example). Maintaining the logins in another component has the benefit that we only need to maintain the list when the feature is enabled.
But no matter which solution we take, i guess we’ll need

1. Support getAllLoginsAsync
2. Be notified when a login is added or removed from geckoview.

Hi Agi, I would like to know your opinion on this and if you have any suggestions, really appreciated the help!

In general I would much rather keep checking logins asynchronously, most of GeckoView expects that things are done asynchronously and this would be a big departure from that.

(In reply to Dimi Lee [:dimi] from comment #1)

If we want to support checking logins synchronously, I think we might have to maintain logins either in storage-geckoview.js or in another component that uses geckoview’s API to retrieve the logins (Add a LoginDetectionService, for example). Maintaining the logins in another component has the benefit that we only need to maintain the list when the feature is enabled.
But no matter which solution we take, i guess we’ll need

1. Support getAllLoginsAsync

Maybe I'm missing something but it looks like we do support getAllLoginsAsync?

2. Be notified when a login is added or removed from geckoview.

I would argue that maybe we don't need this? when new logins are added to GeckoView that would normally mean that a user logged in to a website and decided to save the credentials, AFAIK the only other way to add a login is to do it manually, but I would think it's OK if we don't catch manually-added logins for this? They will be caught in the next restart anyway (which happens very frequently on Android).

So my suggestion would be to fetch logins around startup to compile the list of "high value" websites and then ignore the ones added manually until the next session/restart.

(In reply to [PTO until 22nd] Agi Sferro | :agi | ni? for questions | ⏰ PST | he/him from comment #2)

In general I would much rather keep checking logins asynchronously, most of GeckoView expects that things are done asynchronously and this would be a big departure from that.

(In reply to Dimi Lee [:dimi] from comment #1)

If we want to support checking logins synchronously, I think we might have to maintain logins either in storage-geckoview.js or in another component that uses geckoview’s API to retrieve the logins (Add a LoginDetectionService, for example). Maintaining the logins in another component has the benefit that we only need to maintain the list when the feature is enabled.
But no matter which solution we take, i guess we’ll need

1. Support getAllLoginsAsync

Maybe I'm missing something but it looks like we do support getAllLoginsAsync?

I guess not? It looks like the function either returns a empty list or throws Cr.NS_ERROR_NOT_IMPLEMENTED.

2. Be notified when a login is added or removed from geckoview.

I would argue that maybe we don't need this? when new logins are added to GeckoView that would normally mean that a user logged in to a website and decided to save the credentials, AFAIK the only other way to add a login is to do it manually, but I would think it's OK if we don't catch manually-added logins for this? They will be caught in the next restart anyway (which happens very frequently on Android).

That is a good point! I agree that manually adding/removing logins is an edge case that we can ignore. Manually adding/removing logins rarely happens and Restart happens very frequently on Android. As for the case when a user logged in to a website (no matter they choose to save logins or not), theoretically, it will be caught by the form submission heuristic.

So my suggestion would be to fetch logins around startup to compile the list of "high value" websites and then ignore the ones added manually until the next session/restart.

Yes, I agree.

(In reply to Dimi Lee [:dimi] from comment #3)

(In reply to Dimi Lee [:dimi] from comment #1)

If we want to support checking logins synchronously, I think we might have to maintain logins either in storage-geckoview.js or in another component that uses geckoview’s API to retrieve the logins (Add a LoginDetectionService, for example). Maintaining the logins in another component has the benefit that we only need to maintain the list when the feature is enabled.
But no matter which solution we take, i guess we’ll need

1. Support getAllLoginsAsync

Maybe I'm missing something but it looks like we do support getAllLoginsAsync?

I guess not? It looks like the function either returns a empty list or throws Cr.NS_ERROR_NOT_IMPLEMENTED.

You're correct, I misread the code earlier. So yeah we can implement that as part of this bug I guess (leaving the ni open in case I missed something, I'll come back to it after my PTO).

Let me know if you need any more information from me :)

(In reply to Agi Sferro | :agi | ni? for questions | ⏰ PST | he/him from comment #5)

Let me know if you need any more information from me :)

Hi Agi,
Yes, Since the plan is getting all logins while initialization, we'll need geckoview add an API to support fetching all logins.
Could someone in your team work on that?

Redirecting to etoop.

This patch:

1. adds two 'highValueHasSavedLogin' and 'highValueIsLoggedIn' permission
2. moves 'AddHighValuePermission' from HttpBaseChannel to ProcessIsolation
   to support more high-value permission type.

The new 'searchAllLoginsAsync' API works the same as the getAllLoginsAsync API.
The purpose of adding the API is to make C++ caller easier to use.

Depends on D127101

Depends on D127103

Sites that match the following heuristic are considered high-value:

1. Sites that have a password stored in the password manager
2. Sites that we detect that users submit a form with a password

This patch adds LoginDetectionService to detect login attempts.
When LoginDetectionService finds a site that matches any of the above
heuristics, it adds the corresponding high-value permission to the
permission manager.

Depends on D127104

Right now, all high-value permissions have the same expiration time, but
we might want to set difference expiration time for different
high-value permission type in the future.

Depends on D127105

The preference should be enabled before rolling out "fission for authenticated sites"
to makes sure we have the permissions there when the feature is on.

Depends on D127106

Depends on D127107

Backed out for causing Bp-hybrid bustages on LoginDetectionService.o

Backout link: https://hg.mozilla.org/integration/autoland/rev/93d5e38b8aef8a821d0d3ba97b449facec1daee3
Push with failures: https://treeherder.mozilla.org/jobs?repo=autoland&selectedTaskRun=Z52p_5WaQB6_QgmWLvv8kQ.0&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel&revision=4d3532da565253e343ebab4e85a36c8176509e20
Failure log: https://treeherder.mozilla.org/logviewer?job_id=356742049&repo=autoland

https://hg.mozilla.org/mozilla-central/rev/c7c50c85f911
https://hg.mozilla.org/mozilla-central/rev/4ce2eebab0c4
https://hg.mozilla.org/mozilla-central/rev/fbd6928e34ef
https://hg.mozilla.org/mozilla-central/rev/b8fd0343ca42
https://hg.mozilla.org/mozilla-central/rev/5ff8d3adc6eb
https://hg.mozilla.org/mozilla-central/rev/fcd4b833ad73
https://hg.mozilla.org/mozilla-central/rev/5b2a1799ce4e

Backed out for causing permafailures on browser_web_process_isolation.js

• backout: https://hg.mozilla.org/mozilla-central/rev/1f57920fc04062c989c749169a31d147227a2962

• push: https://treeherder.mozilla.org/jobs?repo=autoland&group_state=expanded&selectedTaskRun=EWiijLLIQ_e05TciFNoQog.0&revision=5b2a1799ce4e1a4098eb8420930589872a772c66

• failure log: https://treeherder.mozilla.org/logviewer?job_id=357040694&repo=autoland&lineNumber=32687

[task 2021-11-04T20:29:58.922Z] 20:29:58     INFO - TEST-PASS | dom/ipc/tests/browser_web_process_isolation.js | Frame org_after_isLoggedIn[0][0] has the expected number of children - 
[task 2021-11-04T20:29:58.922Z] 20:29:58     INFO - Leaving test bound test_isolate_high_value
[task 2021-11-04T20:29:58.924Z] 20:29:58     INFO - Buffered messages finished
[task 2021-11-04T20:29:58.925Z] 20:29:58     INFO - TEST-UNEXPECTED-FAIL | dom/ipc/tests/browser_web_process_isolation.js | This test exceeded the timeout threshold. It should be rewritten or split up. If that's not possible, use requestLongerTimeout(N), but only as a last resort. - 
[task 2021-11-04T20:29:58.926Z] 20:29:58     INFO - GECKO(8951) | MEMORY STAT | vsize 3088MB | residentFast 445MB | heapAllocated 192MB
[task 2021-11-04T20:29:58.927Z] 20:29:58     INFO - TEST-OK | dom/ipc/tests/browser_web_process_isolation.js | took 113992ms

Open the test tab takes some time in this testcase
(https://searchfox.org/mozilla-central/rev/d2f8488b6a704443a5c5bfc6d2878171b5f0d393/dom/ipc/tests/browser_web_process_isolation.js#31-43)
, and we open the tab multiple times in this testcase.

Since we add 2 more test scenarios for the testcase, this testcase need more time to finish.

https://hg.mozilla.org/mozilla-central/rev/f8ab662198b7
https://hg.mozilla.org/mozilla-central/rev/c1a757f547b9
https://hg.mozilla.org/mozilla-central/rev/a765b54eec8c
https://hg.mozilla.org/mozilla-central/rev/11e52a262096
https://hg.mozilla.org/mozilla-central/rev/ea5fc06ac247
https://hg.mozilla.org/mozilla-central/rev/c05cc9249ca5
https://hg.mozilla.org/mozilla-central/rev/767daf581e65
https://hg.mozilla.org/mozilla-central/rev/8a0a41762955

Setting status-firefox95=disabled. We don't need to uplift this fix to Beta 95 because Android Fission is not enabled yet.