Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

change referrer policy. Stop sending referers as much as possible #3636

Merged
merged 3 commits into from
Nov 24, 2019

Conversation

hmdne
Copy link
Contributor

@hmdne hmdne commented Aug 25, 2019

Before this patch I managed to send a bunch of referrers to third party websites with Firefox. Unfortunately, meta name="referrer" isn't enough.
I added "noopener" because that's also what Nextcloud does. One may want to check another pull request from 2015 which may be more complete, but it was denied with a plugin proposal: #2498
References to earlier issues: #1603, #3043

@slel
Copy link
Contributor

slel commented Oct 30, 2019

Isn't that what this plugin is for: https://github.com/ether/ep_hide_referrer

@ldidry
Copy link
Contributor

ldidry commented Nov 5, 2019

@slel We shouldn’t have to use a plugin to avoid pad’s URLs leaking to third party websites.

@ldidry
Copy link
Contributor

ldidry commented Nov 5, 2019

@muxator Would you consider merging this PR for 1.8.0? I think it’s important.

@muxator
Copy link
Contributor

muxator commented Nov 6, 2019

Personally, I am sensitive to the issue: my Firefox always runs with the referrer disabled, or with network.http.referer.spoofSource = true. In summary: I would prefer a more secure default like the one proposed here.

Regarding the PR: I would merge it if it was in a more complete form:

  • mention somewhere in the documentation that no referrer is leaked (and that this is a change that was impelmented starting from 1.8)
  • in the code & the commit message: explain what was done (at least a link to the html spec for noreferrer and noopener)
  • it would also make sense, on the server side, to send the Referrer-Policy header. Something like:
    Referrer-Policy: same-origin
    edit: my original proposal was to use Referrer-Policy: strict-origin-when-cross-origin. The code that was merged with this PR used same-origin, instead, which is stricter. I accepted that version and modified the proposal.

Finally, we have to think if we want to implement this change without offering a configuration option to restore the old behaviour. I do not see a reason to do it, but you never know what's out there among your users.
Edit: let's not overcomplicate things. 1.8.0 will simply change its referrer policy.

@ldidry
Copy link
Contributor

ldidry commented Nov 8, 2019

@ahmadine would you need help to comply to @muxator requested changes?

@muxator
Copy link
Contributor

muxator commented Nov 14, 2019

Gentle bump :)

@hmdne
Copy link
Contributor Author

hmdne commented Nov 14, 2019

@slel It's already an Etherpad semantic to remove the referrers, #3044 has been merged, but Firefox seems to ignore that. This patch made Firefox remove them as well.

@muxator Of course, I will work on this patch more, this or next week. I found more places where the referrer may leak later, but this one is the most urgent.

Added `rel="noreferrer"` to automatically generated links in the main pad window
as well as the chat window.

`rel="noreferrer"` is part of the HTML5 standard. While browser support isn't
100%, it's better than nothing. Future alternative solutions with wider browser
support, such as intermediary redirect pages, are unaffected by this change.

https://html.spec.whatwg.org/multipage/links.html#link-type-noreferrer

This commit was originally part of ether#2498
…er header

Exported HTML can, when loaded from disk or an online server, also leak the
location. Applying the `rel="noreferrer"` HTML5 standard mitigate the problem
for compatible browsers.

https://html.spec.whatwg.org/multipage/links.html#link-type-noreferrer

This commit was originally part of ether#2498
@hmdne hmdne force-pushed the ahmadine-noreferrer branch from be3314d to 1dadf56 Compare November 23, 2019 08:03
hmdne added a commit to hmdne/etherpad-lite that referenced this pull request Nov 23, 2019
Pull request with discussion: ether#3636

What's already there:
* meta name=referrer: already done in 1.6.1: ether#3044
  https://caniuse.com/#feat=referrer-policy
  https://w3c.github.io/webappsec-referrer-policy/#referrer-policy-delivery-meta
  (Chrome>=78, Firefox>=70, Safari>=13, Opera>=64, ~IE[1], ~Edge[1])

