Closed Bug 1440462 Opened 3 years ago Closed 3 years ago

e10s: stop sending httponly cookies to child processes


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




Tracking Status
firefox64 --- fixed


(Reporter: jduell.mcbugs, Assigned: u408661)


(Whiteboard: [necko-triaged])


(1 file)

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.
Whiteboard: [necko-triaged]
Unfortunately that would regress bug 1421324.
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?
Flags: needinfo?(jduell.mcbugs)
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).
Flags: needinfo?(jduell.mcbugs)
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?
Flags: needinfo?(josh)
Nevermind, I've figured out the answer to comment 4.
Flags: needinfo?(josh)
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.
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.
Nick--did you mean to ask for review of this patch?
Flags: needinfo?(hurley)
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?
Flags: needinfo?(hurley)
jdm - see comment 7 and comment 9 (and then phabricator). Thanks!
Flags: needinfo?(josh)
Comment on attachment 9008514 [details]
Send httponly cookie names to content processes.

Josh Matthews [:jdm] has approved the revision.
Attachment #9008514 - Flags: review+
Flags: needinfo?(josh)
Pushed by
Send httponly cookie names to content processes. r=jdm
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla64
You need to log in before you can comment on or make changes to this bug.