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 header format dependent on user agent #16

Closed
wants to merge 2 commits into
base: master
from

Conversation

Projects
None yet
4 participants
@renehamburger

renehamburger commented Sep 12, 2013

Using the correct format is essential for the browser to accept the CSP.
This is particularly important for Firefox < 23 as it used a different set of
directives.
The option 'enforcePolicy' has been added to determine whether unrecognised user agents
should receive all 3 main CSP header formats (enforcePolicy=true) or none.
The main user agents have been added at this stage but the list should be refined to
include other browsers (especially mobile browsers) that support CSP.

@Nilos Nilos referenced this pull request Sep 12, 2013

Closed

X-Webkit-CSP deprecated #7

Show outdated Hide outdated lib/middleware/csp.js
Show outdated Hide outdated lib/middleware/csp.js
@evilpacket

This comment has been minimized.

Show comment
Hide comment
@evilpacket

evilpacket Sep 13, 2013

Thanks @Nilos for the PR review. @renehamburger can you fix those few things up?

Also do we have any test cases / policies or something for each of these so I can test quick?

evilpacket commented Sep 13, 2013

Thanks @Nilos for the PR review. @renehamburger can you fix those few things up?

Also do we have any test cases / policies or something for each of these so I can test quick?

@renehamburger

This comment has been minimized.

Show comment
Hide comment
@renehamburger

renehamburger Sep 13, 2013

Apologies, the commit I based the pull request on was the second last! All debugging was removed in the final commit. I'll update the pull request tomorrow when I'm back at work.

