This CL adds methods to manipulate RTP header extension, particularly

content/browser/renderer_host/p2p/socket_host.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "content/browser/renderer_host/p2p/socket_host.h"
 
 #include "base/sys_byteorder.h"
 #include "content/browser/renderer_host/p2p/socket_host_tcp.h"
 #include "content/browser/renderer_host/p2p/socket_host_tcp_server.h"
 #include "content/browser/renderer_host/p2p/socket_host_udp.h"
+#include "third_party/libjingle/source/talk/base/byteorder.h"
+#include "third_party/libjingle/source/talk/p2p/base/stun.h"
+#include "third_party/libjingle/source/talk/base/messagedigest.h"
+#include "third_party/libjingle/source/talk/base/scoped_ptr.h"
 
 namespace {
 const uint32 kStunMagicCookie = 0x2112A442;
+const int kMinRtpHdrLen = 12;
+const int kDtlsRecordHeaderLen = 13;
+const int kTurnChannelHdrLen = 4;
+const int kRtpExtnHdrLen = 4;
+const int kAbsSendTimeExtnLen = 3;
+const int kOneByteHdrLen = 1;
+
+bool IsTurnChannelData(const char* data) {
+  uint16 type = talk_base::GetBE16(data);
+  return ((type & 0xC000) == 0x4000);  // MSB are 0b01
+}
+
+bool IsDtlsPacket(const char* data, int len) {

[juberti2, 2014/02/14 01:53:27]

need to think about whether this can be demuxed from turn trivially

[Mallinath (Gone from Chromium), 2014/02/19 18:52:24]

I don't think we need any of these methods now.

On 2014/02/14 01:53:27, juberti2 wrote:

+  const uint8* u = reinterpret_cast<const uint8*>(data);
+  return (len >= kDtlsRecordHeaderLen && (u[0] > 19 && u[0] < 64));
+}
+
+bool IsRtcpPacket(const char* data) {
+  int type = (static_cast<uint8>(data[1]) & 0x7F);
+  return (type >= 64 && type < 96);
+}
+
+bool IsStunPacket(const char* data) {
+  uint16 type = talk_base::GetBE16(data);
+  return !(type & 0xC000);
+}
+
+const size_t kBlockSize = 64;  // valid for SHA-256 and down
+
+class HmacCalculator {

[juberti2, 2014/02/14 01:53:27]

Is there no Chrome class to do this? Or can you use talk_base::ComputeHmac?

[Mallinath (Gone from Chromium), 2014/02/14 02:19:07]

ComputeHmac takes only one input. For RTP we have to first auth with message
then index, so I think we need this.

+ public:
+  HmacCalculator();
+  ~HmacCalculator() {}
+
+  bool Init(const void* key, size_t key_len);
+  void Update(const void* input, size_t in_len);
+  void Compute(void* output, size_t out_len);
+
+ private:
+  size_t block_len;
+  scoped_ptr<uint8[]> o_pad_;
+  scoped_ptr<uint8[]> i_pad_;
+  scoped_ptr<uint8[]> inner_;
+  scoped_ptr<talk_base::MessageDigest> digest_;
+};
+
+HmacCalculator::HmacCalculator()
+    : digest_(talk_base::MessageDigestFactory::Create(
+          talk_base::DIGEST_SHA_1)) {
+}
+
+bool HmacCalculator::Init(const void* key, size_t key_len) {
+  if (!digest_) {
+    return false;
+  }
+  block_len = kBlockSize;
+  if (digest_->Size() > 32) {
+    return false;
+  }
+
+  // Copy the key to a block-sized buffer to simplify padding.
+  // If the key is longer than a block, hash it and use the result instead.
+  talk_base::scoped_ptr<uint8[]> new_key(new uint8[block_len]);
+  if (key_len > block_len) {
+    talk_base::ComputeDigest(digest_.get(), key, key_len,
+                             new_key.get(), block_len);
+    memset(new_key.get() + digest_->Size(), 0, block_len - digest_->Size());
+  } else {
+    memcpy(new_key.get(), key, key_len);
+    memset(new_key.get() + key_len, 0, block_len - key_len);
+  }
+
+  // Set up the padding from the key, salting appropriately for each padding.
+  o_pad_.reset(new uint8[block_len]);
+  i_pad_.reset(new uint8[block_len]);
+  for (size_t i = 0; i < block_len; ++i) {
+    o_pad_[i] = 0x5c ^ new_key[i];
+    i_pad_[i] = 0x36 ^ new_key[i];
+  }
+  // Inner hash; hash the inner padding, and then the input buffer.
+  inner_.reset(new uint8[digest_->Size()]);
+  digest_->Update(i_pad_.get(), block_len);
+  return true;
+}
+
+void HmacCalculator::Update(const void* input, size_t in_len) {
+  digest_->Update(input, in_len);
+}
+
+void HmacCalculator::Compute(void* output, size_t out_len) {
+  // Outer hash; hash the outer padding, and then the result of the inner hash.
+  digest_->Update(o_pad_.get(), block_len);
+  digest_->Update(inner_.get(), digest_->Size());
+  digest_->Finish(output, out_len);
+}
+
 }  // namespace
 
 namespace content {
 
 P2PSocketHost::P2PSocketHost(IPC::Sender* message_sender,
                              int id)
     : message_sender_(message_sender),
       id_(id),
-      state_(STATE_UNINITIALIZED) {
+      state_(STATE_UNINITIALIZED),
+      rtp_sendtime_extn_id_(3) {

[juberti2, 2014/02/14 01:53:27]

this will come down in each packet, right?

[Mallinath (Gone from Chromium), 2014/02/14 02:19:07]

yea absolutely, this was just for testing purpose.

On 2014/02/14 01:53:27, juberti2 wrote:

 }
 
 P2PSocketHost::~P2PSocketHost() { }
 
 // Verifies that the packet |data| has a valid STUN header.
 // static
 bool P2PSocketHost::GetStunPacketType(
     const char* data, int data_size, StunMessageType* type) {
 
   if (data_size < kStunHeaderSize)
@@ skipping 62 matching lines @@
     case P2P_SOCKET_STUN_TCP_CLIENT:
     case P2P_SOCKET_STUN_SSLTCP_CLIENT:
     case P2P_SOCKET_STUN_TLS_CLIENT:
       return new P2PSocketHostStunTcp(message_sender, id, type, url_context);
   }
 
   NOTREACHED();
   return NULL;
 }
 
+void P2PSocketHost::MaybeUpdateRtpSendTimeExtn(const char* data, int len) {

[juberti2, 2014/02/14 09:23:56]

probably don't want the data pointer to be const if you are going to update it.

[Mallinath (Gone from Chromium), 2014/02/18 22:40:57]

Done.

+  int raw_packet_len = len;
+  const char* start = data;

[Mallinath (Gone from Chromium), 2014/02/14 01:29:01]

We can always return without doing anything, if PacketOption doesn't has any
auth info.

+
+  // Packet can't be less than RTP header size.
+  if (raw_packet_len < kMinRtpHdrLen) {
+    // Not a valid RTP packet.
+    return;
+  }
+
+  // If the packet is a DTLS packet, no need to proceed further.
+  if (IsDtlsPacket(start, len)) {
+    return;
+  }
+
+  // If packet is a stun packet, most likely it's not a media packet.
+  // Do we need to care about STUN DATA Indication?

[juberti2, 2014/02/14 01:53:27]

yes, unfortunately. until we finish channel bind, we use data indication. edge
case but we would probably be a super hard problem to debug, so we should plan
to handle it.

[Mallinath (Gone from Chromium), 2014/02/14 02:19:07]

sure.
On 2014/02/14 01:53:27, juberti2 wrote:

+  if (IsStunPacket(start)) {
+    return;
+  }
+
+  // AbsSendTime extension is only in a RTP packet.
+  if (IsRtcpPacket(start)) {

[juberti2, 2014/02/14 01:53:27]

i think you can do this right before ParseRtpPacket, so you only need to do it
once

[Mallinath (Gone from Chromium), 2014/02/18 22:40:57]

Done.

+    return;
+  }
+
+  // Now we need to start inspecting the packet.
+  if (IsTurnChannelData(start)) {
+    start += kTurnChannelHdrLen;
+    raw_packet_len -= kTurnChannelHdrLen;
+  }
+
+  // Skip if the wrapped packet is RTCP packet.
+  if (IsRtcpPacket(start)) {
+    // Update the HMAC and return
+    return;
+  }
+
+  if (ParseRtpPacket(start, raw_packet_len)) {
+    // Update HMAC and return.
+  }
+}
+
+bool P2PSocketHost::ParseRtpPacket(const char* data, int in_len) {

[juberti2, 2014/02/14 01:53:27]

Parse is probably not the right term here, since you are finding and replacing
at the same time

[Mallinath (Gone from Chromium), 2014/02/18 22:40:57]

Done.

+   //  0                   1                   2                   3
+   //  0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
+   // +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+   // |V=2|P|X|  CC   |M|     PT      |       sequence number         |
+   // +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+   // |                           timestamp                           |
+   // +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+   // |           synchronization source (SSRC) identifier            |
+   // +=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+
+   // |            contributing source (CSRC) identifiers             |
+   // |                             ....                              |
+   // +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+
+  int cc_count = data[0] & 0x0F;
+  int rtp_hdr_len = kMinRtpHdrLen + 4 * cc_count;

[juberti2, 2014/02/14 01:53:27]

this is confusing since it doesnt include the full header len. maybe
rtp_hdr_len_no_ext, or something similar

[Mallinath (Gone from Chromium), 2014/02/18 22:40:57]

Done.

+  if (rtp_hdr_len > in_len) {
+    DCHECK(false);
+    return false;
+  }
+
+  bool X = (data[0] & 0x10);
+  if (!X)   // Return if extension bit is not set.
+    return true;
+
+  data += rtp_hdr_len; // Includes RTP extension header.
+
+  // Getting extension profile ID and length.
+  uint16 profile_id = talk_base::GetBE16(data);
+  // Length is in 32 bit words.
+  uint16 extn_length = talk_base::GetBE16(data + 2) * 4;

[juberti2, 2014/02/14 01:53:27]

double check that the length is still good now that we know the total header
size

[Mallinath (Gone from Chromium), 2014/02/18 22:40:57]

Done.

+  data += kRtpExtnHdrLen;  // Moving past extn header.
+
+  // WebRTC is using one byte header extension.
+  // TODO(mallinath) - Handle two byte header extension.
+  if (profile_id && 0xBEDE) {  // OneByte extension header

[juberti2, 2014/02/14 01:53:27]

==, not &&

[Mallinath (Gone from Chromium), 2014/02/18 22:40:57]

Done.

+    //  0
+    //  0 1 2 3 4 5 6 7
+    // +-+-+-+-+-+-+-+-+
+    // |  ID   |  len  |
+    // +-+-+-+-+-+-+-+-+
+
+    //  0                   1                   2                   3
+    //  0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
+    // +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+    // |       0xBE    |    0xDE       |           length=3            |
+    // +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+    // |  ID   | L=0   |     data      |  ID   |  L=1  |   data...
+    // +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+    //       ...data   |    0 (pad)    |    0 (pad)    |  ID   | L=3   |
+    // +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+    // |                          data                                 |
+    // +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+    uint16 consumed = 0;
+    while(extn_length - consumed > 0) {
+      const int id = (*data & 0xF0) >> 4;
+      const int len = (*data & 0x0F) + 1;
+      // The 4-bit length is the number minus one of data bytes of this header
+      // extension element following the one-byte header.
+      if (id == rtp_sendtime_extn_id_) {

[Solis, 2014/02/14 10:05:29]

IIUC rtp_sendtime_extn_id_ is the only P2PSocketHost field that is being used in
these functions. Can you make it an argument to MaybeUpdate...() instead and
pull the functions into a separate utility class/namespace, or even tuck them
away in the anonymous ns at the top of this file?

[Mallinath (Gone from Chromium), 2014/02/18 22:40:57]

Yea, this I added for testing purpose. Extension id is part of PacketOptions.
On 2014/02/14 10:05:29, Solis wrote:

+        UpdateRtpSendTimeExtn(data + kOneByteHdrLen, len);
+      }
+      consumed += kOneByteHdrLen + len;
+      // Count padding bytes
+      consumed += SkipPaddingBytes(data + consumed, extn_length - consumed);

[juberti2, 2014/02/14 01:53:27]

this seems overly complicated - feels like you could just do while (*data == 0)
++data, ++consumed

[Mallinath (Gone from Chromium), 2014/02/18 22:40:57]

Done.

+      data += consumed;
+    }
+  }
+  return true;
+}
+
+int P2PSocketHost::SkipPaddingBytes(const char* data, int len) {
+  int zero_bytes = 0;
+  int consumed = 0;
+  while (len - consumed > 0) {
+    if (*data != 0) {
+      return zero_bytes;
+    }
+    ++consumed;
+    ++data;
+    ++zero_bytes;
+  }
+  return zero_bytes;
+}
+
+void P2PSocketHost::UpdateRtpSendTimeExtn(const char* data, int len) {

[Stefan, 2014/02/14 09:02:44]

Would prefer UpdateRtpAbsSendTimeExtension

[Mallinath (Gone from Chromium), 2014/02/18 22:40:57]

Done.

+  // Absolute send time in RTP streams.
+  //
+  // The absolute send time is signaled to the receiver in-band using the
+  // general mechanism for RTP header extensions [RFC5285]. The payload
+  // of this extension (the transmitted value) is a 24-bit unsigned integer
+  // containing the sender's current time in seconds as a fixed point number
+  // with 18 bits fractional part.
+  //
+  // The form of the absolute send time extension block:
+  //
+  //    0                   1                   2                   3
+  //    0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
+  //   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+  //   |  ID   | len=2 |              absolute send time               |
+  //   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+  DCHECK_EQ(len, kAbsSendTimeExtnLen);
+  // Convert micro second to 24-bit unsigned with 18 bit
+  // fractional part
+  uint32 now_second = base::Time::Now().ToInternalValue() /

[Stefan, 2014/02/14 09:02:44]

Chromium doesn't recommend using this method. I'm really not familiar with it,
so I can't tell why. Can't you use InMicroseconds() instead?  Something like:
uint32 now_second = ((base::Time::Now().InMicroseconds() << 18) / 1000000) &
0x00FFFFFF;

https://code.google.com/p/chromium/codesearch#chromium/src/base/time/time.h&l=86

[Mallinath (Gone from Chromium), 2014/02/18 22:40:57]

I think you got confused with base::TimeDelta. Time::ToInternalValue() is good
to use. One in TimeDelta is not recommended.

[Stefan, 2014/02/19 08:17:32]

Looks like it says the same n Time::ToInternalValue():
https://code.google.com/p/chromium/codesearch#chromium/src/base/time/time.h&l...

But to be honest I haven't worked with this class much, so I'm not sure about
the convention. I think I have seen it be used as
(base:Time::Now() - base:Time::UnixEpoch).InMilliseconds() before. Maybe
something like that is preferable?

If you know it is often used like this then it's ok with me, but I couldn't find
any such usage when searching cs.chromium.org.

On 2014/02/18 22:40:57, mallinath2 wrote:

[Mallinath (Gone from Chromium), 2014/02/19 18:52:24]

Ok, I was thinking the discouragement is only for doing arithmetic on the value,
heck we are doing it. But there is TimeTicks class which doesn't have any
restrictions and searched the code base, there are quite few usages of that
method.
 
On 2014/02/19 08:17:32, Stefan wrote:

+      base::Time::kMicrosecondsPerSecond;
+  now_second = (now_second << 18) & 0x00FFFFFF;
+  // This is done to avoid passing a non-const pointer in all places. This way
+  // we can avoid manipulation everywhere than where it's required.

[Solis, 2014/02/14 10:05:29]

IMO that's broken. If MaybeUpdateRtpSendTimeExtn() can actually change the
buffer, it shouldn't take a const pointer.

Alternatively, break the MaybeUpdate...() into two functions:

char* FindRtpHeaderExtension(const char* data, int len, int extension_id, int
extension_len)
(returns null if not present in packet for whatever reason, knows nothing about
the internals of the header extension)

void UpdateAbsoluteSenderTimeExtension(char* data);

[Mallinath (Gone from Chromium), 2014/02/18 22:40:57]

Done.

+  char* data_ptr = const_cast<char*> (data);
+  // TODO(mallinath) - Add SetBE24 to byteorder.h in libjingle.
+  data_ptr[0] = static_cast<uint8>(now_second >> 16);
+  data_ptr[1] = static_cast<uint8>(now_second >> 8);
+  data_ptr[2] = static_cast<uint8>(now_second);
+}
+
+// Assumes |len| is actual packet length + tag length.
+#if 0
+void P2PSocketHost::UpdateHmac(const uint8* packet, int len,
+                               const talk_base::PacketOptions& options) {
+  uint8* auth_start = packet;
+  uint8* auth_tag = packet + (len - options.tag_len);
+  HmacCalculator hmac;
+  if (!hmac.Init(options.key, options.key_len)) {
+    DCHECK(false);
+    return;
+  }
+  hmac.Update(packet, len - options.key_len);
+  hmac.Update(options.srtp_packet_index, 4);
+  hmac.Compute(auth_tag, options.tag_len);

[juberti2, 2014/02/14 01:53:27]

don't forget that the tag is shorter than the total HMAC len, so you may need to
have a temp buffer and memcpy to the (shorter) auth tag

[Mallinath (Gone from Chromium), 2014/02/19 18:52:24]

Done.

+}
+#endif
+
 }  // namespace content