Security restrictions for P2P UDP Sockets.

Security restrictions for P2P UDP Sockets.

BUG=None
TEST=Unittests

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=81039

[Sergey Ulanov, 2011-04-08 23:31:15]

ping

[jam, 2011-04-08 23:38:57]

I'm not familiar with this code, so can't do more than give a rubber stamp lgtm
that the dependencies are fine.  if the p2p socket host code was in a subdir
with its own OWNERS fine, you guys wouldn't need me to rubber stamp this :)

http://codereview.chromium.org/6800023/diff/3002/content/browser/renderer_hos...
File content/browser/renderer_host/p2p_socket_host_udp_unittest.cc (right):

http://codereview.chromium.org/6800023/diff/3002/content/browser/renderer_hos...
content/browser/renderer_host/p2p_socket_host_udp_unittest.cc:21: #include
"content/browser/renderer_host/p2p_socket_host_udp.h"
nit: should move up

[Alpha Left Google, 2011-04-09 00:44:32]

LGTM with some nits. Please also file a bug to do a security review on this
code. We also need to review if this model is secure enough.

http://codereview.chromium.org/6800023/diff/3002/content/browser/renderer_hos...
File content/browser/renderer_host/p2p_socket_host_udp.cc (right):

http://codereview.chromium.org/6800023/diff/3002/content/browser/renderer_hos...
content/browser/renderer_host/p2p_socket_host_udp.cc:20: int routing_id, int id)
nit: indentation.

http://codereview.chromium.org/6800023/diff/3002/content/browser/renderer_hos...
content/browser/renderer_host/p2p_socket_host_udp.cc:100: if
(authorized_peers_.find(recv_address_) == authorized_peers_.end()) {
Authorization based on peer seems to be enough. However I think we should file a
bug to track a security review for this model, it's good to include evanm to the
bug.

http://codereview.chromium.org/6800023/diff/3002/content/browser/renderer_hos...
File content/browser/renderer_host/p2p_socket_host_udp.h (right):

http://codereview.chromium.org/6800023/diff/3002/content/browser/renderer_hos...
content/browser/renderer_host/p2p_socket_host_udp.h:8: #include <map>
#include <set> ?

http://codereview.chromium.org/6800023/diff/3002/content/browser/renderer_hos...
content/browser/renderer_host/p2p_socket_host_udp.h:19: int routing_id, int id);
nit: indentation.

[Sergey Ulanov, 2011-04-09 00:49:10]

I've already talked to cevans about this design. We will definitely need
security review before this feature gets enabled.

http://codereview.chromium.org/6800023/diff/3002/content/browser/renderer_hos...
File content/browser/renderer_host/p2p_socket_host_udp.cc (right):

http://codereview.chromium.org/6800023/diff/3002/content/browser/renderer_hos...
content/browser/renderer_host/p2p_socket_host_udp.cc:20: int routing_id, int id)
On 2011/04/09 00:44:32, Alpha wrote:
> nit: indentation.
err, isn't it correct?

[Sergey Ulanov, 2011-04-09 00:52:36]

http://codereview.chromium.org/6800023/diff/3002/content/browser/renderer_hos...
File content/browser/renderer_host/p2p_socket_host_udp.h (right):

http://codereview.chromium.org/6800023/diff/3002/content/browser/renderer_hos...
content/browser/renderer_host/p2p_socket_host_udp.h:8: #include <map>
On 2011/04/09 00:44:32, Alpha wrote:
> #include <set> ?

Done.

http://codereview.chromium.org/6800023/diff/3002/content/browser/renderer_hos...
content/browser/renderer_host/p2p_socket_host_udp.h:19: int routing_id, int id);
On 2011/04/09 00:44:32, Alpha wrote:
> nit: indentation.
Looks correct to me

http://codereview.chromium.org/6800023/diff/3002/content/browser/renderer_hos...
File content/browser/renderer_host/p2p_socket_host_udp_unittest.cc (right):

http://codereview.chromium.org/6800023/diff/3002/content/browser/renderer_hos...
content/browser/renderer_host/p2p_socket_host_udp_unittest.cc:21: #include
"content/browser/renderer_host/p2p_socket_host_udp.h"
On 2011/04/08 23:38:57, John Abd-El-Malek wrote:
> nit: should move up

Done.