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

Enforce referrer-policy in WebUI #9993

Merged
merged 2 commits into from
Dec 14, 2018
Merged

Conversation

Chocobo1
Copy link
Member

This stops leaking private data to other websites via Referrer header.

@Piccirello
I might merge this before your other PRs, so you'll need to add the line to the new file in #9375.

@Chocobo1 Chocobo1 added the WebUI WebUI-related issues/changes label Dec 10, 2018
@Chocobo1 Chocobo1 added this to the 4.1.5 milestone Dec 10, 2018
@Piccirello
Copy link
Member

Have you considered adding the ‘Referrer-Policy’ header instead? It would allow us to avoid modifying all html files.

@Chocobo1
Copy link
Member Author

Have you considered adding the ‘Referrer-Policy’ header instead?

OK, will change.
Another question is, should this affect alternative webUI? I think it should restrict to built-in webUI.

This stops leaking private data to other websites via Referrer header.
@Piccirello
Copy link
Member

I’d probably leave it enabled for the alt WebUI until someone requested otherwise (which may not happen). But we can always add another checkbox to the options 😄

@Chocobo1
Copy link
Member Author

I’d probably leave it enabled for the alt WebUI until someone requested otherwise (which may not happen).

yeah, however I still think it is best not block their way in the first place, personally I would just give up and quit instead of filing an issue about it.

But we can always add another checkbox to the options

I think this is too much... I'm not trying to write a feature complete web server 😓

@Piccirello
Copy link
Member

Piccirello commented Dec 10, 2018

I think there's two options then, as I see it:

  1. Include the meta tag in each html file
  2. Send the header but disable it when alt webui is enabled

Edit: I see you already updated to option 2. Hadn't even noticed!

Piccirello
Piccirello previously approved these changes Dec 10, 2018
src/webui/webapplication.cpp Show resolved Hide resolved
src/webui/webapplication.cpp Outdated Show resolved Hide resolved
The majority of the CSP is tuned for built-in WebUI, it may not be
suitable for alternative UI.

Also add QLatin1String to strings. This code path is called repeatedly,
it is worth adding QLatin1String to squeeze out the last bit of
performance.
@Chocobo1 Chocobo1 merged commit deed457 into qbittorrent:master Dec 14, 2018
@Chocobo1 Chocobo1 deleted the referer branch December 14, 2018 05:23
@Chocobo1
Copy link
Member Author

Thanks for reviewing!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
WebUI WebUI-related issues/changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants