[Pkg-silc-devel] Bug#482145: silc client segfaults if key is verified after disconnection

[Pekka Riikonen]

On Fri, 11 Jul 2008, Skywing wrote:

: All of the key confirmation prompts suffer from this flaw.  I've got a 
: fix for /getkey being broken, other than the lack of reference counting 
: for the SilcClient and SilcClientConnection objects.  There is nothing 
: that can really be done about the SilcClient object not being refcounted 
:
Both of you have stumbled to the fundamental problem of most asynchronous 
operations; the inability to control them.  That's why SILC's runtime 
provides the SILC Async Operation API (from 1.2 docs: 
http://srt.silc.fi/silcasync_hsilcutil2FAsync20Operation20Interface.html)

Here in a psuedo-code fashion is one way to fix this and all other prompt 
problems in SILC Client:

In this particular problem reported by Jérémy we can fix this by wrapping 
the keyboard_entry_redirect call to our own call that uses the 
SilcAyncOperation:

SilcAsyncOperation
my_keyboard_entry_redirect(SIGNAL_FUNC func,
                           SilcAsyncOperationAbort abort_callback,
                           const char *entry, int flags, void *data)
{
  SilcAsyncOperation op = silc_async_alloc(abort_callback, NULL, data);
  keyboard_entry_redirect(func, format, flags, data);
  return op;
}

The returned op would be saved to some location where it is globally 
available, say, inside SILC_SERVER_REC.  Some other context could be used 
too, but just for the purpose of this example let's use that.

To present user with the key verification prompt, we now would call:

  server->prompt_op = 
    my_keyboard_entry_redirect(verify_public_key_completion,
                               verify_public_key_abort,
                               format, 0, verify);

If the server connection is now closed or whatever happens that would 
invalidate the data inside the structure we used with the call above, we 
now call in whatever callback client library called to indicate the 
disconnection:

  silc_async_abort(server->prompt_op, NULL, NULL);

This will call the verify_public_key_abort which must now mark the 
operation aborted:

void verify_public_key_abort(SilcAsyncOperation op, void *context)
{
  PublicKeyVerify verify = context;
  verify->aborted = TRUE;
}

When the verify_public_key_completion is finally called when user actually 
answers the prompt, we check:

  if (verify->aborted) {
    /* We was aborted, do nothing */
    silc_free(verify);
    server->prompt_op = NULL;
    return;
  }

  /* We wasn't aborted, free operation and proceed */
  silc_async_free(server->prompt_op);
  server->prompt_op = NULL;

NULLing the operation is important to avoid using it accidentally.  Of 
course the operation context must be saved into some place that itself 
cannot be freed underneath the async call.  I'm not sure SILC_SERVE_REC is 
the correct place even though I used it in this example.

The SILC Client Library should use SilcAsyncOperations with all client ops 
it calls to application that take completion callbacks as argument so that 
the library itself could abort the client ops.  This however would require 
changing the SilcClientOperations API...

	Pekka
________________________________________________________________________
 Pekka Riikonen                                 priikone at silcnet.org
 Secure Internet Live Conferencing (SILC)       http://silcnet.org/