zendframework / zend-http

Http component from Zend Framework
BSD 3-Clause "New" or "Revised" License
134 stars 85 forks source link

ContentSecurityPolicy headers overwrite each other #159

Closed markushausammann closed 5 years ago

markushausammann commented 6 years ago

It's often not allowed or recommended to have several headers with the same name. But there are situations where it is allowed or even necessary. The CSP is one of these.

https://w3c.github.io/webappsec-csp/#multiple-policies

and

https://www.w3.org/TR/CSP2/#content-security-policy-header-field

Imagine a case where a main application creates a CSP and different modules also independently add their own CSPs. The framework MUST render them all separately OR do a preemptive union merge which is what the client would otherwise do. It will (hopefully) quickly become standard practice that modules provide their own CSPs.

Code to reproduce the issue

$headers = $controller->getResponse()->getHeaders();
$headers->addHeader(new ContentSecurityPolicy($someDirectives));
$headers->addHeader(new ContentSecurityPolicy($someOtherDirectives));

Expected results

The expected result is a response with two CSP headers (OR a union merged CSP).

Actual results

The second addition overwrites the first, the response only contains that one CSP.

markushausammann commented 6 years ago

PS: It's not enough to have ContentSecurityPolicy implement MultipleHeaderInterface I think. The framework also needs to make sure to check for existing policies and append them automatically.

markushausammann commented 6 years ago

Also I'm trying to understand the MultipleHeaderInterface and I've tried to add such a multi header string via addHeaderLine but that makes Headers throw an exception on line 192: A field name was provided without a field value.

:-o

markushausammann commented 6 years ago

Am I even in the right repository? Seems this one hasn't really been touched for quite some time.

Ocramius commented 6 years ago

Yeah, the repo is correct, but unless someone picks up a failing test case + patch, it's not gonna be solved. If you can manage to create a failing test, that already simplifies things enormously.

markushausammann commented 6 years ago

Ok, I'll try to find time for something... not that I'm the right person to do that.

Ocramius commented 6 years ago

Yes you are: you know exactly what the bug looks like 👍