Skip to content
This repository has been archived by the owner on Apr 21, 2023. It is now read-only.

Commit

Permalink
CSP vs httpd's err_headers_out
Browse files Browse the repository at this point in the history
Using php and/or using "Headers always" in configuration to set up CSP
headers in httpd is probably pretty common. These will both end up in
err_headers_out, a httpd-only concept meaning that the headers should
always be emitted, even on error statusses. We want to consider these
CSP headers living in this collection when rewriting content, but leave
them alone otherwise.
Instead of introducing an err_headers_out (like) concept in RewriteDriver
to model this, we will rely on the internal headers concept. ScanFilter
will also look at kInternalContentSecurityPolicy. This header will be
sanitized from our own output.

Relies on #1695
  • Loading branch information
oschaaf committed Dec 19, 2017
1 parent c587e35 commit acd839e
Show file tree
Hide file tree
Showing 6 changed files with 50 additions and 5 deletions.
2 changes: 2 additions & 0 deletions net/instaweb/rewriter/public/scan_filter.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
#define NET_INSTAWEB_REWRITER_PUBLIC_SCAN_FILTER_H_

#include "pagespeed/kernel/html/empty_html_filter.h"
#include "pagespeed/kernel/base/string_util.h"

namespace net_instaweb {

Expand Down Expand Up @@ -55,6 +56,7 @@ class ScanFilter : public EmptyHtmlFilter {
virtual const char* Name() const { return "Scan"; }

private:
void UpdateCspFromHeaderValues(const ConstStringStarVector& values);
RewriteDriver* driver_;
bool seen_any_nodes_;
bool seen_refs_;
Expand Down
18 changes: 13 additions & 5 deletions net/instaweb/rewriter/scan_filter.cc
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@
#include "pagespeed/kernel/base/charset_util.h"
#include "pagespeed/kernel/base/statistics.h"
#include "pagespeed/kernel/base/string.h"
#include "pagespeed/kernel/base/string_util.h"
#include "pagespeed/kernel/html/html_element.h"
#include "pagespeed/kernel/html/html_name.h"
#include "pagespeed/kernel/html/html_node.h"
Expand All @@ -47,6 +46,14 @@ ScanFilter::ScanFilter(RewriteDriver* driver)
ScanFilter::~ScanFilter() {
}

void ScanFilter::UpdateCspFromHeaderValues(
const ConstStringStarVector& values) {
for (const GoogleString* policy : values) {
driver_->mutable_content_security_policy()->AddPolicy(
CspPolicy::Parse(*policy));
}
}

void ScanFilter::StartDocument() {
// TODO(jmarantz): consider having rewrite_driver access the url in this
// class, rather than poking it into rewrite_driver.
Expand All @@ -65,10 +72,11 @@ void ScanFilter::StartDocument() {
if (driver_->options()->honor_csp() && headers != nullptr) {
ConstStringStarVector values;
if (headers->Lookup(HttpAttributes::kContentSecurityPolicy, &values)) {
for (const GoogleString* policy : values) {
driver_->mutable_content_security_policy()->AddPolicy(
CspPolicy::Parse(*policy));
}
UpdateCspFromHeaderValues(values);
}
values.clear();
if (headers->Lookup(HttpAttributes::kInternalContentSecurityPolicy, &values)) {
UpdateCspFromHeaderValues(values);
}
}
}
Expand Down
18 changes: 18 additions & 0 deletions net/instaweb/rewriter/scan_filter_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,24 @@ TEST_F(ScanFilterTest, CspParse) {
GoogleUrl("http://www.example.org/foo.png"), CspDirective::kImgSrc));
}

TEST_F(ScanFilterTest, CspParseInternal) {
ResponseHeaders headers;
headers.Add("@Content-Security-Policy", "img-src https:");
rewrite_driver()->set_response_headers_ptr(&headers);
ValidateNoChanges("csp_parse",
"<meta http-equiv=\"Content-Security-Policy\" "
"content=\"img-src www.example.com\">");
EXPECT_EQ(2, rewrite_driver()->content_security_policy().policies_size());
EXPECT_TRUE(rewrite_driver()->IsLoadPermittedByCsp(
GoogleUrl("https://www.example.com/foo.png"), CspDirective::kImgSrc));
EXPECT_FALSE(rewrite_driver()->IsLoadPermittedByCsp(
GoogleUrl("http://www.example.com/foo.png"), CspDirective::kImgSrc));
EXPECT_FALSE(rewrite_driver()->IsLoadPermittedByCsp(
GoogleUrl("https://www.example.org/foo.png"), CspDirective::kImgSrc));
EXPECT_FALSE(rewrite_driver()->IsLoadPermittedByCsp(
GoogleUrl("http://www.example.org/foo.png"), CspDirective::kImgSrc));
}

TEST_F(ScanFilterTest, CspParseOff) {
options()->set_honor_csp(false);

Expand Down
13 changes: 13 additions & 0 deletions pagespeed/apache/instaweb_context.cc
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,19 @@ void InstawebContext::Finish() {
void InstawebContext::PopulateHeaders(request_rec* request) {
if (!populated_headers_) {
ApacheRequestToResponseHeaders(*request, response_headers_.get(), NULL);
// Using php and/or using "Headers always" in configuration to set up CSP
// headers in httpd is probably pretty common. These will both end up in
// err_headers_out, a httpd-only concept meaning that the headers should
// always be emitted, even on error statusses. We want to consider these
// CSP headers living in this collection when rewriting content, but leave
// them alone otherwise.
// Instead of introducing an err_headers_out (like) concept in RewriteDriver
// to model this, we will rely on the internal headers concept. ScanFilter
// will also look at kInternalContentSecurityPolicy. This header will be
// sanitized from our own output.
const char* csp = apr_table_get(request->err_headers_out,
HttpAttributes::kContentSecurityPolicy);
response_headers_->Add(HttpAttributes::kInternalContentSecurityPolicy, csp);
populated_headers_ = true;
}
}
Expand Down
2 changes: 2 additions & 0 deletions pagespeed/kernel/http/http_names.cc
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,8 @@ const char HttpAttributes::kXUACompatible[] = "X-UA-Compatible";
const char HttpAttributes::kXSendfile[] = "X-Sendfile";
const char HttpAttributes::kXAccelRedirect[] = "X-Accel-Redirect";
const char HttpAttributes::kXPageSpeedLoop[] = "X-PageSpeed-Loop";
const char HttpAttributes::kInternalContentSecurityPolicy[] =
"@Content-Security-Policy";

const char* HttpStatus::GetReasonPhrase(HttpStatus::Code rc) {
switch (rc) {
Expand Down
2 changes: 2 additions & 0 deletions pagespeed/kernel/http/http_names.h
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,8 @@ struct HttpAttributes {
static const char kXAccelRedirect[];
// PageSpeed Loop detection for proxy mode.
static const char kXPageSpeedLoop[];
// Any CSP httpd hands to us in err_response_headers will be stored here.
static const char kInternalContentSecurityPolicy[];

// Gets a sorted StringPieceVector containing all the end-to-end headers.
// Any fields listed in here should be ignored during sanitization when they
Expand Down

0 comments on commit acd839e

Please sign in to comment.