help-gsasl
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 1/2] _gsasl_gssapi_server_step: don't overwrite maj_stat


From: Andreas Oberritter
Subject: Re: [PATCH 1/2] _gsasl_gssapi_server_step: don't overwrite maj_stat
Date: Tue, 18 Oct 2011 17:07:19 +0200
User-agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.2.23) Gecko/20110922 Lightning/1.0b2 Thunderbird/3.1.15

Hi Simon,

On 18.10.2011 15:26, Simon Josefsson wrote:
> Hi Andreas.  Thank you!  The code definitely looks buggy.  I'm not
> convinced this is exploitable though since GSSAPI means Kerberos V5 and
> the code will in the next steps unwrap/wrap a token which will fail
> unless the context hasn't been negotiated successfully.  Do you agree,
> or do you have some other analysis?
> 
> Your patch makes a direct comparison against GSS_S_COMPLETE which is
> poor style in general, and using GSS_ERROR is preferred.  How about the
> following patch instead?

I considered your proposal before, but dismissed it, because if
gss_release_buffer() fails, sasl->step would already have been
incremented. I'm not sure whether this would be a problem, though.

Because gss_release_buffer() doesn't allow any other successful return
value than GSS_S_COMPLETE, I decided that it would be sufficient to
compare with this value.

Btw.: I'm implementing a custom authentication mechanism on top of
GSSAPI. Even though Kerberos is probably the only public user of GSSAPI,
more unknown users might exist out there.

Regards,
Andreas

> diff --git a/lib/gssapi/server.c b/lib/gssapi/server.c
> index dc05a6f..2cedc2a 100644
> --- a/lib/gssapi/server.c
> +++ b/lib/gssapi/server.c
> @@ -168,13 +168,13 @@ _gsasl_gssapi_server_step (Gsasl_session * sctx,
>        memcpy (*output, bufdesc2.value, bufdesc2.length);
>        *output_len = bufdesc2.length;
>  
> +      if (maj_stat == GSS_S_COMPLETE)
> +     state->step++;
> +
>        maj_stat = gss_release_buffer (&min_stat, &bufdesc2);
>        if (GSS_ERROR (maj_stat))
>       return GSASL_GSSAPI_RELEASE_BUFFER_ERROR;
>  
> -      if (maj_stat == GSS_S_COMPLETE)
> -     state->step++;
> -
>        res = GSASL_NEEDS_MORE;
>        break;
>  




reply via email to

[Prev in Thread] Current Thread [Next in Thread]