CSP: Add 'script-sample' to violation reports.

CSP: Add 'sample' to violation reports.

This patch sketches out a 'sample' attribute similar to what Firefox
has been shipping as 'script-sample' for eons. The key distinctions are:

1.  This approach requires opt-in from the site, via a new
    `'report-sample'` expression in the relevant directive.

2.  We're including inline style violations as well.

Let's see how the spec discussion goes.

Spec: https://github.com/w3c/webappsec-csp/issues/119
Intent to Implement: https://groups.google.com/a/chromium.org/d/msg/blink-dev/6W9r_sX3zTQ/5XCSBUQBEAAJ

BUG=606774
Review-Url: https://codereview.chromium.org/2436003002
Cr-Commit-Position: refs/heads/master@{#454232}
Committed: https://chromium.googlesource.com/chromium/src/+/f98f5c2ccea76c9d7e76646369e2f51e0c926521

[Mike West, 2016-10-20 14:18:56]

The CQ bit was checked by mkwst@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-20 14:19:18]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-20 15:44:27]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-20 15:44:28]

Dry run: Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Mike West, 2016-10-24 14:08:47]

The CQ bit was checked by mkwst@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-24 14:09:03]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Mike West, 2016-10-24 14:09:28]

mkwst@chromium.org changed reviewers:
+ jochen@chromium.org

[Mike West, 2016-10-24 14:09:28]

WDYT of this approach, Jochen? If you're not terribly opposed, I'll spec it out
and send an intent to implement.

[Mike West, 2016-10-24 14:12:31]

Description was changed from

==========
WIP: Sketching out 'script-sample'.

BUG=
==========

to

==========
CSP: Add 'script-sample' to violation reports.

TODO(mkwst): Write a spec. Describe the feature.

BUG=TODO(mkwst)
==========

[jochen (gone - plz use gerrit), 2016-10-24 14:15:38]

yeah, looks good

[commit-bot: I haz the power, 2016-10-24 15:38:37]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-24 15:38:38]

Dry run: Try jobs failed on following builders:
  android_clang_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_clan...)

[Mike West, 2016-11-29 09:37:19]

The CQ bit was checked by mkwst@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-29 09:37:39]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-29 09:48:42]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-29 09:48:42]

Dry run: Try jobs failed on following builders:
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Mike West, 2017-02-17 11:01:54]

On 2016/10/24 at 14:15:38, jochen wrote:
> yeah, looks good

Let's pick this back up. The `report-uri` keyword won't work because it's
treated as a URL in every browser. Maybe we can put it in `script-src` instead.

[Mike West, 2017-02-17 11:49:14]

The CQ bit was checked by mkwst@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-17 11:49:36]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-17 12:01:34]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-17 12:01:35]

Dry run: Try jobs failed on following builders:
  cast_shell_android on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)
  linux_chromium_tsan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Mike West, 2017-02-22 11:00:55]

Description was changed from

==========
CSP: Add 'script-sample' to violation reports.

TODO(mkwst): Write a spec. Describe the feature.

BUG=TODO(mkwst)
==========

to

==========
CSP: Add 'script-sample' to violation reports.

This patch sketches out a 'script-sample' feature similar to what Firefox
has been shipping for eons. The key distinction is that this approach requires
opt-in from the site, via a new `'report-sample'` expression in `script-src`.

Spec: TODO(mkwst)
Intent: TODO(mkwst)

BUG=606774
==========

[Mike West, 2017-02-22 11:01:53]

mkwst@chromium.org changed reviewers:
+ andypaicu@chromium.org

[Mike West, 2017-02-22 11:01:53]

WDYT, Jochen and Andy?

CCing aaj@, mikispag@, lwe@. Maybe they'd like to write more tests? :)

[Mike West, 2017-02-22 11:02:00]

The CQ bit was checked by mkwst@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-22 11:02:51]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-22 12:30:35]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-22 12:30:36]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[andypaicu, 2017-02-22 12:47:03]