The previous two commits I backported in this batch:
* a rel=noreferrer: a pull request denied before: ether#2498
  https://html.spec.whatwg.org/multipage/links.html#link-type-noreferrer
  https://developer.mozilla.org/en-US/docs/Web/HTML/Link_types
  (Firefox>=37, I can't find more info about support)

This commit adds the following:
* a rel=noopener: fixing a not-so-well-known way to extract referer
  https://html.spec.whatwg.org/multipage/links.html#link-type-noopener
  (Chrome>=49, Firefox>=52, Safari>=10.1, Opera>=36, !IE, !Edge)
* Referrer-Policy: same-origin: the last bastion of referrer security
  https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Referrer-Policy
  (Chrome>=61, Firefox>=52, Safari>=11.1, Opera>=48, !IE, !Edge)

meta name=referrer wasn't enough. I happened to leak a few referrers with my Firefox browser,
though for some browsers it could have been enough.

[1] IE>=11, Edge>=18 use a different syntax for meta name=referrer, making it most probably
    incompatible (but I may be wrong on that, they may support both, but I have no way to test
    it currently). The next Edge release will be based on Chromium, so for that the Chrome version
    applies.
…sible

Pull request with discussion: ether#3636

What's already there:
* `meta name=referrer`: already done in 1.6.1:
  ether#3044

  https://caniuse.com/#feat=referrer-policy
  https://w3c.github.io/webappsec-referrer-policy/#referrer-policy-delivery-meta
  (Chrome>=78, Firefox>=70, Safari>=13, Opera>=64, ~IE[1], ~Edge[1])

The previous two commits (by @joelpurra) I backported in this batch:
* `<a rel=noreferrer>`: a pull request denied before:
  ether#2498

  https://html.spec.whatwg.org/multipage/links.html#link-type-noreferrer
  https://developer.mozilla.org/en-US/docs/Web/HTML/Link_types
  (Firefox>=37, I can't find more info about support)

This commit adds the following:
* `<a rel="noopener">`: fixing a not-so-well-known way to extract referer
  https://html.spec.whatwg.org/multipage/links.html#link-type-noopener
  (Chrome>=49, Firefox>=52, Safari>=10.1, Opera>=36, !IE, !Edge)

* `Referrer-Policy: same-origin`: the last bastion of referrer security
  https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Referrer-Policy
  (Chrome>=61, Firefox>=52, Safari>=11.1, Opera>=48, !IE, !Edge)

meta name=referrer wasn't enough. I happened to leak a few referrers with my
Firefox browser, though for some browsers it could have been enough.

[1] IE>=11, Edge>=18 use a different syntax for meta name=referrer, making it
    most probably incompatible (but I may be wrong on that, they may support
    both, but I have no way to test it currently). The next Edge release will be
    based on Chromium, so for that the Chrome version applies.
hmdne added a commit to hmdne/etherpad-lite that referenced this pull request Nov 23, 2019
Pull request with discussion: ether#3636

What's already there:
* meta name=referrer: already done in 1.6.1: ether#3044
  https://caniuse.com/#feat=referrer-policy
  https://w3c.github.io/webappsec-referrer-policy/#referrer-policy-delivery-meta
  (Chrome>=78, Firefox>=70, Safari>=13, Opera>=64, ~IE[1], ~Edge[1])

The previous two commits (by @joelpurra) I backported in this batch:
* a rel=noreferrer: a pull request denied before: ether#2498
  https://html.spec.whatwg.org/multipage/links.html#link-type-noreferrer
  https://developer.mozilla.org/en-US/docs/Web/HTML/Link_types
  (Firefox>=37, I can't find more info about support)

This commit adds the following:
* a rel=noopener: fixing a not-so-well-known way to extract referer
  https://html.spec.whatwg.org/multipage/links.html#link-type-noopener
  (Chrome>=49, Firefox>=52, Safari>=10.1, Opera>=36, !IE, !Edge)
* Referrer-Policy: same-origin: the last bastion of referrer security
  https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Referrer-Policy
  (Chrome>=61, Firefox>=52, Safari>=11.1, Opera>=48, !IE, !Edge)

meta name=referrer wasn't enough. I happened to leak a few referrers with my Firefox browser,
though for some browsers it could have been enough.

[1] IE>=11, Edge>=18 use a different syntax for meta name=referrer, making it most probably
    incompatible (but I may be wrong on that, they may support both, but I have no way to test
    it currently). The next Edge release will be based on Chromium, so for that the Chrome version
    applies.
@hmdne hmdne force-pushed the ahmadine-noreferrer branch from 1dadf56 to 6fa3103 Compare November 23, 2019 08:05
@hmdne
Copy link
Contributor Author

hmdne commented Nov 23, 2019

I rebased my commit on @joelpurra's work. Below I will paste the content of the commit message, because it is quite informative about the issue and I think it should be more visible to people who will be interested in that issue.

    Work towards the greatest level of referrer security.
    Pull request with discussion: https://github.com/ether/etherpad-lite/pull/3636
    
    What's already there:
    * meta name=referrer: already done in 1.6.1: https://github.com/ether/etherpad-lite/pull/3044
      https://caniuse.com/#feat=referrer-policy
      https://w3c.github.io/webappsec-referrer-policy/#referrer-policy-delivery-meta
      (Chrome>=78, Firefox>=70, Safari>=13, Opera>=64, ~IE[1], ~Edge[1])
    
    The previous two commits (by @joelpurra) I backported in this batch:
    * a rel=noreferrer: a pull request denied before: https://github.com/ether/etherpad-lite/pull/2498
      https://html.spec.whatwg.org/multipage/links.html#link-type-noreferrer
      https://developer.mozilla.org/en-US/docs/Web/HTML/Link_types
      (Firefox>=37, I can't find more info about support)
    
    This commit adds the following:
    * a rel=noopener: fixing a not-so-well-known way to extract referer
      https://html.spec.whatwg.org/multipage/links.html#link-type-noopener
      (Chrome>=49, Firefox>=52, Safari>=10.1, Opera>=36, !IE, !Edge)
    * Referrer-Policy: same-origin: the last bastion of referrer security
      https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Referrer-Policy
      (Chrome>=61, Firefox>=52, Safari>=11.1, Opera>=48, !IE, !Edge)
    
    meta name=referrer wasn't enough. I happened to leak a few referrers with my Firefox browser,
    though for some browsers it could have been enough.
    
    [1] IE>=11, Edge>=18 use a different syntax for meta name=referrer, making it most probably
        incompatible (but I may be wrong on that, they may support both, but I have no way to test
        it currently). The next Edge release will be based on Chromium, so for that the Chrome version
        applies.

muxator pushed a commit to hmdne/etherpad-lite that referenced this pull request Nov 24, 2019
Pull request with discussion: ether#3636

What's already there:
* `meta name=referrer`: already done in 1.6.1:
  ether#3044

  https://caniuse.com/#feat=referrer-policy
  https://w3c.github.io/webappsec-referrer-policy/#referrer-policy-delivery-meta
  (Chrome>=78, Firefox>=70, Safari>=13, Opera>=64, ~IE[1], ~Edge[1])

The previous two commits (by @joelpurra) I backported in this batch:
* `<a rel=noreferrer>`: a pull request denied before:
  ether#2498

  https://html.spec.whatwg.org/multipage/links.html#link-type-noreferrer
  https://developer.mozilla.org/en-US/docs/Web/HTML/Link_types
  (Firefox>=37, I can't find more info about support)

This commit adds the following:
* `<a rel="noopener">`: fixing a not-so-well-known way to extract referer
  https://html.spec.whatwg.org/multipage/links.html#link-type-noopener
  (Chrome>=49, Firefox>=52, Safari>=10.1, Opera>=36, !IE, !Edge)

* `Referrer-Policy: same-origin`: the last bastion of referrer security
  https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Referrer-Policy
  (Chrome>=61, Firefox>=52, Safari>=11.1, Opera>=48, !IE, !Edge)

meta name=referrer wasn't enough. I happened to leak a few referrers with my
Firefox browser, though for some browsers it could have been enough.

[1] IE>=11, Edge>=18 use a different syntax for meta name=referrer, making it
    most probably incompatible (but I may be wrong on that, they may support
    both, but I have no way to test it currently). The next Edge release will be
    based on Chromium, so for that the Chrome version applies.
@muxator muxator force-pushed the ahmadine-noreferrer branch from 6fa3103 to 386c65c Compare November 24, 2019 22:00
muxator pushed a commit to hmdne/etherpad-lite that referenced this pull request Nov 24, 2019
Pull request with discussion: ether#3636

What's already there:
* `meta name=referrer`: already done in 1.6.1:
  ether#3044

  https://caniuse.com/#feat=referrer-policy
  https://w3c.github.io/webappsec-referrer-policy/#referrer-policy-delivery-meta
  (Chrome>=78, Firefox>=70, Safari>=13, Opera>=64, ~IE[1], ~Edge[1])

The previous two commits (by @joelpurra) I backported in this batch:
* `<a rel=noreferrer>`: a pull request denied before:
  ether#2498

  https://html.spec.whatwg.org/multipage/links.html#link-type-noreferrer
  https://developer.mozilla.org/en-US/docs/Web/HTML/Link_types
  (Firefox>=37, I can't find more info about support)

This commit adds the following:
* `<a rel="noopener">`: fixing a not-so-well-known way to extract referer
  https://html.spec.whatwg.org/multipage/links.html#link-type-noopener
  (Chrome>=49, Firefox>=52, Safari>=10.1, Opera>=36, !IE, !Edge)

* `Referrer-Policy: same-origin`: the last bastion of referrer security
  https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Referrer-Policy
  (Chrome>=61, Firefox>=52, Safari>=11.1, Opera>=48, !IE, !Edge)

meta name=referrer wasn't enough. I happened to leak a few referrers with my
Firefox browser, though for some browsers it could have been enough.

[1] IE>=11, Edge>=18 use a different syntax for meta name=referrer, making it
    most probably incompatible (but I may be wrong on that, they may support
    both, but I have no way to test it currently). The next Edge release will be
    based on Chromium, so for that the Chrome version applies.
@muxator muxator force-pushed the ahmadine-noreferrer branch from 386c65c to 951c44e Compare November 24, 2019 22:05
muxator pushed a commit to hmdne/etherpad-lite that referenced this pull request Nov 24, 2019
…sible

Pull request with discussion: ether#3636

What's already there:
* `meta name=referrer`: already done in 1.6.1:
  ether#3044

  https://caniuse.com/#feat=referrer-policy
  https://w3c.github.io/webappsec-referrer-policy/#referrer-policy-delivery-meta
  (Chrome>=78, Firefox>=70, Safari>=13, Opera>=64, ~IE[1], ~Edge[1])

The previous two commits (by @joelpurra) I backported in this batch:
* `<a rel=noreferrer>`: a pull request denied before:
  ether#2498

  https://html.spec.whatwg.org/multipage/links.html#link-type-noreferrer
  https://developer.mozilla.org/en-US/docs/Web/HTML/Link_types
  (Firefox>=37, I can't find more info about support)

This commit adds the following:
* `<a rel="noopener">`: fixing a not-so-well-known way to extract referer
  https://html.spec.whatwg.org/multipage/links.html#link-type-noopener
  (Chrome>=49, Firefox>=52, Safari>=10.1, Opera>=36, !IE, !Edge)

* `Referrer-Policy: same-origin`: the last bastion of referrer security
  https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Referrer-Policy
  (Chrome>=61, Firefox>=52, Safari>=11.1, Opera>=48, !IE, !Edge)

meta name=referrer wasn't enough. I happened to leak a few referrers with my
Firefox browser, though for some browsers it could have been enough.

[1] IE>=11, Edge>=18 use a different syntax for meta name=referrer, making it
    most probably incompatible (but I may be wrong on that, they may support
    both, but I have no way to test it currently). The next Edge release will be
    based on Chromium, so for that the Chrome version applies.
@muxator muxator force-pushed the ahmadine-noreferrer branch from 951c44e to 2d9ed8c Compare November 24, 2019 22:08
@muxator muxator changed the title Actually fix the Referrer issue change referrer policy. Stop sending referers as much as possible Nov 24, 2019
@muxator muxator force-pushed the ahmadine-noreferrer branch from 2d9ed8c to 54e0f2d Compare November 24, 2019 22:27
@muxator muxator merged commit 0a0b90c into ether:develop Nov 24, 2019
@muxator
Copy link
Contributor

muxator commented Nov 24, 2019

Pulled in, thanks.

@muxator muxator added this to the 1.8 milestone Nov 24, 2019
muxator pushed a commit to Chocobozzz/etherpad-lite that referenced this pull request Mar 31, 2020
This change augments what was already done in 54e0f2d (PR with discussion
at ether#3636).

For documentation about the meaning of "noopener, noreferrer", see:
https://developer.mozilla.org/en-US/docs/Web/API/Window/open#Window_functionality_features
muxator pushed a commit to Chocobozzz/etherpad-lite that referenced this pull request Apr 1, 2020
This change augments what was already done in 54e0f2d (PR with discussion
at ether#3636).

For documentation about the meaning of "noopener, noreferrer", see:
https://developer.mozilla.org/en-US/docs/Web/API/Window/open#Window_functionality_features
muxator pushed a commit that referenced this pull request Apr 1, 2020
This change augments what was already done in 54e0f2d (PR with discussion
at #3636).

For documentation about the meaning of "noopener, noreferrer", see:
https://developer.mozilla.org/en-US/docs/Web/API/Window/open#Window_functionality_features
muxator pushed a commit that referenced this pull request Apr 3, 2020
This change augments what was already done in 54e0f2d (PR with discussion
at #3636).

For documentation about the meaning of "noopener, noreferrer", see:
https://developer.mozilla.org/en-US/docs/Web/API/Window/open#Window_functionality_features
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants