Bug 1125292 - Sending ALPN header field for WebRTC calls, r=bwc
authorMartin Thomson <martin.thomson@gmail.com>
Tue, 15 Sep 2015 10:28:34 -0700
changeset 262595 5ee49f58051365db87c1fc1a3d1d7b278da21aa2
parent 262594 ff494eb6487b2951588d73eade04339278534e8c
child 262596 408c024f3dc010f61ac698b505f268999448fe8a
push id65072
push usermartin.thomson@gmail.com
push dateTue, 15 Sep 2015 17:30:53 +0000
treeherdermozilla-inbound@5ee49f580513 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbwc
bugs1125292
milestone43.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1125292 - Sending ALPN header field for WebRTC calls, r=bwc
media/mtransport/nricectx.h
media/mtransport/test/proxy_tunnel_socket_unittest.cpp
media/mtransport/third_party/nICEr/src/net/nr_proxy_tunnel.c
media/mtransport/third_party/nICEr/src/net/nr_proxy_tunnel.h
media/webrtc/signaling/src/peerconnection/PeerConnectionMedia.cpp
--- a/media/mtransport/nricectx.h
+++ b/media/mtransport/nricectx.h
@@ -169,30 +169,31 @@ class NrIceTurnServer : public NrIceStun
       NrIceStunServer(transport), username_(username), password_(password) {}
 
   std::string username_;
   std::vector<unsigned char> password_;
 };
 
 class NrIceProxyServer {
  public:
-  NrIceProxyServer() :
-    host_(), port_(0) {
+  NrIceProxyServer(const std::string& host, uint16_t port,
+                   const std::string& alpn) :
+    host_(host), port_(port), alpn_(alpn) {
   }
 
-  NrIceProxyServer(const std::string& host, uint16_t port) :
-    host_(host), port_(port) {
-  }
+  NrIceProxyServer() : NrIceProxyServer("", 0, "") {}
 
   const std::string& host() const { return host_; }
   uint16_t port() const { return port_; }
+  const std::string& alpn() const { return alpn_; }
 
  private:
   std::string host_;
   uint16_t port_;
+  std::string alpn_;
 };
 
 
 class NrIceCtx {
  public:
   enum ConnectionState { ICE_CTX_INIT,
                          ICE_CTX_CHECKING,
                          ICE_CTX_OPEN,
--- a/media/mtransport/test/proxy_tunnel_socket_unittest.cpp
+++ b/media/mtransport/test/proxy_tunnel_socket_unittest.cpp
@@ -41,19 +41,23 @@ const uint16_t kRemotePort = 3333;
 
 const std::string kProxyHost = "example.com";
 const std::string kProxyAddr = "192.0.2.134";
 const uint16_t kProxyPort = 9999;
 
 const std::string kHelloMessage = "HELLO";
 const std::string kGarbageMessage = "xxxxxxxxxx";
 
-std::string connect_message(const std::string &host, uint16_t port, const std::string &tail = "") {
+std::string connect_message(const std::string &host, uint16_t port, const std::string &alpn, const std::string &tail) {
   std::stringstream ss;
-  ss << "CONNECT " << host << ":" << port << " HTTP/1.0\r\n\r\n" << tail;
+  ss << "CONNECT " << host << ":" << port << " HTTP/1.0\r\n";
+  if (!alpn.empty()) {
+    ss << "ALPN: " << alpn << "\r\n";
+  }
+  ss << "\r\n" << tail;
   return ss.str();
 }
 
 std::string connect_response(int code, const std::string &tail = "") {
   std::stringstream ss;
   ss << "HTTP/1.0 " << code << "\r\n\r\n" << tail;
   return ss.str();
 }
@@ -111,41 +115,60 @@ class ProxyTunnelSocketTest : public ::t
         nr_socket_(nullptr) {}
 
   ~ProxyTunnelSocketTest() {
     nr_socket_destroy(&nr_socket_);
     nr_proxy_tunnel_config_destroy(&config_);
   }
 
   void SetUp() {
-    nsRefPtr<DummySocket> dummy(new DummySocket());
-
     nr_resolver_ = resolver_impl_.get_nr_resolver();
 
     int r = nr_str_port_to_transport_addr(
         (char *)kRemoteAddr.c_str(), kRemotePort, IPPROTO_TCP, &remote_addr_);
     ASSERT_EQ(0, r);
 
     r = nr_str_port_to_transport_addr(
         (char *)kProxyAddr.c_str(), kProxyPort, IPPROTO_TCP, &proxy_addr_);
     ASSERT_EQ(0, r);
 
     nr_proxy_tunnel_config_create(&config_);
     nr_proxy_tunnel_config_set_resolver(config_, nr_resolver_);
     nr_proxy_tunnel_config_set_proxy(config_, kProxyAddr.c_str(), kProxyPort);
 
-    r = nr_socket_proxy_tunnel_create(
+    Configure();
+  }
+
+  // This reconfigures the socket with the updated information in config_.
+  void Configure() {
+    if (nr_socket_) {
+      EXPECT_EQ(0, nr_socket_destroy(&nr_socket_));
+      EXPECT_EQ(nullptr, nr_socket_);
+    }
+
+    nsRefPtr<DummySocket> dummy(new DummySocket());
+    int r = nr_socket_proxy_tunnel_create(
         config_,
         dummy->get_nr_socket(),
         &nr_socket_);
     ASSERT_EQ(0, r);
 
     socket_impl_ = dummy.forget();  // Now owned by nr_socket_.
   }
 
+  void Connect(int expectedReturn = 0) {
+    int r = nr_socket_connect(nr_socket_, &remote_addr_);
+    EXPECT_EQ(expectedReturn, r);
+
+    size_t written = 0;
+    r = nr_socket_write(nr_socket_, kHelloMessage.c_str(), kHelloMessage.size(), &written, 0);
+    EXPECT_EQ(0, r);
+    EXPECT_EQ(kHelloMessage.size(), written);
+  }
+
   nr_socket *socket() { return nr_socket_; }
 
  protected:
   nsRefPtr<DummySocket> socket_impl_;
   DummyResolver resolver_impl_;
   nr_socket *nr_socket_;
   nr_resolver *nr_resolver_;
   nr_proxy_tunnel_config *config_;
@@ -161,66 +184,77 @@ TEST_F(ProxyTunnelSocketTest, TestConnec
   int r = nr_socket_connect(nr_socket_, &remote_addr_);
   ASSERT_EQ(0, r);
 
   ASSERT_EQ(0, nr_transport_addr_cmp(socket_impl_->get_connect_addr(), &proxy_addr_,
         NR_TRANSPORT_ADDR_CMP_MODE_ALL));
 }
 
 TEST_F(ProxyTunnelSocketTest, TestConnectProxyRequest) {
-  int r = nr_socket_connect(nr_socket_, &remote_addr_);
-  ASSERT_EQ(0, r);
+  Connect();
+
+  std::string msg = connect_message(kRemoteAddr, kRemotePort, "", kHelloMessage);
+  socket_impl_->CheckWriteBuffer(reinterpret_cast<const uint8_t *>(msg.c_str()), msg.size());
+}
+
+TEST_F(ProxyTunnelSocketTest, TestAlpnConnect) {
+  const std::string alpn = "this,is,alpn";
+  int r = nr_proxy_tunnel_config_set_alpn(config_, alpn.c_str());
+  EXPECT_EQ(0, r);
 
-  size_t written = 0;
-  r = nr_socket_write(nr_socket_, kHelloMessage.c_str(), kHelloMessage.size(), &written, 0);
-  ASSERT_EQ(0, r);
+  Configure();
+  Connect();
+
+  std::string msg = connect_message(kRemoteAddr, kRemotePort, alpn, kHelloMessage);
+  socket_impl_->CheckWriteBuffer(reinterpret_cast<const uint8_t *>(msg.c_str()), msg.size());
+}
 
-  std::string msg = connect_message(kRemoteAddr, kRemotePort, kHelloMessage);
+TEST_F(ProxyTunnelSocketTest, TestNullAlpnConnect) {
+  int r = nr_proxy_tunnel_config_set_alpn(config_, nullptr);
+  EXPECT_EQ(0, r);
+
+  Configure();
+  Connect();
+
+  std::string msg = connect_message(kRemoteAddr, kRemotePort, "", kHelloMessage);
   socket_impl_->CheckWriteBuffer(reinterpret_cast<const uint8_t *>(msg.c_str()), msg.size());
 }
 
 TEST_F(ProxyTunnelSocketTest, TestConnectProxyHostRequest) {
-  int r = nr_socket_destroy(&nr_socket_);
-  ASSERT_EQ(0, r);
-
-  nsRefPtr<DummySocket> dummy(new DummySocket());
-
   nr_proxy_tunnel_config_set_proxy(config_, kProxyHost.c_str(), kProxyPort);
+  Configure();
+  // Because kProxyHost is a domain name and not an IP address,
+  // nr_socket_connect will need to resolve an IP address before continuing.  It
+  // does that, and assumes that resolving the IP will take some time, so it
+  // returns R_WOULDBLOCK.
+  //
+  // However, In this test setup, the resolution happens inline immediately, so
+  // nr_socket_connect is called recursively on the inner socket in
+  // nr_socket_proxy_tunnel_resolved_cb.  That also completes.  Thus, the socket
+  // is actually successfully connected after this call, even though
+  // nr_socket_connect reports an error.
+  //
+  // Arguably nr_socket_proxy_tunnel_connect() is busted, because it shouldn't
+  // report an error when it doesn't need any further assistance from the
+  // calling code, but that's pretty minor.
+  Connect(R_WOULDBLOCK);
 
-  r = nr_socket_proxy_tunnel_create(
-      config_,
-      dummy->get_nr_socket(),
-      &nr_socket_);
-  ASSERT_EQ(0, r);
-
-  socket_impl_ = dummy.forget();  // Now owned by nr_socket_.
-
-  r = nr_socket_connect(nr_socket_, &remote_addr_);
-  ASSERT_EQ(R_WOULDBLOCK, r);
-
-  size_t written = 0;
-  r = nr_socket_write(nr_socket_, kHelloMessage.c_str(), kHelloMessage.size(), &written, 0);
-  ASSERT_EQ(0, r);
-
-  std::string msg = connect_message(kRemoteAddr, kRemotePort, kHelloMessage);
+  std::string msg = connect_message(kRemoteAddr, kRemotePort, "", kHelloMessage);
   socket_impl_->CheckWriteBuffer(reinterpret_cast<const uint8_t *>(msg.c_str()), msg.size());
 }
 
 TEST_F(ProxyTunnelSocketTest, TestConnectProxyWrite) {
-  int r = nr_socket_connect(nr_socket_, &remote_addr_);
-  ASSERT_EQ(0, r);
-
-  size_t written = 0;
-  r = nr_socket_write(nr_socket_, kHelloMessage.c_str(), kHelloMessage.size(), &written, 0);
-  ASSERT_EQ(0, r);
+  Connect();
 
   socket_impl_->ClearWriteBuffer();
 
-  r = nr_socket_write(nr_socket_, kHelloMessage.c_str(), kHelloMessage.size(), &written, 0);
-  ASSERT_EQ(0, r);
+  size_t written = 0;
+  int r = nr_socket_write(nr_socket_, kHelloMessage.c_str(), kHelloMessage.size(), &written, 0);
+  EXPECT_EQ(0, r);
+  EXPECT_EQ(kHelloMessage.size(), written);
 
   socket_impl_->CheckWriteBuffer(reinterpret_cast<const uint8_t *>(kHelloMessage.c_str()),
       kHelloMessage.size());
 }
 
 TEST_F(ProxyTunnelSocketTest, TestConnectProxySuccessResponse) {
   int r = nr_socket_connect(nr_socket_, &remote_addr_);
   ASSERT_EQ(0, r);
--- a/media/mtransport/third_party/nICEr/src/net/nr_proxy_tunnel.c
+++ b/media/mtransport/third_party/nICEr/src/net/nr_proxy_tunnel.c
@@ -36,17 +36,21 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE
 #include <nr_api.h>
 
 #include <assert.h>
 
 #include "nr_proxy_tunnel.h"
 
 #define MAX_HTTP_CONNECT_ADDR_SIZE 256
 #define MAX_HTTP_CONNECT_BUFFER_SIZE 1024
-#define ENDLN "\r\n\r\n"
+#define MAX_ALPN_LENGTH 64
+#ifndef CRLF
+#define CRLF "\r\n"
+#endif
+#define END_HEADERS CRLF CRLF
 
 typedef struct nr_socket_proxy_tunnel_ {
   nr_proxy_tunnel_config *config;
   nr_socket *inner;
   nr_transport_addr remote_addr;
   int connect_requested;
   int connect_answered;
   int connect_failed;
@@ -89,59 +93,76 @@ static nr_socket_vtbl nr_socket_proxy_tu
 };
 
 static int send_http_connect(nr_socket_proxy_tunnel *sock)
 {
   int r, _status;
   int port;
   int printed;
   char addr[MAX_HTTP_CONNECT_ADDR_SIZE];
-  char mesg[MAX_HTTP_CONNECT_ADDR_SIZE + 64];
+  char mesg[MAX_HTTP_CONNECT_ADDR_SIZE + MAX_ALPN_LENGTH + 128];
+  size_t offset = 0;
   size_t bytes_sent;
 
   r_log(LOG_GENERIC,LOG_DEBUG,"send_http_connect");
 
   if ((r=nr_transport_addr_get_port(&sock->remote_addr, &port))) {
     ABORT(r);
   }
 
   if ((r=nr_transport_addr_get_addrstring(&sock->remote_addr, addr, sizeof(addr)))) {
     ABORT(r);
   }
 
-  printed = snprintf(mesg, sizeof(mesg), "CONNECT %s:%d HTTP/1.0%s", addr, port, ENDLN);
-  if (printed < 0 || ((size_t)printed >= sizeof(mesg))) {
+  printed = snprintf(mesg + offset, sizeof(mesg) - offset,
+                     "CONNECT %s:%d HTTP/1.0", addr, port);
+  offset += printed;
+  if (printed < 0 || (offset >= sizeof(mesg))) {
     ABORT(R_FAILED);
   }
 
-  if ((r=nr_socket_write(sock->inner, mesg, strlen(mesg), &bytes_sent, 0))) {
+  if (sock->config->alpn) {
+    printed = snprintf(mesg + offset, sizeof(mesg) - offset,
+                       CRLF "ALPN: %s", sock->config->alpn);
+    offset += printed;
+    if (printed < 0 || (offset >= sizeof(mesg))) {
+      ABORT(R_FAILED);
+    }
+  }
+  if (offset + sizeof(END_HEADERS) >= sizeof(mesg)) {
+    ABORT(R_FAILED);
+  }
+  memcpy(mesg + offset, END_HEADERS, strlen(END_HEADERS));
+  offset += strlen(END_HEADERS);
+
+  if ((r=nr_socket_write(sock->inner, mesg, offset, &bytes_sent, 0))) {
     ABORT(r);
   }
 
-  if (bytes_sent < strlen(mesg)) {
+  if (bytes_sent < offset) {
     /* TODO(bug 1116583): buffering and wait for */
     r_log(LOG_GENERIC,LOG_DEBUG,"send_http_connect should be buffering %lu", (unsigned long)bytes_sent);
     ABORT(R_IO_ERROR);
   }
 
   sock->connect_requested = 1;
 
   _status = 0;
 abort:
   return(_status);
 }
 
 static char *find_http_terminator(char *response, size_t len)
 {
   char *term = response;
   char *end = response + len;
-  int N = strlen(ENDLN);
+  int N = strlen(END_HEADERS);
 
   for (; term = memchr(term, '\r', end - term); ++term) {
-    if (end - term >= N && memcmp(term, ENDLN, N) == 0) {
+    if (end - term >= N && memcmp(term, END_HEADERS, N) == 0) {
       return term;
     }
   }
 
   return NULL;
 }
 
 static int parse_http_response(char *begin, char *end, unsigned int *status)
@@ -362,17 +383,17 @@ int nr_socket_proxy_tunnel_read(void *ob
 
     /* TODO (bug 1115934): Handle authentication challenges. */
     if (http_status < 200 || http_status >= 300) {
       r_log(LOG_GENERIC,LOG_ERR,"nr_socket_proxy_tunnel_read unable to connect %u",
             http_status);
       ABORT(R_FAILED);
     }
 
-    ptr = http_term + strlen(ENDLN);
+    ptr = http_term + strlen(END_HEADERS);
     pending = sock->buffered_bytes - (ptr - sock->buffer);
 
     if (pending == 0) {
       ABORT(R_WOULDBLOCK);
     }
 
     assert(pending <= maxlen);
     *len = pending;
@@ -426,16 +447,17 @@ int nr_proxy_tunnel_config_destroy(nr_pr
 
   if (!configpp || !*configpp)
     return 0;
 
   configp = *configpp;
   *configpp = 0;
 
   RFREE(configp->proxy_host);
+  RFREE(configp->alpn);
   RFREE(configp);
 
   return 0;
 }
 
 int nr_proxy_tunnel_config_set_proxy(nr_proxy_tunnel_config *config,
                                      const char *host, UINT2 port)
 {
@@ -466,30 +488,61 @@ int nr_proxy_tunnel_config_set_resolver(
 {
   r_log(LOG_GENERIC,LOG_DEBUG,"nr_proxy_tunnel_config_set_resolver");
 
   config->resolver = resolver;
 
   return 0;
 }
 
+int nr_proxy_tunnel_config_set_alpn(nr_proxy_tunnel_config *config,
+                                    const char *alpn)
+{
+  r_log(LOG_GENERIC,LOG_DEBUG,"nr_proxy_tunnel_config_set_alpn");
+
+  if (alpn && (strlen(alpn) > MAX_ALPN_LENGTH)) {
+    return R_BAD_ARGS;
+  }
+
+  if (config->alpn) {
+    RFREE(config->alpn);
+  }
+
+  config->alpn = NULL;
+
+  if (alpn) {
+    char *alpndup = r_strdup(alpn);
+
+    if (!alpndup) {
+      return R_NO_MEMORY;
+    }
+
+    config->alpn = alpndup;
+  }
+
+  return 0;
+}
+
 int nr_proxy_tunnel_config_copy(nr_proxy_tunnel_config *config, nr_proxy_tunnel_config **copypp)
 {
   int r,_status;
   nr_proxy_tunnel_config *copy = 0;
 
   if ((r=nr_proxy_tunnel_config_create(&copy)))
     ABORT(r);
 
   if ((r=nr_proxy_tunnel_config_set_proxy(copy, config->proxy_host, config->proxy_port)))
     ABORT(r);
 
   if ((r=nr_proxy_tunnel_config_set_resolver(copy, config->resolver)))
     ABORT(r);
 
+  if ((r=nr_proxy_tunnel_config_set_alpn(copy, config->alpn)))
+    ABORT(r);
+
   *copypp = copy;
 
   _status=0;
 abort:
   if (_status) {
     nr_proxy_tunnel_config_destroy(&copy);
   }
   return(_status);
--- a/media/mtransport/third_party/nICEr/src/net/nr_proxy_tunnel.h
+++ b/media/mtransport/third_party/nICEr/src/net/nr_proxy_tunnel.h
@@ -39,28 +39,32 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE
 #include "nr_socket.h"
 #include "nr_resolver.h"
 #include "nr_socket_wrapper.h"
 
 typedef struct nr_proxy_tunnel_config_ {
   nr_resolver *resolver;
   char *proxy_host;
   UINT2 proxy_port;
+  char *alpn;
 } nr_proxy_tunnel_config;
 
 int nr_proxy_tunnel_config_create(nr_proxy_tunnel_config **config);
 
 int nr_proxy_tunnel_config_destroy(nr_proxy_tunnel_config **config);
 
 int nr_proxy_tunnel_config_set_proxy(nr_proxy_tunnel_config *config,
                                      const char* host, UINT2 port);
 
 int nr_proxy_tunnel_config_set_resolver(nr_proxy_tunnel_config *config,
                                         nr_resolver *resolver);
 
+int nr_proxy_tunnel_config_set_alpn(nr_proxy_tunnel_config *config,
+                                    const char *alpn);
+
 int nr_socket_proxy_tunnel_create(nr_proxy_tunnel_config *config,
                                   nr_socket *inner,
                                   nr_socket **socketpp);
 
 int nr_socket_wrapper_factory_proxy_tunnel_create(nr_proxy_tunnel_config *config,
                                                   nr_socket_wrapper_factory **factory);
 
 #endif
--- a/media/webrtc/signaling/src/peerconnection/PeerConnectionMedia.cpp
+++ b/media/webrtc/signaling/src/peerconnection/PeerConnectionMedia.cpp
@@ -200,19 +200,25 @@ PeerConnectionMedia::ProtocolProxyQueryH
   rv = proxyinfo.GetPort(&httpsProxyPort);
   if (NS_FAILED(rv)) {
     CSFLogError(logTag, "%s: Failed to get proxy server port", __FUNCTION__);
     return;
   }
 
   if (pcm_->mIceCtx.get()) {
     assert(httpsProxyPort >= 0 && httpsProxyPort < (1 << 16));
+    // Note that this could check if PrivacyRequested() is set on the PC and
+    // remove "webrtc" from the ALPN list.  But that would only work if the PC
+    // was constructed with a peerIdentity constraint, not when isolated
+    // streams are added.  If we ever need to signal to the proxy that the
+    // media is isolated, then we would need to restructure this code.
     pcm_->mProxyServer.reset(
       new NrIceProxyServer(httpsProxyHost.get(),
-                           static_cast<uint16_t>(httpsProxyPort)));
+                           static_cast<uint16_t>(httpsProxyPort),
+                           "webrtc,c-webrtc"));
   } else {
     CSFLogError(logTag, "%s: Failed to set proxy server (ICE ctx unavailable)",
         __FUNCTION__);
   }
 }
 
 NS_IMPL_ISUPPORTS(PeerConnectionMedia::ProtocolProxyQueryHandler, nsIProtocolProxyCallback)
 #endif // !defined(MOZILLA_XPCOMRT_API)