Add restriction to ViewHostMsg_GetRawCookies to block access from untrusted r...

Add restriction to ViewHostMsg_GetRawCookies to block access from untrusted renderers.
BUG=35575
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=39100

[yurys, 2010-02-16 15:38:43]

http://codereview.chromium.org/600137/diff/1/2
File chrome/browser/renderer_host/resource_message_filter.cc (right):

http://codereview.chromium.org/600137/diff/1/2#newcode635
chrome/browser/renderer_host/resource_message_filter.cc:635: if
(!ChildProcessSecurityPolicy::GetInstance()->CanReadRawCookies(id()))
is it ok that you don't send any response to the caller?

[pfeldman, 2010-02-16 15:44:59]

http://codereview.chromium.org/600137/diff/1/2
File chrome/browser/renderer_host/resource_message_filter.cc (right):

http://codereview.chromium.org/600137/diff/1/2#newcode635
chrome/browser/renderer_host/resource_message_filter.cc:635: if
(!ChildProcessSecurityPolicy::GetInstance()->CanReadRawCookies(id()))
On 2010/02/16 15:38:44, Yury Semikhatsky wrote:
> is it ok that you don't send any response to the caller?

This code is killing me. Done.

[yurys, 2010-02-16 15:48:26]

LGTM

http://codereview.chromium.org/600137/diff/3/1003
File chrome/browser/renderer_host/resource_message_filter.cc (right):

http://codereview.chromium.org/600137/diff/3/1003#newcode640
chrome/browser/renderer_host/resource_message_filter.cc:640: if
(!ChildProcessSecurityPolicy::GetInstance()->CanReadRawCookies(id())) {
Would it be more clear to write it something like this:
if (!ChildProcessSecurityPolicy::GetInstance()->CanReadRawCookies(id())) {
  policy = net::ERR_ACCESS_DENIED);
} else if (context->cookie_policy()) {...

[yurys, 2010-02-16 15:52:47]

http://codereview.chromium.org/600137/diff/3/1003
File chrome/browser/renderer_host/resource_message_filter.cc (right):

http://codereview.chromium.org/600137/diff/3/1003#newcode652
chrome/browser/renderer_host/resource_message_filter.cc:652: if (policy ==
net::ERR_IO_PENDING)
btw, if the control flow gets to this branch who will delete the callback or
it's a memory leak?

[pfeldman, 2010-02-16 15:59:26]

Sounds like in case or ERR_IO_PENDING, this callback is being re-scheduled.
Looks like it is there just to confuse you.

[pfeldman, 2010-02-16 16:03:37]

On 2010/02/16 15:59:26, pfeldman wrote:
> Sounds like in case or ERR_IO_PENDING, this callback is being re-scheduled.
> Looks like it is there just to confuse you.

And yet I fixed it wrong - I should not have called Run since it would still
return cookies. Fix to follow :)

[yurys, 2010-02-16 16:16:08]

http://codereview.chromium.org/600137/diff/6002/6003
File chrome/browser/renderer_host/resource_message_filter.cc (right):

http://codereview.chromium.org/600137/diff/6002/6003#newcode636
chrome/browser/renderer_host/resource_message_filter.cc:636: if
(!ChildProcessSecurityPolicy::GetInstance()->CanReadRawCookies(id())) {
lgtm, but I'd recommend you ask someone who better understands this stuff to
look at this change?