GetRestrictedCookieManager is only ever called in the browser process and attaches RenderFrameHost::GetLastCommittedOrigin() to the RestrictedCookieManager it returns a connection to. The tests for RestrictedCookieManager ensure that a malicious renderer cannot request cookies outside of those accessible by this origin.

Given this new architecture it's my understanding that the existing tests are made obsolete because a renderer cannot get a Mojo pipe to a CookieManager, only a RestrictedCookieManager.

lukasza@, please let me know if you agree with this.

Ah, I think I see it now - initially I had trouble tracing who calls GetRestrictedCookieManager (in content/browser/renderer_interface_binders.cc), but now I see that RendererInterfaceBinders::TryBindInterface is indeed always providing the origin taken from |frame->GetLastCommittedOrigin()|.

So - yes, this bug can probably resolved as WontFix.

Comment #2, for completeness, can you post which tests verify the malicious renderer enforcements?

These tests all assert that the bad message is reported:

RestrictedCookieManagerTest.GetAllForUrlFromWrongOrigin
RestrictedCookieManagerTest.SetCanonicalCookieFromWrongOrigin
RestrictedCookieManagerTest.AddChangeListenerFromWrongOrigin

We do pass an origin into RestrictedCookieManager, but there are also methods that still accept unscoped origins/URLs parameters. So we need to make sure that we can take action (i.e. kill the renderer) when the constraints are violated.

This is going to be a bit trickier if network service is out-of-process, as we'd like to sandbox it, and we likely don't want to grant it the ability to directly kill renderers.

Not working on Chromium anymore, so removing myself from the issue.  Feel free to ping me by email directly if there's something I can help with.

@dcheng, i'm assigning this to you since i believe you were looking at it. Feel free to reassign.

Let me know this is severe enough to block us from shipping to canary? This seems to be a stable blocker.

I don't think this should block canary.

I just chatted a bit more with jam about this. For the legacy sync cookie path, we'll continue going through the browser process, even with the network service. This is how the code [1] is written today and will be the case for the forseeable future. By always going through the browser process, we'll be able to make sure this kill is enforced.

Restricted cookie manager will need some further discussion (creis, correct me if I'm wrong, but I believe we'd like to keep cookie kills there--or make it impossible to end up in a situation that requires a cookie kill), so I'll assign this to pwnall to figure out the story going forward there.

[1] https://cs.chromium.org/chromium/src/content/browser/frame_host/render_frame_message_filter.cc?rcl=48d9716bcc72f5899be15244c35297c363ab41e0&l=577

since not a canary-blocker marking it as triaged.