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

CSP (Content-Security-Policy) Support #508

Open
oXyiGYJ opened this issue Jul 30, 2020 · 20 comments
Open

CSP (Content-Security-Policy) Support #508

oXyiGYJ opened this issue Jul 30, 2020 · 20 comments
Assignees
Milestone

Comments

@oXyiGYJ
Copy link
Contributor

oXyiGYJ commented Jul 30, 2020

Describe the bug
Multiple.
Currently, with CSP, connecting to the server blocks things like inline code and evals. Requiring the flag usafe-inline and unsafe-eval. batman has already started working on removing inline JS.

Setting default-src (with flags other than outlining the allowed, recommended with nothing else set is 'self') blocks the external calls to gstatic and MaxCDN. Will post another comment outlining the proper use of this header when all is said and done.

Please reference https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Security-Policy for full info on CSP.

To Reproduce
Steps to reproduce the behavior:

  1. Have CSP enabled.
  2. Go to your servers address
  3. Lose your mind

Expected behavior
Documentation should (in my opinion) enable this correctly by default, but if not, allow users the option to enable it and give them the correct header info to do so.

Environment:

  • OS: All?
  • Browser: All modern browsers
  • Version: Modern?

Additional context
Current working header below. Will update with future fixes in the comments of this issue.

add_header Content-Security-Policy "default-src 'self' 'unsafe-inline' 'unsafe-eval' object-src 'none' https;";

@oXyiGYJ oXyiGYJ added the bug label Jul 30, 2020
@jhthorsen jhthorsen added this to the 4.xx milestone Jul 30, 2020
@jhthorsen jhthorsen self-assigned this Jul 30, 2020
@oXyiGYJ
Copy link
Contributor Author

oXyiGYJ commented Jul 30, 2020

Fully working header, correct syntax this time.

Nginx (in the server {} block): add_header Content-Security-Policy "default-src 'none'; base-uri 'self';block-all-mixed-content; manifest-src 'self'; object-src 'none'; frame-ancestors 'none'; frame-src 'self'; img-src 'self' twemoji.maxcdn.com www.gravatar.com repository-images.githubusercontent.com https; connect-src 'self'; font-src 'self' fonts.gstatic.com https; style-src 'self' fonts.googleapis.com 'unsafe-inline' https; script-src 'self' 'unsafe-inline' 'unsafe-eval';" always;

Apache (inside your VirtualHost): Header set Content-Security-Policy "default-src 'none'; base-uri 'self';block-all-mixed-content; manifest-src 'self'; object-src 'none'; frame-ancestors 'none'; frame-src 'self'; img-src 'self' twemoji.maxcdn.com www.gravatar.com repository-images.githubusercontent.com https; connect-src 'self'; font-src 'self' fonts.gstatic.com https; style-src 'self' fonts.googleapis.com 'unsafe-inline' https; script-src 'self' 'unsafe-inline' 'unsafe-eval';" always;

Another reference https://content-security-policy.com/

Edit: update to be more secure and add githubusercontent.com

@oXyiGYJ
Copy link
Contributor Author

oXyiGYJ commented Jul 30, 2020

Okay, images are saved. Using img-src *; is fine, as its only loading images and the browser itself should handle issues there. However, the issue now comes up with the following error in console after a reload of the page The FetchEvent for "https://repository-images.githubusercontent.com/14822868/751ec900-ae2b-11ea-8fbb-1ee16400fa8f" resulted in a network error response: an object that was not a Response was passed to respondWith().

FetchEvent is the response (and that gave rise to your respondWith error). the bug is going to be on the request side. The service worker should use fetch with a Request() rather than a url string and then as part of the Request, specify the RequestDestination

https://developer.mozilla.org/en-US/docs/Web/API/FetchEvent/respondWith

This allows img-src *; to function correctly and load any image on the web, but not be blocked by content-src 'self';.

Edit: added some more info https://stackoverflow.com/questions/57872284/content-security-policy-violations-on-seemingly-valid-urls

@oXyiGYJ
Copy link
Contributor Author

oXyiGYJ commented Aug 8, 2020

