Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(30)

Unified Diff: webrtc/p2p/base/dtlstransportchannel.cc

Issue 2163683003: Relanding: Allow the DTLS fingerprint verification to occur after the handshake. (Closed) Base URL: https://chromium.googlesource.com/external/webrtc.git@master
Patch Set: Fixing comment grammar. Created 4 years, 3 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « webrtc/p2p/base/dtlstransportchannel.h ('k') | webrtc/p2p/base/dtlstransportchannel_unittest.cc » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: webrtc/p2p/base/dtlstransportchannel.cc
diff --git a/webrtc/p2p/base/dtlstransportchannel.cc b/webrtc/p2p/base/dtlstransportchannel.cc
index 3dde8fea0535199e31c6ff75c4c9575d21b37b16..d95bdcd916b1814ae4eda1eaeebd4f333e59f2ac 100644
--- a/webrtc/p2p/base/dtlstransportchannel.cc
+++ b/webrtc/p2p/base/dtlstransportchannel.cc
@@ -179,7 +179,7 @@ bool DtlsTransportChannelWrapper::SetSslMaxProtocolVersion(
}
bool DtlsTransportChannelWrapper::SetSslRole(rtc::SSLRole role) {
- if (dtls_state() == DTLS_TRANSPORT_CONNECTED) {
+ if (dtls_) {
if (ssl_role_ != role) {
LOG(LS_ERROR) << "SSL Role can't be reversed after the session is setup.";
return false;
@@ -235,12 +235,33 @@ bool DtlsTransportChannelWrapper::SetRemoteFingerprint(
}
// At this point we know we are doing DTLS
+ bool fingerprint_changing = remote_fingerprint_value_.size() > 0u;
remote_fingerprint_value_ = std::move(remote_fingerprint_value);
remote_fingerprint_algorithm_ = digest_alg;
- if (dtls_) {
- // If the fingerprint is changing, we'll tear down the DTLS association and
- // create a new one, resetting our state.
+ if (dtls_ && !fingerprint_changing) {
+ // This can occur if DTLS is set up before a remote fingerprint is
+ // received. For instance, if we set up DTLS due to receiving an early
+ // ClientHello.
+ rtc::SSLPeerCertificateDigestError err;
+ if (!dtls_->SetPeerCertificateDigest(
+ remote_fingerprint_algorithm_,
+ reinterpret_cast<unsigned char*>(remote_fingerprint_value_.data()),
+ remote_fingerprint_value_.size(), &err)) {
+ LOG_J(LS_ERROR, this) << "Couldn't set DTLS certificate digest.";
+ set_dtls_state(DTLS_TRANSPORT_FAILED);
+ // If the error is "verification failed", don't return false, because
+ // this means the fingerprint was formatted correctly but didn't match
+ // the certificate from the DTLS handshake. Thus the DTLS state should go
+ // to "failed", but SetRemoteDescription shouldn't fail.
+ return err == rtc::SSLPeerCertificateDigestError::VERIFICATION_FAILED;
+ }
+ return true;
+ }
+
+ // If the fingerprint is changing, we'll tear down the DTLS association and
+ // create a new one, resetting our state.
+ if (dtls_ && fingerprint_changing) {
dtls_.reset(nullptr);
set_dtls_state(DTLS_TRANSPORT_NEW);
set_writable(false);
@@ -282,7 +303,8 @@ bool DtlsTransportChannelWrapper::SetupDtls() {
dtls_->SignalEvent.connect(this, &DtlsTransportChannelWrapper::OnDtlsEvent);
dtls_->SignalSSLHandshakeError.connect(
this, &DtlsTransportChannelWrapper::OnDtlsHandshakeError);
- if (!dtls_->SetPeerCertificateDigest(
+ if (remote_fingerprint_value_.size() &&
+ !dtls_->SetPeerCertificateDigest(
remote_fingerprint_algorithm_,
reinterpret_cast<unsigned char*>(remote_fingerprint_value_.data()),
remote_fingerprint_value_.size())) {
@@ -401,6 +423,10 @@ int DtlsTransportChannelWrapper::SendPacket(
}
}
+bool DtlsTransportChannelWrapper::IsDtlsConnected() {
+ return dtls_ && dtls_->IsTlsConnected();
+}
+
// The state transition logic here is as follows:
// (1) If we're not doing DTLS-SRTP, then the state is just the
// state of the underlying impl()
@@ -481,6 +507,14 @@ void DtlsTransportChannelWrapper::OnReadPacket(
LOG_J(LS_INFO, this) << "Caching DTLS ClientHello packet until DTLS is "
<< "started.";
cached_client_hello_.SetData(data, size);
+ // If we haven't started setting up DTLS yet (because we don't have a
+ // remote fingerprint/role), we can use the client hello as a clue that
+ // the peer has chosen the client role, and proceed with the handshake.
+ // The fingerprint will be verified when it's set.
+ if (!dtls_ && local_certificate_) {
+ SetSslRole(rtc::SSL_SERVER);
+ SetupDtls();
+ }
} else {
LOG_J(LS_INFO, this) << "Not a DTLS ClientHello packet; dropping.";
}
@@ -554,8 +588,20 @@ void DtlsTransportChannelWrapper::OnDtlsEvent(rtc::StreamInterface* dtls,
if (sig & rtc::SE_READ) {
char buf[kMaxDtlsPacketLen];
size_t read;
- if (dtls_->Read(buf, sizeof(buf), &read, NULL) == rtc::SR_SUCCESS) {
+ int read_error;
+ rtc::StreamResult ret = dtls_->Read(buf, sizeof(buf), &read, &read_error);
+ if (ret == rtc::SR_SUCCESS) {
SignalReadPacket(this, buf, read, rtc::CreatePacketTime(0), 0);
+ } else if (ret == rtc::SR_EOS) {
+ // Remote peer shut down the association with no error.
+ LOG_J(LS_INFO, this) << "DTLS channel closed";
+ set_writable(false);
+ set_dtls_state(DTLS_TRANSPORT_CLOSED);
+ } else if (ret == rtc::SR_ERROR) {
+ // Remote peer shut down the association with an error.
+ LOG_J(LS_INFO, this) << "DTLS channel error, code=" << read_error;
+ set_writable(false);
+ set_dtls_state(DTLS_TRANSPORT_FAILED);
}
}
if (sig & rtc::SE_CLOSE) {
« no previous file with comments | « webrtc/p2p/base/dtlstransportchannel.h ('k') | webrtc/p2p/base/dtlstransportchannel_unittest.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698