Skip to content
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.

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

net/imap: support SNI #2077

Open
wants to merge 1 commit into
base: trunk
from

Conversation

2 participants
@Keruspe
Copy link

Keruspe commented Feb 2, 2019

This fixes connecting using TLS 1.3 with openssl 1.1.1a to imap.gmail.com

See ruby/openssl#238

net/imap: support SNI
This fixes connecting using TLS 1.3 to imap.gmail.com

See ruby/openssl#238

Signed-off-by: Marc-Antoine Perennou <Marc-Antoine@Perennou.com>
@@ -1530,6 +1530,9 @@ def start_tls_session(params = {})
end
@sock = SSLSocket.new(@sock, context)
@sock.sync_close = true
unless hostname.nil?
@sock.hostname = hostname
end

This comment has been minimized.

@nevans

nevans Feb 27, 2019

For what it's worth, @host is already being used in this method (just a few lines down) without being passed in as an arg. I think it's safe to assume that it's set. So my patch (which I hadn't gotten around to submitting, so thanks!) simply adds the following line:

         context.verify_callback = VerifyCallbackProc
       end
       @sock = SSLSocket.new(@sock, context)
+      @sock.hostname = @host if context.verify_mode != VERIFY_NONE
       @sock.sync_close = true
       @sock.connect
       if context.verify_mode != VERIFY_NONE

This comment has been minimized.

@nevans

nevans Feb 27, 2019

Although I probably should not have made mine conditional on verify_mode, but should have instead checked to be sure host is not nil, as you did.

This comment has been minimized.

@nevans

nevans Feb 27, 2019

I notice that net/http always sets the address, conditional only on whether hostname= exists (which is I suppose for backwards compatibility with pre-SNI versions of OpenSSL). Probably best to follow that approach.
https://github.com/ruby/ruby/blob/9065336/lib/net/http.rb#L991

@Keruspe

This comment has been minimized.

Copy link
Author

Keruspe commented Feb 27, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.