On Mon, 2008-09-29 at 12:38 -0400, Peter Shoults wrote:
...
> >> usr/src/lib/gss_mechs/mech_krb5/krb5/krb/chk_trans.c
> >>
> >> What is that on line 360 of your modified code?
> >>
> >
> > CTRL-l. Linefeed (I think).
> >
> Weird - should it be there?
Don't think it causes any problems. May affect how its printed. MIT has
it so I don't see a reason to differ here.
...
> >> Actually,
> >> it looks like a lot of the returns in this routine
> >> do not call krb5_free_error(context,krberror) and also
> >> krb5_xfree(clearresult.data).
> >>
> >
> > It looks mostly correct to me. I believe I see a possible mem-leak.
> > Fixed.
> >
> Good - letting MIT know about that?
Yes, I plan on giving back any useful mods to MIT.
> >
> >>
> >> --------------
> >>
> >> usr/src/lib/gss_mechs/mech_krb5/krb5/krb/copy_addrs.c
> >>
> >> Should there be a /* Solaris kerberos */ comment
> >> above line 45 and possibly a Sun Copyright?
> >>
> >
> > The only difference I see between MIT's copy_addrs.c and our
> > copy_addrs.c is on line 32 - and its just a lint comment.
> > Can you elaborate?
> >
> >
> It looked like in other files, the memcpy was the code that was solaris
> specific.
> See usr/src/lib/gss_mechs/mech_krb5/krb5/krb/auth_con.c lines 26 & 40.
> So, my question is if those are Solaris specific in auth_con.c, why not
> here.
I can add a "/* Solaris Kerberos */" comment to the lint comment but we
haven't traditionally commented lint comments with "/* Solaris Kerberos
*/" comments. Perhaps we should? MIT code has no lint comments so its
pretty clear that any lint comments are Solaris specific.
...
> Thanks
> >> Looks like kadm5_chpass_principal_util is supposed to be
> >> call to krb5_change_password.
> >>
> Our code:
>
> 349 if (strcmp(pw0.data, pw1.data) != 0) {
> 350 ret = KRB5_LIBOS_BADPWDMATCH;
> 351 sprintf(banner, "%s. Please try again.", error_message(ret));
> 352 } else if (pw0.length == 0) {
> 353 ret = KRB5_CHPW_PWDNULL;
> 354 sprintf(banner, "%s. Please try again.", error_message(ret));
> 355 } else {
> 356 int result_code;
> 357 krb5_data result_string;
> 358
> 359 result_code = kadm5_chpass_principal_util(server_handle, client,
> 360 pw0.data,
> 361 NULL /* don't need pw
> back */,
> 362 banner,
> 363 sizeof(banner));
> 364
> 365 /* the change succeeded. go on */
>
> MIT's 1.6.3 gic_pwd.c file:
>
>
>
> if (strcmp(pw0.data, pw1.data) != 0) {
> ret = KRB5_LIBOS_BADPWDMATCH;
> sprintf(banner, "%s. Please try again.", error_message(ret));
> } else if (pw0.length == 0) {
> ret = KRB5_CHPW_PWDNULL;
> sprintf(banner, "%s. Please try again.", error_message(ret));
> } else {
> int result_code;
> krb5_data code_string;
> krb5_data result_string;
>
> if ((ret = krb5_change_password(context, &chpw_creds, pw0array,
> &result_code, &code_string,
> &result_string)))
> goto cleanup;
>
> /* the change succeeded. go on */
>
>
> Seems like we did not resync right? Maybe?
Ah, good catch. Looks like it should have been synced.
Fixed.
Thanks,
-Mark
