Prevent DOS attack on UDP echo servers by distinguishing between an echo request

chrome/browser/net/network_stats.cc

Index: chrome/browser/net/network_stats.cc
===================================================================
--- chrome/browser/net/network_stats.cc	(revision 93322)
+++ chrome/browser/net/network_stats.cc	(working copy)
@@ -27,16 +27,58 @@
 
 // This specifies the number of bytes to be sent to the TCP/UDP servers as part
 // of small packet size test.
-static const int kSmallTestBytesToSend = 100;
+static const uint32 kSmallTestBytesToSend = 100;
 
 // This specifies the number of bytes to be sent to the TCP/UDP servers as part
 // of large packet size test.
-static const int kLargeTestBytesToSend = 1200;
+static const uint32 kLargeTestBytesToSend = 1200;
 
+// This specifies the maximum message (payload) size.
+static const uint32 kMaxMessage = 2048;
+
+// This specifies starting position of the version and length of the version in
+// "echo request" and "echo response".
+static const uint32 kVersionNumber = 1;
+static const uint32 kVersionStart = 0;
+static const uint32 kVersionLength = 2;
+static const uint32 kVersionEnd = 2;  // kVersionStart + kVersionLength.

[jar (doing other things), 2011/08/05 19:37:31]

nit: Suggest:

kVersionEnd = kVersionStart + kVersionLength;

[ramant (doing other things), 2011/08/12 01:05:08]

Done.

+
+// This specifies the starting position of the checksum and length of the
+// checksum in "echo request" and "echo response". Maximum value for the
+// checksum is less than (2 ** 31 - 1).
+static const uint32 kChecksumStart = 2;  // kVersionEnd.

[jar (doing other things), 2011/08/05 19:37:31]

nit: assign value rather than comment..

Same below.

[ramant (doing other things), 2011/08/12 01:05:08]

Done.

+static const uint32 kChecksumLength = 10;
+static const uint32 kChecksumEnd = 12;  // kChecksumStart + kChecksumLength.
+
+// This specifies the starting position of the <payload_size> and length of the
+// <payload_size> in "echo request" and "echo response". Maximum number of bytes
+// that can be sent in the <payload> is 9,999,999.
+static const uint32 kPayloadSizeStart = 12;  // kChecksumEnd.
+static const uint32 kPayloadSizeLength = 7;
+// kPayloadSizeEnd = kPayloadSizeStart + kPayloadSizeLength.
+static const uint32 kPayloadSizeEnd = 19;
+
+// This specifies the starting position of the |key_| and length of the |key_|
+// in "echo response". Maximum value for the |key_| is 999,999.
+static const uint32 kKeyStart = 19;  // kPayloadSizeEnd.
+static const uint32 kKeyLength = 6;
+static const uint32 kKeyEnd = 25;  // kKeyStart + kKeyLength.
+
+// This specifies the starting position of the <payload> in "echo request".
+static const uint32 kPayloadStart = 19;  // kPayloadSizeEnd.
+
+// This specifies the starting position of the <encrypted_payload> and length of
+// the <encrypted_payload> in "echo response".
+static const uint32 kEncryptedPayloadStart = 25;  // kKeyEnd.
+
+
 // NetworkStats methods and members.
 NetworkStats::NetworkStats()
-    : bytes_to_read_(0),
+    : load_size_(0),
+      bytes_to_read_(0),
       bytes_to_send_(0),
+      key_(""),
+      key_index_(0),
       ALLOW_THIS_IN_INITIALIZER_LIST(
           read_callback_(this, &NetworkStats::OnReadComplete)),
       ALLOW_THIS_IN_INITIALIZER_LIST(
@@ -49,13 +91,16 @@
   socket_.reset();
 }
 
-void NetworkStats::Initialize(int bytes_to_send,
+void NetworkStats::Initialize(uint32 bytes_to_send,
                               net::CompletionCallback* finished_callback) {
   DCHECK(bytes_to_send);   // We should have data to send.
 
   load_size_ = bytes_to_send;
-  bytes_to_send_ = bytes_to_send;
-  bytes_to_read_ = bytes_to_send;
+  bytes_to_send_ =
+      kVersionLength + kChecksumLength + kPayloadSizeLength + load_size_;
+  bytes_to_read_ =
+      kVersionLength + kChecksumLength + kPayloadSizeLength + kKeyLength +
+      load_size_;
   finished_callback_ = finished_callback;
 }
 
@@ -105,7 +150,7 @@
     return true;
   }
 
