bug with SGN_ALG_MD2_5 case handling in kg_unseal_v1()?

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

bug with SGN_ALG_MD2_5 case handling in kg_unseal_v1()?

Will Fiveash-2
In src/lib/gssapi/krb5/k5unseal.c:kg_unseal_v1() at line 381 which is
part of the case SGN_ALG_MD2_5 block I see:

        code = k5_bcmp(md5cksum.contents, ptr + 14, 8);
        /* Falls through to defective-token??  */

    default:
        *minor_status = 0;
        return(GSS_S_DEFECTIVE_TOKEN);

This seems like a bug given the processing that precedes this, thoughts?

--
Will Fiveash
Oracle Solaris Software Engineer
_______________________________________________
krbdev mailing list             [hidden email]
https://mailman.mit.edu/mailman/listinfo/krbdev
Reply | Threaded
Open this post in threaded view
|

Re: bug with SGN_ALG_MD2_5 case handling in kg_unseal_v1()?

Benjamin Kaduk-2
On Thu, Apr 13, 2017 at 02:55:56PM -0500, Will Fiveash wrote:

> In src/lib/gssapi/krb5/k5unseal.c:kg_unseal_v1() at line 381 which is
> part of the case SGN_ALG_MD2_5 block I see:
>
>         code = k5_bcmp(md5cksum.contents, ptr + 14, 8);
>         /* Falls through to defective-token??  */
>
>     default:
>         *minor_status = 0;
>         return(GSS_S_DEFECTIVE_TOKEN);
>
> This seems like a bug given the processing that precedes this, thoughts?

Perhaps.  On the other hand, how much do you trust anything with MD2
in its name...

-Ben
_______________________________________________
krbdev mailing list             [hidden email]
https://mailman.mit.edu/mailman/listinfo/krbdev
Reply | Threaded
Open this post in threaded view
|

Re: bug with SGN_ALG_MD2_5 case handling in kg_unseal_v1()?

Greg Hudson
In reply to this post by Will Fiveash-2
On 04/13/2017 03:55 PM, Will Fiveash wrote:

> In src/lib/gssapi/krb5/k5unseal.c:kg_unseal_v1() at line 381 which is
> part of the case SGN_ALG_MD2_5 block I see:
>
>         code = k5_bcmp(md5cksum.contents, ptr + 14, 8);
>         /* Falls through to defective-token??  */
>
>     default:
>         *minor_status = 0;
>         return(GSS_S_DEFECTIVE_TOKEN);
>
> This seems like a bug given the processing that precedes this, thoughts?

It does look like a bug.  The code's been like that since 1.0; the
comment was added in 1.2.  It would seem that unwrapping tokens using
SGN_ALG_MD2_5 never worked, and we could probably get rid of the code.
Neither our code nor Heimdal's appears to choose SGN_ALG_MD2_5 when
creating tokens, by my reading.

(According to RFC 1964, "MD2.5" refers to using half of the 128-bit MD5
checksum; it doesn't actually reference the MD2 hash algorithm.)
_______________________________________________
krbdev mailing list             [hidden email]
https://mailman.mit.edu/mailman/listinfo/krbdev
Reply | Threaded
Open this post in threaded view
|

Re: bug with SGN_ALG_MD2_5 case handling in kg_unseal_v1()?

Will Fiveash-2
On Fri, Apr 14, 2017 at 02:12:47AM -0400, Greg Hudson wrote:

> On 04/13/2017 03:55 PM, Will Fiveash wrote:
> > In src/lib/gssapi/krb5/k5unseal.c:kg_unseal_v1() at line 381 which is
> > part of the case SGN_ALG_MD2_5 block I see:
> >
> >         code = k5_bcmp(md5cksum.contents, ptr + 14, 8);
> >         /* Falls through to defective-token??  */
> >
> >     default:
> >         *minor_status = 0;
> >         return(GSS_S_DEFECTIVE_TOKEN);
> >
> > This seems like a bug given the processing that precedes this, thoughts?
>
> It does look like a bug.  The code's been like that since 1.0; the
> comment was added in 1.2.  It would seem that unwrapping tokens using
> SGN_ALG_MD2_5 never worked, and we could probably get rid of the code.
> Neither our code nor Heimdal's appears to choose SGN_ALG_MD2_5 when
> creating tokens, by my reading.
>
> (According to RFC 1964, "MD2.5" refers to using half of the 128-bit MD5
> checksum; it doesn't actually reference the MD2 hash algorithm.)

It would be good to delete that code as it was flagged during recent
code checking for improper case fall-throughs.

--
Will Fiveash
Oracle Solaris Software Engineer
_______________________________________________
krbdev mailing list             [hidden email]
https://mailman.mit.edu/mailman/listinfo/krbdev
Reply | Threaded
Open this post in threaded view
|

Re: bug with SGN_ALG_MD2_5 case handling in kg_unseal_v1()?

Will Fiveash-2
In reply to this post by Benjamin Kaduk-2
On Thu, Apr 13, 2017 at 08:09:23PM -0500, Benjamin Kaduk wrote:

> On Thu, Apr 13, 2017 at 02:55:56PM -0500, Will Fiveash wrote:
> > In src/lib/gssapi/krb5/k5unseal.c:kg_unseal_v1() at line 381 which is
> > part of the case SGN_ALG_MD2_5 block I see:
> >
> >         code = k5_bcmp(md5cksum.contents, ptr + 14, 8);
> >         /* Falls through to defective-token??  */
> >
> >     default:
> >         *minor_status = 0;
> >         return(GSS_S_DEFECTIVE_TOKEN);
> >
> > This seems like a bug given the processing that precedes this, thoughts?
>
> Perhaps.  On the other hand, how much do you trust anything with MD2
> in its name...

Indeed.

--
Will Fiveash
Oracle Solaris Software Engineer
_______________________________________________
krbdev mailing list             [hidden email]
https://mailman.mit.edu/mailman/listinfo/krbdev