Support for Server Name Indication (SNI) missing in http_client_asio.cpp

[megaposer]

Hi,

run into this issue with an OSX build of the latest C++ REST API 2.7.0.

There is no (default) support for the Server Name Indication TLS extension. OpenSSL does not enable the option automatically, so it needs to be set in the http_client_asio.cpp handshake implementation.

We tried adding it via set_native_handle_options first, but did not get it to work properly. This is probably the wrong place to do it anyway, because one would only need to set the SNI option once before the initial handshake, so it is contained within the ClientHello message.

Please see attached patch: cpprest_asio_ssl_sni.patch.txt

The patch unconditionally adds the server host name using the OpenSSL option SSL_set_tlsext_host_name before invoking the asynchronous handshake. Of course this could be improved to be provided via a config option.

Please note, SNI seems to be supported (automatically) on TLS connections when using the native WinHttp library - I remember starting with Windows Vista. Doing wireshark one can see the extension in the ClientHello send from a Windows app using the C++ REST API.

Let me know if you need more information.

Cheers,
Henning

[ras0219-msft]

Thanks for the detailed investigation! If windows sets this by default already, it seems sound to enable it for openssl by default. I need to go read up on SNI, but does it make sense to enable it for secure websockets as well?

Could you submit a PR with the changes? It's much easier for us to work with a pull request than a patchfile.

[kavyako]

We already have the setting SNI support for secure websockets, see websocket_client_config::is_sni_enabled(), set_server_name() APIs.

I agree that we could enable it by default for the http_client. Please go ahead with the PR.
Although adding the additional option where users can specify the server name would be added goodness :)

[megaposer]

I've opened a pull-request in #39 and I am currently waiting for permission on the CLA.

[kavyako]

PR has been merged to the development branch. Closing the issue.