Skip to content

Implement host override of CN checking in the WinHTTP backend #824

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
Aug 10, 2018
Merged

Implement host override of CN checking in the WinHTTP backend #824

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
b31be1d
Implement CN overrides via the Host: header in the WinHTTP backend.
BillyONeal Aug 2, 2018
4c36fbd
Repair lack of fallback CN verification on Windows.
BillyONeal Aug 3, 2018
086c5e8
Set security settings even for non-HTTPS URIs in WinHTTP, in case we …
BillyONeal Aug 3, 2018
fac88ab
Add test to make sure this doesn't explode.
BillyONeal Aug 3, 2018
58ea59d
Repair CPPREST_EXCLUDE_WEBSOCKETS support.
BillyONeal Aug 9, 2018
0fe2140
Add m_ to customCnCheck.
BillyONeal Aug 9, 2018
05eec2d
Merge remote-tracking branch 'upstream/master' into cn_checking
BillyONeal Aug 9, 2018
a73795f
Move x509_cert_utilities.h out of public headers.
BillyONeal Aug 10, 2018
7dc518e
Add m_ to cachedEncodedCert.
BillyONeal Aug 10, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 47 additions & 4 deletions Release/include/cpprest/details/x509_cert_utilities.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,52 @@

#pragma once

#include <string>
#if defined(_WIN32)
#include <Wincrypt.h>

namespace web { namespace http { namespace client { namespace details {

struct winhttp_cert_context
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's avoid adding this to public headers (especially since it'll be dragging in Wincrypt.h).

Instead, can we add it to either src/http/client/http_client_impl.h or create a new src/http/client/x509_cert_utilities_impl.h?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alternatively: move to src/http/client/

{
PCCERT_CONTEXT raw;
winhttp_cert_context() noexcept : raw(nullptr) {}
winhttp_cert_context(const winhttp_cert_context&) = delete;
winhttp_cert_context& operator=(const winhttp_cert_context&) = delete;
~winhttp_cert_context()
{
// https://docs.microsoft.com/en-us/windows/desktop/api/wincrypt/nf-wincrypt-certfreecertificatecontext
// "The function always returns nonzero."
if (raw)
{
(void)CertFreeCertificateContext(raw);
}
}
};

struct winhttp_cert_chain_context
{
PCCERT_CHAIN_CONTEXT raw;
winhttp_cert_chain_context() noexcept : raw(nullptr) {}
winhttp_cert_chain_context(const winhttp_cert_chain_context&) = delete;
winhttp_cert_chain_context& operator=(const winhttp_cert_chain_context&) = delete;
~winhttp_cert_chain_context()
{
if (raw)
{
CertFreeCertificateChain(raw);
}
}
};

#if defined(__APPLE__) || (defined(ANDROID) || defined(__ANDROID__)) || (defined(_WIN32) && !defined(__cplusplus_winrt) && !defined(_M_ARM) && !defined(CPPREST_EXCLUDE_WEBSOCKETS))
}}}} // namespaces
#endif // _WIN32

#if defined(__APPLE__) || (defined(ANDROID) || defined(__ANDROID__)) || (defined(_WIN32) && !defined(__cplusplus_winrt) && !defined(_M_ARM) && !defined(CPPREST_EXCLUDE_WEBSOCKETS))
#define CPPREST_PLATFORM_ASIO_CERT_VERIFICATION_AVAILABLE
#endif

#ifdef CPPREST_PLATFORM_ASIO_CERT_VERIFICATION_AVAILABLE
#include <string>

#if defined(_MSC_VER)
#pragma warning(push)
Expand All @@ -37,7 +80,7 @@ namespace web { namespace http { namespace client { namespace details {

/// <summary>
/// Using platform specific APIs verifies server certificate.
/// Currently implemented to work on iOS, Android, and OS X.
/// Currently implemented to work on Windows, iOS, Android, and OS X.
/// </summary>
/// <param name="verifyCtx">Boost.ASIO context to get certificate chain from.</param>
/// <param name="hostName">Host name from the URI.</param>
Expand All @@ -46,4 +89,4 @@ bool verify_cert_chain_platform_specific(boost::asio::ssl::verify_context &verif

}}}}

#endif
#endif // CPPREST_PLATFORM_ASIO_CERT_VERIFICATION_AVAILABLE
14 changes: 7 additions & 7 deletions Release/src/http/client/http_client_asio.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -479,7 +479,7 @@ class asio_context final : public request_context, public std::enable_shared_fro
, m_needChunked(false)
, m_timer(client->client_config().timeout<std::chrono::microseconds>())
, m_connection(connection)
#if defined(__APPLE__) || (defined(ANDROID) || defined(__ANDROID__))
#ifdef CPPREST_PLATFORM_ASIO_CERT_VERIFICATION_AVAILABLE
, m_openssl_failed(false)
#endif
{}
Expand Down Expand Up @@ -989,11 +989,11 @@ class asio_context final : public request_context, public std::enable_shared_fro
// finally by the root CA self signed certificate.

const auto &host = utility::conversions::to_utf8string(m_http_client->base_uri().host());
#if defined(__APPLE__) || (defined(ANDROID) || defined(__ANDROID__))
// On OS X, iOS, and Android, OpenSSL doesn't have access to where the OS
// stores keychains. If OpenSSL fails we will doing verification at the
// end using the whole certificate chain so wait until the 'leaf' cert.
// For now return true so OpenSSL continues down the certificate chain.
#ifdef CPPREST_PLATFORM_ASIO_CERT_VERIFICATION_AVAILABLE
// Attempt to use platform certificate validation when it is available:
// If OpenSSL fails we will doing verification at the end using the whole certificate chain,
// so wait until the 'leaf' cert. For now return true so OpenSSL continues down the certificate
// chain.
if(!preverified)
{
m_openssl_failed = true;
Expand Down Expand Up @@ -1669,7 +1669,7 @@ class asio_context final : public request_context, public std::enable_shared_fro
boost::asio::streambuf m_body_buf;
std::shared_ptr<asio_connection> m_connection;

#if defined(__APPLE__) || (defined(ANDROID) || defined(__ANDROID__))
#ifdef CPPREST_PLATFORM_ASIO_CERT_VERIFICATION_AVAILABLE
bool m_openssl_failed;
#endif
};
Expand Down
Loading