Re: scram-sha-1-plus

[Simon Josefsson]

Hi!  Before 1.10.0 I thought we should resolve this one way or the
other.  I'm not sure how to best deal with it...

Jeremy Harris <address@hidden> writes:

> Problem:
>   gsasl_client_start(gsasl_ctx , "SCRAM-SHA-1-PLUS", &sctx)
>
> fails with GSASL_NO_CB_TLS_UNIQUE.  But you can't provide that
> prop until you have the sctx.

The idea was to provide it in the callback.  The TLS tls-unique data is
available after the TLS handshake finishes, which is before
gsasl_client_start() happens.

The reason to ask for tls-unique data in gsasl_client_start() is that a
successful start() of a mechanism is a sign that it could be selected if
the server supports it.  If the client does not have a tls-unique value,
it should not select SCRAM-SHA-1-PLUS but instead go with SCRAM-SHA-1.
This logic is in gsasl_client_suggest_mechanism().

> What is the expected sequence of calls?

  res = gsasl_init (&ctx);
  gsasl_callback_set (ctx, callback);
  gsasl_callback_hook_set (ctx, &global_application_state)
...
  strcpy (global_application_state->tls_unique, "foo"); /* from TLS library */
  res = gsasl_client_start (ctx, "SCRAM-SHA-256-PLUS", &client);
  /* during gsasl_client_start() the callback is invoked that set the
     GSASL_CB_TLS_UNIQUE property */
  res = gsasl_step (client, s1, s1len, &s1, &s1len);
...

static int
callback (Gsasl * ctx, Gsasl_session * sctx, Gsasl_property prop)
{
  struct global_application_struct *info = gsasl_callback_hook_get();

  switch (prop)
    {
...
    case GSASL_CB_TLS_UNIQUE:
      gsasl_property_set (sctx, prop, info->tls_unique);
      return GSASL_OK;

> As far as I can see from the example utility code, you have to
> provide _a_ prop during some callbacks that happen when the
> callback is first provided, back at just after gsasl_init()
> is called.
>
> The prop is then presumably set on some irrelevant sctx, which
> happens by luck to still be around during the processing of
> gsasl_client_start().

No -- the sctx you get in the callback is the sctx that will be used for
the authentication attempt that is being start()ed.

However you are right that there is a problem hidden here: consider if
the application is threaded and has one global gsasl_init() call and a
global callback that can be invoked from multiple threads doing
gsasl_client_start() at the same time.  Then it will be unpredictable
which thread wrote last to the info->tls_unique buffer above.  This is
bad design...   Some solutions:

1) Every thread will have to have a separate gsasl_init() call and
library handle.  This wastes resources.

2) Every thread will have to mutex the call to gsasl_client_start() like
this:

  pthread_mutex_start...()
  strcpy (info->tls_unique, our_tls_unique());
  gsasl_client_start ()...
  pthread_mutex_stop...()

this will guarantee that the callback will be using the tls-unique data
from the right TLS channel which it is going to start SASL on.

3) Redesign gsasl somehow.

It is late and I'm not sure there actually is a problem for non-threaded
applications.  Is there really any problem?  The application needs to
set the tls-unique data in the globl buffer available to the callback
just before the gsasl_client_start() call but if it does that it should
be guaranteed to work.

Happy to hear your thoughts on this, if you are able to understand my
understanding of the situation...

/Simon

signature.asc
Description: PGP signature