Bug 61360

Summary: Content Security Policy reports should be reported with content-type application/json
Product: WebKit Reporter: Jeff Stewart <jeffstewart>
Component: WebCore Misc.Assignee: Adam Barth <abarth>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, bmt, eric, jeffstewart, mkwst, ossy, sam, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on:    
Bug Blocks: 53572    
Attachments:
Description Flags
Patch eric: review+, webkit.review.bot: commit-queue-

Description Jeff Stewart 2011-05-24 06:17:23 PDT
Using MacOS nightly webkit build 87124, Content-Security-Policy violations are being reported with content-type application/x-www-form-urlencoded (and the payload is indeed form encoded).  The spec (http://dvcs.w3.org/hg/content-security-policy/raw-file/tip/csp-specification.dev.html#report-uri) says:

The violation report must be sent via an HTTP POST request whose body is comprised of a JSON object containing violation information, and the request must have a Content-Type of application/json.

On a related note, the report does not contain the blocked-uri field:

http://dvcs.w3.org/hg/content-security-policy/raw-file/tip/csp-specification.dev.html#violation-report-syntax

Shows that the following fields should be sent:

request: HTTP request line of the protected resource whose policy was violated including method, URI and HTTP version
request-headers: HTTP request headers sent with the request for the protected resource whose policy was violated
blocked-uri: URI of the resource that was prevented from loading due to the policy violation
violated-directive: The policy directive that was violated
original-policy: The original policy as received by the user-agent. If the policy was received via more than one Content Security Policy response header, this field must contain a comma separated list of original policies.

To reproduce:

The python script at the end of this report will reproduce the problem.  Here's an example capture of a CSP report sent to that server (Cookie header elided, hostnames blacked out):

Host: XXXXXXXX.XXX.XXXX.XXXXXX.com:10000
Origin: null
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_6_4) AppleWebKit/535.1+ (KHTML, like Gecko) Version/5.0.4 Safari/533.20.27
Content-Type: application/x-www-form-urlencoded
Referer: http://XXXXXXXX.XXX.XXXX.XXXXXX.com:10002/
Accept: */*
Accept-Language: en-us
Accept-Encoding: gzip, deflate
Content-Length: 107
Connection: keep-alive

document-url=http%3A%2F%2FXXXXXXXX.XXX.XXXX.XXXXXX.com%3A10000%2F&violated-directive=default-src+%27self%27

Here's a python script that can help reproduce the issue:

from BaseHTTPServer import BaseHTTPRequestHandler
import socket
import SocketServer
import sys

DOCUMENT = """<html><head></head><body>Hi
<img src="http://some.other.server/does/not/exist.jpg"></body></html>"""
PORT = 10000

class MyHandler(BaseHTTPRequestHandler):
  def do_POST(self):
    print self.headers
    bytes = int(self.headers['Content-Length'])
    payload = self.rfile.read(bytes)
    print payload
    self.send_response(200)
    self.end_headers()

  def do_GET(self):
    path = self.path.split('?', 1)[0]
    path = path.split('#', 1)[0]
    if path != '/':
      self.send_response(404)
      return
    ctype = 'text/html'
    length = len(DOCUMENT)
    self.send_response(200)
    # This is supposed to be default-src rather than allow.
    # It will probably break sometime in the future.
    self.send_header('X-Content-Security-Policy-Report-Only',
                     'allow https: data:; '
                     'report-uri http://%s:%s/csp-report' % (
                         socket.gethostname(), PORT))
    # Report-Only is not in the Chrome canary build yet.
    self.send_header('X-WebKit-CSP', 'default-src \'self\'; '
                     'report-uri http://%s:%d/csp-report' % (
                         socket.gethostname(), PORT))
    self.send_header('Content-Type', ctype)
    self.send_header('Content-Length', str(length))
    self.end_headers()
    print >>self.wfile, DOCUMENT,

def main(argv):
  global PORT
  if len(argv) > 1:
    PORT = int(argv[1])
  print 'Listening at http://%s:%d/' % (socket.gethostname(), PORT)
  httpd = SocketServer.TCPServer(("", PORT), MyHandler)
  httpd.serve_forever()



if __name__ == '__main__':
  main(sys.argv)
Comment 1 Adam Barth 2011-05-26 08:53:51 PDT
Thanks for the report.

> Using MacOS nightly webkit build 87124, Content-Security-Policy violations are being reported with content-type application/x-www-form-urlencoded (and the payload is indeed form encoded).  The spec (http://dvcs.w3.org/hg/content-security-policy/raw-file/tip/csp-specification.dev.html#report-uri) says:

This is intentional (at least for the moment).  We've recommended to the working group that the report should be sent form-urlencoded.

> On a related note, the report does not contain the blocked-uri field:

This is also intentional.  There are some subtle security issues with sending the blocked-uri, especially if cross-origin redirects are involved.  There's a spec change in the works to address this issue, which we'll implement once all the details are in the spec.

Thanks!
Comment 2 Adam Barth 2011-10-13 15:45:26 PDT
Sam said he might be interested in tackling this issue.
Comment 3 Adam Barth 2012-05-04 15:49:11 PDT
Created attachment 140347 [details]
Patch
Comment 4 Eric Seidel (no email) 2012-05-04 15:50:54 PDT
Comment on attachment 140347 [details]
Patch

LGTM.
Comment 5 WebKit Review Bot 2012-05-04 15:55:37 PDT
Comment on attachment 140347 [details]
Patch

Rejecting attachment 140347 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2

Last 500 characters of output:
y/report-only-from-header-expected.txt
patching file LayoutTests/http/tests/security/contentSecurityPolicy/report-uri-expected.txt
patching file LayoutTests/http/tests/security/contentSecurityPolicy/report-uri-from-child-frame-expected.txt
patching file LayoutTests/http/tests/security/contentSecurityPolicy/resources/save-report.php

Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', u'--force', u'--reviewer', u'Eric Seidel']" exit_code: 1 cwd: /mnt/git/webkit-commit-queue/

Full output: http://queues.webkit.org/results/12630290
Comment 6 Adam Barth 2012-05-04 15:58:19 PDT
This patch depends on the patch in Bug 85561.
Comment 7 Adam Barth 2012-05-04 16:17:27 PDT
Containing all the required fields is Bug 85682.
Comment 8 Adam Barth 2012-05-06 21:33:36 PDT
Committed r116268: <http://trac.webkit.org/changeset/116268>
Comment 9 Csaba Osztrogonác 2012-05-06 22:40:14 PDT
Reopen, because it broke the !INSPECTOR builds:
../../../../Source/WebCore/page/ContentSecurityPolicy.cpp: In member function 'void WebCore::CSPDirectiveList::reportViolation(const WTF::String&, const WTF::String&) const':
../../../../Source/WebCore/page/ContentSecurityPolicy.cpp:605: error: incomplete type 'WebCore::InspectorObject' used in nested name specifier
../../../../Source/WebCore/page/ContentSecurityPolicy.cpp:606: error: invalid use of incomplete type 'struct WebCore::InspectorObject'
../../../../Source/WebCore/inspector/ScriptCallFrame.h:43: error: forward declaration of 'struct WebCore::InspectorObject'
../../../../Source/WebCore/page/ContentSecurityPolicy.cpp:608: error: invalid use of incomplete type 'struct WebCore::InspectorObject'
../../../../Source/WebCore/inspector/ScriptCallFrame.h:43: error: forward declaration of 'struct WebCore::InspectorObject'
../../../../Source/WebCore/page/ContentSecurityPolicy.cpp:610: error: incomplete type 'WebCore::InspectorObject' used in nested name specifier
../../../../Source/WebCore/page/ContentSecurityPolicy.cpp:611: error: invalid use of incomplete type 'struct WebCore::InspectorObject'
../../../../Source/WebCore/inspector/ScriptCallFrame.h:43: error: forward declaration of 'struct WebCore::InspectorObject'
../../../../Source/WebCore/page/ContentSecurityPolicy.cpp:613: error: invalid use of incomplete type 'struct WebCore::InspectorObject'
../../../../Source/WebCore/inspector/ScriptCallFrame.h:43: error: forward declaration of 'struct WebCore::InspectorObject'
In file included from ../../../../Source/WTF/wtf/RefPtr.h:29,
                 from ../../../../Source/WTF/wtf/VectorTraits.h:26,
                 from ../../../../Source/WTF/wtf/Vector.h:31,
                 from ../../../../Source/WebCore/page/ContentSecurityPolicy.h:32,
                 from ../../../../Source/WebCore/page/ContentSecurityPolicy.cpp:28:
../../../../Source/WTF/wtf/PassRefPtr.h: In function 'void WTF::derefIfNotNull(T*) [with T = WebCore::InspectorObject]':
../../../../Source/WTF/wtf/RefPtr.h:56:   instantiated from 'WTF::RefPtr<T>::~RefPtr() [with T = WebCore::InspectorObject]'
../../../../Source/WebCore/page/ContentSecurityPolicy.cpp:605:   instantiated from here
../../../../Source/WTF/wtf/PassRefPtr.h:52: error: invalid use of incomplete type 'struct WebCore::InspectorObject'
../../../../Source/WebCore/inspector/ScriptCallFrame.h:43: error: forward declaration of 'struct WebCore::InspectorObject'
Comment 10 Adam Barth 2012-05-06 22:48:16 PDT
Hum...  Maybe InspectorValues need to be defined even without the inspector.
Comment 11 Adam Barth 2012-05-06 22:55:25 PDT
Attempted fix in http://trac.webkit.org/changeset/116275
Comment 12 Adam Barth 2012-05-06 23:02:55 PDT
That seems to have worked.