Implement same-origin, strict-origin, strict-origin-when-cross-origin referrer policy

RESOLVED FIXED in Firefox 52

Status

()

defect
P2
normal
RESOLVED FIXED
3 years ago
a year ago

People

(Reporter: tnguyen, Assigned: tnguyen)

Tracking

({dev-doc-complete})

unspecified
mozilla52
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox52 fixed)

Details

(Whiteboard: [domsecurity-active])

Attachments

(3 attachments, 4 obsolete attachments)

Assignee

Description

3 years ago
According to [1], same-origin policy specifies a full URL referrer information is sent when making same-origin requests and no referrer information is sent when making cross-origin requests
[1] https://w3c.github.io/webappsec-referrer-policy/#referrer-policy-same-origin
Whiteboard: [domsecurity-backlog]
Priority: -- → P2
Assignee

Updated

3 years ago
Assignee: nobody → tnguyen
Status: NEW → ASSIGNED
Whiteboard: [domsecurity-backlog] → [domsecurity-active]
Assignee

Updated

3 years ago
Blocks: 704320
Assignee

Comment 1

3 years ago
Update title.
There're 2 news policies which have added to Referrer-Policy spces last week : strict-origin and strict-origin-when-cross-origin. Implementing (and also testing) the support of same-origin, strict-origin and strict-origin-when-cross-origin are quite similar. I would like to fix them all in this bug.
Summary: Implement same-origin referrer policy → Implement same-origin, strict-origin, strict-origin-when-cross-origin referrer policy
Assignee

Comment 2

3 years ago
The 3 policies are only applied to Referrer-Policy header and meta referrer. As [1], the new policies are still not applied to referrer policy element attribute.
[1] https://html.spec.whatwg.org/multipage/infrastructure.html#referrer-policy-attribute
I will keep my eye on the specs to update.
Assignee

Comment 3

3 years ago
MozReview-Commit-ID: 2yIV2zfMABJ
Assignee

Comment 5

3 years ago
Comment on attachment 8772328 [details] [diff] [review]
Implement same-origin, strict-origin, strict-origin-when-cross-origin referrer policy

Hi Patrick,
There're several changes in netwerk, such as adding new referrer policy to nsIHttpChannel.idl and handling policy in HttpBaseChannel.cpp.
Could you please take a look on that?
Thank you very much
Attachment #8772328 - Flags: review?(mcmanus)
Attachment #8772328 - Flags: review?(mcmanus) → review+
(In reply to Thomas Nguyen[:tnguyen] (use ni? plz) from comment #2)
> The 3 policies are only applied to Referrer-Policy header and meta referrer.
> As [1], the new policies are still not applied to referrer policy element
> attribute.
> [1]
> https://html.spec.whatwg.org/multipage/infrastructure.html#referrer-policy-
> attribute

That's on my TODO list (https://github.com/w3c/webappsec-referrer-policy/issues/50#issuecomment-230202635). I will be submitting a patch to the HTML spec soon (hopefully this week).
Comment hidden (obsolete, typo)
Assignee

Comment 8

3 years ago
Great, just checked specs and the 3 policies are added to HTML specs today. In term of implementation, that's enough to resolve this bug. 
We still need to add the specs to Fetch's spec ( [1] I filed an issue for that and probably could help sometime). And we could add that change to bug 1264792 or bug 1184549 which track changes of fetch according to new referrer policy API
[1] https://github.com/whatwg/fetch/issues/342
Thanks Francois for the help
Assignee

Updated

3 years ago
Attachment #8772328 - Attachment is obsolete: true
Assignee

Comment 10

3 years ago
Updated, carried r+ necko reviewed
Assignee

Updated

3 years ago
Attachment #8775431 - Flags: review+
Assignee

Updated

3 years ago
Attachment #8772329 - Attachment is obsolete: true
Assignee

Updated

3 years ago
Attachment #8775432 - Flags: review?(josh)
Assignee

Updated

3 years ago
Attachment #8775431 - Flags: review?(josh)
Assignee

Comment 12

3 years ago
Updated test case, add the new policies to old test case. :)
Beside necko team, I still need a little review on this. Hi Josh, could you please look into this?
Thanks for your help
Attachment #8775431 - Flags: review?(josh) → review+
Comment on attachment 8775432 [details] [diff] [review]
Update and add tests for same-origin, strict-origin, strict-origin-when-cross-origin referrer policy

