Implement GetChain for OpenSSLCertificate.

webrtc/rtc_base/opensslstreamadapter.cc

Index: webrtc/rtc_base/opensslstreamadapter.cc
diff --git a/webrtc/rtc_base/opensslstreamadapter.cc b/webrtc/rtc_base/opensslstreamadapter.cc
index 1c0b57894acd8de29a0bb5d9565ae079168053f6..71adbb4530d8a6b00e0bbce6184802190c9f79a8 100644
--- a/webrtc/rtc_base/opensslstreamadapter.cc
+++ b/webrtc/rtc_base/opensslstreamadapter.cc
@@ -38,6 +38,7 @@
 
 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 {
@@ -1110,15 +1111,20 @@ int OpenSSLStreamAdapter::SSLVerifyCallback(int ok, X509_STORE_CTX* 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));