Closed Bug 1440462 Opened 3 years ago Closed 3 years ago
e10s: stop sending httponly cookies to child processes
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.
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?
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).
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?
Nevermind, I've figured out the answer to comment 4.
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?
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 on attachment 9008514 [details] Send httponly cookie names to content processes. Josh Matthews [:jdm] has approved the revision.
Attachment #9008514 - Flags: review+
Pushed by email@example.com: https://hg.mozilla.org/integration/autoland/rev/4b87a1cf5afe Send httponly cookie names to content processes. r=jdm
You need to log in before you can comment on or make changes to this bug.