-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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.
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
Add support for cert-pinning on Windows and Mac. #702
Changes from 1 commit
917ee0e
fd17845
f875121
9aa9ae5
39c26e0
79c74d2
5d5dca0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -35,6 +35,7 @@ Gery Vessere ([email protected]) | |
Cisco Systems | ||
Gergely Lukacsy (glukacsy) | ||
Chris Deering (deeringc) | ||
Chris O'Gorman (chogorma) | ||
|
||
Ocedo GmbH | ||
Henning Pfeiffer (megaposer) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,48 @@ | ||
/*** | ||
* ==++== | ||
* | ||
* Copyright (c) Microsoft Corporation. All rights reserved. | ||
* Licensed under the Apache License, Version 2.0 (the "License"); | ||
* you may not use this file except in compliance with the License. | ||
* You may obtain a copy of the License at | ||
* http://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, software | ||
* distributed under the License is distributed on an "AS IS" BASIS, | ||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
* See the License for the specific language governing permissions and | ||
* limitations under the License. | ||
* | ||
* ==--== | ||
* =+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+ | ||
* | ||
* Certificate info | ||
* | ||
* For the latest on this and related APIs, please see: https://github.com/Microsoft/cpprestsdk | ||
* | ||
* =-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=- | ||
****/ | ||
|
||
#pragma once | ||
|
||
#include <vector> | ||
#include <string> | ||
|
||
namespace web { namespace http { namespace client { | ||
|
||
using CertificateChain = std::vector<std::vector<unsigned char>>; | ||
|
||
struct certificate_info | ||
{ | ||
CertificateChain certificate_chain; | ||
std::string host_name; | ||
long certificate_error{ 0 }; | ||
bool verified{ false }; | ||
|
||
certificate_info(const std::string host) : host_name(host) {}; | ||
certificate_info(const std::string host, CertificateChain chain, long error = 0) : host_name(host), certificate_chain(chain), certificate_error(error) {}; | ||
}; | ||
|
||
using CertificateChainFunction = std::function<bool(const std::shared_ptr<certificate_info> certificate_Info)>; | ||
|
||
}}} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,4 @@ | ||
|
||
/*** | ||
* Copyright (C) Microsoft. All rights reserved. | ||
* Licensed under the MIT license. See LICENSE.txt file in the project root for full license information. | ||
|
@@ -44,6 +45,7 @@ | |
#include "cpprest/base_uri.h" | ||
#include "cpprest/details/x509_cert_utilities.h" | ||
#include "cpprest/details/http_helpers.h" | ||
#include "cpprest/certificate_info.h" | ||
#include <unordered_set> | ||
#include <memory> | ||
|
||
|
@@ -975,6 +977,9 @@ class asio_context : public request_context, public std::enable_shared_from_this | |
// finally by the root CA self signed certificate. | ||
|
||
const auto &host = utility::conversions::to_utf8string(m_http_client->base_uri().host()); | ||
|
||
using namespace web::http::client::details; | ||
|
||
#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 | ||
|
@@ -986,12 +991,31 @@ class asio_context : public request_context, public std::enable_shared_from_this | |
} | ||
if(m_openssl_failed) | ||
{ | ||
return verify_cert_chain_platform_specific(verifyCtx, host); | ||
} | ||
|
||
if (!is_end_certificate_in_chain(verifyCtx)) | ||
{ | ||
// Continue until we get the end certificate. | ||
return true; | ||
} | ||
|
||
auto chainFunc = [this](const std::shared_ptr<certificate_info>& cert_info) { | ||
return m_http_client->client_config().invoke_certificate_chain_callback(cert_info); | ||
}; | ||
|
||
return http::client::details::verify_cert_chain_platform_specific(verifyCtx, utility::conversions::to_utf8string(host), chainFunc); | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. code cosmetics: indentation of closing bracket |
||
#endif | ||
|
||
boost::asio::ssl::rfc2818_verification rfc2818(host); | ||
return rfc2818(preverified, verifyCtx); | ||
if(!rfc2818(preverified, verifyCtx)) | ||
{ | ||
return false; | ||
} | ||
|
||
auto info = std::make_shared<certificate_info>(host, get_X509_cert_chain_encoded_data(verifyCtx)); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
info->verified = true; | ||
|
||
return m_http_client->client_config().invoke_certificate_chain_callback(info); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. On Linux this is called several times. I'm not exactly sure why, but I suppose it is due to the way OpenSSL handles certificate chains (cf. comments in this method). Do we need a similar workaround as for macOS and Android above? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, you would need to do the same flow on Linux as per the macOS flow above. The "is_end_certificate_in_chain" check above should allow the connection until we get the end certificate, once we get that, then we get the OS to build the full cert chain, only after that, then we do the cert-pinning. If you just wait for the end certificate and don't build the full chain from the OS, you will only have the certs from the SSL handshake, and more than likely that will not be the full chain. |
||
} | ||
|
||
void handle_write_headers(const boost::system::error_code& ec) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How do I interpret this error code? I suppose it's platform dependent?
Please add some description in the comments.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, the error code is platform dependent, we use it for debugging certificate issues.
Mac: https://developer.apple.com/documentation/security/sectrustresulttype
Windows: https://msdn.microsoft.com/en-us/library/windows/desktop/aa377590(v=vs.85).aspx
Will add a comment for that.