Implement GetChain for OpenSSLCertificate.

webrtc/rtc_base/opensslidentity.cc

Index: webrtc/rtc_base/opensslidentity.cc
diff --git a/webrtc/rtc_base/opensslidentity.cc b/webrtc/rtc_base/opensslidentity.cc
index a6b6e1c38eb908e076356d6c219bf2befb1293f3..590abb466fc67c6f4b9b9919f4076621c4dd0938 100644
--- a/webrtc/rtc_base/opensslidentity.cc
+++ b/webrtc/rtc_base/opensslidentity.cc
@@ -16,11 +16,11 @@
 #include "webrtc/rtc_base/win32.h"  // NOLINT
 
 #include <openssl/bio.h>
+#include <openssl/bn.h>
+#include <openssl/crypto.h>
 #include <openssl/err.h>
 #include <openssl/pem.h>
-#include <openssl/bn.h>
 #include <openssl/rsa.h>
-#include <openssl/crypto.h>
 
 #include "webrtc/rtc_base/checks.h"
 #include "webrtc/rtc_base/helpers.h"
@@ -143,7 +143,7 @@ static X509* MakeCertificate(EVP_PKEY* pkey, const SSLIdentityParams& params) {
   LOG(LS_INFO) << "Returning certificate";
   return x509;
 
- error:
+error:
   BN_free(serial_number);
   X509_NAME_free(name);
   X509_free(x509);
@@ -217,8 +217,8 @@ std::string OpenSSLKeyPair::PrivateKeyToPEMString() const {
     RTC_NOTREACHED();
     return "";
   }
-  if (!PEM_write_bio_PrivateKey(
-      temp_memory_bio, pkey_, nullptr, nullptr, 0, nullptr, nullptr)) {
+  if (!PEM_write_bio_PrivateKey(temp_memory_bio, pkey_, nullptr, nullptr, 0,
+                                nullptr, nullptr)) {
     LOG_F(LS_ERROR) << "Failed to write private key";
     BIO_free(temp_memory_bio);
     RTC_NOTREACHED();
@@ -278,8 +278,25 @@ static void PrintCert(X509* x509) {
 }
 #endif
 
+OpenSSLCertificate::OpenSSLCertificate(X509* x509) : x509_(x509) {
+  AddReference(x509_);
+}
+
+OpenSSLCertificate::OpenSSLCertificate(STACK_OF(X509) * chain) {
+  x509_ = sk_X509_value(chain, 0);
+  LOG(LS_INFO) << "leaf_cert:" << x509_->name;

[Taylor Brandstetter, 2017/09/26 00:57:26]

Are these logs statements intended to be in the final CL? Maybe should be
LS_VERBOSE?

[pthatcher1, 2017/09/26 20:55:07]

Even at verbose level I'm not sure they are worth having in there. 

+  AddReference(x509_);
+  for (size_t i = 1; i < sk_X509_num(chain); ++i) {

[Taylor Brandstetter, 2017/09/26 00:57:26]

May be a dumb question, but is there a reason for the chain not including the
leaf cert?

+    X509* x509 = sk_X509_value(chain, i);
+    AddReference(x509);
+    LOG(LS_INFO) << "parent certificate name:" << x509->name;
+    cert_chain_.push_back(new OpenSSLCertificate(x509));

[pthatcher1, 2017/09/26 20:55:07]

Passing in the x509 here cause the constructor to take ownership and
AddReference.  But you also AddReference two lines up.  Does that mean two
references are added?

+  }
+}
+
 OpenSSLCertificate* OpenSSLCertificate::Generate(
-    OpenSSLKeyPair* key_pair, const SSLIdentityParams& params) {
+    OpenSSLKeyPair* key_pair,
+    const SSLIdentityParams& params) {
   SSLIdentityParams actual_params(params);
   if (actual_params.common_name.empty()) {
     // Use a random string, arbitrarily 8chars long.
@@ -362,10 +379,18 @@ bool OpenSSLCertificate::GetSignatureDigestAlgorithm(
 }
 
 std::unique_ptr<SSLCertChain> OpenSSLCertificate::GetChain() const {
-  // Chains are not yet supported when using OpenSSL.
-  // OpenSSLStreamAdapter::SSLVerifyCallback currently requires the remote
-  // certificate to be self-signed.
-  return nullptr;
+  if (cert_chain_.empty()) {
+    return nullptr;
+  }
+  std::vector<SSLCertificate*> new_certs(cert_chain_.size());
+  std::transform(cert_chain_.begin(), cert_chain_.end(), new_certs.begin(),
+                 [](OpenSSLCertificate* cert) -> SSLCertificate* {
+                   return cert->GetReference();
+                 });
+  std::unique_ptr<SSLCertChain> chain(new SSLCertChain(new_certs));
+  std::for_each(new_certs.begin(), new_certs.end(),
+                [](SSLCertificate* cert) { delete cert; });

[pthatcher1, 2017/09/26 20:55:07]

I find this confusing.  We create a copy of the vector by calling GetReference
on each one, then pass that into SSLCertChain, but then we delete all of them
references?  If this is the right way to do it, I'd like a comment explaining
why it's done this way.

+  return chain;
 }
 
 bool OpenSSLCertificate::ComputeDigest(const std::string& algorithm,
@@ -401,7 +426,18 @@ OpenSSLCertificate::~OpenSSLCertificate() {
 }
 
 OpenSSLCertificate* OpenSSLCertificate::GetReference() const {
-  return new OpenSSLCertificate(x509_);
+  if (cert_chain_.empty()) {
+    return new OpenSSLCertificate(x509_);
+  }
+  STACK_OF(X509)* stack_x509 = sk_X509_new_null();
+  sk_X509_push(stack_x509, x509_);
+  LOG(LS_INFO) << "Push cert to stack:" << x509_->name;

[Taylor Brandstetter, 2017/09/26 00:57:26]

Same question about log messages. Should be verbose or be removed.

+  for (auto cert_it = cert_chain_.begin(); cert_it != cert_chain_.end();
+       ++cert_it) {
+    LOG(LS_INFO) << "Push cert to stack:" << (*cert_it)->x509()->name;
+    sk_X509_push(stack_x509, (*cert_it)->x509());
+  }
+  return new OpenSSLCertificate(stack_x509);
 }
 
 std::string OpenSSLCertificate::ToPEMString() const {
@@ -440,12 +476,12 @@ void OpenSSLCertificate::ToDER(Buffer* der_buffer) const {
   BIO_free(bio);
 }
 
-void OpenSSLCertificate::AddReference() const {
-  RTC_DCHECK(x509_ != nullptr);
+void OpenSSLCertificate::AddReference(X509* x509) const {
+  RTC_DCHECK(x509 != nullptr);
 #if defined(OPENSSL_IS_BORINGSSL)
-  X509_up_ref(x509_);
+  X509_up_ref(x509);
 #else
-  CRYPTO_add(&x509_->references, 1, CRYPTO_LOCK_X509);
+  CRYPTO_add(&x509->references, 1, CRYPTO_LOCK_X509);
 #endif
 }
 
@@ -516,9 +552,8 @@ OpenSSLIdentity* OpenSSLIdentity::GenerateForTest(
   return GenerateInternal(params);
 }
 
-SSLIdentity* OpenSSLIdentity::FromPEMStrings(
-    const std::string& private_key,
-    const std::string& certificate) {
+SSLIdentity* OpenSSLIdentity::FromPEMStrings(const std::string& private_key,
+                                             const std::string& certificate) {
   std::unique_ptr<OpenSSLCertificate> cert(
       OpenSSLCertificate::FromPEMString(certificate));
   if (!cert) {
@@ -533,8 +568,7 @@ SSLIdentity* OpenSSLIdentity::FromPEMStrings(
     return nullptr;
   }
 
-  return new OpenSSLIdentity(key_pair,
-                             cert.release());
+  return new OpenSSLIdentity(key_pair, cert.release());
 }
 
 const OpenSSLCertificate& OpenSSLIdentity::certificate() const {
@@ -549,7 +583,7 @@ OpenSSLIdentity* OpenSSLIdentity::GetReference() const {
 bool OpenSSLIdentity::ConfigureIdentity(SSL_CTX* ctx) {
   // 1 is the documented success return code.
   if (SSL_CTX_use_certificate(ctx, certificate_->x509()) != 1 ||
-     SSL_CTX_use_PrivateKey(ctx, key_pair_->pkey()) != 1) {
+      SSL_CTX_use_PrivateKey(ctx, key_pair_->pkey()) != 1) {
     LogSSLErrors("Configuring key and certificate");
     return false;
   }