[PATCH] SSL: shutdown cleanly when other endpoint starts shutdown

[Maxim Dounin]

Hello!

On Mon, Dec 07, 2015 at 02:38:14PM -0800, Judson Wilson wrote:

> > There is a problem with this change: in most cases in practice
> > close from a client means that the socket is no longer open, and
> > trying to send a close_notify alert won't do anything good but
> > will result in RST instead.  Common practice seems to be don't
> > send close_notify alerts in such a case (e.g., I see Chrome doing
> > the same), and section 7.2.1 mentions this as well.
> 
> The behavior I am attempting to produce matches what I am
> observing in the Apache web server (assuming default
> configuration), so it's not unheard of for HTTPS.

For sure it's not something unheard.  The question is mostly about 
balance of positive and negative results in a particular code 
path.  In this particular case it looks like nginx behaviour is 
good enough.

> > If you still think that changes in this area are needed - please
> > provide some more details on what you are trying to fix.
> 
> I am researching ways of auditing TLS communications
> in a read-only way. One method of achieving this is by having the TLS
> client give the keyblock to an auditor after the client is sure that the
> server
> will not accept any more data protected by the keys (assume the software
> for the client and server are controlled by the same party). From my
> investigation of nginx and apache, if a client receives close_notify,
> they can be assured that no more data will be accepted by the server
> under the key (both web server software packages immediately
> destroy the SSL object after calling SSL_shutdown).
> 
> Unfortunately, nginx has this case where a client would attempt to
> close a connection, but never get close_notify in response.
> It seems like simply obeying the TLS standard would fix this.
> (close_notify IS sent on "connection: close" header or keepalive
> timeout as I would expect.)

As far as I understand, just looking for TCP FIN should be good 
enough for this task.

> > Note well that section 7.2.1 you refer to is about avoiding
> > truncation attacks.  On the other hand, the code path you are
> > editing doesn't ensure that a close_notify notify alert was
> > received from the other side.  That is, sending a close_notify
> > alert will make truncation attacks easier, not stop them.
> 
> Yes truncation attacks were not on my mind - that's a different
> problem.
> 
> I'd be happy to look into this a further if you think this work has
> a chance of being incorporated.

I don't think that this change is needed, especially given the 
reasons for the change, and potentical security implications of 
the proposed patch.

-- 
Maxim Dounin
http://nginx.org/