Inline issue after 4.33 update https://imgur.com/oibXCGb
Eval issue after 4.33 update https://imgur.com/aUZHWbq

@oXyiGYJ
Copy link
Contributor Author

oXyiGYJ commented Aug 8, 2020

Current working headers:

nginx: add_header Content-Security-Policy "default-src 'none'; base-uri 'self';block-all-mixed-content; manifest-src 'self'; object-src 'none'; frame-ancestors 'none'; frame-src 'self'; connect-src 'self' www.gravatar.com twemoji.maxcdn.com; img-src *; font-src 'self' fonts.gstatic.com https; style-src 'self' fonts.googleapis.com 'unsafe-inline'; script-src 'self' 'unsafe-inline' 'unsafe-eval';" always;

Apache: Header set Content-Security-Policy "default-src 'none'; base-uri 'self';block-all-mixed-content; manifest-src 'self'; object-src 'none'; frame-ancestors 'none'; frame-src 'self'; connect-src 'self' www.gravatar.com twemoji.maxcdn.com; img-src *; font-src 'self' fonts.gstatic.com https; style-src 'self' fonts.googleapis.com 'unsafe-inline'; script-src 'self' 'unsafe-inline' 'unsafe-eval';" always;

@oXyiGYJ
Copy link
Contributor Author

oXyiGYJ commented Aug 8, 2020

image
image

@stigtsp
Copy link
Contributor

stigtsp commented Aug 19, 2020

@wylel Would it be an idea to set these in either or res->header in mojo?

@jhthorsen jhthorsen assigned oXyiGYJ and unassigned jhthorsen Aug 28, 2020
@oXyiGYJ
Copy link
Contributor Author

oXyiGYJ commented Sep 29, 2020

@stigtsp I will have to look at Mojo as im not familiar with it. i will update when I do.

@jhthorsen
Copy link
Collaborator

I’m not entirely sure, but I think you can add it here: https://github.com/Nordaaker/convos/blob/00a95979a13762b502b07ff6d8d7257247ceda0c/lib/Convos.pm#L126

Let me know if that line does not make sense.

https://convos.chat/doc/develop#starting-the-application shows how to start a “live” development server. Note however that changes in the lib/ directory will cause Convos to reconnect all the IRC connections. Ask on freenode if you want a demo IRC server to test against that doesn’t mind a lot of reconnects.

@stigtsp
Copy link
Contributor

stigtsp commented Sep 29, 2020

I’m not entirely sure, but I think you can add it here:

I think this makes a lot of sense to do it there, not relying on nginx.

@jhthorsen jhthorsen modified the milestones: 4.xx, 5.xx Nov 13, 2020
@jhthorsen
Copy link
Collaborator

jhthorsen commented Jan 13, 2021

Another user had issues with CSP today, so I was wondering if I could add the header from #508 (comment) to Convos? Is there any risk involved in doing so?

I also wonder how difficult it would be to bundle the Google fonts... Maybe I should just do that. I still don't want to bundle the Twitter emojis though, since there's so many of them.

@stigtsp
Copy link
Contributor

stigtsp commented Jan 13, 2021

@oXyiGYJ
Copy link
Contributor Author

oXyiGYJ commented Jan 14, 2021

Let me finally test it and see, if you haven't already. Been busy, sorry about that. Theoretically this should be fine.

