[SSSD] Re: fleet commander integration

[Fabiano Fidêncio]

On Mon, Feb 18, 2019 at 10:13 AM Levin Stanislav  wrote:
>
>
>
> 18.02.2019 11:11, Fabiano Fidêncio пишет:
> > Levin,
> >
> > On Tue, Feb 12, 2019 at 8:21 AM Levin Stanislav  wrote:
> >> Thank you All for the kind support and explanation!
> >>
> >>
> >> My problem is well described here:
> >>
> >> https://pagure.io/SSSD/sssd/issue/3638,
> >>
> >> which come due to https://pagure.io/SSSD/sssd/issue/3621.
> >>
> >>
> >> As I understand #3621 changes are unnecessary right now because they have 
> >> relied on an assumption on future use.
> >>
> >> Thus, mostly #3621 is an optional feature, right?
> >>
> >> Any thoughts?
> >>
> >> Is there a chance to see a related fix in upstream?
> > #3621 is something that has been fixed already and the code does not
> > require DAC_OVERRIDE and the fix is upstream for a long time.
> Fabiano, talking about #3621 I mean:
>
> FC: SSSD: User has to own his profile and directory of profile
> SSSD: FC: No concerns, I will use my root DAC_OVERRIDE
> Systemd: SSSD: DAC_OVERRIDE is too much for you
> SSSD: Systemd: No concerns, I will use my root CAP_SETGID and CAP_SETUID
> (#3621)
> FC: SSSD: actually, we don't need user permissions for his profile so far

I'm not exactly sure on what you mean here.
We changed permissions in order to avoid hitting issues wrt DAC_OVERRIDE.

I'm not sure, though, whether something else was change since I lef
SSSD project.

>
>
> >
> >>
> >> Reading thats issues I saw different opposite points of view about support 
> >> for non-privileged user.
> >>
> >> Please, feel free to tell me if you are not interested in.
> >
> > About #3638, having SSSD fully running for unprivileged users is
> > something that, IMO, is not going to happen anytime soon (but SSSD
> > developers may jump in and correct me). Considering it's not going to
> > happen, I'd assume that #3638 wouldn't happen any faster. :-)
> >
> > Of course, if there are patches to make everything work with
> > unprivileged users and SSSD team has the time to review and merge
> > those. But, please, be patient as it may take some long time till your
> > patches get reviewed due to the high workload of SSSD team.
>
> From my side, I have to use non-privileged user and have to apply FC anyway.
> I would be happy to contribute to SSSD. Just point me in the right
> direction / idea.

Jakub is the right person to point you at the right direction for that
(as I'm not part of SSSD project anymore).

Please, just, please, do sync with Oliver Gutierrez to ensure that the
changes won't break Fleet Commander integration.

>
> Thank you for your time.
>
> >
> >> Thank you.
> >>
> >>
> >>
> >> 11.02.2019 18:16, Oliver Gutierrez пишет:
> >>
> >> Currently dbus service org.freedesktop.FleetCommanderClient is executed 
> >> when called from SSSD when it finished the JSON file deployment. It is 
> >> executed as root so it can create the paths it needs under several places 
> >> for each supported configuration module.
> >>
> >> The only thing I see here is the domain directory it is not needed to  be 
> >> owned by user/user group, only the user directories need to be owned by 
> >> the user (and it is not really needed because 
> >> org.freedesktop.FleetCommanderClient is run as root, but it is better to 
> >> leave that way so we can access that information from user session if 
> >> needed).
> >>
> >>
> >> On Mon, Feb 11, 2019 at 2:40 PM Fabiano Fidêncio  
> >> wrote:
> >>> On Fri, Feb 8, 2019 at 8:30 AM Alexander Bokovoy  
> >>> wrote:
> >>>> On to, 07 helmi 2019, Jakub Hrozek wrote:
> >>>>> On Thu, Feb 07, 2019 at 05:51:06PM +0300, Levin Stanislav wrote:
> >>>>>> Hello,
> >>>>>>
> >>>>>> I want to ask you about design of fleet commander integration, which I
> >>>>>> found on
> >>>>>> https://docs.pagure.org/SSSD.sssd/design_pages/fleet_commander_integration.html.
> >>>>>>
> >>>>>>> The JSON files will be stored in a new directory owned by the
> >>>>>>> |sssd-ipa| subpackage. The top-level directory could be at
> >>>>>>> |/var/lib/sss/deskprofile/| with per-user subdirectori

[SSSD] Re: fleet commander integration

[Fabiano Fidêncio]

Levin,

On Tue, Feb 12, 2019 at 8:21 AM Levin Stanislav  wrote:
>
> Thank you All for the kind support and explanation!
>
>
> My problem is well described here:
>
> https://pagure.io/SSSD/sssd/issue/3638,
>
> which come due to https://pagure.io/SSSD/sssd/issue/3621.
>
>
> As I understand #3621 changes are unnecessary right now because they have 
> relied on an assumption on future use.
>
> Thus, mostly #3621 is an optional feature, right?
>
> Any thoughts?
>
> Is there a chance to see a related fix in upstream?

#3621 is something that has been fixed already and the code does not
require DAC_OVERRIDE and the fix is upstream for a long time.

>
>
> Reading thats issues I saw different opposite points of view about support 
> for non-privileged user.
>
> Please, feel free to tell me if you are not interested in.


About #3638, having SSSD fully running for unprivileged users is
something that, IMO, is not going to happen anytime soon (but SSSD
developers may jump in and correct me). Considering it's not going to
happen, I'd assume that #3638 wouldn't happen any faster. :-)

Of course, if there are patches to make everything work with
unprivileged users and SSSD team has the time to review and merge
those. But, please, be patient as it may take some long time till your
patches get reviewed due to the high workload of SSSD team.

>
> Thank you.
>
>
>
> 11.02.2019 18:16, Oliver Gutierrez пишет:
>
> Currently dbus service org.freedesktop.FleetCommanderClient is executed when 
> called from SSSD when it finished the JSON file deployment. It is executed as 
> root so it can create the paths it needs under several places for each 
> supported configuration module.
>
> The only thing I see here is the domain directory it is not needed to  be 
> owned by user/user group, only the user directories need to be owned by the 
> user (and it is not really needed because 
> org.freedesktop.FleetCommanderClient is run as root, but it is better to 
> leave that way so we can access that information from user session if needed).
>
>
> On Mon, Feb 11, 2019 at 2:40 PM Fabiano Fidêncio  wrote:
>>
>> On Fri, Feb 8, 2019 at 8:30 AM Alexander Bokovoy  wrote:
>> >
>> > On to, 07 helmi 2019, Jakub Hrozek wrote:
>> > >On Thu, Feb 07, 2019 at 05:51:06PM +0300, Levin Stanislav wrote:
>> > >> Hello,
>> > >>
>> > >> I want to ask you about design of fleet commander integration, which I
>> > >> found on
>> > >> https://docs.pagure.org/SSSD.sssd/design_pages/fleet_commander_integration.html.
>> > >>
>> > >> > The JSON files will be stored in a new directory owned by the
>> > >> > |sssd-ipa| subpackage. The top-level directory could be at
>> > >> > |/var/lib/sss/deskprofile/| with per-user subdirectories. So each
>> > >> > per-user JSON file would be stored at
>> > >> > |/var/lib/sss/deskprofile///.json|. The
>> > >> > || directories need to be owned by the user being logged in.
>> > >> > /var/lib/sss/deskprofile///.json
>> > >> >   -- --
>> > >> >  |  |  ||
>> > >> >  v  |  ||
>> > >> > Created by sssd package as  |  ||
>> > >> > root:root (or sssd:sssd)|  ||
>> > >> > and has permissions 0751|  ||
>> > >> > |  ||
>> > >> > v  ||
>> > >> > Owned by user:user_group   ||
>> > >> > and has permissions 0751   ||
>> > >> >||
>> > >> >||
>> > >> >v|
>> > >> > Owned by user:user_group|
>> > >> > and has permissions 0700|
>> > >> > |
>> > >> > v
>> > >> > Owned by user:user_group
>> > >> > and has permissions 0400
>> > >>
>> > >

[SSSD] Re: fleet commander integration

[Fabiano Fidêncio]

On Fri, Feb 8, 2019 at 8:30 AM Alexander Bokovoy  wrote:
>
> On to, 07 helmi 2019, Jakub Hrozek wrote:
> >On Thu, Feb 07, 2019 at 05:51:06PM +0300, Levin Stanislav wrote:
> >> Hello,
> >>
> >> I want to ask you about design of fleet commander integration, which I
> >> found on
> >> https://docs.pagure.org/SSSD.sssd/design_pages/fleet_commander_integration.html.
> >>
> >> > The JSON files will be stored in a new directory owned by the
> >> > |sssd-ipa| subpackage. The top-level directory could be at
> >> > |/var/lib/sss/deskprofile/| with per-user subdirectories. So each
> >> > per-user JSON file would be stored at
> >> > |/var/lib/sss/deskprofile///.json|. The
> >> > || directories need to be owned by the user being logged in.
> >> > /var/lib/sss/deskprofile///.json
> >> >   -- --
> >> >  |  |  ||
> >> >  v  |  ||
> >> > Created by sssd package as  |  ||
> >> > root:root (or sssd:sssd)|  ||
> >> > and has permissions 0751|  ||
> >> > |  ||
> >> > v  ||
> >> > Owned by user:user_group   ||
> >> > and has permissions 0751   ||
> >> >||
> >> >||
> >> >v|
> >> > Owned by user:user_group|
> >> > and has permissions 0700|
> >> > |
> >> > v
> >> > Owned by user:user_group
> >> > and has permissions 0400
> >>
> >> As I see FleetCommander is executed with root privileges (without CAPs
> >> dropping) and is allowed to read user profiles.
> >>
> >> Why is "user" owner of the directory ""? and why should we
> >> grant "user" with any permissions for this path?
> >>
> >> Why is it not just 0700 for dirs, 0400 for profiles, owner
> >> root/sssd_user for all subpaths?
> >>
> >> Could you please explain?
> >>
> >> Thank you in advance!
> >>
> >
> >Fabiano, do you remember?
> I think the original idea was that org.freedesktop.FleetCommanderClient
> runs the settings merge process under actual user. That might have been
> refactored later.

That's exactly what Alexander mentioned. And, no, AFAIR, it was not
refactored later on.

>
> --
> / Alexander Bokovoy
> Sr. Principal Software Engineer
> Security / Identity Management Engineering
> Red Hat Limited, Finland
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
Fedora Code of Conduct: https://getfedora.org/code-of-conduct.html
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives: 
https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org

[SSSD] Re: Announcing SSSD 1.16.3

[Fabiano Fidêncio]

