TCPChannelResourceSecure/TCPTransportInterface may not check server certificate CN/SAN/hostname [8992]

[theopolis]

Hi folks,

I am working on LGTM queries that look for uses of boost::asio::ssl::context::set_verify_mode without using ::set_verify_callback and saw that is the case with TCPChannelResourceSecure::set_tls_verify_mode.

void TCPChannelResourceSecure::set_tls_verify_mode(const TCPTransportDescriptor* options)
{
    using TLSVerifyMode = TCPTransportDescriptor::TLSConfig::TLSVerifyMode;

    if (options->apply_security)
    {
        if (options->tls_config.verify_mode != TLSVerifyMode::UNUSED)
        {
            [...]
            secure_socket_->set_verify_mode(vm);
            // Verify callback is not set here.
        }
    }
}

And also in TCPTransportInterface::apply_tls_config a lambda callback is provided but the logic is essentially the same as if no callback was used. What I mean by this is that only returning preverified is the default behavior when not setting a callback.

void TCPTransportInterface::apply_tls_config()
{
#if TLS_FOUND
    const TCPTransportDescriptor* descriptor = configuration();
    if (descriptor->apply_security)
    {
        ssl_context_.set_verify_callback([](bool preverified, ssl::verify_context&)
        {
            return preverified;
            // This is not inspecting the CN/SAN
        });
       [...]

In both cases this can be addressed with

 sock_->set_verify_callback(boost::asio::ssl::rfc2818_verification(hostname_or_ip));

This means that the server certificate's CN or SAN value is not verified to be the host the client connects to, but rather any certificate in the configured root store. Since there is an option default_verify_path this means any certificate from any authority on the client OS.

I apologize that I am only doing a code review and not testing end-to-end but I wanted to give a heads up to someone who knows the code/project very well.

[Mario-DL]

Hi @theopolis,

Thanks for the report and sorry for the delay in the response.
We will take a closer look at this in the upcoming days.