https://codereview.chromium.org/2436003002/diff/60001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/securitypolicyviolation/script-sample.html
(right):

https://codereview.chromium.org/2436003002/diff/60001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/securitypolicyviolation/script-sample.html:76:
assert_ureached('eval');
Typo here "assert_unreached"

https://codereview.chromium.org/2436003002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/frame/csp/CSPDirectiveList.cpp (right):

https://codereview.chromium.org/2436003002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/frame/csp/CSPDirectiveList.cpp:412:
directive->allowReportSample() ? source : emptyString);
Minor nit: perhaps we can start passing from here the already sanitized and
trimmed sample. Skips having to check for experimental features twice.

[Mike West, 2017-02-22 12:52:46]

The CQ bit was checked by mkwst@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-22 12:53:04]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[lwe, 2017-02-22 13:26:44]

On 2017/02/22 12:47:03, andypaicu wrote:
>
https://codereview.chromium.org/2436003002/diff/60001/third_party/WebKit/Layo...
> File
>
third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/securitypolicyviolation/script-sample.html
> (right):
> 
>
https://codereview.chromium.org/2436003002/diff/60001/third_party/WebKit/Layo...
>
third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/securitypolicyviolation/script-sample.html:76:
> assert_ureached('eval');
> Typo here "assert_unreached"
> 
>
https://codereview.chromium.org/2436003002/diff/60001/third_party/WebKit/Sour...
> File third_party/WebKit/Source/core/frame/csp/CSPDirectiveList.cpp (right):
> 
>
https://codereview.chromium.org/2436003002/diff/60001/third_party/WebKit/Sour...
> third_party/WebKit/Source/core/frame/csp/CSPDirectiveList.cpp:412:
> directive->allowReportSample() ? source : emptyString);
> Minor nit: perhaps we can start passing from here the already sanitized and
> trimmed sample. Skips having to check for experimental features twice.

It would be tremendously useful, if we could also provide a script-sample for
inline event-handlers. Otherwise they're often impossible to track down.

[commit-bot: I haz the power, 2017-02-22 14:17:22]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-22 14:17:23]

Dry run: This issue passed the CQ dry run.

[Mike West, 2017-02-22 14:43:12]

The CQ bit was checked by mkwst@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-22 14:43:26]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Mike West, 2017-02-22 14:52:30]

Description was changed from

==========
CSP: Add 'script-sample' to violation reports.

This patch sketches out a 'script-sample' feature similar to what Firefox
has been shipping for eons. The key distinction is that this approach requires
opt-in from the site, via a new `'report-sample'` expression in `script-src`.

Spec: TODO(mkwst)
Intent: TODO(mkwst)

BUG=606774
==========

to

==========
CSP: Add 'sample' to violation reports.

This patch sketches out a 'sample' attribute similar to what Firefox
has been shipping as 'script-sample' for eons. The key distinctions are:

1.  This approach requires opt-in from the site, via a new
    `'report-sample'` expression in the relevant directive.

2.  We're including inline style violations as well.

Let's see how the spec discussion goes.

Spec: https://github.com/w3c/webappsec-csp/issues/119

BUG=606774
==========

[Mike West, 2017-02-22 14:53:05]

The CQ bit was checked by mkwst@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-22 14:53:18]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Mike West, 2017-02-22 14:54:01]

Ok. Fleshed this out a bit, moved the tests to WPT, and updated the spec. Let's
see what folks think.

https://codereview.chromium.org/2436003002/diff/60001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/securitypolicyviolation/script-sample.html
(right):

https://codereview.chromium.org/2436003002/diff/60001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/securitypolicyviolation/script-sample.html:76:
assert_ureached('eval');
On 2017/02/22 at 12:47:03, andypaicu wrote:
> Typo here "assert_unreached"

I guess it really was unreached, eh? :)

