From c2e0344110c01eef33697c9611c5689e8eee0691 Mon Sep 17 00:00:00 2001 From: Mattes D Date: Tue, 9 May 2023 11:29:35 +0200 Subject: cTCPLink: Use the original connection hostname for SNI. --- src/Bindings/LuaTCPLink.cpp | 3 +-- src/HTTP/UrlClient.cpp | 2 +- src/OSSupport/Network.h | 3 +-- src/OSSupport/TCPLinkImpl.cpp | 22 ++++++++++++++++------ src/OSSupport/TCPLinkImpl.h | 18 ++++++++++++++---- src/mbedTLS++/SslContext.h | 4 +++- 6 files changed, 36 insertions(+), 16 deletions(-) diff --git a/src/Bindings/LuaTCPLink.cpp b/src/Bindings/LuaTCPLink.cpp index 1e8f99410..14ea5c905 100644 --- a/src/Bindings/LuaTCPLink.cpp +++ b/src/Bindings/LuaTCPLink.cpp @@ -193,8 +193,7 @@ AString cLuaTCPLink::StartTLSClient( } } - // TODO : Provide a way to pass SNI from Lua too. - return link->StartTLSClient(ownCert, ownPrivKey, ""); + return link->StartTLSClient(ownCert, ownPrivKey); } return ""; } diff --git a/src/HTTP/UrlClient.cpp b/src/HTTP/UrlClient.cpp index 3985e0707..ed47341c3 100644 --- a/src/HTTP/UrlClient.cpp +++ b/src/HTTP/UrlClient.cpp @@ -299,7 +299,7 @@ public: m_Link = &a_Link; if (m_IsTls) { - m_Link->StartTLSClient(m_ParentRequest.GetOwnCert(), m_ParentRequest.GetOwnPrivKey(), m_ParentRequest.m_UrlHost); + m_Link->StartTLSClient(m_ParentRequest.GetOwnCert(), m_ParentRequest.GetOwnPrivKey()); } else { diff --git a/src/OSSupport/Network.h b/src/OSSupport/Network.h index 32c7ecdd0..32163b710 100644 --- a/src/OSSupport/Network.h +++ b/src/OSSupport/Network.h @@ -113,8 +113,7 @@ public: Returns empty string on success, non-empty error description on failure. */ virtual AString StartTLSClient( cX509CertPtr a_OwnCert, - cCryptoKeyPtr a_OwnPrivKey, - const std::string_view hostname + cCryptoKeyPtr a_OwnPrivKey ) = 0; /** Starts a TLS handshake as a server connection. diff --git a/src/OSSupport/TCPLinkImpl.cpp b/src/OSSupport/TCPLinkImpl.cpp index 86fa24a63..6bd33e9f5 100644 --- a/src/OSSupport/TCPLinkImpl.cpp +++ b/src/OSSupport/TCPLinkImpl.cpp @@ -17,10 +17,11 @@ //////////////////////////////////////////////////////////////////////////////// // cTCPLinkImpl: -cTCPLinkImpl::cTCPLinkImpl(cTCPLink::cCallbacksPtr a_LinkCallbacks): +cTCPLinkImpl::cTCPLinkImpl(const std::string & a_Host, cTCPLink::cCallbacksPtr a_LinkCallbacks): Super(std::move(a_LinkCallbacks)), m_BufferEvent(bufferevent_socket_new(cNetworkSingleton::Get().GetEventBase(), -1, BEV_OPT_CLOSE_ON_FREE | BEV_OPT_THREADSAFE | BEV_OPT_DEFER_CALLBACKS | BEV_OPT_UNLOCK_CALLBACKS)), m_LocalPort(0), + m_RemoteHost(a_Host), m_RemotePort(0), m_ShouldShutdown(false) { @@ -30,7 +31,13 @@ cTCPLinkImpl::cTCPLinkImpl(cTCPLink::cCallbacksPtr a_LinkCallbacks): -cTCPLinkImpl::cTCPLinkImpl(evutil_socket_t a_Socket, cTCPLink::cCallbacksPtr a_LinkCallbacks, cServerHandleImplPtr a_Server, const sockaddr * a_Address, socklen_t a_AddrLen): +cTCPLinkImpl::cTCPLinkImpl( + evutil_socket_t a_Socket, + cTCPLink::cCallbacksPtr a_LinkCallbacks, + cServerHandleImplPtr a_Server, + const sockaddr * a_Address, + socklen_t a_AddrLen +): Super(std::move(a_LinkCallbacks)), m_BufferEvent(bufferevent_socket_new(cNetworkSingleton::Get().GetEventBase(), a_Socket, BEV_OPT_CLOSE_ON_FREE | BEV_OPT_THREADSAFE | BEV_OPT_DEFER_CALLBACKS | BEV_OPT_UNLOCK_CALLBACKS)), m_Server(std::move(a_Server)), @@ -65,7 +72,7 @@ cTCPLinkImplPtr cTCPLinkImpl::Connect(const AString & a_Host, UInt16 a_Port, cTC ASSERT(a_ConnectCallbacks != nullptr); // Create a new link: - cTCPLinkImplPtr res{new cTCPLinkImpl(std::move(a_LinkCallbacks))}; // Cannot use std::make_shared here, constructor is not accessible + cTCPLinkImplPtr res{new cTCPLinkImpl(a_Host, std::move(a_LinkCallbacks))}; // Cannot use std::make_shared here, constructor is not accessible res->m_ConnectCallbacks = std::move(a_ConnectCallbacks); cNetworkSingleton::Get().AddLink(res); res->m_Callbacks->OnLinkCreated(res); @@ -237,8 +244,7 @@ void cTCPLinkImpl::Close(void) AString cTCPLinkImpl::StartTLSClient( cX509CertPtr a_OwnCert, - cCryptoKeyPtr a_OwnPrivKey, - const std::string_view hostname + cCryptoKeyPtr a_OwnPrivKey ) { // Check preconditions: @@ -264,7 +270,11 @@ AString cTCPLinkImpl::StartTLSClient( m_TlsContext->Initialize(true); } - m_TlsContext->SetExpectedPeerName(hostname); + // Enable SNI / peer name verification: + if (!m_RemoteHost.empty()) + { + m_TlsContext->SetExpectedPeerName(m_RemoteHost); + } m_TlsContext->SetSelf(cLinkTlsContextWPtr(m_TlsContext)); diff --git a/src/OSSupport/TCPLinkImpl.h b/src/OSSupport/TCPLinkImpl.h index d26b1e358..c757303d2 100644 --- a/src/OSSupport/TCPLinkImpl.h +++ b/src/OSSupport/TCPLinkImpl.h @@ -40,9 +40,16 @@ public: /** Creates a new link based on the given socket. Used for connections accepted in a server using cNetwork::Listen(). + a_Host is the hostname used for TLS SNI (can be empty in cases TLS is not used). a_Address and a_AddrLen describe the remote peer that has connected. The link is created disabled, you need to call Enable() to start the regular communication. */ - cTCPLinkImpl(evutil_socket_t a_Socket, cCallbacksPtr a_LinkCallbacks, cServerHandleImplPtr a_Server, const sockaddr * a_Address, socklen_t a_AddrLen); + cTCPLinkImpl( + evutil_socket_t a_Socket, + cCallbacksPtr a_LinkCallbacks, + cServerHandleImplPtr a_Server, + const sockaddr * a_Address, + socklen_t a_AddrLen + ); /** Destroys the LibEvent handle representing the link. */ virtual ~cTCPLinkImpl() override; @@ -68,8 +75,7 @@ public: virtual void Close(void) override; virtual AString StartTLSClient( cX509CertPtr a_OwnCert, - cCryptoKeyPtr a_OwnPrivKey, - const std::string_view hostname + cCryptoKeyPtr a_OwnPrivKey ) override; virtual AString StartTLSServer( cX509CertPtr a_OwnCert, @@ -151,6 +157,10 @@ protected: /** The port of the local endpoint. Valid only after the socket has been connected. */ UInt16 m_LocalPort; + /** The original host parameter which was used for creating the link, either hostname or IP address. + Used for TLS SNI. */ + AString m_RemoteHost; + /** The IP address of the remote endpoint. Valid only after the socket has been connected. */ AString m_RemoteIP; @@ -175,7 +185,7 @@ protected: Used for outgoing connections created using cNetwork::Connect(). To be used only by the Connect() factory function. The link is created disabled, you need to call Enable() to start the regular communication. */ - cTCPLinkImpl(const cCallbacksPtr a_LinkCallbacks); + cTCPLinkImpl(const std::string & a_Host, const cCallbacksPtr a_LinkCallbacks); /** Callback that LibEvent calls when there's data available from the remote peer. */ static void ReadCallback(bufferevent * a_BufferEvent, void * a_Self); diff --git a/src/mbedTLS++/SslContext.h b/src/mbedTLS++/SslContext.h index 6343f7e43..b4b184403 100644 --- a/src/mbedTLS++/SslContext.h +++ b/src/mbedTLS++/SslContext.h @@ -51,7 +51,9 @@ public: /** Returns true if the object has been initialized properly. */ bool IsValid(void) const { return m_IsValid; } - /** Sets the SSL peer name expected for this context. Must be called after Initialize(). + /** Sets the SSL peer name expected for this context. + This is used both for TLS SNI and for certificate validation. + Must be called after Initialize(). \param a_ExpectedPeerName CommonName that we expect the SSL peer to have in its cert, if it is different, the verification will fail. An empty string will disable the CN check. */ void SetExpectedPeerName(const std::string_view a_ExpectedPeerName); -- cgit v1.2.3