[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: Banning TLS renegotiation



On Tue, Oct 03, 2017 at 12:21:32PM +0100, Richard W.M. Jones wrote:
> On Tue, Oct 03, 2017 at 12:08:36PM +0100, Daniel P. Berrange wrote:
> > On Tue, Oct 03, 2017 at 11:32:56AM +0100, Richard W.M. Jones wrote:
> > > I implemented TLS support in nbdkit last week.
> > > 
> > > As part of this I came across an unusual corner of the TLS protocol
> > > called ‘renegotiation’, where — in mid-stream — either peer can
> > > request at the TLS level that the TLS handshake is performed again.
> > > 
> > > I found a good explanation of why you might want to do this in the
> > > context of web servers here:
> > > 
> > >   https://security.stackexchange.com/questions/24554/should-i-use-ssl-tls-renegotiation/24569#24569
> > > 
> > > I don't think there is any case where NBD can renegotiate the
> > > exportname mid-stream, so this does not apply to NBD.
> > > 
> > > either qemu (client or server) nor nbdkit (server) implement this.  I
> > > didn't check other implementations in detail, but I would note that
> > > with GnuTLS renegotiation is *not* handled transparently.  What
> > > happens is that any call to gnutls_record_recv can return
> > > GNUTLS_E_REHANDSHAKE and the peer must decide either to perform the
> > > handshake (calling gnutls_rehandshake[1]) or to drop the connection.
> > > So doing this would complicate the implementation of NBD clients and
> > > servers.
> > > 
> > > As this feature is not needed, I think the NBD protocol doc should
> > > explicitly ban it, or at least say that conforming servers and clients
> > > are allowed to immediately terminate peers which try to perform
> > > renegotiation.
> > 
> > In TLS any client or server supporting renegotiation already has to
> > deal with fact that their peer might reject the request to renegotiate. 
> > There is defined behaviour for TLS / GNUTLS about how to reject such a
> > request in a graceful manner.
> 
> Even to deny a handshake, the server must check every call to
> gnutls_record_recv for GNUTLS_E_REHANDSHAKE and send an alert
> (gnutls_alert_send (GNUTLS_A_NO_RENEGOTIATION)).  Plus the client also
> has to be modified to handle alerts (ie. checking every return code
> for GNUTLS_E_WARNING_ALERT_RECEIVED and handling it).
> 
> So this could be complex for an implementation.  Remember that when
> this happens we're somewhere deep in the stack in the data phase of
> the connection.
> 
> What is the client trying to achieve by renegotiating?  Is it ever a
> valuable thing to do?
> 
> > So I think defining that the peer is allowed to terminate the connection
> > is not required or desirable from NBD POV
> > 
> > At most I would think that we could say
> > 
> >   "NBD servers and clients should not assume that the peer will
> >    support TLS renegotiation"
> > 
> > if they wish to try renegotiation anyway, leave it upto the TLS library
> > and app code as to what happens upon rejection.
> 
> nbdkit will currently drop the connection abruptly if the client
> tries this, and I think this should be conforming behaviour.

I would say

  "a peer  SHOULD follow the TLS protocol spec for accepting
   or rejecting a renegotiation, but MAY close the connection
   abruptly"

to encourage TLS compliant behaviour but be realistic about the fact that
no impl currently does that.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|


Reply to: