Skip to content

Report better accept handshake errors instead of "General SSLEngine problem" #37

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

Merged

Conversation

driskell
Copy link
Contributor

@driskell driskell commented Apr 3, 2015

Myself and some users of my program have been getting errors sometimes when trying to accept connections.

Within ruby, the error appears as OpenSSL::SSL::SSLError, "General SSLEngine problem".

After some lengthy diagnosis it turns out the problem is the client was sending a client certificate. On the server side, the default verify_mode is VERIFY_PEER. Thus, with no way to verify it, it cannot handshake. But rather than saying "certificate verify failed" in the accept exception, it says "General SSLEngine problem".

Looking in the java SSL and jruby-openssl code it seems the SSL stack throws in different ways and sometimes "loses" the diagnostic message (certificate verify failed). In jruby-openssl when calling connect this appears to be handled... it traces back through the exception causes to the first non-SSLHandshakeException exception, and raises a ruby exception based on that.

This patch modifies accept so it does the same as connect. This fixes my issue and in my tests my program now correctly reports "certificate verify failed".

Thanks!

kares added a commit that referenced this pull request Apr 4, 2015
…ine_problem

Report better accept handshake errors instead of "General SSLEngine problem"
@kares kares merged commit 62926a0 into jruby:master Apr 4, 2015
@kares
Copy link
Member

kares commented Apr 4, 2015

Excellent, thank you Jason!

@driskell driskell deleted the improve_handshake_general_sslengine_problem branch April 4, 2015 10:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants