Properly handle STUN requests and responses from relay servers.

content/browser/renderer_host/p2p/socket_host_tcp.cc

Index: content/browser/renderer_host/p2p/socket_host_tcp.cc
diff --git a/content/browser/renderer_host/p2p/socket_host_tcp.cc b/content/browser/renderer_host/p2p/socket_host_tcp.cc
index 250de53b5de10dcd3875fab77279754e1d9706d2..56d74a2fac4c052276813712a131664b59c3cc15 100644
--- a/content/browser/renderer_host/p2p/socket_host_tcp.cc
+++ b/content/browser/renderer_host/p2p/socket_host_tcp.cc
@@ -138,8 +138,7 @@ void P2PSocketHostTcp::OnPacket(std::vector<char>& data) {
   if (!authorized_) {
     P2PSocketHost::StunMessageType type;
     bool stun = GetStunPacketType(&*data.begin(), data.size(), &type);
-    if (stun && (type == STUN_BINDING_REQUEST ||
-                 type == STUN_BINDING_RESPONSE)) {
+    if (stun && IsRequestOrResponse(type)) {

[Wez, 2011/08/09 18:09:23]

How does receipt of a binding/allocate request/response imply authorization?

[Sergey Ulanov, 2011/08/09 18:15:41]

By sending/receiving the stun request or response the remote host authorizes
connection with the web app.

[Wez, 2011/08/09 18:30:57]

The only effect that |authorized_| seems to have is to prevent out-going data
traffic; should it also block incoming data traffic if false?  I'm still not
clear on how this is authorization - it seems to reflect whether the channel is
bound, with no security guarantee regarding whether the peer is authorized?

[Sergey Ulanov, 2011/08/09 19:18:26]

We do block incoming data packets before stun request or response is received -
see lines 143-149.

|authorized_| means that we've received STUN request or response from the remote
end, which means it is safe to relay data from/to web app. 'Authorization' may
not be the best term for this, your suggestions on what would be a better name
here are welcome.

Yes, authorized_ is usually set when the connection is bound, but we don't
inspect internals of STUN packets, e.g. don't verify that transaction id in
request and response match, so it will not be correct to call it |bound_|.

[Wez, 2011/08/09 19:54:50]

Sorry; forgot about the |!authorized_| surrounding this block.
The flag indicates whether the channel is bound to a peer, ready for data
transfer, so "ready", "bound" or "connected" would seem appropriate.
Should we be doing that verification here?

[Sergey Ulanov, 2011/08/09 21:06:56]

Ok, connected sounds good. I will rename it in a separate CL.
I don't think so. It will make this code more complicated, increasing
probability of security vulnerabilities, but will not provide much additional
protection.

       authorized_ = true;
     } else if (!stun || type == STUN_DATA_INDICATION) {
       LOG(ERROR) << "Received unexpected data packet from "