Get SNI for OpenSSL #7291
Get SNI for OpenSSL #7291
Conversation
|
Looks good, though in Ruby I think the method is called |
|
(note: I might be wrong) |
|
@asterite the |
There was a problem hiding this comment.
I agree with @asterite. Looking at the OpenSSL documentation, the method could be #hostname with documentation stating that it returns the server name indication (SNI), and there could be SSL_set_tlsext_host_name to implement the #hostname= method which is useful on clients.
|
@straight-shoota regarding specs, sadly can't make those until the etc.. |
|
For which openssl versions are these methods available? Do we need or want to conditionalize defining them for nicer compilation errors? Btw, maybe I'm wrong, but I vaguely remember that you can bundle up your certs and keys into a single file, load that into a context and openssl will pick the first matching one. |
|
Ok, this is really wierd, the function should be present in 1.1.0..master, I can see it here: https://github.com/openssl/openssl/blob/master/apps/s_client.c#L1980 |
|
@asterite @jhass @straight-shoota @RX14 I totally missed we already have the Let's squash and merge, then I'll start working on adding the callback which is the needed part for the whole SNI server flow |
|
@asterite @ysbaddaden @straight-shoota can I get another approval ? |
There was a problem hiding this comment.
Just one last tiny change to use @jhass proposal for the documentation, and it's 👍
|
@jhass @ysbaddaden done. Updated docs comment |
|
@ysbaddaden all green LGTM 🎉 |
| OpenSSL::SSL::Server.open tcp_server, server_context do |server| | ||
| spawn do | ||
| sleep 1 | ||
| OpenSSL::SSL::Socket::Client.open(TCPSocket.new(tcp_server.local_address.address, tcp_server.local_address.port), client_context, hostname: "example.com") do |socket| |
There was a problem hiding this comment.
Could you please split this line? Its complexity makes it hard to read.
There was a problem hiding this comment.
I can, keep in mind its a copy paste of the above tests, I just wanted to keep the same structure, maybe it should be better to merge this PR and make a new one fixing all the examples in the file which use this exact same line?
|
@straight-shoota can I get your approval? |
This PR adds the ability to query the SNI string, having this ability is important when needing to decide which certificate to show the connecting client, or, when creating a proxy that should show the relevant certificate for each host.
There are other reasons for having this ability.
Overall this change is minor and have been tested locally.