update main-with-bazel from master branch

src/crypto/bio/bio_test.cc

@@ -50,6 +50,8 @@ using Socket = int;
 #define INVALID_SOCKET (-1)
 static int closesocket(int sock) { return close(sock); }
 static std::string LastSocketError() { return strerror(errno); }
+static const int kOpenReadOnlyBinary = O_RDONLY;
+static const int kOpenReadOnlyText = O_RDONLY;
 #else
 using Socket = SOCKET;
 static std::string LastSocketError() {
@@ -57,6 +59,8 @@ static std::string LastSocketError() {
   snprintf(buf, sizeof(buf), "%d", WSAGetLastError());
   return buf;
 }
+static const int kOpenReadOnlyBinary = _O_RDONLY | _O_BINARY;
+static const int kOpenReadOnlyText = O_RDONLY | _O_TEXT;
 #endif
 
 class OwnedSocket {
@@ -673,21 +677,16 @@ TEST(BIOTest, Gets) {
 
       {
         SCOPED_TRACE("fd");
-#if defined(OPENSSL_WINDOWS)
-        int open_flags = _O_RDONLY | _O_BINARY;
-#else
-        int open_flags = O_RDONLY;
-#endif
 
         // Test |BIO_NOCLOSE|.
-        ScopedFD fd = file.OpenFD(open_flags);
+        ScopedFD fd = file.OpenFD(kOpenReadOnlyBinary);
         ASSERT_TRUE(fd.is_valid());
         bssl::UniquePtr<BIO> bio(BIO_new_fd(fd.get(), BIO_NOCLOSE));
         ASSERT_TRUE(bio);
         check_bio_gets(bio.get());
 
         // Test |BIO_CLOSE|.
-        fd = file.OpenFD(open_flags);
+        fd = file.OpenFD(kOpenReadOnlyBinary);
         ASSERT_TRUE(fd.is_valid());
         bio.reset(BIO_new_fd(fd.get(), BIO_CLOSE));
         ASSERT_TRUE(bio);
@@ -706,22 +705,87 @@ TEST(BIOTest, Gets) {
   EXPECT_EQ(c, 'a');
 }
 
-// Test that, on Windows, |BIO_read_filename| opens files in binary mode.
-TEST(BIOTest, BinaryMode) {
+// Test that, on Windows, file BIOs correctly handle text vs binary mode.
+TEST(BIOTest, FileMode) {
   if (SkipTempFileTests()) {
     GTEST_SKIP();
   }
 
-  TemporaryFile file;
-  ASSERT_TRUE(file.Init("\r\n"));
+  TemporaryFile temp;
+  ASSERT_TRUE(temp.Init("hello\r\nworld"));
 
-  // Reading from the file should give back the exact bytes we put in.
+  auto expect_file_contents = [](BIO *bio, const std::string &str) {
+    // Read more than expected, to make sure we've reached the end of the file.
+    std::vector<char> buf(str.size() + 100);
+    int len = BIO_read(bio, buf.data(), static_cast<int>(buf.size()));
+    ASSERT_GT(len, 0);
+    EXPECT_EQ(Bytes(buf.data(), len), Bytes(str));
+  };
+  auto expect_binary_mode = [&](BIO *bio) {
+    expect_file_contents(bio, "hello\r\nworld");
+  };
+  auto expect_text_mode = [&](BIO *bio) {
+#if defined(OPENSSL_WINDOWS)
+    expect_file_contents(bio, "hello\nworld");
+#else
+    expect_file_contents(bio, "hello\r\nworld");
+#endif
+  };
+
+  // |BIO_read_filename| should open in binary mode.
   bssl::UniquePtr<BIO> bio(BIO_new(BIO_s_file()));
   ASSERT_TRUE(bio);
-  ASSERT_TRUE(BIO_read_filename(bio.get(), file.path().c_str()));
-  char buf[2];
-  ASSERT_EQ(2, BIO_read(bio.get(), buf, 2));
-  EXPECT_EQ(Bytes(buf, 2), Bytes("\r\n"));
+  ASSERT_TRUE(BIO_read_filename(bio.get(), temp.path().c_str()));
+  expect_binary_mode(bio.get());
+
+  // |BIO_new_file| should use the specified mode.
+  bio.reset(BIO_new_file(temp.path().c_str(), "rb"));
+  ASSERT_TRUE(bio);
+  expect_binary_mode(bio.get());
+
+  bio.reset(BIO_new_file(temp.path().c_str(), "r"));
+  ASSERT_TRUE(bio);
+  expect_text_mode(bio.get());
+
+  // |BIO_new_fp| inherits the file's existing mode by default.
+  ScopedFILE file = temp.Open("rb");
+  ASSERT_TRUE(file);
+  bio.reset(BIO_new_fp(file.get(), BIO_NOCLOSE));
+  ASSERT_TRUE(bio);
+  expect_binary_mode(bio.get());
+
+  file = temp.Open("r");
+  ASSERT_TRUE(file);
+  bio.reset(BIO_new_fp(file.get(), BIO_NOCLOSE));
+  ASSERT_TRUE(bio);
+  expect_text_mode(bio.get());
+
+  // However, |BIO_FP_TEXT| changes the file to be text mode, no matter how it
+  // was opened.
+  file = temp.Open("rb");
+  ASSERT_TRUE(file);
+  bio.reset(BIO_new_fp(file.get(), BIO_NOCLOSE | BIO_FP_TEXT));
+  ASSERT_TRUE(bio);
+  expect_text_mode(bio.get());
+
+  file = temp.Open("r");
+  ASSERT_TRUE(file);
+  bio.reset(BIO_new_fp(file.get(), BIO_NOCLOSE | BIO_FP_TEXT));
+  ASSERT_TRUE(bio);
+  expect_text_mode(bio.get());
+
+  // |BIO_new_fd| inherits the FD's existing mode.
+  ScopedFD fd = temp.OpenFD(kOpenReadOnlyBinary);
+  ASSERT_TRUE(fd.is_valid());
+  bio.reset(BIO_new_fd(fd.get(), BIO_NOCLOSE));
+  ASSERT_TRUE(bio);
+  expect_binary_mode(bio.get());
+
+  fd = temp.OpenFD(kOpenReadOnlyText);
+  ASSERT_TRUE(fd.is_valid());
+  bio.reset(BIO_new_fd(fd.get(), BIO_NOCLOSE));
+  ASSERT_TRUE(bio);
+  expect_text_mode(bio.get());
 }
 
 // Run through the tests twice, swapping |bio1| and |bio2|, for symmetry.

src/crypto/bio/file.c

@@ -73,6 +73,7 @@
 
 #include <openssl/bio.h>
 
+#include <assert.h>
 #include <errno.h>
 #include <stdio.h>
 #include <string.h>
@@ -82,6 +83,10 @@
 
 #include "../internal.h"
 
+#if defined(OPENSSL_WINDOWS)
+#include <fcntl.h>
+#include <io.h>
+#endif
 
 #define BIO_FP_READ 0x02
 #define BIO_FP_WRITE 0x04
@@ -122,14 +127,13 @@ BIO *BIO_new_file(const char *filename, const char *mode) {
   return ret;
 }
 
-BIO *BIO_new_fp(FILE *stream, int close_flag) {
+BIO *BIO_new_fp(FILE *stream, int flags) {
   BIO *ret = BIO_new(BIO_s_file());
-
   if (ret == NULL) {
     return NULL;
   }
 
-  BIO_set_fp(ret, stream, close_flag);
+  BIO_set_fp(ret, stream, flags);
   return ret;
 }
 
@@ -196,6 +200,17 @@ static long file_ctrl(BIO *b, int cmd, long num, void *ptr) {
       break;
     case BIO_C_SET_FILE_PTR:
       file_free(b);
+      static_assert((BIO_CLOSE & BIO_FP_TEXT) == 0,
+                    "BIO_CLOSE and BIO_FP_TEXT must not collide");
+#if defined(OPENSSL_WINDOWS)
+      // If |BIO_FP_TEXT| is not set, OpenSSL will switch the file to binary
+      // mode. BoringSSL intentionally diverges here because it means code
+      // tested under POSIX will inadvertently change the state of |FILE|
+      // objects when wrapping them in a |BIO|.
+      if (num & BIO_FP_TEXT) {
+        _setmode(_fileno(ptr), _O_TEXT);
+      }
+#endif
       b->shutdown = (int)num & BIO_CLOSE;
       b->ptr = ptr;
       b->init = 1;
@@ -287,8 +302,8 @@ int BIO_get_fp(BIO *bio, FILE **out_file) {
   return (int)BIO_ctrl(bio, BIO_C_GET_FILE_PTR, 0, (char *)out_file);
 }
 
-int BIO_set_fp(BIO *bio, FILE *file, int close_flag) {
-  return (int)BIO_ctrl(bio, BIO_C_SET_FILE_PTR, close_flag, (char *)file);
+int BIO_set_fp(BIO *bio, FILE *file, int flags) {
+  return (int)BIO_ctrl(bio, BIO_C_SET_FILE_PTR, flags, (char *)file);
 }
 
 int BIO_read_filename(BIO *bio, const char *filename) {

src/include/openssl/bio.h

@@ -473,22 +473,59 @@ OPENSSL_EXPORT int BIO_get_fd(BIO *bio, int *out_fd);
 OPENSSL_EXPORT const BIO_METHOD *BIO_s_file(void);
 
 // BIO_new_file creates a file BIO by opening |filename| with the given mode.
-// See the |fopen| manual page for details of the mode argument.
+// See the |fopen| manual page for details of the mode argument. On Windows,
+// files may be opened in either binary or text mode so, as in |fopen|, callers
+// must specify the desired option in |mode|.
 OPENSSL_EXPORT BIO *BIO_new_file(const char *filename, const char *mode);
 
-// BIO_new_fp creates a new file BIO that wraps the given |FILE|. If
-// |close_flag| is |BIO_CLOSE|, then |fclose| will be called on |stream| when
-// the BIO is closed.
-OPENSSL_EXPORT BIO *BIO_new_fp(FILE *stream, int close_flag);
+// BIO_FP_TEXT indicates the |FILE| should be switched to text mode on Windows.
+// It has no effect on non-Windows platforms.
+#define BIO_FP_TEXT 0x10
+
+// BIO_new_fp creates a new file BIO that wraps |file|. If |flags| contains
+// |BIO_CLOSE|, then |fclose| will be called on |file| when the BIO is closed.
+//
+// On Windows, if |flags| contains |BIO_FP_TEXT|, this function will
+// additionally switch |file| to text mode. This is not recommended, but may be
+// required for OpenSSL compatibility. If |file| was not already in text mode,
+// mode changes can cause unflushed data in |file| to be written in unexpected
+// ways. See |_setmode| in Windows documentation for details.
+//
+// Unlike OpenSSL, if |flags| does not contain |BIO_FP_TEXT|, the translation
+// mode of |file| is left as-is. In OpenSSL, |file| will be set to binary, with
+// the same pitfalls as above. BoringSSL does not do this so that wrapping a
+// |FILE| in a |BIO| will not inadvertently change its state.
+//
+// To avoid these pitfalls, callers should set the desired translation mode when
+// opening the file. If targeting just BoringSSL, this is sufficient. If
+// targeting both OpenSSL and BoringSSL, callers should set |BIO_FP_TEXT| to
+// match the desired state of the file.
+OPENSSL_EXPORT BIO *BIO_new_fp(FILE *file, int flags);
 
 // BIO_get_fp sets |*out_file| to the current |FILE| for |bio|. It returns one
 // on success and zero otherwise.
 OPENSSL_EXPORT int BIO_get_fp(BIO *bio, FILE **out_file);
 
-// BIO_set_fp sets the |FILE| for |bio|. If |close_flag| is |BIO_CLOSE| then
+// BIO_set_fp sets the |FILE| for |bio|. If |flags| contains |BIO_CLOSE| then
 // |fclose| will be called on |file| when |bio| is closed. It returns one on
 // success and zero otherwise.
-OPENSSL_EXPORT int BIO_set_fp(BIO *bio, FILE *file, int close_flag);
+//
+// On Windows, if |flags| contains |BIO_FP_TEXT|, this function will
+// additionally switch |file| to text mode. This is not recommended, but may be
+// required for OpenSSL compatibility. If |file| was not already in text mode,
+// mode changes can cause unflushed data in |file| to be written in unexpected
+// ways. See |_setmode| in Windows documentation for details.
+//
+// Unlike OpenSSL, if |flags| does not contain |BIO_FP_TEXT|, the translation
+// mode of |file| is left as-is. In OpenSSL, |file| will be set to binary, with
+// the same pitfalls as above. BoringSSL does not do this so that wrapping a
+// |FILE| in a |BIO| will not inadvertently change its state.
+//
+// To avoid these pitfalls, callers should set the desired translation mode when
+// opening the file. If targeting just BoringSSL, this is sufficient. If
+// targeting both OpenSSL and BoringSSL, callers should set |BIO_FP_TEXT| to
+// match the desired state of the file.
+OPENSSL_EXPORT int BIO_set_fp(BIO *bio, FILE *file, int flags);
 
 // BIO_read_filename opens |filename| for reading and sets the result as the
 // |FILE| for |bio|. It returns one on success and zero otherwise. The |FILE|

src/include/openssl/ssl.h

@@ -919,8 +919,9 @@ OPENSSL_EXPORT int SSL_CREDENTIAL_set1_private_key(SSL_CREDENTIAL *cred,
 OPENSSL_EXPORT int SSL_CREDENTIAL_set1_signing_algorithm_prefs(
     SSL_CREDENTIAL *cred, const uint16_t *prefs, size_t num_prefs);
 
-// SSL_CREDENTIAL_set1_cert_chain sets |cred|'s certificate chain to |num_cert|s
-// certificates from |certs|. It returns one on success and zero on error.
+// SSL_CREDENTIAL_set1_cert_chain sets |cred|'s certificate chain, starting from
+// the leaf, to |num_cert|s certificates from |certs|. It returns one on success
+// and zero on error.
 OPENSSL_EXPORT int SSL_CREDENTIAL_set1_cert_chain(SSL_CREDENTIAL *cred,
                                                   CRYPTO_BUFFER *const *certs,
                                                   size_t num_certs);
@@ -1002,11 +1003,13 @@ OPENSSL_EXPORT int SSL_CTX_use_certificate(SSL_CTX *ctx, X509 *x509);
 OPENSSL_EXPORT int SSL_use_certificate(SSL *ssl, X509 *x509);
 
 // SSL_CTX_use_PrivateKey sets |ctx|'s private key to |pkey|. It returns one on
-// success and zero on failure.
+// success and zero on failure. If |ctx| had a private key or
+// |SSL_PRIVATE_KEY_METHOD| previously configured, it is replaced.
 OPENSSL_EXPORT int SSL_CTX_use_PrivateKey(SSL_CTX *ctx, EVP_PKEY *pkey);
 
 // SSL_use_PrivateKey sets |ssl|'s private key to |pkey|. It returns one on
-// success and zero on failure.
+// success and zero on failure. If |ssl| had a private key or
+// |SSL_PRIVATE_KEY_METHOD| previously configured, it is replaced.
 OPENSSL_EXPORT int SSL_use_PrivateKey(SSL *ssl, EVP_PKEY *pkey);
 
 // SSL_CTX_set0_chain sets |ctx|'s certificate chain, excluding the leaf, to

src/ssl/internal.h

@@ -1640,6 +1640,10 @@ struct ssl_credential_st : public bssl::RefCounted<ssl_credential_st> {
   bool SetLeafCert(bssl::UniquePtr<CRYPTO_BUFFER> leaf,
                    bool discard_key_on_mismatch);
 
+  // ClearIntermediateCerts clears intermediate certificates in the certificate
+  // chain, while preserving the leaf.
+  void ClearIntermediateCerts();
+
   // AppendIntermediateCert appends |cert| to the certificate chain. If there is
   // no leaf certificate configured, it leaves a placeholder null in |chain|. It
   // returns one on success and zero on error.

src/ssl/ssl_cert.cc

@@ -191,6 +191,7 @@ static int cert_set_chain_and_key(
     return 0;
   }
 
+  cert->default_credential->ClearCertAndKey();
   if (!SSL_CREDENTIAL_set1_cert_chain(cert->default_credential.get(), certs,
                                       num_certs)) {
     return 0;

src/ssl/ssl_credential.cc

@@ -203,6 +203,16 @@ bool ssl_credential_st::SetLeafCert(UniquePtr<CRYPTO_BUFFER> leaf,
   return true;
 }
 
+void ssl_credential_st::ClearIntermediateCerts() {
+  if (chain == nullptr) {
+    return;
+  }
+
+  while (sk_CRYPTO_BUFFER_num(chain.get()) > 1) {
+    CRYPTO_BUFFER_free(sk_CRYPTO_BUFFER_pop(chain.get()));
+  }
+}
+
 bool ssl_credential_st::AppendIntermediateCert(UniquePtr<CRYPTO_BUFFER> cert) {
   if (!UsesX509()) {
     OPENSSL_PUT_ERROR(SSL, ERR_R_SHOULD_NOT_HAVE_BEEN_CALLED);
@@ -249,6 +259,7 @@ int SSL_CREDENTIAL_set1_private_key(SSL_CREDENTIAL *cred, EVP_PKEY *key) {
   }
 
   cred->privkey = UpRef(key);
+  cred->key_method = nullptr;
   return 1;
 }
 
@@ -259,6 +270,7 @@ int SSL_CREDENTIAL_set_private_key_method(
     return 0;
   }
 
+  cred->privkey = nullptr;
   cred->key_method = key_method;
   return 1;
 }
@@ -275,6 +287,7 @@ int SSL_CREDENTIAL_set1_cert_chain(SSL_CREDENTIAL *cred,
     return 0;
   }
 
+  cred->ClearIntermediateCerts();
   for (size_t i = 1; i < num_certs; i++) {
     if (!cred->AppendIntermediateCert(UpRef(certs[i]))) {
       return 0;

src/ssl/ssl_test.cc

@@ -1317,6 +1317,34 @@ static bssl::UniquePtr<EVP_PKEY> KeyFromPEM(const char *pem) {
       PEM_read_bio_PrivateKey(bio.get(), nullptr, nullptr, nullptr));
 }
 
+static bssl::UniquePtr<CRYPTO_BUFFER> BufferFromPEM(const char *pem) {
+  bssl::UniquePtr<BIO> bio(BIO_new_mem_buf(pem, strlen(pem)));
+  char *name, *header;
+  uint8_t *data;
+  long data_len;
+  if (!PEM_read_bio(bio.get(), &name, &header, &data,
+                    &data_len)) {
+    return nullptr;
+  }
+  OPENSSL_free(name);
+  OPENSSL_free(header);
+
+  auto ret = bssl::UniquePtr<CRYPTO_BUFFER>(
+      CRYPTO_BUFFER_new(data, data_len, nullptr));
+  OPENSSL_free(data);
+  return ret;
+}
+
+static bssl::UniquePtr<X509> X509FromBuffer(
+    bssl::UniquePtr<CRYPTO_BUFFER> buffer) {
+  if (!buffer) {
+    return nullptr;
+  }
+  const uint8_t *derp = CRYPTO_BUFFER_data(buffer.get());
+  return bssl::UniquePtr<X509>(
+      d2i_X509(NULL, &derp, CRYPTO_BUFFER_len(buffer.get())));
+}
+
 static bssl::UniquePtr<X509> GetTestCertificate() {
   static const char kCertPEM[] =
       "-----BEGIN CERTIFICATE-----\n"
@@ -1370,7 +1398,7 @@ static bssl::UniquePtr<SSL_CTX> CreateContextWithTestCertificate(
   return ctx;
 }
 
-static bssl::UniquePtr<X509> GetECDSATestCertificate() {
+static bssl::UniquePtr<CRYPTO_BUFFER> GetECDSATestCertificateBuffer() {
   static const char kCertPEM[] =
       "-----BEGIN CERTIFICATE-----\n"
       "MIIBzzCCAXagAwIBAgIJANlMBNpJfb/rMAkGByqGSM49BAEwRTELMAkGA1UEBhMC\n"
@@ -1384,9 +1412,14 @@ static bssl::UniquePtr<X509> GetECDSATestCertificate() {
       "BgcqhkjOPQQBA0gAMEUCIQDyoDVeUTo2w4J5m+4nUIWOcAZ0lVfSKXQA9L4Vh13E\n"
       "BwIgfB55FGohg/B6dGh5XxSZmmi08cueFV7mHzJSYV51yRQ=\n"
       "-----END CERTIFICATE-----\n";
-  return CertFromPEM(kCertPEM);
+  return BufferFromPEM(kCertPEM);
 }
 
+static bssl::UniquePtr<X509> GetECDSATestCertificate() {
+  return X509FromBuffer(GetECDSATestCertificateBuffer());
+}
+
+
 static bssl::UniquePtr<EVP_PKEY> GetECDSATestKey() {
   static const char kKeyPEM[] =
       "-----BEGIN PRIVATE KEY-----\n"
@@ -1397,24 +1430,6 @@ static bssl::UniquePtr<EVP_PKEY> GetECDSATestKey() {
   return KeyFromPEM(kKeyPEM);
 }
 
-static bssl::UniquePtr<CRYPTO_BUFFER> BufferFromPEM(const char *pem) {
-  bssl::UniquePtr<BIO> bio(BIO_new_mem_buf(pem, strlen(pem)));
-  char *name, *header;
-  uint8_t *data;
-  long data_len;
-  if (!PEM_read_bio(bio.get(), &name, &header, &data,
-                    &data_len)) {
-    return nullptr;
-  }
-  OPENSSL_free(name);
-  OPENSSL_free(header);
-
-  auto ret = bssl::UniquePtr<CRYPTO_BUFFER>(
-      CRYPTO_BUFFER_new(data, data_len, nullptr));
-  OPENSSL_free(data);
-  return ret;
-}
-
 static bssl::UniquePtr<CRYPTO_BUFFER> GetChainTestCertificateBuffer() {
   static const char kCertPEM[] =
       "-----BEGIN CERTIFICATE-----\n"
@@ -1438,16 +1453,6 @@ static bssl::UniquePtr<CRYPTO_BUFFER> GetChainTestCertificateBuffer() {
   return BufferFromPEM(kCertPEM);
 }
 
-static bssl::UniquePtr<X509> X509FromBuffer(
-    bssl::UniquePtr<CRYPTO_BUFFER> buffer) {
-  if (!buffer) {
-    return nullptr;
-  }
-  const uint8_t *derp = CRYPTO_BUFFER_data(buffer.get());
-  return bssl::UniquePtr<X509>(
-      d2i_X509(NULL, &derp, CRYPTO_BUFFER_len(buffer.get())));
-}
-
 static bssl::UniquePtr<X509> GetChainTestCertificate() {
   return X509FromBuffer(GetChainTestCertificateBuffer());
 }
@@ -4047,7 +4052,7 @@ TEST_P(SSLVersionTest, SSLClearFailsWithShedding) {
   ASSERT_FALSE(SSL_clear(server_.get()));
 }
 
-static bool ChainsEqual(STACK_OF(X509) * chain,
+static bool ChainsEqual(const STACK_OF(X509) *chain,
                         const std::vector<X509 *> &expected) {
   if (sk_X509_num(chain) != expected.size()) {
     return false;
@@ -4062,6 +4067,24 @@ static bool ChainsEqual(STACK_OF(X509) * chain,
   return true;
 }
 
+static bool BuffersEqual(const STACK_OF(CRYPTO_BUFFER) *chain,
+                         const std::vector<CRYPTO_BUFFER *> &expected) {
+  if (sk_CRYPTO_BUFFER_num(chain) != expected.size()) {
+    return false;
+  }
+
+  for (size_t i = 0; i < expected.size(); i++) {
+    const CRYPTO_BUFFER *buf = sk_CRYPTO_BUFFER_value(chain, i);
+    if (Bytes(CRYPTO_BUFFER_data(buf), CRYPTO_BUFFER_len(buf)) !=
+        Bytes(CRYPTO_BUFFER_data(expected[i]),
+              CRYPTO_BUFFER_len(expected[i]))) {
+      return false;
+    }
+  }
+
+  return true;
+}
+
 TEST_P(SSLVersionTest, AutoChain) {
   cert_ = GetChainTestCertificate();
   ASSERT_TRUE(cert_);
@@ -4630,6 +4653,133 @@ TEST(SSLTest, OverrideCertAndKey) {
   ASSERT_TRUE(SSL_CTX_use_PrivateKey(ctx.get(), key2.get()));
 }
 
+TEST(SSLTest, OverrideKeyMethodWithKey) {
+  // Make an SSL_PRIVATE_KEY_METHOD that should never be called.
+  static const SSL_PRIVATE_KEY_METHOD kErrorMethod = {
+      [](SSL *ssl, uint8_t *out, size_t *out_len, size_t max_out,
+         uint16_t signature_algorithm, const uint8_t *in,
+         size_t in_len) { return ssl_private_key_failure; },
+      [](SSL *ssl, uint8_t *out, size_t *out_len, size_t max_out,
+         const uint8_t *in, size_t in_len) { return ssl_private_key_failure; },
+      [](SSL *ssl, uint8_t *out, size_t *out_len, size_t max_oun) {
+        return ssl_private_key_failure;
+      },
+  };
+
+  bssl::UniquePtr<EVP_PKEY> key = GetTestKey();
+  ASSERT_TRUE(key);
+  bssl::UniquePtr<X509> leaf = GetTestCertificate();
+  ASSERT_TRUE(leaf);
+
+  bssl::UniquePtr<SSL_CTX> ctx(SSL_CTX_new(TLS_method()));
+  ASSERT_TRUE(ctx);
+  ASSERT_TRUE(SSL_CTX_use_certificate(ctx.get(), leaf.get()));
+
+  // Configuring an |SSL_PRIVATE_KEY_METHOD| and then overwriting it with an
+  // |EVP_PKEY| should clear the |SSL_PRIVATE_KEY_METHOD|.
+  SSL_CTX_set_private_key_method(ctx.get(), &kErrorMethod);
+  ASSERT_TRUE(SSL_CTX_use_PrivateKey(ctx.get(), key.get()));
+
+  bssl::UniquePtr<SSL> client, server;
+  ASSERT_TRUE(ConnectClientAndServer(&client, &server, ctx.get(), ctx.get()));
+}
+
+// Configuring a chain and then overwriting it with a different chain should
+// clear the old one.
+TEST(SSLTest, OverrideChain) {
+  bssl::UniquePtr<EVP_PKEY> key = GetChainTestKey();
+  ASSERT_TRUE(key);
+  bssl::UniquePtr<X509> leaf = GetChainTestCertificate();
+  ASSERT_TRUE(leaf);
+  bssl::UniquePtr<X509> ca = GetChainTestIntermediate();
+  ASSERT_TRUE(ca);
+
+  bssl::UniquePtr<STACK_OF(X509)> chain(sk_X509_new_null());
+  ASSERT_TRUE(chain);
+  ASSERT_TRUE(bssl::PushToStack(chain.get(), bssl::UpRef(ca)));
+
+  bssl::UniquePtr<STACK_OF(X509)> wrong_chain(sk_X509_new_null());
+  ASSERT_TRUE(wrong_chain);
+  ASSERT_TRUE(bssl::PushToStack(wrong_chain.get(), bssl::UpRef(leaf)));
+  ASSERT_TRUE(bssl::PushToStack(wrong_chain.get(), bssl::UpRef(leaf)));
+
+  bssl::UniquePtr<SSL_CTX> ctx(SSL_CTX_new(TLS_method()));
+  ASSERT_TRUE(ctx);
+  ASSERT_TRUE(SSL_CTX_use_certificate(ctx.get(), leaf.get()));
+  ASSERT_TRUE(SSL_CTX_use_PrivateKey(ctx.get(), key.get()));
+
+  // Configure one chain, then replace it with another. Note this API considers
+  // the chain to exclude the leaf.
+  ASSERT_TRUE(SSL_CTX_set1_chain(ctx.get(), wrong_chain.get()));
+  ASSERT_TRUE(SSL_CTX_set1_chain(ctx.get(), chain.get()));
+
+  bssl::UniquePtr<SSL> client, server;
+  ASSERT_TRUE(ConnectClientAndServer(&client, &server, ctx.get(), ctx.get()));
+  EXPECT_TRUE(ChainsEqual(SSL_get_peer_full_cert_chain(client.get()),
+                          {leaf.get(), ca.get()}));
+}
+
+TEST(SSLTest, OverrideChainAndKey) {
+  bssl::UniquePtr<EVP_PKEY> key1 = GetChainTestKey();
+  ASSERT_TRUE(key1);
+  bssl::UniquePtr<CRYPTO_BUFFER> leaf1 = GetChainTestCertificateBuffer();
+  ASSERT_TRUE(leaf1);
+  bssl::UniquePtr<CRYPTO_BUFFER> ca1 = GetChainTestIntermediateBuffer();
+  ASSERT_TRUE(ca1);
+  bssl::UniquePtr<EVP_PKEY> key2 = GetECDSATestKey();
+  ASSERT_TRUE(key2);
+  bssl::UniquePtr<CRYPTO_BUFFER> leaf2 = GetECDSATestCertificateBuffer();
+  ASSERT_TRUE(leaf2);
+
+  bssl::UniquePtr<SSL_CTX> ctx(SSL_CTX_new(TLS_method()));
+  ASSERT_TRUE(ctx);
+
+  // Configure one cert and key pair, then replace it with noather.
+  std::vector<CRYPTO_BUFFER *> certs = {leaf1.get(), ca1.get()};
+  ASSERT_TRUE(SSL_CTX_set_chain_and_key(ctx.get(), certs.data(), certs.size(),
+                                        key1.get(), nullptr));
+  certs = {leaf2.get()};
+  ASSERT_TRUE(SSL_CTX_set_chain_and_key(ctx.get(), certs.data(), certs.size(),
+                                        key2.get(), nullptr));
+
+  bssl::UniquePtr<SSL> client, server;
+  ASSERT_TRUE(ConnectClientAndServer(&client, &server, ctx.get(), ctx.get()));
+  EXPECT_TRUE(
+      BuffersEqual(SSL_get0_peer_certificates(client.get()), {leaf2.get()}));
+}
+
+TEST(SSLTest, OverrideCredentialChain) {
+  bssl::UniquePtr<EVP_PKEY> key = GetChainTestKey();
+  ASSERT_TRUE(key);
+  bssl::UniquePtr<CRYPTO_BUFFER> leaf = GetChainTestCertificateBuffer();
+  ASSERT_TRUE(leaf);
+  bssl::UniquePtr<CRYPTO_BUFFER> ca = GetChainTestIntermediateBuffer();
+  ASSERT_TRUE(ca);
+
+  std::vector<CRYPTO_BUFFER *> chain = {leaf.get(), ca.get()};
+  std::vector<CRYPTO_BUFFER *> wrong_chain = {leaf.get(), leaf.get(),
+                                              leaf.get()};
+
+  bssl::UniquePtr<SSL_CTX> ctx(SSL_CTX_new(TLS_method()));
+  ASSERT_TRUE(ctx);
+  bssl::UniquePtr<SSL_CREDENTIAL> cred(SSL_CREDENTIAL_new_x509());
+  ASSERT_TRUE(cred);
+
+  // Configure one chain (including the leaf), then replace it with another.
+  ASSERT_TRUE(SSL_CREDENTIAL_set1_cert_chain(cred.get(), wrong_chain.data(),
+                                             wrong_chain.size()));
+  ASSERT_TRUE(
+      SSL_CREDENTIAL_set1_cert_chain(cred.get(), chain.data(), chain.size()));
+
+  ASSERT_TRUE(SSL_CREDENTIAL_set1_private_key(cred.get(), key.get()));
+  ASSERT_TRUE(SSL_CTX_add1_credential(ctx.get(), cred.get()));
+
+  bssl::UniquePtr<SSL> client, server;
+  ASSERT_TRUE(ConnectClientAndServer(&client, &server, ctx.get(), ctx.get()));
+  EXPECT_TRUE(BuffersEqual(SSL_get0_peer_certificates(client.get()),
+                           {leaf.get(), ca.get()}));
+}
+
 TEST(SSLTest, SetChainAndKeyCtx) {
   bssl::UniquePtr<SSL_CTX> client_ctx(SSL_CTX_new(TLS_with_buffers_method()));
   ASSERT_TRUE(client_ctx);

src/ssl/ssl_x509.cc

@@ -198,6 +198,7 @@ static void ssl_crypto_x509_cert_flush_cached_chain(CERT *cert) {
 // which case no change to |cert->chain| is made. It preverses the existing
 // leaf from |cert->chain|, if any.
 static bool ssl_cert_set1_chain(CERT *cert, STACK_OF(X509) *chain) {
+  cert->default_credential->ClearIntermediateCerts();
   for (X509 *x509 : chain) {
     UniquePtr<CRYPTO_BUFFER> buffer = x509_to_buffer(x509);
     if (!buffer ||