[Vortex] TLS Failure Segfault

[Jason Dana]

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

-------------- next part --------------
A non-text attachment was scrubbed...
Name: valgrind.out
Type: application/octet-stream
Size: 52993 bytes
Desc: valgrind.out
URL: <http://lists.aspl.es/pipermail/vortex/attachments/20120528/ad4b80d2/attachment-0001.obj>
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: ATT00001.txt
URL: <http://lists.aspl.es/pipermail/vortex/attachments/20120528/ad4b80d2/attachment-0001.txt>