Date: Mon, 31 Dec 2012 07:18:14 -0500 (EST) From: Rick Macklem <rmacklem@uoguelph.ca> To: Garrett Cooper <yanegomi@gmail.com> Cc: bf1783@gmail.com, freebsd-current@freebsd.org Subject: Re: svn commit: r244604 - head/usr.sbin/gssd Message-ID: <44353525.1604353.1356956294487.JavaMail.root@erie.cs.uoguelph.ca> In-Reply-To: <1905046872.1604317.1356954929867.JavaMail.root@erie.cs.uoguelph.ca>
next in thread | previous in thread | raw e-mail | index | archive | help
Rick Macklem wrote: > Garrett Cooper wrote: > > On Sun, Dec 30, 2012 at 4:49 PM, Rick Macklem <rmacklem@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. 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@freebsd.org mailing list > > http://lists.freebsd.org/mailman/listinfo/freebsd-current > > To unsubscribe, send any mail to > > "freebsd-current-unsubscribe@freebsd.org" > _______________________________________________ > freebsd-current@freebsd.org mailing list > http://lists.freebsd.org/mailman/listinfo/freebsd-current > To unsubscribe, send any mail to > "freebsd-current-unsubscribe@freebsd.org"
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?44353525.1604353.1356956294487.JavaMail.root>