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

Don't leak open connections when the TLS handshake fails #383

Merged
merged 1 commit into from
Jul 10, 2017
Merged

Don't leak open connections when the TLS handshake fails #383

merged 1 commit into from
Jul 10, 2017

Conversation

coekie
Copy link
Contributor

@coekie coekie commented Jul 3, 2017

Context: Our client is setup to keep retrying to connect to our neo4j server. When the client could not validate the certificate of the server, this leak caused it to take up all available tcp connections.

Example stacktrace of an exception where the connection was leaked:

Caused by: org.neo4j.driver.v1.exceptions.SecurityException: Failed to establish secured connection with the server: General SSLEngine problem
	at org.neo4j.driver.internal.security.TLSSocketChannel.create(TLSSocketChannel.java:85)
	at org.neo4j.driver.internal.security.TLSSocketChannel.create(TLSSocketChannel.java:73)
	at org.neo4j.driver.internal.net.ChannelFactory.create(ChannelFactory.java:47)
	at org.neo4j.driver.internal.net.SocketClient.start(SocketClient.java:124)
	at org.neo4j.driver.internal.net.SocketConnection.startSocketClient(SocketConnection.java:89)
	at org.neo4j.driver.internal.net.SocketConnection.<init>(SocketConnection.java:64)
	at org.neo4j.driver.internal.net.SocketConnector.createConnection(SocketConnector.java:77)
	at org.neo4j.driver.internal.net.SocketConnector.connect(SocketConnector.java:50)
       ...
Caused by: javax.net.ssl.SSLHandshakeException: General SSLEngine problem
	at sun.security.ssl.Handshaker.checkThrown(Handshaker.java:1478)
	at sun.security.ssl.SSLEngineImpl.checkTaskThrown(SSLEngineImpl.java:535)
	at sun.security.ssl.SSLEngineImpl.writeAppRecord(SSLEngineImpl.java:1214)
	at sun.security.ssl.SSLEngineImpl.wrap(SSLEngineImpl.java:1186)
	at javax.net.ssl.SSLEngine.wrap(SSLEngine.java:469)
	at org.neo4j.driver.internal.security.TLSSocketChannel.wrap(TLSSocketChannel.java:319)
	at org.neo4j.driver.internal.security.TLSSocketChannel.runHandshake(TLSSocketChannel.java:136)
	at org.neo4j.driver.internal.security.TLSSocketChannel.create(TLSSocketChannel.java:81)
        ...

@lutovich
Copy link
Contributor

lutovich commented Jul 3, 2017

Hi @coekie,

Thanks for this fix! Changes look good to me.

To be able to merge them we will need a signed CLA from you.
See http://neo4j.com/developer/cla/ for further information.
Additional information on contributing code is found at
http://neo4j.com/developer/contributing-code/

Build is red because of missing CLA.

@zhenlineo
Copy link
Contributor

@coekie I did not find any CLA from you. Would you update us when you have sent a signed CLA?

@coekie
Copy link
Contributor Author

coekie commented Jul 7, 2017

@zhenlineo sorry that took a bit, had to get it checked by the legal team here first. I just sent the CLA acceptance email.

@lutovich
Copy link
Contributor

@coekie thanks a lot for this fix!

@lutovich lutovich merged commit 8a50285 into neo4j:1.4 Jul 10, 2017
@lutovich
Copy link
Contributor

@coekie your fix has been released in version 1.4.2 which should now be available on maven central. Thanks again.

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.

3 participants