-  if (!stream_.VerifyBytes(read_buffer_->data(), result)) {
+  if (!VerifyBytes(static_cast<uint32>(result))) {
     Finish(READ_VERIFY_FAILED, net::ERR_INVALID_RESPONSE);
     return true;
   }
@@ -152,7 +197,6 @@
 
 void NetworkStats::ReadData() {
   DCHECK(!read_buffer_.get());
-  int kMaxMessage = 2048;
 
   // We release the read_buffer_ in the destructor if there is an error.
   read_buffer_ = new net::IOBuffer(kMaxMessage);
@@ -173,7 +217,7 @@
   do {
     if (!write_buffer_.get()) {
       scoped_refptr<net::IOBuffer> buffer(new net::IOBuffer(bytes_to_send_));
-      stream_.GetBytes(buffer->data(), bytes_to_send_);
+      GetEchoRequest(buffer);
       write_buffer_ = new net::DrainableIOBuffer(buffer, bytes_to_send_);
     }
 
@@ -191,6 +235,87 @@
   return net::OK;
 }
 
+void NetworkStats::GetEchoRequest(net::IOBuffer* io_buffer) {
+  // Copy the <version> into the io_buffer starting from the kVersionStart
+  // position.
+  std::string version = base::StringPrintf("%02d", kVersionNumber);
+  char* buffer = io_buffer->data() + kVersionStart;
+  DCHECK(kVersionLength == version.length());
+  memcpy(buffer, version.c_str(), kVersionLength);
+
+  // Get the <payload> from the |stream_| and copy it into io_buffer starting
+  // from the kPayloadStart position.
+  buffer = io_buffer->data() + kPayloadStart;
+  stream_.GetBytes(buffer, load_size_);
+
+  // Calculate the <checksum> of the <payload>.
+  uint32 sum = 0;
+  unsigned char* src = reinterpret_cast<unsigned char*>(buffer);
+  for (uint32 i = 0; i < load_size_; ++i)
+    sum += src[i];
+
+  // Copy the <checksum> into the io_buffer starting from the kChecksumStart
+  // position.
+  std::string checksum = base::StringPrintf("%010d", sum);
+  buffer = io_buffer->data() + kChecksumStart;
+  DCHECK(kChecksumLength == checksum.length());
+  memcpy(buffer, checksum.c_str(), kChecksumLength);
+
+  // Copy the size of the <payload> into the io_buffer starting from the
+  // kPayloadSizeStart position.
+  buffer = io_buffer->data() + kPayloadSizeStart;
+  std::string payload_size = base::StringPrintf("%07d", load_size_);
+  DCHECK(kPayloadSizeLength == payload_size.length());
+  memcpy(buffer, payload_size.c_str(), kPayloadSizeLength);
+}
+
+bool NetworkStats::VerifyBytes(uint32 buffer_length) {

[jar (doing other things), 2011/08/05 19:37:31]

Suggest copying bytes into a local buffer, and then only verifying when you have
all the bytes.

Logic will be simpler, and it will handle the problems of not getting parts of
the header etc.  In the future, we could even mess with a segmented buffer that
needs to be re-ordered.

[ramant (doing other things), 2011/08/12 01:05:08]

Done.

+  // TODO(rtenneti): Handle the data if it comes out of order.
+  uint32 payload_start = 0;
+  char* buffer;
+
+  if (key_.empty()) {
+    // If the "echo response" doesn't have enough bytes, then return false.
+    if (buffer_length < kEncryptedPayloadStart)
+      return false;
+
+    // Extract the |key_| from the "echo response".
+    buffer = read_buffer_->data() + kKeyStart;
+    char key[kKeyLength + 1];
+    memcpy(key, buffer, kKeyLength);
+    key[kKeyLength] = '\0';
+    key_ = key;

[jar (doing other things), 2011/08/05 19:37:31]

nit: avoid memcpy, and just use a special constructor that takes the number of
bytes to transfer.

Also, you should validate your data as being in the expected range for keys.

[ramant (doing other things), 2011/08/12 01:05:08]

Done.

+
+    // Initialize the starting position of the <payload>.
+    payload_start = kEncryptedPayloadStart;
+  }
+
+  // Read the <encrypted_payload> from the "echo response".
+  DCHECK_GE(buffer_length, payload_start);
+  uint32 message_length = buffer_length - payload_start;
+  message_length = std::min(message_length, kMaxMessage);
+  buffer = read_buffer_->data() + payload_start;
+  char encrypted_data[kMaxMessage + 1];
+  memcpy(encrypted_data, buffer, message_length);
+
+  // Decrypt the data.
+  char decrypted_data[kMaxMessage + 1];
+  uint32 kIdx = key_index_;
+  // Loop through the string and XOR each byte with the |key_| to get the
+  // decrypted byte. Append the decrypted byte to the |decrypted_data|.
+  for (uint32 idx = 0; idx < message_length; ++idx) {
+    int encrypted_byte = static_cast<int>(encrypted_data[idx]);
+    int key_byte = static_cast<int>(key_[kIdx]);

[jar (doing other things), 2011/08/05 19:37:31]

suggest using "char" as the types for lines 307 and 308, and then yo udon't need
to cast in 309 (and you might not need casts in 307 and/or 308).

[ramant (doing other things), 2011/08/12 01:05:08]

Done.

+    char decrypted_byte = static_cast<char>(encrypted_byte ^ key_byte);
+    decrypted_data[idx] = decrypted_byte;
+    kIdx = (kIdx + 1) % kKeyLength;
+  }
+  // Save the kIdx in case we haven't received the whole "echo response".
+  key_index_ = kIdx;
+
+  return stream_.VerifyBytes(decrypted_data, message_length);
+}
+
 // UDPStatsClient methods and members.
 UDPStatsClient::UDPStatsClient()
     : NetworkStats() {
@@ -201,7 +326,7 @@
 
 bool UDPStatsClient::Start(const std::string& ip_str,
                            int port,
-                           int bytes_to_send,
+                           uint32 bytes_to_send,
                            net::CompletionCallback* finished_callback) {
   DCHECK(port);
   DCHECK(bytes_to_send);   // We should have data to send.
@@ -272,7 +397,7 @@
 
 bool TCPStatsClient::Start(net::HostResolver* host_resolver,
                            const net::HostPortPair& server_host_port_pair,
-                           int bytes_to_send,
+                           uint32 bytes_to_send,
                            net::CompletionCallback* finished_callback) {
   DCHECK(bytes_to_send);   // We should have data to send.