1440462 - e10s: stop sending httponly cookies to child processes

Status: RESOLVED FIXED

(Core :: Networking: Cookies, enhancement, P2)

Description

[Jason Duell]

Looks like we removed *access* to httponly cookies in the child in bug 1339129, but we're still storing them on the child.  We should simply stop sending them to the child.

Comment 1

[Josh Matthews [:jdm]]

Unfortunately that would regress bug 1421324.

Comment 2

[u408661]

Hrm... I don't have access to that bug, which indicates to me it's a sec bug. I'm guessing we don't want to regress one of those. Jason?

Comment 3

[Jason Duell]

Nick--I cc'd you.  Looks like we'll need to handle the case where an httponly cookie overwrites a non-httponly cookie, i.e. instead of simply not sending httponly cookies to the child, we should send them, just so we can check to delete any matching non-httponly cookies, then drop them on the floor (at least that's my read of it).

Comment 4

[u408661]

I'm not sure it'll be that simple - what happens (heck, what is *supposed* to happen) when, after we do the steps Jason outlined, some script on the child sets a (non-httponly) cookie of the same name as the httponly cookie? I have to assume that is supposed to fail... but is it supposed to, say, throw an exception, or just silently not set the cookie?

jdm - do you know the cookie spec well enough to answer those hypotheticals?

Comment 5

[u408661]

Nevermind, I've figured out the answer to comment 4.

Comment 6

[Jason Duell]

Nick--you're right, comment 3 isn't enough.  We can't just drop the httponly cookie info on the child, we have to keep it around so we can stop JS from overwriting existing httponly cookies.   But we don't need to store the /value/ of the cookie, just the name and attributes (path, etc).  That should be enough for our purposes here.

Comment 7

[u408661]

Attachment: Send httponly cookie names to content processes.

Previously, if script tried to set a cookie that matched a cookie we had
received via Set-Cookie that was labeled httponly, script would think
that cookie was properly set (even though it wasn't). This ensures that
script knows just enough about httponly cookies to prevent this
inconsistent view while avoiding leakages of the potentially-sensitive
cookie values.

Comment 8

[Jason Duell]

Nick--did you mean to ask for review of this patch?

Comment 9

[u408661]

Yes. It's marked as review requested in phabricator. I've heard they're going to no longer sync those flags, so maybe that's why it doesn't show up here?

Comment 10

[u408661]

jdm - see comment 7 and comment 9 (and then phabricator). Thanks!

Comment 11

[Josh Matthews [:jdm]]

Comment on attachment 9008514 [details]
Send httponly cookie names to content processes.

Josh Matthews [:jdm] has approved the revision.

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4b87a1cf5afe
Send httponly cookie names to content processes. r=jdm

Comment 13

[Stefan Hindli [:stefan_hindli]]

https://hg.mozilla.org/mozilla-central/rev/4b87a1cf5afe