1264556 - Isolate blob URLs to first party; no blobURLs in Web Workers (Tor 15502)

Status: RESOLVED DUPLICATE of bug 1260931

(Core :: DOM: Security, defect, P2)

Description

[Jonathan Hao (inactive) [:jhao]]

Attachment: WIP patch from Tor Browser

This is one of the patches of Tor Browser that we're planning to add to gecko.
https://torpat.ch/15502

Comment 2

[Dave Huseby [:huseby]]

Attachment: the Tor Browser patch (WIP) part 2

Comment 3

[Dave Huseby [:huseby]]

This is not a testing bug.  This is just a series of patches to isolate blob URLS.

Comment 4

[Arthur Edelstein [:arthur]]

(In reply to Dave Huseby [:huseby] from comment #2)
> Created attachment 8746829 [details] [diff] [review]
> the Tor Browser patch (WIP) part 2

This patch is out of date. The only two patches now in use for isolating blobs to first party in Tor Browser are here:
https://torpat.ch/15502

Comment 5

[Tanvi Vyas[:tanvi]]

Sorry for the unnecessary bug traffic.

Comment 6

[Dave Huseby [:huseby]]

So the patch from the Tor Browser needs to be refactored to use the firstPartyDomain origin attribute instead of the ThirdPartyUtil::GetFirstPartyURI function.

In the patch: https://gitweb.torproject.org/tor-browser.git/patch/?id=e2333fd at the top of the URL::CreateObjectURL function there is a call to ThirdPartyUtil::GetFirstPartyHost().  It looks like we can remove all of that.  The call to AddDataEntry already takes the principal that came from the GlobalObject.  nsHostObjectProtocolHandler::AddDataEntry doesn't need to take the isolation key.  It can grab the value of the firstPartyDomain origin attribute from the principal and include that in the isolation keying it does.

The nsHostObjectProtocolHandler::RemoveDataEntry() should probably also take the principal and do the same thing: get the value of the firstPartyDomain from the origin attributes in the principal.

Looking at nsHostObjectProtocolHandler::AddDataEntry, I'd make the function GenerateURIString to create a URI with suffix from the origin attributes on the principal.  I wouldn't include the Tor changes to the DataInfo object.

Looking at nsHostObjectProtocolHandler::RemoveDataEntry, I'd add a principal and do the same thing as ::AddDataEntry when looking up the DataInfo object to remove.

Comment 7

[Arthur Edelstein [:arthur]]

(In reply to Dave Huseby [:huseby] from comment #6)

> In the patch:
> https://gitweb.torproject.org/tor-browser.git/patch/?id=e2333fd at the top

This patch is out of date -- Tor Browser's current version can be found at https://torpat.ch/15502

Comment 8

[Andrea Marchesini [:baku]]

> Looking at nsHostObjectProtocolHandler::RemoveDataEntry, I'd add a principal
> and do the same thing as ::AddDataEntry when looking up the DataInfo object
> to remove.

Why so? The reason why this can be useful is to avoid that a BlobURL is revoke by a different origin/principal.
But this is not defined yet in the spec.

Comment 9

[Anthony Miyaguchi [:amiyaguchi]]

Thanks Dave, you've given me some good context and information to work with. I'll let you know if I come across anything that seems out of place.

Comment 10

[Andrea Marchesini [:baku]]

I'm happy to review the nsHostObjectProtocolHandler and BlobURL part.

Comment 11

[Anthony Miyaguchi [:amiyaguchi]]

This patch will probably end up with portions of https://torpat.ch/6539, which corresponds to work on bug 962365.

Comment 12

[Ethan Tseng [:ethan]]

Hi Anthony,

Did your internship end?  Will you still work on this bug?

Comment 13

[Jonathan Hao (inactive) [:jhao]]

The blob url is isolated by first party uri after bug 1260931 is landed.  The test is also about to be landed in bug 1264573.  However, the tor browser's patch also disabled createObjectURL for web workers.  We can probably do that under a pref.

Arthur, what do you think?

Comment 14

[Georg Koppen]

I think if we can isolate createObjectURL in a web worker context, too, that would be preferable.

Comment 15

[Arthur Edelstein [:arthur]]

(In reply to Jonathan Hao [:jhao] from comment #13)
> The blob url is isolated by first party uri after bug 1260931 is landed. 
> The test is also about to be landed in bug 1264573.  However, the tor
> browser's patch also disabled createObjectURL for web workers.  We can
> probably do that under a pref.
> 
> Arthur, what do you think?

I agree with Georg. I had disabled createObjectURL in web workers as a stopgap until a way to do the isolation properly could be found.

Comment 16

[Jonathan Hao (inactive) [:jhao]]

(In reply to Georg Koppen from comment #14)
> I think if we can isolate createObjectURL in a web worker context, too, that
> would be preferable.

With bug 1260931 landed, createObjectURL by web workers is isolated and tested in bug 1264573, so I'm gonna close this bug.