1509738 - CSP: Snapshot nonce at load start time (and use that nonce during redirects)

Status: RESOLVED FIXED

(Core :: DOM: Security, enhancement, P2)

Description

[Christoph Kerschbaumer [:ckerschb]]

Currently our CSP implementation checks the nonce of a (style,script) at load start and then re-queries the nonce if the load gets redirected. We should snapshot the nonce at start load time, potentially store in the loadinfo and then use that nonce throughout the lifecycle of a load.

Additionally we should write a web platform test.

Comment 1

[Christoph Kerschbaumer [:ckerschb]]

Once Bug 965637 is landed we should fix that bug.

Comment 2

[Christoph Kerschbaumer [:ckerschb]]

Copying over review statement for additional info from Bug 965637:

That said, is the nonce really supposed to be examined at redirect time, not snapshotted at load start time?   That is, if we start the load, then change the nonce, is the CSP check during redirects supposed to pick up the new nonce??  https://fetch.spec.whatwg.org/#concept-request-nonce-metadata makes me think the nonce is captured at request start time, so we should not need an element to do a nonce check.   And in particular, we should be able to do this enforcement completely in the parent process.  Please file a followup bug on this.  I bet there is no test coverage for this.  :(

Comment 3

[Christoph Kerschbaumer [:ckerschb]]

Attachment: Summary: Snapshot nonce at load start time and store it in the loadinfo so CSP can use it.

Comment 4

[Christoph Kerschbaumer [:ckerschb]]

Attachment: Summary: Test nonce snapshot for CSP loades

Comment 5

[Christoph Kerschbaumer [:ckerschb]]

Attachment: Summary: Bug 1509738: CSP - snapshot nonce at load start time

Comment 6

[Christoph Kerschbaumer [:ckerschb]]

Attachment: Bug 1509738: CSP snapshot nonce at load start time

Comment 7

[Christoph Kerschbaumer [:ckerschb]]

I stacked the revisions and lando shows both patches stacked for landing - when I hit 'preview landing' however it tells me:

You have insufficient permissions to land. Level 3 Commit Access is required

I do have Level 3 Commit access, but don't have time right now to trouble shoot - can someone land that for me in the meantime?

Thanks!

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/44a945b21f87
CSP snapshot nonce at load start time r=baku

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5cbc3f79a126
Summary: Test nonce snapshot for CSP loades r=jkt

Comment 10

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

(In reply to Christoph Kerschbaumer [:ckerschb] from comment #7)

I do have Level 3 Commit access, but don't have time right now to trouble shoot - can someone land that for me in the meantime?
Any chance the API key in Lando belongs to your personal address but you are logged in with mozilla.com? There was another developer yesterday who had moved their hg permissions to the mozilla.com one and had to provide a new API key in Lando to be able to push. In Lando at the top right, open the menu and open the accounts settings on the left.

Comment 11

[Andreea Pavel [:apavel]]

Backed out for failing /propagate-nonce-external-classic.htm

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception&classifiedState=unclassified&selectedJob=228048743&revision=44a945b21f87d60d4cf839ef69ce38b33fa168e3

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=228048743&repo=autoland&lineNumber=4070

Backout: https://hg.mozilla.org/integration/autoland/rev/36ae1038bf8c00285c7e22d89e119c7910cc28d9

Comment 12

[Christoph Kerschbaumer [:ckerschb]]

(In reply to Andreea Pavel [:apavel] from comment #11)

Backed out for failing /propagate-nonce-external-classic.htm

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception&classifiedState=unclassified&selectedJob=228048743&revision=44a945b21f87d60d4cf839ef69ce38b33fa168e3

Mhm, I can't reproduce locally and it seems to work on TRY now:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=2ac265f9e8d9db1079a01ffcaa946941eeb553fa

Comment 13

[Christoph Kerschbaumer [:ckerschb]]

:aryx, :apavel, can we try to reland that? Please see green TRY push in comment 12. I don't know what the issue was, but it works locally on mac and linux and it seems try is happy as well now.

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/53f624bc7c22
Test nonce snapshot for CSP loades. r=jkt
https://hg.mozilla.org/integration/autoland/rev/fbf4b73c8786
CSP snapshot nonce at load start time. r=baku

Comment 15

[Noemi Erli[:noemi_erli]]

Backed out 2 changesets (Bug 1509738) for failures in propagate-nonce-external-classic.html

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception&classifiedState=unclassified&revision=fbf4b73c87865ecf3ee583d8b11a03ae6f330ee6

https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception&classifiedState=unclassified&revision=c98d6ecd9e7a8be09bc9622c5e30cf3f86f80f2d&selectedJob=228171970

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=228171012&repo=autoland&lineNumber=3876

Backout: https://hg.mozilla.org/integration/autoland/rev/caab6cb818201ad2b62983efb6b04705c02143e8

Comment 16

[Christoph Kerschbaumer [:ckerschb]]

Hey Noemi, any idea what might be the problem here? This changeset got already backed out once (see comment 11). I couldn't reproduce the error locally. I then re-pushed to try and got a green try run (see bottom of comment 12). Now it got backed out again - beats me...

Any idea what I could try to figure the problem?

Comment 17

[Christoph Kerschbaumer [:ckerschb]]

(In reply to Christoph Kerschbaumer [:ckerschb] from comment #16)

Any idea what I could try to figure the problem?

So, the problem was that Bug 1525006 landed in the meantime which added a new content policy type for modules which caused propagate-nonce-external-classic.html to fail. I was able to reproduce locally and fixed it locally. I rebased - pushing to try to make sure everything is fine now:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=030463fec2bf0f43898fa845c1a39d0a31902bea

Comment 18

[Christoph Kerschbaumer [:ckerschb]]

Attachment: Summary: Bug 1509738: CSP snapshot nonce at load start time

Comment 19

[Christoph Kerschbaumer [:ckerschb]]

Third time's a charm :-)

I rebased the patches and uploaded a new version, the two phabricator patches:

• https://phabricator.services.mozilla.com//D19357
• https://phabricator.services.mozilla.com//D19677

should land together - see also green try run from comment 17.

Can someone check that in for me please? Thanks!

Comment 20

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/53354f120211
Test nonce snapshot for CSP loades. r=jkt
https://hg.mozilla.org/integration/autoland/rev/4c1eb1293bbf
CSP snapshot nonce at load start time. r=baku

Comment 21

[Stefan Hindli [:stefan_hindli]]

https://hg.mozilla.org/mozilla-central/rev/53354f120211
https://hg.mozilla.org/mozilla-central/rev/4c1eb1293bbf