Add disabled certificate check support to IceServer PeerConnection API.
Refactor "OPT_SSLTCP" renaming it to "OPT_TLS_FAKE", making it clear
that it's not actually some kind of SSL over TCP. Also making it clear
that it's mutually exclusive with OPT_TLS. Maintaining deprecated
backwards compatible support for "OPT_SSLTCP".
Add "OPT_TLS_INSECURE" that implements the new certificate-check
disabled TLS mode, which is also mutually exclusive with the other
TLS options.
PortAllocator: Add a new TLS policy enum TlsCertPolicy which defines
the new insecure mode and added it as a RelayCredentials member.
TurnPort: Add new TLS policy member with appropriate getter and setter
to avoid constructor bloat. Initialize it from the RelayCredentials
after the TurnPort is created.
Expose the new feature in the PeerConnection API via
IceServer.tls_certificate_policy as well as via the Android JNI
PeerConnection API.
For security reasons we ensure that:
1) The policy is always explicitly initialized to secure.
2) API users have to explicitly integrate with the feature to
use it, and will otherwise get no change in behavior.
3) The feature is not immediately exposed in non-native
contexts. For example, disabling of certificate validation
is not implemented via URI parsing since this would
immediately allow it to be used from a web page.
This is a second attempt of https://codereview.webrtc.org/2557803002/
which was rolled back in https://codereview.webrtc.org/2590153002/
BUG=webrtc:6840
Review-Url: https://codereview.webrtc.org/2594623002
Cr-Original-Commit-Position: refs/heads/master@{#15967}
Cr-Mirrored-From: https://chromium.googlesource.com/external/webrtc
Cr-Mirrored-Commit: 0483362377fb38556009f2101816fa565885eef9
diff --git a/api/peerconnection.cc b/api/peerconnection.cc
index cdf5a97..f0efe90 100644
--- a/api/peerconnection.cc
+++ b/api/peerconnection.cc
@@ -305,8 +305,14 @@
break;
case TURN:
case TURNS: {
- turn_servers->push_back(cricket::RelayServerConfig(
- address, port, username, server.password, turn_transport_type));
+ cricket::RelayServerConfig config = cricket::RelayServerConfig(
+ address, port, username, server.password, turn_transport_type);
+ if (server.tls_cert_policy ==
+ PeerConnectionInterface::kTlsCertPolicyInsecureNoCheck) {
+ config.tls_cert_policy =
+ cricket::TlsCertPolicy::TLS_CERT_POLICY_INSECURE_NO_CHECK;
+ }
+ turn_servers->push_back(config);
break;
}
case INVALID:
diff --git a/api/peerconnection_unittest.cc b/api/peerconnection_unittest.cc
index 4f34cfa..4fe29f2 100644
--- a/api/peerconnection_unittest.cc
+++ b/api/peerconnection_unittest.cc
@@ -40,6 +40,7 @@
#include "webrtc/base/virtualsocketserver.h"
#include "webrtc/media/engine/fakewebrtcvideoengine.h"
#include "webrtc/p2p/base/p2pconstants.h"
+#include "webrtc/p2p/base/portinterface.h"
#include "webrtc/p2p/base/sessiondescription.h"
#include "webrtc/p2p/base/testturnserver.h"
#include "webrtc/p2p/client/basicportallocator.h"
@@ -2623,11 +2624,21 @@
bool ParseUrl(const std::string& url,
const std::string& username,
const std::string& password) {
+ return ParseUrl(
+ url, username, password,
+ PeerConnectionInterface::TlsCertPolicy::kTlsCertPolicySecure);
+ }
+
+ bool ParseUrl(const std::string& url,
+ const std::string& username,
+ const std::string& password,
+ PeerConnectionInterface::TlsCertPolicy tls_certificate_policy) {
PeerConnectionInterface::IceServers servers;
PeerConnectionInterface::IceServer server;
server.urls.push_back(url);
server.username = username;
server.password = password;
+ server.tls_cert_policy = tls_certificate_policy;
servers.push_back(server);
return webrtc::ParseIceServers(servers, &stun_servers_, &turn_servers_);
}
@@ -2659,6 +2670,18 @@
EXPECT_EQ(0U, stun_servers_.size());
EXPECT_EQ(1U, turn_servers_.size());
EXPECT_EQ(cricket::PROTO_TLS, turn_servers_[0].ports[0].proto);
+ EXPECT_TRUE(turn_servers_[0].tls_cert_policy ==
+ cricket::TlsCertPolicy::TLS_CERT_POLICY_SECURE);
+ turn_servers_.clear();
+
+ EXPECT_TRUE(ParseUrl(
+ "turns:hostname", "", "",
+ PeerConnectionInterface::TlsCertPolicy::kTlsCertPolicyInsecureNoCheck));
+ EXPECT_EQ(0U, stun_servers_.size());
+ EXPECT_EQ(1U, turn_servers_.size());
+ EXPECT_TRUE(turn_servers_[0].tls_cert_policy ==
+ cricket::TlsCertPolicy::TLS_CERT_POLICY_INSECURE_NO_CHECK);
+ EXPECT_EQ(cricket::PROTO_TLS, turn_servers_[0].ports[0].proto);
turn_servers_.clear();
// invalid prefixes
diff --git a/api/peerconnectioninterface.h b/api/peerconnectioninterface.h
index e81eee2..5f5a58b4 100644
--- a/api/peerconnectioninterface.h
+++ b/api/peerconnectioninterface.h
@@ -216,15 +216,28 @@
kIceConnectionMax,
};
+ // TLS certificate policy.
+ enum TlsCertPolicy {
+ // For TLS based protocols, ensure the connection is secure by not
+ // circumventing certificate validation.
+ kTlsCertPolicySecure,
+ // For TLS based protocols, disregard security completely by skipping
+ // certificate validation. This is insecure and should never be used unless
+ // security is irrelevant in that particular context.
+ kTlsCertPolicyInsecureNoCheck,
+ };
+
struct IceServer {
// TODO(jbauch): Remove uri when all code using it has switched to urls.
std::string uri;
std::vector<std::string> urls;
std::string username;
std::string password;
+ TlsCertPolicy tls_cert_policy = kTlsCertPolicySecure;
+
bool operator==(const IceServer& o) const {
return uri == o.uri && urls == o.urls && username == o.username &&
- password == o.password;
+ password == o.password && tls_cert_policy == o.tls_cert_policy;
}
bool operator!=(const IceServer& o) const { return !(*this == o); }
};
diff --git a/p2p/base/basicpacketsocketfactory.cc b/p2p/base/basicpacketsocketfactory.cc
index 51e9b07..b794904 100644
--- a/p2p/base/basicpacketsocketfactory.cc
+++ b/p2p/base/basicpacketsocketfactory.cc
@@ -87,8 +87,8 @@
return NULL;
}
- // If using SSLTCP, wrap the TCP socket in a pseudo-SSL socket.
- if (opts & PacketSocketFactory::OPT_SSLTCP) {
+ // If using fake TLS, wrap the TCP socket in a pseudo-SSL socket.
+ if (opts & PacketSocketFactory::OPT_TLS_FAKE) {
ASSERT(!(opts & PacketSocketFactory::OPT_TLS));
socket = new AsyncSSLSocket(socket);
}
@@ -129,15 +129,24 @@
proxy_info.username, proxy_info.password);
}
- // If using TLS, wrap the socket in an SSL adapter.
- if (opts & PacketSocketFactory::OPT_TLS) {
- ASSERT(!(opts & PacketSocketFactory::OPT_SSLTCP));
+ // Assert that at most one TLS option is used.
+ int tlsOpts =
+ opts & (PacketSocketFactory::OPT_TLS | PacketSocketFactory::OPT_TLS_FAKE |
+ PacketSocketFactory::OPT_TLS_INSECURE);
+ ASSERT((tlsOpts & (tlsOpts - 1)) == 0);
+ if ((tlsOpts & PacketSocketFactory::OPT_TLS) ||
+ (tlsOpts & PacketSocketFactory::OPT_TLS_INSECURE)) {
+ // Using TLS, wrap the socket in an SSL adapter.
SSLAdapter* ssl_adapter = SSLAdapter::Create(socket);
if (!ssl_adapter) {
return NULL;
}
+ if (tlsOpts & PacketSocketFactory::OPT_TLS_INSECURE) {
+ ssl_adapter->set_ignore_bad_cert(true);
+ }
+
socket = ssl_adapter;
if (ssl_adapter->StartSSL(remote_address.hostname().c_str(), false) != 0) {
@@ -145,9 +154,8 @@
return NULL;
}
- // If using SSLTCP, wrap the TCP socket in a pseudo-SSL socket.
- } else if (opts & PacketSocketFactory::OPT_SSLTCP) {
- ASSERT(!(opts & PacketSocketFactory::OPT_TLS));
+ } else if (tlsOpts & PacketSocketFactory::OPT_TLS_FAKE) {
+ // Using fake TLS, wrap the TCP socket in a pseudo-SSL socket.
socket = new AsyncSSLSocket(socket);
}
diff --git a/p2p/base/packetsocketfactory.h b/p2p/base/packetsocketfactory.h
index 290d9ca..c3b2417 100644
--- a/p2p/base/packetsocketfactory.h
+++ b/p2p/base/packetsocketfactory.h
@@ -22,9 +22,15 @@
class PacketSocketFactory {
public:
enum Options {
- OPT_SSLTCP = 0x01, // Pseudo-TLS.
- OPT_TLS = 0x02,
OPT_STUN = 0x04,
+
+ // The TLS options below are mutually exclusive.
+ OPT_TLS = 0x02, // Real and secure TLS.
+ OPT_TLS_FAKE = 0x01, // Fake TLS with a dummy SSL handshake.
+ OPT_TLS_INSECURE = 0x08, // Insecure TLS without certificate validation.
+
+ // Deprecated, use OPT_TLS_FAKE.
+ OPT_SSLTCP = OPT_TLS_FAKE,
};
PacketSocketFactory() { }
diff --git a/p2p/base/portallocator.h b/p2p/base/portallocator.h
index 93de0e1..5147429 100644
--- a/p2p/base/portallocator.h
+++ b/p2p/base/portallocator.h
@@ -95,6 +95,17 @@
CF_ALL = 0x7,
};
+// TLS certificate policy.
+enum class TlsCertPolicy {
+ // For TLS based protocols, ensure the connection is secure by not
+ // circumventing certificate validation.
+ TLS_CERT_POLICY_SECURE,
+ // For TLS based protocols, disregard security completely by skipping
+ // certificate validation. This is insecure and should never be used unless
+ // security is irrelevant in that particular context.
+ TLS_CERT_POLICY_INSECURE_NO_CHECK,
+};
+
// TODO(deadbeef): Rename to TurnCredentials (and username to ufrag).
struct RelayCredentials {
RelayCredentials() {}
@@ -147,6 +158,7 @@
PortList ports;
RelayCredentials credentials;
int priority = 0;
+ TlsCertPolicy tls_cert_policy = TlsCertPolicy::TLS_CERT_POLICY_SECURE;
};
class PortAllocatorSession : public sigslot::has_slots<> {
diff --git a/p2p/base/relayport.cc b/p2p/base/relayport.cc
index 5887aa0..5d851a1 100644
--- a/p2p/base/relayport.cc
+++ b/p2p/base/relayport.cc
@@ -491,8 +491,9 @@
rtc::SocketAddress(port_->ip(), 0),
port_->min_port(), port_->max_port());
} else if (ra->proto == PROTO_TCP || ra->proto == PROTO_SSLTCP) {
- int opts = (ra->proto == PROTO_SSLTCP) ?
- rtc::PacketSocketFactory::OPT_SSLTCP : 0;
+ int opts = (ra->proto == PROTO_SSLTCP)
+ ? rtc::PacketSocketFactory::OPT_TLS_FAKE
+ : 0;
socket = port_->socket_factory()->CreateClientTcpSocket(
rtc::SocketAddress(port_->ip(), 0), ra->address,
port_->proxy(), port_->user_agent(), opts);
diff --git a/p2p/base/tcpport.cc b/p2p/base/tcpport.cc
index 9a78870..ada88fb 100644
--- a/p2p/base/tcpport.cc
+++ b/p2p/base/tcpport.cc
@@ -491,7 +491,7 @@
ASSERT(outgoing_);
// TODO(guoweis): Handle failures here (unlikely since TCP).
int opts = (remote_candidate().protocol() == SSLTCP_PROTOCOL_NAME)
- ? rtc::PacketSocketFactory::OPT_SSLTCP
+ ? rtc::PacketSocketFactory::OPT_TLS_FAKE
: 0;
socket_.reset(port()->socket_factory()->CreateClientTcpSocket(
rtc::SocketAddress(port()->ip(), 0), remote_candidate().address(),
diff --git a/p2p/base/turnport.cc b/p2p/base/turnport.cc
index d2d9126..f97642e 100644
--- a/p2p/base/turnport.cc
+++ b/p2p/base/turnport.cc
@@ -329,8 +329,14 @@
// Apply server address TLS and insecure bits to options.
if (server_address_.proto == PROTO_TLS) {
- opts |= rtc::PacketSocketFactory::OPT_TLS;
+ if (tls_cert_policy_ ==
+ TlsCertPolicy::TLS_CERT_POLICY_INSECURE_NO_CHECK) {
+ opts |= rtc::PacketSocketFactory::OPT_TLS_INSECURE;
+ } else {
+ opts |= rtc::PacketSocketFactory::OPT_TLS;
+ }
}
+
socket_ = socket_factory()->CreateClientTcpSocket(
rtc::SocketAddress(ip(), 0), server_address_.address,
proxy(), user_agent(), opts);
diff --git a/p2p/base/turnport.h b/p2p/base/turnport.h
index 3bb09bc..b421453 100644
--- a/p2p/base/turnport.h
+++ b/p2p/base/turnport.h
@@ -87,6 +87,12 @@
virtual ProtocolType GetProtocol() const { return server_address_.proto; }
+ virtual TlsCertPolicy GetTlsCertPolicy() const { return tls_cert_policy_; }
+
+ virtual void SetTlsCertPolicy(TlsCertPolicy tls_cert_policy) {
+ tls_cert_policy_ = tls_cert_policy;
+ }
+
virtual void PrepareAddress();
virtual Connection* CreateConnection(
const Candidate& c, PortInterface::CandidateOrigin origin);
@@ -255,6 +261,7 @@
bool FailAndPruneConnection(const rtc::SocketAddress& address);
ProtocolAddress server_address_;
+ TlsCertPolicy tls_cert_policy_ = TlsCertPolicy::TLS_CERT_POLICY_SECURE;
RelayCredentials credentials_;
AttemptedServerSet attempted_server_addresses_;
diff --git a/p2p/client/basicportallocator.cc b/p2p/client/basicportallocator.cc
index 664701e..b7092d4 100644
--- a/p2p/client/basicportallocator.cc
+++ b/p2p/client/basicportallocator.cc
@@ -1394,6 +1394,7 @@
session_->allocator()->origin());
}
ASSERT(port != NULL);
+ port->SetTlsCertPolicy(config.tls_cert_policy);
session_->AddAllocatedPort(port, this, true);
}
}
diff --git a/sdk/android/api/org/webrtc/PeerConnection.java b/sdk/android/api/org/webrtc/PeerConnection.java
index b4ad579..ef26737 100644
--- a/sdk/android/api/org/webrtc/PeerConnection.java
+++ b/sdk/android/api/org/webrtc/PeerConnection.java
@@ -39,6 +39,12 @@
CLOSED
}
+ /** Tracks PeerConnectionInterface::TlsCertPolicy */
+ public enum TlsCertPolicy {
+ TLS_CERT_POLICY_SECURE,
+ TLS_CERT_POLICY_INSECURE_NO_CHECK,
+ }
+
/** Tracks PeerConnectionInterface::SignalingState */
public enum SignalingState {
STABLE,
@@ -93,6 +99,7 @@
public final String uri;
public final String username;
public final String password;
+ public final TlsCertPolicy tlsCertPolicy;
/** Convenience constructor for STUN servers. */
public IceServer(String uri) {
@@ -100,13 +107,18 @@
}
public IceServer(String uri, String username, String password) {
+ this(uri, username, password, TlsCertPolicy.TLS_CERT_POLICY_SECURE);
+ }
+
+ public IceServer(String uri, String username, String password, TlsCertPolicy tlsCertPolicy) {
this.uri = uri;
this.username = username;
this.password = password;
+ this.tlsCertPolicy = tlsCertPolicy;
}
public String toString() {
- return uri + "[" + username + ":" + password + "]";
+ return uri + " [" + username + ":" + password + "] [" + tlsCertPolicy + "]";
}
}
diff --git a/sdk/android/src/jni/classreferenceholder.cc b/sdk/android/src/jni/classreferenceholder.cc
index 565f47f..b163be1 100644
--- a/sdk/android/src/jni/classreferenceholder.cc
+++ b/sdk/android/src/jni/classreferenceholder.cc
@@ -82,6 +82,7 @@
LoadClass(jni, "org/webrtc/PeerConnection$IceGatheringState");
LoadClass(jni, "org/webrtc/PeerConnection$IceTransportsType");
LoadClass(jni, "org/webrtc/PeerConnection$TcpCandidatePolicy");
+ LoadClass(jni, "org/webrtc/PeerConnection$TlsCertPolicy");
LoadClass(jni, "org/webrtc/PeerConnection$CandidateNetworkPolicy");
LoadClass(jni, "org/webrtc/PeerConnection$KeyType");
LoadClass(jni, "org/webrtc/PeerConnection$SignalingState");
diff --git a/sdk/android/src/jni/peerconnection_jni.cc b/sdk/android/src/jni/peerconnection_jni.cc
index e729cc4..017619f 100644
--- a/sdk/android/src/jni/peerconnection_jni.cc
+++ b/sdk/android/src/jni/peerconnection_jni.cc
@@ -1634,6 +1634,23 @@
return PeerConnectionInterface::GATHER_ONCE;
}
+static PeerConnectionInterface::TlsCertPolicy JavaTlsCertPolicyTypeToNativeType(
+ JNIEnv* jni,
+ jobject j_ice_server_tls_cert_policy) {
+ std::string enum_name =
+ GetJavaEnumName(jni, "org/webrtc/PeerConnection$TlsCertPolicy",
+ j_ice_server_tls_cert_policy);
+
+ if (enum_name == "TLS_CERT_POLICY_SECURE")
+ return PeerConnectionInterface::kTlsCertPolicySecure;
+
+ if (enum_name == "TLS_CERT_POLICY_INSECURE_NO_CHECK")
+ return PeerConnectionInterface::kTlsCertPolicyInsecureNoCheck;
+
+ RTC_CHECK(false) << "Unexpected TlsCertPolicy enum_name " << enum_name;
+ return PeerConnectionInterface::kTlsCertPolicySecure;
+}
+
static void JavaIceServersToJsepIceServers(
JNIEnv* jni, jobject j_ice_servers,
PeerConnectionInterface::IceServers* ice_servers) {
@@ -1645,16 +1662,24 @@
GetFieldID(jni, j_ice_server_class, "username", "Ljava/lang/String;");
jfieldID j_ice_server_password_id =
GetFieldID(jni, j_ice_server_class, "password", "Ljava/lang/String;");
+ jfieldID j_ice_server_tls_cert_policy_id =
+ GetFieldID(jni, j_ice_server_class, "tlsCertPolicy",
+ "Lorg/webrtc/PeerConnection$TlsCertPolicy;");
+ jobject j_ice_server_tls_cert_policy =
+ GetObjectField(jni, j_ice_server, j_ice_server_tls_cert_policy_id);
jstring uri = reinterpret_cast<jstring>(
GetObjectField(jni, j_ice_server, j_ice_server_uri_id));
jstring username = reinterpret_cast<jstring>(
GetObjectField(jni, j_ice_server, j_ice_server_username_id));
jstring password = reinterpret_cast<jstring>(
GetObjectField(jni, j_ice_server, j_ice_server_password_id));
+ PeerConnectionInterface::TlsCertPolicy tls_cert_policy =
+ JavaTlsCertPolicyTypeToNativeType(jni, j_ice_server_tls_cert_policy);
PeerConnectionInterface::IceServer server;
server.uri = JavaToStdString(jni, uri);
server.username = JavaToStdString(jni, username);
server.password = JavaToStdString(jni, password);
+ server.tls_cert_policy = tls_cert_policy;
ice_servers->push_back(server);
}
}
