[Freeipa-devel] [PATCH 0019] Prefer TCP connections to UDP in krb5 clients

Fri Nov 14 09:32:23 UTC 2014

On Thu, Nov 13, 2014 at 02:40:28PM -0500, Simo Sorce wrote:
> On Thu, 13 Nov 2014 14:20:14 -0500
> Nathaniel McCallum <npmccallum at redhat.com> wrote:
> 
> > On Thu, 2014-11-13 at 14:02 -0500, Nathaniel McCallum wrote:
> > > 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 <mkosek at redhat.com>
> > > > >> wrote:
> > > > >>
> > > > >> On 11/07/2014 03:03 PM, Simo Sorce wrote:
> > > > >>> On Fri, 07 Nov 2014 14:53:17 +0100
> > > > >>> Martin Kosek <mkosek at redhat.com> wrote:
> > > > >>>
> > > > >>>> On 11/07/2014 02:51 PM, Simo Sorce wrote:
> > > > >>>>> On Fri, 07 Nov 2014 14:26:06 +0100
> > > > >>>>> Martin Kosek <mkosek at redhat.com> wrote:
> > > > >>>>>
> > > > >>>>>> On 11/07/2014 02:20 PM, Simo Sorce wrote:
> > > > >>>>>>> On Fri, 07 Nov 2014 08:02:02 +0100
> > > > >>>>>>> Martin Kosek <mkosek at redhat.com> wrote:
> > > > >>>>>>>
> > > > >>>>>>>> On 11/07/2014 01:46 AM, Simo Sorce wrote:
> > > > >>>>>>>>> On Thu, 06 Nov 2014 18:00:21 -0500
> > > > >>>>>>>>> Nathaniel McCallum <npmccallum at 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?
> > 
> > Simo has just informed me that we will be getting /etc/krb5.conf.d at
> > some point in the near future. So I think for now I'd like to have the
> > original patch[1] reviewed (not the includedir experiment patch). We
> > then document the TCP issue for upgrades. Then, in the future, when we
> > get /etc/krb5.conf.d, we can just stick the default in there.
> > 
> > Is everyone cool with this?
> 
> +1

If there's an easy way to set this option in SSSD, I'm fine with setting
the same default in SSSD as well. 

Sorry we dropped the ball on this item from the SSSD side, I think it's
just b/c there was no ticket filed ...