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

Lukas Slebodnik lslebodn at redhat.com
Fri Jun 7 08:14:36 UTC 2013


On (06/06/13 23:46), Jakub Hrozek wrote:
>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 ?
I am not sure, but I inspired with this idea in koji build.

For example:
http://kojipkgs.fedoraproject.org//packages/sssd/1.10.0/6.fc19.beta1/data/logs/x86_64/build.log
Only linker process used -Wl flags.
"-Wl,-z,relro -specs=/usr/lib/rpm/redhat/redhat-hardened-ld"

>Would LDFLAGS be any better?
Yes, this is the right variable, but I am not sure about default values for it.
(like "%{optflags}" for CFLAGS)

LS



More information about the sssd-devel mailing list