STR:

1. use the location bar to open https://shhnjk.azurewebsites.net/SameSite.php from the location bar
2. close tab, open new tab
3. use the location bar to open https://shhnjk.azurewebsites.net/SameSite.php a second time (may need to clear cache / force refresh, without clearing cookies)

ER:
we send both samesite=strict and samesite=lax cookies

AR:
(I haven't tried repro'ing myself, but based on what I'm seeing reported)
We do not send samesite=strict cookies. Unsure about samesite=lax cookies.

(earlier/related discussion in bug 1522829 )

AIUI this is not a security bug, just a correctness issue - we send fewer cookies than expected.

Andrea, does this look like a Gecko cookie bug? GeckoView isn't doing anything that ought to interfere with cookies. snorp wondered if the fact Fenix/GeckoView tabs are treated separate "windows" (IIUC) could cause a problem.

(In reply to Chris Peterson [:cpeterson] from comment #1)

Andrea, does this look like a Gecko cookie bug? GeckoView isn't doing anything that ought to interfere with cookies. snorp wondered if the fact Fenix/GeckoView tabs are treated separate "windows" (IIUC) could cause a problem.

I can't speak for baku, but FWIW, in bug 1522829, snorp said:

When you enter an address through the location bar, csp isn't in play and the triggering principal is a null one.

and samesite=strict cookies work off the triggering/loading principal, AIUI - I expect it needs to be the principal of the site, or system, for us to send samesite=strict cookies. Viz:

https://searchfox.org/mozilla-central/rev/3a61fb322f74a0396878468e50e4f4e97e369825/netwerk/base/nsNetUtil.cpp#2049-2054,2066-2069
and
https://searchfox.org/mozilla-central/rev/3a61fb322f74a0396878468e50e4f4e97e369825/netwerk/cookie/CookieServiceChild.cpp#328-332

I know desktop uses system principal for URL bar loads; I expect fennec does the same. So I actually suspect this is a geckoview problem.

I can't speak for baku, but FWIW, in bug 1522829, snorp said:

Can you give me access to this bug?

https://searchfox.org/mozilla-central/rev/3a61fb322f74a0396878468e50e4f4e97e369825/netwerk/cookie/CookieServiceChild.cpp#328-332

This is right. Which principal is used by geckoView?

(In reply to Andrea Marchesini [:baku] from comment #3)

I can't speak for baku, but FWIW, in bug 1522829, snorp said:

Can you give me access to this bug?

Done.

https://searchfox.org/mozilla-central/rev/3a61fb322f74a0396878468e50e4f4e97e369825/netwerk/cookie/CookieServiceChild.cpp#328-332

This is right. Which principal is used by geckoView?

:snorp seemed to suggest null principal. I haven't checked this myself, mostly because I don't know where to look...

(In reply to :Gijs (he/him) from comment #4)

This is right. Which principal is used by geckoView?

:snorp seemed to suggest null principal. I haven't checked this myself, mostly because I don't know where to look...

https://searchfox.org/mozilla-central/source/mobile/android/modules/geckoview/GeckoViewNavigation.jsm#185

:snorp seemed to suggest null principal. I haven't checked this myself, mostly because I don't know where to look...

https://searchfox.org/mozilla-central/source/mobile/android/modules/geckoview/GeckoViewNavigation.jsm#185

So GeckoView is indeed using a null principal. jchen made that change last year (in bug 1496220):

For improved security, default to a null triggering principal for
GeckoView.loadUri calls, except when loading certain privileged schemes
such as "resource" and "file".

Is there any reason GeckoView should not use the system principal like desktop does?

This was reviewed pretty recently by Christoph and I think we determined null was ok, but we can do whatever the Right Thing is. Thomas, maybe you have an opinion here since Christoph is out? Should we be loading user-specified URLs with the system principal?

I think, using null principal as a fallback in the code makes sense. However, the URL bar loads also could use system principal because the user has granted the action explicitly and not from a page. It would be better that we could find the url bar loads call and pass system principal explicitly.

Using SystemPrincipal as a fallback is another option. I think it's not nice, though we prevent from inheriting the system principal in most of the unappropriate cases.

Moving to GeckoView.

Setting priority P2 so the GV team will consider this bug for our September sprint. Omitting some cookies (in this case SameSite=strict) on a user-initiated page load sounds like it could cause problems.

(In reply to Chris Peterson [:cpeterson] from comment #10)

Setting priority P2 so the GV team will consider this bug for our September sprint. Omitting some cookies (in this case SameSite=strict) on a user-initiated page load sounds like it could cause problems.

Is this still on the GV team's radar (or did it perhaps get fixed in the interim) ?

(In reply to :Gijs (he/him) from comment #11)

(In reply to Chris Peterson [:cpeterson] from comment #10)

Setting priority P2 so the GV team will consider this bug for our September sprint. Omitting some cookies (in this case SameSite=strict) on a user-initiated page load sounds like it could cause problems.

Is this still on the GV team's radar (or did it perhaps get fixed in the interim) ?

Redirectly needinfo to James.

Baku, Christoph -- this appears to be a similar issue as Bug 1626335, but with the null principal instead of the system one. Should we make a similar change for the null principal? Or should GV use the system principal for loads initiated by the user? A content principal?

(In reply to James Willcox (:snorp) (jwillcox@mozilla.com) (he/him) from comment #13)

Baku, Christoph -- this appears to be a similar issue as Bug 1626335, but with the null principal instead of the system one. Should we make a similar change for the null principal? Or should GV use the system principal for loads initiated by the user? A content principal?

If the load is truly triggered by the user (e.g. manually entering the URL), then using systemPrincipal is correct, this is what we use for Desktop Firefox. If we share codepaths, meaning that also not user triggered loads get a systemPrincipal then we should rather consider something else, maybe similar than what we did in Bug 1626335.

(In reply to Christoph Kerschbaumer [:ckerschb] from comment #14)

(In reply to James Willcox (:snorp) (jwillcox@mozilla.com) (he/him) from comment #13)

Baku, Christoph -- this appears to be a similar issue as Bug 1626335, but with the null principal instead of the system one. Should we make a similar change for the null principal? Or should GV use the system principal for loads initiated by the user? A content principal?

If the load is truly triggered by the user (e.g. manually entering the URL), then using systemPrincipal is correct, this is what we use for Desktop Firefox. If we share codepaths, meaning that also not user triggered loads get a systemPrincipal then we should rather consider something else, maybe similar than what we did in Bug 1626335.

Do we know why didn't Bug 1626335 affect desktop wrt third-party cookies if it uses the system principal?

The place where we currently use the null principal is used for any load that the app directs itself. In practice this means two places

1. URLs entered by the user
2. URLs passed to the browser by another app (like when you click a link in gmail and it opens Fenix).

To me these seem equivalent, but maybe not? If you agree, I'll just switch GV to use the system principal for loads initiated by the app.

(In reply to James Willcox (:snorp) (jwillcox@mozilla.com) (he/him) from comment #15)

Do we know why didn't Bug 1626335 affect desktop wrt third-party cookies if it uses the system principal?

That is a very fair question, and in the short time available today I was not able to find an answer; in my opinion Desktop should also have the same problem with same-site cookies because it uses triggeringPrincipal->IsThirdPartyChannel(aChannel, &isForeign);. Maybe Baku knows off hand.

The place where we currently use the null principal is used for any load that the app directs itself. In practice this means two places

1. URLs entered by the user
2. URLs passed to the browser by another app (like when you click a link in gmail and it opens Fenix).
   To me these seem equivalent, but maybe not? If you agree, I'll just switch GV to use the system principal for loads initiated by the app.

For (1) that's toally fine, for (2) it's more of a risk because if I can trick you into clicking a link under my control (potentially a download an attacker can guess) then we might face security problems again.

Let's wait for the answer from baku for the previous question, based on that we can decide how to move forward.

What is (still) not clear to me is, what principal should we use when

1. loading a bookmark
2. restoring a tab for which we don't have state saved (so we need to call loadUri)
3. loading a url from history
4. loading a search url from search terms

do/should all of these get a systemPrincipal? ckerschb, do you have opinions?

if a system principal is appropriate for this, we could use a systemPrincipal whenever we don't have LOAD_FLAGS_EXTERNAL.

somewhat on a tangent, I think we should treat page loads coming from external apps more similarly to page loads triggered by a content website rather than coming from the user.

Alternatively we could add a LOAD_FLAGS_USER_INPUT or whatever, but I'm not sure what it would mean to have LOAD_FLAGS_EXTERNAL=0 and LOAD_FLAGS_USER_INPUT=0 at the same time (and also what it means to have USER_INPUT=1 and EXTERNAL=1 at the same time)

This is my understanding from desktop -- hopefully Christoph can simply confirm this tomorrow (and if any of it is wrong, desktop probably needs fixing, too!):

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

What is (still) not clear to me is, what principal should we use when

1. loading a bookmark

system (user interaction, url isn't provided by web content)

2. restoring a tab for which we don't have state saved (so we need to call loadUri)

Desktop stores session restore information including triggering principal, principal to inherit, and the partitioned principal to inherit, as serialized base64 data using a base64 version of Services.scriptSecurityManager.principalToJSON called on the relevant principal (which it fetches off the session history entry object, cf. https://searchfox.org/mozilla-central/rev/7067bbd8194f4346ec59d77c33cd88f06763e090/docshell/shistory/nsISHEntry.idl#160-177 ). So it uses the stored principal. This is the best way to guarantee that loading the same page a second time gets the same server response etc.

I'm not sure in what context we're talking about a "tab for which there's no state saved" -- but if you don't have a principal stored, you could use system principal provided you're not going to be trying to run a javascript: or data: URI with a system triggering principal. The next best thing would be a content principal for the URI you're loading.

3. loading a url from history

For session history (which is kept by the browser/nsIWebNavigation) you can use the goBack and goForward and goToIndex and similar APIs, and they don't need a triggering principal (which I assume is kept internal to the session history entry in docshell land somewhere)

If you mean, from a list of previously visited URL+title combinations in a UI somewhere that the user is picking from, then system principal (again, user interaction, url isn't provided by web content).

See previous question for session-restore type functionality.

4. loading a search url from search terms

System (again, user interaction, url isn't provided by web content)

if a system principal is appropriate for this, we could use a systemPrincipal whenever we don't have LOAD_FLAGS_EXTERNAL.

I am confused by this because loads from link clicks (or long tap + open in new [private] tab) on other websites shouldn't have LOAD_FLAGS_EXTERNAL and those shouldn't get system principal, either...

somewhat on a tangent, I think we should treat page loads coming from external apps more similarly to page loads triggered by a content website rather than coming from the user.

This is interesting and I would like to discuss this in more detail but I don't know that this bug is the best place for this.

(In reply to :Gijs (he/him) from comment #20)

Desktop stores session restore information including triggering principal, principal to inherit, and the partitioned principal to inherit, as serialized base64 data using a base64 version of Services.scriptSecurityManager.principalToJSON called on the relevant principal (which it fetches off the session history entry object, cf. https://searchfox.org/mozilla-central/rev/7067bbd8194f4346ec59d77c33cd88f06763e090/docshell/shistory/nsISHEntry.idl#160-177 ). So it uses the stored principal. This is the best way to guarantee that loading the same page a second time gets the same server response etc.

I believe we do that too. Sometimes however the app ends up without session restore information (apps can be killed at any time so data is not necessarily consistent), in that case the app would just call loadUri with the last URI saved from the tab.

I'm not sure in what context we're talking about a "tab for which there's no state saved" -- but if you don't have a principal stored, you could use system principal provided you're not going to be trying to run a javascript: or data: URI with a system triggering principal. The next best thing would be a content principal for the URI you're loading.

so sounds like the system principal is OK here too. I'm assuming we never want a system principal for javascript: or data: URIs anyway.

3. loading a url from history

For session history (which is kept by the browser/nsIWebNavigation) you can use the goBack and goForward and goToIndex and similar APIs, and they don't need a triggering principal (which I assume is kept internal to the session history entry in docshell land somewhere)

If you mean, from a list of previously visited URL+title combinations in a UI somewhere that the user is picking from, then system principal (again, user interaction, url isn't provided by web content).

Yep I was referring to the latter.

4. loading a search url from search terms

System (again, user interaction, url isn't provided by web content)

if a system principal is appropriate for this, we could use a systemPrincipal whenever we don't have LOAD_FLAGS_EXTERNAL.

I am confused by this because loads from link clicks (or long tap + open in new [private] tab) on other websites shouldn't have LOAD_FLAGS_EXTERNAL and those shouldn't get system principal, either...

ok this is interesting. I want to clarify here that I'm specifically referring to the times where the app calls GeckoView's loadUri API. The clicking on a link case shouldn't involve the app at all, so we're covered there. In the long tap + open in new tab case, the app should be passing a referrer in. Whenever we get a referrer we bypass picking the principal and we just use the one from the referrer.

So to summarize, it would be: we use a system principal whenever the app calls loadUri without LOAD_FLAGS_EXTERNAL and does not pass a referrer session. So far this should work for all cases that I can think of so far.

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

1. loading a bookmark
   ...
   somewhat on a tangent, I think we should treat page loads coming from external apps more similarly to page loads triggered by a content website rather than coming from the user.

Another case to consider is a URL saved with "Add to Home screen". From the perspective of a user it looks the same as a bookmark and samesite=strict cookies should be sent, but it would be considered differently to bookmarks with respect to the principal.

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

So to summarize, it would be: we use a system principal whenever the app calls loadUri without LOAD_FLAGS_EXTERNAL and does not pass a referrer session. So far this should work for all cases that I can think of so far.

Right. As far as I can tell Gijs and you summarized it correctly. Again, the rule of thumb is, if the URL to be loaded can not be influenced by web content then using SystemPrincipal is ok. Entering a URL into the URL-bar is triggered by the end user and our convention is that such loads should also use the Systemprincipal. Everything related to Session history should have a seralized principal attached to it and we should re-use that. FWIW, Gecko should handle all of that correctly.

I think in some other bugs we agreed that everything in GV that has a referrer is triggered by the web and should not use SystemPrincipal.

Now to the interesting case of LOAD_FLAGS_EXTERNAL which is e.g. when a user clicks a link in the email client for example which is then opened in the browser. This should also not use the Systemprincipal, but I think we handle that case correctly.

Anything we are missing?

I'll take a run at this.

As discussed in the bug, when the app calls loadUri without
LOAD_FLAGS_EXTERNAL and there is no referrer session, we may always use the
system principal as the requesting principal.

I removed all the special-casing of the other various schemes under the
expectation that, under the conditions described above, they are also subject
to the same policy.

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