From 51125b0fdc00e47629bc0009eeac020e2d1ddfb2 Mon Sep 17 00:00:00 2001 From: Sushrut Shringarputale Date: Mon, 12 May 2025 14:36:19 -0700 Subject: [PATCH] Add SSL caching share for curl (#6537) * Add SSL caching share for curl * Use correct destructor for CURLSH RAII * Unit tests passing locally * Add curloptions test for coverage and fix clangformat errors * whitespace reverts * Fix clang format errors * Address PR comments * Update changelog and CI line coverage target * Update Changelog and set curl options test to disable ssl caching for validating non-default codepath * Update sdk/core/azure-core/CHANGELOG.md * Update CHANGELOG.md * Update cspell.json --------- Co-authored-by: Anton Kolesnyk <41349689+antkmsft@users.noreply.github.com> --- .vscode/cspell.json | 2 + sdk/core/azure-core/CHANGELOG.md | 8 ++++ .../inc/azure/core/http/curl_transport.hpp | 5 ++ sdk/core/azure-core/src/http/curl/curl.cpp | 48 +++++++++++++++++++ .../src/http/curl/curl_connection_private.hpp | 28 +++++++++++ .../azure-core/test/ut/curl_options_test.cpp | 37 ++++++++++++++ sdk/core/ci.yml | 4 +- 7 files changed, 130 insertions(+), 2 deletions(-) diff --git a/.vscode/cspell.json b/.vscode/cspell.json index 6faac403e..f223e1a56 100644 --- a/.vscode/cspell.json +++ b/.vscode/cspell.json @@ -252,6 +252,7 @@ "sdpath", "serializers", "Seriot", + "Shringarputale", "southafricanorth", "southcentralus", "southeastasia", @@ -261,6 +262,7 @@ "stoll", "stoull", "STREQ", + "Sushrut", "Sutou", "testid", "swedencentral", diff --git a/sdk/core/azure-core/CHANGELOG.md b/sdk/core/azure-core/CHANGELOG.md index 908df3d5e..eabb0e43c 100644 --- a/sdk/core/azure-core/CHANGELOG.md +++ b/sdk/core/azure-core/CHANGELOG.md @@ -4,12 +4,20 @@ ### Features Added +- [[#6535]](https://github.com/Azure/azure-sdk-for-cpp/issues/6535) Enable SSL caching for libcurl transport by default, which is backwards compatible behavior with older libcurl versions, so using the default settings won't result in transport error when using libcurl >= 8.12. The option is controlled by `CurlTransportOptions::EnableCurlSslCaching`, and is on by default. (A community contribution, courtesy of _[chewi](https://github.com/sushshring)_) + ### Breaking Changes ### Bugs Fixed ### Other Changes +### Acknowledgments + +Thank you to our developer community members who helped to make Azure Core better with their contributions to this release: + +- Sushrut Shringarputale _([GitHub](https://github.com/sushshring))_ + ## 1.15.0 (2025-03-06) ### Features Added diff --git a/sdk/core/azure-core/inc/azure/core/http/curl_transport.hpp b/sdk/core/azure-core/inc/azure/core/http/curl_transport.hpp index d1780a5e1..2ffd76cee 100644 --- a/sdk/core/azure-core/inc/azure/core/http/curl_transport.hpp +++ b/sdk/core/azure-core/inc/azure/core/http/curl_transport.hpp @@ -194,6 +194,11 @@ namespace Azure { namespace Core { namespace Http { * @brief If set, integrates libcurl's internal tracing with Azure logging. */ bool EnableCurlTracing = false; + + /** + * @brief If set, enables libcurl's internal SSL session caching. + */ + bool EnableCurlSslCaching = true; }; /** diff --git a/sdk/core/azure-core/src/http/curl/curl.cpp b/sdk/core/azure-core/src/http/curl/curl.cpp index c27369a23..3ca3ef575 100644 --- a/sdk/core/azure-core/src/http/curl/curl.cpp +++ b/sdk/core/azure-core/src/http/curl/curl.cpp @@ -82,6 +82,17 @@ inline bool SetLibcurlOption( *outError = curl_easy_setopt(handle.get(), option, value); return *outError == CURLE_OK; } + +inline bool SetLibcurlShareOption( + std::unique_ptr const& handle, + CURLSHoption option, + curl_lock_data value, + CURLSHcode* outError) +{ + *outError = curl_share_setopt(handle->share_handle, option, value); + return *outError == CURLSHE_OK; +} + #if defined(_MSC_VER) #pragma warning(pop) #endif @@ -2303,6 +2314,43 @@ CurlConnection::CurlConnection( } CURLcode result; + m_sslShareHandle = std::make_unique(); + if (!m_sslShareHandle->share_handle) + { + throw Azure::Core::Http::TransportException( + _detail::DefaultFailedToGetNewConnectionTemplate + hostDisplayName + ". " + + std::string("curl_share_init returned Null")); + } + + if (!options.EnableCurlSslCaching) + { + // Disable SSL session ID caching + if (!SetLibcurlOption(m_handle, CURLOPT_SSL_SESSIONID_CACHE, 0L, &result)) + { + throw Azure::Core::Http::TransportException( + _detail::DefaultFailedToGetNewConnectionTemplate + hostDisplayName + ". " + + std::string(curl_easy_strerror(result))); + } + } + else + { + CURLSHcode shResult; + if (!SetLibcurlShareOption( + m_sslShareHandle, CURLSHOPT_SHARE, CURL_LOCK_DATA_SSL_SESSION, &shResult)) + { + throw Azure::Core::Http::TransportException( + _detail::DefaultFailedToGetNewConnectionTemplate + hostDisplayName + ". " + + std::string(curl_share_strerror(shResult))); + } + + if (!SetLibcurlOption(m_handle, CURLOPT_SHARE, m_sslShareHandle.get(), &result)) + { + throw Azure::Core::Http::TransportException( + _detail::DefaultFailedToGetNewConnectionTemplate + hostDisplayName + ". " + + std::string(curl_easy_strerror(result))); + } + } + if (options.EnableCurlTracing) { if (!SetLibcurlOption( diff --git a/sdk/core/azure-core/src/http/curl/curl_connection_private.hpp b/sdk/core/azure-core/src/http/curl/curl_connection_private.hpp index dc7bb3675..b122af19f 100644 --- a/sdk/core/azure-core/src/http/curl/curl_connection_private.hpp +++ b/sdk/core/azure-core/src/http/curl/curl_connection_private.hpp @@ -42,6 +42,33 @@ namespace Azure { namespace Core { { using type = _internal::BasicUniqueHandle; }; + + /** + * + * @brief Unique handle wrapper for CURLSH handles. + * + * @note Internally, CURL and CURLSH are declared as the same void type, + * so to avoid collisions we need this wrapper. + */ + struct CURLSHWrapper + { + CURLSH* share_handle; + + CURLSHWrapper() : share_handle{curl_share_init()} {}; + + ~CURLSHWrapper() + { + if (share_handle != nullptr) + { + curl_share_cleanup(share_handle); + } + } + }; + + /** + * @brief Unique handle for CURLSHWrapper handles + */ + using UniqueCURLSHHandle = std::unique_ptr; } // namespace _detail namespace Http { @@ -142,6 +169,7 @@ namespace Azure { namespace Core { class CurlConnection final : public CurlNetworkConnection { private: Azure::Core::_internal::UniqueHandle m_handle; + Azure::Core::_detail::UniqueCURLSHHandle m_sslShareHandle; curl_socket_t m_curlSocket; std::chrono::steady_clock::time_point m_lastUseTime; std::string m_connectionKey; diff --git a/sdk/core/azure-core/test/ut/curl_options_test.cpp b/sdk/core/azure-core/test/ut/curl_options_test.cpp index 72f10fb73..d7dfa778f 100644 --- a/sdk/core/azure-core/test/ut/curl_options_test.cpp +++ b/sdk/core/azure-core/test/ut/curl_options_test.cpp @@ -365,4 +365,41 @@ namespace Azure { namespace Core { namespace Test { 0); } + TEST(CurlTransportOptions, disableSslCaching) + { + Azure::Core::Http::CurlTransportOptions curlOptions; + curlOptions.EnableCurlSslCaching = false; + + auto transportAdapter = std::make_shared(curlOptions); + Azure::Core::Http::Policies::TransportOptions options; + options.Transport = transportAdapter; + auto transportPolicy + = std::make_unique(options); + + std::vector> policies; + policies.emplace_back(std::move(transportPolicy)); + Azure::Core::Http::_internal::HttpPipeline pipeline(policies); + + Azure::Core::Url url(AzureSdkHttpbinServer::Get()); + Azure::Core::Http::Request request(Azure::Core::Http::HttpMethod::Get, url); + + std::unique_ptr response; + EXPECT_NO_THROW(response = pipeline.Send(request, Azure::Core::Context{})); + if (response) + { + auto responseCode = response->GetStatusCode(); + int expectedCode = 200; + EXPECT_PRED2( + [](int expected, int code) { return expected == code; }, + expectedCode, + static_cast::type>( + responseCode)); + } + + // Clean the connection from the pool *Windows fails to clean if we leave to be clean upon + // app-destruction + EXPECT_NO_THROW(Azure::Core::Http::_detail::CurlConnectionPool::g_curlConnectionPool + .ConnectionPoolIndex.clear()); + } + }}} // namespace Azure::Core::Test diff --git a/sdk/core/ci.yml b/sdk/core/ci.yml index c382dcff3..ddf1953bd 100644 --- a/sdk/core/ci.yml +++ b/sdk/core/ci.yml @@ -51,8 +51,8 @@ extends: CtestRegex: azure-core.|json-test LiveTestCtestRegex: azure-core.|json-test LiveTestTimeoutInMinutes: 90 # default is 60 min. We need a little longer on worst case for Win+jsonTests - LineCoverageTarget: 85 - BranchCoverageTarget: 63 + LineCoverageTarget: 84 + BranchCoverageTarget: 62 PreTestSteps: - pwsh: | $(Build.SourcesDirectory)/sdk/core/azure-core-amqp/Test-Setup.ps1