Jakub Hrozek wrote:
On Mon, Nov 12, 2012 at 05:42:21PM +0100, Jan Cholasta wrote:
On 9.11.2012 17:58, Jakub Hrozek wrote:
On Wed, Nov 07, 2012 at 05:20:30PM +0100, Martin Kosek wrote:
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:


That must have been krb5 updated, sssd does not have much to do with
bare kinit.

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


Thanks, the requires were misplaced, they were present in the server
section and should have been in the client section..and because I only
tested with make client-rpms (see below), I didn't notice the typo.

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.


Done. I didn't realize the codepaths were different.


Besides these 2 issues (and the SELinux ones), the patch should be good to go.

Martin

New patches are attached.


We have discussed the patch with Jakub off-list and decided that the
upgrade should be done in %post (with an appropriate $1 check)
instead of %posttrans.


With a little bit more context about why I chose %posttrans initially at
all..I wasn't sure if yum/rpm guarantees it would install the
dependencies (in this case sssd) before installing ipa-client-install,
so I initially did the upgrade in %posttrans to make sure all packages
were in place.

Besides that, ACK.

Thank you, new patches are attached.

I was waiting for selinux-policy to be updated so the included files would work in enforcing mode. That new package is on its way to updates-testing now.

I pushed these three patches plus one which sets a new minimum for selinux-policy.

pushed to master and ipa-3-0

rob

_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to