On 11/07/2012 12:53 AM, Jakub Hrozek wrote: > On Wed, Oct 31, 2012 at 01:22:52PM +0100, Martin Kosek wrote: >> On 10/31/2012 11:00 AM, Jakub Hrozek wrote: >>> On Mon, Oct 22, 2012 at 02:14:00PM -0400, Simo Sorce wrote: >>>> On Mon, 2012-10-22 at 17:15 +0200, Martin Kosek wrote: >>>>> On 10/08/2012 08:27 PM, Rob Crittenden wrote: >>>>>> Jakub Hrozek wrote: >>>>>>> On Fri, Aug 17, 2012 at 12:20:27PM -0400, Simo Sorce wrote: >>>>>>>> >>>>>>>> >>>>>>>> ----- Original Message ----- >>>>>>>>> Hi, >>>>>>>>> >>>>>>>>> the attached patches add the directory the SSSD writes domain-realm >>>>>>>>> mappings as includedir to krb5.conf when installing the client. >>>>>>>>> >>>>>>>>> [PATCH 1/3] ipachangeconf: allow specifying non-default delimeter for >>>>>>>>> options >>>>>>>>> ipachangeconf only allows one delimeter between keys and values. This >>>>>>>>> patch adds the possibility of also specifying "delim" in the option >>>>>>>>> dictionary to override the default delimeter. >>>>>>>>> >>>>>>>>> On a slightly-unrelated note, we really should think about adopting >>>>>>>>> Augeas. Changing configuration with home-grown scripts is getting >>>>>>>>> tricky. >>>>>>>>> >>>>>>>>> [PATCH 2/3] Specify includedir in krb5.conf on new installs >>>>>>>>> This patch utilizes the new functionality from the previous patch to >>>>>>>>> add >>>>>>>>> the includedir on top of the krb5.conf file >>>>>>>>> >>>>>>>>> [PATCH 3/3] Add the includedir to krb5.conf on upgrades >>>>>>>>> This patch is completely untested and I'm only posting it to get >>>>>>>>> opinions. At first I was going to use an upgrade script in %post but >>>>>>>>> then I thought it would be overengineering when all we want to do is >>>>>>>>> prepend one line.. Would a simple munging like this be acceptable or >>>>>>>>> shall I write a full script? >>>>>>>> >>>>>>>> NACK, using a scriptlet is fine, but not the way you did, as it has a >>>>>>>> huge >>>>>>>> race condition where krb5.conf exists and has only one line in it (the >>>>>>>> include line). >>>>>>>> >>>>>>>> You should first create the new file: echo "include ..." > >>>>>>>> /etc/krb.conf.ipanew >>>>>>>> Then cat the contents of the existing file in i:t cat /etc/krb.conf >> >>>>>>>> /etc/krb.conf.ipanew >>>>>>>> And finally atomically rename it: mv /etc/krb.conf.ipanew /etc/krb.conf >>>>>>>> >>>>>>>> This method is also safe wrt something killing the yum process ... >>>>>>>> >>>>>>>> Simo. >>>>>>> >>>>>>> I'm attaching a new revision of the patches not even two months after >>>>>>> the original nack. >>>>>>> >>>>>>> I also think it might be nice to have a more general way of upgrading >>>>>>> the client config so I filed >>>>>>> https://fedorahosted.org/freeipa/ticket/3149 >>>>>> >>>>>> I don't think grepping for a string is an effective way to determine if >>>>>> the >>>>>> client has been configured. Someone could have removed that line. >>>>>> >>>>>> I'd prefer using /var/lib/ipa-client/sysrestore/sysrestore.index. If it >>>>>> exists >>>>>> and has more than 2 lines in it ([files] + one other file) then it is >>>>>> safe to >>>>>> say the client is configured, or at least partially configured. >>>>>> >>>>>> rob >>>>>> >>>>> >>>>> I just found one more issue. What if ipa-client-install is run with >>>>> --no-sssd >>>>> option? In that case I assume we should not include the SSSD's >>>>> krb5.include.d, >>>>> right? The same would also appy for upgrades, we would need to check if >>>>> client >>>>> was actually configured with SSSD before mangling their krb5.conf. >>>> >>>> Yeah that's right, we should have all sssd related changes under a >>>> conditional that is true only when sssd is enabled. >>>> >>>> Simo. >>> >>> OK, new patches are attached. In new installs, the includedir is only >>> added when options.sssd is true. During upgrades, I checked for >>> sssd.conf's existence, I'm not sure if there's any other way to check if >>> the client was configured with sssd? >> >> Hello Jakub, thanks for these patches. I think that checking if >> /etc/sssd.conf >> exists as actually not so bad way to test if it was configured. Anyway, I did >> few tests on server and client but I still see few issues: >> >> 1) SELinux context of krb5.conf is not as expected (but I am not sure what >> real >> issue could that cause): >> >> # restorecon -FvvR /etc/krb5.conf >> restorecon reset /etc/krb5.conf context >> unconfined_u:object_r:etc_t:s0->system_u:object_r:krb5_conf_t:s0 >> > > Fixed. Thanks, I shouldn have noticed that doing mv would just replace > the original context. > >> 2) I can no longer kinit on IPA server after applying your patch >> # rpm -q sssd >> sssd-1.9.90-0.20121030T1436Zgitf46bf56.fc17.x86_64 >> # rpm -Uvh --force freeipa-*.rpm >> # head -n 5 /etc/krb5.conf >> includedir /var/lib/sss/pubconf/krb5.include.d/ >> [logging] >> default = FILE:/var/log/krb5libs.log >> kdc = FILE:/var/log/krb5kdc.log >> admin_server = FILE:/var/log/kadmind.log >> # KRB5_TRACE=/dev/stdout kinit admin >> [21059] 1351684052.658548: Getting initial credentials for >> ad...@idm.lab.bos.redhat.com >> [21059] 1351684052.665269: Sending request (200 bytes) to >> IDM.LAB.BOS.REDHAT.COM >> [21059] 1351684052.665989: Resolving hostname vm-044.idm.lab.bos.redhat.com >> [21059] 1351684052.667511: Sending initial UDP request to dgram >> 10.16.78.44:88 >> [21059] 1351684052.672514: Received answer from dgram 10.16.78.44:88 >> [21059] 1351684052.672653: Response was from master KDC >> [21059] 1351684052.672751: Received error from KDC: -1765328370/KDC has no >> support for encryption type >> kinit: KDC has no support for encryption type while getting initial >> credentials >> >> >> Now when I comment includedir: >> # head -n 5 /etc/krb5.conf >> # kinit admin >> Password for ad...@idm.lab.bos.redhat.com: >> # echo $? >> 0 >> >> When I upgraded client machine (without krb5kdc), kinit worked fine. Does >> that >> mean that krb5.conf can only be changed on client machines? >> > > I'm still looking into this. I'm not sure why kinit does that and why it > does that on the IPA server only. Unfortunately the default krb5 package > is so optimized that I need to rebuild one without optimizations. > >> 3) We should also add Requires on sssd >= 1.9.0 in FreeIPA spec file to pick >> up >> the new feature. Otherwise I just get an error on client: >> >> # kinit admin >> kinit: Included profile directory could not be read while initializing >> Kerberos >> 5 library >> > > Done > >> 4) (Optional) I think we can make the process of checking if IPA is >> configured >> easier and follow a similar way that Sumit did: >> https://fedorahosted.org/freeipa/changeset/fe66fbe637132ac5eb22eea388e2261f33497bf5/ >> >> This section: >> >> +restore=0 >> +test -f '/var/lib/ipa-client/sysrestore/sysrestore.index' && restore=$(wc -l >> '/var/lib/ipa-client/sysrestore/sysrestore.index' | awk '{print $1}') >> + >> +if [ -f '/etc/sssd/sssd.conf' -a $restore -ge 2 ]; then >> + if ! egrep -q '/var/lib/sss/pubconf/krb5.include.d/' /etc/krb5.conf >> 2>/dev/null ; then >> >> could then be replaced by something like this: >> >> python -c "import sys; from ipapython import ipautil; sys.exit(0 if >> ipapython.is_ipaclient_configured() else 1);" > /dev/null 2>&1 >> if [ $? -eq 0 ]; then >> >> I am not saying you need to do this step, this can be done later by us. > > That code currently only exists for IPA server, right? At least judging > by: > from ipaserver.install import installutils; > > Then I would prefer to do it separately. It's a good idea, though, the > postscript as it is now is ugly. >
Thanks for updated patch, now when I updated to the most recent sssd, kinit worked for me even though IPA master krb5.conf was updated. Few more issues I found follows: 1) I see a build error: # make rpms ... rpmbuild --define "_topdir /root/freeipa-master/rpmbuild" -ba freeipa.spec error: line 179: Bad Requireflags: qualifiers: Requires(postttrans): policycoreutils make: *** [rpms] Error 1 This is the reason: +Requires(postttrans): policycoreutils should be: +Requires(posttrans): policycoreutils 2) IPA server krb5.conf is not updated for clean server/replica installation. The includedir can get there only with next package update. install/share/krb5.conf.template would also need to be updated. Besides these 2 issues (and the SELinux ones), the patch should be good to go. Martin _______________________________________________ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel