It seems that gecko view does not pass any referrer (at least to me it seems we always pass null) and also no referrer-policy within loadURI. We should investigate and evaluate that:

https://searchfox.org/mozilla-central/source/mobile/android/geckoview/src/main/java/org/mozilla/geckoview/GeckoSession.java#1574

Thomas, can you take a look whenever you get a chance?

https://searchfox.org/mozilla-central/rev/b3b401254229f0a26f7ee625ef5f09c6c31e3949/mobile/android/chrome/content/browser.js#315

Same is true for TriggeringPrincipal (see Bug 1560022) - just adding that reference in case someone updates the API that person should probably also look at Bug 1560022 so we do not have to update the API twice :-)

We use default referrer policy. I am not quite sure the right person to ping. Hi Snorp, what do you think if we add referrer policy arg to the api?

(In reply to Thomas Nguyen from comment #4)

We use default referrer policy. I am not quite sure the right person to ping. Hi Snorp, what do you think if we add referrer policy arg to the api?

The GeckoSession.loadUri() method is intended to be used for app-directed loads (e.g., the location bar), so I'm not sure if passing a referrer policy would make sense. Would it override the Referrer-Policy header? Or provide a default?

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

The GeckoSession.loadUri() method is intended to be used for app-directed loads (e.g., the location bar), so I'm not sure if passing a referrer policy would make sense. Would it override the Referrer-Policy header? Or provide a default?

No, it will not override the header or default. It provides a correct referrer policy we should use if the request is called from an existing document (or worker). If GeckoSession.loadUri() is called separately and only for a new page, I think it should be ok. Otherwise, if we can call the API from an existing page or session, it will break the spec.
Thanks

(In reply to Thomas Nguyen from comment #6)

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

The GeckoSession.loadUri() method is intended to be used for app-directed loads (e.g., the location bar), so I'm not sure if passing a referrer policy would make sense. Would it override the Referrer-Policy header? Or provide a default?

No, it will not override the header or default. It provides a correct referrer policy we should use if the request is called from an existing document (or worker). If GeckoSession.loadUri() is called separately and only for a new page, I think it should be ok. Otherwise, if we can call the API from an existing page or session, it will break the spec.
Thanks

Alright, I think loadUri() should still be ok as-is, then.

I am still a little concerned, do we have any way to prevent (or at least adding some notes) the API from wrong usage?
Given that if the API is used to create a new request from the existing session, then we will leak referrer information.
Thanks

I would like to reopen it until I get an answer in comment 8

(In reply to Thomas Nguyen from comment #8)

I am still a little concerned, do we have any way to prevent (or at least adding some notes) the API from wrong usage?
Given that if the API is used to create a new request from the existing session, then we will leak referrer information.
Thanks

We can certainly add notes, but I'm not sure how we could guard against incorrect usage. Can you give me an example of what you're thinking about?

Sorry, I cannot have the right answer at the moment. But for me, in term of spec and privacy, it seems not right to let one could manually decide which referrer we should use (except Fetch). I don't see in our code base when we pass the referrer to that API, so it would be better that we do not expose that API public. Probably am I missing important thing somewhere?

The reason this API exists is because it's needed for some interactions with other apps. In particular, when you click a link in Twitter, it will try to open that in a Custom Tab, which is a special "lightweight" view of the browser. As part of that protocol, the app can specify a referrer for the URI it's passing along. Twitter wants that to be twitter.com for obvious reasons. So even though this is a top-level load, we do need the ability to set a specific referrer. I think from the GeckoView perspective we have to trust that the application will use this responsibly. Ideally the security/privacy reviews would evaluate usage of these APIs for our own internal apps (like Fenix).

Thomas, do you feel James' explanation is reasonable?

This is an app-level bug. Perhaps we can create conformance tests that apps using GV can run to make sure they're doing the right thing?

Thanks for the explanation.
Now it becomes more concerned to me because, this is the thing attackers can gain full control of referrer (triggering CSRF ?).
I need more discussion about that. Christoph, WDYT?

James, let me try to summarize my understanding to make sure we are not missing anything.

1. If loadURI() is called from an external app, then it's not under our (GeckoView's) control that the correct referrer and refferPolicy is set. For intent based navigations I don't think there even is a referrer/referrerPolicy, hence passing null in that case is fine. We can treat it basically the same as if a user would enter the URL into the URLbar.

2. Regarding your question if the referrerPolicy is overwritten. Let me try to answer with an example. If a document sets a referrerPolicy (e.g. through a meta tag) then all subresource loads within that document are subject to that referrerPolicy. Now if you click a link within that document, then it's that document's referrer policy that should be applied to the new document load, which in our case is triggered through ::loadURI(). After that new document is loaded, the lifetime of the referrerPolicy passed to ::loadURI is over. The new document can potentially have a new meta referrerPolicy which is then applied to all subresource loads within that new document and potentially any navigations thereafter. Does that make sense and answer your question?

3. For all the loads within Gecko, the correct referrerPolicy will be propagated automagically. We only need to worry about document navigations that are somehow short circuited in GV that basically leave Gecko and re-enter Gecko through the GV layer. Is that somehow possible? Same holds true for CSP and the TriggeringPrincipal.

Does that all make sense and can we write code to end up with that state?

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

James, let me try to summarize my understanding to make sure we are not missing anything.

1. If loadURI() is called from an external app, then it's not under our (GeckoView's) control that the correct referrer and refferPolicy is set. For intent based navigations I don't think there even is a referrer/referrerPolicy, hence passing null in that case is fine. We can treat it basically the same as if a user would enter the URL into the URLbar.

Not quite. Intents can specify a referring URI. https://developer.android.com/reference/android/content/Intent.html#EXTRA_REFERRER

Otherwise everything here sounds right to me.

I should add: I don't believe any of our products are setting the referrer from the Intent today, but I expect that to change.

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

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

James, let me try to summarize my understanding to make sure we are not missing anything.

1. If loadURI() is called from an external app, then it's not under our (GeckoView's) control that the correct referrer and refferPolicy is set. For intent based navigations I don't think there even is a referrer/referrerPolicy, hence passing null in that case is fine. We can treat it basically the same as if a user would enter the URL into the URLbar.