Review of attachment 8775432 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/base/test/test_anchor_area_referrer_invalid.html
@@ +62,5 @@
>           SCHEME_FROM: 'https',
>           SCHEME_TO: 'http',
>           DESC: "origin-when-cross-origin (anchor) with origin in meta",
> +         RESULT: 'origin'},
> +        {ATTRIBUTE_POLICY: 'strict-origin-when-cross-origin',

I'm not sure that this is a useful test to add here. This file has evolved from its original purpose (verify that invalid referrer-policy attributes did the right thing) since bug 1178337, so these tests seem like they're just duplicating cases we have in other files at this point.

::: dom/base/test/test_iframe_referrer_invalid.html
@@ +70,5 @@
>          {NAME: 'no-referrer-in-meta',
>           META_POLICY: 'no-referrer',
>           DESC: "no-referrer in meta",
> +         RESULT: 'none'},
> +        {ATTRIBUTE_POLICY: 'strict-origin-when-cross-origin',

I have the same concern in this test file as the other one.

::: dom/base/test/test_referrer_policy_header.html
@@ +1,5 @@
> +<!DOCTYPE HTML>
> +<html>
> +<head>
> +  <meta charset="utf-8">
> +  <title>Test same-origin, strict-origin, strict-origin-when-cross-origin referrer policy header for Bug 1276836</title>

Rather than creating this new test from scratch (which only tests the new header values, not the old ones), why don't we update the existing Referrer-Policy header tests that exist in web-platform-tests instead?
Attachment #8775432 - Flags: review?(josh) → review-
Assignee

Comment 14

3 years ago
Thanks Josh,
Just found that our web-platform-test is synced to [1] and many new same-origin policy tests are added last 2 weeks. I will update our web-platform-test meta according to that.

[1] https://github.com/w3c/web-platform-tests/tree/master/referrer-policy


(In reply to Josh Matthews [:jdm] from comment #13)
> Rather than creating this new test from scratch (which only tests the new
> header values, not the old ones), why don't we update the existing
> Referrer-Policy header tests that exist in web-platform-tests instead?

I would like to add more test cases for strict-policy* similar to same-origin and others policies, because we have tools/generate.py and templates to do that. It may increase the number of adding file in the patch :)
Assignee

Comment 15

3 years ago
Should be better to fix in github web-platform-tests and sync to mozilla-central later.
Link:
https://github.com/w3c/web-platform-tests/issues/3436
Assignee

Comment 16

3 years ago
Thanks Josh for looking into https://github.com/w3c/web-platform-tests/issues/3436

Hi James
We have new strict-origin* referrer policy web platform tests are added to github. Could we sync the new web-platform-tests from github sooner?
Thanks
Flags: needinfo?(james)
This is ongoing, but for various reasons (PTO + etc.) is taking a little longer than normal. I have a not-quite green try run now, so with a little more metadata adjustment we should get there.
Flags: needinfo?(james)
Assignee

Comment 18

3 years ago
Thanks James, I see the tests are synced.
Updated web-platform tests, but I would like to wait for landing Bug 1292299 to rebase.
Assignee

Updated

3 years ago
See Also: → 1302421
Blocks: 1302421
See Also: 1302421
Assignee

Updated

3 years ago
Attachment #8793987 - Attachment is obsolete: true
Assignee

Updated

3 years ago
Attachment #8775432 - Attachment is obsolete: true
Assignee

Updated

3 years ago
Attachment #8793988 - Flags: review?(josh)
Assignee

Updated

3 years ago
Attachment #8793989 - Flags: review?(josh)
Assignee

Comment 23

3 years ago
Rebased and updated web-platform-tests.
Hi Josh, could you please take a look at this?
Thanks
Attachment #8793989 - Flags: review?(josh) → review+
Attachment #8793988 - Flags: review?(josh) → review+
Assignee

Updated

3 years ago
Keywords: checkin-needed

Comment 25

3 years ago
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/890c2d9417c5
Implement same-origin, strict-origin, strict-origin-when-cross-origin referrer policy. r=mcmanus,jdm
https://hg.mozilla.org/integration/mozilla-inbound/rev/3500d2927d52
Update and add tests for same-origin, strict-origin, strict-origin-when-cross-origin referrer policy. r=jdm
https://hg.mozilla.org/integration/mozilla-inbound/rev/63454fe35579
Update web-platform-test same-origin, strict-origin, strict-origin-when-cross-origin referrer policy. r=jdm
Keywords: checkin-needed

Comment 26

3 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/890c2d9417c5
https://hg.mozilla.org/mozilla-central/rev/3500d2927d52
https://hg.mozilla.org/mozilla-central/rev/63454fe35579
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
Depends on: 1305622

Updated

a year ago
Depends on: 1435582
You need to log in before you can comment on or make changes to this bug.