feat(auth): send a nonce-based CSP on the consent and error pages - #242
Open
mattzcarey wants to merge 2 commits into
Open
mattzcarey wants to merge 2 commits into
mattzcarey wants to merge 2 commits into
Conversation
Inline scripts and styles on both pages now run only with a per-response nonce, so markup that slips past escaping cannot execute. The consent form's form-action allows Cloudflare's authorization origin and the client's redirect origin, because Chrome checks form-action on the redirect after each button. The error page's close link moves from an inline onclick to a nonced listener.
…the consent page renderPage() generates the nonce, puts it on the page's only <style> and <script>, and sets the Content-Security-Policy that names it, so a page template can't add inline code the policy doesn't allow, or forget the nonce. It also escapes the title and sets X-Frame-Options for both pages. The consent page no longer sends form-action. Chrome applies it to the redirect after a submission, so it had to list Cloudflare's origin for Continue and the client's redirect origin for Cancel, and CSP can't write an IPv6 loopback address at all. The client that picks that origin also supplies the text an injection would come from, so listing it let an injected form post there anyway, and the form's only field is a handle bound to this browser. The error page has no form and keeps form-action 'none'.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The consent and error pages show text anyone can set: a DCR
client_name, a Client ID Metadata Document's name and redirect URI, a scope name repeated in an error. It is all escaped. This adds the backstop for a value that ever isn't: only the response's own inline script and styles run.Both pages render through
renderPage(). It generates the nonce, puts it on the page's only<style>and<script>, and sets the policy on the same response, so no template can carry inline code the policy doesn't allow or forget the nonce. It also escapes the title and setsX-Frame-Options: DENY.The error page's close link used an inline
onclickand ajavascript:URL. It is now a<button>with a nonced listener, and the page sendsform-action 'none'.The consent page sends no
form-action, on purpose. Chrome applies it to the redirect after a submission too:So it would have to list the client's redirect origin, next to Cloudflare's for Continue. The client picks that origin and also supplies the text an injection would come from, so an injected form could post there anyway. CSP can't write an IPv6 loopback address (
http://[::1]:…) at all. And the form's only field is a handle bound to this browser.