jhthorsen pushed a commit that referenced this issue Jan 15, 2021
  Ubuntu Mono, UbuntuMono-Regular, https://fonts.gstatic.com/s/ubuntumono/v9/KFOjCneDtsqEr0keqCMhbCc3CsTYl4BOQ3o.woff2
  Ubuntu Mono, UbuntuMono-Regular, https://fonts.gstatic.com/s/ubuntumono/v9/KFOjCneDtsqEr0keqCMhbCc3CsTYl4BOQ3o.woff2
  Ubuntu Mono, UbuntuMono-Regular, https://fonts.gstatic.com/s/ubuntumono/v9/KFOjCneDtsqEr0keqCMhbCc-CsTYl4BOQ3o.woff2
  Ubuntu Mono, UbuntuMono-Regular, https://fonts.gstatic.com/s/ubuntumono/v9/KFOjCneDtsqEr0keqCMhbCc2CsTYl4BOQ3o.woff2
  Ubuntu Mono, UbuntuMono-Regular, https://fonts.gstatic.com/s/ubuntumono/v9/KFOjCneDtsqEr0keqCMhbCc5CsTYl4BOQ3o.woff2
  Ubuntu Mono, UbuntuMono-Regular, https://fonts.gstatic.com/s/ubuntumono/v9/KFOjCneDtsqEr0keqCMhbCc0CsTYl4BOQ3o.woff2
  Ubuntu Mono, UbuntuMono-Regular, https://fonts.gstatic.com/s/ubuntumono/v9/KFOjCneDtsqEr0keqCMhbCc6CsTYl4BO.woff2
  PT Sans, PTSans-Regular, https://fonts.gstatic.com/s/ptsans/v11/jizaRExUiTo99u79D0-ExcOPIDUg-g.woff2
  PT Sans, PTSans-Regular, https://fonts.gstatic.com/s/ptsans/v11/jizaRExUiTo99u79D0aExcOPIDUg-g.woff2
  PT Sans, PTSans-Regular, https://fonts.gstatic.com/s/ptsans/v11/jizaRExUiTo99u79D0yExcOPIDUg-g.woff2
  PT Sans, PTSans-Regular, https://fonts.gstatic.com/s/ptsans/v11/jizaRExUiTo99u79D0KExcOPIDU.woff2
  PT Sans Bold, PTSans-Bold, https://fonts.gstatic.com/s/ptsans/v11/jizfRExUiTo99u79B_mh0OOtLR8a8zILig.woff2
  PT Sans Bold, PTSans-Bold, https://fonts.gstatic.com/s/ptsans/v11/jizfRExUiTo99u79B_mh0OqtLR8a8zILig.woff2
  PT Sans Bold, PTSans-Bold, https://fonts.gstatic.com/s/ptsans/v11/jizfRExUiTo99u79B_mh0OCtLR8a8zILig.woff2

  wget https://fonts.gstatic.com/s/ubuntumono/v9/KFOjCneDtsqEr0keqCMhbCc3CsTYl4BOQ3o.woff2
  wget https://fonts.gstatic.com/s/ubuntumono/v9/KFOjCneDtsqEr0keqCMhbCc-CsTYl4BOQ3o.woff2
  wget https://fonts.gstatic.com/s/ubuntumono/v9/KFOjCneDtsqEr0keqCMhbCc2CsTYl4BOQ3o.woff2
  wget https://fonts.gstatic.com/s/ubuntumono/v9/KFOjCneDtsqEr0keqCMhbCc5CsTYl4BOQ3o.woff2
  wget https://fonts.gstatic.com/s/ubuntumono/v9/KFOjCneDtsqEr0keqCMhbCc0CsTYl4BOQ3o.woff2
  wget https://fonts.gstatic.com/s/ubuntumono/v9/KFOjCneDtsqEr0keqCMhbCc6CsTYl4BO.woff2
  wget https://fonts.gstatic.com/s/ptsans/v11/jizaRExUiTo99u79D0-ExcOPIDUg-g.woff2
  wget https://fonts.gstatic.com/s/ptsans/v11/jizaRExUiTo99u79D0aExcOPIDUg-g.woff2
  wget https://fonts.gstatic.com/s/ptsans/v11/jizaRExUiTo99u79D0yExcOPIDUg-g.woff2
  wget https://fonts.gstatic.com/s/ptsans/v11/jizaRExUiTo99u79D0KExcOPIDU.woff2
  wget https://fonts.gstatic.com/s/ptsans/v11/jizfRExUiTo99u79B_mh0OOtLR8a8zILig.woff2
  wget https://fonts.gstatic.com/s/ptsans/v11/jizfRExUiTo99u79B_mh0OqtLR8a8zILig.woff2
  wget https://fonts.gstatic.com/s/ptsans/v11/jizfRExUiTo99u79B_mh0OCtLR8a8zILig.woff2
  wget https://fonts.gstatic.com/s/ptsans/v11/jizfRExUiTo99u79B_mh0O6tLR8a8zI.woff2
@jhthorsen
Copy link
Collaborator

