[Freeipa-devel] [PATCH] client: include the directory with domain-realm mappings in krb5.conf
Jakub Hrozek
jhrozek at redhat.com
Tue Nov 13 10:06:44 UTC 2012
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
> >>>>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:
> >>
> >
> >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.
-------------- next part --------------
>From 9a98459b258da23529a072b9d77568628d2486e6 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek at redhat.com>
Date: Fri, 17 Aug 2012 11:19:03 +0200
Subject: [PATCH 1/3] ipachangeconf: allow specifying non-default delimeter
for options
---
ipa-client/ipaclient/ipachangeconf.py | 35 +++++++++++++++++++++++------------
1 file changed, 23 insertions(+), 12 deletions(-)
diff --git a/ipa-client/ipaclient/ipachangeconf.py b/ipa-client/ipaclient/ipachangeconf.py
index 6cf47b807957c245fe03ff4259e35526c49175a9..bdc5579fccd82193e837b5e86cbc694c2619317c 100644
--- a/ipa-client/ipaclient/ipachangeconf.py
+++ b/ipa-client/ipaclient/ipachangeconf.py
@@ -174,9 +174,12 @@ class IPAChangeConf:
self.subsectdel[1]))
continue
if o['type'] == "option":
+ delim = o.get('delim', self.dassign)
+ if delim not in self.assign:
+ raise ValueError('Unknown delim "%s" must be one of "%s"' % (delim, " ".join([d for d in self.assign])))
output.append(self._dump_line(self.indent[level],
o['name'],
- self.dassign,
+ delim,
o['value']))
continue
if o['type'] == "comment":
@@ -200,13 +203,21 @@ class IPAChangeConf:
'type': 'comment',
'value': value.rstrip()} # pylint: disable=E1103
+ o = dict()
parts = line.split(self.dassign, 1)
if len(parts) < 2:
- raise SyntaxError('Syntax Error: Unknown line format')
+ # The default assign didn't match, try the non-default
+ for d in self.assign[1:]:
+ parts = line.split(d, 1)
+ if len(parts) >= 2:
+ o['delim'] = d
+ break
- return {'name': parts[0].strip(),
- 'type': 'option',
- 'value': parts[1].rstrip()}
+ if 'delim' not in o:
+ raise SyntaxError, 'Syntax Error: Unknown line format'
+
+ o.update({'name':parts[0].strip(), 'type':'option', 'value':parts[1].rstrip()})
+ return o
def findOpts(self, opts, type, name, exclude_sections=False):
@@ -256,13 +267,13 @@ class IPAChangeConf:
'value': val})
continue
if o['type'] == 'option':
- val = self._dump_line(self.indent[level],
- o['name'],
- self.dassign,
- o['value'])
- opts.append({'name': 'comment',
- 'type': 'comment',
- 'value': val})
+ delim = o.get('delim', self.dassign)
+ if delim not in self.assign:
+ val = self._dump_line(self.indent[level],
+ o['name'],
+ delim,
+ o['value'])
+ opts.append({'name':'comment', 'type':'comment', 'value':val})
continue
if o['type'] == 'comment':
opts.append(o)
--
1.8.0
-------------- next part --------------
>From 550e10db91f82db916ac12cc1559eb1dbf901703 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek at redhat.com>
Date: Wed, 31 Oct 2012 10:59:04 +0100
Subject: [PATCH 2/3] Specify includedir in krb5.conf on new installs
---
freeipa.spec.in | 2 +-
install/share/krb5.conf.template | 2 ++
ipa-client/ipa-install/ipa-client-install | 7 ++++++-
3 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/freeipa.spec.in b/freeipa.spec.in
index 3f446032360a37d6aeb55c287eea2c0cd088bf31..6c631315506fc0d7dfc05bbb8e7e6cefe3146004 100644
--- a/freeipa.spec.in
+++ b/freeipa.spec.in
@@ -84,7 +84,7 @@ BuildRequires: pylint
BuildRequires: python-polib
BuildRequires: libipa_hbac-python
BuildRequires: python-memcached
-BuildRequires: sssd >= 1.8.0
+BuildRequires: sssd >= 1.9.2
BuildRequires: python-lxml
BuildRequires: python-pyasn1 >= 0.0.9a
BuildRequires: python-dns
diff --git a/install/share/krb5.conf.template b/install/share/krb5.conf.template
index f8b1a6f09868c55e47f21279b6d061fbd8251171..ed30b9e0fe37151c097d25e8b2e9fcf600a0a690 100644
--- a/install/share/krb5.conf.template
+++ b/install/share/krb5.conf.template
@@ -1,3 +1,5 @@
+includedir /var/lib/sss/pubconf/krb5.include.d
+
[logging]
default = FILE:/var/log/krb5libs.log
kdc = FILE:/var/log/krb5kdc.log
diff --git a/ipa-client/ipa-install/ipa-client-install b/ipa-client/ipa-install/ipa-client-install
index 190efb183d8c96e2c9665cf51d5346dc1111ae24..5bfaf3319e3f3f015059150b7cb9030495f3c7b8 100755
--- a/ipa-client/ipa-install/ipa-client-install
+++ b/ipa-client/ipa-install/ipa-client-install
@@ -729,7 +729,7 @@ def configure_krb5_conf(cli_realm, cli_domain, cli_server, cli_kdc, dnsok,
options, filename, client_domain):
krbconf = ipaclient.ipachangeconf.IPAChangeConf("IPA Installer")
- krbconf.setOptionAssignment(" = ")
+ krbconf.setOptionAssignment((" = ", " "))
krbconf.setSectionNameDelimiters(("[","]"))
krbconf.setSubSectionDelimiters(("{","}"))
krbconf.setIndent((""," "," "))
@@ -737,6 +737,11 @@ def configure_krb5_conf(cli_realm, cli_domain, cli_server, cli_kdc, dnsok,
opts = [{'name':'comment', 'type':'comment', 'value':'File modified by ipa-client-install'},
{'name':'empty', 'type':'empty'}]
+ # SSSD include dir
+ if options.sssd:
+ opts.append({'name':'includedir', 'type':'option', 'value':'/var/lib/sss/pubconf/krb5.include.d/', 'delim':' '})
+ opts.append({'name':'empty', 'type':'empty'})
+
#[libdefaults]
libopts = [{'name':'default_realm', 'type':'option', 'value':cli_realm}]
if not dnsok or not cli_kdc or options.force:
--
1.8.0
-------------- next part --------------
>From f51e6c94f57bb0103b1251558c0b2f3d206dbfef Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek at redhat.com>
Date: Wed, 31 Oct 2012 10:15:28 +0100
Subject: [PATCH 3/3] Add the includedir to krb5.conf on upgrades
---
freeipa.spec.in | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/freeipa.spec.in b/freeipa.spec.in
index 6c631315506fc0d7dfc05bbb8e7e6cefe3146004..9a918ddf7ae9799b78603ef5cc7ed99c8bbfe5cd 100644
--- a/freeipa.spec.in
+++ b/freeipa.spec.in
@@ -283,6 +283,7 @@ Requires: libsss_autofs
Requires: autofs
Requires: libnfsidmap
Requires: nfs-utils
+Requires(post): policycoreutils
Obsoletes: ipa-client >= 1.0
@@ -611,6 +612,21 @@ if [ $1 -eq 0 ]; then
fi
%endif
+%post client
+if [ $1 -gt 1 ] ; then
+ # Has the client been configured?
+ 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
+ echo "includedir /var/lib/sss/pubconf/krb5.include.d/" > /etc/krb5.conf.ipanew
+ cat /etc/krb5.conf >> /etc/krb5.conf.ipanew
+ mv /etc/krb5.conf.ipanew /etc/krb5.conf
+ /sbin/restorecon /etc/krb5.conf
+ fi
+ fi
+fi
%if ! %{ONLY_CLIENT}
%files server -f server-python.list
--
1.8.0
More information about the Freeipa-devel
mailing list