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.

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

Added support to ignore invalid cert common name #4361

Merged
merged 3 commits into from
Feb 25, 2023
Merged

Added support to ignore invalid cert common name #4361

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
764aca7
Added support to ignore invalid cert common name
Jinming-Hu Feb 15, 2023
0cdc6ab
f
Jinming-Hu Feb 22, 2023
830da35
aaa
Jinming-Hu Feb 24, 2023
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
3 changes: 3 additions & 0 deletions sdk/core/azure-core/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@

### Features Added

- Added the ability to ignore invalid certificate common name for TLS connections in WinHTTP transport.
- Added `DisableTlsCertificateValidation` in `TransportOptions`.

### Breaking Changes

### Bugs Fixed
Expand Down
13 changes: 13 additions & 0 deletions sdk/core/azure-core/inc/azure/core/http/policies/policy.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,19 @@ namespace Azure { namespace Core { namespace Http { namespace Policies {
*/
bool EnableCertificateRevocationListCheck{false};

/**
* @brief Disable SSL/TLS certificate verification. This option allows transport layer to
* perform insecure SSL/TLS connections and skip SSL/TLS certificate checks while still having
* SSL/TLS-encrypted communications.
*
* @remark Disabling TLS security is generally a bad idea because it allows malicious actors to
* spoof the target server and should never be enabled in production code.
*
* @remark This field is only used if the customer has not specified a default transport
* adapter. If the customer has set a Transport adapter, this option is ignored.
*/
bool DisableTlsCertificateValidation{false};

/**
* @brief Base64 encoded DER representation of an X.509 certificate expected in the certificate
* chain used in TLS connections.
Expand Down
5 changes: 5 additions & 0 deletions sdk/core/azure-core/inc/azure/core/http/win_http_transport.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,11 @@ namespace Azure { namespace Core {
*/
bool IgnoreUnknownCertificateAuthority{false};

/**
* @brief When `true`, allows an invalid common name in a certificate.
*/
bool IgnoreInvalidCertificateCommonName{false};

/**
* Proxy information.
*/
Expand Down
1 change: 1 addition & 0 deletions sdk/core/azure-core/src/http/curl/curl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -311,6 +311,7 @@ Azure::Core::Http::CurlTransportOptions CurlTransportOptionsFromTransportOptions
curlOptions.SslOptions.PemEncodedExpectedRootCertificates
= PemEncodeFromBase64(transportOptions.ExpectedTlsRootCertificate, "CERTIFICATE");
}
curlOptions.SslVerifyPeer = !transportOptions.DisableTlsCertificateValidation;
Copy link
Member

Choose a reason for hiding this comment

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

Just making sure (because it seems to be ok), but @LarryOsterman would we need to update the connection key logic that we use for the connection pooling/caching? It is meant to be unique for each set of options (based on SslVerifyPeer being set).

azure-sdk-for-cpp/sdk/core/azure-core/src/http/curl/curl.cpp

Line 1293 in e2a1b99

key.append(options.SslVerifyPeer ? "1" : "0");

Copy link
Member

Choose a reason for hiding this comment

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

Good question: Based on my reading, the SslVerifyPeer option is already included in the connection key logic - see line 1284 of GetConnectionKey.

return curlOptions;
}

Expand Down
10 changes: 5 additions & 5 deletions sdk/core/azure-core/src/http/transport_policy.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,11 @@ namespace Azure { namespace Core { namespace Http { namespace Policies { namespa
*/
bool AreAnyTransportOptionsSpecified(TransportOptions const& transportOptions)
{
return (
transportOptions.HttpProxy.HasValue() || transportOptions.ProxyPassword.HasValue()
|| transportOptions.ProxyUserName.HasValue()
|| transportOptions.EnableCertificateRevocationListCheck
|| !transportOptions.ExpectedTlsRootCertificate.empty());
return (transportOptions.HttpProxy.HasValue() || transportOptions.ProxyPassword.HasValue()
|| transportOptions.ProxyUserName.HasValue()
|| transportOptions.EnableCertificateRevocationListCheck
|| !transportOptions.ExpectedTlsRootCertificate.empty())
|| transportOptions.DisableTlsCertificateValidation;
}
} // namespace

Expand Down
16 changes: 16 additions & 0 deletions sdk/core/azure-core/src/http/winhttp/win_http_transport.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -765,6 +765,12 @@ WinHttpTransportOptions WinHttpTransportOptionsFromTransportOptions(
httpOptions.IgnoreUnknownCertificateAuthority = true;
}

if (transportOptions.DisableTlsCertificateValidation)
{
httpOptions.IgnoreUnknownCertificateAuthority = true;
httpOptions.IgnoreInvalidCertificateCommonName = true;
}

return httpOptions;
}
} // namespace
Expand Down Expand Up @@ -918,6 +924,16 @@ _detail::WinHttpRequest::WinHttpRequest(
}
}

if (options.IgnoreInvalidCertificateCommonName)
{
auto option = SECURITY_FLAG_IGNORE_CERT_CN_INVALID;
if (!WinHttpSetOption(
m_requestHandle.get(), WINHTTP_OPTION_SECURITY_FLAGS, &option, sizeof(option)))
{
GetErrorAndThrow("Error while setting ignore invalid certificate common name.");
}
}

if (options.EnableCertificateRevocationListCheck)
{
DWORD value = WINHTTP_ENABLE_SSL_REVOCATION;
Expand Down