[SSSD] [PATCH] Split the providers into separate subpackages

Jakub Hrozek jhrozek at redhat.com
Thu Jun 6 21:46:05 UTC 2013


On Thu, Jun 06, 2013 at 09:56:20PM +0200, Lukas Slebodnik wrote:
> On (06/06/13 20:11), Lukas Slebodnik wrote:
> >On (05/06/13 14:50), Jakub Hrozek wrote:
> >>On Thu, Oct 11, 2012 at 09:39:41AM -0400, Simo Sorce wrote:
> >>> On Thu, 2012-10-11 at 13:14 +0200, Jakub Hrozek wrote:
> >>> > 
> >>> > Hi,
> >>> > 
> >>> > the attached patch splits the previously monolithic sssd package into
> >>> > sssd-common that contains the deamon and the responders and
> >>> > per-provider
> >>> > packages such as sssd-ldap or sssd-ipa.
> >>> > 
> >>> > This split would benefit two parties:
> >>> >     1) security auditors who are often trying to find the smallest
> >>> > package
> >>> >     set including dependencies needed for the package to function.
> >>> > They
> >>> >     would be able to i.e. install sssd-ldap and not bother about
> >>> >     sssd-ipa or sssd-ad pulling in more dependencies.
> >>> >     2) 3rd party programs such as realmd or authconfig that would only
> >>> >     be able to require or install on demand the needed packages.
> >>> > 
> >>> > The patch addresses https://fedorahosted.org/sssd/ticket/1510 and must
> >>> > b
> >>> > applied on the two specfile patches I sent earlier (the thread subject
> >>> > included libsss_sudo).
> >>> 
> >>> Questions inline.
> >>> 
> >>
> >>Not even nine months after the initial submission, here comes a revised
> >>patch. I remember we had a discussion on IRC with Simo about this
> >>problem, but I'll reply to the thread.
> >>
> >>With the Radius provider patches on the list and requiring Samba bits in
> >>the last couple of releases, I think that splitting the providers is
> >>something we really should do.
> >>
> >>> > 
> >>> > 
> >>> > 
> >>> > 
> >>> > 
> >>> > 
> >>> > plain text
> >>> > document
> >>> > attachment
> >>> > (0001-Split-the-providers-into-separate-subpackages.patch)
> >>> > 
> >>> > From f59cfde30777a2c46f0ba2d6bd57dff62561851f Mon Sep 17 00:00:00 2001
> >>> > From: Jakub Hrozek <jhrozek at redhat.com>
> >>> > Date: Fri, 28 Sep 2012 09:21:18 +0200
> >>> > Subject: [PATCH] Split the providers into separate subpackages
> >>> > 
> >>> > ---
> >>> >  contrib/sssd.spec.in | 145
> >>> > ++++++++++++++++++++++++++++++++++++++++-----------
> >>> >  1 file changed, 115 insertions(+), 30 deletions(-)
> >>> > 
> >>> > diff --git a/contrib/sssd.spec.in b/contrib/sssd.spec.in
> >>> > index
> >>> > e194245d166c7dee2f1988019b414e5fb47df2de..9b5a9b475544d245fbad0cbdd056ab55a0df4437 100644
> >>> > --- a/contrib/sssd.spec.in
> >>> > +++ b/contrib/sssd.spec.in
> >>> > @@ -45,17 +45,13 @@ BuildRoot: %(mktemp -ud
> >>> > %{_tmppath}/%{name}-%{version}-%{release}-XXXXXX)
> >>> >  Patch0001:  sssd-1.9-man-change-default-ccache.patch
> >>> >  
> >>> >  ### Dependencies ###
> >>> > -
> >>> > -Requires: libldb >= 0.9.3
> >>> > -Requires: libtdb >= 1.1.3
> >>> > +Conflicts: sssd < %{version}-%{release}
> >>> >  Requires: sssd-client%{?_isa} = %{version}-%{release}
> >>> > -Requires: libipa_hbac = %{version}-%{release}
> >>> > -Requires: libsss_idmap = %{version}-%{release}
> >>> > -Requires: cyrus-sasl-gssapi
> >>> > -Requires: keyutils-libs
> >>> > -Requires(post): initscripts chkconfig
> >>> > -Requires(preun):  initscripts chkconfig
> >>> > -Requires(postun): initscripts chkconfig
> >>> > +Requires: sssd-common = %{version}-%{release}
> >>> > +Requires: sssd-ldap = %{version}-%{release}
> >>> > +Requires: sssd-krb5 = %{version}-%{release}
> >>> > +Requires: sssd-ipa = %{version}-%{release}
> >>> > +Requires: sssd-ad = %{version}-%{release}
> >>> 
> >>> 
> >>> Doesn't this set of requires makes the split useless ?
> >>> If I read it correctly it means sssd will require all subpackages anyway
> >>> so you cannot pick and choose to install only one as you say the purpose
> >>> is in the mail message.
> >>> 
> >>
> >>The intent of the sssd package requiring all the dependencies is to make sure
> >>that any kickstart that specified "sssd" would get the whole set, because
> >>we can't currently know what functionality and which provider was used.
> >>
> >>To pick the minimal set for LDAP, you can run:
> >># yum install sssd-ldap
> >>for instance.
> >>
> >>> >  %global servicename sssd
> >>> >  %global sssdstatedir %{_localstatedir}/lib/sss
> >>> > @@ -126,6 +122,21 @@ the system and a pluggable backend system to
> >>> > connect to multiple different
> >>> >  account sources. It is also the basis to provide client auditing and
> >>> > policy
> >>> >  services for projects like FreeIPA.
> >>> >  
> >>> > +%package common
> >>> > +Summary: Common files for the SSSD
> >>> > +Group: Applications/System
> >>> > +License: GPLv3+
> >>> > +Requires: libldb >= 0.9.3
> >>> > +Requires: libtdb >= 1.1.3
> >>> > +Requires: sssd-client%{?_isa} = %{version}-%{release}
> >>> > +Requires(post): initscripts chkconfig
> >>> > +Requires(preun):  initscripts chkconfig
> >>> > +Requires(postun): initscripts chkconfig
> >>> > +Conflicts: sssd < %{version}-%{release}
> >>> > +
> >>> > +%description common
> >>> > +Common files for the SSSD.
> >>> > +
> >>> >  %package client
> >>> >  Summary: SSSD Client libraries for NSS and PAM
> >>> >  Group: Applications/System
> >>> > @@ -141,7 +152,7 @@ service.
> >>> >  Summary: Userspace tools for use with the SSSD
> >>> >  Group: Applications/System
> >>> >  License: GPLv3+
> >>> > -Requires: sssd = %{version}-%{release}
> >>> > +Requires: sssd-common = %{version}-%{release}
> >>> >  
> >>> >  %description tools
> >>> >  Provides userspace tools for manipulating users, groups, and nested
> >>> > groups in
> >>> > @@ -153,6 +164,61 @@ Also provides several other administrative tools:
> >>> >      * sss_seed which pre-creates a user entry for use in kickstarts
> >>> >      * sss_obfuscate for generating an obfuscated LDAP password
> >>> >  
> >>> > +%package ldap
> >>> > +Summary: The LDAP back end of the SSSD
> >>> > +Group: Applications/System
> >>> > +License: GPLv3+
> >>> > +Conflicts: sssd < %{version}-%{release}
> >>> > +Requires: cyrus-sasl-gssapi
> >>> > +Requires: sssd-common = %{version}-%{release}
> >>> > +Requires: libsss_idmap = %{version}-%{release}
> >>> > +
> >>> > +%description ldap
> >>> > +Provides the LDAP back end that the SSSD can utilize to fetch
> >>> > identity data
> >>> > +from and authenticate against an LDAP server.
> >>> > +
> >>> > +%package krb5
> >>> > +Summary: The Kerberos authentication back end for the SSSD
> >>> > +Group: Applications/System
> >>> > +License: GPLv3+
> >>> > +Conflicts: sssd < %{version}-%{release}
> >>> > +Requires: cyrus-sasl-gssapi
> >>> > +Requires: sssd-common = %{version}-%{release}
> >>> > +
> >>> > +%description krb5
> >>> > +Provides the Kerberos back end that the SSSD can utilize authenticate
> >>> > +against a Kerberos server.
> >>> > +
> >>> > +%package ipa
> >>> > +Summary: The IPA back end of the SSSD
> >>> > +Group: Applications/System
> >>> > +License: GPLv3+
> >>> > +Conflicts: sssd < %{version}-%{release}
> >>> > +Requires: sssd-common = %{version}-%{release}
> >>> > +Requires: sssd-ldap = %{version}-%{release}
> >>> > +Requires: sssd-krb5 = %{version}-%{release}
> >>> > +Requires: libipa_hbac = %{version}-%{release}
> >>> > +Requires: libsss_idmap = %{version}-%{release}
> >>> > +Requires: bind-utils
> >>> 
> >>> Does the ipa provider really need the sssd-ldap and sssd-krb5
> >>> subpackages ?
> >>> IIRC we statically compile the ldap and krb5 packages bits we need in
> >>> the ipa provider.
> >>> If you change this you probably want a require on cyrus-sasl-gssapi
> >>> here.
> >>> 
> >>> (if it is just for the ldap and krb child processes shouldn't we simply
> >>> keep those binaries in the sssd or sssd-common package ?)
> >>
> >>Yes, the intent was to make sure the ldap child and krb5 child processes
> >>are pulled in. But now that we switched to internal shared libraries, I
> >>think a better solution is to have the krb5_common internal shared
> >>library along with the ldap and krb5 child in a subpackage of its own
> >>and let the Kerberos-aware providers pull these in.
> >>
> >>> 
> >>> 
> >>> > +%description ipa
> >>> > +Provides the IPA back end that the SSSD can utilize to fetch identity
> >>> > data
> >>> > +from and authenticate against an IPA server.
> >>> > +
> >>> > +%package ad
> >>> > +Summary: The AD back end of the SSSD
> >>> > +Group: Applications/System
> >>> > +License: GPLv3+
> >>> > +Conflicts: sssd < %{version}-%{release}
> >>> > +Requires: sssd-common = %{version}-%{release}
> >>> > +Requires: sssd-ldap = %{version}-%{release}
> >>> > +Requires: sssd-krb5 = %{version}-%{release}
> >>> > +Requires: libsss_idmap = %{version}-%{release}
> >>> 
> >>> SAme questions as for the ipa subpackage
> >>> 
> >>> > +%description ad
> >>> > +Provides the Active Directory back end that the SSSD can utilize to
> >>> > fetch
> >>> > +identity data from and authenticate against an Active Directory
> >>> > server.
> >>> > +
> >>> >  %package -n libsss_idmap
> >>> >  Summary: FreeIPA Idmap library
> >>> >  Group: Development/Libraries
> >>> > @@ -205,7 +271,7 @@ used by Python applications.
> >>> >  Summary: A library to allow communication between SUDO and SSSD
> >>> >  Group: Development/Libraries
> >>> >  License: LGPLv3+
> >>> > -Requires: sssd = %{version}-%{release}
> >>> > +Requires: sssd-ldap = %{version}-%{release}
> >>> >  Requires(post): /sbin/ldconfig
> >>> >  Requires(postun): /sbin/ldconfig
> >>> 
> >>> why libsss_idmap would require the sssd-ldap subpakage ?
> >>
> >>I think this was a mass-replace bug, fixed.
> >>
> >>> 
> >>> Simo.
> >>
> >>There are also two patches preceding the one that splits the providers:
> >>
> >>[PATCH 1/3] rpm: Fold libsss_sudo and libsss_autofs back into the main SSSD package
> >>https://fedorahosted.org/sssd/ticket/1845
> >>
> >>libsss_sudo and libsss_autofs are separate packages that contain just a
> >>single client library with no additional dependencies. This separation
> >>comes from the F-17 timeframe where the feature was really just a tech
> >>preview so we didn't want it to be packaged in sssd proper. On the other
> >>hand users are getting regularly confused about "sudo not working" when
> >>all they really miss is the single library.
> >>
> >>This patch moves the files owned by the libsss_autofs and libsss_sudo
> >>packages back to the main sssd package. We also no longer build the
> >>libsss_sudo documentation by default and do not ship the header file as
> >>it was just a private one.
> >>
> >
> >I tested upgrade with installed freeipa-client and sssd
> >and upgrade worked as expected.
> >
> >ACK
> >
> >>
> >>
> >>[PATCH 2/3] rpm: Use hardened flags for RPM build
> >>https://fedorahosted.org/sssd/ticket/1797
> >>
> >>This patch adds relro and bind_now linker flags to produce hardened
> >>binaries. The change amounts to adding "-Wl,-z,now".
> >
> >Rpm packages are built without problems and Stephen blessed this patch.
> >ACK
> Second patch NACK.
> 
> +export CFLAGS="%{optflags} -Wl,-z,now"
>                             ^^^^^^^^^^
>                         This is linker argument and should not be among CFLAGS

Does it matter if it's preceded by -Wl ?
Would LDFLAGS be any better?



More information about the sssd-devel mailing list