Relanding: Allow the DTLS fingerprint verification to occur after the handshake.

webrtc/base/opensslstreamadapter.cc

 /*
  *  Copyright 2004 The WebRTC Project Authors. All rights reserved.
  *
  *  Use of this source code is governed by a BSD-style license
  *  that can be found in the LICENSE file in the root of the source
  *  tree. An additional intellectual property rights grant can be found
  *  in the file PATENTS.  All contributing project authors may
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
@@ skipping 295 matching lines @@
   return peer_certificate_ ? std::unique_ptr<SSLCertificate>(
                                  peer_certificate_->GetReference())
                            : nullptr;
 }
 
 bool OpenSSLStreamAdapter::SetPeerCertificateDigest(const std::string
                                                     &digest_alg,
                                                     const unsigned char*
                                                     digest_val,
                                                     size_t digest_len) {
-  ASSERT(!peer_certificate_);
-  ASSERT(peer_certificate_digest_algorithm_.size() == 0);
-  ASSERT(ssl_server_name_.empty());
+  ASSERT(!certificate_verified_);
+  ASSERT(!have_peer_certificate_digest());
+  ASSERT(topology_ != TOPOLOGY_CLIENT_SERVER);

[pthatcher1, 2016/07/22 18:58:27]

Should we make these DCHECKS instead of crashing in production?

[Taylor Brandstetter, 2016/07/25 23:54:53]

ASSERTS are still only run in debug builds. I think the only difference is how
the error is recorded. I'll change the ASSERTS to DCHECKS though.

[pthatcher1, 2016/07/27 19:32:35]

Oh, right.  I got confused with CHECK.  We have too many similar things :).

   size_t expected_len;
 
   if (!OpenSSLDigest::GetDigestSize(digest_alg, &expected_len)) {
     LOG(LS_WARNING) << "Unknown digest algorithm: " << digest_alg;
     return false;
   }
-  if (expected_len != digest_len)
+  if (expected_len != digest_len) {
     return false;
+  }
 
   peer_certificate_digest_value_.SetData(digest_val, digest_len);
   peer_certificate_digest_algorithm_ = digest_alg;
 
+  if (peer_certificate_) {
+    if (VerifyPeerCertificate()) {

[pthatcher1, 2016/07/22 18:58:27]

I'd prefer some early returns:

if (!peer_certificate_) {
  return true;
}

if (!VerifyPeerCertificate()) {
  Error(...);
  return false;
}

if (state_ == SSL_CONNECTED) {
  ...
}

return true;

[Taylor Brandstetter, 2016/07/25 23:54:53]

Done.

+      if (state_ == SSL_CONNECTED) {
+        // Post event to unwind stack. Caller may not be expecting this event
+        // to occur synchronously.
+        PostEvent(SE_OPEN | SE_READ | SE_WRITE, 0);
+      }
+    } else {
+      Error("SetPeerCertificateDigest", -1, false);
+      return false;
+    }
+  }
   return true;
 }
 
 std::string OpenSSLStreamAdapter::SslCipherSuiteToName(int cipher_suite) {
 #ifdef OPENSSL_IS_BORINGSSL
   const SSL_CIPHER* ssl_cipher = SSL_get_cipher_by_value(cipher_suite);
   if (!ssl_cipher) {
     return std::string();
   }
   char* cipher_name = SSL_CIPHER_get_rfc_name(ssl_cipher);
@@ skipping 121 matching lines @@
 
   *crypto_suite = srtp_profile->id;
   ASSERT(!SrtpCryptoSuiteToName(*crypto_suite).empty());
   return true;
 #else
   return false;
 #endif
 }
 
 int OpenSSLStreamAdapter::StartSSLWithServer(const char* server_name) {
+  if (topology_ != TOPOLOGY_UNKNOWN || state_ != SSL_NONE) {
+    // Don't allow StartSSL to be called twice.
+    return -1;
+  }
   ASSERT(server_name != NULL && server_name[0] != '\0');
   ssl_server_name_ = server_name;
+  topology_ = TOPOLOGY_CLIENT_SERVER;
   return StartSSL();
 }
 
 int OpenSSLStreamAdapter::StartSSLWithPeer() {
+  if (topology_ != TOPOLOGY_UNKNOWN || state_ != SSL_NONE) {
+    // Don't allow StartSSL to be called twice.
+    return -1;
+  }
   ASSERT(ssl_server_name_.empty());
+  topology_ = TOPOLOGY_PEER_TO_PEER;
   // It is permitted to specify peer_certificate_ only later.
   return StartSSL();
 }
 
 void OpenSSLStreamAdapter::SetMode(SSLMode mode) {
   ASSERT(state_ == SSL_NONE);
   ssl_mode_ = mode;
 }
 
 void OpenSSLStreamAdapter::SetMaxProtocolVersion(SSLProtocolVersion version) {
@@ skipping 12 matching lines @@
   switch (state_) {
   case SSL_NONE:
     // pass-through in clear text
     return StreamAdapterInterface::Write(data, data_len, written, error);
 
   case SSL_WAIT:
   case SSL_CONNECTING:
     return SR_BLOCK;
 
   case SSL_CONNECTED:
+    if (waiting_to_verify_client_cert()) {
+      return SR_BLOCK;
+    }
     break;
 
   case SSL_ERROR:
   case SSL_CLOSED:
   default:
     if (error)
       *error = ssl_error_code_;
     return SR_ERROR;
   }
 
@@ skipping 39 matching lines @@
   switch (state_) {
     case SSL_NONE:
       // pass-through in clear text
       return StreamAdapterInterface::Read(data, data_len, read, error);
 
     case SSL_WAIT:
     case SSL_CONNECTING:
       return SR_BLOCK;
 
     case SSL_CONNECTED:
+      if (waiting_to_verify_client_cert()) {
+        return SR_BLOCK;
+      }
       break;
 
     case SSL_CLOSED:
       return SR_EOS;
 
     case SSL_ERROR:
     default:
       if (error)
         *error = ssl_error_code_;
       return SR_ERROR;
@@ skipping 82 matching lines @@
   ASSERT(state_ == SSL_CLOSED || state_ == SSL_ERROR);
   StreamAdapterInterface::Close();
 }
 
 StreamState OpenSSLStreamAdapter::GetState() const {
   switch (state_) {
     case SSL_WAIT:
     case SSL_CONNECTING:
       return SS_OPENING;
     case SSL_CONNECTED:
+      if (waiting_to_verify_client_cert()) {
+        return SS_OPENING;
+      }
       return SS_OPEN;
     default:
       return SS_CLOSED;
   };
   // not reached
 }
 
 void OpenSSLStreamAdapter::OnEvent(StreamInterface* stream, int events,
                                    int err) {
   int events_to_signal = 0;
@@ skipping 43 matching lines @@
     // SE_CLOSE is the only event that uses the final parameter to OnEvent().
     ASSERT(signal_error == 0);
     signal_error = err;
   }
   if (events_to_signal)
     StreamAdapterInterface::OnEvent(stream, events_to_signal, signal_error);
 }
 
 int OpenSSLStreamAdapter::StartSSL() {
   ASSERT(state_ == SSL_NONE);
+  ASSERT(topology_ != TOPOLOGY_UNKNOWN);
 
   if (StreamAdapterInterface::GetState() != SS_OPEN) {
     state_ = SSL_WAIT;
     return 0;
   }
 
   state_ = SSL_CONNECTING;
   if (int err = BeginSSL()) {
     Error("BeginSSL", err, false);
     return err;
   }
 
   return 0;
 }
 
 int OpenSSLStreamAdapter::BeginSSL() {
   ASSERT(state_ == SSL_CONNECTING);
-  // The underlying stream has open. If we are in peer-to-peer mode
-  // then a peer certificate must have been specified by now.
-  ASSERT(!ssl_server_name_.empty() ||
-         !peer_certificate_digest_algorithm_.empty());
   LOG(LS_INFO) << "BeginSSL: "
-               << (!ssl_server_name_.empty() ? ssl_server_name_ :
-                                               "with peer");
+               << (topology_ == TOPOLOGY_CLIENT_SERVER ? ssl_server_name_
+                                                       : "with peer");
 
   BIO* bio = NULL;
 
   // First set up the context
   ASSERT(ssl_ctx_ == NULL);
   ssl_ctx_ = SetupSSLContext();
   if (!ssl_ctx_)
     return -1;
 
   bio = BIO_new_stream(static_cast<StreamInterface*>(stream()));
@@ skipping 49 matching lines @@
 
   // Clear the DTLS timer
   Thread::Current()->Clear(this, MSG_TIMEOUT);
 
   int code = (role_ == SSL_CLIENT) ? SSL_connect(ssl_) : SSL_accept(ssl_);
   int ssl_error;
   switch (ssl_error = SSL_get_error(ssl_, code)) {
     case SSL_ERROR_NONE:
       LOG(LS_VERBOSE) << " -- success";
 
-      if (!SSLPostConnectionCheck(ssl_, ssl_server_name_.c_str(), NULL,
-                                  peer_certificate_digest_algorithm_)) {
+      if (!SSLPostConnectionCheck()) {
         LOG(LS_ERROR) << "TLS post connection check failed";
         return -1;
       }
 
       state_ = SSL_CONNECTED;
-      StreamAdapterInterface::OnEvent(stream(), SE_OPEN|SE_READ|SE_WRITE, 0);
+      if (!waiting_to_verify_client_cert()) {
+        // We have everything we need to start the connection, so signal
+        // SE_OPEN. If we need a client certificate fingerprint and don't have
+        // it yet, we'll instead signal SE_OPEN in SetPeerCertificateDigest.
+        StreamAdapterInterface::OnEvent(stream(), SE_OPEN | SE_READ | SE_WRITE,
+                                        0);
+      }
       break;
 
     case SSL_ERROR_WANT_READ: {
         LOG(LS_VERBOSE) << " -- error want read";
         struct timeval timeout;
         if (DTLSv1_get_timeout(ssl_, &timeout)) {
           int delay = timeout.tv_sec * 1000 + timeout.tv_usec/1000;
 
           Thread::Current()->PostDelayed(RTC_FROM_HERE, delay, this,
                                          MSG_TIMEOUT, 0);
@@ skipping 189 matching lines @@
     if (SSL_CTX_set_tlsext_use_srtp(ctx, srtp_ciphers_.c_str())) {
       SSL_CTX_free(ctx);
       return NULL;
     }
   }
 #endif
 
   return ctx;
 }
 
-int OpenSSLStreamAdapter::SSLVerifyCallback(int ok, X509_STORE_CTX* store) {
-  // Get our SSL structure from the store
-  SSL* ssl = reinterpret_cast<SSL*>(X509_STORE_CTX_get_ex_data(
-                                        store,
-                                        SSL_get_ex_data_X509_STORE_CTX_idx()));
-  OpenSSLStreamAdapter* stream =
-    reinterpret_cast<OpenSSLStreamAdapter*>(SSL_get_app_data(ssl));
-
-  if (stream->peer_certificate_digest_algorithm_.empty()) {
-    return 0;
-  }
-  X509* cert = X509_STORE_CTX_get_current_cert(store);
-  int depth = X509_STORE_CTX_get_error_depth(store);
-
-  // For now We ignore the parent certificates and verify the leaf against
-  // the digest.
-  //
-  // TODO(jiayl): Verify the chain is a proper chain and report the chain to
-  // |stream->peer_certificate_|.
-  if (depth > 0) {
-    LOG(LS_INFO) << "Ignored chained certificate at depth " << depth;
-    return 1;
+bool OpenSSLStreamAdapter::VerifyPeerCertificate() {
+  if (!have_peer_certificate_digest() || !peer_certificate_) {
+    LOG(LS_WARNING) << "Missing digest or peer certificate.";
+    return false;
   }
 
   unsigned char digest[EVP_MAX_MD_SIZE];
   size_t digest_length;
   if (!OpenSSLCertificate::ComputeDigest(
-           cert,
-           stream->peer_certificate_digest_algorithm_,
-           digest, sizeof(digest),
-           &digest_length)) {
+          peer_certificate_->x509(), peer_certificate_digest_algorithm_, digest,
+          sizeof(digest), &digest_length)) {
     LOG(LS_WARNING) << "Failed to compute peer cert digest.";
-    return 0;
+    return false;
   }
 
   Buffer computed_digest(digest, digest_length);
-  if (computed_digest != stream->peer_certificate_digest_value_) {
+  if (computed_digest != peer_certificate_digest_value_) {
     LOG(LS_WARNING) << "Rejected peer certificate due to mismatched digest.";
     return 0;
   }
   // Ignore any verification error if the digest matches, since there is no
   // value in checking the validity of a self-signed cert issued by untrusted
   // sources.
   LOG(LS_INFO) << "Accepted peer certificate.";
+  certificate_verified_ = true;
+  return true;
+}
+
+int OpenSSLStreamAdapter::SSLVerifyCallback(int ok, X509_STORE_CTX* store) {
+  // Get our SSL structure from the store
+  SSL* ssl = reinterpret_cast<SSL*>(
+      X509_STORE_CTX_get_ex_data(store, SSL_get_ex_data_X509_STORE_CTX_idx()));
+  X509* cert = X509_STORE_CTX_get_current_cert(store);
+  int depth = X509_STORE_CTX_get_error_depth(store);
+
+  // For now we ignore the parent certificates and verify the leaf against
+  // the digest.
+  //
+  // TODO(jiayl): Verify the chain is a proper chain and report the chain to
+  // |stream->peer_certificate_|.
+  if (depth > 0) {
+    LOG(LS_INFO) << "Ignored chained certificate at depth " << depth;
+    return 1;
+  }
+
+  OpenSSLStreamAdapter* stream =
+      reinterpret_cast<OpenSSLStreamAdapter*>(SSL_get_app_data(ssl));
 
   // Record the peer's certificate.
   stream->peer_certificate_.reset(new OpenSSLCertificate(cert));
-  return 1;
+
+  // If the peer certificate digest isn't known yet, we'll wait to verify
+  // until it's known, and for now just return a success status.
+  if (stream->peer_certificate_digest_algorithm_.empty()) {
+    LOG(LS_INFO) << "Waiting to verify certificate until digest is known.";
+    return 1;
+  }
+
+  return stream->VerifyPeerCertificate();
 }
 
 // This code is taken from the "Network Security with OpenSSL"
 // sample in chapter 5
-bool OpenSSLStreamAdapter::SSLPostConnectionCheck(SSL* ssl,
-                                                  const char* server_name,
-                                                  const X509* peer_cert,
-                                                  const std::string
-                                                  &peer_digest) {
-  ASSERT(server_name != NULL);
+bool OpenSSLStreamAdapter::SSLPostConnectionCheck() {
   bool ok;
-  if (server_name[0] != '\0') {  // traditional mode
-    ok = OpenSSLAdapter::VerifyServerName(ssl, server_name, ignore_bad_cert());
+  if (topology_ == TOPOLOGY_CLIENT_SERVER) {
+    ASSERT(!ssl_server_name_.empty());
+    ok = OpenSSLAdapter::VerifyServerName(ssl_, ssl_server_name_.c_str(),
+                                          ignore_bad_cert());
 
     if (ok) {
-      ok = (SSL_get_verify_result(ssl) == X509_V_OK ||
+      ok = (SSL_get_verify_result(ssl_) == X509_V_OK ||
             custom_verification_succeeded_);
     }

[pthatcher1, 2016/07/22 18:58:27]

We can probably just delete all of this.

-  } else {  // peer-to-peer mode
-    ASSERT((peer_cert != NULL) || (!peer_digest.empty()));
+  } else {
+    ASSERT(topology_ == TOPOLOGY_PEER_TO_PEER);
     // no server name validation
-    ok = true;
+    ok = peer_certificate_ || !client_auth_enabled();
   }
 
   if (!ok && ignore_bad_cert()) {
-    LOG(LS_ERROR) << "SSL_get_verify_result(ssl) = "
-                  << SSL_get_verify_result(ssl);
+    LOG(LS_ERROR) << "SSL_get_verify_result(ssl_) = "
+                  << SSL_get_verify_result(ssl_);
     LOG(LS_INFO) << "Other TLS post connection checks failed.";
     ok = true;
   }
 
   return ok;
 }
 
 bool OpenSSLStreamAdapter::HaveDtls() {
   return true;
 }
@@ skipping 89 matching lines @@
         return true;
     }
   }
 
   return false;
 }
 
 }  // namespace rtc
 
 #endif  // HAVE_OPENSSL_SSL_H