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
Handle tls connection during cancellation #227
Handle tls connection during cancellation #227
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will look more precisely a bit later, but this password
thing is absolutely necessary to fix
…o make use of the newly introduced fun, open_socket
…he epgsql_sock state
8826bac
to
0709950
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's fine now. Have some minor comments regarding the tests.
Self = self(), | ||
spawn_link(fun() -> | ||
?assertMatch(?QUERY_CANCELED, Module:equery(C, "SELECT pg_sleep(5)")), | ||
Self ! done |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
another option is to send the result to Self
and assert in receiver:
Self ! {done, Module:equery(C, "..")}
end,
<..>
receive {done, Result} ->
?assertMatch(?QUERY_CANCELLED, Result)
after ....
…uced tests, handle open_socket/x results better while cancelling the query
07cd20f
to
4ae561e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks fine. Thanks. Let's see if @davidw has some comments.
It would be nice if I could get some traction on this PR. |
Merged! Sorry for the slow response. |
Hi,
Looking at the
epgsql:cancel/x
it appears likeepgsql
opens a new temporary TCP connection and sends the cancellation request. This is fine for applications which uses normal non-TLS connections.However, if we are dealing with an application which uses TLS connections for all its queries except for the cancellation request then we might open ourselves to the man-in-the-middle-attack.
The attacker can in theory listen to our cancellation request and detect the previously generated secret key during the connection establishment of the original connection.
Once the attacker gets hold of that key while the original connection can still accept queries even after its last query was cancelled then the attacker can in theory cancel all the queries fired from the original connection.
This PR tries to address this issue.
I decided to rewrite part of
epgsql_cmd_connect
to better decouple opening a socket with the rest of the logic. This newly introduced function,open_socket/x
, was then called fromepgsql_sock.erl
during the cancellation.Please let me know if I missed something!