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

start_tls infinite loops if alert is sent during handshake #2635

Open
jcrist opened this issue Mar 28, 2019 · 1 comment
Open

start_tls infinite loops if alert is sent during handshake #2635

jcrist opened this issue Mar 28, 2019 · 1 comment
Labels

Comments

@jcrist
Copy link

jcrist commented Mar 28, 2019

I have a TLS proxy server that uses the SNI in the handshake to do the routing. When the SNI specified isn't found, alert 112 (Unrecognized name) is sent as a fatal alert, and the connection is closed (relevant code: https://github.com/jcrist/dask-gateway/blob/master/dask-gateway-proxy/schedulerproxy.go#L164-L168).

Everything works fine if the SNI name is valid, but in the case of an unrecognized SNI, tornado infinite loops when awaiting on start_tls. I've debugged this down to Python's ssl implementation raising SSLErrorWantRead, which causes tornado to retry (https://github.com/tornadoweb/tornado/blob/master/tornado/iostream.py#L1392-L1394). I'm not sure why that error is being raised on the alert - it may be a problem with my server code, but I've checked things with both a go client and openssl s_client and things seem to work fine there.

Apologies for not providing a minimal reproducible issue here - figured I'd see if anyone had any ideas on this before spending time making a minimal issue.

@bdarnell
Copy link
Member

I haven't seen anything like this. My understanding is that TLS alerts are supposed to turn into errors other than SSLWant{Read,Write}Error, which suggests that this is a bug in the error handling of the cpython ssl library.

Or it could be related to #2533 - we've seen that recent changes in openssl (related to TLS 1.3) have caused some of Tornado's assumptions about the ordering of messages, and this can lead to hangs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants