1206151 - Rely on loadInfo->GetSecurityMode() rather than loadInfo->GetEnforceSecurit() within packagedAppService

Status: RESOLVED INVALID

(Core :: DOM: Security, defect)

Comment 1

[Christoph Kerschbaumer [:ckerschb}]

Attachment: bug_1206151_packagedappservice.patch

There is no channel redirect and we should rather rely on GetSecurityMode() than GetEnforceSecurity(). I missed that when reviewing Bug 1196021.

Comment 2

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

How did we get to this piece of code if no-one called AsyncOpen/AsyncOpen2 on a channel?

Comment 3

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Comment on attachment 8663036 [details] [diff] [review]
bug_1206151_packagedappservice.patch

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

Please ask Valentine for review if we actually want to land this.

Comment 4

[Christoph Kerschbaumer [:ckerschb}]

(In reply to Jonas Sicking (:sicking) from comment #3)
> Comment on attachment 8663036 [details] [diff] [review]
> bug_1206151_packagedappservice.patch
> 
> Review of attachment 8663036 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Please ask Valentine for review if we actually want to land this.

I think this actually became INVALID after chatting with you on IRC. But let me confirm with Valentin.

Valentin, GetResource is called from within nsHttpChannel::BeginConnect(). If I understand the flow of execution correctly, then this means that the channel already was openend using ::AsyncOpen2(), in which case we would have set the EnforeSecurityFlag here [2] and all good. If that is indeed the case then we can close this bug as INVALID. Otherwise we have to dig a little deeper and make sure that GetResource can not bypass any security checks.

[1] http://mxr.mozilla.org/mozilla-central/source/netwerk/protocol/http/nsHttpChannel.cpp#5248
[2] http://mxr.mozilla.org/mozilla-central/source/dom/security/nsContentSecurityManager.cpp#304

Comment 5

[Valentin Gosu [:valentin] (he/him)]

(In reply to Christoph Kerschbaumer [:ckerschb] from comment #4)
> Valentin, GetResource is called from within nsHttpChannel::BeginConnect().
> If I understand the flow of execution correctly, then this means that the
> channel already was openend using ::AsyncOpen2(), in which case we would
> have set the EnforeSecurityFlag here [2] and all good. If that is indeed the
> case then we can close this bug as INVALID. Otherwise we have to dig a
> little deeper and make sure that GetResource can not bypass any security
> checks.
> 

You are correct. Currently that is the only call to getResource, and technically the newly created channel will also call AsyncOpen2 if the calling one did so too.

Comment 6

[Christoph Kerschbaumer [:ckerschb}]

(In reply to Valentin Gosu [:valentin] from comment #5)
> You are correct. Currently that is the only call to getResource, and
> technically the newly created channel will also call AsyncOpen2 if the
> calling one did so too.

Ok, because the loadInfo is copied from the outer channel to the inner channel. So all good then. Closing as INVALID. Thanks!