[vlc-devel] [PATCH] gnutls: disable False Start

[Hugo Beauzée-Luyssen]

Hi,

On Thu, Feb 21, 2019, at 7:38 PM, Rémi Denis-Courmont wrote:
> Le torstaina 21. helmikuuta 2019, 20.10.25 EET Thomas Guillem a écrit :
> > Here is how I understand the GnuTLS documentation about thread safety,
> > handshake and GNUTLS_ENABLE_FALSE_START:
> > 
> > cf. https://gnutls.org/manual/html_node/Thread-safety.html
> > "...care must be taken during key updates and re-handshakes to be handled
> > only by a single thread"
> 
> As I already mentioned, VLC does not do re-handshakes on client side. " In 
> case of a client, [re-handshake] message may be simply ignored". And re-
> handshakes happen in already established sessions, while False Start occurs at 
> start. They are not related.
> 

Re-handshake is not the problem here, but I think we agree.

> > GNUTLS_ENABLE_FALSE_START causes the handshake to be delayed when receiving
> > data.
> 
> No. False Start causes the handshake to terminate early (before the session is 
> securely established):
> https://www.gnutls.org/manual/html_node/False-Start.html
> 

That is not my understanding. From what I can see false start will update a state in the gnutls' session. 
That state is read latter on during send/recv.
The said state isn't updated in an atomic way, which causes a race during the first send/receive.

See https://gitlab.com/gnutls/gnutls/issues/713 for the tsan traces.

> Sending is still done from within gnutls_record_send() and receiving from 
> within gnutls_record_recv(), not affecting the polling and threading semantics. 
> Otherwise, it would wreck event handling even in single thread mode. It has 
> indeed been working fine with GnuTLS 3.5.
> 

The commit exhibiting the race wasn't there in 3.5.x, but the race itself did.

> Contribs needs to be reverted back to a non-broken GnuTLS version, or patched.
> 

I hope to find a solution with the GnuTLS people. I'm fine with a temporary patch reverting https://gitlab.com/gnutls/gnutls/commit/6a62ddfc416a4ec2118704f93c97fdd448d66566 in our contribs meanwhile.

Thomas' patches would also fix the issue in a more reliable way, until a better fix gets pushed to GnuTLS.

Regards,

-- 
  Hugo Beauzée-Luyssen
  hugo at beauzee.fr