Skip to content

[#600] Add TLS SNI hostname to client options #601

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

Jump to bottom
Merged
merged 1 commit into from
Feb 19, 2016
Merged

[#600] Add TLS SNI hostname to client options #601

merged 1 commit into from
Feb 19, 2016

Conversation

theopolis
Copy link

This PR adds a new member and API to http::client::options for an optional TLS SNI hostname. HTTPS client option callsites may optionally support/provide an SNI hostname:

http::client::options options;
options.openssl_sni_hostname("my.shared.host");

http::client client(options);

The implementation within ssl_delegate and sync_ssl arrive at an OpenSSL (and equivalent API implementations): SSL_set_tlsext_host_name. See the OpenSSL wiki for TLS SNI details, as well as usage examples.

Also, there are various non-thesis related style changes pulled in via clang-format and the project's local format file. Please let me know if it is unacceptable to piggy-back these changes and I will try to remove them. Most of my editors auto-format so pulling out the changes is doable. 🙀 😉

@glynos
Copy link
Member

glynos commented Feb 16, 2016

@theopolis The CI builds are failing, could you update the PR? Also, could you apply this on the master branch? Unfortunately, master and 0.12-devel have diverged in the last few weeks, so it means some duplication of effort.

@theopolis
Copy link
Author

I'm going to attempt some E2E testing of the SNI support today. I can also submit a PR against cpp-netlib:master, but I wasn't sure if that code was a candidate for a 0.12 release. It seems there's some significant divergence from the 0.11 build w.r.t ASIO and boost.

@theopolis theopolis mentioned this pull request Feb 16, 2016
Merged
@theopolis
Copy link
Author

You'll most likely choose to merge #602, but for posterity: this implementation of SNI does work.

Although not captured in unit tests, a traffic dump shows the TLS server_name extension when the .openssl_sni_hostname is set. I tested against a host requiring SNI and the test client works as expected, whereas without the SNI client support it does not.

@theopolis theopolis closed this Feb 17, 2016
@theopolis theopolis reopened this Feb 19, 2016
@glynos
Copy link
Member

glynos commented Feb 19, 2016

LGTM

glynos added a commit that referenced this pull request Feb 19, 2016
@glynos
Merge pull request #601 from theopolis/0.12-devel
6689911 
[#600] Add TLS SNI hostname to client options
@glynos glynos merged commit 6689911 into cpp-netlib:0.12-devel Feb 19, 2016
@theopolis theopolis mentioned this pull request Feb 23, 2016
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@theopolis @glynos