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

Improve SSL closing sequence, fix for #393 #400

Merged
merged 3 commits into from Jul 29, 2013

Conversation

Projects
None yet
2 participants
@jrudolph
Member

jrudolph commented Jul 26, 2013

No description provided.

jrudolph added some commits Jul 26, 2013

= io: test for invalid connection closure for SSL connections, refs #393


 * tests that try to more closely make sure that the messages are sent
   in proper order
 * added tests for checking that no sessions are lost by broken SSL
   closure as this is the most important consequence of wrong SSL
   closing behaviour
@jrudolph

This comment has been minimized.

Show comment
Hide comment
@jrudolph

jrudolph Jul 26, 2013

Member

One thing to keep in mind is that there's no support for half-closed connections in SSL (but SSL on the other side requires half-closed connections from its transport layer). This means

  1. keepOpenOnPeerClosed is not supported on top of SSL (once you receive PeerClosed the connection is closed)
  2. keepOpenOnPeerClosed should always be enabled on the transport layer beneath SSL so that one can wait for the other side's SSL level close_notify message without barfing RST to the peer because this socket is already gone
Member

jrudolph commented Jul 26, 2013

One thing to keep in mind is that there's no support for half-closed connections in SSL (but SSL on the other side requires half-closed connections from its transport layer). This means

  1. keepOpenOnPeerClosed is not supported on top of SSL (once you receive PeerClosed the connection is closed)
  2. keepOpenOnPeerClosed should always be enabled on the transport layer beneath SSL so that one can wait for the other side's SSL level close_notify message without barfing RST to the peer because this socket is already gone
= io: improve SSL closing sequence, fixes #393
Changes:
  * when user wants to close connection, don't do so immediately but
    let SSLEngine drive the decision (encrypt -> CLOSED)
  * also when peer closes connection on SSL level (decrypt -> CLOSED),
    don't warn (and close underlying transport) but follow the usual
    connection closing path
@sirthias

This comment has been minimized.

Show comment
Hide comment
@sirthias

sirthias Jul 29, 2013

Member

In order to ensure proper keepOpenOnPeerClosed behavior this patch requires #401

Member

sirthias commented Jul 29, 2013

In order to ensure proper keepOpenOnPeerClosed behavior this patch requires #401

sirthias added a commit that referenced this pull request Jul 29, 2013

@sirthias sirthias merged commit 02b95a2 into spray:master Jul 29, 2013

1 check passed

default The Travis CI build passed
Details

octo47 pushed a commit to octo47/akka that referenced this pull request Nov 9, 2013

=act #3389 Improve SSL closing sequence
* Port of this fix in Spray by @jrudolph from this pull request spray/spray#400

sourcedelica pushed a commit to sourcedelica/akka that referenced this pull request Nov 23, 2013

=act #3389 Improve SSL closing sequence
* Port of this fix in Spray by @jrudolph from this pull request spray/spray#400
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment