zqz/platform-external-webrtc
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Fixed Hostname Validation in OpenSSLAdapter.

Browse Source 
This changeset addresses concerns about how the OpenSSLAdapter does certificate
name matching. The current approach has a number of issues which are outlined
in the bug description. The approach taken in this changeset is to use the
standard function X509_check_host which should correctly parse the wildcard
expansions and is directly supported in OpenSSL instead of attempting my own
implementation. This changeset uses this as an opportunity to add additional
parameter checking and refactoring logging code out of the main code path.

Bug: webrtc:8888
Change-Id: Iaffe1daddcd52193ba674489f613ce8515b81e91
Reviewed-on: https://webrtc-review.googlesource.com/65022
Commit-Queue: Benjamin Wright <benwright@webrtc.org>
Reviewed-by: Taylor Brandstetter <deadbeef@webrtc.org>
Reviewed-by: Emad Omara <emadomara@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#22755}
This commit is contained in:
Benjamin Wright
2018-04-05 12:12:26 -07:00
committed by Commit Bot
parent 5150ee40f4
commit 9201d1aa8a
6 changed files with 293 additions and 93 deletions
Download Patch File Download Diff File Expand all files Collapse all files

3
rtc_base/BUILD.gn
View File

@ -855,6 +855,8 @@ rtc_static_library("rtc_base_generic") {
"openssl.h",
"openssladapter.cc",
"openssladapter.h",
"opensslcommon.cc",
"opensslcommon.h",
"openssldigest.cc",
"openssldigest.h",
"opensslidentity.cc",
@ -1356,6 +1358,7 @@ if (rtc_include_tests) {
if (is_posix || is_fuchsia) {
sources += [
"openssladapter_unittest.cc",
"opensslcommon_unittest.cc",
"ssladapter_unittest.cc",
"sslidentity_unittest.cc",
"sslstreamadapter_unittest.cc",

101
rtc_base/openssladapter.cc
View File

@ -32,6 +32,7 @@
#include "rtc_base/logging.h"
#include "rtc_base/numerics/safe_conversions.h"
#include "rtc_base/openssl.h"
#include "rtc_base/opensslcommon.h"
#include "rtc_base/sslroots.h"
#include "rtc_base/stringencode.h"
#include "rtc_base/stringutils.h"
@ -414,7 +415,7 @@ int OpenSSLAdapter::ContinueSSL() {
int code = (role_ == SSL_CLIENT) ? SSL_connect(ssl_) : SSL_accept(ssl_);
switch (SSL_get_error(ssl_, code)) {
case SSL_ERROR_NONE:
if (!SSLPostConnectionCheck(ssl_, ssl_host_name_.c_str())) {
if (!SSLPostConnectionCheck(ssl_, ssl_host_name_)) {
RTC_LOG(LS_ERROR) << "TLS post connection check failed";
// make sure we close the socket
Cleanup();
@ -790,96 +791,18 @@ void OpenSSLAdapter::OnCloseEvent(AsyncSocket* socket, int err) {
AsyncSocketAdapter::OnCloseEvent(socket, err);
}
bool OpenSSLAdapter::VerifyServerName(SSL* ssl, const char* host,
bool ignore_bad_cert) {
if (!host)
return false;
bool OpenSSLAdapter::SSLPostConnectionCheck(SSL* ssl, const std::string& host) {
bool is_valid_cert_name = openssl::VerifyPeerCertMatchesHost(ssl, host) &&
(SSL_get_verify_result(ssl) == X509_V_OK ||
custom_verification_succeeded_);
// Checking the return from SSL_get_peer_certificate here is not strictly
// necessary. With our setup, it is not possible for it to return
// null. However, it is good form to check the return.
X509* certificate = SSL_get_peer_certificate(ssl);
if (!certificate)
return false;
// Logging certificates is extremely verbose. So it is disabled by default.
#ifdef LOG_CERTIFICATES
{
RTC_DLOG(LS_INFO) << "Certificate from server:";
BIO* mem = BIO_new(BIO_s_mem());
X509_print_ex(mem, certificate, XN_FLAG_SEP_CPLUS_SPC, X509_FLAG_NO_HEADER);
BIO_write(mem, "\0", 1);
char* buffer;
BIO_get_mem_data(mem, &buffer);
RTC_DLOG(LS_INFO) << buffer;
BIO_free(mem);
char* cipher_description =
SSL_CIPHER_description(SSL_get_current_cipher(ssl), nullptr, 128);
RTC_DLOG(LS_INFO) << "Cipher: " << cipher_description;
OPENSSL_free(cipher_description);
if (!is_valid_cert_name && ignore_bad_cert_) {
RTC_DLOG(LS_WARNING) << "Other TLS post connection checks failed."
"ignore_bad_cert_ set to true. Overriding name "
"verification failure!";
is_valid_cert_name = true;
}
#endif
bool ok = false;
GENERAL_NAMES* names = reinterpret_cast<GENERAL_NAMES*>(
X509_get_ext_d2i(certificate, NID_subject_alt_name, nullptr, nullptr));
if (names) {
for (size_t i = 0; i < static_cast<size_t>(sk_GENERAL_NAME_num(names));
i++) {
const GENERAL_NAME* name = sk_GENERAL_NAME_value(names, i);
if (name->type != GEN_DNS)
continue;
std::string value(
reinterpret_cast<const char*>(ASN1_STRING_data(name->d.dNSName)),
ASN1_STRING_length(name->d.dNSName));
// string_match takes NUL-terminated strings, so check for embedded NULs.
if (value.find('\0') != std::string::npos)
continue;
if (string_match(host, value.c_str())) {
ok = true;
break;
}
}
GENERAL_NAMES_free(names);
}
char data[256];
X509_NAME* subject;
if (!ok && ((subject = X509_get_subject_name(certificate)) != nullptr) &&
(X509_NAME_get_text_by_NID(subject, NID_commonName, data, sizeof(data)) >
0)) {
data[sizeof(data)-1] = 0;
if (_stricmp(data, host) == 0)
ok = true;
}
X509_free(certificate);
// This should only ever be turned on for debugging and development.
if (!ok && ignore_bad_cert) {
RTC_DLOG(LS_WARNING) << "TLS certificate check FAILED. "
<< "Allowing connection anyway.";
ok = true;
}
return ok;
}
bool OpenSSLAdapter::SSLPostConnectionCheck(SSL* ssl, const char* host) {
bool ok = VerifyServerName(ssl, host, ignore_bad_cert_);
if (ok) {
ok = (SSL_get_verify_result(ssl) == X509_V_OK ||
custom_verification_succeeded_);
}
if (!ok && ignore_bad_cert_) {
RTC_DLOG(LS_INFO) << "Other TLS post connection checks failed.";
ok = true;
}
return ok;
return is_valid_cert_name;
}
#if !defined(NDEBUG)

6
rtc_base/openssladapter.h
View File

@ -91,9 +91,8 @@ class OpenSSLAdapter : public SSLAdapter, public MessageHandler {
void OnMessage(Message* msg) override;
static bool VerifyServerName(SSL* ssl, const char* host,
bool ignore_bad_cert);
bool SSLPostConnectionCheck(SSL* ssl, const char* host);
bool SSLPostConnectionCheck(SSL* ssl, const std::string& host);
#if !defined(NDEBUG)
// In debug builds, logs info about the state of the SSL connection.
static void SSLInfoCallback(const SSL* ssl, int where, int ret);
@ -152,7 +151,6 @@ class OpenSSLAdapterFactory : public SSLAdapterFactory {
void SetMode(SSLMode mode) override;
OpenSSLAdapter* CreateAdapter(AsyncSocket* socket) override;
static OpenSSLAdapterFactory* Create();
private:

95
rtc_base/opensslcommon.cc Normal file
View File

@ -0,0 +1,95 @@
/*
* Copyright 2018 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.
*/
#include "rtc_base/opensslcommon.h"
#if defined(WEBRTC_POSIX)
#include <unistd.h>
#endif
#if defined(WEBRTC_WIN)
// Must be included first before openssl headers.
#include "rtc_base/win32.h" // NOLINT
#endif // WEBRTC_WIN
#include <openssl/bio.h>
#include <openssl/crypto.h>
#include <openssl/x509.h>
#include <openssl/x509v3.h>
#include "rtc_base/checks.h"
#include "rtc_base/logging.h"
#include "rtc_base/openssl.h"
namespace rtc {
namespace openssl {
// Holds various helper methods.
namespace {
void LogCertificates(SSL* ssl, X509* certificate) {
// Logging certificates is extremely verbose. So it is disabled by default.
#ifdef LOG_CERTIFICATES
BIO* mem = BIO_new(BIO_s_mem());
if (mem == nullptr) {
RTC_DLOG(LS_ERROR) << "BIO_new() failed to allocate memory.";
return;
}
RTC_DLOG(LS_INFO) << "Certificate from server:";
X509_print_ex(mem, certificate, XN_FLAG_SEP_CPLUS_SPC, X509_FLAG_NO_HEADER);
BIO_write(mem, "\0", 1);
char* buffer = nullptr;
BIO_get_mem_data(mem, &buffer);
if (buffer != nullptr) {
RTC_DLOG(LS_INFO) << buffer;
} else {
RTC_DLOG(LS_ERROR) << "BIO_get_mem_data() failed to get buffer.";
}
BIO_free(mem);
const char* cipher_name = SSL_CIPHER_get_name(SSL_get_current_cipher(ssl));
if (cipher_name != nullptr) {
RTC_DLOG(LS_INFO) << "Cipher: " << cipher_name;
} else {
RTC_DLOG(LS_ERROR) << "SSL_CIPHER_DESCRIPTION() failed to get cipher_name.";
}
#endif
}
} // namespace
bool VerifyPeerCertMatchesHost(SSL* ssl, const std::string& host) {
if (host.empty()) {
RTC_DLOG(LS_ERROR) << "Hostname is empty. Cannot verify peer certificate.";
return false;
}
if (ssl == nullptr) {
RTC_DLOG(LS_ERROR) << "SSL is nullptr. Cannot verify peer certificate.";
return false;
}
X509* certificate = SSL_get_peer_certificate(ssl);
if (certificate == nullptr) {
RTC_DLOG(LS_ERROR)
<< "SSL_get_peer_certificate failed. This should never happen.";
return false;
}
LogCertificates(ssl, certificate);
bool is_valid_cert_name =
X509_check_host(certificate, host.c_str(), host.size(), 0, nullptr) == 1;
X509_free(certificate);
return is_valid_cert_name;
}
} // namespace openssl
} // namespace rtc

29
rtc_base/opensslcommon.h Normal file
View File

@ -0,0 +1,29 @@
/*
* Copyright 2018 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.
*/
#ifndef RTC_BASE_OPENSSLCOMMON_H_
#define RTC_BASE_OPENSSLCOMMON_H_
#include <string>
typedef struct ssl_st SSL;
namespace rtc {
// The openssl namespace holds static helper methods. All methods related
// to OpenSSL that are commonly used and don't require global state should be
// placed here.
namespace openssl {
// Verifies that the hostname provided matches that in the peer certificate
// attached to this SSL state.
bool VerifyPeerCertMatchesHost(SSL* ssl, const std::string& host);
} // namespace openssl
} // namespace rtc
#endif // RTC_BASE_OPENSSLCOMMON_H_

152
rtc_base/opensslcommon_unittest.cc Normal file
View File

@ -0,0 +1,152 @@
/*
* Copyright 2018 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.
*/
#include <string>
#include <vector>
#if defined(WEBRTC_POSIX)
#include <unistd.h>
#endif
#if defined(WEBRTC_WIN)
// Must be included first before openssl headers.
#include "rtc_base/win32.h" // NOLINT
#endif // WEBRTC_WIN
#include <openssl/bio.h>
#include <openssl/crypto.h>
#include <openssl/x509.h>
#include <openssl/x509v3.h>
#include "rtc_base/arraysize.h"
#include "rtc_base/gunit.h"
#include "rtc_base/numerics/safe_conversions.h"
#include "rtc_base/openssl.h"
#include "rtc_base/opensslcommon.h"
#include "rtc_base/sslroots.h"
#include "test/gmock.h"
namespace rtc {
namespace {
// Fake Self-Signed SSL Certifiacte with CN: *.webrtc.org.
// This is only to be used for testing (it isn't signed by a CA anyway).
const unsigned char kFakeSSLCertificate[] = {
0x30, 0x82, 0x02, 0x68, 0x30, 0x82, 0x02, 0x12, 0xA0, 0x03, 0x02, 0x01,
0x02, 0x02, 0x09, 0x00, 0xC8, 0x83, 0x59, 0x4D, 0x90, 0xC3, 0x5F, 0xC8,
0x30, 0x0D, 0x06, 0x09, 0x2A, 0x86, 0x48, 0x86, 0xF7, 0x0D, 0x01, 0x01,
0x0B, 0x05, 0x00, 0x30, 0x81, 0x8D, 0x31, 0x0B, 0x30, 0x09, 0x06, 0x03,
0x55, 0x04, 0x06, 0x13, 0x02, 0x55, 0x53, 0x31, 0x0B, 0x30, 0x09, 0x06,
0x03, 0x55, 0x04, 0x08, 0x0C, 0x02, 0x57, 0x41, 0x31, 0x2C, 0x30, 0x2A,
0x06, 0x03, 0x55, 0x04, 0x0A, 0x0C, 0x23, 0x46, 0x61, 0x6B, 0x65, 0x20,
0x57, 0x65, 0x62, 0x52, 0x54, 0x43, 0x20, 0x43, 0x65, 0x72, 0x74, 0x69,
0x66, 0x69, 0x63, 0x61, 0x74, 0x65, 0x20, 0x46, 0x6F, 0x72, 0x20, 0x54,
0x65, 0x73, 0x74, 0x69, 0x6E, 0x67, 0x31, 0x2C, 0x30, 0x2A, 0x06, 0x03,
0x55, 0x04, 0x0B, 0x0C, 0x23, 0x46, 0x61, 0x6B, 0x65, 0x20, 0x57, 0x65,
0x62, 0x52, 0x54, 0x43, 0x20, 0x43, 0x65, 0x72, 0x74, 0x69, 0x66, 0x69,
0x63, 0x61, 0x74, 0x65, 0x20, 0x46, 0x6F, 0x72, 0x20, 0x54, 0x65, 0x73,
0x74, 0x69, 0x6E, 0x67, 0x31, 0x15, 0x30, 0x13, 0x06, 0x03, 0x55, 0x04,
0x03, 0x0C, 0x0C, 0x2A, 0x2E, 0x77, 0x65, 0x62, 0x72, 0x74, 0x63, 0x2E,
0x6F, 0x72, 0x67, 0x30, 0x1E, 0x17, 0x0D, 0x31, 0x38, 0x30, 0x34, 0x30,
0x33, 0x32, 0x31, 0x35, 0x34, 0x30, 0x38, 0x5A, 0x17, 0x0D, 0x31, 0x39,
0x30, 0x34, 0x30, 0x33, 0x32, 0x31, 0x35, 0x34, 0x30, 0x38, 0x5A, 0x30,
0x81, 0x8D, 0x31, 0x0B, 0x30, 0x09, 0x06, 0x03, 0x55, 0x04, 0x06, 0x13,
0x02, 0x55, 0x53, 0x31, 0x0B, 0x30, 0x09, 0x06, 0x03, 0x55, 0x04, 0x08,
0x0C, 0x02, 0x57, 0x41, 0x31, 0x2C, 0x30, 0x2A, 0x06, 0x03, 0x55, 0x04,
0x0A, 0x0C, 0x23, 0x46, 0x61, 0x6B, 0x65, 0x20, 0x57, 0x65, 0x62, 0x52,
0x54, 0x43, 0x20, 0x43, 0x65, 0x72, 0x74, 0x69, 0x66, 0x69, 0x63, 0x61,
0x74, 0x65, 0x20, 0x46, 0x6F, 0x72, 0x20, 0x54, 0x65, 0x73, 0x74, 0x69,
0x6E, 0x67, 0x31, 0x2C, 0x30, 0x2A, 0x06, 0x03, 0x55, 0x04, 0x0B, 0x0C,
0x23, 0x46, 0x61, 0x6B, 0x65, 0x20, 0x57, 0x65, 0x62, 0x52, 0x54, 0x43,
0x20, 0x43, 0x65, 0x72, 0x74, 0x69, 0x66, 0x69, 0x63, 0x61, 0x74, 0x65,
0x20, 0x46, 0x6F, 0x72, 0x20, 0x54, 0x65, 0x73, 0x74, 0x69, 0x6E, 0x67,
0x31, 0x15, 0x30, 0x13, 0x06, 0x03, 0x55, 0x04, 0x03, 0x0C, 0x0C, 0x2A,
0x2E, 0x77, 0x65, 0x62, 0x72, 0x74, 0x63, 0x2E, 0x6F, 0x72, 0x67, 0x30,
0x5C, 0x30, 0x0D, 0x06, 0x09, 0x2A, 0x86, 0x48, 0x86, 0xF7, 0x0D, 0x01,
0x01, 0x01, 0x05, 0x00, 0x03, 0x4B, 0x00, 0x30, 0x48, 0x02, 0x41, 0x00,
0xAE, 0xAE, 0x85, 0x2A, 0x40, 0xD6, 0x99, 0x35, 0x09, 0x34, 0x1B, 0xC5,
0xAC, 0x6C, 0x79, 0xC7, 0xC3, 0xDE, 0x1B, 0xCF, 0x17, 0x8D, 0x6B, 0x84,
0xEC, 0x8B, 0x4E, 0x2B, 0xC1, 0x83, 0x43, 0xDF, 0x76, 0x0F, 0x5F, 0x5A,
0xA9, 0x7D, 0x94, 0xC0, 0x54, 0x5C, 0xFF, 0xBC, 0x7C, 0x86, 0xDC, 0x9A,
0xCE, 0xB9, 0xDF, 0xE6, 0x0B, 0xC4, 0x5B, 0x6E, 0x56, 0x9F, 0xBC, 0x40,
0xF5, 0xA0, 0x52, 0xA7, 0x02, 0x03, 0x01, 0x00, 0x01, 0xA3, 0x53, 0x30,
0x51, 0x30, 0x1D, 0x06, 0x03, 0x55, 0x1D, 0x0E, 0x04, 0x16, 0x04, 0x14,
0xB7, 0xC0, 0x9A, 0xA7, 0x22, 0xAF, 0xF8, 0x7D, 0xFF, 0x68, 0xDB, 0x80,
0xAC, 0x0A, 0xB6, 0xDC, 0x64, 0x89, 0xDB, 0xD4, 0x30, 0x1F, 0x06, 0x03,
0x55, 0x1D, 0x23, 0x04, 0x18, 0x30, 0x16, 0x80, 0x14, 0xB7, 0xC0, 0x9A,
0xA7, 0x22, 0xAF, 0xF8, 0x7D, 0xFF, 0x68, 0xDB, 0x80, 0xAC, 0x0A, 0xB6,
0xDC, 0x64, 0x89, 0xDB, 0xD4, 0x30, 0x0F, 0x06, 0x03, 0x55, 0x1D, 0x13,
0x01, 0x01, 0xFF, 0x04, 0x05, 0x30, 0x03, 0x01, 0x01, 0xFF, 0x30, 0x0D,
0x06, 0x09, 0x2A, 0x86, 0x48, 0x86, 0xF7, 0x0D, 0x01, 0x01, 0x0B, 0x05,
0x00, 0x03, 0x41, 0x00, 0x50, 0x6D, 0xCC, 0x62, 0xAE, 0xD1, 0x7C, 0x4D,
0xEF, 0x90, 0x1E, 0x9B, 0x72, 0x73, 0xE0, 0x56, 0x66, 0x32, 0x6A, 0x78,
0xE8, 0x0F, 0xAD, 0x21, 0x32, 0x54, 0xA5, 0xB3, 0xB8, 0x14, 0x54, 0xBC,
0x50, 0xF7, 0x7F, 0x73, 0xD6, 0x44, 0x1E, 0x82, 0xD9, 0x4B, 0x49, 0x48,
0x9E, 0x02, 0x8B, 0xFE, 0xC3, 0xFD, 0x5D, 0x15, 0x02, 0xE1, 0x78, 0xAC,
0x9A, 0xAE, 0xFC, 0xC7, 0x48, 0xC6, 0x48, 0x6B};
// Simple testing helper method to create a fake SSL_SESSION with a testing
// peer connection set.
SSL_SESSION* CreateSSLSessionWithFakePeerCertificate(SSL_CTX* ssl_ctx) {
SSL_SESSION* ssl_session = SSL_SESSION_new(ssl_ctx);
const unsigned char* cert_buffer = kFakeSSLCertificate;
size_t cert_buffer_len = arraysize(kFakeSSLCertificate);
X509* ssl_peer_certificate = d2i_X509(
nullptr, &cert_buffer, checked_cast<long>(cert_buffer_len)); // NOLINT
EXPECT_NE(ssl_peer_certificate, nullptr);
#ifdef OPENSSL_IS_BORINGSSL
ssl_session->x509_peer = ssl_peer_certificate;
#else
ssl_session->peer = ssl_peer_certificate;
#endif
return ssl_session;
}
} // namespace
TEST(OpenSSLCommonTest, VerifyPeerCertMatchesHostFailsOnNoPeerCertificate) {
SSL_CTX* ssl_ctx = SSL_CTX_new(DTLSv1_2_client_method());
SSL* ssl = SSL_new(ssl_ctx);
EXPECT_FALSE(openssl::VerifyPeerCertMatchesHost(ssl, "webrtc.org"));
SSL_free(ssl);
SSL_CTX_free(ssl_ctx);
}
TEST(OpenSSLCommonTest, VerifyPeerCertMatchesHostSucceedsOnCorrectHostname) {
SSL_CTX* ssl_ctx = SSL_CTX_new(DTLSv1_2_client_method());
SSL* ssl = SSL_new(ssl_ctx);
SSL_SESSION* ssl_session = CreateSSLSessionWithFakePeerCertificate(ssl_ctx);
SSL_set_session(ssl, ssl_session);
EXPECT_TRUE(openssl::VerifyPeerCertMatchesHost(ssl, "www.webrtc.org"));
EXPECT_TRUE(openssl::VerifyPeerCertMatchesHost(ssl, "alice.webrtc.org"));
EXPECT_TRUE(openssl::VerifyPeerCertMatchesHost(ssl, "bob.webrtc.org"));
SSL_SESSION_free(ssl_session);
SSL_free(ssl);
SSL_CTX_free(ssl_ctx);
}
TEST(OpenSSLCommonTest, VerifyPeerCertMatchesHostFailsOnInvalidHostname) {
SSL_CTX* ssl_ctx = SSL_CTX_new(DTLSv1_2_client_method());
SSL* ssl = SSL_new(ssl_ctx);
SSL_SESSION* ssl_session = CreateSSLSessionWithFakePeerCertificate(ssl_ctx);
SSL_set_session(ssl, ssl_session);
EXPECT_FALSE(openssl::VerifyPeerCertMatchesHost(ssl, "a.b.webrtc.org"));
EXPECT_FALSE(openssl::VerifyPeerCertMatchesHost(ssl, "notwebrtc.org"));
EXPECT_FALSE(openssl::VerifyPeerCertMatchesHost(ssl, "webrtc.org"));
SSL_SESSION_free(ssl_session);
SSL_free(ssl);
SSL_CTX_free(ssl_ctx);
}
} // namespace rtc