Skip to content
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

[PERF FW] Insecure cert libcurl #3434

Merged
merged 15 commits into from
Mar 21, 2022
2 changes: 2 additions & 0 deletions sdk/core/azure-core/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@
## 1.5.0-beta.1 (Unreleased)

### Features Added

- When a `RequestFailedException` exception is thrown, the `what()` method now includes information about the HTTP request which failed.
- Adding option `WinHttpTransportOptions.IgnoreUnknownServerCert`. It can be used to disable verifying server certificate for the `WinHttpTransport`.

### Other Changes

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,11 @@ namespace Azure { namespace Core { namespace Http {
*/
struct WinHttpTransportOptions final
{
// Empty struct reserved for future options.
/**
* @brief When `true`, allows an invalid certificate authority. If this flag is set, the
* application does not receive a WINHTTP_CALLBACK_STATUS_FLAG_INVALID_CA callback.
*/
bool IgnoreUnknownServerCert = false;
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we avoid abbreviations and use IgnoreUnknownCertificateAuthority?

nit: What should the reader infer from server here in the name?

Copy link
Member

Choose a reason for hiding this comment

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

In a TLS 1.2 connection, there are two possible certificates: The server certificate and the client certificate. The server validates the client certificate, and the client validates the server certificate. This variable controls the validation of the server certificate, so it makes sense to me that the variable name should probably include "Server", since there is some level of ambiguity present.

Having said that, the variable doesn't control if it ignores an unknown server certificate, instead as you mentioned, the flag controls if an unknown CA should be allowed, which is somewhat different (a server certificate could be invalid if (for instance) the subject didn't match the IP address of the server - this flag doesn't disable that check).

};

/**
Expand Down
10 changes: 10 additions & 0 deletions sdk/core/azure-core/src/http/winhttp/win_http_transport.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -347,6 +347,16 @@ void WinHttpTransport::CreateRequestHandle(std::unique_ptr<_detail::HandleManage
GetErrorAndThrow("Error while setting client cert context to ignore..");
}
}

if (m_options.IgnoreUnknownServerCert)
{
auto option = SECURITY_FLAG_IGNORE_UNKNOWN_CA;
if (!WinHttpSetOption(
handleManager->m_requestHandle, WINHTTP_OPTION_SECURITY_FLAGS, &option, sizeof(option)))
{
GetErrorAndThrow("Error while setting ignore unknown server certificate..");
}
}
}

// For PUT/POST requests, send additional data using WinHttpWriteData.
Expand Down
43 changes: 27 additions & 16 deletions sdk/core/perf/inc/azure/perf/base_test.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,31 @@ namespace Azure { namespace Perf {
std::string m_recordId;
std::string m_proxy;
bool m_isPlayBackMode = false;
bool m_isInsecureEnabled = false;

/**
* @brief Updates the performance test to use a test-proxy for running.
*
* @note A tes-proxy is not a general proxy in the middle of the test and a server. This is an
* SDK specific tool https://github.com/Azure/azure-sdk-tools/tree/main/tools/test-proxy that
* provides record and playback features to a performance test. Do not use a general purpose
* proxy for the test.
*
* @param proxy A test-proxy server url.
*/
void SetTestProxy(std::string const& proxy) { m_proxy = proxy; }

/**
* @brief Set the performance test to run insecure.
*
* @details Running insecure means that for an SSL connection, the server certificate won't be
* validated to be a known certificate. Use this to stablish conversation with Https servers
* using self-signed certificates.
*
* @param value Boolean value use to set the insecure mode ON of OFF.
*/
void AllowInsecureConnections(bool value) { m_isInsecureEnabled = value; }

/**
* @brief Define actions to run after test set up and before the actual test.
*
Expand All @@ -59,12 +81,7 @@ namespace Azure { namespace Perf {
*/
void PreCleanUp();

/**
* @brief Set the client options depending on the test options.
*
* @param clientOptions ref to the client options that contains the http pipeline policies.
*/
void ConfigureCoreClientOptions(Azure::Core::_internal::ClientOptions* clientOptions);
void ConfigureInsecureConnection(Azure::Core::_internal::ClientOptions& clientOptions);

protected:
Azure::Perf::TestOptions m_options;
Expand Down Expand Up @@ -127,17 +144,11 @@ namespace Azure { namespace Perf {
virtual void GlobalCleanup() {}

/**
* @brief Update an existing \p clientOptions with the test configuration set by the
* environment.
*
* @note If test proxy env var is set, the proxy policy is added to the \p clientOptions.
* @brief Set the client options depending on the test options.
*
* @param clientOptions Ref to the client options that contains the Http client policies.
* @param clientOptions ref to the client options that contains the http pipeline policies.
*/
template <class T> void ConfigureClientOptions(T* clientOptions)
{
ConfigureCoreClientOptions(clientOptions);
}
void ConfigureClientOptions(Azure::Core::_internal::ClientOptions& clientOptions);

/**
* @brief Create and return client options with test configuration set in the environment.
Expand All @@ -147,7 +158,7 @@ namespace Azure { namespace Perf {
template <class T> T InitClientOptions()
{
T options;
ConfigureClientOptions(&options);
ConfigureClientOptions(options);
return options;
}
};
Expand Down
2 changes: 1 addition & 1 deletion sdk/core/perf/src/arg_parser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ Azure::Perf::GlobalTestOptions Azure::Perf::Program::ArgParser::Parse(
}
if (parsedArgs["Insecure"])
{
options.Insecure = parsedArgs["Insecure"].as<bool>();
options.Insecure = true;
}
if (parsedArgs["Iterations"])
{
Expand Down
41 changes: 37 additions & 4 deletions sdk/core/perf/src/base_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,15 @@
// SPDX-License-Identifier: MIT

#include "azure/perf/base_test.hpp"
#include "azure/core/http/policies/policy.hpp"
#include "azure/core/internal/http/pipeline.hpp"

#if defined(BUILD_CURL_HTTP_TRANSPORT_ADAPTER)
#include <azure/core/http/curl_transport.hpp>
#endif
#if defined(BUILD_TRANSPORT_WINHTTP_ADAPTER)
#include <azure/core/http/win_http_transport.hpp>
#endif
#include <azure/core/http/policies/policy.hpp>
#include <azure/core/internal/http/pipeline.hpp>

#include <functional>
#include <string>
Expand Down Expand Up @@ -96,12 +103,36 @@ class ProxyPolicy final : public HttpPolicy {

namespace Azure { namespace Perf {

void BaseTest::ConfigureCoreClientOptions(Azure::Core::_internal::ClientOptions* clientOptions)
void BaseTest::ConfigureInsecureConnection(Azure::Core::_internal::ClientOptions& clientOptions)
{
// NOTE: perf-fm is injecting the SSL config and transport here for the client options
// If the test overrides the options/transport, this can be undone.
#if defined(BUILD_CURL_HTTP_TRANSPORT_ADAPTER)
if (m_isInsecureEnabled)
{
Azure::Core::Http::CurlTransportOptions curlOptions;
curlOptions.SslVerifyPeer = false;
clientOptions.Transport.Transport
= std::make_shared<Azure::Core::Http::CurlTransport>(curlOptions);
}
#elif defined(BUILD_TRANSPORT_WINHTTP_ADAPTER)
Azure::Core::Http::WinHttpTransportOptions winHttpOptions;
winHttpOptions.IgnoreUnknownServerCert = true;
clientOptions.Transport.Transport
= std::make_shared<Azure::Core::Http::WinHttpTransport>(winHttpOptions);
#else
// avoid the variable not used warning
(void)clientOptions;
#endif
}

void BaseTest::ConfigureClientOptions(Azure::Core::_internal::ClientOptions& clientOptions)
{
if (!m_proxy.empty())
{
clientOptions->PerRetryPolicies.push_back(std::make_unique<ProxyPolicy>(this));
clientOptions.PerRetryPolicies.push_back(std::make_unique<ProxyPolicy>(this));
}
ConfigureInsecureConnection(clientOptions);
}

void BaseTest::PostSetUp()
Expand All @@ -111,6 +142,7 @@ namespace Azure { namespace Perf {
{
Azure::Core::_internal::ClientOptions clientOp;
clientOp.Retry.MaxRetries = 0;
ConfigureInsecureConnection(clientOp);
std::vector<std::unique_ptr<Azure::Core::Http::Policies::HttpPolicy>> policiesOp;
std::vector<std::unique_ptr<Azure::Core::Http::Policies::HttpPolicy>> policiesRe;
Azure::Core::Http::_internal::HttpPipeline pipeline(
Expand Down Expand Up @@ -180,6 +212,7 @@ namespace Azure { namespace Perf {
{
Azure::Core::_internal::ClientOptions clientOp;
clientOp.Retry.MaxRetries = 0;
ConfigureInsecureConnection(clientOp);
std::vector<std::unique_ptr<Azure::Core::Http::Policies::HttpPolicy>> policiesOp;
std::vector<std::unique_ptr<Azure::Core::Http::Policies::HttpPolicy>> policiesRe;
Azure::Core::Http::_internal::HttpPipeline pipeline(
Expand Down
2 changes: 1 addition & 1 deletion sdk/core/perf/src/options.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ std::vector<Azure::Perf::TestOption> Azure::Perf::GlobalTestOptions::GetOptionMe
"Duration of the test in seconds. Default to 10 seconds.",
1},
{"Host", {"--host"}, "Host to redirect HTTP requests. No redirection by default.", 1},
{"Insecure", {"--insecure"}, "Allow untrusted SSL certs. Default to false.", 1},
{"Insecure", {"--insecure"}, "Allow untrusted SSL certs. Default to false.", 0},
{"Iterations",
{"-i", "--iterations"},
"Number of iterations of main test loop. Default to 1.",
Expand Down
4 changes: 4 additions & 0 deletions sdk/core/perf/src/program.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -320,6 +320,10 @@ void Azure::Perf::Program::Run(
// Take the corresponding proxy from the list in round robin
parallelTest[i]->SetTestProxy(options.TestProxies[i % options.TestProxies.size()]);
}
if (options.Insecure)
{
parallelTest[i]->AllowInsecureConnections(true);
}
}

/******************** Global Set up ******************************/
Expand Down