Security restrictions for P2P UDP Sockets.

content/browser/renderer_host/p2p_socket_host_udp.cc

Index: content/browser/renderer_host/p2p_socket_host_udp.cc
diff --git a/content/browser/renderer_host/p2p_socket_host_udp.cc b/content/browser/renderer_host/p2p_socket_host_udp.cc
index 9d9a0636bd6988a05fdc11e5a8b558b2c06c240d..5c2e0da698d15b10ed873ea9800164557a0d9c85 100644
--- a/content/browser/renderer_host/p2p_socket_host_udp.cc
+++ b/content/browser/renderer_host/p2p_socket_host_udp.cc
@@ -4,7 +4,6 @@
 
 #include "content/browser/renderer_host/p2p_socket_host_udp.h"
 
-#include "content/browser/renderer_host/p2p_sockets_host.h"
 #include "content/common/p2p_messages.h"
 #include "net/base/io_buffer.h"
 #include "net/base/net_errors.h"
@@ -12,13 +11,16 @@
 
 namespace {
 
+// UDP packets cannot be bigger than 64k.
 const int kReadBufferSize = 65536;
 
 }  // namespace
 
-P2PSocketHostUdp::P2PSocketHostUdp(P2PSocketsHost* host, int routing_id, int id)
-    : P2PSocketHost(host, routing_id, id),
+P2PSocketHostUdp::P2PSocketHostUdp(IPC::Message::Sender* message_sender,
+                                   int routing_id, int id)

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

nit: indentation.

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

err, isn't it correct?

+    : P2PSocketHost(message_sender, routing_id, id),
       state_(STATE_UNINITIALIZED),
+      socket_(new net::UDPServerSocket(NULL, net::NetLog::Source())),
       send_pending_(false),
       ALLOW_THIS_IN_INITIALIZER_LIST(
           recv_callback_(this, &P2PSocketHostUdp::OnRecv)),
@@ -34,9 +36,7 @@ P2PSocketHostUdp::~P2PSocketHostUdp() {
 }
 
 bool P2PSocketHostUdp::Init(const net::IPEndPoint& local_address) {
-  net::UDPServerSocket* socket = new net::UDPServerSocket(
-      NULL, net::NetLog::Source());
-  socket_.reset(socket);
+  DCHECK_EQ(state_, STATE_UNINITIALIZED);
 
   int result = socket_->Listen(local_address);
   if (result < 0) {
@@ -61,7 +61,7 @@ bool P2PSocketHostUdp::Init(const net::IPEndPoint& local_address) {
   recv_buffer_ = new net::IOBuffer(kReadBufferSize);
   DoRead();
 
-  host_->Send(new P2PMsg_OnSocketCreated(routing_id_, id_, address));
+  message_sender_->Send(new P2PMsg_OnSocketCreated(routing_id_, id_, address));
 
   return true;
 }
@@ -70,7 +70,7 @@ void P2PSocketHostUdp::OnError() {
   socket_.reset();
 
   if (state_ == STATE_UNINITIALIZED || state_ == STATE_OPEN)
-    host_->Send(new P2PMsg_OnError(routing_id_, id_));
+    message_sender_->Send(new P2PMsg_OnError(routing_id_, id_));
 
   state_ = STATE_ERROR;
 }
@@ -96,15 +96,28 @@ void P2PSocketHostUdp::DidCompleteRead(int result) {
 
   if (result > 0) {
     std::vector<char> data(recv_buffer_->data(), recv_buffer_->data() + result);
-    host_->Send(new P2PMsg_OnDataReceived(routing_id_, id_,
-                                          recv_address_, data));
+
+    if (authorized_peers_.find(recv_address_) == authorized_peers_.end()) {

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

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.

+      int type = GetStunPacketType(&*data.begin(), data.size());
+      if (type == STUN_BINDING_REQUEST || type == STUN_BINDING_RESPONSE) {
+        authorized_peers_.insert(recv_address_);
+      } else if (type != STUN_BINDING_ERROR) {
+        LOG(ERROR) << "Received unexpected packet which is not a STUN binding "
+            "request or response from " << recv_address_.ToString()
+                   << " before STUN binding is finished.";
+        return;
+      }
+    }
+
+    message_sender_->Send(new P2PMsg_OnDataReceived(routing_id_, id_,
+                                                    recv_address_, data));
   } else if (result < 0 && result != net::ERR_IO_PENDING) {
     LOG(ERROR) << "Error when reading from UDP socket: " << result;
     OnError();
   }
 }
 
-void P2PSocketHostUdp::Send(const net::IPEndPoint& socket_address,
+void P2PSocketHostUdp::Send(const net::IPEndPoint& to,
                             const std::vector<char>& data) {
   if (send_pending_) {
     // Silently drop packet if previous send hasn't finished.
@@ -112,10 +125,21 @@ void P2PSocketHostUdp::Send(const net::IPEndPoint& socket_address,
     return;
   }
 
+  if (authorized_peers_.find(to) == authorized_peers_.end()) {
+    int type = GetStunPacketType(&*data.begin(), data.size());
+    if (type != STUN_BINDING_REQUEST && type != STUN_BINDING_RESPONSE &&
+        type != STUN_BINDING_ERROR) {
+      LOG(ERROR) << "Page tried to send an UDP packet which is not a "
+          "STUN message to " << to.ToString()
+                 << " before STUN binding is finished.";
+      OnError();
+      return;
+    }
+  }
+
   scoped_refptr<net::IOBuffer> buffer = new net::IOBuffer(data.size());
   memcpy(buffer->data(), &data.begin()[0], data.size());
-  int result = socket_->SendTo(buffer, data.size(), socket_address,
-                               &send_callback_);
+  int result = socket_->SendTo(buffer, data.size(), to, &send_callback_);
   if (result == net::ERR_IO_PENDING) {
     send_pending_ = true;
   } else if (result < 0) {