On Fri, 2014-11-07 at 15:39 +0100, Martin Kosek wrote:
> On 11/07/2014 03:28 PM, Nathaniel McCallum wrote:
> >
> >> On Nov 7, 2014, at 9:21 AM, Martin Kosek <mko...@redhat.com> wrote:
> >>
> >> On 11/07/2014 03:03 PM, Simo Sorce wrote:
> >>> On Fri, 07 Nov 2014 14:53:17 +0100
> >>> Martin Kosek <mko...@redhat.com> wrote:
> >>>
> >>>> On 11/07/2014 02:51 PM, Simo Sorce wrote:
> >>>>> On Fri, 07 Nov 2014 14:26:06 +0100
> >>>>> Martin Kosek <mko...@redhat.com> wrote:
> >>>>>
> >>>>>> On 11/07/2014 02:20 PM, Simo Sorce wrote:
> >>>>>>> On Fri, 07 Nov 2014 08:02:02 +0100
> >>>>>>> Martin Kosek <mko...@redhat.com> wrote:
> >>>>>>>
> >>>>>>>> On 11/07/2014 01:46 AM, Simo Sorce wrote:
> >>>>>>>>> On Thu, 06 Nov 2014 18:00:21 -0500
> >>>>>>>>> Nathaniel McCallum <npmccal...@redhat.com> wrote:
> >>>>>>>>>
> >>>>>>>>>> On Fri, 2013-10-04 at 06:12 -0400, Simo Sorce wrote:
> >>>>>>>>>>>
> >>>>>>>>>>> ----- Original Message -----
> >>>>>>>>>>>> On 3.10.2013 23:43, Nathaniel McCallum wrote:
> >>>>>>>>>>>>> Patch attached.
> >>>>>>>>>>>>
> >>>>>>>>>>>> I'm curious - what is the purpose of this patch? To prevent 1
> >>>>>>>>>>>> second timeouts and re-transmits when OTP is in place?
> >>>>>>>>>>>>
> >>>>>>>>>>>> What is the expected performance impact? Could it be
> >>>>>>>>>>>> configured for OTP separately - somehow? (I guess that it is
> >>>>>>>>>>>> not possible now ...)
> >>>>>>>>>>>
> >>>>>>>>>>> It benefits also communication of large packets (when large
> >>>>>>>>>>> MS-PAC or CAMMAC AD Data are attached), so it is a better
> >>>>>>>>>>> choice for IPA in general. Especially given we have multiple
> >>>>>>>>>>> KDC processes configured we do not want clients wasting KDC
> >>>>>>>>>>> resources by making multiple processes do the same operation.
> >>>>>>>>>>
> >>>>>>>>>> So apparently this patch never got reviewed over a year ago.
> >>>>>>>>>>
> >>>>>>>>>> It was related to a bug which was opened in SSSD. However, when
> >>>>>>>>>> it became clear we wanted to solve this in FreeIPA, the SSSD
> >>>>>>>>>> bug was closed but no corresponding FreeIPA bug was opened. The
> >>>>>>>>>> patch then fell through the cracks.
> >>>>>>>>
> >>>>>>>> Right - without an associated ticket tracking the patch, it is
> >>>>>>>> too easy to loose it unless the author prods people to review it.
> >>>>>>>>
> >>>>>>>>>> Without this patch, if OTP validation runs long we get
> >>>>>>>>>> retransmits and failures.
> >>>>>>>>>>
> >>>>>>>>>> One question I have is how to handle this for upgrades since (I
> >>>>>>>>>> think) this patch only handles new installs.
> >>>>>>>>>>
> >>>>>>>>>> Anyway, this patch is somewhat urgent now. So help is
> >>>>>>>>>> appreciated.
> >>>>>>>>>>
> >>>>>>>>>> I have attached a rebased version which has no other changes.
> >>>>>>>>>>
> >>>>>>>>>> Nathaniel
> >>>>>>>>>
> >>>>>>>>> I am not sure we can do much on updates, we do not have a
> >>>>>>>>> client-update tool, I would just document it I guess.
> >>>>>>>>> Otherwise we'd have to go back to sssd which can inject
> >>>>>>>>> additional values in krb5.conf, however I am not sure it would
> >>>>>>>>> be ok to set something like this in the sssd's pubconf
> >>>>>>>>> includes ...
> >>>>>>>>
> >>>>>>>> Agreed, pubconf update does not sound right.
> >>>>>>>>
> >>>>>>>> However, we already update krb5.conf on client updates, in %post:
> >>>>>>>>
> >>>>>>>> %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 ! grep -E -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
> >>>>>>>> ...
> >>>>>>>>
> >>>>>>>>
> >>>>>>>> This particular update is more difficult as not a first line
> >>>>>>>> needs to be changed. Without adding ipa client update tool with
> >>>>>>>> some reasonable krb5.conf parser, we could do something along
> >>>>>>>> the lines of
> >>>>>>>>
> >>>>>>>> sed -E 0i 's/(forwardable = \w+)/\1\n udp_preference_limit =
> >>>>>>>> 0/g' /etc/krb5.conf
> >>>>>>>>
> >>>>>>>> (tested), but it is not pretty.
> >>>>>>>
> >>>>>>> What happen the next time you run it again ?
> >>>>>>>
> >>>>>>> Simo.
> >>>>>>>
> >>>>>>
> >>>>>> It would of course be added again, you would need to first grep for
> >>>>>> presence of udp_preference_limit setting. Question is if this
> >>>>>> approach is safe enough to be in our client %post upgrade. We
> >>>>>> already upgrade krb5.conf here, just the change is much easier as
> >>>>>> we just add a line to the beginning of the file.
> >>>>>
> >>>>> Well the concern (aside of duplication) is that  an admin may
> >>>>> "correct" the krb5.conf file to remove that option (for example
> >>>>> because his clients also connect to a differen (older) KDC and must
> >>>>> use UDP in preference. But now we end up messing with its krb5.conf
> >>>>> every time an update is released. An update tool that keep tracks
> >>>>> of whether a specific update has already been applied and does not
> >>>>> retry every time would be needed IMO.
> >>>>>
> >>>>> Simo.
> >>>>
> >>>> In 4.1.x (as there is not much time to develop a separate client
> >>>> update tool), we could grep just for "udp_preference_limit" presence
> >>>> so that if admin changes it's value or comment it, it would not be
> >>>> added again.
> >>>
> >>> Ok then maybe we add this:
> >>>
> >>> # The following value has been added by a freeipa client update
> >>> # if you want to disable it, please comment it, do not delete it
> >>> # or it will be re-added on the next update
> >>> udp_preference_limit = 0
> >>>
> >>> What do you think ?
> >>> Simo.
> >>>
> >>
> >> Sure, this could work (though it is quite lengthy).
> >
> > I think it would be sufficient to only perform the addition of 
> > udp_preference_limit if it does not already exist and if the version number 
> > of the package we are upgrading from is less than the one where we 
> > introduced the change.
> >
> > Nathaniel
> >
> 
> Or to simply call python and get the value of our state store keeping track 
> what was done on upgrades. This would work:
> 
> # python2 -c "from ipaserver.install import sysupgrade; import sys; 
> sys.exit(1 
> if sysupgrade.get_upgrade_state('krb5.conf', 'udp_preference_limit_set') else 
> 0)"
> # echo $?
> 0
> 
> # python2 -c "from ipaserver.install import sysupgrade; 
> sysupgrade.set_upgrade_state('krb5.conf', 'udp_preference_limit_set', True)"
> 
> # python2 -c "from ipaserver.install import sysupgrade; import sys; 
> sys.exit(1 
> if sysupgrade.get_upgrade_state('krb5.conf', 'udp_preference_limit_set') else 
> 0)"
> # echo $?
> 1
> 
> You would just need to come up with something that's present on the client, 
> this is just on the server sub package.

What about the attached approach? It is untested, but I can test it if
we like this method.

Basically, create an include directory that we ship defaults in. Admins
can update /etc/krb5.conf to override the defaults. If we need to change
the default, we just change the file we ship.

We can enable this on upgrades by ensuring that the appropriate
includedir statement appears at the top of the file. The nice thing
about this approach is that is preserves admin modifications (unless
they removed the configuration directive rather than changing it).

Thoughts?

Nathaniel
From caa9e85f7ca7ecf2d36975a87ff060b26a5bea15 Mon Sep 17 00:00:00 2001
From: Nathaniel McCallum <npmccal...@redhat.com>
Date: Thu, 13 Nov 2014 13:54:05 -0500
Subject: [PATCH 2/2] Prefer TCP connections to UDP in krb5 clients

https://fedorahosted.org/sssd/ticket/914
https://fedorahosted.org/freeipa/ticket/4725
---
 contrib/RHEL4/ipa-client-setup                | 1 +
 install/share/krb5.include.d/libdefaults.conf | 1 +
 install/tools/ipa-replica-conncheck           | 1 +
 ipa-client/ipa-install/ipa-client-install     | 1 +
 4 files changed, 4 insertions(+)

diff --git a/contrib/RHEL4/ipa-client-setup b/contrib/RHEL4/ipa-client-setup
index 4d1fead981d0e10232e974527222a2f9a62252b4..6edfa7c38a1d0c17236c3f755f3f7480b14d6a7c 100644
--- a/contrib/RHEL4/ipa-client-setup
+++ b/contrib/RHEL4/ipa-client-setup
@@ -310,6 +310,7 @@ def main():
         libopts.append({'name':'dns_lookup_kdc', 'type':'option', 'value':'true'})
         libopts.append({'name':'ticket_lifetime', 'type':'option', 'value':'24h'})
         libopts.append({'name':'forwardable', 'type':'option', 'value':'yes'})
+        libopts.append({'name':'udp_preference_limit', 'type':'option', 'value':'0'})
 
         opts.append({'name':'libdefaults', 'type':'section', 'value':libopts})
         opts.append({'name':'empty', 'type':'empty'})
diff --git a/install/share/krb5.include.d/libdefaults.conf b/install/share/krb5.include.d/libdefaults.conf
index 8059683c88b8f4341d864ce5ecea72ad10907750..229d600d613e0b7d4dc111fd0a4bad70995881c8 100644
--- a/install/share/krb5.include.d/libdefaults.conf
+++ b/install/share/krb5.include.d/libdefaults.conf
@@ -1,4 +1,5 @@
 [libdefaults]
+ udp_preference_limit = 0
  dns_lookup_realm = false
  dns_lookup_kdc = true
  ticket_lifetime = 24h
diff --git a/install/tools/ipa-replica-conncheck b/install/tools/ipa-replica-conncheck
index 88e42bafbc600fb7c36b7727c770e75edccd2196..22348fc2158e59afc2e1aa51e3d3f51e90b99e39 100755
--- a/install/tools/ipa-replica-conncheck
+++ b/install/tools/ipa-replica-conncheck
@@ -208,6 +208,7 @@ def configure_krb5_conf(realm, kdc, filename):
     libdefaults.append({'name':'rdns', 'type':'option', 'value':'false'})
     libdefaults.append({'name':'ticket_lifetime', 'type':'option', 'value':'24h'})
     libdefaults.append({'name':'forwardable', 'type':'option', 'value':'yes'})
+    libdefaults.append({'name':'udp_preference_limit', 'type':'option', 'value':'0'})
 
     opts.append({'name':'libdefaults', 'type':'section', 'value': libdefaults})
     opts.append({'name':'empty', 'type':'empty'})
diff --git a/ipa-client/ipa-install/ipa-client-install b/ipa-client/ipa-install/ipa-client-install
index 612ff62a12a24672e6bc390bcd5165cd20bf834a..0fe93a05b372b4304c30c9d6c488556d64929273 100755
--- a/ipa-client/ipa-install/ipa-client-install
+++ b/ipa-client/ipa-install/ipa-client-install
@@ -1043,6 +1043,7 @@ def configure_krb5_conf(cli_realm, cli_domain, cli_server, cli_kdc, dnsok,
     libopts.append({'name':'rdns', 'type':'option', 'value':'false'})
     libopts.append({'name':'ticket_lifetime', 'type':'option', 'value':'24h'})
     libopts.append({'name':'forwardable', 'type':'option', 'value':'yes'})
+    libopts.append({'name':'udp_preference_limit', 'type':'option', 'value':'0'})
 
     # Configure KEYRING CCACHE if supported
     if kernel_keyring.is_persistent_keyring_supported():
-- 
2.1.0

From ff9ede6c0d2c9803306a64998e56f158594705fe Mon Sep 17 00:00:00 2001
From: Nathaniel McCallum <npmccal...@redhat.com>
Date: Thu, 13 Nov 2014 13:45:37 -0500
Subject: [PATCH 1/2] Migrate Kerberos client defaults to an include directory

This allows administrators to update /etc/krb5.conf and override the
defaults while also allowing us to change the defaults.
---
 install/share/Makefile.am                     |  7 +++++++
 install/share/krb5.conf.template              | 11 +----------
 install/share/krb5.include.d/libdefaults.conf |  6 ++++++
 install/share/krb5.include.d/logging.conf     |  4 ++++
 4 files changed, 18 insertions(+), 10 deletions(-)
 create mode 100644 install/share/krb5.include.d/libdefaults.conf
 create mode 100644 install/share/krb5.include.d/logging.conf

diff --git a/install/share/Makefile.am b/install/share/Makefile.am
index 878d8868bbbb4f774d378b1d2e886841e2b4b7e4..5d11103879869ae9e2713752676a5c0a496bc497 100644
--- a/install/share/Makefile.am
+++ b/install/share/Makefile.am
@@ -79,8 +79,15 @@ app_DATA =				\
 	schema-update.ldif		\
 	$(NULL)
 
+krb5confdir = $(appdir)/krb5.include.d
+krb5conf_DATA =				\
+	krb5.include.d/logging.conf	\
+	krb5.include.d/libdefaults.conf	\
+	$(NULL)
+
 EXTRA_DIST =				\
 	$(app_DATA)			\
+	$(krb5confdir_DATA)		\
 	$(NULL)
 
 MAINTAINERCLEANFILES =			\
diff --git a/install/share/krb5.conf.template b/install/share/krb5.conf.template
index 7c82083e3331cfacccc1995cd9dfa6ddd88edd1f..8651cdd8d1c3bfa0c77468decc5486051c94c647 100644
--- a/install/share/krb5.conf.template
+++ b/install/share/krb5.conf.template
@@ -1,17 +1,8 @@
 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
+includedir /usr/share/ipa/krb5.include.d/
 
 [libdefaults]
  default_realm = $REALM
- dns_lookup_realm = false
- dns_lookup_kdc = true
- rdns = false
- ticket_lifetime = 24h
- forwardable = yes
 $OTHER_LIBDEFAULTS
 [realms]
  $REALM = {
diff --git a/install/share/krb5.include.d/libdefaults.conf b/install/share/krb5.include.d/libdefaults.conf
new file mode 100644
index 0000000000000000000000000000000000000000..8059683c88b8f4341d864ce5ecea72ad10907750
--- /dev/null
+++ b/install/share/krb5.include.d/libdefaults.conf
@@ -0,0 +1,6 @@
+[libdefaults]
+ dns_lookup_realm = false
+ dns_lookup_kdc = true
+ ticket_lifetime = 24h
+ forwardable = yes
+ rdns = false
diff --git a/install/share/krb5.include.d/logging.conf b/install/share/krb5.include.d/logging.conf
new file mode 100644
index 0000000000000000000000000000000000000000..d3813ae69c7e1af152f8705fdd6ed1032a318d03
--- /dev/null
+++ b/install/share/krb5.include.d/logging.conf
@@ -0,0 +1,4 @@
+[logging]
+ default = FILE:/var/log/krb5libs.log
+ kdc = FILE:/var/log/krb5kdc.log
+ admin_server = FILE:/var/log/kadmind.log
-- 
2.1.0

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

Reply via email to