From 1e46466114b285b523803946447f8856bc34397e Mon Sep 17 00:00:00 2001 From: Robin Sommer Date: Wed, 12 Oct 2022 15:41:32 +0200 Subject: [PATCH] Add option to disable hostname check (#399) * Suppress compiler warnings about unused elements. * Enable CMake's compilation database. * Add TLS option to disable checking a certificate's host name. * Add `--disable-hostname-validation` to `ws`. * Add test for disabling hostname validation. --- CMakeLists.txt | 1 + docs/usage.md | 2 ++ ixwebsocket/IXSocketAppleSSL.cpp | 4 ++- ixwebsocket/IXSocketMbedTLS.cpp | 11 +++--- ixwebsocket/IXSocketOpenSSL.cpp | 16 +++++++-- ixwebsocket/IXSocketTLSOptions.h | 3 ++ test/.certs/wrong-name-server-crt.pem | 20 +++++++++++ test/.certs/wrong-name-server-key.pem | 27 +++++++++++++++ test/IXHttpClientTest.cpp | 48 +++++++++++++++++++++++++++ ws/ws.cpp | 28 +--------------- 10 files changed, 126 insertions(+), 34 deletions(-) create mode 100644 test/.certs/wrong-name-server-crt.pem create mode 100644 test/.certs/wrong-name-server-key.pem diff --git a/CMakeLists.txt b/CMakeLists.txt index 16996c97..7a4b36b6 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -11,6 +11,7 @@ project(ixwebsocket C CXX) set (CMAKE_CXX_STANDARD 11) set (CXX_STANDARD_REQUIRED ON) set (CMAKE_CXX_EXTENSIONS OFF) +set (CMAKE_EXPORT_COMPILE_COMMANDS yes) option (BUILD_DEMO OFF) diff --git a/docs/usage.md b/docs/usage.md index 6a3ddc9f..57799be7 100644 --- a/docs/usage.md +++ b/docs/usage.md @@ -628,3 +628,5 @@ For a client, specifying `caFile` can be used if connecting to a server that use For a server, specifying `caFile` implies that: 1. You require clients to present a certificate 1. It must be signed by one of the trusted roots in the file + +By default, a destination's hostname is always validated against the certificate that it presents. To accept certificates with any hostname, set `ix::SocketTLSOptions::disable_hostname_validation` to `true`. diff --git a/ixwebsocket/IXSocketAppleSSL.cpp b/ixwebsocket/IXSocketAppleSSL.cpp index f58e0c8d..3a529dd2 100644 --- a/ixwebsocket/IXSocketAppleSSL.cpp +++ b/ixwebsocket/IXSocketAppleSSL.cpp @@ -205,7 +205,9 @@ namespace ix _sslContext, SocketAppleSSL::readFromSocket, SocketAppleSSL::writeToSocket); SSLSetConnection(_sslContext, (SSLConnectionRef)(long) _sockfd); SSLSetProtocolVersionMin(_sslContext, kTLSProtocol12); - SSLSetPeerDomainName(_sslContext, host.c_str(), host.size()); + + if (!_tlsOptions.disable_hostname_validation) + SSLSetPeerDomainName(_sslContext, host.c_str(), host.size()); if (_tlsOptions.isPeerVerifyDisabled()) { diff --git a/ixwebsocket/IXSocketMbedTLS.cpp b/ixwebsocket/IXSocketMbedTLS.cpp index 01f8c870..600061cb 100644 --- a/ixwebsocket/IXSocketMbedTLS.cpp +++ b/ixwebsocket/IXSocketMbedTLS.cpp @@ -48,7 +48,7 @@ namespace ix mbedtls_pk_init(&_pkey); } - bool SocketMbedTLS::loadSystemCertificates(std::string& errorMsg) + bool SocketMbedTLS::loadSystemCertificates(std::string& /* errorMsg */) { #ifdef _WIN32 DWORD flags = CERT_STORE_READONLY_FLAG | CERT_STORE_OPEN_EXISTING_FLAG | @@ -195,10 +195,13 @@ namespace ix return false; } - if (!host.empty() && mbedtls_ssl_set_hostname(&_ssl, host.c_str()) != 0) + if (!_tlsOptions.disable_hostname_validation) { - errMsg = "SNI setup failed"; - return false; + if (!host.empty() && mbedtls_ssl_set_hostname(&_ssl, host.c_str()) != 0) + { + errMsg = "SNI setup failed"; + return false; + } } return true; diff --git a/ixwebsocket/IXSocketOpenSSL.cpp b/ixwebsocket/IXSocketOpenSSL.cpp index 7fb6f6dc..12c95676 100644 --- a/ixwebsocket/IXSocketOpenSSL.cpp +++ b/ixwebsocket/IXSocketOpenSSL.cpp @@ -301,7 +301,11 @@ namespace ix } bool SocketOpenSSL::openSSLCheckServerCert(SSL* ssl, +#if OPENSSL_VERSION_NUMBER < 0x10100000L const std::string& hostname, +#else + const std::string& /* hostname */, +#endif std::string& errMsg) { X509* server_cert = SSL_get_peer_certificate(ssl); @@ -390,6 +394,11 @@ namespace ix int connect_result = SSL_connect(_ssl_connection); if (connect_result == 1) { + if (_tlsOptions.disable_hostname_validation) + { + return true; + } + return openSSLCheckServerCert(_ssl_connection, host, errMsg); } int reason = SSL_get_error(_ssl_connection, connect_result); @@ -754,8 +763,11 @@ namespace ix // (The docs say that this should work from 1.0.2, and is the default from // 1.1.0, but it does not. To be on the safe side, the manual test // below is enabled for all versions prior to 1.1.0.) - X509_VERIFY_PARAM* param = SSL_get0_param(_ssl_connection); - X509_VERIFY_PARAM_set1_host(param, host.c_str(), host.size()); + if (!_tlsOptions.disable_hostname_validation) + { + X509_VERIFY_PARAM* param = SSL_get0_param(_ssl_connection); + X509_VERIFY_PARAM_set1_host(param, host.c_str(), host.size()); + } #endif handshakeSuccessful = openSSLClientHandshake(host, errMsg, isCancellationRequested); } diff --git a/ixwebsocket/IXSocketTLSOptions.h b/ixwebsocket/IXSocketTLSOptions.h index e396b388..2b5793b5 100644 --- a/ixwebsocket/IXSocketTLSOptions.h +++ b/ixwebsocket/IXSocketTLSOptions.h @@ -33,6 +33,9 @@ namespace ix // whether tls is enabled, used for server code bool tls = false; + // whether to skip validating the peer's hostname against the certificate presented + bool disable_hostname_validation = false; + bool hasCertAndKey() const; bool isUsingSystemDefaults() const; diff --git a/test/.certs/wrong-name-server-crt.pem b/test/.certs/wrong-name-server-crt.pem new file mode 100644 index 00000000..24549fde --- /dev/null +++ b/test/.certs/wrong-name-server-crt.pem @@ -0,0 +1,20 @@ +-----BEGIN CERTIFICATE----- +MIIDNDCCAhwCFCl+O/rR8flqYKKvD0iwkucFwMaLMA0GCSqGSIb3DQEBCwUAMEEx +FDASBgNVBAoMC21hY2hpbmV6b25lMRQwEgYDVQQKDAtJWFdlYlNvY2tldDETMBEG +A1UEAwwKdHJ1c3RlZC1jYTAgFw0yMjA4MjMyMDM2MjVaGA80MjgxMDYwMTIwMzYy +NVowajELMAkGA1UEBhMCVVMxCzAJBgNVBAgMAkNBMREwDwYDVQQHDAhCZXJrZWxl +eTEbMBkGA1UECgwSRHVtbXkgT3JnYW5pemF0aW9uMR4wHAYDVQQDDBVub3QuYS52 +YWxpZC5ob3N0Lm5hbWUwggEiMA0GCSqGSIb3DQEBAQUAA4IBDwAwggEKAoIBAQC2 +9N806IjCvA82zfk9CPNwaEHOygNDJSXaZ38YDSI4C+Wf2imnMxrLQKaoccHUi+9L +4rQN/hSCg+uTULQUZ0iyssGDaIh4IcAeoEcNlXYHTrgP+aAaby3q5zeZ80K3+6e4 +rqcuBPV2lLszJu3XXwEKbDSxW3De0gc2N8m9DN8Lx7i70DRf0F4m6RIMFF/kHXwa +zZLeG7rZb4xL684lmmQsWtk5Z600CvrBtUE7fQ7bhy0QhSt66kdTSL8IKQrbWcGj +q0tggFlOqeyZSi73gqUiAIuGO8/tRgmp4HygNyC24jpOB5nObOPPJTUEf5Mk1Bum +kD5a+yL6YbVdhiaK17wbAgMBAAEwDQYJKoZIhvcNAQELBQADggEBAKsLXGLfO1IZ +LbofGc7/TCQwRayR3RdG4864PBy97KfJWyizg7Wm4X4yPFRG+6q3X5NKW32Ew9lI +jXulXCTjWOiSxiG4Pk20uczkOhWVHFdnS9gZvykPC/ElxBKPalT6MMstZWxpElsk +rCDKXj4LkD0po8bZrjlgSZQQQk6XMRkoRai2GWLJqIjaNCSF8nqb1wM/1OE1tAwi +polO1eFMA24yypvlXcNrNXjqcQ7LaoQFQltmi/RV+uTk7EK2F2jgYVzJ/pe2ET0i +RIMbGZTlAiemDGL9SpMsxftG6fSmP6QqDqYExmmPlZMLprb2da/2kelWFA+VkvdG +zFrnIcyfMY4= +-----END CERTIFICATE----- diff --git a/test/.certs/wrong-name-server-key.pem b/test/.certs/wrong-name-server-key.pem new file mode 100644 index 00000000..b946f29a --- /dev/null +++ b/test/.certs/wrong-name-server-key.pem @@ -0,0 +1,27 @@ +-----BEGIN RSA PRIVATE KEY----- +MIIEpAIBAAKCAQEAtvTfNOiIwrwPNs35PQjzcGhBzsoDQyUl2md/GA0iOAvln9op +pzMay0CmqHHB1IvvS+K0Df4UgoPrk1C0FGdIsrLBg2iIeCHAHqBHDZV2B064D/mg +Gm8t6uc3mfNCt/unuK6nLgT1dpS7Mybt118BCmw0sVtw3tIHNjfJvQzfC8e4u9A0 +X9BeJukSDBRf5B18Gs2S3hu62W+MS+vOJZpkLFrZOWetNAr6wbVBO30O24ctEIUr +eupHU0i/CCkK21nBo6tLYIBZTqnsmUou94KlIgCLhjvP7UYJqeB8oDcgtuI6TgeZ +zmzjzyU1BH+TJNQbppA+Wvsi+mG1XYYmite8GwIDAQABAoIBAGRzAXG9EglI01mV +sPfvyCi5NRhiFXRyGtxU4pTD8TuwXHxtfV0NU/KwJlBpVLBrvBCAAbeE/qHB6D9T +metx4ZorRs/tPrAmZ6LpANnWa50LfUdYGK0qyZ0lIYPm6YS2KJnfWm6LznEyq60j +/IW45YthaXTO7aOI0OjVrG+dd4CxU1g1NtCQ9bdDMDjfXFVnoOifXIl8W22eRMoZ +LzCz+0sI0R0LenXCPT566de21F0NDkIK7NaQ1l5BMX4PA+RsN3cZlzyruA1woPKI +aBR2LQGNrBfDVGMATtUm89RpWAftb8FmXqYUsM7zAzTO6dViitiB7OFlw7Ax15YH +MTj5zGECgYEA35ocEEMfyahBN70bjyiqOHlzKwFjDl9DsUf8xqHsNhYAL+GrOK9A +8lN5ByzcnbV3TJtU4WYbPgQJld8gXFx4h3eS+SkA/ASkAdtgHfdMImZ1v7k3TIPf +DXOCsHzELsQY6OgiI572Nwzx/Tl+0dFwaOfLjU9iEmmqL667j1Y4NiMCgYEA0Xch +9K/vwZ1I9gM3ySvG40R2TRriC9Bf8jwrEWeRsWNvBcqtMMrgwAMsMCKDugSZR7n6 +o3WZV6mpvYVLFx0b93v07i7EpFP27kMw3gLNBKX62snR9JbqwAMK7tktgLds5pKM +DvLHuAQ9XMMXMLX7WlSyhmtFeU7IDulTSHHqdakCgYEAywITCpy2xpKRK7bwx4gH +C6EQc/IdahYJ0nHmSL0IRY6x+sbrelp7H8ezcVVEs5bmylGYvc/DWgm2XjCnI9P8 +xhlFAhw9PZJFCT6QRISaxfy6WSgi0cBEieTeubd9MmxtpT/khuyy5AZHyj0iLAL4 +CPayMwjopIj0r7f3p8qC3HsCgYBmq2kmYVI4aZrIkv02CtIatYTy+DlSJxnQRvOp +PUWpWB6kDRrk7pxJIYT4NwKwG+7xvFQA6PR3hn7fmUUcGDWMEeMVGDFkho9ja+W4 +/FB3dc/Gi+PwakS4RwWF20e1brLfNXeXICMKrHFTVYC5bIm+VgOHZW8RLa9bt7wN +p2CPuQKBgQCjW+rCODmMzEues/I143mYMDdZ1IschtWGiXBNrpkHm/DcZSutbacm +5C7Kwv44pfA90NHDTjuaIgRgfeUTawkrl8uPXEuj80mW72agf5oS8lJzD+2jibCj +Q4K52G+0LaTxHLZxufil28Rgja01c0mTcuLbhKtCgHl5EHP19wOKEg== +-----END RSA PRIVATE KEY----- diff --git a/test/IXHttpClientTest.cpp b/test/IXHttpClientTest.cpp index 2f9899dd..2bc58162 100644 --- a/test/IXHttpClientTest.cpp +++ b/test/IXHttpClientTest.cpp @@ -7,7 +7,9 @@ #include "catch.hpp" #include #include +#include #include +#include using namespace ix; @@ -95,6 +97,52 @@ TEST_CASE("http_client", "[http]") } #endif +#if defined(IXWEBSOCKET_USE_TLS) && !defined(IXWEBSOCKET_USE_SECURE_TRANSPORT) + SECTION("Disable hostname validation") + { + static auto test_cert_with_wrong_name = [](bool validate_hostname) + { + int port = getFreePort(); + ix::HttpServer server(port, "127.0.0.1"); + + SocketTLSOptions tlsOptionsServer; + tlsOptionsServer.tls = true; + tlsOptionsServer.caFile = "NONE"; + tlsOptionsServer.certFile = "./.certs/wrong-name-server-crt.pem"; + tlsOptionsServer.keyFile = "./.certs/wrong-name-server-key.pem"; + server.setTLSOptions(tlsOptionsServer); + + auto res = server.listen(); + REQUIRE(res.first); + server.start(); + + HttpClient httpClient; + SocketTLSOptions tlsOptionsClient; + tlsOptionsClient.caFile = "./.certs/trusted-ca-crt.pem"; + tlsOptionsClient.disable_hostname_validation = validate_hostname; + httpClient.setTLSOptions(tlsOptionsClient); + + std::string url("https://localhost:" + std::to_string(port)); + auto args = httpClient.createRequest(url); + args->connectTimeout = 10; + args->transferTimeout = 10; + + auto response = httpClient.get(url, args); + + std::cerr << "Status: " << response->statusCode << std::endl; + std::cerr << "Error code: " << (int) response->errorCode << std::endl; + std::cerr << "Error message: " << response->errorMsg << std::endl; + + server.stop(); + return std::make_tuple(response->errorCode, response->statusCode); + }; + + REQUIRE(test_cert_with_wrong_name(false) == + std::make_tuple(HttpErrorCode::CannotConnect, 0)); + REQUIRE(test_cert_with_wrong_name(true) == std::make_tuple(HttpErrorCode::Ok, 404)); + } +#endif + SECTION("Async API, one call") { bool async = true; diff --git a/ws/ws.cpp b/ws/ws.cpp index 21288cff..a513e9bb 100644 --- a/ws/ws.cpp +++ b/ws/ws.cpp @@ -77,24 +77,6 @@ namespace return std::make_pair(res.first, std::string(vec.begin(), vec.end())); } - // Assume the file exists - std::string readBytes(const std::string& path) - { - std::vector memblock; - std::ifstream file(path); - - file.seekg(0, file.end); - std::streamoff size = file.tellg(); - file.seekg(0, file.beg); - - memblock.reserve((size_t) size); - memblock.insert( - memblock.begin(), std::istream_iterator(file), std::istream_iterator()); - - std::string bytes(memblock.begin(), memblock.end()); - return bytes; - } - std::string truncate(const std::string& str, size_t n) { if (str.size() < n) @@ -107,12 +89,6 @@ namespace } } - bool fileExists(const std::string& fileName) - { - std::ifstream infile(fileName); - return infile.good(); - } - std::string extractFilename(const std::string& path) { std::string::size_type idx; @@ -2486,10 +2462,8 @@ int main(int argc, char** argv) bool verbose = false; bool save = false; bool quiet = false; - bool fluentd = false; bool compress = false; bool compressRequest = false; - bool stress = false; bool disableAutomaticReconnection = false; bool disablePerMessageDeflate = false; bool greetings = false; @@ -2505,7 +2479,6 @@ int main(int argc, char** argv) int transferTimeout = 1800; int maxRedirects = 5; int delayMs = -1; - int count = 1; int msgCount = 1000 * 1000; uint32_t maxWaitBetweenReconnectionRetries = 10 * 1000; // 10 seconds int pingIntervalSecs = 30; @@ -2529,6 +2502,7 @@ int main(int argc, char** argv) "A (comma/space/colon) separated list of ciphers to use for TLS"); app->add_flag("--tls", tlsOptions.tls, "Enable TLS (server only)"); app->add_flag("--verify_none", verifyNone, "Disable peer cert verification"); + app->add_flag("--disable-hostname-validation", tlsOptions.disable_hostname_validation, "Disable validation of certificates' hostnames"); }; app.add_flag("--version", version, "Print ws version");