1194519 - Call AsyncOpen2 within child and parent if securityFlags are present

Status: RESOLVED FIXED

(Core :: DOM: Security, defect)

Comment 1

[Christoph Kerschbaumer [:ckerschb]]

We should also branch within HttpChannelChild redirects and potentially call asyncOpen2() if securityFlags are present.

Comment 2

[Christoph Kerschbaumer [:ckerschb]]

Attachment: bug_1194519_asyncopen2_in_httpchannelchild.patch

Comment 3

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

Comment on attachment 8647793 [details] [diff] [review]
bug_1194519_asyncopen2_in_httpchannelchild.patch

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

I don't actually know this code well enough to be sure that this is correct :(

Can you get someone else to review?

Comment 4

[Christoph Kerschbaumer [:ckerschb]]

Comment on attachment 8647793 [details] [diff] [review]
bug_1194519_asyncopen2_in_httpchannelchild.patch

Bill, whenever we open a redirected channel (in regular mode), we check if the 'enforcesecurity' flag is set in the loadInfo, which basically indicates, if the original channel was openend using asyncOpen2, then also open the redirected channel using asyncOpen2. I am pretty sure we should do the same thing in e10s. Does that sound reasonable?

[1] http://mxr.mozilla.org/mozilla-central/source/netwerk/protocol/http/nsHttpChannel.cpp#2056

Comment 5

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Comment on attachment 8647793 [details] [diff] [review]
bug_1194519_asyncopen2_in_httpchannelchild.patch

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

This seems fine to me, but Jason should probably review it.

Comment 6

[Christoph Kerschbaumer [:ckerschb]]

Comment on attachment 8647793 [details] [diff] [review]
bug_1194519_asyncopen2_in_httpchannelchild.patch

Jason, any chance you could look at that?

Comment 7

[Jason Duell]

Comment on attachment 8647793 [details] [diff] [review]
bug_1194519_asyncopen2_in_httpchannelchild.patch

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

Makes sense to me.

Comment 8

[Christoph Kerschbaumer [:ckerschb]]

Attachment: bug_1194519_asyncopen2_child_parent.patch

Jason, I just realized that there are more callsites of AsyncOpen() within *Child.cpp as well as *Parent.cpp which we have to inspect and update. Sorry, I haven't included those the first time I flagged you for review.

Anyway, whenever securityFlags are set to true, we should call AsyncOpen2 instead of AsyncOpen(), otherwise we security checks are bypassed.

Comment 9

[Jason Duell]

Comment on attachment 8650696 [details] [diff] [review]
bug_1194519_asyncopen2_child_parent.patch

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

Sorry, I feel like a dope for missing those too :)

Comment 10

[Christoph Kerschbaumer [:ckerschb]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=3861f0f36afd

Comment 11

[Christoph Kerschbaumer [:ckerschb]]

url:        https://hg.mozilla.org/integration/mozilla-inbound/rev/8a5579a007a9e78fe78bd83b4dce4a2a3dc53bf3
changeset:  8a5579a007a9e78fe78bd83b4dce4a2a3dc53bf3
user:       Christoph Kerschbaumer <[e-mail]>
date:       Fri Aug 21 20:33:56 2015 -0700
description:
Bug 1194519 - call AsyncOpen2 within child and parent if securityFlags are present (r=jduell)

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/8a5579a007a9