[OpenAFS] Heimdal KDC bug mentioned in rekeying document

[Ragnar Sundblad]

On 26 jul 2013, at 12:18, Sergio Gelato <Sergio.Gelato@astro.su.se> =
wrote:

> * Ragnar Sundblad [2013-07-26 11:43:57 +0200]:
>>=20
>> On 26 jul 2013, at 10:57, Sergio Gelato <Sergio.Gelato@astro.su.se> =
wrote:
>>=20
>>> Secondly, the following patch is required:
>>> --- a/kdc/kerberos5.c
>>> +++ b/kdc/kerberos5.c
>>> @@ -183,9 +183,10 @@
>>> 	    }
>>> 	}
>>> 	if (clientbest !=3D (krb5_enctype)ETYPE_NULL &&
>>> -	    enctype =3D=3D (krb5_enctype)ETYPE_NULL)
>>> +	    enctype =3D=3D (krb5_enctype)ETYPE_NULL) {
>>> 	    enctype =3D clientbest;
>>> -	else if (enctype =3D=3D (krb5_enctype)ETYPE_NULL)
>>> +	    ret =3D 0;
>>> +	} else if (enctype =3D=3D (krb5_enctype)ETYPE_NULL)
>>> 	    ret =3D KRB5KDC_ERR_ETYPE_NOSUPP;
>>> 	if (ret =3D=3D 0 && ret_enctype !=3D NULL)
>>> 	    *ret_enctype =3D enctype;
>>>=20
>>> I'll submit it to heimdal-bugs for consideration.
>>=20
>> I believe you should change the test to also check that ret_key =3D=3D =
NULL:
>>        if (clientbest !=3D ETYPE_NULL && enctype =3D=3D ETYPE_NUL && =
ret_key =3D=3D NULL) {
>>            enctype =3D clientbest;
>>            ret =3D 0;
>> 	}
>> since if there is no common key-type, key will be NULL, and the later
>>        if (ret =3D=3D 0 && ret_key !=3D NULL)
>>            *ret_key =3D key;
>> will return a NULL pointer.
>=20
> Yes, good point.

(Please double check that this is correct, I haven't tried it, only read =
it. :-)

>> Does your change really work as expected? (I am a bit surprised,
>> since in krb5tgs.c:tgs_build_reply() the result of the enctype is
>> ignored and the key is the one used (strangely!).
>=20
> What version of krb5tgs.c are you looking at? What I see is
>            ret =3D _kdc_find_etype(context,
>                                  krb5_principal_is_krbtgt(context, sp) =
?
>                                  config->tgt_use_strongest_session_key =
:
>                                  =
config->svc_use_strongest_session_key, FALSE,
>                                  server, b->etype.val, b->etype.len, =
&etype,
>                                  NULL);
> so ret_key (the last argument) is NULL.

Aha! I am looking at 1.5.2, and there it is:
...
                                  server, b->etype.val, b->etype.len, =
NULL,
	                          &skey);

What version are you using?

> This is also consistent with my understanding that the session key is
> randomly generated by the KDC at the time of printing the ticket; it
> should be unrelated to any long-term keys.

It does generate a random key in 1.5.2 too, but the enctype of the
session key is taken from the enctype of "skey" above, instead of
from "etype" in your excerpt.

(
krb5tgs.c 1.5.2:
...
            etype =3D skey->key.keytype;
...
        ret =3D krb5_generate_random_keyblock(context, etype, =
&sessionkey);
...
)

> As for whether my change works, I'll admit that my testing was limited =
to
> verifying that I could get a service ticket (with AES for the ticket =
and
> DES for the session key) and a token with 1.6.4's aklog. Checking that =
the
> token is actually acceptable to AFS servers will come next.

Ok, that is quite good! (As I read it in 1.5.2 I think you shouldn't
have gotten that far at all, I think...)

Thanks for your work!

/ragge