https://codereview.chromium.org/2436003002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/frame/csp/CSPDirectiveList.cpp (right):

https://codereview.chromium.org/2436003002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/frame/csp/CSPDirectiveList.cpp:412:
directive->allowReportSample() ? source : emptyString);
On 2017/02/22 at 12:47:03, andypaicu wrote:
> Minor nit: perhaps we can start passing from here the already sanitized and
trimmed sample. Skips having to check for experimental features twice.

I'd prefer to keep the sanitization bits in `::reportViolation` (and it's
helpers), as that means we don't have to rely on all the callsites to do the
right thing. Also, doing it in `::reportViolation` means that we don't spend the
time/memory on copying the string unless we actually need to deliver it (because
we can pass it by reference here).

https://codereview.chromium.org/2436003002/diff/120001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/external/wpt/content-security-policy/securitypolicyviolation/script-sample.html
(right):

https://codereview.chromium.org/2436003002/diff/120001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/external/wpt/content-security-policy/securitypolicyviolation/script-sample.html:33:
assert_equals(e.sample, "assert_unreached('inline event handler')");
lwe@: Hey, look, the thing you asked for is already here!

[Mike West, 2017-02-22 15:06:37]

https://codereview.chromium.org/2436003002/diff/120001/third_party/WebKit/Lay...
File third_party/WebKit/LayoutTests/TestExpectations (right):

https://codereview.chromium.org/2436003002/diff/120001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/TestExpectations:1858: crbug.com/692105
external/wpt/content-security-policy/securitypolicyviolation [ Pass ]
I'll move over the other bits and pieces in a separate CL, and start working
through the rest... Or maybe ask Andy to work through the rest. *cough* :)

https://codereview.chromium.org/2436003002/diff/120001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/external/wpt/content-security-policy/securitypolicyviolation/idl-expected.txt
(right):

https://codereview.chromium.org/2436003002/diff/120001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/external/wpt/content-security-policy/securitypolicyviolation/idl-expected.txt:2:
FAIL SecurityPolicyViolationEvent interface: existence and properties of
interface object Cannot read property 'has_extended_attribute' of undefined
The failures here are issues in our IDL implementation, not this patch in
particular. :(

https://codereview.chromium.org/2436003002/diff/120001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/external/wpt/content-security-policy/securitypolicyviolation/targeting.html
(right):

https://codereview.chromium.org/2436003002/diff/120001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/external/wpt/content-security-policy/securitypolicyviolation/targeting.html:57:

Moved this over from //http/tests because it was broken in WPT. This was a
load-bearing newline! :(

[commit-bot: I haz the power, 2017-02-22 16:28:01]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-22 16:28:03]

Dry run: This issue passed the CQ dry run.

[andypaicu, 2017-02-24 09:45:49]

https://codereview.chromium.org/2436003002/diff/120001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/external/wpt/content-security-policy/securitypolicyviolation/script-sample-no-opt-in.html
(right):

https://codereview.chromium.org/2436003002/diff/120001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/external/wpt/content-security-policy/securitypolicyviolation/script-sample-no-opt-in.html:63:
assert_ureached('eval');
The typo is elusive and finds its way back in.

[Mike West, 2017-02-24 10:07:27]

The CQ bit was checked by mkwst@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-24 10:07:37]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Mike West, 2017-02-24 10:16:57]

Ok, should have actually fixed the typo this time. :)

Jochen, WDYT about landing this behind the flag?

[commit-bot: I haz the power, 2017-02-24 12:26:37]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-24 12:26:37]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[jochen (gone - plz use gerrit), 2017-02-24 16:01:58]

I didn't have a chance to review this CL. Should this have an intent to
implement maybe?

landing behind a flag lgtm if somebody else double-checked the implementation

[Mike West, 2017-03-02 08:59:52]

Description was changed from

==========
CSP: Add 'sample' to violation reports.

