Support whitelisting to handle insecure origins as trustworthy origins (blink)

Support whitelisting to handle insecure origins as trustworthy origins

The whitelist will be given and initialized via
command-line flag, which is done in chromium-side patch:
https://codereview.chromium.org/1072933006/

BUG=441605
TEST=blink_platform_unittests:SecureOriginTest.SecurityPolicyTest.TrustworthyWhiteList

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=194072

[kinuko, 2015-04-20 06:03:38]

Patchset #2 (id:20001) has been deleted

[kinuko, 2015-04-20 06:16:46]

kinuko@chromium.org changed reviewers:
+ falken@chromium.org, mkwst@chromium.org

[kinuko, 2015-04-20 08:06:40]

kinuko@chromium.org changed reviewers:
- falken@chromium.org

[kinuko, 2015-04-20 08:07:06]

Mike, can you take a look?

[Mike West, 2015-04-20 09:25:01]

https://codereview.chromium.org/1082173003/diff/60001/Source/platform/weborig...
File Source/platform/weborigin/SecurityPolicy.cpp (right):

https://codereview.chromium.org/1082173003/diff/60001/Source/platform/weborig...
Source/platform/weborigin/SecurityPolicy.cpp:129: // Must be called before we
start other threads.
One of the use-cases is devtools, right? Would we be able to reuse this
mechanism to whitelist origins via devtools, or will that need to go a separate
route?

https://codereview.chromium.org/1082173003/diff/60001/Source/platform/weborig...
Source/platform/weborigin/SecurityPolicy.cpp:140: return
trustworthyOriginSet().contains(origin.toRawString());
Hrm. Why store this as a String and not a SecurityOrigin?

https://codereview.chromium.org/1082173003/diff/60001/Source/platform/weborig...
File Source/platform/weborigin/SecurityPolicyTest.cpp (right):

https://codereview.chromium.org/1082173003/diff/60001/Source/platform/weborig...
Source/platform/weborigin/SecurityPolicyTest.cpp:148:
"filesystem:http://d.test/path/t/file.html",
I think we also need to verify that whitelisting `blob:[origin]` also whitelists
`[origin]`. That might not be clear to someone just skimming this test. It
probably means breaking out of the nice `for`-loop style, though. :(

https://codereview.chromium.org/1082173003/diff/60001/public/web/WebSecurityP...
File public/web/WebSecurityPolicy.h (right):

https://codereview.chromium.org/1082173003/diff/60001/public/web/WebSecurityP...
public/web/WebSecurityPolicy.h:104: BLINK_EXPORT static void
addOriginTrustworthyWhiteList(const WebURL& origin);
Why not pass in a WebSecurityOrigin?

[kinuko, 2015-04-20 15:10:45]

Patchset #4 (id:80001) has been deleted

[kinuko, 2015-04-20 15:43:19]

Thanks, addressed some of the comments.

https://codereview.chromium.org/1082173003/diff/60001/Source/platform/weborig...
File Source/platform/weborigin/SecurityPolicy.cpp (right):

https://codereview.chromium.org/1082173003/diff/60001/Source/platform/weborig...
Source/platform/weborigin/SecurityPolicy.cpp:129: // Must be called before we
start other threads.
Yup, the plan is not clear yet but if we support the use case in DevTools we'll
likely use this mechanism.  When we do so we can easily change this part (by
introducing lock etc), so I prefer keeping this simple until then. (The PS4 is
using lock for reference)

https://codereview.chromium.org/1082173003/diff/60001/Source/platform/weborig...
Source/platform/weborigin/SecurityPolicy.cpp:140: return
trustworthyOriginSet().contains(origin.toRawString());
On 2015/04/20 09:25:00, Mike West wrote:
> Hrm. Why store this as a String and not a SecurityOrigin?

What we want to compare in the end is a tuple of 'scheme, host, port', and using
String seems easiest for origin matching with HashSet.  Also I felt storing
RefPtr<SecurityOrigin> is a bit overkill. (I could be convinced though)

https://codereview.chromium.org/1082173003/diff/60001/Source/platform/weborig...
File Source/platform/weborigin/SecurityPolicyTest.cpp (right):

https://codereview.chromium.org/1082173003/diff/60001/Source/platform/weborig...
Source/platform/weborigin/SecurityPolicyTest.cpp:148:
"filesystem:http://d.test/path/t/file.html",
On 2015/04/20 09:25:00, Mike West wrote:
> I think we also need to verify that whitelisting `blob:[origin]` also
whitelists
> `[origin]`. That might not be clear to someone just skimming this test. It
> probably means breaking out of the nice `for`-loop style, though. :(

Good point, done.

https://codereview.chromium.org/1082173003/diff/60001/public/web/WebSecurityP...
File public/web/WebSecurityPolicy.h (right):

https://codereview.chromium.org/1082173003/diff/60001/public/web/WebSecurityP...
public/web/WebSecurityPolicy.h:104: BLINK_EXPORT static void
addOriginTrustworthyWhiteList(const WebURL& origin);
On 2015/04/20 09:25:00, Mike West wrote:
> Why not pass in a WebSecurityOrigin?

Done.

[Mike West, 2015-04-20 18:21:52]

LGTM, thanks!

[palmer, 2015-04-20 21:22:00]

LGTM

[kinuko, 2015-04-21 01:37:37]

The CQ bit was checked by kinuko@chromium.org

[commit-bot: I haz the power, 2015-04-21 01:38:14]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1082173003/40002

[commit-bot: I haz the power, 2015-04-21 02:42:51]

Committed patchset #5 (id:40002) as
https://src.chromium.org/viewvc/blink?view=rev&revision=194072