Closed Bug 1276836 Opened 4 years ago Closed 4 years ago

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

Categories

(Core :: DOM: Security, defect, P2)

defect

Tracking

()

RESOLVED FIXED
mozilla52
Tracking Status
firefox52 --- fixed

People

(Reporter: tnguyen, Assigned: tnguyen)

References

Details

(Keywords: dev-doc-complete, Whiteboard: [domsecurity-active])

Attachments

(3 files, 4 obsolete files)

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: nobody → tnguyen
Status: NEW → ASSIGNED
Whiteboard: [domsecurity-backlog] → [domsecurity-active]
Blocks: 704320
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
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.
MozReview-Commit-ID: 2yIV2zfMABJ
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).
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
Attachment #8772328 - Attachment is obsolete: true
Updated, carried r+ necko reviewed
Attachment #8775431 - Flags: review+
Attachment #8772329 - Attachment is obsolete: true
Attachment #8775432 - Flags: review?(josh)
Attachment #8775431 - Flags: review?(josh)
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-
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 :)
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
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)
Thanks James, I see the tests are synced.
Updated web-platform tests, but I would like to wait for landing Bug 1292299 to rebase.
See Also: → 1302421
Blocks: 1302421
See Also: 1302421
Attachment #8793987 - Attachment is obsolete: true
Attachment #8775432 - Attachment is obsolete: true
Attachment #8793988 - Flags: review?(josh)
Attachment #8793989 - Flags: review?(josh)
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+
Keywords: checkin-needed
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
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
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
Depends on: 1435582
You need to log in before you can comment on or make changes to this bug.