Bug 1125292 - Sending ALPN header field for WebRTC calls, r=bwc

media/mtransport/nricectx.h

@@ -174,20 +174,21 @@ class NrIceTurnServer : public NrIceStunServer {
 
 class NrIceProxyServer {
  public:
-  NrIceProxyServer() :
+  NrIceProxyServer(const std::string& host, uint16_t port,
-    host_(), port_(0) {
+                   const std::string& alpn) :
+    host_(host), port_(port), alpn_(alpn) {
   }
 
-  NrIceProxyServer(const std::string& host, uint16_t port) :
+  NrIceProxyServer() : NrIceProxyServer("", 0, "") {}
-    host_(host), port_(port) {
-  }
 
   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_;
 };
 
 

media/mtransport/test/proxy_tunnel_socket_unittest.cpp

@@ -46,9 +46,13 @@ 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();
 }
 
@@ -116,8 +120,6 @@ class ProxyTunnelSocketTest : public ::testing::Test {
   }
 
   void SetUp() {
-    nsRefPtr<DummySocket> dummy(new DummySocket());
-
     nr_resolver_ = resolver_impl_.get_nr_resolver();
 
     int r = nr_str_port_to_transport_addr(
@@ -132,7 +134,18 @@ class ProxyTunnelSocketTest : public ::testing::Test {
     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_);
@@ -141,6 +154,16 @@ class ProxyTunnelSocketTest : public ::testing::Test {
     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:
@@ -166,56 +189,67 @@ TEST_F(ProxyTunnelSocketTest, TestConnectProxyAddress) {
 }
 
 TEST_F(ProxyTunnelSocketTest, TestConnectProxyRequest) {
-  int r = nr_socket_connect(nr_socket_, &remote_addr_);
+  Connect();
-  ASSERT_EQ(0, r);
 
-  size_t written = 0;
+  std::string msg = connect_message(kRemoteAddr, kRemotePort, "", kHelloMessage);
-  r = nr_socket_write(nr_socket_, kHelloMessage.c_str(), kHelloMessage.size(), &written, 0);
+  socket_impl_->CheckWriteBuffer(reinterpret_cast<const uint8_t *>(msg.c_str()), msg.size());
-  ASSERT_EQ(0, r);
+}
 
-  std::string msg = connect_message(kRemoteAddr, kRemotePort, kHelloMessage);
+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);
+
+  Configure();
+  Connect();
+
+  std::string msg = connect_message(kRemoteAddr, kRemotePort, alpn, kHelloMessage);
+  socket_impl_->CheckWriteBuffer(reinterpret_cast<const uint8_t *>(msg.c_str()), msg.size());
+}
+
+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(
+  std::string msg = connect_message(kRemoteAddr, kRemotePort, "", kHelloMessage);
-      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);
   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_);
+  Connect();
-  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);
 
   socket_impl_->ClearWriteBuffer();
 
-  r = nr_socket_write(nr_socket_, kHelloMessage.c_str(), kHelloMessage.size(), &written, 0);
+  size_t written = 0;
-  ASSERT_EQ(0, r);
+  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());

media/mtransport/third_party/nICEr/src/net/nr_proxy_tunnel.c

@@ -41,7 +41,11 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
 #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;
@@ -94,7 +98,8 @@ static int send_http_connect(nr_socket_proxy_tunnel *sock)
   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");
@@ -107,16 +112,32 @@ static int send_http_connect(nr_socket_proxy_tunnel *sock)
     ABORT(r);
   }
 
-  printed = snprintf(mesg, sizeof(mesg), "CONNECT %s:%d HTTP/1.0%s", addr, port, ENDLN);
+  printed = snprintf(mesg + offset, sizeof(mesg) - offset,
-  if (printed < 0 || ((size_t)printed >= sizeof(mesg))) {
+                     "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);
@@ -133,10 +154,10 @@ 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;
     }
   }
@@ -367,7 +388,7 @@ int nr_socket_proxy_tunnel_read(void *obj, void * restrict buf, size_t maxlen,
       ABORT(R_FAILED);
     }
 
-    ptr = http_term + strlen(ENDLN);
+    ptr = http_term + strlen(END_HEADERS);
     pending = sock->buffered_bytes - (ptr - sock->buffer);
 
     if (pending == 0) {
@@ -431,6 +452,7 @@ int nr_proxy_tunnel_config_destroy(nr_proxy_tunnel_config **configpp)
   *configpp = 0;
 
   RFREE(configp->proxy_host);
+  RFREE(configp->alpn);
   RFREE(configp);
 
   return 0;
@@ -471,6 +493,34 @@ int nr_proxy_tunnel_config_set_resolver(nr_proxy_tunnel_config *config,
   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;
@@ -485,6 +535,9 @@ int nr_proxy_tunnel_config_copy(nr_proxy_tunnel_config *config, nr_proxy_tunnel_
   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;

media/mtransport/third_party/nICEr/src/net/nr_proxy_tunnel.h

@@ -44,6 +44,7 @@ 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);
@@ -56,6 +57,9 @@ int nr_proxy_tunnel_config_set_proxy(nr_proxy_tunnel_config *config,
 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);

media/webrtc/signaling/src/peerconnection/PeerConnectionMedia.cpp

@@ -205,9 +205,15 @@ PeerConnectionMedia::ProtocolProxyQueryHandler::SetProxyOnPcm(
 
   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__);