Re: svn commit: r244604 - head/usr.sbin/gssd

Rick Macklem wrote:
> Rick Macklem wrote:
> > Garrett Cooper wrote:
> > > On Sun, Dec 30, 2012 at 4:49 PM, Rick Macklem
> > > <rmacklem_at_uoguelph.ca>
> > > wrote:
> > > > bf1783 wrote:
> > > >> >Author: rmacklem
> > > >> >Date: Sat Dec 22 23:21:17 2012
> > > >> >New Revision: 244604
> > > >> >URL: http://svnweb.freebsd.org/changeset/base/244604
> > > >> >
> > > >> >Log:
> > > >> >  It was reported via email that some sshds create kerberos
> > > >> >  credential cache files with names other than
> > > >> >  /tmp/krb5cc_<uid>.
> > > >> >  The gssd daemon does not know how to find these credential
> > > >> >  caches.
> > > >> >  This patch implements a new option "-s" that does a search
> > > >> >  for
> > > >> >  credential cache files, using roughly the same algorithm as
> > > >> >  the
> > > >> >  gssd daemon for Linux uses. The gssd behaviour is only
> > > >> >  changed
> > > >> >  if the new "-s" option is specified. It also implements two
> > > >> >  other
> > > >> >  new options related to the "-s" option.
> > > >> >
> > > >> >  Reported by: Piete.Brooks at cl.cam.ac.uk, Herbert Poeckl
> > > >> >  Tested by: Herbert Poeckl (admin at ist.tugraz.at), Illias
> > > >> >  A.
> > > >> >  Marinos
> > > >> >  MFC after: 2 weeks
> > > >>
> > > >> ...
> > > >>
> > > >> >+#include <krb5.h>
> > > >>
> > > >> Rick:
> > > >>
> > > >> This breaks world built WITHOUT_KERBEROS and WITH_GSSAPI.
> > > >>
> > > >> Regards,
> > > >> b.
> > > > Could you please test the attached patch.
> > > >
> > > > Also, if someone who is familiar with the build/Makefile side
> > > > of things could review this, it would be appreciated.
> > >
> > > 1. I would name WITHOUT_KERBEROS to KERBEROS_SUPPORT in the
> > > sourcefile
> > > and CFLAGS to avoid potential confusion/noise with build logic.
> > >
> > WITHOUT_KERBEROS is used other places, like telnetd. Were you aware
> > of
> > that?
> > (I just thought it would keep it consistent, but if you think it is
> > better
> > to use a different name, I don't care.)
> >
> Oh, I see you were suggesting that the polarity be reversed. Well,
> although the #ifndef is a bit ugly, the utility is useless without
> Kerberos, so I think I'd rather stick with "enabled by default".
> 
> Also, there is KPROGS in head/kerberos5/Makefile, which is a list
> of programs that depend on kerberos. gssd isn't in the list, but
> maybe it should be? (And that list is used to "dekerberise" them
> by setting -DWITHOUT_KERBEROS.)
> 
> So, unless others feel strongly about it, I think I'd rather stick
> with using WITHOUT_KEREBEROS.
> 
Oh, and I've attached the updated patch, rick

> rick
> 
> > > 2. This code should be revised per style(9):
> > >
> > > +#else
> > > + fprintf(stderr, "This option not available when built"
> > > + " without MK_KERBEROS\n");
> > > + exit(1);
> > >
> > > In particular:
> > >
> > > errx(1, "This option requires Kerberos support");
> > >
> > > Seems more succinct and addresses the actual item at hand.
> > >
> > Yea, I'll switch it to errx(). I just cribbed the code further
> > down, that used fprintf().
> >
> > > 3. This could be simplified as well potentially:
> > >
> > > +.if ${MK_KERBEROS} != "no"
> > > DPADD= ${LIBGSSAPI} ${LIBKRB5} ${LIBHX509} ${LIBASN1} ${LIBROKEN}
> > > ${LIBCOM_ERR} ${LIBCRYPT} ${LIBCRYPTO}
> > > LDADD= -lgssapi -lkrb5 -lhx509 -lasn1 -lroken -lcom_err -lcrypt
> > > -lcrypto
> > > +.else
> > > +CFLAGS+= -DWITHOUT_KERBEROS
> > > +DPADD= ${LIBGSSAPI}
> > > +LDADD= -lgssapi
> > > +.endif
> > >
> > > to this:
> > >
> > > DPADD= ${LIBGSSAPI}
> > > LDADD= -lgssapi
> > > .if ${MK_KERBEROS} != "no"
> > > CFLAGS+= -DKERBEROS_SUPPORT
> > > DPADD+= ${LIBKRB5} ${LIBHX509} ${LIBASN1} ${LIBROKEN}
> > > ${LIBCOM_ERR}
> > > ${LIBCRYPT} ${LIBCRYPTO}
> > > LDADD+= -lkrb5 -lhx509 -lasn1 -lroken -lcom_err -lcrypt -lcrypto
> > > .endif
> > >
> > Yea, I can do this change too. I think the latter is more readable.
> >
> > Thanks, rick
> >
> > > Thanks!
> > > -Garrett
> > > _______________________________________________
> > > freebsd-current_at_freebsd.org mailing list
> > > http://lists.freebsd.org/mailman/listinfo/freebsd-current
> > > To unsubscribe, send any mail to
> > > "freebsd-current-unsubscribe_at_freebsd.org"
> > _______________________________________________
> > freebsd-current_at_freebsd.org mailing list
> > http://lists.freebsd.org/mailman/listinfo/freebsd-current
> > To unsubscribe, send any mail to
> > "freebsd-current-unsubscribe_at_freebsd.org"
> _______________________________________________
> freebsd-current_at_freebsd.org mailing list
> http://lists.freebsd.org/mailman/listinfo/freebsd-current
> To unsubscribe, send any mail to
> "freebsd-current-unsubscribe_at_freebsd.org"