This patch sketches out a 'sample' attribute similar to what Firefox
has been shipping as 'script-sample' for eons. The key distinctions are:

1.  This approach requires opt-in from the site, via a new
    `'report-sample'` expression in the relevant directive.

2.  We're including inline style violations as well.

Let's see how the spec discussion goes.

Spec: https://github.com/w3c/webappsec-csp/issues/119

BUG=606774
==========

to

==========
CSP: Add 'sample' to violation reports.

This patch sketches out a 'sample' attribute similar to what Firefox
has been shipping as 'script-sample' for eons. The key distinctions are:

1.  This approach requires opt-in from the site, via a new
    `'report-sample'` expression in the relevant directive.

2.  We're including inline style violations as well.

Let's see how the spec discussion goes.

Spec: https://github.com/w3c/webappsec-csp/issues/119
Intent to Implement:
https://groups.google.com/a/chromium.org/d/msg/blink-dev/6W9r_sX3zTQ/5XCSBUQB...

BUG=606774
==========

[Mike West, 2017-03-02 08:59:55]

The CQ bit was checked by mkwst@chromium.org

[commit-bot: I haz the power, 2017-03-02 09:00:11]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-02 09:02:51]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-02 09:02:53]

Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-device-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-xcode-...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
  ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Mike West, 2017-03-02 09:08:21]

The CQ bit was unchecked by mkwst@chromium.org

[Mike West, 2017-03-02 09:13:32]

The CQ bit was checked by mkwst@chromium.org

[Mike West, 2017-03-02 09:13:33]

The patchset sent to the CQ was uploaded after l-g-t-m from jochen@chromium.org
Link to the patchset: https://codereview.chromium.org/2436003002/#ps160001
(title: "Rebase")

[commit-bot: I haz the power, 2017-03-02 09:13:44]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-02 11:09:53]

CQ is committing da patch.

Bot data: {"patchset_id": 160001, "attempt_start_ts": 1488446012745720,
"parent_rev": "b0d271768d0a592227d753f4cf60399c4dd2af4f", "commit_rev":
"f98f5c2ccea76c9d7e76646369e2f51e0c926521"}

[commit-bot: I haz the power, 2017-03-02 11:10:31]

Description was changed from

==========
CSP: Add 'sample' to violation reports.

This patch sketches out a 'sample' attribute similar to what Firefox
has been shipping as 'script-sample' for eons. The key distinctions are:

1.  This approach requires opt-in from the site, via a new
    `'report-sample'` expression in the relevant directive.

2.  We're including inline style violations as well.

Let's see how the spec discussion goes.

Spec: https://github.com/w3c/webappsec-csp/issues/119
Intent to Implement:
https://groups.google.com/a/chromium.org/d/msg/blink-dev/6W9r_sX3zTQ/5XCSBUQB...

BUG=606774
==========

to

==========
CSP: Add 'sample' to violation reports.

This patch sketches out a 'sample' attribute similar to what Firefox
has been shipping as 'script-sample' for eons. The key distinctions are:

1.  This approach requires opt-in from the site, via a new
    `'report-sample'` expression in the relevant directive.

2.  We're including inline style violations as well.

Let's see how the spec discussion goes.

Spec: https://github.com/w3c/webappsec-csp/issues/119
Intent to Implement:
https://groups.google.com/a/chromium.org/d/msg/blink-dev/6W9r_sX3zTQ/5XCSBUQB...

BUG=606774

Review-Url: https://codereview.chromium.org/2436003002
Cr-Commit-Position: refs/heads/master@{#454232}
Committed:
https://chromium.googlesource.com/chromium/src/+/f98f5c2ccea76c9d7e76646369e2...
==========

[commit-bot: I haz the power, 2017-03-02 11:10:32]

Committed patchset #9 (id:160001) as
https://chromium.googlesource.com/chromium/src/+/f98f5c2ccea76c9d7e76646369e2...