jhthorsen commented Jan 15, 2021

fonts.googleapis.com and fonts.gstatic.com should not be required after 4c22cb9. I haven't built the new assets though, so just pulling "master" won't be enough to test it.

I wonder if we can use native emojis these days... Does anyone have an idea if the major Linux distros ship with emoji support now?

And I hope "unsafe-inline" and "unsafe-eval" for "script" is fixed already.

@oXyiGYJ
Copy link
Contributor Author

oXyiGYJ commented Jan 15, 2021

This can be added. Adding this for the sake of getting it later/tweaking before PR.

$c->res->headers->header('Content-Security-Policy', qq(default-src 'none'; base-uri 'self'; block-all-mixed-content; manifest-src 'self'; object-src 'none'; frame-ancestors 'none'; frame-src 'self'; connect-src 'self'; img-src *; font-src 'self'; style-src 'self' 'unsafe-inline'; script-src 'self' 'unsafe-inline' 'unsafe-eval';));

Another good doc on potentially getting rid of the unsafe-inline with sha256. https://www.getastra.com/kb/knowledgebase/content-security-policy-all-you-need-to-know/

@jhthorsen as for the emojis, doing img-src *; is not that big of a deal, as long as nothing else requires them (like content-src or connect-src).

@jhthorsen
Copy link
Collaborator

jhthorsen commented Jan 15, 2021

I think doing somehting like this is more readable and it's also easier to read the diff, in case we need to change one of the parameters.

package Convos;
...
$c->res->headers->content_security_policy($c->app->_content_security_policy);
...
sub _content_security_policy {
  return join(
    ' ',
    map { @$_ == 2 ? sprintf q(%s %s;), @$_ : "$_->[0];" } (
      ['default-src' => q('none')],
      ['base-uri'    => q('self')],
      ['block-all-mixed-content'],
      ['manifest-src'    => q('self')],
      ['object-src'      => q('none')],
      ['frame-ancestors' => q('none')],
      ['frame-src'       => q('self')],
      ['connect-src'     => q('self')],
      ['img-src'         => q(*)],
      ['font-src'        => q('self' fonts.gstatic.com https)],
      ['style-src'       => q('self' fonts.googleapis.com 'unsafe-inline')],
      ['script-src'      => q('self' 'unsafe-inline' 'unsafe-eval')],
    )
  );
}

If possible, please sort the lines. Example: Would be nice if "object-src" could come after "img-src".

@jhthorsen
Copy link
Collaborator

jhthorsen commented Feb 12, 2021

Here is another way to write the code:

sub _content_security_policy {
  return join(' ',
    map {"$_;"} q(block-all-mixed-content),
    q(base-uri 'self'),
    q(connect-src 'self'),
    q(frame-ancestors 'none'),
    q(manifest-src 'self'),
    q(default-src 'none'),
    q(font-src 'self' fonts.gstatic.com https),
    q(frame-src 'self'),
    q(img-src *),
    q(object-src 'none'),
    q(script-src 'self' 'unsafe-inline' 'unsafe-eval'),
    q(style-src 'self' fonts.googleapis.com 'unsafe-inline'),
  );
}

@jhthorsen
Copy link
Collaborator

#508 will get merged one way or another, so I'm closing this issue.

@jhthorsen
Copy link
Collaborator

jhthorsen commented Feb 15, 2021

#508 broke Convos, so I had to revert it. This needs to work with (at least)

  • Embeds with images from third parties
  • Twitter emojis
  • Twitter iframe

I'll try to test it better next time before I make a release.

@jhthorsen jhthorsen reopened this Feb 15, 2021
jhthorsen pushed a commit that referenced this issue Feb 15, 2021
@oXyiGYJ
Copy link
Contributor Author

oXyiGYJ commented Feb 15, 2021

Which section broke emojis and embeds of images?

@jhthorsen jhthorsen modified the milestones: 5.xx, 6.xx Feb 22, 2021
@jhthorsen jhthorsen modified the milestones: 6.xx, Backlog Dec 28, 2021
@jhthorsen
Copy link
Collaborator

Should also have a look at https://digi.ninja/blog/svg_xss.php

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants