[Freeipa-devel] [PATCH] client: include the directory with domain-realm mappings in krb5.conf

Wed Nov 7 16:20:30 UTC 2012

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
>> admin at 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 admin at 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