Implement GetChain for OpenSSLCertificate.

webrtc/rtc_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 20 matching lines @@
 #include "webrtc/rtc_base/openssldigest.h"
 #include "webrtc/rtc_base/opensslidentity.h"
 #include "webrtc/rtc_base/safe_conversions.h"
 #include "webrtc/rtc_base/stream.h"
 #include "webrtc/rtc_base/stringutils.h"
 #include "webrtc/rtc_base/thread.h"
 #include "webrtc/rtc_base/timeutils.h"
 
 namespace {
   bool g_use_time_callback_for_testing = false;
+  const int kMaxSupportedCertChainDepth = 3;

[davidben_webrtc, 2017/09/26 23:21:46]

3 is a remarkably small number. Where did you get that value from?

 }
 
 namespace rtc {
 
 #if (OPENSSL_VERSION_NUMBER < 0x10001000L)
 #error "webrtc requires at least OpenSSL version 1.0.1, to support DTLS-SRTP"
 #endif
 
 // SRTP cipher suite table. |internal_name| is used to construct a
 // colon-separated profile strings which is needed by
@@ skipping 1052 matching lines @@
 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));
+  if (depth == 0) {

[davidben_webrtc, 2017/09/26 23:21:46]

Huh? This is the error depth...

+    // Record the peer's certificate.
+    stream->peer_certificate_.reset(new OpenSSLCertificate(cert));
+  } else {
+    STACK_OF(X509)* chain = X509_STORE_CTX_get_chain(store);
+    if (sk_X509_num(chain) >= kMaxSupportedCertChainDepth) {
+      LOG(LS_INFO) << "Ignore chained certificate at depth " << depth;
+      return 1;
+    }
+    stream->peer_certificate_.reset(new OpenSSLCertificate(chain));

[davidben_webrtc, 2017/09/26 23:21:46]

This callback gets called multiple times as the certificate gets looked at. This
is not a reasonable place to stash the chain. You also definitely should not be
special-casing the case with and the case without the chain. They should be
uniform.

It looks like WebRTC is trying to do custom verification altogether and not
build a normal certificate chain? Is that correct, pthatcher? In that case,
SSL_CTX_set_cert_verify_callback will let you override the whole thing.

To be honest, I'm a little puzzled what this code is currently doing, since the
callback you all are currently using does not skip the usual cert verification
logic. It just lets you modify it. You're probably doing all sorts of
computation you don't want to do.

+  }
 
   // Record the peer's certificate.
   stream->peer_certificate_.reset(new OpenSSLCertificate(cert));
 
   // 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;
   }
@@ skipping 78 matching lines @@
   }
 
   return false;
 }
 
 void OpenSSLStreamAdapter::enable_time_callback_for_testing() {
   g_use_time_callback_for_testing = true;
 }
 
 }  // namespace rtc