Revert of Add disabled certificate check support to IceServer PeerConnection API. (patchset #8 id:140001 of https://codereview.webrtc.org/2557803002/ )

Reason for revert:
This CL broke all Chromium WebRTC FYI bots. A roll+fix was attempted here: https://codereview.chromium.org/2590783003/, but failed to land. I'm reverting this CL now to make the tree green again. Make the API change gradual when you reland so that we can update Chromium between.

Original issue's description:
> 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.
>
> 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.
>
> BUG=webrtc:6840
>
> Review-Url: https://codereview.webrtc.org/2557803002
> Cr-Commit-Position: refs/heads/master@{#15670}
> Committed: b0f04fdb9e

TBR=pthatcher@webrtc.org,deadbeef@webrtc.org,hnsl@webrtc.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=webrtc:6840

Review-Url: https://codereview.webrtc.org/2590153002
Cr-Commit-Position: refs/heads/master@{#15703}
This commit is contained in:
magjed
2016-12-20 02:22:06 -08:00
committed by Commit bot
parent 03ab4e7818
commit d5236e2948
14 changed files with 18 additions and 136 deletions

View File

@ -306,14 +306,8 @@ bool ParseIceServerUrl(const PeerConnectionInterface::IceServer& server,
break;
case TURN:
case TURNS: {
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);
turn_servers->push_back(cricket::RelayServerConfig(
address, port, username, server.password, turn_transport_type));
break;
}
case INVALID:

View File

@ -40,7 +40,6 @@
#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"
@ -2620,21 +2619,11 @@ class IceServerParsingTest : public testing::Test {
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_);
}
@ -2666,18 +2655,6 @@ TEST_F(IceServerParsingTest, ParseStunPrefixes) {
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

View File

@ -209,28 +209,15 @@ class PeerConnectionInterface : public rtc::RefCountInterface {
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 && tls_cert_policy == o.tls_cert_policy;
password == o.password;
}
bool operator!=(const IceServer& o) const { return !(*this == o); }
};

View File

@ -87,8 +87,8 @@ AsyncPacketSocket* BasicPacketSocketFactory::CreateServerTcpSocket(
return NULL;
}
// If using fake TLS, wrap the TCP socket in a pseudo-SSL socket.
if (opts & PacketSocketFactory::OPT_TLS_FAKE) {
// If using SSLTCP, wrap the TCP socket in a pseudo-SSL socket.
if (opts & PacketSocketFactory::OPT_SSLTCP) {
ASSERT(!(opts & PacketSocketFactory::OPT_TLS));
socket = new AsyncSSLSocket(socket);
}
@ -129,24 +129,15 @@ AsyncPacketSocket* BasicPacketSocketFactory::CreateClientTcpSocket(
proxy_info.username, proxy_info.password);
}
// 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 using TLS, wrap the socket in an SSL adapter.
if (opts & PacketSocketFactory::OPT_TLS) {
ASSERT(!(opts & PacketSocketFactory::OPT_SSLTCP));
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) {
@ -154,8 +145,9 @@ AsyncPacketSocket* BasicPacketSocketFactory::CreateClientTcpSocket(
return NULL;
}
} else if (tlsOpts & PacketSocketFactory::OPT_TLS_FAKE) {
// Using fake TLS, wrap the TCP socket in a pseudo-SSL socket.
// If using SSLTCP, wrap the TCP socket in a pseudo-SSL socket.
} else if (opts & PacketSocketFactory::OPT_SSLTCP) {
ASSERT(!(opts & PacketSocketFactory::OPT_TLS));
socket = new AsyncSSLSocket(socket);
}

View File

@ -22,12 +22,9 @@ class AsyncResolverInterface;
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.
};
PacketSocketFactory() { }

View File

@ -95,17 +95,6 @@ enum {
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() {}
@ -158,7 +147,6 @@ struct RelayServerConfig {
PortList ports;
RelayCredentials credentials;
int priority = 0;
TlsCertPolicy tls_cert_policy = TlsCertPolicy::TLS_CERT_POLICY_SECURE;
};
class PortAllocatorSession : public sigslot::has_slots<> {

View File

@ -491,9 +491,8 @@ void RelayEntry::Connect() {
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_TLS_FAKE
: 0;
int opts = (ra->proto == PROTO_SSLTCP) ?
rtc::PacketSocketFactory::OPT_SSLTCP : 0;
socket = port_->socket_factory()->CreateClientTcpSocket(
rtc::SocketAddress(port_->ip(), 0), ra->address,
port_->proxy(), port_->user_agent(), opts);

View File

@ -491,7 +491,7 @@ void TCPConnection::CreateOutgoingTcpSocket() {
ASSERT(outgoing_);
// TODO(guoweis): Handle failures here (unlikely since TCP).
int opts = (remote_candidate().protocol() == SSLTCP_PROTOCOL_NAME)
? rtc::PacketSocketFactory::OPT_TLS_FAKE
? rtc::PacketSocketFactory::OPT_SSLTCP
: 0;
socket_.reset(port()->socket_factory()->CreateClientTcpSocket(
rtc::SocketAddress(port()->ip(), 0), remote_candidate().address(),

View File

@ -329,14 +329,8 @@ bool TurnPort::CreateTurnClientSocket() {
// Apply server address TLS and insecure bits to options.
if (server_address_.proto == PROTO_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);

View File

@ -87,12 +87,6 @@ class TurnPort : public Port {
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);
@ -261,7 +255,6 @@ class TurnPort : public Port {
bool FailAndPruneConnection(const rtc::SocketAddress& address);
ProtocolAddress server_address_;
TlsCertPolicy tls_cert_policy_ = TlsCertPolicy::TLS_CERT_POLICY_SECURE;
RelayCredentials credentials_;
AttemptedServerSet attempted_server_addresses_;

View File

@ -1388,7 +1388,6 @@ void AllocationSequence::CreateTurnPort(const RelayServerConfig& config) {
session_->allocator()->origin());
}
ASSERT(port != NULL);
port->SetTlsCertPolicy(config.tls_cert_policy);
session_->AddAllocatedPort(port, this, true);
}
}

View File

@ -39,12 +39,6 @@ public class PeerConnection {
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,7 +87,6 @@ public class PeerConnection {
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) {
@ -101,18 +94,13 @@ public class PeerConnection {
}
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 + "] [" + tlsCertPolicy + "]";
return uri + "[" + username + ":" + password + "]";
}
}

View File

@ -82,7 +82,6 @@ ClassReferenceHolder::ClassReferenceHolder(JNIEnv* jni) {
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");

View File

@ -1566,23 +1566,6 @@ static PeerConnectionInterface::ContinualGatheringPolicy
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) {
@ -1594,24 +1577,16 @@ static void JavaIceServersToJsepIceServers(
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);
}
}