Regression in Chrome 64: CSP directive form-action 'self' prevents <form target="_blank"> submit
Reported by
steffen....@gmail.com,
Jan 3 2018
|
|||||||||||||
Issue descriptionUserAgent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/63.0.3239.108 Safari/537.36 Steps to reproduce the problem: 1. Open https://www.computerbase.de/chrome-form-target-blank-csp-bug/ 2. Click "Submit" What is the expected behavior? Chrome should open a new tab and load the page What went wrong? Chrome opens a new tab that is completely empty. The developer tools log the following error: Refused to send form data to 'https://www.computerbase.de/suche/?q=foo' because it violates the following Content Security Policy directive: "form-action 'self'". Did this work before? N/A Does this work in other browsers? N/A Chrome version: 64.0.3282.39 Channel: beta OS Version: Flash Version: The page is served with the CSP directive "form-action 'self'". This has worked fine up to Chrome 63. It doesn't work anymore in Chrome 64 and 65 and I'm quiet sure this is a bug.
,
Jan 3 2018
,
Jan 3 2018
Able to reproduce the issue with latest Chrome beta i.e., 64.0.3282.71 on Windows, Mac and Linux. Please find the regression range below : You are probably looking for a change made after 518244 (known good), but no later than 518245 (first known bad). CHANGELOG URL: The script might not always return single CL as suspect as some perf builds might get missing due to failure. https://chromium.googlesource.com/chromium/src/+log/daaa9faed34a50b6d218eb0f4c1f4f9b7de59885..209f225b2d51334eaf69ffdf002e25eaa1e0d448
,
Jan 3 2018
,
Jan 8 2018
andypaicu@ Gentle Ping! This issue is marked as RB-Stable, could you please let us know is there any latest update available on this issue?
,
Jan 8 2018
If it's `form-action`, and started in M64, it might well be PlzNavigate. Arthur, would you mind taking a look?
,
Jan 8 2018
PlzNavigate was enabled on M61 on all platforms (except for WebView on M63). So this is not caused by enabling PlzNavigate, but something after that. I tried: 63.0.3239.108 (Official Build) (64-bit) -> Do not reproduce the bug. 65.0.3316.0 (Developer Build) (64-bit) -> Can reproduce the bug. I will try to understand the issue. Thanks "steffen.weber" for the repro steps!
,
Jan 8 2018
I understand the issue. 1) A RenderFrameImpl display a page with "form-action 'self'" CSP and self="https://www.computerbase.de/chrome-form-target-blank-csp-bug/ 2) A form submission happens with target="_blank" 3) A new RenderFrameImpl is created and self is undefined. 4) CSP checks fails. The issue is that the form-action is checked against the CSP of the navigating frame, not the one that has initiated the navigation in the first place. This is the current behavior and it has always been the case. I submitted a bug a while ago about this here: issue 700964 This doesn't explain why this bug didn't occur on M63. I will do a bisection in order to understand why it was working before.
,
Jan 8 2018
I think it was working before because the CSP was not propagated to the second RenderFrameImpl. It think it was fixed by: https://chromium.googlesource.com/chromium/src/+/209f225b2d51334eaf69ffdf002e25eaa1e0d448. This bug probably starts failing after this CL. I am considering options we have. 1) Maybe we can do browser side checks only on redirects and rely on the renderer side checks for the initial url. 2) Find a way to get the RenderFrameHost that has initiated the navigation and check CSP on it instead of the new one. That would be the right way, but I believe it is complicated. 3) We have now more confidence in BeginNavigationParams::initiator_origin to be correct. It could be used to be 'self' in this case. 4) Find a way to call RenderFrameHostImpl::SetSelf([...]) when the RenderFrameHostImpl is constructed from a form submission with target="_blank". Do you have any suggestions? 1) is the easy way. 2) is the complicated and correct way. I don't have any opinions on 3) and 4). I need to take a closer look.
,
Jan 8 2018
RE: #c10 AFAICT, https://www.w3.org/TR/CSP3/#directive-form-action says that the the form-action directive should apply to "the target of a form submissions" (not to the initiator of the navigation as mentioned in #c10). Please also note the test cases added in https://codereview.chromium.org/2464123004 (e.g. third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/1.1/form-action-blocked-when-target-blank.html).
,
Jan 9 2018
Re: #c11 If that's true, that would be very nice for fixing this bug (but very weird too). When I read https://www.w3.org/TR/CSP3/#directive-form-action, I still understand what I said in #c10. Let me formalize what I am taking about: Let A and B to be 2 frames. 1) A does a form submit and target B (with url X). 2) Then B starts a navigation and checks the form-action CSP. => It must check if the url X match A's CSP (and not B's CSP)
,
Jan 9 2018
Thanks for the clarification in #c13 - that makes perfect sense now. See also https://crbug.com/630332#c20 and https://github.com/w3c/webappsec-csp/issues/8 where I try to argue that form-action should focus on blocking form->immediate-target-url (and potentially ignore form->final-redirect-destination-of-target-url). AFAIR, the fix in https://crbug.com/630332#c28 would ignore the final-redirect form->final-redirect-destination-of-target-url in case of OOPIFs (i.e. AFAIR the fix adds checking of the URL in A's process, using the terminology from #c13; also - AFAIR I was hoping to eventually remove the checks in B's process after the w3c discussion linked above is resolved, but this never happened :-( - see issue 663512).
,
Jan 9 2018
,
Jan 9 2018
I started a CL. I have a test and a fix that does the 1) options. I still need to update: http/tests/security/contentSecurityPolicy/1.1/form-action-resubmission-* I introduced these tests to document what is the current behavior (not necessary correct BTW)
,
Jan 11 2018
Friendly reminder: M64 Stable is only 11 days away, and last M64 beta is next week.
,
Jan 12 2018
FYI: The CL is here: https://chromium-review.googlesource.com/c/chromium/src/+/857503 It needs LGTMs though. This really need to be merged into the next (and last) M64 beta.
,
Jan 15 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/078468ed4f8ad666f7436086cd8c069c9ae414ec commit 078468ed4f8ad666f7436086cd8c069c9ae414ec Author: arthursonzogni <arthursonzogni@chromium.org> Date: Mon Jan 15 12:27:51 2018 CSP: Fix blocked form submission with target="_blank" This fixes bug 798698 . It happens when the "form-action 'self'" CSP is used and a form submission with the attribute "target='_blank'" happens. Since the form-action CSP is checked against the CSP of the new windows instead of the one of the old window, it was blocked. This regression probably started after this CL (which is good): https://chromium-review.googlesource.com/c/chromium/src/+/765969. Solution was to disable browser side checks and only rely on renderer side checks for the initial load. Bug: 798698 Change-Id: Iade17b80f493af265ddb86fe95305d96c7ce0975 Reviewed-on: https://chromium-review.googlesource.com/857503 Commit-Queue: Arthur Sonzogni <arthursonzogni@chromium.org> Reviewed-by: Mike West <mkwst@chromium.org> Reviewed-by: Łukasz Anforowicz <lukasza@chromium.org> Reviewed-by: Alex Moshchuk <alexmos@chromium.org> Cr-Commit-Position: refs/heads/master@{#529255} [modify] https://crrev.com/078468ed4f8ad666f7436086cd8c069c9ae414ec/content/browser/frame_host/form_submission_throttle.cc [modify] https://crrev.com/078468ed4f8ad666f7436086cd8c069c9ae414ec/content/browser/frame_host/form_submission_throttle.h [modify] https://crrev.com/078468ed4f8ad666f7436086cd8c069c9ae414ec/content/browser/frame_host/form_submission_throttle_browsertest.cc [delete] https://crrev.com/d659b8182fc16950711020a0e1b797575500949e/content/test/data/form_submission_throttle/form_action_with_path.html [add] https://crrev.com/078468ed4f8ad666f7436086cd8c069c9ae414ec/third_party/WebKit/LayoutTests/external/wpt/content-security-policy/form-action/form-action-self-allowed-target-blank.html [add] https://crrev.com/078468ed4f8ad666f7436086cd8c069c9ae414ec/third_party/WebKit/LayoutTests/external/wpt/content-security-policy/support/postmessage-pass-to-opener.html [delete] https://crrev.com/d659b8182fc16950711020a0e1b797575500949e/third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/1.1/form-action-resubmission-iframe-reload-from-child-expected.txt [delete] https://crrev.com/d659b8182fc16950711020a0e1b797575500949e/third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/1.1/form-action-resubmission-iframe-reload-from-child.html [delete] https://crrev.com/d659b8182fc16950711020a0e1b797575500949e/third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/1.1/form-action-resubmission-iframe-reload-from-parent-expected.txt [delete] https://crrev.com/d659b8182fc16950711020a0e1b797575500949e/third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/1.1/form-action-resubmission-iframe-reload-from-parent.html [delete] https://crrev.com/d659b8182fc16950711020a0e1b797575500949e/third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/1.1/form-action-resubmission-main-page-expected.txt [delete] https://crrev.com/d659b8182fc16950711020a0e1b797575500949e/third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/1.1/form-action-resubmission-main-page.html [delete] https://crrev.com/d659b8182fc16950711020a0e1b797575500949e/third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/1.1/form-action-resubmission-window-reload-from-child-expected.txt [delete] https://crrev.com/d659b8182fc16950711020a0e1b797575500949e/third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/1.1/form-action-resubmission-window-reload-from-child.html [delete] https://crrev.com/d659b8182fc16950711020a0e1b797575500949e/third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/1.1/form-action-resubmission-window-reload-from-parent-expected.txt [delete] https://crrev.com/d659b8182fc16950711020a0e1b797575500949e/third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/1.1/form-action-resubmission-window-reload-from-parent.html [delete] https://crrev.com/d659b8182fc16950711020a0e1b797575500949e/third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/resources/form-action-none-and-reload.html [delete] https://crrev.com/d659b8182fc16950711020a0e1b797575500949e/third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/resources/form-action-none.html [delete] https://crrev.com/d659b8182fc16950711020a0e1b797575500949e/third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/resources/form-action-resubmission-main-page-callee.html [delete] https://crrev.com/d659b8182fc16950711020a0e1b797575500949e/third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/resources/form-action-resubmission-main-page-caller.html
,
Jan 15 2018
I request a merge for M64. I prepared the cherry-pick here: https://chromium-review.googlesource.com/c/chromium/src/+/866750
,
Jan 15 2018
This bug requires manual review: We are only 7 days from stable. Please contact the milestone owner if you have questions. Owners: cmasso@(Android), cmasso@(iOS), kbleicher@(ChromeOS), abdulsyed@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Jan 16 2018
Can we merge this CL? I know that the next (and last) M64 beta will be built today. This CL really needs to be included into it. I didn't get any answer yesterday, probably because of public holiday in the U.S. I will leave the office now (EMEA). Please, feel free to merge it once it it approved: https://chromium-review.googlesource.com/c/chromium/src/+/866750
,
Jan 16 2018
Thanks Arthur for the fix! Can you please comment on why this is absolutely urgent? What are the implications if we wait until M65? How safe is this merge and is this well tested, and are we absolutely sure this will not introduce any new regressions?
,
Jan 16 2018
If we don't merge this CL. Website owner that are using the "form-action" CSP will see form submission blocked when it targets a new window. Big website are using CSP, so I think it will break several of them. I think it is severe. I am confident that it will not introduce new regression. The change is only 2 line of code. If there is a regression, it will be a navigation not being blocked. The website owner don't expect this navigation to happen in the first place (CSP is a defensive feature.) This CL disable browser side checks for the initial load. Those checks still happen on the renderer side. The modified class is here almost only for checking CSP on redirects. This part is not removed.
,
Jan 16 2018
Approving merge for M64 based on #23 and #24. Branch:3282
,
Jan 16 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/6426a279940d16afb442b2c261362f31af6c2712 commit 6426a279940d16afb442b2c261362f31af6c2712 Author: arthursonzogni <arthursonzogni@chromium.org> Date: Tue Jan 16 18:58:53 2018 CSP: Fix blocked form submission with target="_blank" This fixes bug 798698 . It happens when the "form-action 'self'" CSP is used and a form submission with the attribute "target='_blank'" happens. Since the form-action CSP is checked against the CSP of the new windows instead of the one of the old window, it was blocked. This regression probably started after this CL (which is good): https://chromium-review.googlesource.com/c/chromium/src/+/765969. Solution was to disable browser side checks and only rely on renderer side checks for the initial load. Bug: 798698 Change-Id: Iade17b80f493af265ddb86fe95305d96c7ce0975 Reviewed-on: https://chromium-review.googlesource.com/857503 Commit-Queue: Arthur Sonzogni <arthursonzogni@chromium.org> Reviewed-by: Mike West <mkwst@chromium.org> Reviewed-by: Łukasz Anforowicz <lukasza@chromium.org> Reviewed-by: Alex Moshchuk <alexmos@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#529255}(cherry picked from commit 078468ed4f8ad666f7436086cd8c069c9ae414ec) Reviewed-on: https://chromium-review.googlesource.com/866750 Reviewed-by: Arthur Sonzogni <arthursonzogni@chromium.org> Cr-Commit-Position: refs/branch-heads/3282@{#508} Cr-Branched-From: 5fdc0fab22ce7efd32532ee989b223fa12f8171e-refs/heads/master@{#520840} [modify] https://crrev.com/6426a279940d16afb442b2c261362f31af6c2712/content/browser/frame_host/form_submission_throttle.cc [modify] https://crrev.com/6426a279940d16afb442b2c261362f31af6c2712/content/browser/frame_host/form_submission_throttle.h [modify] https://crrev.com/6426a279940d16afb442b2c261362f31af6c2712/content/browser/frame_host/form_submission_throttle_browsertest.cc [delete] https://crrev.com/b7040e0bd25998bf16cae683de3b575dc18d7c26/content/test/data/form_submission_throttle/form_action_with_path.html [add] https://crrev.com/6426a279940d16afb442b2c261362f31af6c2712/third_party/WebKit/LayoutTests/external/wpt/content-security-policy/form-action/form-action-self-allowed-target-blank.html [add] https://crrev.com/6426a279940d16afb442b2c261362f31af6c2712/third_party/WebKit/LayoutTests/external/wpt/content-security-policy/support/postmessage-pass-to-opener.html [delete] https://crrev.com/b7040e0bd25998bf16cae683de3b575dc18d7c26/third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/1.1/form-action-resubmission-iframe-reload-from-child-expected.txt [delete] https://crrev.com/b7040e0bd25998bf16cae683de3b575dc18d7c26/third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/1.1/form-action-resubmission-iframe-reload-from-child.html [delete] https://crrev.com/b7040e0bd25998bf16cae683de3b575dc18d7c26/third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/1.1/form-action-resubmission-iframe-reload-from-parent-expected.txt [delete] https://crrev.com/b7040e0bd25998bf16cae683de3b575dc18d7c26/third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/1.1/form-action-resubmission-iframe-reload-from-parent.html [delete] https://crrev.com/b7040e0bd25998bf16cae683de3b575dc18d7c26/third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/1.1/form-action-resubmission-main-page-expected.txt [delete] https://crrev.com/b7040e0bd25998bf16cae683de3b575dc18d7c26/third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/1.1/form-action-resubmission-main-page.html [delete] https://crrev.com/b7040e0bd25998bf16cae683de3b575dc18d7c26/third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/1.1/form-action-resubmission-window-reload-from-child-expected.txt [delete] https://crrev.com/b7040e0bd25998bf16cae683de3b575dc18d7c26/third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/1.1/form-action-resubmission-window-reload-from-child.html [delete] https://crrev.com/b7040e0bd25998bf16cae683de3b575dc18d7c26/third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/1.1/form-action-resubmission-window-reload-from-parent-expected.txt [delete] https://crrev.com/b7040e0bd25998bf16cae683de3b575dc18d7c26/third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/1.1/form-action-resubmission-window-reload-from-parent.html [delete] https://crrev.com/b7040e0bd25998bf16cae683de3b575dc18d7c26/third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/resources/form-action-none-and-reload.html [delete] https://crrev.com/b7040e0bd25998bf16cae683de3b575dc18d7c26/third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/resources/form-action-none.html [delete] https://crrev.com/b7040e0bd25998bf16cae683de3b575dc18d7c26/third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/resources/form-action-resubmission-main-page-callee.html [delete] https://crrev.com/b7040e0bd25998bf16cae683de3b575dc18d7c26/third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/resources/form-action-resubmission-main-page-caller.html
,
Jan 17 2018
,
Jan 17 2018
Thank you! :) |
|||||||||||||
►
Sign in to add a comment |
|||||||||||||
Comment 1 by steffen....@gmail.com
, Jan 3 2018