-------- Original Message --------
From: Adam Baldwin notifications@github.com
Sent: Fri Sep 13 10:12:57 GMT+02:00 2013
To: evilpacket/helmet helmet@noreply.github.com
Cc: Rene Hamburger rene.hamburger@gmail.com
Subject: Re: [helmet] CSP header format dependent on user agent (#16)

Thanks @Nilos for the PR review. @renehamburger can you fix those few things up?

Also do we have any test cases / policies or something for each of these so I can test quick?


Reply to this email directly or view it on GitHub:

#16 (comment)

Sent from my Android device with K-9 Mail. Please excuse my brevity.

renehamburger commented Sep 13, 2013

Apologies, the commit I based the pull request on was the second last! All debugging was removed in the final commit. I'll update the pull request tomorrow when I'm back at work.

-------- Original Message --------
From: Adam Baldwin notifications@github.com
Sent: Fri Sep 13 10:12:57 GMT+02:00 2013
To: evilpacket/helmet helmet@noreply.github.com
Cc: Rene Hamburger rene.hamburger@gmail.com
Subject: Re: [helmet] CSP header format dependent on user agent (#16)

Thanks @Nilos for the PR review. @renehamburger can you fix those few things up?

Also do we have any test cases / policies or something for each of these so I can test quick?


Reply to this email directly or view it on GitHub:

#16 (comment)

Sent from my Android device with K-9 Mail. Please excuse my brevity.

@evilpacket

This comment has been minimized.

Show comment
Hide comment
@evilpacket

evilpacket Sep 13, 2013

No worries, thanks for your help on making this a better library. CSP
support in helmet does need love.

Rene Hamburger wrote:

Apologies, the commit I based the pull request on was the second last!
All debugging was removed in the final commit. I'll update the pull
request tomorrow when I'm back at work.

-------- Original Message --------
From: Adam Baldwin notifications@github.com
Sent: Fri Sep 13 10:12:57 GMT+02:00 2013
To: evilpacket/helmet helmet@noreply.github.com
Cc: Rene Hamburger rene.hamburger@gmail.com
Subject: Re: [helmet] CSP header format dependent on user agent (#16)

Thanks @Nilos for the PR review. @renehamburger can you fix those few
things up?

Also do we have any test cases / policies or something for each of
these so I can test quick?


Reply to this email directly or view it on GitHub:

#16 (comment)

Sent from my Android device with K-9 Mail. Please excuse my brevity.


Reply to this email directly or view it on GitHub
#16 (comment).

evilpacket commented Sep 13, 2013

No worries, thanks for your help on making this a better library. CSP
support in helmet does need love.

Rene Hamburger wrote:

Apologies, the commit I based the pull request on was the second last!
All debugging was removed in the final commit. I'll update the pull
request tomorrow when I'm back at work.

-------- Original Message --------
From: Adam Baldwin notifications@github.com
Sent: Fri Sep 13 10:12:57 GMT+02:00 2013
To: evilpacket/helmet helmet@noreply.github.com
Cc: Rene Hamburger rene.hamburger@gmail.com
Subject: Re: [helmet] CSP header format dependent on user agent (#16)

Thanks @Nilos for the PR review. @renehamburger can you fix those few
things up?

Also do we have any test cases / policies or something for each of
these so I can test quick?


Reply to this email directly or view it on GitHub:

#16 (comment)

Sent from my Android device with K-9 Mail. Please excuse my brevity.


Reply to this email directly or view it on GitHub
#16 (comment).

CSP header format dependent on user agent
Using the correct format is essential for the browser to accept the CSP.
This is particularly important for Firefox < 23 as it used a different set of
directives.
The option 'enforcePolicy' has been added to determine whether unrecognised user agents
should receive all 3 main CSP header formats (enforcePolicy=true) or none.
The main user agents have been added at this stage but the list should be refined to
include other browsers (especially mobile browsers) that support CSP.
IE allows for CSP headers only when sandboxing is activated. It can be activated by elements like
{ sandbox: [] } or { sandbox: ['allow-forms', 'allow-scripts'] } to the policy.
@renehamburger

This comment has been minimized.

Show comment
Hide comment
@renehamburger

renehamburger Sep 14, 2013

I've amended the original commit as my final changes added very little new content.

I haven't got any test cases, I've only been testing it with our setup and a few browsers (in particular the current and older Firefox versions).

As you'll see in the code, older Firefox browsers (v.4-22) handle the following directives differently:

  • script-src 'unsafe-inline' = options inline-script
  • script-src 'unsafe-eval' = options eval-script
  • style-src unsafe-inline is ignored
  • default-src = allow for FF4
  • missing default-src equates to default-src 'none' rather than default-src *
  • connect-src = xhr-src

If you're able to test with FF22 or older you should include some of these options.

I'm not aware of any of the other CSP-compatible browsers departing from what is now defined in the CSP 1.0 specs. So for them the only difference is the title of the header-field.

renehamburger commented Sep 14, 2013

I've amended the original commit as my final changes added very little new content.

I haven't got any test cases, I've only been testing it with our setup and a few browsers (in particular the current and older Firefox versions).

As you'll see in the code, older Firefox browsers (v.4-22) handle the following directives differently:

  • script-src 'unsafe-inline' = options inline-script
  • script-src 'unsafe-eval' = options eval-script
  • style-src unsafe-inline is ignored
  • default-src = allow for FF4
  • missing default-src equates to default-src 'none' rather than default-src *
  • connect-src = xhr-src

If you're able to test with FF22 or older you should include some of these options.

I'm not aware of any of the other CSP-compatible browsers departing from what is now defined in the CSP 1.0 specs. So for them the only difference is the title of the header-field.

Removed sandbox directive for unsupported browsers
Firefox does not yet support the CSP sandbox directive
(see https://wiki.mozilla.org/Features/Platform/CSP_Sandbox)
and throws an error message if it finds it. For now I've removed
it for all browsers except for IE10+ and Chrome 14+. I wasn't able
to determine when Chrome started to support this feature.
For all unrecognised browsers 'enforcePolicy' will determine whether
the sandbox directive is removed or not.
@EvanHahn

This comment has been minimized.

Show comment
Hide comment
@EvanHahn

EvanHahn Nov 28, 2013

Member

Merged, but GitHub didn't register it.

Thanks! I'll clean this up a bit and add tests.

Member

EvanHahn commented Nov 28, 2013

Merged, but GitHub didn't register it.

Thanks! I'll clean this up a bit and add tests.

@EvanHahn EvanHahn closed this Nov 28, 2013

@EvanHahn

This comment has been minimized.

Show comment
Hide comment
@EvanHahn

EvanHahn Aug 5, 2016

Member

@renehamburger I'm making a new website for Helmet.js and I want to credit everyone who's contributed. Do you have a name and/or website you'd like me to use to credit you?

Member

EvanHahn commented Aug 5, 2016

@renehamburger I'm making a new website for Helmet.js and I want to credit everyone who's contributed. Do you have a name and/or website you'd like me to use to credit you?

@renehamburger

This comment has been minimized.

Show comment
Hide comment
@renehamburger

renehamburger Aug 9, 2016

@EvanHahn: My public GitHub activity is minimal, but just use https://github.com/renehamburger. Thanks!

renehamburger commented Aug 9, 2016

@EvanHahn: My public GitHub activity is minimal, but just use https://github.com/renehamburger. Thanks!

@EvanHahn

This comment has been minimized.

Show comment
Hide comment
@EvanHahn

EvanHahn Aug 9, 2016

Member
Member

EvanHahn commented Aug 9, 2016

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