add_header quoting issue for Content-Security-Policy

[bryangwilliam]

I am unable to use add_header for adding a Content-Security-Policy header to a server. Because the CSP syntax uses single-quotes explicitly (https://content-security-policy.com/) for certain terms, the single-quotes that are being used to wrap the header value (added in #804) cause the nginx syntax to break.

Passing { "Content-Security-Policy" : "default-src 'self';" } into the $add_header parameter of nginx::resource::server will result in the configuration add_header 'Content-Security-Policy' 'default-src 'self';', which is invalid. I tried using double-quotes in the CSP header as a workaround, but it breaks on certain keywords in the browser. You can use the raw_prepend option as a workaround, but a more elegant solution would be nice.

This could be fixed by either

• changing to double-quotes in the template when applying the add_header values
• allowing users to supply their own quotes
• something else ?

[wyardley]

Not sure how
https://github.com/voxpupuli/puppet-nginx/blob/master/spec/defines/resource_server_spec.rb#L1197-L1227
are passing as of now, since they're not matching the single quotes.

Does #992 resolve this for you? (note that if you're not using git, you may need to change the filenames, due to the vhost => server rename)

[bryangwilliam]

That seems to do the trick.

Your comment about possibly having double quotes in the header is valid, but I just did some research and found that DQUOTE is the acceptable delimiter for quoted-strings within the HTTP syntax (https://tools.ietf.org/html/rfc7230 "3.2.6. Field Value Components"). As best I can tell, this means there is no case where you can include an unescaped double-quote within a header value (DQUOTE is explicitly disallowed as a value in a token and must be escaped within a quoted-string since double-quotes are the delimiters). Granted this is just my interpretation, but it makes sense in my head.

[benh57]

I have the same problem but wiht the 'always' parameter to Access-Control-Allow-Origin. This used to work with this module, but now that the value is being single quoted, it fails.

  add_header           =>  { 'Access-Control-Allow-Origin'      => '"*" always' } ,

results in:

add_header 'Access-Control-Allow-Origin' '"*" always';

Which fails in the browser with:

The 'Access-Control-Allow-Origin' header contains multiple values '"*" always', but only one is allowed.

[bryangwilliam]

I have been doing some more reading on this, and I think my initial interpretation was incorrect.

Single quotes are definitely allowed as part of a token and in some cases are a necessary part of the syntax, so always wrapping in single quotes is not going to work across the board. In conflict with my previous assumption, it would appear that although DQUOTE cannot be used within a token, quoted-strings are a valid part of the common syntax, meaning they most definitely can be used (the example from @benh57 above being a perfect example).

Another thing from the @benh57 example: the 'always' options is the decisive point for me. Since it is an nginx directive and not actually part of the header, it cannot be quoted with either style and still work correctly.

After all this, and looking at the Nginx documentation relating to this (http://nginx.org/en/docs/http/ngx_http_headers_module.html), I believe the correct solution should actually be not to use any quotes when building add_header directives, effectively undoing the change in #804. If a particular header needs the quotes, such as the examples in this issue or the example which #804 fixed, then the user ought to provide the quotes explicitly rather than having the module enforce one particular methodology.

My case would then work by allowing me to specify the single quote as a part of my CSP
add_header => { 'Content-Security-Policy' => 'default-src \'self\';' }
the example from @benh57 would work like it did before, and the example from #804 could be made to work by changing it to
add_header => { 'Access-Control-Allow-Methods' => '\'GET, POST, OPTIONS\'' }

It does make the header declarations slightly less pretty, but seems to be the only approach that satisfies all situations.

[bryangwilliam]

@wyardley I know it has been awhile since the initial offer, but I would be interested in getting an environment set up so I can try my hand at contributing some code to the community. Could you point me in the right direction to get that started? I looked over the documentation for anything regarding the test environment and contribution guidelines and couldn't find anything.

[wyardley]

@bryangwilliam I know it's in an odd spot, but check out https://github.com/voxpupuli/puppet-nginx/blob/master/.github/CONTRIBUTING.md (and all files in that directory). There is a bit of work involved in getting the tests setup, but once you've got it going, it's all pretty easy.

You can also pop in #voxpupuli on Freenode (IRC) or in the Puppet Community Slack:
https://voxpupuli.org/docs/
folks there will generally be happy to help.