On Sun, Aug 12, 2018 at 10:15 PM, Fabiano Fidêncio  wrote:
>
>
> On Sun, Aug 12, 2018, 15:57 Jakub Hrozek  wrote:
>>
>> SSSD 1.16.3
>> ===
>>
>> The SSSD team is proud to announce the release of version 1.16.3 of the
>> System Security Services Daemon.
>>
>> The tarball can be downloaded from https://releases.pagure.org/SSSD/sssd/
>>
>> RPM packages will be made available for Fedora shortly.
>>
>> Feedback
>> 
>> Please provide comments, bugs and other feedback via the sssd-devel or
>> sssd-users mailing lists:
>>   https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
>>   https://lists.fedorahosted.org/mailman/listinfo/sssd-users
>>
>> Highlights
>> --
>>
>> New Features
>> 
>> * The ``kdcinfo`` files that SSSD uses to inform libkrb5 about which KDCs
>>   were discovered for a Kerberos realm used to be only generated for the
>>   joined domain, not the trusted domains.  Starting with this release, the
>>   ``kdcinfo`` files are generated automatically also for trusted domains
>> in
>>   setups that use ``id_provider=ad`` and IPA masters in a trust
>> relationship
>>   with an AD domain.
>> * The SSSD Kerberos locator plugin which processes the kdcinfo files and
>>   actually tells libkrb5 about the available KDCs can now process multiple
>>   address if SSSD generates more than one. At the moment, this feature
>>   is only used on IPA clients (see below). Please see the
>>   ``sssd_krb5_locator_plugin(8)`` manual page for more information about
>>   the Kerberos locator plugin.
>> * On IPA clients, the AD DCs or the AD site which should be used to
>>   authenticate users can now be listed in a subdomain section. Please
>>   see `the feature design page
>> <https://docs.pagure.org/SSSD.sssd/design_pages/kdcinfo_improvements.html>`_
>>   or the section "trusted domains configuration" for more details.
>>
>> Notable bug fixes
>> ^
>> * SECURITY: The permissions on ``/var/lib/sss/pipes/sudo`` were set
>>   so that anyone could read anyone else's sudo rules. This was considered
>>   an information leak and assigned CVE-2018-10852 (#3766)
>> * IMPORTANT: The 1.16.2 release was storing the cached passwords without
>>   a salt prefix string. This bug was fixed in this release, but any
>>   password hashes generated by 1.16.2 are incompatible with the hashes
>>   generated by 1.16.3. The effect is that upgrade from 1.16.2 to 1.16.3
>>   should be done when the authentication server is reachable so that the
>>   first authentication after the upgrade fix the cached password.
>> * The ``sss_ssh`` proces leaked file descriptors when converting more than
>>   one x509 certificate to SSH public key (#3794)
>> * SSSD, when configured with ``id_provider=ad`` was using too expensive
>>   LDAP search to find out whether the required POSIX attributes
>>   were replicated to the Global Catalog. Instead, SSSD now consults
>>   the Partial Attribute Set, which is much more effective (#3755)
>> * The PAC responder is now able to process Domain Local in case the
>>   PAC uses SID compression. Typicaly this is the case with Windows Server
>>   2012 and newer (#3767)
>> * Some versions of OpenSSH (e.g. the one shipped in RHEL-7.5) would
>>   close the pipe towards ``sss_ssh_authorizedkeys`` when the matching
>>   key is found before the rest of the output is read. The
>>   ``sss_ssh_authorizedkeys`` helper was not handling this behaviour
>>   well and would exit with SIGPIPE, which also meant the public key
>>   authentication failed (#3747)
>> * User lookups no longer fail if user's e-mail address conflicts with
>>   another user's fully qualified name (#3607)
>> * The ``override_shell`` and ``override_homedir`` options are no longer
>>   applied to entries from the files domain. (#3758)
>> * Several bugs related  to the FleetCommander integration were fixed
>> (#3773,
>>   #3774)
>> * The grace logins with an expired password when authenticating against
>>   certain newer versions of the 389DS/RHDS LDAP server did not work
>> (#3597)
>> * Whitespace around netgroup triple separator is now stripped
>> * The ``sss_ssh_knownhostproxy`` utility can now print the host key
>> without
>>   proxying the connection.
>> * Due to an overly restrictive check, the fast in-memory cache was
>> sometimes
>>   skipped, which caused a high load on the ``sssd_nss`` process (#3776).
>>
>>
>> Packaging Changes
>> --

[SSSD] Re: Announcing SSSD 1.16.3

[Fabiano Fidêncio]

ure.io/SSSD/sssd/issue/3796>`_ - The IPA selinux
> provider can return an error if SELinux is completely disabled
> * `3794 <https://pagure.io/SSSD/sssd/issue/3794>`_ - sssd_ssh leaks file
> descriptors when more than one certificate is converted into an SSH key
> * `3791 <https://pagure.io/SSSD/sssd/issue/3791>`_ - The cached password
> does not store the salt prefix
> * `3778 <https://pagure.io/SSSD/sssd/issue/3778>`_ - When sssd is running
> as non-root user, the sudo pipe is created as sssd:sssd but then the
> private pipe ownership fails
> * `3777 <https://pagure.io/SSSD/sssd/issue/3777>`_ - If access check for
> a privileged pipe fails, the responder loops indefinitely
> * `3776 <https://pagure.io/SSSD/sssd/issue/3776>`_ - Spurious check in
> the sssd nss memcache can cause the memory cache to be skipped
> * `3774 <https://pagure.io/SSSD/sssd/issue/3774>`_ - Desktop Profile: The
> 10th policy is producing a wrong file name
> * `3773 <https://pagure.io/SSSD/sssd/issue/3773>`_ - SSSD bails out
> saving desktop profiles in case an invalid profile is found
> * `3767 <https://pagure.io/SSSD/sssd/issue/3767>`_ - Groups go missing
> with PAC enabled in sssd
> * `3766 <https://pagure.io/SSSD/sssd/issue/3766>`_ - CVE-2018-10852:
> information leak from the sssd-sudo responder
> * `3758 <https://pagure.io/SSSD/sssd/issue/3758>`_ - override_homedir
> should not apply to the files provider
> * `3755 <https://pagure.io/SSSD/sssd/issue/3755>`_ - The search filter
> for detecting POSIX attributes in global catalog is too broad and can cause
> a high load on the servers
> * `3754 <https://pagure.io/SSSD/sssd/issue/3754>`_ - SSSD AD uses LDAP
> filter to detect POSIX attributes stored in AD GC also for regular AD DC
> queries
> * `3747 <https://pagure.io/SSSD/sssd/issue/3747>`_ -
> sss_ssh_authorizedkeys exits abruptly if SSHD closes its end of the pipe
> before reading all the SSH keys
> * `3652 <https://pagure.io/SSSD/sssd/issue/3652>`_ - kdcinfo doesn't get
> populated for other domains
> * `3607 <https://pagure.io/SSSD/sssd/issue/3607>`_ - Handle conflicting
> e-mail addresses more gracefully
> * `3597 <https://pagure.io/SSSD/sssd/issue/3597>`_ - sssd doesn't allow
> user with expired password to login when PasswordgraceLimit set
> * `3596 <https://pagure.io/SSSD/sssd/issue/3596>`_ - A combination of the
> same qualified and unqualified sudoUser causes Error: 17: File exists
> * `3542 <https://pagure.io/SSSD/sssd/issue/3542>`_ - Get host key without
> proxying connection
> * `3475 <https://pagure.io/SSSD/sssd/issue/3475>`_ - Full information
> regarding priority of lookup of principal  in keytab not in man page
> * `3291 <https://pagure.io/SSSD/sssd/issue/3291>`_ - RFE: sssd in cross
> realm trust configuration should be use AD KDC from a list or site defined
> in the config file
>
> Detailed Changelog
> --
>
>
> * Alexander Bokovoy (2):
>
>  * ipa provider: always use a special keytab to talk to a trusted DC
>  * ipa provider: expand search base to cover trusted domain objects
>
> * Alexey Sheplyakov (1):
>
>  * nss: skip incomplete groups instead of bailing out
>
> * Amit Kumar (1):
>
>  * Responder: simplify if-else structure in sss_dp_get_account_msg()
>
> * Fabiano Fidêncio (18):
>
>  * intg: Do not hardcode nsslibdir
>  * files: do not apply override_homedir to files provider
>  * tests: add override_homedir tests for files provider
>  * files: do not apply override_shell to files provider
>  * tests: add override_shell tests for files provider
>  * util: add is_files_provider() helper
>  * files: make use of is_files_provider() helper
>  * cache_req: keep the files provider as the first domain to be
> searched
>  * tests: add basic tests for
> cache_req_domain_new_list_from_domain_resolution_order()
>  * tests: add a test to ensure the output_fqnames is false for files
> provider
>  * deskprofile: don't bail if we fail to save one profile
>  * sdap: respect passwordGracelimit
>  * deskprofile: fix a typo in _get_filename_path()
>  * tests: add tests for ipa_deskprofile_get_filename_path()
>  * util: introduce sss_ssh_print_pubkey()
>  * ssh: make use of sss_ssh_print_pubkey()
>  * sss_ssh_knownhostsproxy: add option to only print the pubkey
>  * nss: remove unused label
>
> * Jakub Hrozek (38):
>
>  * Bumping the version to track the 1.16.3 development
>  * TESTS: Extend the schema with sshPublicKey attribute
>  * TESTS: Allow adding sshPublicKey for users
>  * TESTS: Add a ba

[SSSD] Re: all tests fail in intgcheck

[Fabiano Fidêncio]

Chris,

On Mon, Jun 11, 2018 at 1:10 PM, Chris Kowalczyk  wrote:
> Hello,
>
> Do you have any idea what could be wrong with my environment?
>

Please, join #sssd on freenode.
I have an opensuse system here, I'll give it a try and get back to you.

> Sorry for bothering, but this blocks my PR ..

Not bothering, at all. And sorry for the delay in the responses.

>
> Regard,
> Chris
>
>
>
>
> On 06/07/2018 02:18 PM, Chris Kowalczyk wrote:
>>
>> Hello All,
>>
>> I have been trying to run sssd intgcheck for some time. Thanks to your
>> help, there is some progress but I am not 'there' yet..
>>
>> So, I managed to build the tests by using the following command:
>>
>> > make intgcheck INTGCHECK_CONFIGURE_FLAGS="
>> --build=$SSS_ARCH-unknown-linux-gnu \
>> --host=$SSS_ARCH-unknown-linux-gnu \
>> --program-prefix= \
>> --prefix=/usr \
>> --exec-prefix=/usr \
>> --bindir=/usr/bin \
>> --sbindir=/usr/sbin \
>> --sysconfdir=/etc \
>> --datadir=/usr/share \
>> --includedir=/usr/include \
>> --libdir=$SSS_LIBDIR \
>> --libexecdir=/usr/libexec \
>> --localstatedir=/var \
>> --sharedstatedir=/var/lib \
>> --mandir=/usr/share/man \
>> --infodir=/usr/share/info \
>> --enable-nsslibdir=/$SSS_LIB \
>> --enable-pammoddir=/$SSS_LIB/security \
>> --with-krb5-rcache-dir=/var/cache/krb5rcache \
>> --with-initscript=systemd \
>> --with-syslog=journald \
>> --enable-all-experimental-features \
>> --cache-file=/tmp/sssd/fileconfig.cache \
>> --with-test-dir=/tmp/sssd/tests   "
>>
>> > echo $SSS_LIBDIR
>> /usr/lib64
>>
>> > echo $SSS_LIB
>> lib64
>>
>> This command is based on:
>> https://github.com/SSSD/sssd/blob/master/contrib/fedora/bashrc_sssd
>>
>> The tests run but they (almost) all fail. What I discovered is that the
>> created test folder is rather empty:
>>
>> /tmp/sssd-intg.33or699m
>> $ tree
>> .
>> ├── etc
>> │   └── ldap
>> └── var
>> ├── lib
>> └── run
>>
>> 5 directories, 0 files
>>
>> ( BTW, shouldn't the test folder be /tmp/sssd/tests, as I specified in the
>> make command? )
>>
>> I was trying to modify various things but to no success - the test folders
>> were always empty. As a result, I get many errors from the testcases like:
>> "/tmp/sssd-intg.33or699m/etc/sssd/sssd.conf" or
>> "/tmp/sssd-intg.33or699m/lib64/getsockopt_wrapper.so".
>>
>> Would you have any idea how to 'properly' point everything to be put into
>> those test folders?
>>
>> Kind regards,
>> Chris
>>
> ___
> sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
> To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
> Fedora Code of Conduct: https://getfedora.org/code-of-conduct.html
> List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
> List Archives:
> https://lists.fedoraproject.org/archives/list/sssd-devel@lists.fedorahosted.org/message/TMJRTLQG7T5MSAFX3XDH2RYFXKSE23UK/
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
Fedora Code of Conduct: https://getfedora.org/code-of-conduct.html
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives: 
https://lists.fedoraproject.org/archives/list/sssd-devel@lists.fedorahosted.org/message/BXPE6DZBQZNROH2DLUYQDAVM4X6JZBPG/

[SSSD] Re: RFC: 1.16.2 release notes

[Fabiano Fidêncio]

Jakub,

On Fri, Jun 8, 2018 at 11:02 AM, Jakub Hrozek  wrote:
> Hi,
>
> below are the release notes for 1.16.2. Please comment :-)
>
> SSSD 1.16.2
> ===
>
> Highlights
> --
>
> New Features
> 
>  * The smart card authentication, or in more general certificate 
> authentication
>code now supports OpenSSL in addition to previously supported NSS (#3489).
>In addition, the SSH responder can now return public SSH keys derived from
>the public keys stored in a X.509 certificate. Please refer to the
>``ssh_use_certificate_keys`` option in the man pages.
>  * The files provider now supports mirroring multiple passwd or group
>files. This enhancement can be used to use the SSSD files provider instead
>of the nss_altfiles module
>
> Notable bug fixes
> ^

In this section I'd also mention:
* A potential crash in AUTOFS responder's code was fixed (#3752)

>  * A memory handling issue in the ``nss_ex`` interface was fixed. This bug
>would manifest in IPA environments with a trusted AD domain as a crash of
>the ns-slapd process, because a ``ns-slapd`` plugin loads the ``nss_ex``
>interface (#3715)
>  * Several fixes for the KCM deamon were merged (see #3687, #3671, #3633)
>  * The ``ad_site`` override is now honored in GPO code as well (#3646)
>  * Several potential crashes in the NSS responder's netgroup code were fixed
>(#3679, #3731)
>  * The LDAP provider now supports group renaming (#2653)
>  * The GPO access control code no longer returns an error if one of the
>relevant GPO rules contained no SIDs at all (#3680)
>  * A memory leak in the IPA provider related to resolving external AD
>groups was fixed (#3719)
>  * Setups that used multiple domains where one of the domains had its ID
>space limited using the ``min_id/max_id`` options did not resolve requests
>by ID properly (#3728)
>  * Overriding IDs or names did not work correctly when the domain resolution
>order was set as well (#3595)
>  * A version mismatch between certain newer Samba versions (e.g. those shipped
>in RHEL-7.5) and the Winbind interface provided by SSSD was fixed. To 
> further
>prevent issues like this in the future, the correct interface is now 
> detected
>at build time (#3741)
>  * The files provider no longer returns a qualified name in case domain
>resolution order is used (#3743)
>  * A race condition between evaluating IPA group memberships and AD group
>memberships in setups with IPA-AD trusts that would have manifested as
>randomly losing IPA group memberships assigned to an AD user was fixed
>(#3744)
>  * Setting an SELinux login label was broken in setups where the domain
>resolution order was used (#3740)
>  * SSSD start up issue on systems that use the libldb library with version
>1.4.0 or newer was fixed.
>
> Packaging Changes
> -
>  * Several new build requirements were added in order to support the OpenSSL
>certificate authentication
>
> Documentation Changes
> -
>  * The files provider gained two new configuration options ``passwd_files``
>and ``group_files.`` These can be used to specify the additional files
>to mirror.
>  * A new ``ssh_use_certificate_keys`` option toggles whether the SSH responder
>would return public SSH keys derived from X.509 certificates.
>  * The ``local_negative_timeout`` option is now enabled by default. This
>means that if SSSD fails to find a user in the configured domains,
>but is then able to find the user with an NSS call such as getpwnam,
>it would negatively cache the request for the duration of the
>local_negative_timeout option.
> ___
> sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
> To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
> Fedora Code of Conduct: https://getfedora.org/code-of-conduct.html
> List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
> List Archives: 
> https://lists.fedoraproject.org/archives/list/sssd-devel@lists.fedorahosted.org/message/JPHPMXWJQOTPVEBSHNIUK52VPLIWP4FV/
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
Fedora Code of Conduct: https://getfedora.org/code-of-conduct.html
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives: 
https://lists.fedoraproject.org/archives/list/sssd-devel@lists.fedorahosted.org/message/JZBKDVGAZ352MJGIEEKSNJJRJZSJSRWF/

[SSSD] Re: What's the best way to debug SELinux issues on SSSD?

[Fabiano Fidêncio]

On Mon, May 21, 2018 at 10:32 PM, Jakub Hrozek  wrote:
>
>
>> On 21 May 2018, at 21:39, Fabiano Fidêncio  wrote:
>>
>> People,
>>
>> I've been trying to debug a SELinux issue related to the domain
>> resolution order.
>>
>> Basically, if there's no domain_reoslution_order set:
>> [root@client1 vagrant]# ssh -l admin localhost
>> Password:
>> Last login: Mon May 21 19:00:06 2018 from ::1
>> [admin@client1 ~]$ id -Z
>> staff_u:staff_r:staff_t:s0-s0:c0.c1023
>>
>> But, if domain_resolution_order is set:
>> [root@client1 vagrant]# ssh -l admin localhost
>> Password:
>> Last login: Mon May 21 19:30:45 2018 from ::1
>> [admin@ipa.example@client1 ~]$ id -Z
>> unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023
>
> Since sssd calls into libsemanage, then I would start with looking at the 
> output of “semanage login -l”.

You gave me this tip before, I did test it but I forgot to add details
about it :-/

The output is basically the same with or without domain_resolution_order set:

[root@client1 x86_64]# cat /etc/sssd/sssd.conf | grep domain_resolution_order
domain_resolution_order = foo
[root@client1 x86_64]# systemctl restart sssd
[root@client1 x86_64]# semanage login -l

Login Name   SELinux User MLS/MCS RangeService

__default__  unconfined_u s0-s0:c0.c1023   *
adminstaff_u  s0-s0:c0.c1023   *
root unconfined_u s0-s0:c0.c1023   *


[root@client1 x86_64]# vim /etc/sssd/sssd.conf
[root@client1 x86_64]# cat /etc/sssd/sssd.conf | grep domain_resolution_order
#domain_resolution_order = foo
[root@client1 x86_64]# systemctl restart sssd
[root@client1 x86_64]# semanage login -l

Login Name   SELinux User MLS/MCS RangeService

__default__  unconfined_u s0-s0:c0.c1023   *
adminstaff_u  s0-s0:c0.c1023   *
root unconfined_u s0-s0:c0.c1023   *


> Is the context after selinux_child runs to completion set for admin or 
> admin@ipa.example?

In the selinux_child.log I can only see:
[[sssd[selinux_child[18149 [unpack_buffer] (0x2000): username: admin

And it happens for both cases, with or without domain_resolution_order
being used.

> Maybe libsemanage canonicalizes the name or UID with getpnam/uid and 
> therefore uses the qualified name but because the provider has no sense of 
> the domain_resolution_order, it calls sss_set_seuser() with the shortname?

That's a good question. sss_set_seuser() hasn't been called, at all.

>
> Looking at whether we call sss_set_seuser() with the correct parameters might 
> give some hint as well.
>
>>
>> First thing that came to my mind was to take a look at
>> selinux_child.logs, but it didn't give me any clue as the logs are
>> exactly the same for both cases:
>>
>> No domain_resolution_order set:
>> (Mon May 21 19:30:44 2018) [[sssd[selinux_child[23351 [main]
>> (0x0400): selinux_child started.
>> (Mon May 21 19:30:44 2018) [[sssd[selinux_child[23351 [main]
>> (0x2000): Running with effective IDs: [0][0].
>> (Mon May 21 19:30:44 2018) [[sssd[selinux_child[23351 [main]
>> (0x2000): Running with real IDs [0][0].
>> (Mon May 21 19:30:44 2018) [[sssd[selinux_child[23351 [main]
>> (0x0400): context initialized
>> (Mon May 21 19:30:44 2018) [[sssd[selinux_child[23351
>> [unpack_buffer] (0x2000): seuser length: 7
>> (Mon May 21 19:30:44 2018) [[sssd[selinux_child[23351
>> [unpack_buffer] (0x2000): seuser: staff_u
>> (Mon May 21 19:30:44 2018) [[sssd[selinux_child[23351
>> [unpack_buffer] (0x2000): mls_range length: 14
>> (Mon May 21 19:30:44 2018) [[sssd[selinux_child[23351
>> [unpack_buffer] (0x2000): mls_range: s0-s0:c0.c1023
>> (Mon May 21 19:30:44 2018) [[sssd[selinux_child[23351
>> [unpack_buffer] (0x2000): username length: 5
>> (Mon May 21 19:30:44 2018) [[sssd[selinux_child[23351
>> [unpack_buffer] (0x2000): username: admin
>> (Mon May 21 19:30:44 2018) [[sssd[selinux_child[23351 [main]
>> (0x0400): performing selinux operations
>> (Mon May 21 19:30:44 2018) [[sssd[selinux_child[23351
>> [seuser_needs_update] (0x2000): getseuserbyname: ret: 0 seuser:
>> staff_u mls: s0-s0:c0.c1023
>> (Mon May 21 19:30:44 2018) [[sssd[selinux_child[23351
>> [pack_buffer] (0x0400): result [0]
>> (Mon May 21 19:30:44 2018) [[sssd[selinux_child[23351
>> [prepare_response] (0x4000): r->size: 4
>> (Mon May 21 19:30:44 2018) [[sssd[selinux_child[23351 [main]
>> (0x0400): selinux_child completed successfully
>>
>> dom

[SSSD] What's the best way to debug SELinux issues on SSSD?

[Fabiano Fidêncio]

People,

I've been trying to debug a SELinux issue related to the domain
resolution order.

Basically, if there's no domain_reoslution_order set:
[root@client1 vagrant]# ssh -l admin localhost
Password:
Last login: Mon May 21 19:00:06 2018 from ::1
[admin@client1 ~]$ id -Z
staff_u:staff_r:staff_t:s0-s0:c0.c1023

But, if domain_resolution_order is set:
[root@client1 vagrant]# ssh -l admin localhost
Password:
Last login: Mon May 21 19:30:45 2018 from ::1
[admin@ipa.example@client1 ~]$ id -Z
unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023

First thing that came to my mind was to take a look at
selinux_child.logs, but it didn't give me any clue as the logs are
exactly the same for both cases:

No domain_resolution_order set:
(Mon May 21 19:30:44 2018) [[sssd[selinux_child[23351 [main]
(0x0400): selinux_child started.
(Mon May 21 19:30:44 2018) [[sssd[selinux_child[23351 [main]
(0x2000): Running with effective IDs: [0][0].
(Mon May 21 19:30:44 2018) [[sssd[selinux_child[23351 [main]
(0x2000): Running with real IDs [0][0].
(Mon May 21 19:30:44 2018) [[sssd[selinux_child[23351 [main]
(0x0400): context initialized
(Mon May 21 19:30:44 2018) [[sssd[selinux_child[23351
[unpack_buffer] (0x2000): seuser length: 7
(Mon May 21 19:30:44 2018) [[sssd[selinux_child[23351
[unpack_buffer] (0x2000): seuser: staff_u
(Mon May 21 19:30:44 2018) [[sssd[selinux_child[23351
[unpack_buffer] (0x2000): mls_range length: 14
(Mon May 21 19:30:44 2018) [[sssd[selinux_child[23351
[unpack_buffer] (0x2000): mls_range: s0-s0:c0.c1023
(Mon May 21 19:30:44 2018) [[sssd[selinux_child[23351
[unpack_buffer] (0x2000): username length: 5
(Mon May 21 19:30:44 2018) [[sssd[selinux_child[23351
[unpack_buffer] (0x2000): username: admin
(Mon May 21 19:30:44 2018) [[sssd[selinux_child[23351 [main]
(0x0400): performing selinux operations
(Mon May 21 19:30:44 2018) [[sssd[selinux_child[23351
[seuser_needs_update] (0x2000): getseuserbyname: ret: 0 seuser:
staff_u mls: s0-s0:c0.c1023
(Mon May 21 19:30:44 2018) [[sssd[selinux_child[23351
[pack_buffer] (0x0400): result [0]
(Mon May 21 19:30:44 2018) [[sssd[selinux_child[23351
[prepare_response] (0x4000): r->size: 4
(Mon May 21 19:30:44 2018) [[sssd[selinux_child[23351 [main]
(0x0400): selinux_child completed successfully

domain_resolution_order set:
(Mon May 21 19:31:36 2018) [[sssd[selinux_child[23398 [main]
(0x0400): selinux_child started.
(Mon May 21 19:31:36 2018) [[sssd[selinux_child[23398 [main]
(0x2000): Running with effective IDs: [0][0].
(Mon May 21 19:31:36 2018) [[sssd[selinux_child[23398 [main]
(0x2000): Running with real IDs [0][0].
(Mon May 21 19:31:36 2018) [[sssd[selinux_child[23398 [main]
(0x0400): context initialized
(Mon May 21 19:31:36 2018) [[sssd[selinux_child[23398
[unpack_buffer] (0x2000): seuser length: 7
(Mon May 21 19:31:36 2018) [[sssd[selinux_child[23398
[unpack_buffer] (0x2000): seuser: staff_u
(Mon May 21 19:31:36 2018) [[sssd[selinux_child[23398
[unpack_buffer] (0x2000): mls_range length: 14
(Mon May 21 19:31:36 2018) [[sssd[selinux_child[23398
[unpack_buffer] (0x2000): mls_range: s0-s0:c0.c1023
(Mon May 21 19:31:36 2018) [[sssd[selinux_child[23398
[unpack_buffer] (0x2000): username length: 5
(Mon May 21 19:31:36 2018) [[sssd[selinux_child[23398
[unpack_buffer] (0x2000): username: admin
(Mon May 21 19:31:36 2018) [[sssd[selinux_child[23398 [main]
(0x0400): performing selinux operations
(Mon May 21 19:31:36 2018) [[sssd[selinux_child[23398
[seuser_needs_update] (0x2000): getseuserbyname: ret: 0 seuser:
staff_u mls: s0-s0:c0.c1023
(Mon May 21 19:31:36 2018) [[sssd[selinux_child[23398
[pack_buffer] (0x0400): result [0]
(Mon May 21 19:31:36 2018) [[sssd[selinux_child[23398
[prepare_response] (0x4000): r->size: 4
(Mon May 21 19:31:36 2018) [[sssd[selinux_child[23398 [main]
(0x0400): selinux_child completed successfully

Taking a look at the IPA provider, logs also do like the very same:
https://paste.fedoraproject.org/paste/FKhvxyj3clzXuE5C7tMGhw (pastebin
is huge!)

Some tip on which logs I could take a look and/or part of the code I
could instrument in order to, at least, get some directions?

Thanks in advance,
--
Fabiano Fidêncio
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
Fedora Code of Conduct: https://getfedora.org/code-of-conduct.html
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives: 
https://lists.fedoraproject.org/archives/list/sssd-devel@lists.fedorahosted.org/message/2G5K5WWNZZHHOXTIWF62ETMA4QA6ZJGW/

[SSSD] Re: [RFC] sbus2 integration

[Fabiano Fidêncio]

On Fri, May 18, 2018 at 9:50 PM, Simo Sorce  wrote:
> On Fri, 2018-05-18 at 16:11 +0200, Sumit Bose wrote:
>> On Fri, May 18, 2018 at 02:33:32PM +0200, Pavel Březina wrote:
>> > Hi folks,
>> > I sent a mail about new sbus implementation (I'll refer to it as sbus2) 
>> > [1].
>
> Sorry Pavel,
> but I need to ask, why a new bus instead of somthing like varlink ?

For those who are not familiar with varlink: https://lwn.net/Articles/742675/

>
>> > Now, I'm integrating it into SSSD. The work is quite difficult since it
>> > touches all parts of SSSD and the changes are usually interconnected but 
>> > I'm
>> > slowly moving towards the goal [2].
>> >
>> > At this moment, I'm trying to take "miminum changes" paths so the code can
>> > be built and function with sbus2, however to take full advantage of it, it
>> > will take further improvements (that will not be very difficult).
>> >
>> > There is one big change that I would like to take though, that needs to be
>> > discussed. It is about how we currently handle sbus connections.
>> >
>> > In current state, monitor and each backend creates a private sbus server.
>> > The current implementation of a private sbus server is not a message bus, 
>> > it
>> > only serves as an address to create point to point nameless connection. 
>> > Thus
>> > each client must maintain several connections:
>> >  - each responder is connected to monitor and to all backends
>> >  - each backend is connected to monitor
>> >  - we have monitor + number of backends private servers
>> >  - each private server maintains about 10 active connections
>> >
>> > This has several disadvantages - there are many connections, we cannot
>> > broadcast signals, if a process wants to talk to other process it needs to
>> > connect to its server and maintain the connection. Since responders do not
>> > currently provider a server, they cannot talk between each other.
>
> This design has a key advantage, a single process going down does not
> affect all other processes communication. How do you recover if the
> "switch-board" goes down during message processing with sbus ?
>
>> > sbus2 implements proper private message bus. So it can work in the same way
>> > as session or system bus. It is a server that maintains the connections,
>> > keep tracks of their names and then routes messages from one connection to
>> > another.
>> >
>> > My idea is to have only one sbus server managed by monitor.
>
> This conflict wth the idea of getting rid of the monitor process, do
> not know if this is currently still pursued but it was brought up over
> and over many times that we might want to use systemd as the "monitor"
> and let socket activation deal with the rest.
>
>> >  Other processes
>> > will connect to this server with a named connection (e.g. sssd.nss,
>> > sssd.backend.dom1, sssd.backend.dom2). We can then send message to this
>> > message bus (only one connection) and set destination to name (e.g. 
>> > sssd.nss
>> > to invalidate memcache). We can also send signals to this bus and it will
>> > broadcast it to all connections that listens to this signals. So, it is
>> > proper way how to do it. It will simplify things and allow us to send
>> > signals and have better IPC in general.
>> >
>> > I know we want to eventually get rid of the monitor, the process would stay
>> > as an sbus server. It would become a single point of failure, but the
>> > process can be restarted automatically by systemd in case of crash.
>> >
>> > Also here is a bonus question - do any of you remember why we use private
>> > server at all?
>>
>> In the very original design there was a "switch-board" process which
>> received a request from one component and forwarded it to the right
>> target. I guess at this time we didn't know a lot about DBus to
>> implement this properly. In the end we thought it was a useless overhead
>> and removed it. I think we didn't thought about signals to all components
>> or the backend sending requests to the frontends.
>>
>> > Why don't we connect to system message bus?
>>
>> Mainly because we do not trust it to handle plain text passwords and
>> other credentials with the needed care.
>
> That and because at some point there was a potential chicken-egg issue
> at startup, and also because we didn't want to handle additional error
> recovery if the system message bus was restarted.
>
> Fundamentally the system message bus is useful only for services
> offering a "public" service, otherwise it is just an overhead, and has
> security implications.
>
>> > I do not see any benefit in having a private server.
>
> There is no way to break into sssd via a bug in the system message bus.
> This is one good reason, aside for the other above.
>
> Fundamentally we needed a private structured messaging system we could
> easily integrate with tevent. The only usable option back then was
> dbus, and given we already had ideas about offering some plugic
> interface over the message bus we went that way so we could

[SSSD] Re: [8Dev] RHEL-8 systemd default presets

[Fabiano Fidêncio]

On Fri, May 18, 2018 at 4:08 PM, Tomas Mlcoch  wrote:

> Hi all,
>
> I would like to ask you to review systemd default presets we have in
> RHEL-8 redhat-release package:
>
> https://docs.google.com/document/d/1dAXKlx1Tv5eQ-fBm-
> cPc_x6YGWdifuokHr_JiXO-1fE/edit?usp=sharing
>
> There is a diff against what Fedora 28 has. If something should be
> added/removed/modified, please add a comment to the document. Thank you!
>
> Regards
> Tomas
>
> --
>
> Tomáš Mlčoch
>
> Senior Software Engineer, rcm
>
> Red Hat Brno 
>
> Purkyňova 71/99, 612 00 Brno
> 
>
> tmlc...@redhat.comIM: tmlcoch
> 
>

Ack from SSSD side!
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
Fedora Code of Conduct: https://getfedora.org/code-of-conduct.html
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives: 
https://lists.fedoraproject.org/archives/list/sssd-devel@lists.fedorahosted.org/message/WYIVUIZGZLHE2MUCZNLROUCJXDYJ66PG/

[SSSD] Re: Should we close several stalled PRs?

[Fabiano Fidêncio]

Jakub,

On Wed, May 16, 2018 at 10:46 AM, Jakub Hrozek  wrote:
> Hi,
> there are several PRs that were not touched for months. I would like to close 
> them with a friendly message that the reporter can reopen them if they are 
> inclined:
>
> https://github.com/SSSD/sssd/pull/175 - Add module for starting services
>  - does not apply anymore. It’s a good effort in general, though and I would 
> like to see someone revive it, there is too much code duplication in the 
> integration tests

I'm not sure about this one. I'd really like to hear from Lukáš here
whether he plans to work on this or not. If that's not the case, we
can close the PR and someone else could open a new PR when needed.

>
> https://github.com/SSSD/sssd/pull/247 - Subdomain inherit
>  - we want this change to be done eventually, but there’s no reason to keep 
> tracking this PR as long as we have a ticket upstream

Ack!

>
> https://github.com/SSSD/sssd/pull/387 - Setting ldap_sudo_include_regexp to 
> false
>  - no updates from the submitter for several months

Ack!

>
> https://github.com/SSSD/sssd/pull/410 - IPA: sanitize name in override search 
> filter - Backport to SSSD-1.13
>  - I don’t think anyone will respin this PR..

Ack! (And mea-culpa here that never got back to it).

>
> https://github.com/SSSD/sssd/pull/430 - tests: Remove the pysss.local 
> interface
>  - no updates from the submitter for several months
>

Ack!

> https://github.com/SSSD/sssd/pull/431 - Remove 
> ldap_groups_use_matching_rule_in_chain
>  - no updates from the submitter for several months

Ack!

>
> https://github.com/SSSD/sssd/pull/436 - subdomains: Remove code only used in 
> tests
>   - no updates from the submitter for several months

Ack!

>
> Is anyone against closing these?
> ___
> sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
> To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org


Note: "Ack!" meaning "let's close!" :-)
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] Re: Shall we revert test_resp_idle_timeout_shutdown_slow()?

[Fabiano Fidêncio]

On Fri, May 4, 2018 at 10:20 AM, Sumit Bose  wrote:
> On Fri, May 04, 2018 at 09:57:51AM +0200, Fabiano Fidêncio wrote:
>> This test was introduced in
>> https://github.com/SSSD/sssd/commit/ac9c3ad8228000140d80f91d4c5492d89d6e79f6
>> and its failing every now and then when running in our internal CI.
>>
>> I'd like to have it reverted, at least for now, and re-added later
>> whenever we have a more stable CI or a more stable test.
>>
>> Any objections?
>
> In general I agree, but I wonder if test_resp_idle_timeout_shutdown_slow
> would become more reliable if the timeout is just increased a bit. The
> comment says:
>
> # With the responder_idle_timeout set to 60 seconds, we need to wait at
> # least 90, because the internal timer ticks every timeout/2 seconds, so
> # so it would tick at 30, 60 and 90 seconds and the responder_idle_timeout
> # uses a greater-than comparison, so the 60-seconds tick wouldn't yet
> # trigger the process' shutdown.
>
> So is the 60s tick is missed and SSSD will really run 90s using exactly
> 90 in p.wait(timeout=90) might be a bit on the edge. I wonder if you can
> start some CI runs where e.g. p.wait(timeout=100) is used to see if
> this will pass more reliable?

I can fire a bunch of jobs this afternoon and see the results on Monday.

> Or is there a reason for the timeout being
> exactly 90s?

The reason is because the tests would be even slower, which is a
problem for some developers.



In any case, I'll increase the timeout and run 15~20 jobs over the weekend.
>
> bye,
> Sumit
>
>>
>> Best Regards,
>> --
>> Fabiano Fidêncio
>> ___
>> sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
>> To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
> ___
> sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
> To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] Shall we revert test_resp_idle_timeout_shutdown_slow()?

[Fabiano Fidêncio]

This test was introduced in
https://github.com/SSSD/sssd/commit/ac9c3ad8228000140d80f91d4c5492d89d6e79f6
and its failing every now and then when running in our internal CI.

I'd like to have it reverted, at least for now, and re-added later
whenever we have a more stable CI or a more stable test.

Any objections?

Best Regards,
--
Fabiano Fidêncio
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] Re: Github labels: Suggestion

[Fabiano Fidêncio]

On Thu, Mar 8, 2018 at 12:46 PM, Jakub Hrozek  wrote:
>
>
>> On 8 Mar 2018, at 12:34, Pavel Březina  wrote:
>>
>> On 03/08/2018 12:22 PM, Jakub Hrozek wrote:
>>>> On 8 Mar 2018, at 12:13, Fabiano Fidêncio  wrote:
>>>>
>>>> On Thu, Mar 8, 2018 at 12:00 PM, Jakub Hrozek  wrote:
>>>>>
>>>>>
>>>>>> On 8 Mar 2018, at 10:33, Fabiano Fidêncio  wrote:
>>>>>>
>>>>>> People,
>>>>>>
>>>>>> I've noticed that I'm getting a little bit lost with github and the
>>>>>> way SSSD has its tags organized there.
>>>>>>
>>>>>> As it may actually affect other people (and in case it does not, let's
>>>>>> just skip the following suggestion) ... I'd like to suggest the
>>>>>> following tags to the project:
>>>>>>
>>>>>> - Accepted: We already have it;
>>>>>>
>>>>>> - Rejected: We already have it.
>>>>>>
>>>>>> - Tests needed: This one can either replace the "Changes Requested"
>>>>>> (in case it's split in a few different tags) or be used together ...
>>>>>> but the idea is to identify that tests are missing from a PR without
>>>>>> going through the whole discussions happening there;
>>>>>
>>>>> What do you propose would be the action after tests needed? Should it be 
>>>>> a follow up PR, a ticket for the project, a ticket for downstream..?
>>>>>
>>>>
>>>> After the "Tests needed" tag is added the developer should either:
>>>> - Write the tests upstream (considering that we have infra for that,
>>>> which is not the case for all the PRs)
>>> Here I’m really worried that unless we have a ticket, this won’t happen. 
>>> Look at the “CI” milestone in pagure.. So I would say this case should 
>>> result in Changes requested, filing a ticket or asking downstream QE to 
>>> write a test.
>>
>> I would use this like this:
>>
>> a) You push a PR so it can be reviewed and you plan to provide tests later.
>>
>> b) You push a PR without tests and someone decides tests are mandatory for 
>> this PR.
>>
>>>> - Provide a "link" of the related downstream tests that were
>>>> broken/were added passing
>>>>
>>> This makes sense, although I would argue this should already be default. 
>>> But if you don’t think so, we can try the tag and see how it goes.
>>>> So, summing up, no ticket for the project, no ticket downstream ...
>>>> just making clear that the PR is stalled because "Tests are needed".
>>>> Does that make sense?
>>>>
>>>>> My worry about not supplying tests along with PRs is that the tests will 
>>>>> never be supplied..at least not in upstream..
>>>>
>>>> I understand why you're worried and I agree with that. See the answer
>>>> above and let me know if it fits your expectations.
>>>>
>>>>>
>>>>>>
>>>>>> - Depends on (or something similar): This one can either replace the
>>>>>> "Changes Requested" (in case it's split in a few different tags) or be
>>>>>> used together ... but the idea is to identify that we depend on
>>>>>> somework that still has to be done (either another PR, ticket or
>>>>>> something else that has to be implemented). Mind that I'm not sure
>>>>>> whether we'd be able to simply add a field saying what the PR depends
>>>>>> on …
>>>>>
>>>>> I think this makes sense. At least for a casual observer it would be 
>>>>> clear that there is no work needed on this P >>>
>>>>>>
>>>>>> - Postponed/Deferred: We have something similar for 2.0, but would be
>>>>>> nice to have a way to clearly see in which release we're going to take
>>>>>> a look into a specific PR without having to dig in the discussions.
>>>>>> Here we could also have 1.16.1, 1.16.2, 2.0, …
>>>>>
>>>>> Tags are cheap, we can even have a postponed/$version. I guess even 
>>>>> depends/$PR might be OK as long as we only had a handful of dependecies.
>>
>> Patch dependencies are not that common so we can just set ta

[SSSD] Re: Github labels: Suggestion

[Fabiano Fidêncio]

On Thu, Mar 8, 2018 at 12:44 PM, Jakub Hrozek  wrote:
>
>
>> On 8 Mar 2018, at 12:30, Fabiano Fidêncio  wrote:
>>
>> On Thu, Mar 8, 2018 at 12:22 PM, Jakub Hrozek  wrote:
>>>
>>>
>>>> On 8 Mar 2018, at 12:13, Fabiano Fidêncio  wrote:
>>>>
>>>> On Thu, Mar 8, 2018 at 12:00 PM, Jakub Hrozek  wrote:
>>>>>
>>>>>
>>>>>> On 8 Mar 2018, at 10:33, Fabiano Fidêncio  wrote:
>>>>>>
>>>>>> People,
>>>>>>
>>>>>> I've noticed that I'm getting a little bit lost with github and the
>>>>>> way SSSD has its tags organized there.
>>>>>>
>>>>>> As it may actually affect other people (and in case it does not, let's
>>>>>> just skip the following suggestion) ... I'd like to suggest the
>>>>>> following tags to the project:
>>>>>>
>>>>>> - Accepted: We already have it;
>>>>>>
>>>>>> - Rejected: We already have it.
>>>>>>
>>>>>> - Tests needed: This one can either replace the "Changes Requested"
>>>>>> (in case it's split in a few different tags) or be used together ...
>>>>>> but the idea is to identify that tests are missing from a PR without
>>>>>> going through the whole discussions happening there;
>>>>>
>>>>> What do you propose would be the action after tests needed? Should it be 
>>>>> a follow up PR, a ticket for the project, a ticket for downstream..?
>>>>>
>>>>
>>>> After the "Tests needed" tag is added the developer should either:
>>>> - Write the tests upstream (considering that we have infra for that,
>>>> which is not the case for all the PRs)
>>>
>>> Here I’m really worried that unless we have a ticket, this won’t happen. 
>>> Look at the “CI” milestone in pagure.. So I would say this case should 
>>> result in Changes requested, filing a ticket or asking downstream QE to 
>>> write a test.
>>
>> Hmm. My original thought is that the PR wouldn't even be pushed
>> without the tests (as it happened already with a few PRs) ... not that
>> we'd push and leave the tag there.
>
> So it would mean “the code looks good and will be pushed as long as you sort 
> out tests one way or another” ?

I do think so. But this is something that must be agreed between us (the team).

>
>>
>>>
>>>> - Provide a "link" of the related downstream tests that were
>>>> broken/were added passing
>>>>
>>>
>>> This makes sense, although I would argue this should already be default. 
>>> But if you don’t think so, we can try the tag and see how it goes.
>>
>> Hmm. Indeed. I guess we can postpone this at least for now and focus
>> on your "downstream tests passed" tag ... which would be a better
>> investment of time.
>> Agreed?
>
> Well, I don’t think the tests passed tag would come soon (as in, not this 
> week, not the next one). If you see a use-case for more tags, use them. But 
> as Pavel said, I would at least initially add a comment what do you want from 
> the other part of the PR until we find out how the process works best for us.
>
> I mean, I don’t want to impose my workflow on others. If you think it makes 
> sense, let’s try it. Worst thing, we stop using the tags and remove them..
>
>>
>>>
>>>> So, summing up, no ticket for the project, no ticket downstream ...
>>>> just making clear that the PR is stalled because "Tests are needed".
>>>> Does that make sense?
>>>>
>>>>> My worry about not supplying tests along with PRs is that the tests will 
>>>>> never be supplied..at least not in upstream..
>>>>
>>>> I understand why you're worried and I agree with that. See the answer
>>>> above and let me know if it fits your expectations.
>>>>
>>>>>
>>>>>>
>>>>>> - Depends on (or something similar): This one can either replace the
>>>>>> "Changes Requested" (in case it's split in a few different tags) or be
>>>>>> used together ... but the idea is to identify that we depend on
>>>>>> somework that still has to be done (either another PR, ticket or
>>>>>> something else that has to be implemented). Mind that I'

[SSSD] Re: Github labels: Suggestion

[Fabiano Fidêncio]

On Thu, Mar 8, 2018 at 12:22 PM, Jakub Hrozek  wrote:
>
>
>> On 8 Mar 2018, at 12:13, Fabiano Fidêncio  wrote:
>>
>> On Thu, Mar 8, 2018 at 12:00 PM, Jakub Hrozek  wrote:
>>>
>>>
>>>> On 8 Mar 2018, at 10:33, Fabiano Fidêncio  wrote:
>>>>
>>>> People,
>>>>
>>>> I've noticed that I'm getting a little bit lost with github and the
>>>> way SSSD has its tags organized there.
>>>>
>>>> As it may actually affect other people (and in case it does not, let's
>>>> just skip the following suggestion) ... I'd like to suggest the
>>>> following tags to the project:
>>>>
>>>> - Accepted: We already have it;
>>>>
>>>> - Rejected: We already have it.
>>>>
>>>> - Tests needed: This one can either replace the "Changes Requested"
>>>> (in case it's split in a few different tags) or be used together ...
>>>> but the idea is to identify that tests are missing from a PR without
>>>> going through the whole discussions happening there;
>>>
>>> What do you propose would be the action after tests needed? Should it be a 
>>> follow up PR, a ticket for the project, a ticket for downstream..?
>>>
>>
>> After the "Tests needed" tag is added the developer should either:
>> - Write the tests upstream (considering that we have infra for that,
>> which is not the case for all the PRs)
>
> Here I’m really worried that unless we have a ticket, this won’t happen. Look 
> at the “CI” milestone in pagure.. So I would say this case should result in 
> Changes requested, filing a ticket or asking downstream QE to write a test.

Hmm. My original thought is that the PR wouldn't even be pushed
without the tests (as it happened already with a few PRs) ... not that
we'd push and leave the tag there.

>
>> - Provide a "link" of the related downstream tests that were
>> broken/were added passing
>>
>
> This makes sense, although I would argue this should already be default. But 
> if you don’t think so, we can try the tag and see how it goes.

Hmm. Indeed. I guess we can postpone this at least for now and focus
on your "downstream tests passed" tag ... which would be a better
investment of time.
Agreed?

>
>> So, summing up, no ticket for the project, no ticket downstream ...
>> just making clear that the PR is stalled because "Tests are needed".
>> Does that make sense?
>>
>>> My worry about not supplying tests along with PRs is that the tests will 
>>> never be supplied..at least not in upstream..
>>
>> I understand why you're worried and I agree with that. See the answer
>> above and let me know if it fits your expectations.
>>
>>>
>>>>
>>>> - Depends on (or something similar): This one can either replace the
>>>> "Changes Requested" (in case it's split in a few different tags) or be
>>>> used together ... but the idea is to identify that we depend on
>>>> somework that still has to be done (either another PR, ticket or
>>>> something else that has to be implemented). Mind that I'm not sure
>>>> whether we'd be able to simply add a field saying what the PR depends
>>>> on …
>>>
>>> I think this makes sense. At least for a casual observer it would be clear 
>>> that there is no work needed on this PR.
>>>
>>>>
>>>> - Postponed/Deferred: We have something similar for 2.0, but would be
>>>> nice to have a way to clearly see in which release we're going to take
>>>> a look into a specific PR without having to dig in the discussions.
>>>> Here we could also have 1.16.1, 1.16.2, 2.0, …
>>>
>>> Tags are cheap, we can even have a postponed/$version. I guess even 
>>> depends/$PR might be OK as long as we only had a handful of dependecies.
>>>>
>>>> - Reworked: Although just removing the "Changes Requested" label is
>>>> fine, maybe having a tag explicitly saying that something was Reworked
>>>> would be a clean way to differentiate between new PRs and PRs which
>>>> have been through a review already …
>>>
>>> I don’t know how this tag would be used, could you give me an example, 
>>> please?
>>
>> I usually have no idea (just by a quick look on github) whether a PR
>> has been re-worked or it's a new PR that's never been reviewed.
>> My impression is 

[SSSD] Re: Github labels: Suggestion

[Fabiano Fidêncio]

On Thu, Mar 8, 2018 at 12:00 PM, Jakub Hrozek  wrote:
>
>
>> On 8 Mar 2018, at 10:33, Fabiano Fidêncio  wrote:
>>
>> People,
>>
>> I've noticed that I'm getting a little bit lost with github and the
>> way SSSD has its tags organized there.
>>
>> As it may actually affect other people (and in case it does not, let's
>> just skip the following suggestion) ... I'd like to suggest the
>> following tags to the project:
>>
>> - Accepted: We already have it;
>>
>> - Rejected: We already have it.
>>
>> - Tests needed: This one can either replace the "Changes Requested"
>> (in case it's split in a few different tags) or be used together ...
>> but the idea is to identify that tests are missing from a PR without
>> going through the whole discussions happening there;
>
> What do you propose would be the action after tests needed? Should it be a 
> follow up PR, a ticket for the project, a ticket for downstream..?
>

After the "Tests needed" tag is added the developer should either:
- Write the tests upstream (considering that we have infra for that,
which is not the case for all the PRs)
- Provide a "link" of the related downstream tests that were
broken/were added passing

So, summing up, no ticket for the project, no ticket downstream ...
just making clear that the PR is stalled because "Tests are needed".
Does that make sense?

> My worry about not supplying tests along with PRs is that the tests will 
> never be supplied..at least not in upstream..

I understand why you're worried and I agree with that. See the answer
above and let me know if it fits your expectations.

>
>>
>> - Depends on (or something similar): This one can either replace the
>> "Changes Requested" (in case it's split in a few different tags) or be
>> used together ... but the idea is to identify that we depend on
>> somework that still has to be done (either another PR, ticket or
>> something else that has to be implemented). Mind that I'm not sure
>> whether we'd be able to simply add a field saying what the PR depends
>> on …
>
> I think this makes sense. At least for a casual observer it would be clear 
> that there is no work needed on this PR.
>
>>
>> - Postponed/Deferred: We have something similar for 2.0, but would be
>> nice to have a way to clearly see in which release we're going to take
>> a look into a specific PR without having to dig in the discussions.
>> Here we could also have 1.16.1, 1.16.2, 2.0, …
>
> Tags are cheap, we can even have a postponed/$version. I guess even 
> depends/$PR might be OK as long as we only had a handful of dependecies.
>>
>> - Reworked: Although just removing the "Changes Requested" label is
>> fine, maybe having a tag explicitly saying that something was Reworked
>> would be a clean way to differentiate between new PRs and PRs which
>> have been through a review already …
>
> I don’t know how this tag would be used, could you give me an example, please?

I usually have no idea (just by a quick look on github) whether a PR
has been re-worked or it's a new PR that's never been reviewed.
My impression is that having the "Reworked" tag would make simpler for
people to jump in and do a follow-up review on what has been addressed
in the first round(s) of review and then give their ACK instead of
just leaving it for the reviewer. Of course, the same can be achieved
without that tag ... so, it's just something that looks more
"organized" to me.

>
>>
>> Does the suggestion make sense? In case we have an agreement about
>> this topic, may I re-tag our PRs and start using those new tags from
>> new PRs?
>
> Another tag I was thinking of was “passes downstream tests”. With the amount 
> of time our downstream tests take, I’m not even sure how to integrate them 
> with the usual github flow like travis or centos CI use. So I was thinking 
> about a bot that would nightly scan PRs that have neither “pass” or “fail” 
> tag, bundle those up in an RPM, run the nightly tests and report back using a 
> tag.

I really like the idea!

Another tag that may be added is something like "Urgent" for PRs that
are *really* *needed* for some specific reason (downstream, release,
etc ...)

>
>>
>> Best Regards,
>> --
>> Fabiano Fidêncio
>> ___
>> sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
>> To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
> ___
> sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
> To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

Let me know in case I was not able to answer all your questions.

Best Regards,
-- 
Fabiano Fidêncio
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] Github labels: Suggestion

[Fabiano Fidêncio]

People,

I've noticed that I'm getting a little bit lost with github and the
way SSSD has its tags organized there.

As it may actually affect other people (and in case it does not, let's
just skip the following suggestion) ... I'd like to suggest the
following tags to the project:

- Accepted: We already have it;

- Rejected: We already have it.

- Tests needed: This one can either replace the "Changes Requested"
(in case it's split in a few different tags) or be used together ...
but the idea is to identify that tests are missing from a PR without
going through the whole discussions happening there;

- Depends on (or something similar): This one can either replace the
"Changes Requested" (in case it's split in a few different tags) or be
used together ... but the idea is to identify that we depend on
somework that still has to be done (either another PR, ticket or
something else that has to be implemented). Mind that I'm not sure
whether we'd be able to simply add a field saying what the PR depends
on ...

- Postponed/Deferred: We have something similar for 2.0, but would be
nice to have a way to clearly see in which release we're going to take
a look into a specific PR without having to dig in the discussions.
Here we could also have 1.16.1, 1.16.2, 2.0, ...

- Reworked: Although just removing the "Changes Requested" label is
fine, maybe having a tag explicitly saying that something was Reworked
would be a clean way to differentiate between new PRs and PRs which
have been through a review already ...

Does the suggestion make sense? In case we have an agreement about
this topic, may I re-tag our PRs and start using those new tags from
new PRs?

Best Regards,
-- 
Fabiano Fidêncio
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] Re: RFC: 1.16.1 release notes

[Fabiano Fidêncio]

next lookup. This
>prevents time outs in case SSSD was discovering the site using the global
>list of DCs where some of the global DCs might be unreachable. (#3265)
>
>  * SSSD no longer starts the implicit file domain when configured with
>``id_provider=proxy`` and ``proxy_lib_name=files``. This bug prevented
>SSSD from being used in setups that combine identities from UNIX files
>together with authentication against a remote source unless a files
>domain was explicitly configured (#3590)
>
>  * The IPA provider can handle switching between different ID views better
>(#3579)
>
>  * Previously, the IPA provider kept SSH public keys and certificates from
>an ID view in its cache and returned them even if the public key or
>certificate was then removed from the override (#3602, #3603)
>
>  * FleetCommander profiles coming from IPA are applied even if they are
>assigned globally (to ``category: ALL``), previously, only profiles
>assigned to a host or a hostgroup were applied (#3449)
>
>  * It is now possible to reset an expired password for users with 2FA
>authentication enabled (#3585)
>
>  * A bug in the AD provider which could have resulted in built-in AD groups
>being incorrectly cached was fixed (#3610)
>
>  * The SSSD watchdog can now cope better with time drifts (#3285)
>
>  * The ``nss_sss`` NSS module's return codes for invalid cases were fixed
>
>  * A bug in the LDAP provider that prevented setups with id_provider=proxy
>and auth_provider=ldap with LDAP servers that do not allow anonymous
>binds from working was fixed (#3451)
>
> Packaging Changes
> -
>  * The FleetCommander desktop profile path now uses stricter permissions,
>751 instead of 755 (#3621)
>
>  * A new option ``--logger`` was added to the ``sssd(8)`` binary. This option
>obsoletes old options such as ``--debug-to-files``, although the old 
> options
>are kept for backwards compatibility.
>
> Documentation Changes
> -
> There are no notable documentation changes such as options changing default
> values etc in this release.
>
> Tickets Fixed
> -
>
> Detailed Changelog
> --
> ___
> sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
> To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

Best Regards,
-- 
Fabiano Fidêncio
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] Re: Fleet Commander: design changes due to the drop of DAC_OVERRIDE capability

[Fabiano Fidêncio]

On Thu, Feb 1, 2018 at 11:25 AM, Sumit Bose  wrote:

> On Wed, Jan 31, 2018 at 10:01:15AM -0500, Simo Sorce wrote:
> > On Wed, 2018-01-31 at 14:55 +0100, Fabiano Fidêncio wrote:
> > > Sumit,
> > >
> > > On Fri, Jan 26, 2018 at 9:01 PM, Sumit Bose  wrote:
> >
> > [..]
> >
> > > I'll leave the other 2 questions to Simo. :-)
> > >
> > > I wonder if there is a chance that e.g. a signal can force to backend
> to do
> > > > something else when running with the changed euid?
> >
> > If I am not wrong we pipe all signals back quickly into tevent events,
> > so signals should not cause issues. This should be carefully checked
> > but I would be surprised if any of our signal handlers do any I/O
> > besides triggering a tevent via pipe, it is not recommended to do that
> > anyway.
> > On whether seteuid influences the delivery of signals I am not 100%
> > sure, but I think only a real uid change will affect that, not the
> > effective uid. (on the receiving side which is what we care about).
> >
> > So I think we are OK here.
> >
> > That said, are we sure we retain CAP_SETUID ?
> >
> > > > Is there something you do not like about the approach of PR498?
> >
> > Here my comments from the tracker:
> >
> > FWIW,
> > I see nothing wrong in keeping CAP_DAC_OVERRIDE, the process is running
> > as root and can impersonate users at any time so removing this
> > capability does not change the security stance.
> >
> > However it may make some errors less severe if people use seteuid() to
> > change the effective id of the user, because then the process cannot
> > alter data or access compartmentalized data by mistake.
> >
> > Opening permissions on the files negates the benefit of using seteuid()
> > so it is the least desirable way to handle this "issue".
>
> Thank you for the explanations, so I'm fine with using seteuid()/setegid().
>


I've updated the design page and send the patches to FleetCommander
developers so they can test it.
I'll update the github PR as soon as I have their feedback.

So, summing up, thanks for the inputs!


>
> Fabiano, nevertheless I'd like to ask some questions about the profile
> files, please let me know if I should better ask the Fleetcommander
> developers.
>

> Is it expected that the user can change the data in the file? Or is it
> the other way round that it would be better that the user only has
> read-only access so that the admin can force some settings?
>

I'd say not. So, maybe going for a read-only solution would be better.
Here, I'll confirm this with Oliver.


> Is it possible that there is any sensitive information in the profile
> files or would it be ok if anyone can read it? (hm, while writing the
> question I started wondering if I would be happy if everyone knows that
> my desktop background is pink_unicorns.jpg?).
>

This is a quite good question and I'll redirect it to Oliver and get back
to you as soon as I have both answers.


>
> bye,
> Sumit
> >
> > HTH,
> > Simo.
> >
> > --
> > Simo Sorce
> > Sr. Principal Software Engineer
> > Red Hat, Inc
> > ___
> > sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
> > To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
> ___
> sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
> To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
>
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] Re: Fleet Commander: design changes due to the drop of DAC_OVERRIDE capability

[Fabiano Fidêncio]

Sumit,

On Fri, Jan 26, 2018 at 9:01 PM, Sumit Bose  wrote:

> On Fri, Jan 26, 2018 at 03:06:31PM +0100, Fabiano Fidêncio wrote:
> > On Mon, Jan 22, 2018 at 3:20 PM, Simo Sorce  wrote:
> >
> > > On Mon, 2018-01-22 at 15:10 +0100, Fabiano Fidêncio wrote:
> > > > People,
> > > >
> > > > Let's start with the context of this email:
> > > > https://bugzilla.redhat.com/show_bug.cgi?id=1536854
> > > > So, seems that even without knowing that, I've relied on
> CAP_DAC_OVERRIDE
> > > > in order to have the Fleet Commander integration working as expected
> and
> > > in
> > > > the implementation details of this feature.
> > > >
> > > > The desktop profiles are stored in a dir like:
> > > > /var/lib/sss/deskprofile/$domain/$user/$profile.
> > > >
> > > > Currently, the way I've been creating those are:
> > > > $domain = 755 (root:root)
> > > > $user = 600 ($user:$user_group)
> > > > $profile = 600 ($user:$user_group)
> > > >
> > > > Now, as mentioned in the bugzilla linked in this email, the current
> code
> > > > fails with an EACCES.
> > > >
> > > > With all this background, I'd like to discuss what's the best
> approach to
> > > > take. I've opened a PR (https://github.com/SSSD/sssd/pull/498) which
> > > makes
> > > > everything work again, but does the following changes:
> > > >
> > > > $domain = 755 (root:root) -- NO changes here
> > > > $user = 770 ($user:root) --> changed from 600 ($user:$user_group)
> > > > $profile = 660 ($user:root) --> changed from 600 ($user:$user_group)
> > > >
> > > > This is one way to solve the issue suggested at
> > > > https://bugzilla.redhat.com/show_bug.cgi?id=1536854#c5.
> > > >
> > > > Another suggestion, also mentioned in the bugzilla, would be to only
> > > > fchown()/fchmod() the files/dirs *after* all the operations we do are
> > > over.
> > > >
> > > > Is there any other suggestion? Whatever comes out of this discussion
> will
> > > > be used to update the feature's design page accordingly.
> > >
> > > Change euid to that of the user during operations, leave the
> > > permissions strict ?
> > >
> >
> > Does the other developers agree with Simo's proposal? If yes, I'll just
> > update the DesignPage accordingly and provide the patches.
>
>
Firstly, sorry it took me so long to reply (I really thought I had done it
already ... or maybe I have done it mentally only). :-/



> This happens in the main backend process not in a child, right?


Yes, this happens in the main backend process.

I'll leave the other 2 questions to Simo. :-)

I wonder if there is a chance that e.g. a signal can force to backend to do
> something else when running with the changed euid?
>

> Is there something you do not like about the approach of PR498?
>
> bye,
> Sumit
>
> >
> >
> > >
> > > Simo.
> > >
> > > --
> > > Simo Sorce
> > > Sr. Principal Software Engineer
> > > Red Hat, Inc
> > > ___
> > > sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
> > > To unsubscribe send an email to sssd-devel-leave@lists.
> fedorahosted.org
> > >
>
> > ___
> > sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
> > To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
> ___
> sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
> To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
>
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] Re: Fleet Commander: design changes due to the drop of DAC_OVERRIDE capability

[Fabiano Fidêncio]

On Mon, Jan 22, 2018 at 3:20 PM, Simo Sorce  wrote:

> On Mon, 2018-01-22 at 15:10 +0100, Fabiano Fidêncio wrote:
> > People,
> >
> > Let's start with the context of this email:
> > https://bugzilla.redhat.com/show_bug.cgi?id=1536854
> > So, seems that even without knowing that, I've relied on CAP_DAC_OVERRIDE
> > in order to have the Fleet Commander integration working as expected and
> in
> > the implementation details of this feature.
> >
> > The desktop profiles are stored in a dir like:
> > /var/lib/sss/deskprofile/$domain/$user/$profile.
> >
> > Currently, the way I've been creating those are:
> > $domain = 755 (root:root)
> > $user = 600 ($user:$user_group)
> > $profile = 600 ($user:$user_group)
> >
> > Now, as mentioned in the bugzilla linked in this email, the current code
> > fails with an EACCES.
> >
> > With all this background, I'd like to discuss what's the best approach to
> > take. I've opened a PR (https://github.com/SSSD/sssd/pull/498) which
> makes
> > everything work again, but does the following changes:
> >
> > $domain = 755 (root:root) -- NO changes here
> > $user = 770 ($user:root) --> changed from 600 ($user:$user_group)
> > $profile = 660 ($user:root) --> changed from 600 ($user:$user_group)
> >
> > This is one way to solve the issue suggested at
> > https://bugzilla.redhat.com/show_bug.cgi?id=1536854#c5.
> >
> > Another suggestion, also mentioned in the bugzilla, would be to only
> > fchown()/fchmod() the files/dirs *after* all the operations we do are
> over.
> >
> > Is there any other suggestion? Whatever comes out of this discussion will
> > be used to update the feature's design page accordingly.
>
> Change euid to that of the user during operations, leave the
> permissions strict ?
>

Does the other developers agree with Simo's proposal? If yes, I'll just
update the DesignPage accordingly and provide the patches.


>
> Simo.
>
> --
> Simo Sorce
> Sr. Principal Software Engineer
> Red Hat, Inc
> ___
> sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
> To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
>
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] Fleet Commander: design changes due to the drop of DAC_OVERRIDE capability

[Fabiano Fidêncio]

People,

Let's start with the context of this email:
https://bugzilla.redhat.com/show_bug.cgi?id=1536854
So, seems that even without knowing that, I've relied on CAP_DAC_OVERRIDE
in order to have the Fleet Commander integration working as expected and in
the implementation details of this feature.

The desktop profiles are stored in a dir like:
/var/lib/sss/deskprofile/$domain/$user/$profile.

Currently, the way I've been creating those are:
$domain = 755 (root:root)
$user = 600 ($user:$user_group)
$profile = 600 ($user:$user_group)

Now, as mentioned in the bugzilla linked in this email, the current code
fails with an EACCES.

With all this background, I'd like to discuss what's the best approach to
take. I've opened a PR (https://github.com/SSSD/sssd/pull/498) which makes
everything work again, but does the following changes:

$domain = 755 (root:root) -- NO changes here
$user = 770 ($user:root) --> changed from 600 ($user:$user_group)
$profile = 660 ($user:root) --> changed from 600 ($user:$user_group)

This is one way to solve the issue suggested at
https://bugzilla.redhat.com/show_bug.cgi?id=1536854#c5.

Another suggestion, also mentioned in the bugzilla, would be to only
fchown()/fchmod() the files/dirs *after* all the operations we do are over.

Is there any other suggestion? Whatever comes out of this discussion will
be used to update the feature's design page accordingly.

Best Regards,
-- 
Fabiano Fidêncio
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] Re: Every PR should come with a test.

[Fabiano Fidêncio]

On Wed, Jan 10, 2018 at 2:28 PM, Jakub Hrozek  wrote:

> On Wed, Jan 10, 2018 at 10:52:56AM +0100, Sumit Bose wrote:
> > On Wed, Jan 10, 2018 at 10:04:49AM +0100, Fabiano Fidêncio wrote:
> > > People,
> > >
> > > Ideally every PR should come with a test (unit, integration, ...), but
> > > unfortunately we're a little bit far from the ideal situation. Thus,
> I'd
> > > like to ask whether we have documented somewhere (apart from our code
> > > itself) which are the parts of SSSD code that can be easily tested by
> our
> > > unit and integration tests.
> > >
> > > My understanding (and please, correct me if I'm mistaken) is that by
> having
> > > a updated list of our tests coverage would help any newcomer submitting
> > > something new to the project and also not so experienced reviewers to
> > > easily detect that a PR touching this or that part would need a test
> > > (otherwise we don't even start reviewing the patches).
> > >
> > > So, does this list exist somewhere? Would be a fair request to create
> this
> > > list and have it linked to our "Contribute" page?
> >
> > iirc the CI scripts create coverage data. Would this help?
>
> I guess partially, because it looks like only unit tests (and not
> integration tests) are generating the coverage.
>
> Also, I think this question comes from
> https://github.com/SSSD/sssd/pull/476 and there the test would have to
> be written in the IPA tree (there are already some tests for netgroups)
>

Yes, the question comes from PR 476 but it's more general than that.
IMO (and here I may be totally wrong) having a list of what we cover,
what's covered by IPA, what's covered downstream would be useful to any
*non* experienced reviewer (which is my case).


> ___
> sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
> To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
>
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] Every PR should come with a test.

[Fabiano Fidêncio]

People,

Ideally every PR should come with a test (unit, integration, ...), but
unfortunately we're a little bit far from the ideal situation. Thus, I'd
like to ask whether we have documented somewhere (apart from our code
itself) which are the parts of SSSD code that can be easily tested by our
unit and integration tests.

My understanding (and please, correct me if I'm mistaken) is that by having
a updated list of our tests coverage would help any newcomer submitting
something new to the project and also not so experienced reviewers to
easily detect that a PR touching this or that part would need a test
(otherwise we don't even start reviewing the patches).

So, does this list exist somewhere? Would be a fair request to create this
list and have it linked to our "Contribute" page?

Best Regards,
-- 
Fabiano Fidêncio
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] Re: SSSD Virtual Test Suite

[Fabiano Fidêncio]

On Tue, Nov 14, 2017 at 11:17 AM, Pavel Březina  wrote:
> On 11/13/2017 05:43 PM, Fabiano Fidêncio wrote:
>>
>> On Mon, Nov 13, 2017 at 11:16 AM, Pavel Březina 
>> wrote:
>>>
>>> Hello,
>>>
>>> It took me a lot longer than I expected but here it is at last. This is
>>> my
>>> set of scripts that use vagrant and Ansible to automatically provision
>>> virtual environment that I use to develop and test SSSD.
>>>
>>> To create this environment you only need to run one command:
>>> $ ./setup.sh
>>>
>>> and after a while you have several machines provisioned and ready. This
>>> machines include LDAP, IPA and AD servers with one machine dedicated to
>>> SSSD. This machine is already enrolled to those servers.
>>>
>>> To start building and/or testing SSSD with all available providers, you
>>> can
>>> just run:
>>> $ vagrant ssh client
>>>
>>> Additionally, it allows you to automatically source your set of scripts
>>> on
>>> each login and access IPA web-ui from your browser.
>>>
>>> I tried to make the provisioning as fast as possible but it still takes
>>> approximately one hour on my machine. So be patient.
>>>
>>> Any ideas and patches for improvements are welcomed.
>>>
>>>
>>> The source is available at:
>>> https://github.com/pbrezina/sssd-test-suite
>>
>>
>>
>> Okay, I've found some small issues related to the readme and some few
>> annoyances while trying to run the scripts.
>>
>> For the former, I'll open some PRs. For the latter, it's worth to
>> discuss what's your preference/understand better the requirements:
>>
>> 1) Why do have to run the script as root? AFAIU there's some way to
>> escalate privileges when running an Ansible script (example, running
>> sudo whenever it's needed). Is that something desired?
>
>
> Scripts do not require root privileges, Ansible will use sudo when needed.
> But libvirt does, so everytime you run vagrant you have to provide root
> password, unless you change it through policy kit.
>
> Given the fact that the primary use case is for developers I didn't spend
> time on making this configurable and ansible will create a polkit rule to
> always allow access.
>
>> 2) Restarting NetworkManager is quite intrusive, mainly without any
>> kind of warning.
>
>
> Please, send a PR for readme, I'll see if there can be any prompt by
> Ansible.
>
>> 3) Why do we need Vagrant 2.0 as the minimum version?
>
>
> Communications with Windows machine require WinRM protocol which, as I
> understood, is not yet handled by older vagrant versions. Vagrant 2 was
> recommended by the windows boxes creator.
>
> Maybe it will work with lower version, I did not test it.
>
>> 4) The guide was written for Fedora systems ... what's the reason to
>> choose Fedora over CentOS?
>
>
> I run Fedora on my machine, did not test it on other systems.
>
>> It will take a long time to download all the vagrant images, but I'll
>> get back here with the feedback as soon as this process is over.
>
>
> I hope it will work. Each time I though I'm finished, some other problem has
> appeared. But this version got stable on my machine.
>
>> Amazing initiative! Thanks a lot, Pavel!
>
>
> Thank you.

Another important thing. In case the other developers agree I really
would like to have this *officially* as part of SSSD group on
pagure/github as it's something quite important for the project and
because this can be a nice way to provision the environment where the
upstream tests could run.

>
> ___
> sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
> To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] Re: SSSD Virtual Test Suite

[Fabiano Fidêncio]

On Mon, Nov 13, 2017 at 11:16 AM, Pavel Březina  wrote:
> Hello,
>
> It took me a lot longer than I expected but here it is at last. This is my
> set of scripts that use vagrant and Ansible to automatically provision
> virtual environment that I use to develop and test SSSD.
>
> To create this environment you only need to run one command:
> $ ./setup.sh
>
> and after a while you have several machines provisioned and ready. This
> machines include LDAP, IPA and AD servers with one machine dedicated to
> SSSD. This machine is already enrolled to those servers.
>
> To start building and/or testing SSSD with all available providers, you can
> just run:
> $ vagrant ssh client
>
> Additionally, it allows you to automatically source your set of scripts on
> each login and access IPA web-ui from your browser.
>
> I tried to make the provisioning as fast as possible but it still takes
> approximately one hour on my machine. So be patient.
>
> Any ideas and patches for improvements are welcomed.
>
>
> The source is available at:
> https://github.com/pbrezina/sssd-test-suite


Okay, I've found some small issues related to the readme and some few
annoyances while trying to run the scripts.

For the former, I'll open some PRs. For the latter, it's worth to
discuss what's your preference/understand better the requirements:

1) Why do have to run the script as root? AFAIU there's some way to
escalate privileges when running an Ansible script (example, running
sudo whenever it's needed). Is that something desired?

2) Restarting NetworkManager is quite intrusive, mainly without any
kind of warning.

3) Why do we need Vagrant 2.0 as the minimum version?

4) The guide was written for Fedora systems ... what's the reason to
choose Fedora over CentOS?

It will take a long time to download all the vagrant images, but I'll
get back here with the feedback as soon as this process is over.

Amazing initiative! Thanks a lot, Pavel!


> ___
> sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
> To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] Re: Which tickets do we need to close before the release of the next upstream version?

[Fabiano Fidêncio]

On Thu, Oct 12, 2017 at 4:01 PM, Fabiano Fidêncio  wrote:
> On Wed, Oct 11, 2017 at 5:29 PM, Jakub Hrozek  wrote:
>> Hi,
>>
>> Because of downstream deadline, we need to release the next SSSD tarball
>> by the end of next week, or on the beginning of the next one at latest.
>>
>> And the 1.16.0 milestone is still really big and there are still tickets
>> in 1.15.4, so I'm trying to trim 1.16.0 and merge it with 1.15.4,
>> because realistically, I don't think we have another choice.
>>
>> Here's what I propose should *stay* in 1.16.0. Any other tickets should
>> be moved to 1.16.1:
>> - https://pagure.io/SSSD/sssd/issue/3507 - Long search filters are
>>   created during IPA sudo command + command group retrieval
>> - there is a PR which pbrezina is assigned to. Let's keep the
>>   ticket for now, if the PR can't be reviewed in time, we will
>>   move the ticket. Downstreams can always patch their tarball
>>
>> - https://pagure.io/SSSD/sssd/issue/3496 - [RFE] Add a configuration
>>   option to SSSD to disable the memory cache
>> - there is a PR already, let's review it
>>
>> - https://pagure.io/SSSD/sssd/issue/1872 - [RFE] Support User
>>   Private Groups for main domains, too
>> - there is a PR already, let's review it
>>
>> - https://pagure.io/SSSD/sssd/issue/3503 - Do not index objectclass,
>>   add and index objectcategory instead
>> - this is WIP, there was already a PR at one point
>>
>> - https://pagure.io/SSSD/sssd/issue/2653 - Group renaming issue when
>>   "id_provider = ldap" is set.
>> - there is a PR already and a test, let's review both
>>
>> - https://pagure.io/SSSD/sssd/issue/2727 - Add a memcache for
>>   SID-by-id lookups
>> - Sumit indicated that this is WIP, but we can move the ticket
>>   to 1.16.1 if the PR doesn't make the cut
>>
>> - https://pagure.io/SSSD/sssd/issue/3468 - SSSD doesn't use AD
>>   global catalog for gidnumber lookup, resulting in unacceptable
>>   delay for large forests
>> - I'm working on this now
>>
>> - https://pagure.io/SSSD/sssd/issue/3307 -  RFE: Log to syslog when
>>   sssd cannot contact servers, goes offline
>> - Fabiano is working on this
>>
>> - https://pagure.io/SSSD/sssd/issue/3265 - [RFE] sssd should
>>   remember DNS sites from first search
>> - I'd like to have this ticket fixed for the next release and
>>   per discussion with Pavel, it's not a lot of work either
>>
>> - https://pagure.io/SSSD/sssd/issue/3264 - [RFE] Make 2FA prompting
>>   configurable
>> - same as above
>>
>> At the same time, I would like to change the "Milestone" tag of all bugs
>> that were fixed in 1.15.4 to 1.16.0 and do not release another 1.15.x
>> version at all, but go straight to 1.16.0.
>>
>> And finally, all the unfished tickets in 1.15.4 would be moved to 1.16.1
>> and at the same time triaged, because there are some tickets that can be
>> closed or moved to a future milestone completely.
>>
>> Opinions?
>
> Would be really nice if we could have the PR#275
> (https://github.com/SSSD/sssd/pull/275) merged as well as part of 1.16
> release, as the code seems good (according to the reviewer), has tests
> and comes from an external contributor.

So, after 4 days I have to tell that I'm pretty much worried that we
will *not* reach our theoretical goal for 1.16.

From the bugs in the list, I'm concerned about:

- https://pagure.io/SSSD/sssd/issue/2653 - Group renaming issue when
 "id_provider = ldap" is set.
  - there is a PR already and a test, let's review both
  Although there's a PR and a test written, this bug is stalled
*again*. I'm sincerely failing to see the interest from SSSD team in
order to have it merged;

There are a few other PRs that would be nice to have merged in the tarball:
 - https://pagure.io/SSSD/sssd/pull-request/3320
   What's the status of this one? Is it planned to be part of 1.16?
What's needed in order to have it merged? What has been already done?
I failed to find any info or status in the PR itself.

 - https://pagure.io/SSSD/sssd/issue/3463
   This issue is related to the enumeration tests which are a blocker
for this release. I've tried to work on this and failed due to the
lack of knowledge both from the code and from the decisions
historically taken when this code has been implemented. Would be
really nice if someone could take a look on this one before the
release.

Best Regards,
--
Fabiano Fidêncio
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] Re: Which tickets do we need to close before the release of the next upstream version?

[Fabiano Fidêncio]

On Wed, Oct 11, 2017 at 5:29 PM, Jakub Hrozek  wrote:
> Hi,
>
> Because of downstream deadline, we need to release the next SSSD tarball
> by the end of next week, or on the beginning of the next one at latest.
>
> And the 1.16.0 milestone is still really big and there are still tickets
> in 1.15.4, so I'm trying to trim 1.16.0 and merge it with 1.15.4,
> because realistically, I don't think we have another choice.
>
> Here's what I propose should *stay* in 1.16.0. Any other tickets should
> be moved to 1.16.1:
> - https://pagure.io/SSSD/sssd/issue/3507 - Long search filters are
>   created during IPA sudo command + command group retrieval
> - there is a PR which pbrezina is assigned to. Let's keep the
>   ticket for now, if the PR can't be reviewed in time, we will
>   move the ticket. Downstreams can always patch their tarball
>
> - https://pagure.io/SSSD/sssd/issue/3496 - [RFE] Add a configuration
>   option to SSSD to disable the memory cache
> - there is a PR already, let's review it
>
> - https://pagure.io/SSSD/sssd/issue/1872 - [RFE] Support User
>   Private Groups for main domains, too
> - there is a PR already, let's review it
>
> - https://pagure.io/SSSD/sssd/issue/3503 - Do not index objectclass,
>   add and index objectcategory instead
> - this is WIP, there was already a PR at one point
>
> - https://pagure.io/SSSD/sssd/issue/2653 - Group renaming issue when
>   "id_provider = ldap" is set.
> - there is a PR already and a test, let's review both
>
> - https://pagure.io/SSSD/sssd/issue/2727 - Add a memcache for
>   SID-by-id lookups
> - Sumit indicated that this is WIP, but we can move the ticket
>   to 1.16.1 if the PR doesn't make the cut
>
> - https://pagure.io/SSSD/sssd/issue/3468 - SSSD doesn't use AD
>   global catalog for gidnumber lookup, resulting in unacceptable
>   delay for large forests
> - I'm working on this now
>
> - https://pagure.io/SSSD/sssd/issue/3307 -  RFE: Log to syslog when
>   sssd cannot contact servers, goes offline
> - Fabiano is working on this
>
> - https://pagure.io/SSSD/sssd/issue/3265 - [RFE] sssd should
>   remember DNS sites from first search
> - I'd like to have this ticket fixed for the next release and
>   per discussion with Pavel, it's not a lot of work either
>
> - https://pagure.io/SSSD/sssd/issue/3264 - [RFE] Make 2FA prompting
>   configurable
> - same as above
>
> At the same time, I would like to change the "Milestone" tag of all bugs
> that were fixed in 1.15.4 to 1.16.0 and do not release another 1.15.x
> version at all, but go straight to 1.16.0.
>
> And finally, all the unfished tickets in 1.15.4 would be moved to 1.16.1
> and at the same time triaged, because there are some tickets that can be
> closed or moved to a future milestone completely.
>
> Opinions?

Would be really nice if we could have the PR#275
(https://github.com/SSSD/sssd/pull/275) merged as well as part of 1.16
release, as the code seems good (according to the reviewer), has tests
and comes from an external contributor.

> ___
> sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
> To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] PRs priorities for this release

[Fabiano Fidêncio]

reed LDAPURLDesc if domain for AD DC cannot be found
  IMO this one should be part of this release

So, can we have an agreement that we're going to focus on reviewing:
- Fix group renaming issue when "id_provider = ldap" is set
(https://github.com/SSSD/sssd/pull/128)
- provider: Move hostid from ipa to sdap (https://github.com/SSSD/sssd/pull/237)
- Add support for ActiveDirectory's logonHorous restrictions
(https://github.com/SSSD/sssd/pull/269)
- Merge sss_cache and sss_debuglevel into sssctl
(https://github.com/SSSD/sssd/pull/274)
- Implement access verification by rhost using ldap_access_order rhost
option (https://github.com/SSSD/sssd/pull/275)
- IPA: Add threshold for sudo command and command group searches
(https://github.com/SSSD/sssd/pull/374)
- sssd_client: add mutex protected call to the PAC responder
(https://github.com/SSSD/sssd/pull/389)
- GPO: Don't use freed LDAPURLDesc if domain for AD DC cannot be found

And also, do we agree that the bugs mentioned above are the material
for this release and pretty much anything else that is already opened?

Best Regards,
--
Fabiano Fidêncio
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] Re: 1.13.5 release?

[Fabiano Fidêncio]

On Tue, Sep 19, 2017 at 8:50 PM, Jakub Hrozek  wrote:
> Hi,
>
> Timo mentioned last week on IRC that he would appreciate if we released
> 1.13.5.
>
> Does anyone have some patches to merge in sssd-1-13 or can we release
> the tarball?
>
> I know there are some pending PRs with backports and some patches for RHEL-6
> bugs were proposed in bugzilla.redhat.com, but there are already quite a
> few patches on top of 1.13.4 so I would prefer to release the tarball now
> and then, around the time of RHEL-6.10 development freeze, release 1.13.6.
>
> Thoughts?

Sounds good.

> ___
> sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
> To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] CI: CentOS7 issue

[Fabiano Fidêncio]

I've hit the following issue with our CI:

error: unmatched (
error: unmatched (
error: /dev/stdin:111: bad %if condition
Traceback (most recent call last):
  File "/root/payload/contrib/ci/rpm-spec-builddeps", line 33, in 
spec = rpm.spec(sys.argv[1])
ValueError: can't parse specfile

The full log can be found at:
https://ci.centos.org/job/sssd-CentOS7/1025/console

Is there some easy way to connect to the machine where it happened and
debug it from there?

Best Regards,
--
Fabiano Fidêncio
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] Re: Can someone review PR #225 (secrets quotas) ?

[Fabiano Fidêncio]

On Tue, Aug 29, 2017 at 11:17 AM, Jakub Hrozek  wrote:
> Hi,
>
> I've got a PR opened for some time that I would really like to merge for
> the next version:
> https://github.com/SSSD/sssd/pull/225 - SECRETS: Apply separate
> quotas for cn=secrets and cn=kcm
>
> It's been through several passes of a review already, so I think it
> would be easy to add the final touch and ACK the PR. Any takers?

I'll finish this review Today.

> ___
> sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
> To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] Re: Shall we freeze the development till #3463 is solved?

[Fabiano Fidêncio]

On Thu, Aug 10, 2017 at 10:15 AM, Lukas Slebodnik  wrote:
> On (09/08/17 14:43), Jakub Hrozek wrote:
>>
>>> On 8 Aug 2017, at 16:51, Fabiano Fidêncio  wrote:
>>>
>>> People,
>>>
>>> There's a test, part of our internal CI, recurrently failing in the
>>> past few weeks:
>>>
>>> === FAILURES 
>>> ===
>>> _ test_add_remove_user 
>>> _
>>> Traceback (most recent call last):
>>>  File 
>>> "/var/lib/jenkins/workspace/ci/label/debian_testing/src/tests/intg/test_enumeration.py",
>>> line 418, in test_add_remove_user
>>>ent.assert_passwd(ent.contains_only())
>>>  File 
>>> "/var/lib/jenkins/workspace/ci/label/debian_testing/src/tests/intg/ent.py",
>>> line 345, in assert_passwd
>>>assert not d, d
>>> AssertionError: list mismatch:
>>> unexpected users found:
>>> [{'dir': '/home/user',
>>>  'gecos': '2001',
>>>  'gid': 2000,
>>>  'name': 'user',
>>>  'passwd': '*',
>>>  'shell': '/bin/bash',
>>>  'uid': 2001}]
>>>  1 failed, 226 passed in 976.19 seconds 
>>> 
>>>
>>> There's already an open issue for this case:
>>> https://pagure.io/SSSD/sssd/issue/3463 in order to track the issue.
>>>
>>> So, as the subject says, shall we officially stop pushing patches till
>>> we have this issue solved?
>>>
>>
>>I personally think pushing your workaround patch (that essentially just 
>>increases wait time) is good enough to unblock the development now and we 
>>should keep making the fix cleaner at the same time.
>>
>
> Increasing timeout without explanation "why" is almost the same as disabling
> test. I vote for disabling test rather then blindly increasing timeout.

I'm not sure whether you're following the project or not, but that
pull request has been closed as rejected.

>
>>But in general I agree that a test that is consistently failing should be 
>>inspected asap..
>>
> +1
>
> And ASAP does not mean after two weeks :-)
>
> LS
> ___
> sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
> To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] Shall we freeze the development till #3463 is solved?

[Fabiano Fidêncio]

People,

There's a test, part of our internal CI, recurrently failing in the
past few weeks:

=== FAILURES ===
_ test_add_remove_user _
Traceback (most recent call last):
  File 
"/var/lib/jenkins/workspace/ci/label/debian_testing/src/tests/intg/test_enumeration.py",
line 418, in test_add_remove_user
ent.assert_passwd(ent.contains_only())
  File 
"/var/lib/jenkins/workspace/ci/label/debian_testing/src/tests/intg/ent.py",
line 345, in assert_passwd
assert not d, d
AssertionError: list mismatch:
unexpected users found:
[{'dir': '/home/user',
  'gecos': '2001',
  'gid': 2000,
  'name': 'user',
  'passwd': '*',
  'shell': '/bin/bash',
  'uid': 2001}]
 1 failed, 226 passed in 976.19 seconds 

There's already an open issue for this case:
https://pagure.io/SSSD/sssd/issue/3463 in order to track the issue.

So, as the subject says, shall we officially stop pushing patches till
we have this issue solved?

Best Regards,
-- 
Fabiano Fidêncio
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] Re: [sssd PR#327][comment] RESPONDERS: Fix terminating idle connections

[Fabiano Fidêncio]

On Wed, Jul 19, 2017 at 6:56 PM, lslebodn
 wrote:
>   URL: https://github.com/SSSD/sssd/pull/327
> Title: #327: RESPONDERS: Fix terminating idle connections
>
> lslebodn commented:
> """
> On (19/07/17 09:28), fidencio wrote:
>>fidencio commented on this pull request.
>>> @@ -100,11 +100,11 @@ def setup_for_secrets(request):
>> [secrets]
>> max_secrets = 10
>> max_payload_size = 2
>>+client_idle_timeout = 10
>>
>>@lslebodn: I'm just quoting the commit message as I do believe it answers 
>>your question about the 10 seconds:
>>"The client timeout in the test has to be at least 10 seconds because
>>internally, the responders don't allow a shorter timeout."
>>
> I checked only man page and that's not mention there.
> And it still does not explain why do we need
> to wait 50% longer then `client_idle_timeout`

The reason we have to wait 50% longer than the `client_idle_timeout`
can be find here (please, see the commit message):
https://github.com/SSSD/sssd/commit/560daa14ef013aa14e2aedeea10b07f623d84ec8

And the commit that introduced that the client_idle_timeout has the
minimum value of 10 seconds is this one:
https://github.com/SSSD/sssd/commit/bb79e7559dae451a14150377099e32d6b5159a6c

Hopefully everything is clear now.

>
> LS
>
> """
>
> See the full comment at 
> https://github.com/SSSD/sssd/pull/327#issuecomment-316449831
>
> ___
> sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
> To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
>
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] Re: RFC: 1.15.2 release notes

[Fabiano Fidêncio]

On Wed, Mar 15, 2017 at 4:17 PM, Jakub Hrozek  wrote:
> Hi,
>
> I prepared release notes for today's release. They are written in
> anticipation that PR#186 with the subdomain config will be merged.
>
> The RST I pushed to the sssd/docs repo is below:
>
> SSSD 1.15.2
> ===
>
> Highlights
> --
>  * It is now possible to configure certain parameters of a trusted domain
>in a configuration file sub-section. In particular, it is now possible
>to configure which Active Directory DCs the SSSD talks to with a
>configuration like this::
>
> [domain/ipa.test]
> # IPA domain configuration. This domain trusts a Windows domain win.test
>
> [domain/ipa.test/win.test]
> ad_server = dc.win.test
>
>  * Several issues related to socket-activating the NSS service, especially
>if SSSD was configured to use a non-privileged user were fixed. The NSS
>service now starts as root to avoid triggering a name-service lookup
>while the NSS service is not running yet. Additionally, the NSS service
>is started before any other service to make sure username resolution works
>and the other service can resolve the SSSD user correctly.

So, this part is not exactly accurate.
NSS responder always been only used as root. What we did is not
changing the owner of the nss log file for the socket-activated NSS
responder.

My suggestion is: "(...). The NSS service now doesn't change the
ownership of its log files to avoid triggering (...)"

>
>  * A new option ``cache_first`` allows the administrator to change the way
>multiple domains are searched. When this option is enabled, SSSD will
>first try to "pin" the requested name or ID to a domain by searching
>the entries that are already cached and contact the domain that contains
>the cached entry first. Previously, SSSD would check the cache and the
>remote server for each domain. This option brings performance benefit
>for setups that use multiple domains (even auto-discovered trusted
>domains), especially for ID lookups that would previously iterate over
>all domains. Please note that this option must be enabled with care as the
>administrator must ensure that the ID space of domains does not overlap.
>
>  * The SSSD D-Bus interface gained two new methods:
>``FindByNameAndCertificate`` and ``ListByCertificate``. These methods will
>be used primarily by IPA to correctly match multple users who use the
>same certificate for Smart Card login.
>
>  * A bug where SSSD did not properly sanitize a username with a newline
>character in it was fixed.
>
> Packaging Changes
> -
> None in this release
>
> Documentation Changes
> -
>  * A new option ``cache_first`` was added. Please see the Highlights
>section for more details
>
>  * The ``override_homedir`` option supports a new template expansion ``l``
>that expands to the first letter of username
>
>
> Tickets Fixed
> -
> Please note that due to a bug in the pagure.io tracker, some tickets that
> have dependencies set to other tickets cannot be closed at the moment.
>
>  * `#3317 `_ - Newline characters 
> (\n) must be sanitized before LDAP requests take place
>  * `#3316 `_ - sssd-secrets doesn't 
> exit on idle
>  * `#3314 `_ - sssd ignores entire 
> groups from proxy provider if one member is listed twice
>  * `#3164 `_ - when group is 
> invalidated using sss_cache dataExpireTimestamp entry in the domain and 
> timestamps cache are inconsistent
>  * `#2668 `_ - [RFE] Add more 
> flexible templating for override_homedir config option
>  * `#2599 `_ - Make it possible to 
> configure AD subdomain in the server mode
>  * `#3223 `_ - The 
> sssd-$RESPONDER.service units should bind to their socket units
>  * `#3322 `_ - chown in ExecStartPre 
> of sssd-nss.service hangs forever
>  * `#843 `_ -  Login time increases 
> strongly if more than one domain is configured
>  * `#2320 `_ - use the sss_parse_inp 
> request in other responders than dbus
>
> Detailed Changelog
> --
> ___
> sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
> To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] Re: RFC: 1.15.1 release notes

[Fabiano Fidêncio]

On Fri, Mar 3, 2017 at 1:07 PM, Jakub Hrozek  wrote:
> Hi,
>
> I prepared the release notes for the upcoming 1.15.1 release. You can
> view them in your browser:
> https://docs.pagure.org/jhrozek-doctest/users/releases/notes_1_15_1.html
>
> Or read the inline RST text. Comments welcome!
>
> SSSD 1.15.1
> ===
>
> Highlights
> --
>  * Several issues related to starting the SSSD services on-demand by the
>systemd service manager were fixed. In particular, it is no longer
>possible to have a service started both by sssd and by systemd. Another
>bug which might have caused the responder to start before SSSD started
>and cause issues especially on system startup was fixed.
>  * A new ``files`` provider was added. This provider mirrors the contents
>of ``/etc/passwd`` and ``/etc/shadow`` into the SSSD database. The purpose
>of this new provider is to make it possible to use SSSD's interfaces,
>such as the D-Bus interface for local users and enable leveraging the
>in-memory fast cache for local users as well, as a replacement for `nscd`.
>In future, we intend to extend the D-Bus interface to also provide setting
>and retrieving additional custom attributes for the files users.
>  * SSSD now autogenerates a fallback configuration that enables the
>files domain if no SSSD configuration exists. This allows distributions
>to enable the ``sssd`` service when the SSSD package is installed. Please
>note that SSSD must be build with the configuration option
>``--enable-files-domain`` for this functionality to be enabled.
>  * Support for public-key authentication with Kerberos (PKINIT) was
>added. This support will enable users who authenticate with a Smart Card
>to obtain a Kerberos ticket during authentication.
>
> Packaging Changes
> -
>  * The new files provider comes as a new shared library ``libsss_files.so``
>and a new manual page
>  * A new helper binary called ``sssd_check_socket_activated_responders``
>was added. This binary is used in the ``ExecStartPre`` directive to check
>if the service that corresponds to socket about to be started was also
>started explicitly and abort the socket startup if it was.
>
> Documentation Changes
> -
>  * A new PAM module option ``prompt_always`` was added. This option is
>related to fixing ``_ which
>changed the behaviour of the PAM module so that ``pam_sss`` always
>uses an auth token that was on stack. The new ``prompt_always`` option
>makes it possible to restore the previous behaviour.
>
> Tickets Fixed
> -
>  * `#3112 `_ - When sssd.conf is 
> missing, create one with id_provider=files
>  * `#3220 `_ - Improve successful 
> Dynamic DNS update log messages
>  * `#3227 `_ - sssd doesn't update 
> PTR records if A/PTR zones are configured as non-secure and secure
>  * `#3230 `_ - Use the same logic for 
> matching GC results in initgroups and user lookups
>  * `#3260 `_ - handle 
> default_domain_suffix for ssh requests with default_domain_suffix
>  * `#3262 `_ - Implement a files 
> provider to mirror the contents of /etc/passwd and /etc/groups
>  * `#3270 `_ - [RFE] Add PKINIT 
> support to SSSD Kerberos proivder
>  * `#3298 `_ - Socket activation of 
> SSSD doesn't work and leads to chaos
>  * `#3299 `_ - SSSD does not start if 
> using only the local provider and services line is empty
>  * `#3300 `_ - Avoid running two 
> instances of the same service
>  * `#3309 `_ - Coverity warns about 
> an unused value in IPA sudo code
>  * `#3313 `_ - cache_req should use 
> an negative cache entry for UPN based lookups
>  * `#2984 `_ - Don't prompt for 
> password if there is already one on the stack
>  * `#1126 `_ - Reuse cache_req() in 
> responder code
>
> Detailed Changelog
> --
> ___
> sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
> To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

Looks good to me!
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] Re: RFC: Migrating SSSD documentation to pagure.io

[Fabiano Fidêncio]

On Fri, Mar 3, 2017 at 9:57 AM, Jakub Hrozek  wrote:
> Hi,
>
> I started on migrating the wiki content from fedorahosted to pagure.
> Because we are working hard on finishing the 1.15.1 and 1.15.2 releases,
> I want to only migrate the content that we need right now, which is the
> releases page (so that we can put the release notes somewhere) and
> design pages for 1.15 features.
>
> So far I was trying the documentation out in a sandbox repo:
> https://pagure.io/docs/jhrozek-doctest/
> I think it looks OK and I like the rst->sphinx->html workflow:
> https://docs.pagure.org/pagure/usage/using_doc.html
>
> Regarding the repositories, I propose we do this:
> 1) create a new repo under the SSSD namespace, perhaps:
> https://pagure.io/SSSD/docs
> This repo will only contain the rst sources. I would like to ask
> Patrick to mirror this repo to github so we can use PRs to update
> documentation.
>
> 2) When a PR to this repo is merged, one of the maintainers will run
> "make html" in the docs repo. This will generate HTML documentation
> from the rst sources.
>
> 3) This HTML documentation will be pushed to:
> ssh://g...@pagure.io/docs/SSSD/sssd.git
>
> Since I would only like to put up the releases and changelogs for now, I
> would like to ask to temporarily not review the RST changes with PRs
> until we are done with 1.15 development.
>
> Please speak up if you disagree :-)

I don't disagree but I also don't think the approach is optimal (but
well, we have been working together it Pagure guys in order to make it
optimal at some point).

Having a way to edit the docs similarly to what we had in Fedora host
sounds better. But here I'm not sure if it was really better or if it
is just something different that we will get used to.

Having the docs in a separate git repo makes me think that at least
one review will be done for any change in the repo and it's, at least
for me, a good thing.

So, please, go ahead with your approach and let's give it a try.

Best Regards,
--
Fabiano Fidêncio
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] Re: Avoid running two instances of the same service (#3300)

[Fabiano Fidêncio]

On Wed, Feb 8, 2017 at 1:14 PM, Lukas Slebodnik  wrote:
> On (08/02/17 12:33), Fabiano Fidêncio wrote:
>>On Wed, Feb 8, 2017 at 12:29 PM, Lukas Slebodnik  wrote:
>>> On (08/02/17 12:24), Fabiano Fidêncio wrote:
>>>>On Wed, Feb 8, 2017 at 12:10 PM, Lukas Slebodnik  
>>>>wrote:
>>>>> On (05/02/17 23:24), Fabiano Fidêncio wrote:
>>>>>>I've spent some amount of time trying to properly deal with this issue
>>>>>>and I really need the opinion/suggestion of more experienced
>>>>>>developers.
>>>>>>
>>>>>>Basically, as explained in #3300 this situation can happen is two
>>>>>>scenarios were the admin is mixing up socket-activated and explicitly
>>>>>>configured services:
>>>>>>- Scenario 1:
>>>>>>  - nss responder is explicitly activated;
>>>>>>  - nss responder's socket is enabled;
>>>>>>  - boot the system
>>>>>>  - both nss processes will be up
>>>>>>
>>>>> This is pure misconfuguration and I do not think a reason why we shoudl 
>>>>> solve
>>>>> it.
>>>>
>>>>Do you prefer to not deal with a misconfiguration scneraio (that may
>>>>not be caused by the admins themselves, but by the packager ...) even
>>>>if we can do that?
>>>>Interesting, at least.
>>>>
>>> Yes, I do not want to deal with misconfiguration.
>>> We might passively detect it and refuse such state.
>>> e.g. Fail if unix sockets are already created(by systemd) and monitor
>>> wants to start responders.
>>
>>How do we prevent systemd to start a responder in case the responder
>>has been already started by the monitor?
>>How do you cope with this "mixed" scenario?
>>
> Systemd sockets are created earlier in the boot phase.

They are because systemd automatically adds any socket to be started
during sockets.target start up, but they shouldn't according to our
design page.
According to the design page the sockets would be started when SSSD is started.

May be the case the design done is wrong? Of course.
So, let's get back to the design page as the first thing and solve the
problems we have there.

> So this situation will not happen at boot.
>
> And if sssd is already started I wonder how systemd can create
> sockets which are already owned by other process.
>
> LS
> ___
> sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
> To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] Re: Avoid running two instances of the same service (#3300)

[Fabiano Fidêncio]

On Wed, Feb 8, 2017 at 12:29 PM, Lukas Slebodnik  wrote:
> On (08/02/17 12:24), Fabiano Fidêncio wrote:
>>On Wed, Feb 8, 2017 at 12:10 PM, Lukas Slebodnik  wrote:
>>> On (05/02/17 23:24), Fabiano Fidêncio wrote:
>>>>I've spent some amount of time trying to properly deal with this issue
>>>>and I really need the opinion/suggestion of more experienced
>>>>developers.
>>>>
>>>>Basically, as explained in #3300 this situation can happen is two
>>>>scenarios were the admin is mixing up socket-activated and explicitly
>>>>configured services:
>>>>- Scenario 1:
>>>>  - nss responder is explicitly activated;
>>>>  - nss responder's socket is enabled;
>>>>  - boot the system
>>>>  - both nss processes will be up
>>>>
>>> This is pure misconfuguration and I do not think a reason why we shoudl 
>>> solve
>>> it.
>>
>>Do you prefer to not deal with a misconfiguration scneraio (that may
>>not be caused by the admins themselves, but by the packager ...) even
>>if we can do that?
>>Interesting, at least.
>>
> Yes, I do not want to deal with misconfiguration.
> We might passively detect it and refuse such state.
> e.g. Fail if unix sockets are already created(by systemd) and monitor
> wants to start responders.
>
>>>
>>>>So, what I've thought so far, for each of the scenarios, is:
>>>>- Scenario 1:
>>>>  Implement a way to either bypass the monitor in case the responder's
>>>>socket it up or do not start the process through systemd in case one
>>>>instance of the process is already being ran by the monitor. The main
>>>>question here is how to do this without adding more logic to the
>>>>monitor
>>> What is a goal here?
>>
>>The goal here is be bulletproof against wrong configurations/admins
>>trying to do something they're not supposed to.
>>But, as far as I understand from your previous comment, it's not of
>>your (and thus the project) interest to be robust on these cases.
>>
> I am not against to be robust. I am against magical workarounding
> such misconfuguration.

I do not understand why do you think using systemd when we can using
systemd is a magical workaroud.

>
> LS
> ___
> sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
> To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] Re: Avoid running two instances of the same service (#3300)

[Fabiano Fidêncio]

On Wed, Feb 8, 2017 at 12:29 PM, Lukas Slebodnik  wrote:
> On (08/02/17 12:24), Fabiano Fidêncio wrote:
>>On Wed, Feb 8, 2017 at 12:10 PM, Lukas Slebodnik  wrote:
>>> On (05/02/17 23:24), Fabiano Fidêncio wrote:
>>>>I've spent some amount of time trying to properly deal with this issue
>>>>and I really need the opinion/suggestion of more experienced
>>>>developers.
>>>>
>>>>Basically, as explained in #3300 this situation can happen is two
>>>>scenarios were the admin is mixing up socket-activated and explicitly
>>>>configured services:
>>>>- Scenario 1:
>>>>  - nss responder is explicitly activated;
>>>>  - nss responder's socket is enabled;
>>>>  - boot the system
>>>>  - both nss processes will be up
>>>>
>>> This is pure misconfuguration and I do not think a reason why we shoudl 
>>> solve
>>> it.
>>
>>Do you prefer to not deal with a misconfiguration scneraio (that may
>>not be caused by the admins themselves, but by the packager ...) even
>>if we can do that?
>>Interesting, at least.
>>
> Yes, I do not want to deal with misconfiguration.
> We might passively detect it and refuse such state.
> e.g. Fail if unix sockets are already created(by systemd) and monitor
> wants to start responders.

How do we prevent systemd to start a responder in case the responder
has been already started by the monitor?
How do you cope with this "mixed" scenario?

>
>>>
>>>>So, what I've thought so far, for each of the scenarios, is:
>>>>- Scenario 1:
>>>>  Implement a way to either bypass the monitor in case the responder's
>>>>socket it up or do not start the process through systemd in case one
>>>>instance of the process is already being ran by the monitor. The main
>>>>question here is how to do this without adding more logic to the
>>>>monitor
>>> What is a goal here?
>>
>>The goal here is be bulletproof against wrong configurations/admins
>>trying to do something they're not supposed to.
>>But, as far as I understand from your previous comment, it's not of
>>your (and thus the project) interest to be robust on these cases.
>>
> I am not against to be robust. I am against magical workarounding
> such misconfuguration.
>
> LS
> ___
> sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
> To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] Re: Avoid running two instances of the same service (#3300)

[Fabiano Fidêncio]

On Wed, Feb 8, 2017 at 12:10 PM, Lukas Slebodnik  wrote:
> On (05/02/17 23:24), Fabiano Fidêncio wrote:
>>I've spent some amount of time trying to properly deal with this issue
>>and I really need the opinion/suggestion of more experienced
>>developers.
>>
>>Basically, as explained in #3300 this situation can happen is two
>>scenarios were the admin is mixing up socket-activated and explicitly
>>configured services:
>>- Scenario 1:
>>  - nss responder is explicitly activated;
>>  - nss responder's socket is enabled;
>>  - boot the system
>>  - both nss processes will be up
>>
> This is pure misconfuguration and I do not think a reason why we shoudl solve
> it.

Do you prefer to not deal with a misconfiguration scneraio (that may
not be caused by the admins themselves, but by the packager ...) even
if we can do that?
Interesting, at least.

>
>>So, what I've thought so far, for each of the scenarios, is:
>>- Scenario 1:
>>  Implement a way to either bypass the monitor in case the responder's
>>socket it up or do not start the process through systemd in case one
>>instance of the process is already being ran by the monitor. The main
>>question here is how to do this without adding more logic to the
>>monitor
> What is a goal here?

The goal here is be bulletproof against wrong configurations/admins
trying to do something they're not supposed to.
But, as far as I understand from your previous comment, it's not of
your (and thus the project) interest to be robust on these cases.

> Why do we want to bypass monitor?

I want to leave the responsibility to start a service to systemd
whenever it's possible.
Why? To avoid having two services of the same responder running.

Those patches are not strict needed unless we care at least a little
about robustness of SSSD.

Best Regards,
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] Re: Avoid running two instances of the same service (#3300)

[Fabiano Fidêncio]

On Tue, Feb 7, 2017 at 12:58 PM, Pavel Březina  wrote:
> On 02/07/2017 12:31 PM, Fabiano Fidêncio wrote:
>>
>> On Tue, Feb 7, 2017 at 12:20 PM, Pavel Březina 
>> wrote:
>>>
>>> On 02/05/2017 11:24 PM, Fabiano Fidêncio wrote:
>>>>
>>>>
>>>> I've spent some amount of time trying to properly deal with this issue
>>>> and I really need the opinion/suggestion of more experienced
>>>> developers.
>>>
>>>
>>>
>>> Hi, I'm not sure I understand it correctly so let us clarify it.
>>>
>>>> Basically, as explained in #3300 this situation can happen is two
>>>> scenarios were the admin is mixing up socket-activated and explicitly
>>>> configured services:
>>>> - Scenario 1:
>>>>   - nss responder is explicitly activated;
>>>
>>>
>>>
>>> This means services = nss in sssd.conf, right?
>>
>>
>> Yes, services = nss
>>
>>>
>>>>   - nss responder's socket is enabled;
>>>
>>>
>>>
>>> systemctl enable sssd-nss.socket?
>>
>>
>> Yes, systemctl enable sssd-nss.socket
>>
>>>
>>>>   - boot the system
>>>>   - both nss processes will be up
>>>
>>>
>>>
>>> Why?
>>
>>
>> As far as I have debugged what happens is:
>> - monitor will start NSS responder because it's part of services line;
>> - sssd-nss.socket will be up as, considering how the git master code
>> is, it's part of sockets.target;
>> - if any request goes to the nss socket, it will bring
>> sssd-nss.service up and as the first was started by the monitor,
>> systemd has no idea about a nss responder already running;
>>
>>>
>>>> - Scenario 2
>>>>   - any responder is explicitly activated
>>>>   - systemctl enable sssd-@responder@.service
>>>
>>>
>>>
>>> In scenario 1 we are talking about socket, here about service. Is this
>>> correct or a typo?
>>
>>
>> That's a typo, but in the enable part. Consider the admin has pam
>> responder in the services line ...
>> - d starts -> pam responder is running
>> - admin does: systemctl start sssd-pam.service
>>
>> You will end up with two pam responders running at the same time.
>>
>>>
>>> Why we end up running it twice?
>>
>>
>> I hope I answered that above ...
>>
>>>
>>>> So, what I've thought so far, for each of the scenarios, is:
>>>> - Scenario 1:
>>>>   Implement a way to either bypass the monitor in case the responder's
>>>> socket it up or do not start the process through systemd in case one
>>>> instance of the process is already being ran by the monitor. The main
>>>> question here is how to do this without adding more logic to the
>>>> monitor (Lukáš made his opinion quite clear that we should not be
>>>> adding more logic to the monitor and I agree with him ... but maybe
>>>> this case is an exception?) and also decide which of the approaches
>>>> would be the less hackish one.
>>>>   Personally I'd go for always leaving the responder's startup to
>>>> systemd, if possible. The main problem with this approach is that
>>>> checking whether a the unit is loaded or not is done through sd-bus
>>>> APIs, which may be to new for some enterprise distros (including
>>>> CentOS here ...) /o\
>>>>
>>>> - Scenario 2:
>>>>   Here I do believe we could force the services to not be started
>>>> manually as systemd already provides "RefuseManualStart=" option.
>>>>
>>>>
>>>> I'm really looking forward to hearing your opinion on what would be
>>>> the best approach to take, new ideas and so on.
>>>>
>>>>
>>>> Best Regards,
>>>> --
>>>> Fabiano Fidêncio
>>>
>>>
>>
>> So, the path I've taken is:
>> - If possible, always leave the service to be started up by systemd.
>> IOW, we check whether the sssd-@responder@.socket is enabled. In case
>> it is, just do not start the service through the monitor and let it be
>> socket-activated when neede.
>> - Refuse manual startup of the responders (and let they be
>> automatically started up through socket-activation).
>>
>> Does that make sense?
>
>
> Yes, thank you for explanation. Is it possible to use sd_notify() in
> responders to tell systemd that the service is running?

I've checked that and as far I as understand it wouldn't work as the
service has not been started by systemd.
Thus I've taken the approach to prefer systemd to monitor whenever
it's possible.

>
>
> ___
> sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
> To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] Re: Avoid running two instances of the same service (#3300)

[Fabiano Fidêncio]

On Tue, Feb 7, 2017 at 12:20 PM, Pavel Březina  wrote:
> On 02/05/2017 11:24 PM, Fabiano Fidêncio wrote:
>>
>> I've spent some amount of time trying to properly deal with this issue
>> and I really need the opinion/suggestion of more experienced
>> developers.
>
>
> Hi, I'm not sure I understand it correctly so let us clarify it.
>
>> Basically, as explained in #3300 this situation can happen is two
>> scenarios were the admin is mixing up socket-activated and explicitly
>> configured services:
>> - Scenario 1:
>>   - nss responder is explicitly activated;
>
>
> This means services = nss in sssd.conf, right?

Yes, services = nss

>
>>   - nss responder's socket is enabled;
>
>
> systemctl enable sssd-nss.socket?

Yes, systemctl enable sssd-nss.socket

>
>>   - boot the system
>>   - both nss processes will be up
>
>
> Why?

As far as I have debugged what happens is:
- monitor will start NSS responder because it's part of services line;
- sssd-nss.socket will be up as, considering how the git master code
is, it's part of sockets.target;
- if any request goes to the nss socket, it will bring
sssd-nss.service up and as the first was started by the monitor,
systemd has no idea about a nss responder already running;

>
>> - Scenario 2
>>   - any responder is explicitly activated
>>   - systemctl enable sssd-@responder@.service
>
>
> In scenario 1 we are talking about socket, here about service. Is this
> correct or a typo?

That's a typo, but in the enable part. Consider the admin has pam
responder in the services line ...
- d starts -> pam responder is running
- admin does: systemctl start sssd-pam.service

You will end up with two pam responders running at the same time.

>
> Why we end up running it twice?

I hope I answered that above ...

>
>> So, what I've thought so far, for each of the scenarios, is:
>> - Scenario 1:
>>   Implement a way to either bypass the monitor in case the responder's
>> socket it up or do not start the process through systemd in case one
>> instance of the process is already being ran by the monitor. The main
>> question here is how to do this without adding more logic to the
>> monitor (Lukáš made his opinion quite clear that we should not be
>> adding more logic to the monitor and I agree with him ... but maybe
>> this case is an exception?) and also decide which of the approaches
>> would be the less hackish one.
>>   Personally I'd go for always leaving the responder's startup to
>> systemd, if possible. The main problem with this approach is that
>> checking whether a the unit is loaded or not is done through sd-bus
>> APIs, which may be to new for some enterprise distros (including
>> CentOS here ...) /o\
>>
>> - Scenario 2:
>>   Here I do believe we could force the services to not be started
>> manually as systemd already provides "RefuseManualStart=" option.
>>
>>
>> I'm really looking forward to hearing your opinion on what would be
>> the best approach to take, new ideas and so on.
>>
>>
>> Best Regards,
>> --
>> Fabiano Fidêncio
>

So, the path I've taken is:
- If possible, always leave the service to be started up by systemd.
IOW, we check whether the sssd-@responder@.socket is enabled. In case
it is, just do not start the service through the monitor and let it be
socket-activated when neede.
- Refuse manual startup of the responders (and let they be
automatically started up through socket-activation).

Does that make sense?

Best Regards,
--
Fabiano Fidêncio
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] Re: Avoid running two instances of the same service (#3300)

[Fabiano Fidêncio]

On Sun, Feb 5, 2017 at 11:24 PM, Fabiano Fidêncio  wrote:
> I've spent some amount of time trying to properly deal with this issue
> and I really need the opinion/suggestion of more experienced
> developers.
>
> Basically, as explained in #3300 this situation can happen is two
> scenarios were the admin is mixing up socket-activated and explicitly
> configured services:
> - Scenario 1:
>   - nss responder is explicitly activated;
>   - nss responder's socket is enabled;
>   - boot the system
>   - both nss processes will be up
>
> - Scenario 2
>   - any responder is explicitly activated
>   - systemctl enable sssd-@responder@.service
>
> So, what I've thought so far, for each of the scenarios, is:
> - Scenario 1:
>   Implement a way to either bypass the monitor in case the responder's
> socket it up or do not start the process through systemd in case one
> instance of the process is already being ran by the monitor. The main
> question here is how to do this without adding more logic to the
> monitor (Lukáš made his opinion quite clear that we should not be
> adding more logic to the monitor and I agree with him ... but maybe
> this case is an exception?) and also decide which of the approaches
> would be the less hackish one.
>   Personally I'd go for always leaving the responder's startup to
> systemd, if possible. The main problem with this approach is that
> checking whether a the unit is loaded or not is done through sd-bus
> APIs, which may be to new for some enterprise distros (including
> CentOS here ...) /o\
>
> - Scenario 2:
>   Here I do believe we could force the services to not be started
> manually as systemd already provides "RefuseManualStart=" option.
>
>
> I'm really looking forward to hearing your opinion on what would be
> the best approach to take, new ideas and so on.

This discussion can be done on https://github.com/SSSD/sssd/pull/146

Best Regards,
--
Fabiano Fidêncio
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] Avoid running two instances of the same service (#3300)

[Fabiano Fidêncio]

I've spent some amount of time trying to properly deal with this issue
and I really need the opinion/suggestion of more experienced
developers.

Basically, as explained in #3300 this situation can happen is two
scenarios were the admin is mixing up socket-activated and explicitly
configured services:
- Scenario 1:
  - nss responder is explicitly activated;
  - nss responder's socket is enabled;
  - boot the system
  - both nss processes will be up

- Scenario 2
  - any responder is explicitly activated
  - systemctl enable sssd-@responder@.service

So, what I've thought so far, for each of the scenarios, is:
- Scenario 1:
  Implement a way to either bypass the monitor in case the responder's
socket it up or do not start the process through systemd in case one
instance of the process is already being ran by the monitor. The main
question here is how to do this without adding more logic to the
monitor (Lukáš made his opinion quite clear that we should not be
adding more logic to the monitor and I agree with him ... but maybe
this case is an exception?) and also decide which of the approaches
would be the less hackish one.
  Personally I'd go for always leaving the responder's startup to
systemd, if possible. The main problem with this approach is that
checking whether a the unit is loaded or not is done through sd-bus
APIs, which may be to new for some enterprise distros (including
CentOS here ...) /o\

- Scenario 2:
  Here I do believe we could force the services to not be started
manually as systemd already provides "RefuseManualStart=" option.


I'm really looking forward to hearing your opinion on what would be
the best approach to take, new ideas and so on.


Best Regards,
--
Fabiano Fidêncio
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] Using cache_req code for PAM responder

[Fabiano Fidêncio]

I've done a first WIP patch for this matter but Jakub pointed out the
approach is not correct as the PAM doesn't use the cache the same way
as other responders do.

Differently from the other responders, PAM tries to conatct the Data
Provider on almost every request.

Looking at the code, what's done is:
- While looping the domains in pam_check_user_search():
  - call pam_initgr_check_timeout()
- in case the timeout is still valid:
  - get the entry from sysdb
- otherwise
  - call the data provider first

As the using cache_req code for PAM responder has two main goals
(decrease code duplicaton and make it possible to log in with a
shortname to a trusted domain) Jakub suggested to, maybe write a new
cache_req plugin (specifically for PAM?) and decrease the number of
duplicated code by just reusing this new code from cache_req.

The main reason behind his idea is that he thinks we want to keep the
pam_initgr_check_timeout() while looping the domains in the cache_req
code.

So, as I'm not that much familiar with none of those two pieces of
code ... I'd like to know what's Pavel Březina opinion on these ideas.

Best Regards,
--
Fabiano Fidêncio
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] [RFC] Discussion about enabling socket-activate services to refresh configuration

[Fabiano Fidêncio]

Last Thursday we had a face to face discussion about how
socket-activate services could refresh the confdb before starting.

There were five of us attending the discussion (Pavel Březina, Lukáš
Slebodnik, Simo Sorce, Stephen Gallagher and myself) and the main
focus of the discussion was on what to do in a near-future scenario
where the monitor won't be present anymore.

So, keeping in mind the "no monitor" scenario what has been suggested
by Simo is:
- Have some sort of inotify in the confdb that, when updated, will
trigger the restart of the services. It's really important to note
that we want a way to be notified that the file was changed as a whole
and not trigger several restarts for each small change possible done
in the file;
- There were some ideas suggested about adding a timestamp to the
confdb or a hash of the file (?) in order to keep track of the latest
changes;
- As we will restart the services when notified that the confdb
changes, we have to be more robust when shutting down any service: as
far as I understand/remember it means we must answer the requests
already done while not accepting any new request. It applies pretty
much to any running service at the moment the confdb becomes updated;
- sssd --genconf may be one way to regenerate the confdb and could be
used as ExecPreStart in the SSSD unit.

For the scneario where the monitor is still around Simo suggested to:
- sssd --genconf may be one way to regenerate the confdb (but in the
end we won't do that)
- SSSD will log that the admin must restart SSSD service in order to
have the confdb updated.

I'm really not sure about the technical details of this discussion as
I didn't have time enough to dig into any of the topics I mentioned in
the email.

For those who also where in the meeting, please, politely provide some
feedback on what I've missed, on what you'd like to expand and so on.

Best Regards,
--
Fabiano Fidêncio
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] Re: $ reconfig && chmake failed

[Fabiano Fidêncio]

On Wed, Jan 18, 2017 at 10:44 AM, amit kumar  wrote:
> PASS: refcount-tests
> FAIL: fail_over-tests
> PASS: find_uid-tests
> PASS: auth-tests
> PASS: ipa_ldap_opt-tests
> PASS: ad_ldap_opt-tests
> PASS: crypto-tests
> PASS: util-tests
> PASS: debug-tests
> PASS: ipa_hbac-tests
> PASS: sss_idmap-tests
> PASS: responder_socket_access-tests
> PASS: sysdb-tests
> PASS: safe-format-tests
> PASS: sysdb_ssh-tests
> PASS: sbus_tests
> PASS: sbus_codegen_tests
> PASS: test_fo_srv
> PASS: resolv-tests
> 
> Testsuite summary for sssd 1.14.90
> 
> # TOTAL: 83
> # PASS:  82
> # SKIP:  0
> # XFAIL: 0
> # FAIL:  1
> # XPASS: 0
> # ERROR: 0
> 
> See ./test-suite.log
> Please report to sssd-devel@lists.fedorahosted.org
> 
>
> # vim ./test_suite.log
>
> 
>sssd 1.14.90: ./test-suite.log
> 
>
> # TOTAL: 83
> # PASS:  82
> # SKIP:  0
> # XFAIL: 0
> # FAIL:  1
> # XPASS: 0
> # ERROR: 0
>
> .. contents:: :depth: 2
>
> FAIL: fail_over-tests
> =
>
> Running suite(s): fail_over
> 50%: Checks: 2, Failures: 1, Errors: 0
> ../src/tests/fail_over-tests.c:162:F:FAIL_OVER
> Tests:test_fo_resolve_service:0: ../src/tests/fail_over-tests.c:254:
> Expected return of 0, got 110
> FAIL fail_over-tests (exit status: 1)
>
>
>
>
> --
> Thanks
> Amit Kumar
> There are three ways to get something done:
>   (1) Do it yourself.
>   (2) Hire someone to do it for you.
>   (3) Forbid your kids to do it.
>
>
> _______
> sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
> To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
>

Which system are you using?

-- 
Fabiano Fidêncio
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] Re: Group renaming issue when "id_provider = ldap" is set.

[Fabiano Fidêncio]

On Mon, Jan 9, 2017 at 11:51 AM, Jakub Hrozek  wrote:
> On Sun, Jan 08, 2017 at 09:58:22PM +0100, Fabiano Fidêncio wrote:
>> I've been working on rhbz#1401241
>> (https://bugzilla.redhat.com/show_bug.cgi?id=1401241) and I'd like to
>> clarify some doubts that showed up.
>>
>> So, let's consider that there's a group called "foo" and user "user"
>> is part of this group. Group "foo" gets renamed to "bar'. Now. let me
>> describe what I've found out considering "id_provider = ldap" and
>> "id_provider = ad"
>>
>> - id_provider = ldap:
>>   cache has "foo" entry
>>   After renaming "foo" to "bar"
>> cache has entry "bar" added
>> both entries have the same gid
>>
>> - id_provider = ad
>>   cache has "foo" entry
>>   After renaming "foo" to "bar"
>> nothing is changed in the cache
>>
>> Any of these situation look exactly right for me (and here I'm
>> probably wrong). My expectations are that we should, for both cases:
>> - check for the gid in the cache
>> - update the entry
>
> Unfortunately renaming users and groups is not so easy because of the
> member and memberof links. The memberof link points from the user to the
> group and contains the name of the entry they point to, therefore need to
> be changed one way or another during the group rename.
>
> If the group was renamed using ldb_rename(), then the link change should
> be done by the memberof ldb plugin, but it's not implemented there.
>
> Since the memberof plugin only implements add, del and modify
> operations, we solve this by removing the group and re-adding it with
> different name in sysdb_store_new_group().
>
>>
>> So in the ldap case we would avoid having two entries with the same
>> gid and in the ad case we would be able to properly the updated name
>> of the group that the user is part of. Does it make sense?
>
> I wonder why we have two entries with a duplicate gid in the first
> place. When saving the duplicate "bar", don't we hit
> sysdb_store_new_group() -> sysdb_delete_group() ?

According to my tests this part is never triggered. May be because
it's using rfc2307bis?

What I can see here is that I ended up in:
- sdap_add_incomplete_groups(), which tries to search the group by its
name and, obviously doesn't find it, so the missing group ends up
being saved (again).

I've tried to search by the group id instead of searching by the group
name, which works fine for this part, but then breaks when saving the
user memberships (in case searching a user that is not cached), as the
group with the new name is not found.

So, yeah, I guess what we are missing here is just deleting the old
group and add the new one in this case, but I still have to figure out
how to do that (and if I get stuck on it, don't worry, I'll bother
some of the developers on IRC).

One thing that I'd like to be sure is that it does make sense to be
done when using rfc3207bis.

[snip]

Best Regards,
--
Fabiano Fidêncio
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] Re: [sssd PR#107][comment] WATCHDOG: Avoid non async-signal-safe from the signal_handler

[Fabiano Fidêncio]

On Tue, Jan 10, 2017 at 10:13 AM, lslebodn
 wrote:
>   URL: https://github.com/SSSD/sssd/pull/107
> Title: #107: WATCHDOG: Avoid non async-signal-safe from the signal_handler
>
> lslebodn commented:
> """
> Are there any objection to the proposed change?
> """
>
> See the full comment at 
> https://github.com/SSSD/sssd/pull/107#issuecomment-271523691
>
> ___
> sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
> To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
>

I kinda okay with the changes. There are a few things that go against
my preference but, sincerely, doesn't worth even arguing about then.

Feel free to squash your patch on mine before pushing ...

Or do you want me to submit a new patchset?
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] Re: RFC: Socket-activation, some changes in the architecture.

[Fabiano Fidêncio]

On Mon, Jan 9, 2017 at 1:35 PM, Jakub Hrozek  wrote:
> On Mon, Jan 09, 2017 at 01:25:48PM +0100, Pavel Březina wrote:
>> On 01/08/2017 09:44 PM, Fabiano Fidêncio wrote:
>> > People,
>> >
>> > Recently I've faced some issues when testing the socket-activation
>> > working running as sssd-user, which will force me to take a different
>> > path for a few things and I really would like to know your opinion on
>> > those things.
>> >
>> > So, currently, this is what the nss.service looks like:
>> >
>> > [Unit]
>> > Description=SSSD NSS Service responder
>> > Documentation=man:sssd.conf(5)
>> > After=sssd.service
>> > BindsTo=sssd.service
>> >
>> > [Install]
>> > Also=sssd-nss.socket
>> >
>> > [Service]
>> > ExecStartPre=-/bin/chown @SSSD_USER@:@SSSD_USER@ @logpath@/sssd_nss.log
>> > ExecStart=@libexecdir@/sssd/sssd_nss --debug-to-files --unprivileged-start
>> > Restart=on-failure
>> > User=@SSSD_USER@
>> > Group=@SSSD_USER@
>> > PermissionsStartOnly=true
>> >
>> > As you probably noticed, I've been using systemd's machinery to change
>> > the debug files' owner and to start the responder by the proper user
>> > (sssd or root). Well, it doesn't work that well as expected as systemd
>> > ends up calling initgroups(sssd, ...) in order to start any service
>> > using "sssd" user and this call is done _before_ starting the NSS
>> > responder, which will hang for the "default client timeout" (300s).
>> >
>> > Okay, we have to change it and here is where I need your help!
>>
>> The simplest solution would be to disable socket activation for NSS
>> responder. Socket activation is supposed to be used for responders that are
>> seldom used.
>
> I also wonder if this was the easiest. Just enable the service as well
> in the RPM..

And it still would have to be running as root.

Latest suggestion from Lukáš makes my life way easier (just leave this
reponder to be ran as root and that's it).
Although, I really think we could do it in a better and more generic way

Best Regards,
--
Fabiano Fidêncio
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] Re: RFC: Socket-activation, some changes in the architecture.

[Fabiano Fidêncio]

On Mon, Jan 9, 2017 at 11:23 AM, Lukas Slebodnik  wrote:
> On (08/01/17 21:44), Fabiano Fidêncio wrote:
>>diff --git a/src/util/server.c b/src/util/server.c
>>index c9a2726..d9da803 100644
>>--- a/src/util/server.c
>>+++ b/src/util/server.c
>>@@ -445,6 +445,7 @@ static const char *get_pid_path(void)
>>
>> int server_setup(const char *name, int flags,
>>  uid_t uid, gid_t gid,
>>+ char *sssd_user,
>>  const char *conf_entry,
>>  struct main_context **main_ctx)
>> {
>>@@ -463,6 +464,22 @@ int server_setup(const char *name, int flags,
>>
>> setenv("_SSS_LOOPS", "NO", 0);
>>
>>+if (sssd_user != NULL) {
>>+struct passwd *pw;
>>+
>>+pw = getpwnam(sssd_user);
>>+if (pw == NULL) {
>>+DEBUG(SSSDBG_MINOR_FAILURE,
>>+  "Cannot get the information about the user %s. "
>>+  "The process will be run as root\n", sssd_user);
>   BTW We should not fall back to root.
>   User might expect that service will be running as non-root
>   and due to typo it will fall back to root. They needn't notice it.

Okay, so aborting is the best thing to do?
I'm okay with that as well.


>>+uid = 0;
>>+gid = 0;
>>+} else {
>>+uid = pw->pw_uid;
>>+gid = pw->pw_gid;
>>+}
>>+}
>>+
>> ret = chown_debug_file(NULL, uid, gid);
>>
>>Okay, okay. It would work. Would it be okay for the rest of the developers?
>>
> //snip
>>So, summing up the questions ...
>>- Is there any problem on calling 'setenv("_SSS_LOOPS", "NO", 0)'
>>earlier in the process?
>>- Would be okay adding "--sssd-user" option for starting the responders?
> This option is required just for nss responder due to circular dependency
> with socket activation. Does it worth to add it to each service?
> Because other services can be easily handled by systemd non-root solution
> without any problem.

Yeah, they can, but then, IMO, it won't be consistent (considering
that we can have something a bt more consistent, I'd go for it).

>
>>- Shall I get rid of the "--uid and --gid" options in order to only
>>use the new added "--sssd-user" one?
>
>
>>- Shall I add a new "--socket-activated" option for starting the
>>responders or just abuse the "--sssd-user" one?
> -1 for misusing other options

I completely agree here. So "--socket-activated" may be a good way to go.
.
>
> If others developers prefer replacing --uid/gid with "--sssd-user"
> then we would not be able to distinguish between socket activated
> non-root and sssd monitor version of non-root.

Well. that's not true for two reasons:
- Currently we don't distinguish those and I don't see why we should.
By the code path taken in the monitor side it knows whether the
process is socket-activated or not and, actually, let the service know
whether it was socket-activated or not.
- Adding "---socket-activated" option would makes things simpler and
let me remove the patch where the monitor lets the responders know
whether they were socket-activated or not.

> And we still need to
> be able to handle current solution due backward compatibility.

Please, I don't completely understand here.
Which backward compatibility problem you see? Could you be a bit more verbose?

Best Regards,
--
Fabiano Fidêncio
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] Re: RFC: Socket-activation, some changes in the architecture.

[Fabiano Fidêncio]

On Mon, Jan 9, 2017 at 10:27 AM, Lukas Slebodnik  wrote:
> On (08/01/17 21:44), Fabiano Fidêncio wrote:
>>People,
>>
>>Recently I've faced some issues when testing the socket-activation
>>working running as sssd-user, which will force me to take a different
>>path for a few things and I really would like to know your opinion on
>>those things.
>>
>>So, currently, this is what the nss.service looks like:
>>
>>[Unit]
>>Description=SSSD NSS Service responder
>>Documentation=man:sssd.conf(5)
>>After=sssd.service
>>BindsTo=sssd.service
>>
>>[Install]
>>Also=sssd-nss.socket
>>
>>[Service]
>>ExecStartPre=-/bin/chown @SSSD_USER@:@SSSD_USER@ @logpath@/sssd_nss.log
>>ExecStart=@libexecdir@/sssd/sssd_nss --debug-to-files --unprivileged-start
>>Restart=on-failure
>>User=@SSSD_USER@
>>Group=@SSSD_USER@
>>PermissionsStartOnly=true
>>
>>As you probably noticed, I've been using systemd's machinery to change
>>the debug files' owner and to start the responder by the proper user
>>(sssd or root). Well, it doesn't work that well as expected as systemd
>>ends up calling initgroups(sssd, ...) in order to start any service
>>using "sssd" user and this call is done _before_ starting the NSS
>>responder, which will hang for the "default client timeout" (300s).
>>
> It is not just about initgroups(). I checked the source code and
> it also calls getpwname get grname for User/GID !=0 and Group/GID !=0.
> It is not a problem ATM but it would be problem after changing nsswitch
>  "passwd: files sss" -> "passwd: sss files"
>
> Yes; we can remove this options from nss service file; but there
> still will be a chown in ExecStartPre; which will cause problems
> after changing nsswitch configuration. It would not be a problem
> with "chown $uid:$gid" but these values are not constant.

Sorry, maybe I was not clear enough when writing the email.
My proposal is to have --sssd-user=@SSSD_USER@ passed as a command
line argument to the responder.
So, exactly as it's done nowadays, we would be able to own the debug
files and become the user without any issue (with the patches I've
described in the email).

> And we need to change owner of the log file otherwise we would
> not be able to start responder after upgrade from root user without
> socket activation -> non-root user with socket activation.

Answered above.

>
> We cannot translate sssd user to uid/gid at build time
> because values would be different on build machine (chroot ...)
> and on installed machine.

Answered above.

>
> I think the only reasonable solution for non-root mode and nss responder
> would be to provide drop-in file with uid/gid in /etc/systemd/system

I wrote the email giving another reasonable solution for non-root mode. :-\

>
> The same as for logging to journald
> /etc/systemd/system/sssd.service.d/journal.conf
> We can replace values in rpm posinstall scriptlet and enable it by default.
> Other distributions might do the same or they can rely on manual change.
> That will be a downstream packager decision.
>
> LS
> ___
> sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
> To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

Best Regards,
--
Fabiano Fidêncio
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] Group renaming issue when "id_provider = ldap" is set.

[Fabiano Fidêncio]

I've been working on rhbz#1401241
(https://bugzilla.redhat.com/show_bug.cgi?id=1401241) and I'd like to
clarify some doubts that showed up.

So, let's consider that there's a group called "foo" and user "user"
is part of this group. Group "foo" gets renamed to "bar'. Now. let me
describe what I've found out considering "id_provider = ldap" and
"id_provider = ad"

- id_provider = ldap:
  cache has "foo" entry
  After renaming "foo" to "bar"
cache has entry "bar" added
both entries have the same gid

- id_provider = ad
  cache has "foo" entry
  After renaming "foo" to "bar"
nothing is changed in the cache

Any of these situation look exactly right for me (and here I'm
probably wrong). My expectations are that we should, for both cases:
- check for the gid in the cache
- update the entry

So in the ldap case we would avoid having two entries with the same
gid and in the ad case we would be able to properly the updated name
of the group that the user is part of. Does it make sense?

Summing up the questions:
- When a group has its name changed, shall we update the entry that
contains its name (and then all memberOf that contains the old name?)?
- In case not, following what "id_provider = ad" does would be a good
fix for the issue?

Looking forward to hearing your opinion!

Best Regards,
--
Fabiano Fidêncio
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] RFC: Socket-activation, some changes in the architecture.

[Fabiano Fidêncio]

People,

Recently I've faced some issues when testing the socket-activation
working running as sssd-user, which will force me to take a different
path for a few things and I really would like to know your opinion on
those things.

So, currently, this is what the nss.service looks like:

[Unit]
Description=SSSD NSS Service responder
Documentation=man:sssd.conf(5)
After=sssd.service
BindsTo=sssd.service

[Install]
Also=sssd-nss.socket

[Service]
ExecStartPre=-/bin/chown @SSSD_USER@:@SSSD_USER@ @logpath@/sssd_nss.log
ExecStart=@libexecdir@/sssd/sssd_nss --debug-to-files --unprivileged-start
Restart=on-failure
User=@SSSD_USER@
Group=@SSSD_USER@
PermissionsStartOnly=true

As you probably noticed, I've been using systemd's machinery to change
the debug files' owner and to start the responder by the proper user
(sssd or root). Well, it doesn't work that well as expected as systemd
ends up calling initgroups(sssd, ...) in order to start any service
using "sssd" user and this call is done _before_ starting the NSS
responder, which will hang for the "default client timeout" (300s).

Okay, we have to change it and here is where I need your help!

- Adding a new "sssd-user" option:
By adding this option we could just start the responders calling:
libexecdir@/sssd/sssd_nss --debug-to-files --sssd-user=@SSSD_USER@.
The unit file would be changed a little as the ExectStartPre, User and
Group options could just go away.
The server code would be changed a little in order to invoke
setenv("_SSS_LOOPS", "NO", 0) earlier, as done here:
index 52e6ed7..013e572 100644
--- a/src/util/server.c
+++ b/src/util/server.c
@@ -461,8 +461,6 @@ int server_setup(const char *name, int flags,
 char *locale;
 int watchdog_interval;

-setenv("_SSS_LOOPS", "NO", 0);
-
 ret = chown_debug_file(NULL, uid, gid);
 if (ret != EOK) {
 DEBUG(SSSDBG_MINOR_FAILURE,
@@ -481,6 +479,8 @@ int server_setup(const char *name, int flags,
 return ENOMEM;
 }

+setenv("_SSS_LOOPS", "NO", 0);
+
 /* To make sure the domain cannot be set from the environment, unset the
  * variable explicitly when setting up any server. Backends later set the
  * value after reading domain from the configuration */

Also, the server_setup() would receive the user passed to --sssd-user
and perform a getpwnam() in order to get the uid/gid and then be able
to change debug files' owner and become the user, as done here:

diff --git a/src/util/server.c b/src/util/server.c
index c9a2726..d9da803 100644
--- a/src/util/server.c
+++ b/src/util/server.c
@@ -445,6 +445,7 @@ static const char *get_pid_path(void)

 int server_setup(const char *name, int flags,
  uid_t uid, gid_t gid,
+ char *sssd_user,
  const char *conf_entry,
  struct main_context **main_ctx)
 {
@@ -463,6 +464,22 @@ int server_setup(const char *name, int flags,

 setenv("_SSS_LOOPS", "NO", 0);

+if (sssd_user != NULL) {
+struct passwd *pw;
+
+pw = getpwnam(sssd_user);
+if (pw == NULL) {
+DEBUG(SSSDBG_MINOR_FAILURE,
+  "Cannot get the information about the user %s. "
+  "The process will be run as root\n", sssd_user);
+uid = 0;
+gid = 0;
+} else {
+uid = pw->pw_uid;
+gid = pw->pw_gid;
+}
+}
+
 ret = chown_debug_file(NULL, uid, gid);

Okay, okay. It would work. Would it be okay for the rest of the developers?

Considering that it will be okay ... doesn't it look a little bit
weird/inconsistent passing the sssd-user sometimes and passing the
uid/gid in the most part of the times? Wouldn't be better just pass
the sssd-user all the time and do the getpwnam() call here instead of
doing it in the monitor when starting it?

Now, considering we will user "--sssd-user" when calling the
responders ... I can take advantage of this in case we _only_ use this
option when socket-activating the responders, as the responder will
know from the beginning whether it was socket activated or explicitly
started (and it's useful for shutting down the responder when it's
inactive). Another option (that I personally prefer) is having, again,
a new option "--socket-activated" that would make things clear for
whoever ends up taking care of this code later on (probably the future
me). Would that be okay?

So, summing up the questions ...
- Is there any problem on calling 'setenv("_SSS_LOOPS", "NO", 0)'
earlier in the process?
- Would be okay adding "--sssd-user" option for starting the responders?
- Shall I get rid of the "--uid and --gid" options in order to only
use the new added "--sssd-user" one?
- Shall I 

[SSSD] Re: Design discussion: Fleet Commander integration

[Fabiano Fidêncio]

On Thu, Dec 22, 2016 at 2:11 PM, Oliver Gutierrez  wrote:
> Hi everybody!
>
> We have been investigating about the process to include Fleet Commander in
> RHEL and we were aiming to have it included in RHEL 7.4.
>
> Can you tell us the planning for including the FreeIPA/SSSD modifications in
> RHEL? Will be in RHEL 7.4?

I sincerely don't think we should rush it in for RHEL-7.4.

Alexander said he will work in the missing part by the end of this
year/beginning of next year. After that I'll have to work on the SSSD
part, have it released upstream and then discuss whether it makes
sense or not (and here is a question for Martin Košek) to have it for
7.4, but that's something that shouldn't be discussed here (on this
ML) yet.

[snip]

Best Regards,
--
Fabiano Fidêncio
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] Re: Design document - Socket-activatable responders

[Fabiano Fidêncio]

On Thu, Dec 1, 2016 at 3:46 PM, Simo Sorce  wrote:
> On Thu, 2016-12-01 at 15:22 +0100, Pavel Březina wrote:
>> On 12/01/2016 02:56 PM, Simo Sorce wrote:
>> > On Thu, 2016-12-01 at 14:44 +0100, Pavel Březina wrote:
>> >> On 11/24/2016 02:33 PM, Fabiano Fidêncio wrote:
>> >>> The design page is done [0] and it's based on this discussion [1] we
>> >>> had on this very same mailing list. A pull-request with the
>> >>> implementation is already opened [2].
>> >>>
>> >>> [0]: 
>> >>> https://fedorahosted.org/sssd/wiki/DesignDocs/SocketActivatableResponders
>> >>> [1]: 
>> >>> https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org/message/H6JOF5SGGSIJUIWYNANDA73ODHWBS7J2/
>> >>> [2]: https://github.com/SSSD/sssd/pull/84
>> >>
>> >> I think we should also provide 'disabled_services' option, to give
>> >> admins a way to explicitly disable some responders if the don't want to
>> >> used them.
>> >
>> > How would this work ?
>>
>> If responder is listed in disabled_services, it won't be allowed to
>> start via socket activation. If disabling the socket as Fabiano
>> mentioned in the other mail is enough, I'm fine with it, plese test.
>
> I am not sure this is a good behavior as clients will see a connection
> being accept and then dropped, and may misbehave or report strange
> errors.

So, thinking a little bit about it Pavel's idea is not bad.
If you have a list of "not allowed"/"disabled" services we can, at
least, report the proper error when enabling the service.

Does this sound reasonable to you?

>
> Simo.
>
> --
> Simo Sorce * Red Hat, Inc * New York
> ___
> sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
> To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] Re: Design document - Socket-activatable responders

[Fabiano Fidêncio]

On Thu, Dec 1, 2016 at 2:44 PM, Pavel Březina  wrote:
> On 11/24/2016 02:33 PM, Fabiano Fidêncio wrote:
>>
>> The design page is done [0] and it's based on this discussion [1] we
>> had on this very same mailing list. A pull-request with the
>> implementation is already opened [2].
>>
>> [0]:
>> https://fedorahosted.org/sssd/wiki/DesignDocs/SocketActivatableResponders
>> [1]:
>> https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org/message/H6JOF5SGGSIJUIWYNANDA73ODHWBS7J2/
>> [2]: htatps://github.com/SSSD/sssd/pull/84
>
>
> I think we should also provide 'disabled_services' option, to give admins a
> way to explicitly disable some responders if the don't want to used them.

I have to double check a few things here but, AFAIU, just having the
socket disabled (systemctl disable sssd-@responder@.socket) should be
enough.


>
> ___
> sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
> To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] Re: Design document - Socket-activatable responders

[Fabiano Fidêncio]

On Tue, Nov 29, 2016 at 10:24 AM, Fabiano Fidêncio  wrote:
> On Tue, Nov 29, 2016 at 10:01 AM, Lukas Slebodnik  wrote:
>> On (28/11/16 11:27), Jakub Hrozek wrote:
>>>On Mon, Nov 28, 2016 at 10:57:44AM +0100, Pavel Březina wrote:
>>>> On 11/28/2016 10:47 AM, Jakub Hrozek wrote:
>>>> > On Thu, Nov 24, 2016 at 02:33:04PM +0100, Fabiano Fidêncio wrote:
>>>> > > The design page is done [0] and it's based on this discussion [1] we
>>>> > > had on this very same mailing list. A pull-request with the
>>>> > > implementation is already opened [2].
>>>> > >
>>>> > > [0]: 
>>>> > > https://fedorahosted.org/sssd/wiki/DesignDocs/SocketActivatableResponders
>>>> > > [1]: 
>>>> > > https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org/message/H6JOF5SGGSIJUIWYNANDA73ODHWBS7J2/
>>>> > > [2]: https://github.com/SSSD/sssd/pull/84
>>>> > >
>>>> > > The full text of c&p here:
>>>> >
>>>> > In general looks good to me, but note that I was involved a bit with
>>>> > Fabiano in the discussion, so my view might be tainted.
>>>>
>>>> I finally got to it. The design page looks good and I'll start reviewing 
>>>> the
>>>> patches.
>>>>
>>>> The only think I wonder about is whether we want to pass parameters " --uid
>>>> 0 --gid 0 --debug-to-files" or we will read the from sssd.conf? I prefer
>>>> reading them.
>>>>
>>>> Also what do we use the private sockets for? It is used only for root?
>>>
>>>Yes, that's where we route PAM requests started by UID 0 to.
>>>
>> For example. The nss responder need't run as root. It does not require
>> any extra privileges. And the privileges are dropped as soon as possible.
>> The only issue might be with switching from root to non-root.
>> A responder need to change owner of log files.
>> But it could be solved with ExecStartPre in service file
>>
>> e.g.
>> ExecStartPre=/usr/bin/chown sssd:sssd /var/log/sssd/sssd_nss.log
>> ExecStart=/usr/libexec/sssd/sssd_nss --debug-to-files
>> User=sssd
>> Group=sssd
>> PermissionsStartOnly=true
>>
>> @see the explanation of PermissionsStartOnly in man 5 systemd.service
>
> I like the suggestion. But I also would like to ask which are the
> responders that have to executed as root?

This question still stands ...

We have the following responders: autofs, ifp, nss, pac, pam, ssh and sudo.
All of those can run as sssd user?

>
> Best Regards,
> --
> Fabiano Fidêncio
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] Re: Design document - Socket-activatable responders

[Fabiano Fidêncio]

On Tue, Nov 29, 2016 at 11:28 AM, Sumit Bose  wrote:
> On Thu, Nov 24, 2016 at 02:33:04PM +0100, Fabiano Fidêncio wrote:
>> The design page is done [0] and it's based on this discussion [1] we
>> had on this very same mailing list. A pull-request with the
>> implementation is already opened [2].
>>
>> [0]: 
>> https://fedorahosted.org/sssd/wiki/DesignDocs/SocketActivatableResponders
>> [1]: 
>> https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org/message/H6JOF5SGGSIJUIWYNANDA73ODHWBS7J2/
>> [2]: https://github.com/SSSD/sssd/pull/84
>>
>> The full text of c&p here:
>
> Hi Fabiano,
>
> thank you for the design page.
>
>>
>> = Socket Activatable Responders =
>>
> ...
>>  KCM 
>> The KCM responder is only seldom needed, when libkrb5 needs to access
>
> I'm not sure id 'seldom' is correct here. It will be used by mainly
> during authentication but since it might be to default storage for any
> Kerberos credential it will be use by the user for any kind of
> Kerberos/GSSAPI related authentication to services.
>
>> the credentials store. At the same time, the KCM responder must be
>> running if the Kerberos credentials cache defaults to `KCM`.
>> Socket-activating the responder would solve both of these cases.
>>
>
> But my main question is about the monitor and
> /var/lib/sss/db/config.ldb. If I understand the design correctly the
> monitor will still run and create config.ldb which is good because then
> all socket-activated components will see the same config. So a
> configuration change required a restart of the monitor. How will the
> socket activated services handled in this case. Will the monitor shut
> them down or is systemd smart enough to see that the "parent" service is
> shut down and will shutdown the children as well.

systemd is smart enough to do that, or almost. :-)
Having "PartOf=sssd.service" as part of responders' unit files is
enough to make then restart/shutdown any time systemd is
restarted/shutdown.

Best Regards,
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] Re: Design document - Socket-activatable responders

[Fabiano Fidêncio]

On Tue, Nov 29, 2016 at 10:01 AM, Lukas Slebodnik  wrote:
> On (28/11/16 11:27), Jakub Hrozek wrote:
>>On Mon, Nov 28, 2016 at 10:57:44AM +0100, Pavel Březina wrote:
>>> On 11/28/2016 10:47 AM, Jakub Hrozek wrote:
>>> > On Thu, Nov 24, 2016 at 02:33:04PM +0100, Fabiano Fidêncio wrote:
>>> > > The design page is done [0] and it's based on this discussion [1] we
>>> > > had on this very same mailing list. A pull-request with the
>>> > > implementation is already opened [2].
>>> > >
>>> > > [0]: 
>>> > > https://fedorahosted.org/sssd/wiki/DesignDocs/SocketActivatableResponders
>>> > > [1]: 
>>> > > https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org/message/H6JOF5SGGSIJUIWYNANDA73ODHWBS7J2/
>>> > > [2]: https://github.com/SSSD/sssd/pull/84
>>> > >
>>> > > The full text of c&p here:
>>> >
>>> > In general looks good to me, but note that I was involved a bit with
>>> > Fabiano in the discussion, so my view might be tainted.
>>>
>>> I finally got to it. The design page looks good and I'll start reviewing the
>>> patches.
>>>
>>> The only think I wonder about is whether we want to pass parameters " --uid
>>> 0 --gid 0 --debug-to-files" or we will read the from sssd.conf? I prefer
>>> reading them.
>>>
>>> Also what do we use the private sockets for? It is used only for root?
>>
>>Yes, that's where we route PAM requests started by UID 0 to.
>>
> For example. The nss responder need't run as root. It does not require
> any extra privileges. And the privileges are dropped as soon as possible.
> The only issue might be with switching from root to non-root.
> A responder need to change owner of log files.
> But it could be solved with ExecStartPre in service file
>
> e.g.
> ExecStartPre=/usr/bin/chown sssd:sssd /var/log/sssd/sssd_nss.log
> ExecStart=/usr/libexec/sssd/sssd_nss --debug-to-files
> User=sssd
> Group=sssd
> PermissionsStartOnly=true
>
> @see the explanation of PermissionsStartOnly in man 5 systemd.service

I like the suggestion. But I also would like to ask which are the
responders that have to executed as root?

Best Regards,
--
Fabiano Fidêncio
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] Re: Design document - Socket-activatable responders

[Fabiano Fidêncio]

On Thu, Nov 24, 2016 at 2:33 PM, Fabiano Fidêncio  wrote:
> The design page is done [0] and it's based on this discussion [1] we
> had on this very same mailing list. A pull-request with the
> implementation is already opened [2].
>
> [0]: https://fedorahosted.org/sssd/wiki/DesignDocs/SocketActivatableResponders
> [1]: 
> https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org/message/H6JOF5SGGSIJUIWYNANDA73ODHWBS7J2/
> [2]: https://github.com/SSSD/sssd/pull/84

Well, PR#94: https://github.com/SSSD/sssd/pull/94

>
> The full text of c&p here:
>
> = Socket Activatable Responders =
>
> Related ticket(s):
>  * https://fedorahosted.org/sssd/ticket/2243
>  * https://fedorahosted.org/sssd/ticket/3129
>
> === Problem statement ===
> SSSD has some responders which don't have to be running all the time,
> but could be socket-activated instead in platforms where it's
> supported. That's the case, for instance, for the IFP, ssh and sudo
> responders.
> Making these responders socket-activated would provide a better use
> experience, as these services could be started on-demand when a client
> needs them and exist after a period of inactivity. Currently the admin
> has to explicitly list all the services that might potentially be
> needed in the `services` section and the processes have to be running
> all the time.
>
> === Use cases ===
>
>  sssctl  
> As more and more features that had been added depending on the IFP
> responder, we should make sure that the responder is activated on
> demand and the admins doesn't have to activate it manually.
>
>  KCM 
> The KCM responder is only seldom needed, when libkrb5 needs to access
> the credentials store. At the same time, the KCM responder must be
> running if the Kerberos credentials cache defaults to `KCM`.
> Socket-activating the responder would solve both of these cases.
>
>  autofs 
> The autofs responder is typically only needed when a share is about to
> be mounted.
>
> === Overview of the solution ===
> The solution agreed on the mailing list is to add a new unit for each
> one of the responders. Once a responder is started, it will
> communicate to the monitor in order to let the monitor know that it's
> up and the monitor will do the registration of the responder, which
> basically consists in marking the service as started, increasing the
> services' counter, getting the responder's configuratiom, adding the
> responder to the service's list.
> A configurable idle timeout will be implemented in each responder, as
> part of this task, in order to exit the responder in case it's not
> used for a few minutes.
>
> === Implementation details ===
> In order to achieve our goal we will need a small modification in
> responders' common code in order to make it ready for
> socket-activation, add some systemd units for each of the responders
> and finally small changes in the monitor code in order to manage the
> new activated service.
>
> The change in the responders' common code is quite trivial, just
> chnage the sss_process_init code to call activate_unix_sockets()
> istead of set_unix_socket(). Something like:
> {{{
> -ret = set_unix_socket(rctx, conn_setup);
> +ret = activate_unix_sockets(rctx, conn_setup);
> }}}
>
> The units that have to be added for each responder must look like:
>
> sssd-@respon...@.service.in:
> {{{
> [Unit]
> Description=SSSD @responder@ Service responder
> Documentation=man:sssd.conf(5)
> Requires=sssd.service
> PartOf=sssd.service
> After=sssd.service
>
> [Install]
> Also=sssd-@responder@.socket
>
> [Service]
> ExecStart=@libexecdir@/sssd/sssd_@responder@ --uid 0 --gid 0 --debug-to-files
> }}}
>
> sssd-@respon...@.socket.in:
> {{{
> [Unit]
> Description=SSSD @responder@ Service responder socket
> Documentation=man:sssd.conf(5)
>
> [Socket]
> ListenStream=@pipepath@/@responder@
>
> [Install]
> WantedBy=sockets.target
> }}}
>
> Some responders may have more than one socket, which is the case of
> PAM, so another unit will be needed.
>
> sssd-@respon...@-priv.socket.in:
> {{{
> [Unit]
> Description=SSSD @responder@ Service responder private socket
> Documentation=man:sssd.conf(5)
>
> [Socket]
> ListenStream=@pipepath@/private/@responder@
>
> [Install]
> WantedBy=sockets.target
> }}}
>
> Last but not least, the IFP responder doesn't have a socket. It's
> going to be D-Bus activated and some small changes will be required on
> its D-Bus service unit.
> {{{
> -Exec=@libexecdir@/sssd/sss_signal
> +Exec=@libexecdir@/sssd/sssd_@responder@
&g

[SSSD] Design document - Socket-activatable responders

[Fabiano Fidêncio]

 the service's restart number;
 * Add the service to the services' list.

=== Configuration changes ===
After this design is implemented, the "services" line in sssd.conf
will become optional for platforms where systemd is present. Note that
in order to keep backward compatibility, if the "services" line is
present, the services will behave exactly as they did before these
changes.

=== How To Test ===
The easiest way to test is removing the "services" line from sssd.conf
and try to use SSSD normally.
Using sssctl tool without having the ifp responder set in the
"services" line is another way to test.

=== How To Debug ===
The easiest way to debug this new feature is taking a look on the
responders' common initialization code and in the monitors' client
registration code.
Is worth to mention that disabling the systemd's services/sockets will
prevent the responders' services to be started.

=== Authors ===
Fabiano Fidêncio 

Best Regards,
--
Fabiano Fidêncio
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] Re: [RFC] Socket-activate responders

[Fabiano Fidêncio]

On Wed, Nov 16, 2016 at 11:20 AM, Jakub Hrozek  wrote:
> On Wed, Nov 16, 2016 at 10:03:36AM +0100, Fabiano Fidêncio wrote:
>> People,
>>
>> I've spent some time looking at the code and trying to understand what
>> are the needed changes in order to have this task done. I'll start by
>> writing down how things are working nowadays, what we want to achieve,
>> what are the parts that will need to be touched and what are the steps
>> that I'm going to take. Please, keep in mind that I may have a wrong
>> (or at least not so clear) understanding of all these points that I'm
>> about to explain and in case I made some mistake feel free to jump in
>> and correct me. Also, whatever we sum up from this email will be
>> written down in our DesignFeatures page. Let's start ...
>>
>> How things work nowadays:
>> 
>>
>> Nowadays all the services are started and taken care by the monitor.
>> It basically means that the monitor checks which are the services
>> listed to be started, starts those and registers them in order to
>> relay them signals coming from our tools. Here I'm not sure whether
>> the monitor relays signals from something else but the tools, but
>> doesn't seem to be case (and please, correct me in case I'm wrong).
>
> Monitor also relays:
> - signals that the administrator sends manually (see man sssd(8) and
>   search for USR1/USR2/HUP)

This actually affects the responders, right?

> - notifications that /etc/resolv.conf had changed and the domains
>   should reset their offline status
> - notifications that networking on the machine had changed (received
>   via the netlink socket) and the domains should reset their offline
>   status

These don't as far as I understand.

>
> These mostly (except for clearing the memory cache of NSS responder and
> rotating the logs) concern the providers which we won't be changing now.
>
> And actually I wouldn't see too much harm with moving the resolv.conf
> monitoring and netlink monitoring to back ends themselves..both 'just'
> mean an open fd and the number of sssd_be processes running is usually small.

Right, I like the suggestion but I'd like to keep this out of the
scope for this task right now.

>
>>
>> What we want to achieve:
>> -
>>
>> While my personal desire is to start slowly killing the monitor,
>> that's not going to be case right now.
>
> Well, we can incrementally downsize the monitor..
>
>> We don't want to do any change in the code that wouldn't also
>> contemplate platforms where systemd is not available.
>
> Yes, but perhaps we can move the non-systemd code to a separate module
> to avoid a lot of code in the monitor being #ifdef-ed out. (I don't know
> if this will actually be the case in the end)

That's a good idea.
I'll go pretty much if-def'ing the code and in the end split this part
into a separate module in case it makes sense.

>
>> That being said,
>> let's move to the important part ...
>>
>> Here what we want to achieve is to have all responders (at least for
>> now, yes, just the responders) socket-activatable as some of them
>> don't actually have to be running all the time (that's the case, for
>> example, of the ssh, sudo and ifp responders). We also have to keep
>> into consideration that _any_ change in behaviour should _not_ happen,
>> which means that we still have to honor our compromise with sssd.conf
>> and still make the responders manually activated there to start and be
>> running from the moment sssd is running (as we do nowadays).
>
> Right, this is a possibility. I think we can use the "services" line to
> configure how the responders are started. If any service was explicitly
> listed in "services", then it would be also explicitly started by the
> monitor just as it is now.

Right.

>
> I think we also want to add an idle timeout to the services and the services
> that are started explicitly would just have an infinite idle timeout..

Why? If the services are started explicitly we can just use the same
logic used nowadays.
So, I have the feeling that no changes would be needed at all in this case.

>
> Other systemd services that correspond to a responder might be disabled
> by default and the admin can just activate them to allow them to be
> started on demand (that's already the case with sssd-secrets, right?)

Yes, you're right.

>
> But this could also be just a middle step..in future, since the respond

[SSSD] [RFC] Socket-activate responders

[Fabiano Fidêncio]

timation about when we will have
it done, mainly because I'd like to hear the feedback about this from
others in the team.

Looking forward for your reading back from you!

Best Regards,
--
Fabiano Fidêncio
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] Re: Design discussion: Fleet Commander integration

[Fabiano Fidêncio]

On Fri, Oct 7, 2016 at 10:22 AM, Jakub Hrozek  wrote:
> On Thu, Oct 06, 2016 at 06:38:23PM +0200, Sumit Bose wrote:
>> On Thu, Oct 06, 2016 at 04:41:10PM +0200, Jakub Hrozek wrote:
>> > Hi,
>> >
>> > with Alexander's help, I wrote up a design page about how SSSD should
>> > read Fleet Commander data from IPA and present them to the FC client
>> > component. The SSSD part is described here:
>> > https://fedorahosted.org/sssd/wiki/DesignDocs/FleetCommanderIntegration
>> > and the IPA part is here:
>> > 
>> > https://github.com/abbra/freeipa-desktop-profile/blob/master/plugin/Feature.mediawiki
>> >
>> > For convenience, I copied the SSSD wiki page below. Comments are welcome!
>> >
>>
>> ...
>>
>> >
>> >  Looking up the Fleet Commander profiles and storing the JSON profile 
>> > data 
>> > Since the first implementation will only fetch rules that are linked to
>> > this host and the user in question, the SSSD's session provider will issue
>> > an LDAP search along these lines:
>> > {{{
>> > 
>> > (&(objectclass=ipadeskprofilerule)(memberHost=my_fqdn_or_my_host_group)(memberUser=user_login_or_group))
>> > }}}
>> >
>> > All host groups the IPA client is a member of must be included in the
>> > `memberHost` part of the filter. Additionally, all user groups must be
>> > included in the `memberUser` part of the filter. Since in most cases,
>> > the user's groups will be resolved during the login, we will only issue
>> > an initgroups request in case the user's initgroups are expired already
>> > to cover cases where the sessions provider was invoked separately.
>>
>> I wonder if it would be more efficient to read all profiles which apply
>> to the host in a single run store them in the cache and do the remaining
>> part of the processing locally? Iirc this is what we do with HBAC rules
>> and there might be a chance to reuse some of the HBAC code but just look
>> for objectclass ipadeskprofilerule instead of ipahbacrule?
>>
>> Since there are host and user categories mentioned on the server side
>> design page I guess the underlying objectclass is ipaAssociation and
>> because of this it makes even more sense to reuse as much of the HBAC
>> lookup code as possible.
>
> Yes, of course you are right, fetching the per-host data is almost always
> a good idea. I changed the wiki page:
> 
> https://fedorahosted.org/sssd/wiki/DesignDocs/FleetCommanderIntegration?action=diff&version=3&old_version=1

Since I started working on this a few changes have been done in the
Design (and I've talked to Jakub on IRC about those all the time).
In case anyone is interested, here are the changes:
https://fedorahosted.org/sssd/wiki/DesignDocs/FleetCommanderIntegration?action=diff&version=7&old_version=3

Best Regards,
--
Fabiano Fidêncio
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] Re: Should we use VMs or containers for (some) tests?

[Fabiano Fidêncio]

On Tue, Nov 1, 2016 at 10:36 AM, Jakub Hrozek  wrote:
> On Tue, Nov 01, 2016 at 10:31:20AM +0200, Nikolai Kondrashov wrote:
>> Hi Jakub,
>>
>> On 10/27/2016 05:20 PM, Jakub Hrozek wrote:
>> > I'm currently working on integration tests for the 'files' provider and
>> > during this work I started to feel we are pushing the boundaries around our
>> > test infrastructure already quite a bit. When SSSD talks over network to
>> > a server, then we're more or less okay, but for some parts of SSSD, like
>> > the files provider, we have to mock a lot of pieces and the end result
>> > is that we are testing something that resembles the target system, but
>> > probably has its own bugs. Additionally, we can't run some tests at all
>> > (anything against IPA) and I suspect we'll be running into this sort of
>> > a problem even more in the future.
>> >
>> > So I'm interested in hearing what are other's thoughts on exploring how
>> > to run some of our tests in a privileged environment, either in a VM or
>> > in a container?
>> >
>> > Our current tests have the big advantage of being able to provision a
>> > test locally in a screen session, but maybe something similar would be
>> > possible by e.g. running a screen in a container and attaching to its
>> > tty.. And for "simple" tests like LDAP provider we could keep the current
>> > infrastructure around.
>>
>> I agree that the current test setup is limiting, can be difficult to work
>> with, and bring its own issues. However I believe we haven't employed it
>> fully yet, and I'm not sure how spending time on another test system compares
>> to just writing more tests (e.g. for Samba and just general tests).
>
> Yes, many tests can still continue exploring the wrapped scenario. I
> think I will be perfectly happy writing tests for the KCM service using a
> cwrapped KDC. If your tests talks to a server, then as long as the server
> is running and you're talking to it over socket_wrapper, you're fine.
>
> But to give a concrete example, here are the issues I ran into with the
> files provider tests:
> - in production, the files provider dlopens libnss_files.so and
>   calls its functions. I had to wrap a different module for tests
>   that calls nss_wrapped functions instead to reach nss_wrapper's
>   passwd and groups. This is just additional work, but I wouldn't have
>   to do it in a VM/container.
> - more importantly, one part of testing the files provider
>   is detecting the changes the admin does to /etc/passwd and
>   /etc/groups. In tests, I wrote a simple module that modifies
>   nss_wrappers' passwd and group files, but then I need to take care
>   so that my module changes the files using the same modifications
>   (create a new file and replace the old one with it, not modify
>   the old file in-place) otherwise the files provider was receiving
>   different inotify callbacks. In a container or a VM, I would just
>   call the shadow-utils binaries.
>
> Other examples of things we can't test easily with wrappers are D-Bus
> system bus interfaces (these wouldn't be too hard to wrap, though,
> 'just' additional work) and anything that involves an IPA server.
>
>>
>> Regardless, I was casually thinking about this for a while and have this to
>> say. I think being able to run tests locally, on your machine, easily is very
>> important. It is also important to let outside developers run those tests 
>> with
>> minimal setup.
>
> Yes, I really like being able to spin up a screen session and work on my
> test there. But what I'm worried about is that (see the examples about
> files provider) by writings wrappers and helpers for my tests so that
> they run in a cwrapped environment is actually testing something
> different than what the real system would do.
>
>>
>> The Docker container registry helps with that a lot: we can just publish the
>> containers we built and pull/refresh them automatically with each run, and so
>> could outside developers. At least as far as I understand it. I don't think
>> there is a similar service for VM images, and I don't think we want to build
>> our own.
>
> There is:
> https://atlas.hashicorp.com/boxes/search
> but of course, this would require the system running the test to have
> vagrant installed.
>
> Looking at how Cockpit runs their test, they simply publish qcow2 disk
> images on Internet and fetch them from there..
>
>>
>> Even though Docker is still limiting, it is less limiting than the current
>> setup.
>
> (Replying to Michal also here..)
>
> In my view, whether we use containers or VMs should be a detail as long
> as the test is concerned, the test would just execute and output its
> results "somewhere". Of course, the test runner must set up the
> environment for the test and that's where we either provision a VM or a
> container.
>
> Containers might be more dense (less resource-hungry) and it's quite
> easy to bind-mount directories to share data like log fil

[SSSD] Re: Help : Tickets suitable for newcomers

[Fabiano Fidêncio]

On Wed, Oct 26, 2016 at 8:51 PM, Jakub Hrozek  wrote:
> On Wed, Oct 26, 2016 at 11:37:22PM +0530, Amit Kumar wrote:
>> Hello,
>> Thanks for response.
>> I made code change in src/providers/ipa/ipa_access.c
>> # make
>> # make intgcheck
>> configure: error: source directory already configured; run "make distclean"
>> there first
>> Makefile:27077: recipe for target 'intgcheck-prepare' failed
>> make[1]: *** [intgcheck-prepare] Error 1
>> make[1]: Leaving directory '/root/sssd'
>> Makefile:27110: recipe for target 'intgcheck' failed
>> make: *** [intgcheck] Error 2
>
> I presume you ran configure in the same directory as the sources? It's
> better to use a parallel build directory. See the bash helper at
> contrib/fedora/bashrc_sssd, it contains some macros that you can use..

And here is a link explaining the best way to contribute to SSSD,
which includes the bash helper mentioned by Jakub:
https://fedorahosted.org/sssd/wiki/Contribute

If you find something that's inconsistent there, feel free to propose
a change and we will happily apply.

Best Regards,
--
Fabiano Fidêncio
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] Re: Help : Tickets suitable for newcomers

[Fabiano Fidêncio]

Amit,

On Wed, Oct 26, 2016 at 2:40 PM, Amit Kumar  wrote:
>
> Hi,
> https://fedorahosted.org/sssd/ticket/1372
>
> Is this ticket still valid?
> Owned by:pcech
> Opened 4 years ago Last modified 3 months ago
>
> Which tickets should be picked?
>
> Thanks Much!!!
> Amit Kumar
>
> ___
> sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
> To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
>

Firstly, thanks for your interest to hack on SSSD!

The ticket is still valid. If you want to discuss something about it
feel free to join #sssd channel at freenode (be patient there, please,
answers don't come as fast as some people want to, but they eventually
come at some point ;-)) or even ask your question in the ticket you
have mentioned.

Also, we have an "easy fixes" tag[0] in our trac with bugs that are
suitable for newcomers. Again, if you want to discuss something about
those bugs, feel free to contact us on #sssd or through the ticket
itself.

[0]: 
https://fedorahosted.org/sssd/query?status=assigned&status=new&status=reopened&keywords=~easyfix&col=id&col=summary&col=type&col=status&col=priority&col=milestone&col=component&order=priority&report=34

Best Regards,
--
Fabiano Fidêncio
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] Re: [sssd PR#33][synchronized] SECRETS: Some small misc fixes + fixing #3168

[Fabiano Fidêncio]

Lukaš,

On Mon, Oct 17, 2016 at 4:59 PM, Lukas Slebodnik  wrote:
> On (17/10/16 14:35), Fabiano Fidêncio wrote:
>>On Mon, Oct 17, 2016 at 11:46 AM, Lukas Slebodnik  wrote:
>>> On (30/09/16 16:55), fidencio wrote:
>>>>   URL: https://github.com/SSSD/sssd/pull/33
>>>>Author: fidencio
>>>> Title: #33: SECRETS: Some small misc fixes + fixing #3168
>>>>Action: synchronized
>>>>
>>>>To pull the PR as Git branch:
>>>>git remote add ghsssd https://github.com/SSSD/sssd
>>>>git fetch ghsssd pull/33/head:pr33
>>>>git checkout pr33
>>>
>>> >From 06a0a81193d6bbe3a0932c8b584433f3cc13fa51 Mon Sep 17 00:00:00 2001
>>>>From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= 
>>>>Date: Sun, 25 Sep 2016 20:49:16 +0200
>>>>Subject: [PATCH 1/6] CONFIG: Add secrets responder to the allowed sections
>>>>MIME-Version: 1.0
>>>>Content-Type: text/plain; charset=UTF-8
>>>>Content-Transfer-Encoding: 8bit
>>>>
>>>>The regular expression used is quite specific for the two cases we
>>>>support:
>>>>- [secrets]
>>>>- [secrets/users/$uid]
>>>>
>>>>It could be done a bit more generic, but the way it's right now it can
>>>>easily catch errors like: [secrets/usrs/$uid] or [secrets/].
>>>>
>>>>Related:
>>>>https://fedorahosted.org/sssd/ticket/3207
>>>>
>>>>Signed-off-by: Fabiano Fidêncio 
>>>>---
>>>> src/config/cfg_rules.ini | 1 +
>>>> 1 file changed, 1 insertion(+)
>>>>
>>>>diff --git a/src/config/cfg_rules.ini b/src/config/cfg_rules.ini
>>>>index 01be0c6..023ceac 100644
>>>>--- a/src/config/cfg_rules.ini
>>>>+++ b/src/config/cfg_rules.ini
>>>>@@ -8,6 +8,7 @@ section = autofs
>>>> section = ssh
>>>> section = pac
>>>> section = ifp
>>>>+section_re = ^secrets\(/users/\([0-9]\+\)\?\)\?$
>>>> section_re = ^domain/.*$
>>>
>>> Is it expected that section the name "secrets/users/"
>>> is allowed.
>>
>>I don't think so.
>>I'll answer your questions based on the my understanding of the
>>conversation I had with Jakub on the #sssd channel.
>>Jakub, Simo, please, feel free to jump in and correct me if I'm
>>mistaken in any point.
>>
>>>
>>> Which of following section should be allowed?
>>>
>>> sh# cat /etc/sssd/conf.d/10_secrets.conf
>>> [secrets
>>> description = temp
>>
>>Not allowed, but [secrets] is allowed.
>>
> This one was a typo on my side.
>
>>>
>>> [secrets/users]
>>> description = temp
>>>
>>
>>Shouldn't be allowed.

This is wrong right now :-\

>>
>>> [secrets/users/]
>>> description = temp
>>>
>>
>>Shouldn't be allowed.
>>
> But it is allowed.

Anything terminating with a / will cause the following error and it's
not related to secrets only.
(Mon Oct 17 16:59:22:585705 2016) [sssd] [confdb_init_db] (0x0010):
Could not create LDIF for confdb
(Mon Oct 17 16:59:22:585898 2016) [sssd] [confdb_setup] (0x0010):
ConfDB initialization has failed [22]: Invalid argument
(Mon Oct 17 16:59:22:586078 2016) [sssd] [sss_tool_confdb_init]
(0x0010): Unable to setup ConfDB [22]: Invalid argument


>
> Following change should fix it.
> I didn't tested it.
>
> diff --git a/src/config/cfg_rules.ini b/src/config/cfg_rules.ini
> index b6316be..0a654ff 100644
> --- a/src/config/cfg_rules.ini
> +++ b/src/config/cfg_rules.ini
> @@ -8,7 +8,7 @@ section = autofs
>  section = ssh
>  section = pac
>  section = ifp
> -section_re = ^secrets\(/users/\([0-9]\+\)\?\)\?$
> +section_re = ^secrets\(/users/[0-9]\+\)\?$
>  section_re = ^domain/.*$
>
>  [rule/allowed_sssd_options]
>

Yeah, the full patch should be:

diff --git a/src/config/cfg_rules.ini b/src/config/cfg_rules.ini
index b6316be..5a4394d 100644
--- a/src/config/cfg_rules.ini
+++ b/src/config/cfg_rules.ini
@@ -8,7 +8,7 @@ section = autofs
 section = ssh
 section = pac
 section = ifp
-section_re = ^secrets\(/users/\([0-9]\+\)\?\)\?$
+section_re = ^secrets\(/users/[0-9]\+\?\)\?$
 section_re = ^domain/.*$

 [rule/allowed_sssd_options]
@@ -212,7 +212,7 @@ option = user_attributes

 [rule/allowed_sec_options]
 validator = ini_allowed_options
-section_re = ^secrets\(/users/\([0-9]\+\)\?\)\?$
+section_re = ^secrets\(/users/[0-9]\+\?\)\?$

Just tested it on my side here.

>
>
>>> [secrets/users/$uid]
>>> description = temp
>&g

[SSSD] Re: [sssd PR#33][synchronized] SECRETS: Some small misc fixes + fixing #3168

[Fabiano Fidêncio]

On Mon, Oct 17, 2016 at 11:46 AM, Lukas Slebodnik  wrote:
> On (30/09/16 16:55), fidencio wrote:
>>   URL: https://github.com/SSSD/sssd/pull/33
>>Author: fidencio
>> Title: #33: SECRETS: Some small misc fixes + fixing #3168
>>Action: synchronized
>>
>>To pull the PR as Git branch:
>>git remote add ghsssd https://github.com/SSSD/sssd
>>git fetch ghsssd pull/33/head:pr33
>>git checkout pr33
>
> >From 06a0a81193d6bbe3a0932c8b584433f3cc13fa51 Mon Sep 17 00:00:00 2001
>>From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= 
>>Date: Sun, 25 Sep 2016 20:49:16 +0200
>>Subject: [PATCH 1/6] CONFIG: Add secrets responder to the allowed sections
>>MIME-Version: 1.0
>>Content-Type: text/plain; charset=UTF-8
>>Content-Transfer-Encoding: 8bit
>>
>>The regular expression used is quite specific for the two cases we
>>support:
>>- [secrets]
>>- [secrets/users/$uid]
>>
>>It could be done a bit more generic, but the way it's right now it can
>>easily catch errors like: [secrets/usrs/$uid] or [secrets/].
>>
>>Related:
>>https://fedorahosted.org/sssd/ticket/3207
>>
>>Signed-off-by: Fabiano Fidêncio 
>>---
>> src/config/cfg_rules.ini | 1 +
>> 1 file changed, 1 insertion(+)
>>
>>diff --git a/src/config/cfg_rules.ini b/src/config/cfg_rules.ini
>>index 01be0c6..023ceac 100644
>>--- a/src/config/cfg_rules.ini
>>+++ b/src/config/cfg_rules.ini
>>@@ -8,6 +8,7 @@ section = autofs
>> section = ssh
>> section = pac
>> section = ifp
>>+section_re = ^secrets\(/users/\([0-9]\+\)\?\)\?$
>> section_re = ^domain/.*$
>
> Is it expected that section the name "secrets/users/"
> is allowed.

I don't think so.
I'll answer your questions based on the my understanding of the
conversation I had with Jakub on the #sssd channel.
Jakub, Simo, please, feel free to jump in and correct me if I'm
mistaken in any point.

>
> Which of following section should be allowed?
>
> sh# cat /etc/sssd/conf.d/10_secrets.conf
> [secrets
> description = temp

Not allowed, but [secrets] is allowed.

>
> [secrets/users]
> description = temp
>

Shouldn't be allowed.

> [secrets/users/]
> description = temp
>

Shouldn't be allowed.

> [secrets/users/$uid]
> description = temp
>

Shouldn't be allowed.

> [secrets/users/0]
> description = temp
>

Should be allowed.

> [secrets/users/1]
> description = temp

Should be allowed.

>
> [secrets/users/1000]
> description = temp

Should be allowed.

>
> LS
> ___
> sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
> To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

Is some of these cases breaking to you?
If yes, please, let me know and I'll provide a follow up patch fixing the issue.

Best Regards,
--
Fabiano Fidêncio
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] Re: Monotonic clock for timed events

[Fabiano Fidêncio]

Victor,

On Mon, Oct 10, 2016 at 10:04 AM, Victor Tapia
 wrote:
> Hi list,
>
> I've faced a race condition when SSSD boots in a machine with a big
> clock drift. This is what I see:
>
> 1. SSSD starts before the network is up, queries the LDAP server without
> success and sets a retry timer (~60 secs)
> 2. NTP starts and corrects the clock, 1 hour back for example.
> 3. SSSD takes ~60 secs + the drift correction (1 hour) to retry the
> connection.
>
> In this particular scenario the credentials cache is disabled, so the
> wait time to login is noticeable. How feasible would it be to use a
> monotonic clock for this kind of timed events?

Are you running git master? This issue is supposed to be already
solved by 
https://github.com/SSSD/sssd/commit/b8ceaeb80cffb00c26390913ea959b77f7e848b9

Best Regards,
-- 
Fabiano Fidêncio
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] Re: Milestone names

[Fabiano Fidêncio]

On Fri, Oct 7, 2016 at 12:20 PM, Jakub Hrozek  wrote:
> Hi,
>
> for better or worse, our milestone and release planning is not great. We
> normally decide on what we want to work on for the next release and release
> new versions based on Fedora or RHEL releases (mostly because there is
> normally no other driver..if there are other projects or distributions
> who would like us to release on a different schedule, please just speak up!).
>
> The result is that our milestones don't really reflect the reality -- we
> have a huge milestone called "1.16" that is not really going to become
> 1.16, it's rather a set of tickets we'd like to work on "sometimes in
> the future". In addition, we have milestones that track bug fixing in
> released versions, currently 1.13.5 and 1.14.2 and the version currently
> under development, which is 1.15 at the moment.
>
> What I propose to make it clearer to outsiders what the expectations are
> is simply the following:
> - rename the current 1.16 bucket to "Future releases", maybe with a
>   more explanatory note, like "Future releases - not planned for a
>   particular date"
> - triage the milestones like 1.17 or 2.0, move tickets from there
>   either to deferred or just close them
> - rename the "Deferred" milestone to "Patches welcome".
>
> I think this would make it clearer a bit in terms of what the sssd
> upstream is working on now and what we are working on next..
>
> Comments? Thoughts?

ACK. I really like the idea, it does look more organized both for us
and for whoever is consuming SSSD.

Best Regards,
--
Fabiano Fidêncio
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] Re: Refactoring work for the next couple of weeks

[Fabiano Fidêncio]

On Mon, Oct 3, 2016 at 2:07 PM, Pavel Březina  wrote:
> On 10/03/2016 12:27 PM, Jakub Hrozek wrote:
>>
>> Hi,
>>
>> we might be able to do some refactoring in the next couple of weeks
>> prior to working on the next release.
>>
>> I wrote up a proposal here:
>>
>> https://fedorahosted.org/sssd/wiki/DesignDocs/OneFifteenCodeRefactoring
>> for your convenience, I copied the contents inline as well.
>
>
>> === Use the shared `cache_req` module for responder look-ups ===
>
>
> Already in progress, some responders are done. I'm currently working on NSS.
> This is a must have and doable in the scope.
>
>> === Refactor group lookups for better performance ===
>
>
> Ack.
>
>> === Refactor the sdap_id_ops.c module ===
>
>
> The sdap_id_ops calls should at least be moved into low level ldap code and
> turned to one-return-code-only api. We often forget to use it or we use it
> incorrectly.
>
>> === Provide a way to pass intermediate data between requests ===
>
>
> This can be probably done nice and clean with our new DP interface. Just
> fire a new request with new data.
>
>> === Upstream the PoC tests that utilize Samba AD for AD provider testing
>> ===
>
>
> For me, higher priority is to make CI test more useful when debugging.
>
> I added two new sections: SBUS Improvements and Failover refactoring.

How the SBUS Improvements are linked with the (possible) idea to make
SBUS a library and then we can link easier against it?
Asking because I had some issues trying to link against SBUS when
playing with IFP responder and then that's what you suggested me.

Best Regards,
--
Fabiano Fidêncio
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] Re: [sssd PR#32][comment] Requesting a pull to SSSD:master from fidencio:wip/#3138

[Fabiano Fidêncio]

On Wed, Sep 28, 2016 at 10:32 AM, jhrozek
 wrote:
>   URL: https://github.com/SSSD/sssd/pull/32
> Title: #32: Requesting a pull to SSSD:master from fidencio:wip/#3138
>
> jhrozek commented:
> """
> Hi, thank you for working on this. I think there are two "architectural" 
> questions we should answer.
>
> 1) Lukas raised the suggestion of using PreExec with `sssd --genconf` on 
> #sssd on IRC. Did you explore this or did you not like this solution because 
> of race-conditions?

I do believe it may work, but I would have to adapt sssd --genconf
logic, in order to avoid race-conditions.Shall I go for it?
I was thinking about waiting a little bit before doing any changes to
let people raise their questions/comments so I won't spend one week on
something that's not going to be used at all.

There are still the suggestions/questions raised by Pavel in the bug
report, where Dmitri jumped in as well, that just makes me think that
people are not in sync about the confdb itself and any kind of effort
on this area must have some agreement beforehand.
What do you think about this?

> 2) If we don't use `sssd --genconf`, I wonder if it would be actually better 
> to add the new socket to the monitor process rather than a new service 
> (sorry, I realize it was me who lead you down this path of adding a new 
> service...I only remembered this idea now once @simo5 mentioned it on IRC). 
> IMO this would have one advantage which is that the sssd process is already 
> permitted by SELinux to read sssd.conf and write to confdb. And long-term we 
> wanted to make the sssd process only initialize sssd and then exit since the 
> services would (in the ideal case of a modern Linux system) self-monitor 
> themselves.

I really don't know what's the best approach and I completely opened
for suggestions.

> """
>
> See the full comment at 
> https://github.com/SSSD/sssd/pull/32#issuecomment-250105213
>
> ___
> sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
> To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
>

Best Regards,
-- 
Fabiano Fidêncio
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] Re: [sssd PR#34][opened] cache_req: move from switch to plugins

[Fabiano Fidêncio]

Pavel,

On Tue, Sep 27, 2016 at 1:25 PM, pbrezina
 wrote:
>URL: https://github.com/SSSD/sssd/pull/34
> Author: pbrezina
>  Title: #34: cache_req: move from switch to plugins
> Action: opened
>
> PR body:
> """
> cache_req grown quite big from the original code and it turned out
> that using switch statements to branch code for different cases
> makes the code quite hard to read and further extend and any
> modification to the logic itself is difficult.
>
> This patch changes the switch statements to plugins with small
> functions and separates logic into multiple modules. This gives
> us better control over the code and improves readability and
> maintainability while keeping code duplication to minimum.
>
> At the moment only cache req user by name, id and upn is
> implemented as a proof of concept. If you like it, I will
> finish the rest, it won't take much time.
> """
>

+1 for the idea.

Best Regards,
--
Fabiano Fidêncio
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] Re: [RFC] Use GNULIB's compiler warning code

[Fabiano Fidêncio]

On Mon, Sep 26, 2016 at 9:58 PM, Lukas Slebodnik  wrote:
> On (26/09/16 21:47), Fabiano Fidêncio wrote:
>>On Mon, Sep 26, 2016 at 9:26 PM, Lukas Slebodnik  wrote:
>>> On (26/09/16 12:14), Fabiano Fidêncio wrote:
>>>>Jakub,
>>>>
>>>>On Mon, Sep 26, 2016 at 11:35 AM, Jakub Hrozek  wrote:
>>>>> On Mon, Sep 05, 2016 at 03:39:19PM +0200, Fabiano Fidêncio wrote:
>>>>>> On Thu, Aug 11, 2016 at 2:33 PM, Fabiano Fidêncio  
>>>>>> wrote:
>>>>>> > Howdy!
>>>>>> >
>>>>>> > I've suggested, a long time ago, that we could start making use of
>>>>>> > GNULIB's compiler warnings from 'manywarnings' module. This is
>>>>>> > basically what we have been doing in a few projects that I (used to
>>>>>> > and still) maintain (like spice-gtk and libosinfo, for instance).
>>>>>> >
>>>>>> > For now I didn't try to fix any of the warnings that we cannot cope
>>>>>> > with, mainly because I'm not sure whether you guys will agree on using
>>>>>> > it or not.
>>>>>> >
>>>>>> > Here is an experimental patch that works properly on Fedora 24. I
>>>>>> > still have to make some tests on RHEL-6, RHEL-7 and a few other
>>>>>> > systems (Debian, at least) in order to make sure that we won't break
>>>>>> > the build because of the patch.
>>>>>> >
>>>>>> > If you are okay with the change, I'll start going through the warnings
>>>>>> > that we cannot cope with and slowly start fixing them. Although, I
>>>>>> > have the feeling that fixing some of them would cause a lot of
>>>>>> > undesired changes, which will just bring troubles for ourselves when
>>>>>> > backporting fixes downstream (and here I'm talking about
>>>>>> > -Wformat-signedess, -Wsign-compare, -Wunused-parameter, ... for
>>>>>> > instance).
>>>>>> >
>>>>>> > I'm looking forward to hear some feedback!
>>>>>> >
>>>>>> > Best Regards,
>>>>>> > --
>>>>>> > Fabiano Fidêncio
>>>>>>
>>>>>> ping?
>>>>>
>>>>> I'm sorry this patch totally stalled.
>>>>>
>>>>> In general I'm all for adding more checks and let a machine help us
>>>>> write safer code. I'm not sure if adding all warnings on all platforms
>>>>> will ever be possible, though. For example, there was a warning in
>>>>> krb5_child because an old libkrb5 release used a "char *" parameter
>>>>> where a "const char *" was more appropriate and we said we'd never fix
>>>>> this because a newer version fixed the API (or used a different
>>>>> function? Not sure..)
>>>>>
>>>>> But I wonder how to move this patch forward the best. Are there any
>>>>> warnings that you think are more important to enable than others?
>>>>>
>>>>> What about enabling a single warning, then running SSSD build and
>>>>> creating a ticket with the warnings (running make 1>>>> good way to start..). Then we can see what needs fixing in SSSD for each
>>>>> of the additional warnings or if we decide this is not worth our time,
>>>>> we can either close or defer that ticket.
>>>>>
>>>>> These tickets might also be a good way for a newcomer to contribute some
>>>>> code to SSSD!
>>>>
>>>>So, general comments here.
>>>>
>>>>We have a list of the current warnings that we cannot deal with in
>>>>src/sssd-compile-warnings.m4. From this list we can, already, have a
>>>>good idea about what is worth to fix or not.
>>>>
>>>>About the idea to try to build SSSD ... well, it can be done.
>>>>I'll take some time later on and prepare a bug for each of the
>>>>warnings that we can't cope with and them we can discuss there whether
>>>>would make sense to fix those or not.
>>>>
>>> Here are few of my observation.
>>> The patch added 255 new configure time checks.
>>>
>>> The configure tooks 5 extra seconds in my case (everything in ram-disk).
>>

[SSSD] Re: [RFC] Use GNULIB's compiler warning code

[Fabiano Fidêncio]

On Mon, Sep 26, 2016 at 9:26 PM, Lukas Slebodnik  wrote:
> On (26/09/16 12:14), Fabiano Fidêncio wrote:
>>Jakub,
>>
>>On Mon, Sep 26, 2016 at 11:35 AM, Jakub Hrozek  wrote:
>>> On Mon, Sep 05, 2016 at 03:39:19PM +0200, Fabiano Fidêncio wrote:
>>>> On Thu, Aug 11, 2016 at 2:33 PM, Fabiano Fidêncio  
>>>> wrote:
>>>> > Howdy!
>>>> >
>>>> > I've suggested, a long time ago, that we could start making use of
>>>> > GNULIB's compiler warnings from 'manywarnings' module. This is
>>>> > basically what we have been doing in a few projects that I (used to
>>>> > and still) maintain (like spice-gtk and libosinfo, for instance).
>>>> >
>>>> > For now I didn't try to fix any of the warnings that we cannot cope
>>>> > with, mainly because I'm not sure whether you guys will agree on using
>>>> > it or not.
>>>> >
>>>> > Here is an experimental patch that works properly on Fedora 24. I
>>>> > still have to make some tests on RHEL-6, RHEL-7 and a few other
>>>> > systems (Debian, at least) in order to make sure that we won't break
>>>> > the build because of the patch.
>>>> >
>>>> > If you are okay with the change, I'll start going through the warnings
>>>> > that we cannot cope with and slowly start fixing them. Although, I
>>>> > have the feeling that fixing some of them would cause a lot of
>>>> > undesired changes, which will just bring troubles for ourselves when
>>>> > backporting fixes downstream (and here I'm talking about
>>>> > -Wformat-signedess, -Wsign-compare, -Wunused-parameter, ... for
>>>> > instance).
>>>> >
>>>> > I'm looking forward to hear some feedback!
>>>> >
>>>> > Best Regards,
>>>> > --
>>>> > Fabiano Fidêncio
>>>>
>>>> ping?
>>>
>>> I'm sorry this patch totally stalled.
>>>
>>> In general I'm all for adding more checks and let a machine help us
>>> write safer code. I'm not sure if adding all warnings on all platforms
>>> will ever be possible, though. For example, there was a warning in
>>> krb5_child because an old libkrb5 release used a "char *" parameter
>>> where a "const char *" was more appropriate and we said we'd never fix
>>> this because a newer version fixed the API (or used a different
>>> function? Not sure..)
>>>
>>> But I wonder how to move this patch forward the best. Are there any
>>> warnings that you think are more important to enable than others?
>>>
>>> What about enabling a single warning, then running SSSD build and
>>> creating a ticket with the warnings (running make 1>> good way to start..). Then we can see what needs fixing in SSSD for each
>>> of the additional warnings or if we decide this is not worth our time,
>>> we can either close or defer that ticket.
>>>
>>> These tickets might also be a good way for a newcomer to contribute some
>>> code to SSSD!
>>
>>So, general comments here.
>>
>>We have a list of the current warnings that we cannot deal with in
>>src/sssd-compile-warnings.m4. From this list we can, already, have a
>>good idea about what is worth to fix or not.
>>
>>About the idea to try to build SSSD ... well, it can be done.
>>I'll take some time later on and prepare a bug for each of the
>>warnings that we can't cope with and them we can discuss there whether
>>would make sense to fix those or not.
>>
> Here are few of my observation.
> The patch added 255 new configure time checks.
>
> The configure tooks 5 extra seconds in my case (everything in ram-disk).
> But in our CI machines it added +20 seconds in average.
> The configure scriot is execute 5 times in our CI-script
> (debug build, distcheck, intgcheck, mock, coverage build)
>
> In Summary, the patch would add unnecessary 100 seconds.
>
> I would prefer if all compile time warnings were part of AM_CFLAGS
> and we shoul add compile time checks only for warnings which are supported by
> new compilers or are supported only by gcc or only by clang.

I'm completely fine dropping the patch.

Best Regards,
--
Fabiano Fidêncio
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] Re: [RFC] Use GNULIB's compiler warning code

[Fabiano Fidêncio]

Jakub,

On Mon, Sep 26, 2016 at 11:35 AM, Jakub Hrozek  wrote:
> On Mon, Sep 05, 2016 at 03:39:19PM +0200, Fabiano Fidêncio wrote:
>> On Thu, Aug 11, 2016 at 2:33 PM, Fabiano Fidêncio  
>> wrote:
>> > Howdy!
>> >
>> > I've suggested, a long time ago, that we could start making use of
>> > GNULIB's compiler warnings from 'manywarnings' module. This is
>> > basically what we have been doing in a few projects that I (used to
>> > and still) maintain (like spice-gtk and libosinfo, for instance).
>> >
>> > For now I didn't try to fix any of the warnings that we cannot cope
>> > with, mainly because I'm not sure whether you guys will agree on using
>> > it or not.
>> >
>> > Here is an experimental patch that works properly on Fedora 24. I
>> > still have to make some tests on RHEL-6, RHEL-7 and a few other
>> > systems (Debian, at least) in order to make sure that we won't break
>> > the build because of the patch.
>> >
>> > If you are okay with the change, I'll start going through the warnings
>> > that we cannot cope with and slowly start fixing them. Although, I
>> > have the feeling that fixing some of them would cause a lot of
>> > undesired changes, which will just bring troubles for ourselves when
>> > backporting fixes downstream (and here I'm talking about
>> > -Wformat-signedess, -Wsign-compare, -Wunused-parameter, ... for
>> > instance).
>> >
>> > I'm looking forward to hear some feedback!
>> >
>> > Best Regards,
>> > --
>> > Fabiano Fidêncio
>>
>> ping?
>
> I'm sorry this patch totally stalled.
>
> In general I'm all for adding more checks and let a machine help us
> write safer code. I'm not sure if adding all warnings on all platforms
> will ever be possible, though. For example, there was a warning in
> krb5_child because an old libkrb5 release used a "char *" parameter
> where a "const char *" was more appropriate and we said we'd never fix
> this because a newer version fixed the API (or used a different
> function? Not sure..)
>
> But I wonder how to move this patch forward the best. Are there any
> warnings that you think are more important to enable than others?
>
> What about enabling a single warning, then running SSSD build and
> creating a ticket with the warnings (running make 1 good way to start..). Then we can see what needs fixing in SSSD for each
> of the additional warnings or if we decide this is not worth our time,
> we can either close or defer that ticket.
>
> These tickets might also be a good way for a newcomer to contribute some
> code to SSSD!

So, general comments here.

We have a list of the current warnings that we cannot deal with in
src/sssd-compile-warnings.m4. From this list we can, already, have a
good idea about what is worth to fix or not.

About the idea to try to build SSSD ... well, it can be done.
I'll take some time later on and prepare a bug for each of the
warnings that we can't cope with and them we can discuss there whether
would make sense to fix those or not.

Best Regards,
--
Fabiano Fidêncio
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] Re: [PATCH] BUILD: intgcheck need to fail if pytest fails

[Fabiano Fidêncio]

On Mon, Sep 26, 2016 at 10:23 AM, Lukas Slebodnik  wrote:
> ehlo,
>
> make intgcheck silently passed in case of failure/error in pytest.
> This regression was introduced by spliting make intgcheck into
> sub-targets. I am soory I had to catch that as part of review.
>
> Simple patch is attached.

Looks good to me!
Acked-by: Fabiano Fidêncio 

Best Regards,
--
Fabiano Fidêncio
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] Re: fedorahosted.org sunset

[Fabiano Fidêncio]

Howdy!

On Fri, Sep 16, 2016 at 11:09 AM, Jakub Hrozek  wrote:
> Hi,
>
> fedorahosted.org is being decomissioned:
> 
> https://lists.fedoraproject.org/archives/list/annou...@lists.fedoraproject.org/thread/RLL3LFUPLYMAUKGZ5B3O64XKJXBT24KZ/
> so we need to find a new home for SSSD..
>
> I wanted to ask:
> 1) anyone from the core development team who is interested in
>finding a new home to raise a hand, we need someone to "own" this
>work
> 2) anyone from outside or inside the team who has an opinion to
>voice it :) so far we haven't even thought about the requirements
>in too much detail, though..
>
> In the meantime I just wrote up what fields we use from Trac and
> therefore what info we need to keep in the new system:
> https://fedorahosted.org/sssd/wiki/ticket_fields
> ___
> sssd-devel mailing list
> sssd-devel@lists.fedorahosted.org
> https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

So, I've seen that both pagure and github have pretty much the same
"issues" system and, AFAIR, it doesn't fit as well.
Considering Pavel's suggestion (something self-hosted), why not
Phabricator? Phabricator's trac seems to fit pretty much what we need.

Best Regards,
--
Fabiano Fidêncio
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] Re: [PATCH] PROXY: Adding proxy_max_children option

[Fabiano Fidêncio]

On Mon, Sep 12, 2016 at 9:40 AM, Petr Cech  wrote:
> Bump.
>
>
> --
> Petr^4 Čech
> ___
> sssd-devel mailing list
> sssd-devel@lists.fedorahosted.org
> https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

Patch looks good and all the requested changed were done.
I haven't done any tests with the patch, but the changes themselves
look good to me.

Best Regards,
--
Fabiano Fidêncio
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] Re: [sssd PR#10] UTIL: Remove support to libini older than 1.0.0 (comment)

[Fabiano Fidêncio]

On Wed, Sep 7, 2016 at 9:03 AM, Lukas Slebodnik  wrote:
> On (07/09/16 08:46), Fabiano Fidêncio wrote:
>>On Wed, Sep 7, 2016 at 8:34 AM, Lukas Slebodnik  wrote:
>>> On (06/09/16 21:38), Fabiano Fidêncio wrote:
>>>>On Tue, Sep 6, 2016 at 8:49 PM, lslebodn
>>>> wrote:
>>>>> lslebodn commented on a pull request
>>>>>
>>>>> """
>>>>> IMHO, it might be better to close this PR.
>>>>> If we decide to dor support for libini_config < 1.1 or 1.2
>>>>> then it will be a different patch anyway. @see my previous comment
>>>>> """
>>>>
>>>>Please, take a look on Jakub's comment and see what he proposed.
>>>>
>>> Do you mean:
>>>>
>>>>jhrozek commented on a pull request
>>>>
>>>>"""
>>>>> - Ubuntu 12.04 LTS and older
>>>>
>>>>12.04 is often used (for example travis-ci still offers only this
>>>>distro) and ends its life in 2017.
>>>>
>>>>On one hand, it's unlikely that users of LTS distributions will run
>>>>master, they will probably only run sssd-1-13 or some PPAs, on the other
>>>>hand, I don't see too much value except for cleaner code.
>>>>
>>>>So all in all I suggest we nack this patchset for now and push it when
>>>>Ubuntu 12.04 goes EOL.
>>>>
>>>>Does that sound like a reasonable compromise?
>>>>
>>>>"""
>>> If we decide to drop support for older versions of libini_config
>>> then we should also remove unnecessary wrappers in src/util/sss_ini.c.
>>> and replace sss_ini* functions with direct usage of ini_*.
>>>
>>> Therefore this patchset will need to be changed.
>>> IMHO, it will be much simpler to create new patch-set then.
>>>
>>>
>>>>Anyways, I'm fine with closing this PR and opening a new one when we
>>>>can drop support to libini_config < 1.3.
>>>>
>>>>Just for the record, from this whole discussion I could see that
>>>>dropping support to augeas in order to use libini is something that
>>>>shouldn't and is not going to happen any time soon
>>> I do not understand how is this patch-set related to replacing augeas
>>> with libini_config.
>>
>>Well, this patch set is _not_ related to replacing augeas with
>>libini_config, we just will fall under the same issue in the future
>>when doing that.
>>If we introduce a new code that will deal with configuration files the
>>least I would expect is that old systems could have some benefit from it
>>in the same way they can have benefit from the current code.
> Maybe you did not get my point and therefore you get wrong expectations.
> I do not expect that old systems will benefit from new features.
> e.g.
> Currently rhel6 cannot:
> * validate configuration file because it has old version of libini_config
> * be integrated with "CIFS Client"[1] because it does not have necessary
>   dependencies
> ...
>
>>So, if I'm the one writing or reviewing the code with this discussion in mind
>>I'd opt for something that is present in the major distros and for
>>sure libini > 1.3 is not. So, why not use augeas instead of it? :-)
>>
> Augeas is optional dependency libini_config-1.2 is optional dependency.
> I cannot see a conflict for replacing it :-)
> But it also does not make a sense
> because we should drop manipulation with sssd.conf

Okay, okay,

>
>>>
>>> a) augeas is an optional dependency
>>>if BUILD_IFP
>>>if BUILD_CONFIG_LIB
>>>pkglib_LTLIBRARIES += libsss_config.la
>>> b) replacing augeas with libini_config requires minimal version 1.2
>>>which is already optionally used in master.
>>> c) IIRC discussion from devel meeting. Some developers prefer
>>>to drop feature for manipulation of sssd.conf and write from
>>>scratch if there will be new requirements. Current version is very
>>>limited.
>>>
>>>>as well and for the
>>>>very same reasons that this patch wasn't accepted. We could, in the
>>>>best (or worst?) case scenario support/depend on both due to
>>>>compatibility with elder systems, but I can't see any advantage on
>>>>that.
>>>>
>>> I do not suggest to support all features in old systems.
>>> But I would like to support at least minimal feature set (ldap+krb5).
>>> And parsing configuration file is required for minimal feature set :-)
>>>
>>
>>Sure, sure.
>>
>>We wasted a lot of time on this patch set that could have been used
>>reviewing more useful patches on the queue.
>>Please, close the PR if you want to.
>>
> I'm sorry if you think we wasted a lot of time.

There's no reason to be sorry for.

> I did not want to close PR without proper explanation why I would
> prefer to keep this optional code in sssd. It would be impolite
> to close PR without general agreement.

I don't think we are going to agree here. So, please, just close the PR.

Best Regards,
--
Fabiano Fidêncio
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] Re: [sssd PR#10] UTIL: Remove support to libini older than 1.0.0 (comment)

[Fabiano Fidêncio]

On Wed, Sep 7, 2016 at 8:34 AM, Lukas Slebodnik  wrote:
> On (06/09/16 21:38), Fabiano Fidêncio wrote:
>>On Tue, Sep 6, 2016 at 8:49 PM, lslebodn
>> wrote:
>>> lslebodn commented on a pull request
>>>
>>> """
>>> IMHO, it might be better to close this PR.
>>> If we decide to dor support for libini_config < 1.1 or 1.2
>>> then it will be a different patch anyway. @see my previous comment
>>> """
>>
>>Please, take a look on Jakub's comment and see what he proposed.
>>
> Do you mean:
>>
>>jhrozek commented on a pull request
>>
>>"""
>>> - Ubuntu 12.04 LTS and older
>>
>>12.04 is often used (for example travis-ci still offers only this
>>distro) and ends its life in 2017.
>>
>>On one hand, it's unlikely that users of LTS distributions will run
>>master, they will probably only run sssd-1-13 or some PPAs, on the other
>>hand, I don't see too much value except for cleaner code.
>>
>>So all in all I suggest we nack this patchset for now and push it when
>>Ubuntu 12.04 goes EOL.
>>
>>Does that sound like a reasonable compromise?
>>
>>"""
> If we decide to drop support for older versions of libini_config
> then we should also remove unnecessary wrappers in src/util/sss_ini.c.
> and replace sss_ini* functions with direct usage of ini_*.
>
> Therefore this patchset will need to be changed.
> IMHO, it will be much simpler to create new patch-set then.
>
>
>>Anyways, I'm fine with closing this PR and opening a new one when we
>>can drop support to libini_config < 1.3.
>>
>>Just for the record, from this whole discussion I could see that
>>dropping support to augeas in order to use libini is something that
>>shouldn't and is not going to happen any time soon
> I do not understand how is this patch-set related to replacing augeas
> with libini_config.

Well, this patch set is _not_ related to replacing augeas with
libini_config, we just will fall under the same issue in the future
when doing that.
If we introduce a new code that will deal with configuration files the
least I would expect is that old systems could have some benefit from
it in the same way they can have benefit from the current code. So, if
I'm the one writing or reviewing the code with this discussion in mind
I'd opt for something that is present in the major distros and for
sure libini > 1.3 is not. So, why not use augeas instead of it? :-)

>
> a) augeas is an optional dependency
>if BUILD_IFP
>if BUILD_CONFIG_LIB
>pkglib_LTLIBRARIES += libsss_config.la
> b) replacing augeas with libini_config requires minimal version 1.2
>which is already optionally used in master.
> c) IIRC discussion from devel meeting. Some developers prefer
>to drop feature for manipulation of sssd.conf and write from
>scratch if there will be new requirements. Current version is very
>limited.
>
>>as well and for the
>>very same reasons that this patch wasn't accepted. We could, in the
>>best (or worst?) case scenario support/depend on both due to
>>compatibility with elder systems, but I can't see any advantage on
>>that.
>>
> I do not suggest to support all features in old systems.
> But I would like to support at least minimal feature set (ldap+krb5).
> And parsing configuration file is required for minimal feature set :-)
>

Sure, sure.

We wasted a lot of time on this patch set that could have been used
reviewing more useful patches on the queue.
Please, close the PR if you want to.

Best Regards,
--
Fabiano Fidêncio
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] Re: [sssd PR#10] UTIL: Remove support to libini older than 1.0.0 (comment)

[Fabiano Fidêncio]

On Tue, Sep 6, 2016 at 8:49 PM, lslebodn
 wrote:
> lslebodn commented on a pull request
>
> """
> IMHO, it might be better to close this PR.
> If we decide to dor support for libini_config < 1.1 or 1.2
> then it will be a different patch anyway. @see my previous comment
> """

Please, take a look on Jakub's comment and see what he proposed.

Anyways, I'm fine with closing this PR and opening a new one when we
can drop support to libini_config < 1.3.

Just for the record, from this whole discussion I could see that
dropping support to augeas in order to use libini is something that
shouldn't and is not going to happen any time soon as well and for the
very same reasons that this patch wasn't accepted. We could, in the
best (or worst?) case scenario support/depend on both due to
compatibility with elder systems, but I can't see any advantage on
that.

Best Regards,
-- 
Fabiano Fidêncio
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] Re: [sssd PR#10] UTIL: Remove support to libini older than 1.0.0 (comment)

[Fabiano Fidêncio]

On Tue, Sep 6, 2016 at 2:30 PM, jhrozek
 wrote:
> jhrozek commented on a pull request
>
> """
> On Tue, Sep 06, 2016 at 05:16:00AM -0700, fidencio wrote:
>> The distributions that would break with this patch are:
>> - RHEL/CentOS 5 and older
>
> I don't think we care about RHEL-5 with master and I'm not sure sssd
> master even builds there.
>
>> - Debian Wheezy (from 2013) and older
>
> OK, this one I think we care about for the basic functionality:
> 7.0 Wheezy May 4th 2013 April 26th 2016 (full) / May 2018 (LTS)
> But LTS means bug fixes and for those I think sssd-1-13 would be OK.
>
>> - Ubuntu 12.04 LTS and older
>
> 12.04 is often used (for example travis-ci still offers only this
> distro) and ends its life in 2017.
>
> On one hand, it's unlikely that users of LTS distributions will run
> master, they will probably only run sssd-1-13 or some PPAs, on the other
> hand, I don't see too much value except for cleaner code.
>
> So all in all I suggest we nack this patchset for now and push it when
> Ubuntu 12.04 goes EOL.
>
> Does that sound like a reasonable compromise?

Works for me.

>
> """
>
> See the full comment at 
> https://github.com/SSSD/sssd/pull/10#issuecomment-244935329
>
> ___
> sssd-devel mailing list
> sssd-devel@lists.fedorahosted.org
> https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org
>
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] Re: [PATCH] PROXY: Adding proxy_max_children option

[Fabiano Fidêncio]

Petr,

On Mon, Sep 5, 2016 at 3:43 PM, Petr Cech  wrote:
> On 09/05/2016 09:57 AM, Fabiano Fidêncio wrote:
>>
>> Petr,
>>
>> I see you have updated the OPT_MAX_CHILDREN_DEFAULT to 10 instead of
>> 50. However, you haven't update the value on sssd.conf.5.xml:
>>
>> diff --git a/src/man/sssd.conf.5.xml b/src/man/sssd.conf.5.xml
>> index
>> ae291e0fc8f2f9afabcdf32f18a5ec12252f..1bf3e799047d9c722487be8657bbee5cfd479cdd
>> 100644
>> --- a/src/man/sssd.conf.5.xml
>> +++ b/src/man/sssd.conf.5.xml
>> @@ -2464,6 +2464,18 @@ subdomain_inherit = ldap_purge_cache_timeout
>>  
>>  
>>
>> +
>> +proxy_max_children (integer)
>> +
>> +
>> +The number of preforked proxy children.
>> +
>> +
>> +Default: 50
>>here: ^^^
>>
>> +
>> +
>> +
>> +
>>  
>>  
>>
>> Apart from this minor the patch seems to be following everything that
>> was requested during the review process. However, I'm not comfortable
>> with the text used to describe the new option, so adding there a bit
>> more information would be super. Like, I don't know what's the
>> influence of the preforked proxy children to the rest of the code
>> (probably because I'm a newbie here ;-)), but would be nice to have it
>> clear in the documentation (for newbies like myself ;-)).
>>
>> Best Regards,
>> --
>> Fabiano Fidêncio
>
>
> Hi Fabiano,
>
> thanks for code review. I fixed the default value in man page and I
> reformulated description. Is it better?
>
> Regards
>
> --
> Petr^4 Čech
>
> ___
> sssd-devel mailing list
> sssd-devel@lists.fedorahosted.org
> https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org
>

Looking at the changes in the manual ...

+In busy environments it is possible sssd
runs out of
+available child slots and starts queuing requests
+in proxy mode. This option introduces the number of
+preforked proxy children.

I personally go for something like:

"This option introduces the number of pre-forked proxy children. It's
useful for busy environments* where sssd may run out of available
child slots, which would cause some issues due to the requests being
queued".

*: Not sure whether busy environments is something clear for everyone ...

IMO the patch is good to go as soon as we have this part done/reviewed
by a native speaker.
Maybe Justin can help us here?

Best Regards,
--
Fabiano Fidêncio
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] Re: [RFC] Use GNULIB's compiler warning code

[Fabiano Fidêncio]

On Thu, Aug 11, 2016 at 2:33 PM, Fabiano Fidêncio  wrote:
> Howdy!
>
> I've suggested, a long time ago, that we could start making use of
> GNULIB's compiler warnings from 'manywarnings' module. This is
> basically what we have been doing in a few projects that I (used to
> and still) maintain (like spice-gtk and libosinfo, for instance).
>
> For now I didn't try to fix any of the warnings that we cannot cope
> with, mainly because I'm not sure whether you guys will agree on using
> it or not.
>
> Here is an experimental patch that works properly on Fedora 24. I
> still have to make some tests on RHEL-6, RHEL-7 and a few other
> systems (Debian, at least) in order to make sure that we won't break
> the build because of the patch.
>
> If you are okay with the change, I'll start going through the warnings
> that we cannot cope with and slowly start fixing them. Although, I
> have the feeling that fixing some of them would cause a lot of
> undesired changes, which will just bring troubles for ourselves when
> backporting fixes downstream (and here I'm talking about
> -Wformat-signedess, -Wsign-compare, -Wunused-parameter, ... for
> instance).
>
> I'm looking forward to hear some feedback!
>
> Best Regards,
> --
> Fabiano Fidêncio

ping?
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] Re: [PATCH SET] SYSDB: Adding message to inform about cache

[Fabiano Fidêncio]

On Mon, Sep 5, 2016 at 11:59 AM, Fabiano Fidêncio  wrote:
> Petr,
>
> I went through your patches and in general they look good to me.
> However, I haven't done any tests yet with your patches (and I'll do
> it after lunch).

I've done some tests and I've been able to see the ldif changes in the
domain log. So, I assume it's working.
For sure it's a good improvement! Would be worth to link some
documentation about ldiff as it may be confusing for someone who is
not used to it.

I'll wait for a new version of the patches and go through them again.

I really would like to have someone's else opinion on this series.

>
> Please, below you can see a few comments. Feel completely free to
> ignore the first one if you feel like doing it, it's just a minor :-)
> For the other comments, I'd like to understand a few changes you have done.
>
>
> Patch 0001: SYSDB: Adding message to inform which cache is used
>
> About the following part of the patch:
> +static const char *get_attr_storage(int state_mask)
> +{
> +const char *storage = "";
> +
> +if (state_mask == SSS_SYSDB_BOTH_CACHE ) {
> +storage = "cache, ts_cache";
> +} else if (state_mask == SSS_SYSDB_TS_CACHE) {
> +storage = "ts_cache";
> +} else if (state_mask == SSS_SYSDB_CACHE) {
> +storage = "cache";
> +}
> +
> +return storage;
> +}
>
> I personally don't like this kind of comparison done with flags. I'd
> go for something like: if ((state_mask & SSS_SYSDB_BOTH_CACHE) != 0)
> ...
> But this is a really minor and feel free to ignore it.
>
>
> Patch 0002: SYSDB: Adding message about reason why cache changed
>
> LGTM
>
>
> Patch 0003: SYSDB: Adding wrappers for ldb_* operations
>
> About the following parts of the patch:
>
> On src/db/sysdb_ldb_wrapper.c
>
> +#define ERR_FN_ENOMEM (-1 * ENOMEM)
> +#define ERR_FN_ENOENT (-1 * ENOENT)
>
> Why? I failed to understand why you're doing this here.
>
> +if (print_ctx == NULL) {
> +return -1;
> +return ERR_FN_ENOMEM;
> +}
>
> I guess the return -1 is a leftover :-)
>
> +if (print_ctx->ldif == NULL) {
> +return -2;
> +return ERR_FN_ENOENT;
> +}
>
> I guess the return -2 is also a leftover :-)
>
> +if (ret < 0) {
> +DEBUG(SSSDBG_MINOR_FAILURE, "ldb_ldif_write() failed with 
> [%d][%s].\n",
> +-1 * ret, sss_strerror(-1 * ret));
> +goto done;
> +}
>
> And here again this dance multiplying by -1 that I don't understand
> the reason :-\
>
> +done:
> +if (ldb_print_ctx != NULL && ldb_print_ctx->ldif != NULL) {
> +talloc_free(ldb_print_ctx->ldif);
> +}
> +talloc_free(ldb_print_ctx);
>
> AFAIU talloc_free can gracefully handle NULL. Considering that's the
> case I'd just check for (if ldb_print_ctx != NULL)
> talloc_free(ldb_print_ctx->ldif);
> Considering it doesn't, we may have some issues on trying to free
> (ldb_print_ctx)
>
> On src/db/sysdb_ldb_wrapper.h:
>
> +int sss_ldb_rename(struct ldb_context *ldb,
> +   struct ldb_dn * olddn,
> +   struct ldb_dn *newdn);
>
> Just a really minor codying style change here, remove the extra space
> between * and olddn: struct ldb_dn * olddn,  ->  struct ldb_dn *olddn,
>
>
> Patch0004: SYSDB: ldb_add --> sss_ldb_add in sysdb
> Patch0005: SYSDB: ldb_delete --> sss_ldb_delete in sysdb
> Patch0006: SYSDB: ldb_modify --> sss_ldb_modify in sysdb
> Patch0007: SYSDB: ldb_rename --> sss_ldb_rename in sysdb
>
> LGTM
>
>
> Best Regards,
> --
> Fabiano Fidêncio
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] Re: [PATCH SET] SYSDB: Adding message to inform about cache

[Fabiano Fidêncio]

Petr,

I went through your patches and in general they look good to me.
However, I haven't done any tests yet with your patches (and I'll do
it after lunch).

Please, below you can see a few comments. Feel completely free to
ignore the first one if you feel like doing it, it's just a minor :-)
For the other comments, I'd like to understand a few changes you have done.


Patch 0001: SYSDB: Adding message to inform which cache is used

About the following part of the patch:
+static const char *get_attr_storage(int state_mask)
+{
+const char *storage = "";
+
+if (state_mask == SSS_SYSDB_BOTH_CACHE ) {
+storage = "cache, ts_cache";
+} else if (state_mask == SSS_SYSDB_TS_CACHE) {
+storage = "ts_cache";
+} else if (state_mask == SSS_SYSDB_CACHE) {
+storage = "cache";
+}
+
+return storage;
+}

I personally don't like this kind of comparison done with flags. I'd
go for something like: if ((state_mask & SSS_SYSDB_BOTH_CACHE) != 0)
...
But this is a really minor and feel free to ignore it.


Patch 0002: SYSDB: Adding message about reason why cache changed

LGTM


Patch 0003: SYSDB: Adding wrappers for ldb_* operations

About the following parts of the patch:

On src/db/sysdb_ldb_wrapper.c

+#define ERR_FN_ENOMEM (-1 * ENOMEM)
+#define ERR_FN_ENOENT (-1 * ENOENT)

Why? I failed to understand why you're doing this here.

+if (print_ctx == NULL) {
+return -1;
+return ERR_FN_ENOMEM;
+}

I guess the return -1 is a leftover :-)

+if (print_ctx->ldif == NULL) {
+return -2;
+return ERR_FN_ENOENT;
+}

I guess the return -2 is also a leftover :-)

+if (ret < 0) {
+DEBUG(SSSDBG_MINOR_FAILURE, "ldb_ldif_write() failed with [%d][%s].\n",
+-1 * ret, sss_strerror(-1 * ret));
+goto done;
+}

And here again this dance multiplying by -1 that I don't understand
the reason :-\

+done:
+if (ldb_print_ctx != NULL && ldb_print_ctx->ldif != NULL) {
+talloc_free(ldb_print_ctx->ldif);
+}
+talloc_free(ldb_print_ctx);

AFAIU talloc_free can gracefully handle NULL. Considering that's the
case I'd just check for (if ldb_print_ctx != NULL)
talloc_free(ldb_print_ctx->ldif);
Considering it doesn't, we may have some issues on trying to free
(ldb_print_ctx)

On src/db/sysdb_ldb_wrapper.h:

+int sss_ldb_rename(struct ldb_context *ldb,
+   struct ldb_dn * olddn,
+   struct ldb_dn *newdn);

Just a really minor codying style change here, remove the extra space
between * and olddn: struct ldb_dn * olddn,  ->  struct ldb_dn *olddn,


Patch0004: SYSDB: ldb_add --> sss_ldb_add in sysdb
Patch0005: SYSDB: ldb_delete --> sss_ldb_delete in sysdb
Patch0006: SYSDB: ldb_modify --> sss_ldb_modify in sysdb
Patch0007: SYSDB: ldb_rename --> sss_ldb_rename in sysdb

LGTM


Best Regards,
--
Fabiano Fidêncio
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] Re: [PATCH] PROXY: Adding proxy_max_children option

[Fabiano Fidêncio]

Petr,

I see you have updated the OPT_MAX_CHILDREN_DEFAULT to 10 instead of
50. However, you haven't update the value on sssd.conf.5.xml:

diff --git a/src/man/sssd.conf.5.xml b/src/man/sssd.conf.5.xml
index 
ae291e0fc8f2f9afabcdf32f18a5ec12252f..1bf3e799047d9c722487be8657bbee5cfd479cdd
100644
--- a/src/man/sssd.conf.5.xml
+++ b/src/man/sssd.conf.5.xml
@@ -2464,6 +2464,18 @@ subdomain_inherit = ldap_purge_cache_timeout
 
 

+
+proxy_max_children (integer)
+
+
+The number of preforked proxy children.
+
+
+Default: 50
   here: ^^^

+
+
+
+
 
 

Apart from this minor the patch seems to be following everything that
was requested during the review process. However, I'm not comfortable
with the text used to describe the new option, so adding there a bit
more information would be super. Like, I don't know what's the
influence of the preforked proxy children to the rest of the code
(probably because I'm a newbie here ;-)), but would be nice to have it
clear in the documentation (for newbies like myself ;-)).

Best Regards,
--
Fabiano Fidêncio
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] Re: [PATCH SYSDB: Fix handling of version in sysdb_cache_connect_helper

[Fabiano Fidêncio]

On Thu, Sep 1, 2016 at 6:02 PM, Lukas Slebodnik  wrote:
> On (01/09/16 17:22), Fabiano Fidêncio wrote:
>>On Thu, Sep 1, 2016 at 5:12 PM, Lukas Slebodnik  wrote:
>>> On (30/08/16 17:07), Lukas Slebodnik wrote:
>>>>On (30/08/16 16:59), Fabiano Fidêncio wrote:
>>>>>Lukaš,
>>>>>
>>>>>On Tue, Aug 30, 2016 at 4:54 PM, Lukas Slebodnik  
>>>>>wrote:
>>>>>> ehlo,
>>>>>>
>>>>>> Clang static analyzer assume that ldb_search can found
>>>>>> 0 entries in the tree "cn=sysdb". Thenvariable version
>>>>>> could be used uninitialized.
>>>>>>
>>>>>> We cannot get to such state in sssd but we already handle
>>>>>> a case for more then one entry.
>>>>>
>>>>>I don't think this is the right approach as res->count == 0 seems to
>>>>>be a valid case for a newly created database (please, correct me if
>>>>>I'm wrong).
>>>>>
>>>>Agree
>>>>
>>>>I should have tried to run unit test before sending a patch
>>>>
>>> I looked deeper to the clang report
>>> And there are wrong assumption that output variable
>>> "version" is not initialized if function sysdb_cache_connect
>>> returns ERR_SYSDB_VERSION_TOO_OLD or ERR_SYSDB_VERSION_TOO_NEW
>>>
>>> The reality is that output variable "version" is initialized
>>> especially for these two case.
>>>
>>> It is a false positive but we might suppress the warning
>>> with initializing variable to NULL.
>>
>>Indeed!
>>Are you planning to submit this one-liner for review as well?
>>
> I can.

Please, just fix a typo before pushing:
(...) to NULL suppress" -> "(...) to NULL suppresses".

Acked-by: Fabiano Fid6encio 

Best Regards,
-- 
Fabiano Fidêncio
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] Re: [PATCH SYSDB: Fix handling of version in sysdb_cache_connect_helper

[Fabiano Fidêncio]

On Thu, Sep 1, 2016 at 5:12 PM, Lukas Slebodnik  wrote:
> On (30/08/16 17:07), Lukas Slebodnik wrote:
>>On (30/08/16 16:59), Fabiano Fidêncio wrote:
>>>Lukaš,
>>>
>>>On Tue, Aug 30, 2016 at 4:54 PM, Lukas Slebodnik  wrote:
>>>> ehlo,
>>>>
>>>> Clang static analyzer assume that ldb_search can found
>>>> 0 entries in the tree "cn=sysdb". Thenvariable version
>>>> could be used uninitialized.
>>>>
>>>> We cannot get to such state in sssd but we already handle
>>>> a case for more then one entry.
>>>
>>>I don't think this is the right approach as res->count == 0 seems to
>>>be a valid case for a newly created database (please, correct me if
>>>I'm wrong).
>>>
>>Agree
>>
>>I should have tried to run unit test before sending a patch
>>
> I looked deeper to the clang report
> And there are wrong assumption that output variable
> "version" is not initialized if function sysdb_cache_connect
> returns ERR_SYSDB_VERSION_TOO_OLD or ERR_SYSDB_VERSION_TOO_NEW
>
> The reality is that output variable "version" is initialized
> especially for these two case.
>
> It is a false positive but we might suppress the warning
> with initializing variable to NULL.

Indeed!
Are you planning to submit this one-liner for review as well?

Best Regards,
-- 
Fabiano Fidêncio
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] Re: [PATCH] DEBUG: Apend line feed to messages from libsemanage

[Fabiano Fidêncio]

On Thu, Sep 1, 2016 at 8:16 AM, Lukas Slebodnik  wrote:
> ehlo,
>
> @see
> (Wed Aug 31 14:12:18 2016) [[sssd[selinux_child[7088 [libsemanage] 
> (0x0020): could not query record value(Wed Aug 31 14:12:18 2016) 
> [[sssd[selinux_child[7088 [get_seuser] (0x0020): Cannot query for dennis
> (Wed Aug 31 14:12:18 2016) [[sssd[selinux_child[7088 
> [seuser_needs_update] (0x2000): get_seuser: ret: 5 seuser: unknown mls: 
> unknown
> (Wed Aug 31 14:12:30 2016) [[sssd[selinux_child[7088 [libsemanage] 
> (0x0020): setfiles returned error code 1.(Wed Aug 31 14:12:33 2016) 
> [[sssd[selinux_child[7088 [libsemanage] (0x0020): setfiles returned error 
> code 1.(Wed Aug 31 14:12:33 2016) [[sssd[selinux_child[7088 [set_seuser] 
> (0x0020): Cannot commit SELinux transaction
> (Wed Aug 31 14:12:33 2016) [[sssd[selinux_child[7088 [main] (0x0020): 
> Cannot set SELinux login context.
>
>
> LS
>
> ___
> sssd-devel mailing list
> sssd-devel@lists.fedorahosted.org
> https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org
>

There is a typo in the commit shot log: Apend -> Append (no need to
resend the patch because of the typo, just fix before pushing)
Patch is really simple and quite useful. Have you checked if there is
any other place where this change would also be welcome?

Best Regards,
--
Fabiano Fidêncio
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] [RFC] Cleaning up the IFP responder (mainly) and socket-activatable responders

[Fabiano Fidêncio]

Howdy!

Taking a look on https://fedorahosted.org/sssd/ticket/2395 seems that
there are a few ways to achieve what's proposed by Simo and I'd like
to discuss one of those before start implementing it.

As far as I understand the bug, SSSD cannot be run as a non-root user
because of IFP responder, which needs to be run as root in order to
write to sssd.conf file. Simo's suggestion was to create a new
service, which should be socket-activated, to handle this writing to
sssd.conf part and Jakub's suggested to completely get rid of this
part. However, Dmitri pointed out that we must have some service
playing with sssd.conf as it's the way that cockpit and ansible can
remotely manage the SSSD configuration.

Okay, in order to actually start solving the issue I'd like to propose
the follow steps:

1) Create a new service, still explicitly enabled in sssd.conf, that
would take care of the managing the SSSD configuration.
2) Get rid of the code dealing with the SSSD configuration from the
IFP responder.
3) Get rid of the OpenLMI specific code from the IFP responder

Till this point the problem wasn't solved at all, but those seem to be
the logical steps in order to make our own lives easier for working in
the next two steps.

4) Make the new service (created in 1) socket-activated/bus-activated.
5) Make the IFP responder also bus-activated/socket-activated.

So, does it sound like a reasonable way to approach the problem?

If yes, I'd like to ask a few questions, mainly about items 4 and 5,
as it eems that in order to achieve what we want,
https://fedorahosted.org/sssd/ticket/2243 and
https://fedorahosted.org/sssd/ticket/3129 have to be solved together,
which seems to be a not so small task.

So, do we have to care about systems that don't have systemd support?
IOW, do we have to keep the current code and also add
socket-activation? If yes, would it be explicitly enabled/disabled on
configure time? Maybe a lame question, but what are the advantages of
the socket-activation over the bus-activation approach (I still didn't
look at any of those :-\)?

Thanks in advance,
--
Fabiano Fidêncio
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] Re: [PATCH] SDAP: Fix settig paging attribute in sdap_get_generic_ext_send

[Fabiano Fidêncio]

On Tue, Aug 30, 2016 at 5:26 PM, Lukas Slebodnik  wrote:
> ehlo,
>
> We should set pagging flag in state and not in local
> variable which is not read anywhere in the function.
>
> Found by clang static analyzer.
>
> Do we need this patch also to stable branch?
>
> LS
>
> ___
> sssd-devel mailing list
> sssd-devel@lists.fedorahosted.org
> https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org
>

Acked-by: Fabiano Fidêncio 
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] [PATCHES] Remove leftovers from diag_cmd and force_timeout

[Fabiano Fidêncio]

Seems that when I sent the v2 of 7579cf99 and ac35fe74 I attached the
wrong patches that ended up being pushed.
Those patches were incomplete as there are still some leftovers.

My bad, sorry :-\

See these 2 attached patches

Best Regards,
--
Fabiano Fidêncio
From 44a4238a169fa885478a2b3189af1c0e0a7a3692 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= 
Date: Tue, 30 Aug 2016 18:17:46 +0200
Subject: [PATCH 1/2] MONITOR: Remove leftovers from diag_cmd
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Seems that when I sent the v2 of 7579cf99 I attached the wrong patch
that ended up being pushed.
That patch was incomplete as there are still some leftovers.

Related:
https://fedorahosted.org/sssd/ticket/3051

Signed-off-by: Fabiano Fidêncio 
---
 src/config/SSSDConfig/__init__.py.in | 1 -
 src/config/SSSDConfigTest.py | 1 -
 src/config/cfg_rules.ini | 9 -
 src/config/etc/sssd.api.conf | 1 -
 4 files changed, 12 deletions(-)

diff --git a/src/config/SSSDConfig/__init__.py.in b/src/config/SSSDConfig/__init__.py.in
index b3f04ac..fb07127 100644
--- a/src/config/SSSDConfig/__init__.py.in
+++ b/src/config/SSSDConfig/__init__.py.in
@@ -51,7 +51,6 @@ option_strings = {
 'reconnection_retries' : _('Number of times to attempt connection to Data Providers'),
 'fd_limit' : _('The number of file descriptors that may be opened by this responder'),
 'client_idle_timeout' : _('Idle time before automatic disconnection of a client'),
-'diag_cmd' : _('The command to run when a service ping times out'),
 
 # [sssd]
 'services' : _('SSSD Services to start'),
diff --git a/src/config/SSSDConfigTest.py b/src/config/SSSDConfigTest.py
index 8fcd1a5..575a124 100755
--- a/src/config/SSSDConfigTest.py
+++ b/src/config/SSSDConfigTest.py
@@ -309,7 +309,6 @@ class SSSDConfigTestSSSDService(unittest.TestCase):
 'reconnection_retries',
 'fd_limit',
 'client_idle_timeout',
-'diag_cmd',
 'description',
 'certificate_verification',
 'override_space']
diff --git a/src/config/cfg_rules.ini b/src/config/cfg_rules.ini
index df10538..a2c3fa2 100644
--- a/src/config/cfg_rules.ini
+++ b/src/config/cfg_rules.ini
@@ -25,7 +25,6 @@ option = fd_limit
 option = client_idle_timeout
 option = force_timeout
 option = description
-option = diag_cmd
 
 # Monitor service
 option = services
@@ -57,7 +56,6 @@ option = fd_limit
 option = client_idle_timeout
 option = force_timeout
 option = description
-option = diag_cmd
 
 # Name service
 option = user_attributes
@@ -96,7 +94,6 @@ option = fd_limit
 option = client_idle_timeout
 option = force_timeout
 option = description
-option = diag_cmd
 
 # Authentication service
 option = offline_credentials_expiration
@@ -130,7 +127,6 @@ option = fd_limit
 option = client_idle_timeout
 option = force_timeout
 option = description
-option = diag_cmd
 
 # sudo service
 option = sudo_timed
@@ -152,7 +148,6 @@ option = fd_limit
 option = client_idle_timeout
 option = force_timeout
 option = description
-option = diag_cmd
 
 # autofs service
 option = autofs_negative_timeout
@@ -173,7 +168,6 @@ option = fd_limit
 option = client_idle_timeout
 option = force_timeout
 option = description
-option = diag_cmd
 
 # ssh service
 option = ssh_hash_known_hosts
@@ -196,7 +190,6 @@ option = fd_limit
 option = client_idle_timeout
 option = force_timeout
 option = description
-option = diag_cmd
 
 # PAC responder
 option = allowed_uids
@@ -218,7 +211,6 @@ option = fd_limit
 option = client_idle_timeout
 option = force_timeout
 option = description
-option = diag_cmd
 
 # InfoPipe responder
 option = allowed_uids
@@ -239,7 +231,6 @@ option = fd_limit
 option = client_idle_timeout
 option = force_timeout
 option = description
-option = diag_cmd
 
 #Available provider types
 option = id_provider
diff --git a/src/config/etc/sssd.api.conf b/src/config/etc/sssd.api.conf
index 5e69414..b2f20c5 100644
--- a/src/config/etc/sssd.api.conf
+++ b/src/config/etc/sssd.api.conf
@@ -15,7 +15,6 @@ fd_limit = int, None, false
 client_idle_timeout = int, None, false
 force_timeout = int, None, false
 description = str, None, false
-diag_cmd = str, None, false
 
 [sssd]
 # Monitor service
-- 
2.7.4

From 19ab71baeacfcbe45d28deaeae82012551c05968 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= 
Date: Tue, 30 Aug 2016 18:25:21 +0200
Subject: [PATCH 2/2] MONITOR: Remove leftovers from kill_service
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Seems that wen I sent the v2 of ac35fe74 I attached the wrong pacth that
ended up being pushed.
The patch was incomplete as there are still some leftovers.

The .po and sssd-docs.pot were not to