launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging ~cjwatson/launchpad:loggerhead-headers into launchpad:master.

Commit message:
Loggerhead: Set most of Launchpad's usual HTTP headers

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/399124

We need to tell the client that our response varies based on authentication, as otherwise you may be trying to debug an OpenID exchange and get stupendously confused by an old cached response from Loggerhead in the middle of it.  More generally, we don't want caches to serve cached responses from Loggerhead unless authentication headers match.

While we're here, also add most of the usual web security headers added by BasicLaunchpadRequest.  I omitted Strict-Transport-Security for now, as bazaar.launchpad.net has historically been happy to serve public resources over HTTP, so we may need to think a bit more carefully about how to transition to that.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:loggerhead-headers into launchpad:master.

diff --git a/lib/launchpad_loggerhead/tests.py b/lib/launchpad_loggerhead/tests.py
index 4c8482c..0bc4145 100644
--- a/lib/launchpad_loggerhead/tests.py
+++ b/lib/launchpad_loggerhead/tests.py
@@ -206,3 +206,24 @@ class TestWSGI(TestCaseWithFactory):
         self.assertEqual(
             versioninfo.revision,
             response.headers['X-Launchpad-Revision'])
+
+    def test_vary_header_present(self):
+        db_branch, _ = self.create_branch_and_tree()
+        branch_url = "http://127.0.0.1:%d/%s"; % (
+            config.codebrowse.port, db_branch.unique_name)
+        response = requests.get(branch_url)
+        self.assertEqual(200, response.status_code)
+        self.assertEqual('Cookie, Authorization', response.headers['Vary'])
+
+    def test_security_headers_present(self):
+        db_branch, _ = self.create_branch_and_tree()
+        branch_url = "http://127.0.0.1:%d/%s"; % (
+            config.codebrowse.port, db_branch.unique_name)
+        response = requests.get(branch_url)
+        self.assertEqual(200, response.status_code)
+        self.assertEqual(
+            "frame-ancestors 'self';",
+            response.headers['Content-Security-Policy'])
+        self.assertEqual('SAMEORIGIN', response.headers['X-Frame-Options'])
+        self.assertEqual('nosniff', response.headers['X-Content-Type-Options'])
+        self.assertEqual('1; mode=block', response.headers['X-XSS-Protection'])
diff --git a/lib/launchpad_loggerhead/wsgi.py b/lib/launchpad_loggerhead/wsgi.py
index f4a9773..3d2892a 100644
--- a/lib/launchpad_loggerhead/wsgi.py
+++ b/lib/launchpad_loggerhead/wsgi.py
@@ -49,6 +49,26 @@ log = logging.getLogger("loggerhead")
 SESSION_VAR = "lh.session"
 
 
+def set_standard_headers(app):
+    def wrapped(environ, start_response):
+        def response_hook(status, response_headers, exc_info=None):
+            response_headers.extend([
+                # Our response always varies based on authentication.
+                ('Vary', 'Cookie, Authorization'),
+
+                # Prevent clickjacking and content sniffing attacks.
+                ('Content-Security-Policy', "frame-ancestors 'self';"),
+                ('X-Frame-Options', 'SAMEORIGIN'),
+                ('X-Content-Type-Options', 'nosniff'),
+                ('X-XSS-Protection', '1; mode=block'),
+                ])
+            return start_response(status, response_headers, exc_info)
+
+        return app(environ, response_hook)
+
+    return wrapped
+
+
 def log_request_start_and_stop(app):
     def wrapped(environ, start_response):
         url = construct_url(environ)
@@ -169,6 +189,7 @@ class LoggerheadApplication(Application):
         app = HTTPExceptionHandler(app)
         app = SessionHandler(app, SESSION_VAR, secret)
         app = RevisionHeaderHandler(app)
+        app = set_standard_headers(app)
         app = log_request_start_and_stop(app)
         app = PrefixMiddleware(app)
         app = oops_middleware(app)