[Vortex] TLS Failure Segfault

[Francis Brosnan Blazquez]

Hi Jason,

Thanks for the info and sorry about the delay in the reply.

This is now fixed (svn 5014) with a reg test that was able to reproduce
the problem and test the fix introduced (test_05a1). 

Now the action handler checks the connection id in search of changes, so
it returns -1 (reuse reference) or 2 (update reference) to ensure the
caller always get a valid VortexConnection object (no matter if it is
connected or not). 

Thanks for the diagnostics, 
Best Regards,

> Francis,
> 
> Thank you for your fast response!
> 
> > Hi Jason,
> > 
> >> When utilizing vortex_tls_set_auto_tls and then vortex_connection_new,
> >> should the connection returned be the newly created TLS attempted
> >> connection?  It appears we are receiving the original, non-TLS,
> >> connection back.
> > 
> > Ok, it depends on where the failure happens. It if is before TLS BEEP
> > handshake finishes (request for tuning reset), then the original
> > connection is returned. If it during the TLS handshake it self, then the
> > new connection is returned. 
> > 
> > But if it happens after these both steps (greetings not accepted after
> > TLS is running), in such case, the new connection is returned too.
> > 
> > In all these cases, vortex_connection_is_ok (conn, axl_true/axl_false)
> > should work without problem :-?
> > 
> > In your case, where happens the problem?
> > 
> > Are you allowing tls failures (allow_tls_failures param) at
> > vortex_tls_set_auto_tls?
> 
> TLS failures are not being allowed.  The failure happens in function vortex_tls_invoke_tls_activation, stage 6.1 it appears via the logs.  At this point, the original connection has been unref'd and destroyed.  The function returns and, tracing it back, gets to vortex_tls_auto_tlsfixate_connection where this piece of code lies:
> 
>                 connection = vortex_tls_start_negotiation_sync (connection,              
>                                                                 tls_ctx->connection_auto_tls_server_name,
>                                                                 NULL, NULL);             
>                 if (!vortex_connection_is_ok (connection, axl_false)) {
>                         return -1;
>                 }       
> 
> connection is a function argument pointer and will not retain this value when the function returns.  If the TLSification were successful then this happens:
> 
>                 (*new_conn) = connection;
> 
> Upon returning to the calling function, vortex_connection_actions_notify, in which case this block of code gets hit:
> 
>                        case -1:
>                                 if (vortex_connection_is_ok (conn, axl_false))
>                                         __vortex_connection_set_not_connected (conn,
>                                                                                "connection action failed, closing session",
>                                                                                VortexConnectionCloseCalled);
>                                 /* unlock mutex */
>                                 vortex_mutex_unlock (&ctx->connection_actions_mutex);
>                                 return axl_false;
> 
> The call to vortex_connection_is_ok is invalid at this point since the conn is essentially trash.  If the TLS had passed, caller_conn and conn get assigned the new_conn which assigned during the action() call.  Since the caller_conn and conn never get assigned the new_conn (which is now the TLS connection), the connection is lost and the already destroyed connection pointer continues to be passed around.
> 
> Please let me know if I am erroneously following the code or misinterpreting it.
> 
> >>> 2) The patch proposes to move the unref code until the end, but that
> >>>  will cause leaks due to all wrong branches before (wrong
> >> greetings,  
> >>>  TLS handshake failure, ....) where the reference is not finished.
> >> 
> >> My apologies, upon closer inspection of the valgrind output it does
> >> appear that when segfaulting and my patch have the same memory leaks.
> >> This appears to be due to the TLS connection being lost.
> >> 
> >>> Just a question about your description. What's the connection being
> >>> checked with vortex_connection_is_ok (conn, axl_true) after tls
> >> failure?
> >>> The one returned by the tls activation or the starting connection?
> >>> 
> >>> You must forget about the starting connection when you call to
> >> activate
> >>> TLS because that function may replace your reference, even after
> >> failure
> >>> (something may go wrong after TLS finished right, for example BEEP
> >>> session doesn't agree)....
> >> 
> >> How should the connection be checked?  
> > 
> > Using vortex_connection_is_ok, without exception.
> > 
> >> Rather, how should the new, failed, TLS connection be acquired?  I
> >> cannot seem to locate a method that will retrieve the failed TLS
> >> connection that has been created.  When the TLS connection succeeds,
> >> the connection returned is the TLS-ificated connection, but not when
> >> it fails.
> > 
> > In both cases you get the connection that replaces the old, so there is
> > no specific method to get the connection when a failure happens. 
> > 
> > The case you describe is covered by several reg tests, especially
> > test-05-a [1], that's why I don't see how you get a wrong reference
> > connection on failure.
> > 
> > Maybe you can provide a valgrind trace showing relevant information or a
> > test case to reproduce the problem...
> 
> I will work on getting you a small test example.  Until then, the valgrind output is attached.
> 
> The first "Invalid read of size 4" block references a call to vortex_connection_is_ok within vortex_connection_actions_notify which is operating on the already destroyed connection.
> 
> Thanks again!
> 
> Jason
> 

-- 
Francis Brosnan Blázquez <francis.brosnan at aspl.es>
ASPL
91 134 14 22 - 91 134 14 45 - 91 116 07 57

AVISO LEGAL

Este mensaje se dirige exclusivamente a su destinatario. Los datos
incluidos en el presente correo son confidenciales y sometidos a secreto
profesional, se prohíbe divulgarlos, en virtud de las leyes vigentes. Si
usted no lo es y lo ha recibido por error o tiene conocimiento del mismo
por cualquier motivo, le rogamos que nos lo comunique por este medio y
proceda a destruirlo o borrarlo.

En virtud de lo dispuesto en la Ley Orgánica 15/1999, de 13 de
diciembre, de Protección de Datos de Carácter Personal, le informamos de
que sus datos de carácter personal, recogidos de fuentes accesibles al
público o datos que usted nos ha facilitado previamente, proceden de
bases de datos propiedad de Advanced Software Production Line, S.L.
(ASPL). No obstante, usted puede ejercitar sus derechos de acceso,
rectificación, cancelación y oposición dispuestos en la mencionada Ley
Orgánica, notificándolo por escrito a:
ASPL - Protección Datos, C/Antonio Suárez 10 A-102, 28802, Alcalá de
Henares (Madrid).