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 sources are incorrectly removed when both wildcards and schemes are present #376

Open
tessereth opened this issue Nov 29, 2017 · 5 comments

Comments

@tessereth
Copy link

When I have the following as part of my csp header config:

  config.csp = {
    frame_ancestors: %w[*.foo.com http://www.foo.com],
    preserve_schemes: true,
    ...
  }

and I'm serving my frame over https to be embedded on http://www.foo.com, the http://www.foo.com is required but is being removed by minify_source_list

Expected outcome

I expect to have frame-ancestors *.foo.com http://www.foo.com in my CSP header

Actual outcome

The CSP header had only frame-ancestors *.foo.com

Config

SecureHeaders::Configuration.default do |config|
  config.csp = {
    default_src: %w['self'],
    script_src: %w['self'],
    frame_ancestors: %w[*.foo.com http://www.foo.com],
    preserve_schemes: true
  }
end

Generated headers

Content-Security-Policy: default-src 'self'; frame-ancestors *.foo.com; script-src 'self'

@jacobbednarz
Copy link
Contributor

I think this is correct however I'll need to re-read the match source expression another 10 times to let it sync in and work out whether this is valid policy or not. There is also a section on frame ancestors multiple source values which you might want to keep in mind.

Out of curiosity, have you tried frame_ancestors: %w[https://*.foo.com http://www.foo.com]? Veryyy sketchy and ill advised but might help here.

@tessereth
Copy link
Author

That is indeed the correct response to reading the match source expression document 😜 . As for the other one, I hadn't seen it so thanks. We already do something along those lines but we can still end up with more than one entry with out current solution.

I can definitely work around this if I need to (by only putting one entry in the list or adding the https prefix, which does work thanks) but I figured if it's a bug here then it would make sense to fix it here. I'm sure someone else will run into it at some stage.

@jacobbednarz
Copy link
Contributor

Personally, I'd recommend always being specific about the protocol to avoid DDoSing your reporting reporting endpoint due to broken Safari.

@oreoshake
Copy link
Contributor

Personally, I'd recommend always being specific about the protocol

secure_headers is pretty aggressive about stripping protocols. GitHub doesn't ingest reports, is this causing an issue for others or does the current behavior work well enough?

@jacobbednarz
Copy link
Contributor

I'll be able to give you an idea very soon 😄 Right now I'm just wrapping up some infrastructure testing combined with scaling out https://github.com/jacobbednarz/go-csp-collector and we'll be ingesting all of ours to perform analysis. I too would like to drop the protocols however it still looks like the safari bug is a thing for us but easy enough to work around.

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

No branches or pull requests

3 participants