[Freeipa-devel] [PATCH 0019] Prefer TCP connections to UDP in krb5 clients
Simo Sorce
ssorce at redhat.com
Fri Nov 14 14:06:57 UTC 2014
On Fri, 14 Nov 2014 10:32:23 +0100
Jakub Hrozek <jhrozek at redhat.com> wrote:
> 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 ...
We discussed doing that, but we think sssd is not the right place, as
an admin may legitimately want to remove that option.
Simo.
--
Simo Sorce * Red Hat, Inc * New York
More information about the Freeipa-devel
mailing list