Bug 201127 - Tainted origin flag logic should affect no-cors redirects
Summary: Tainted origin flag logic should affect no-cors redirects
Status: RESOLVED DUPLICATE of bug 186030
Alias: None
Product: WebKit
Classification: Unclassified
Component: Page Loading (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Rob Buis
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-08-25 12:43 PDT by Rob Buis
Modified: 2020-03-12 04:14 PDT (History)
8 users (show)

See Also:

Attachments
Patch (5.62 KB, patch)
2019-08-25 12:46 PDT, Rob Buis 		no flags Details | Formatted Diff | Diff
Patch (6.87 KB, patch)
2019-08-25 14:00 PDT, Rob Buis 		no flags Details | Formatted Diff | Diff
Patch (8.13 KB, patch)
2019-08-26 00:21 PDT, Rob Buis 		no flags Details | Formatted Diff | Diff
Patch (7.87 KB, patch)
2019-08-26 23:35 PDT, Rob Buis 		no flags Details | Formatted Diff | Diff
Patch (8.12 KB, patch)
2019-08-30 13:14 PDT, Rob Buis 		no flags Details | Formatted Diff | Diff
Patch (8.19 KB, patch)
2019-09-03 02:36 PDT, Rob Buis 		no flags Details | Formatted Diff | Diff
Patch (1.60 KB, patch)
2019-12-16 05:39 PST, Rob Buis 		no flags Details | Formatted Diff | Diff
Patch (9.54 KB, patch)
2019-12-16 08:09 PST, Rob Buis 		no flags Details | Formatted Diff | Diff
Show Obsolete (7) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Comment 1 Rob Buis 2019-08-25 12:46:33 PDT 
Created attachment 377224 [details]
Patch
Comment 2 Rob Buis 2019-08-25 14:00:08 PDT 
Created attachment 377225 [details]
Patch
Comment 3 Rob Buis 2019-08-26 00:21:43 PDT 
Created attachment 377230 [details]
Patch
Comment 4 Rob Buis 2019-08-26 23:35:00 PDT 
Created attachment 377321 [details]
Patch
Comment 5 Frédéric Wang (:fredw) 2019-08-29 01:24:06 PDT 
Is it ready for review? I'm a bit confused where the tainting flag comes in play in this patch.
Comment 6 Rob Buis 2019-08-30 13:14:21 PDT 
Created attachment 377743 [details]
Patch
Comment 7 Rob Buis 2019-08-30 13:43:02 PDT 
(In reply to Frédéric Wang (:fredw) from comment #5)
> Is it ready for review? I'm a bit confused where the tainting flag comes in
> play in this patch.

It was not, but is now :)
Comment 8 youenn fablet 2019-09-02 12:28:08 PDT 
Comment on attachment 377743 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=377743&action=review

> Source/WebKit/NetworkProcess/NetworkLoadChecker.cpp:328
> +            request.setHTTPOrigin(SecurityOrigin::createUnique()->toString());

We should probably also set m_origin here.
This will ensure that the CORP check is fine.

It might be interesting to do some more refactoring.
In particular, add a routine that implements https://fetch.spec.whatwg.org/#append-a-request-origin-header.
This will make sure we add the Origin header for POST but not for GET.
We should probably add a test for GET to check that no Origin header in no-cors is set after a cross origin redirection.

doesNotNeedCORSCheck includes navigate mode for which we do not want to add a null origin.
We might want to remove doesNotNeedCORSCheck and add some early checks for navigate mode and same origin mode.
Then we could have some common code for no-cors and cors cases.
This should get us closer to the fetch spec.

> LayoutTests/imported/w3c/web-platform-tests/fetch/security/embedded-credentials.tentative.sub-expected.txt:3
>  Blocked access to external URL http://user:pass@www.localhost:8800/images/red.png

This test should be updated to use hosts[alt].
Comment 9 Rob Buis 2019-09-03 02:36:40 PDT 
Created attachment 377877 [details]
Patch
Comment 10 Rob Buis 2019-12-16 05:39:01 PST 
Created attachment 385755 [details]
Patch
Comment 11 Rob Buis 2019-12-16 08:09:17 PST 
Created attachment 385768 [details]
Patch
Comment 12 Rob Buis 2020-03-12 04:14:31 PDT 
It is possible the code in these patches is still valuable, but in the end 186030 is similar/same so let's try to fix that one instead.

*** This bug has been marked as a duplicate of bug 186030 ***