Not quite. Intents can specify a referring URI. https://developer.android.com/reference/android/content/Intent.html#EXTRA_REFERRER

Otherwise everything here sounds right to me.
(In reply to James Willcox (:snorp) (jwillcox@mozilla.com) (he/him) from comment #17)
I should add: I don't believe any of our products are setting the referrer from the Intent today, but I expect that to change.

Sorry I was PTO in the last couple of days. That is interesting and good to know. Thanks James,
At the moment browsers only accept http, ftp, https scheme so every application/intent referrer like android-app: or intent: scheme will be omitted. I think the purpose of the intent is to determine where the app is called from and it would be totally fine to leave referrer null in this case.

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

I should add: I don't believe any of our products are setting the referrer from the Intent today, but I expect that to change.

Right, probably that is going to change in the upcoming future. Please keep in mind that whenever we are updating those bits we should also make sure that we are setting the 'external' load flag. I know that the manifest does not allow 'data:' URIs to be loaded at the moment, but that might change eventually as well. In case that happens, we should make sure our data: URI phishing filter can kick in. Thanks!

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

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

I should add: I don't believe any of our products are setting the referrer from the Intent today, but I expect that to change.

Right, probably that is going to change in the upcoming future. Please keep in mind that whenever we are updating those bits we should also make sure that we are setting the 'external' load flag. I know that the manifest does not allow 'data:' URIs to be loaded at the moment, but that might change eventually as well. In case that happens, we should make sure our data: URI phishing filter can kick in. Thanks!

Thanks Christoph for the explanation. Using a new load flag then handling external referrer seems to be a good solution.

James says this bug should be fixed by bug 1561913.

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

James says this bug should be fixed by bug 1561913.
Anyone CC me on that bug? Thanks

Thomas, done.

fixed by bug 1561913