1186312 - Cache API should not accept system principals for child processes

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Ben Kelly [:bkelly, not reviewing]]

Currently Cache API allows child processes to be opened for the system principal.  There are no known uses cases for this currently.  We should prevent this for now to avoid the case where the system principal is spoofed.

Note, IDB does allow child processes to use the system principal, but that is necessary for a couple specific databases on b2g.  We should not replicate this if its not necessary.

Also, this will still allow things like about:serviceworkers to function.  Utilities like this are ultimately opening CacheStorage on behalf of a real origin, not the system principal.

Comment 1

[Ben Kelly [:bkelly, not reviewing]]

Dimi, do you have any interest in implementing this one since you've been looking at PrincipalVerifier recently?  I think its basically adding an "if system principal and aContentParent actor exists" check.  If you hit that condition then we know there is a remote child saying it has the system principal and we should return failure.

Comment 2

[Dimi Lee [:dimi]]

(In reply to Ben Kelly [:bkelly] from comment #1)
> Dimi, do you have any interest in implementing this one since you've been
> looking at PrincipalVerifier recently?  I think its basically adding an "if
> system principal and aContentParent actor exists" check.  If you hit that
> condition then we know there is a remote child saying it has the system
> principal and we should return failure.

No problem, i will take it

Comment 3

[Dimi Lee [:dimi]]

Attachment: patch v1

Hi Ben,
Could you suggest a good way to verify this ? thanks!

Comment 4

[Ben Kelly [:bkelly, not reviewing]]

Comment on attachment 8637794 [details] [diff] [review]
patch v1

Review of attachment 8637794 [details] [diff] [review]:
-----------------------------------------------------------------

Exactly what I was thinking.  Thank you!

Unfortunately I'm not sure we can positively test that it blocks spoofing in our automation tests.  We should at least verify this doesn't break any existing tests or functionality.

In addition to a try build, can you try the update/unregister buttons on about:serviceworkers in e10s desktop?  I don't think those should trigger this, but lets verify.

Comment 5

[Dimi Lee [:dimi]]

Try result : https://treeherder.mozilla.org/#/jobs?repo=try&revision=2196ddd752fd

I also test update/unregister buttons on about:serviceworkers in e10s desktop as mentioned in Comment 4, it works fine.

Comment 6

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/97d2f6256237

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/97d2f6256237