[SSSD] Re: Design document: Enhanced NSS API

2017-11-02 Thread Sumit Bose
On Fri, Oct 27, 2017 at 08:43:28AM -0400, Simo Sorce wrote:
> On Thu, 2017-10-26 at 22:14 +0200, Sumit Bose wrote:
> > On Thu, Oct 26, 2017 at 02:43:29PM -0400, Simo Sorce wrote:
> > > On Thu, 2017-10-26 at 12:16 +0200, Jakub Hrozek wrote:
> > > > On Wed, Oct 25, 2017 at 05:39:21PM +0200, Sumit Bose wrote:
> > > > > Hi,
> > > > > 
> > > > > please find below the design document for the enhanced NSS API
> > > > > which
> > > > > makes e.g. the client side timeouts which where recently
> > > > > refactored
> > > > > available to callers.
> > > > > 
> > > > > A more visual friendly version can be found at:
> > > > > https://pagure.io/fork/sbose/SSSD/docs/blob/07514ce52845d47fe6b
> > > > > b327
> > > > > f782a865bfa75628a/f/design_pages/enhanced_nss_api.rst
> > > > > 
> > > > > bye,
> > > > > Sumit
> > > > 
> > > > LGTM!
> > > > 
> > > 
> > > Looking at this I have some questions, if you are going to create a
> > > new
> > > library and just need to set a timeout it seem it would be a much
> > > better interface to use a context handle you allocate and pass into
> > > each call, and then have getters setters to set timeouts or any
> > > other
> > > flags that should influence the whole behavior. This will allow you
> > > also to control how many concurrent connections you want to have
> > > against sssd, as each new context will create a  new socket
> > > connection
> > > and all.
> > > 
> > > In the original libnss_sss.so we could not do that because the
> > > glibc
> > > interface does not offer any way to hold a context, but there is no
> > > reason to continue along that line in a *new* API. And not using
> > > contexts in threaded applications is generally a bad idea, as you
> > > end
> > > up *requiring* te use of mutexes when that is really not always a
> > > need
> > > (separated threads can open separate connections and not share any
> > > data
> > > that require mutexes).
> > 
> > This sounds like a good 2.0 feature, are you interested in creating a
> > more detailed design page for this?
> 
> Sure.
> 
> >  Currently my goal was to reuse as
> > much or the current trusted code we already have.
> > 
> > > 
> > > On the responder side I also do not see why new calls are being
> > > created. You clearly want a client-wide behavior, introduce ONE new
> > > call that sets flags for the rest of the connection, and just reuse
> > > the
> > > usual commands otherwise.
> > 
> > The current flags, like invalidating a cached entry are per-request,
> > only the single object address by the current request should be
> > invalidate not all object which are requested on the same connection.
> 
> I would probably add a command to explicitly invalidate individual
> caches, this would avoid having special paths on every other call,
> resulting in cleaner code, at the cost of one more roundtrip though, so
> I guess it is a matter of figuring out what is the right balance here.
> 
> > > 
> > > I do not understand what is the point of nss_truste_users why a
> > > force
> > > reload is a privileged operation ?
> > 
> > Since it can force expensive operations on the backends which will
> > hit servers I think not everybody should be allowed to do this.
> 
> You can already force this by requesting unexisting users/groups, I am
> not convinced this necessarily needs to be a privileged operation as
> there are already ways to cause work for SSSD.
> I would rather drop it. If we really want to deal with potential abuse
> we should introduce rate-limiting per uid, and basically slow down to a
> halt abusing clients by giving weights and possibly quotas, doling out
> privileges is cumbersome anyway and does not really prevent a malicious
> client to cause hard ATM.
> 
> IMHO nss_trusted_users gets a NACK as a concept and should be dropped.

Of course I can remove it, but since removing it later is easier than
adding it later I'd like to try to explain again why I think it would be
useful.

You are right that it is already possible to send requests to the
servers via SSSD. But as you said this are "only" searches for
unexisting user and groups which should be handled by the indexes on the
server quite efficiently and causes no disk-I/O on the client.
Additionally we try to avoid accidental misuse of this with the negative
cache.

The flags might trigger operations like looking up all groups a user is
a member of or looking up a group with all members. While only the first
might cause a more expensive operation on the server both might cause a
lot of disk-I/O on the client.

Rate limitations would help to mitigate misuse as well. But a typical
SSSD client would have no use for the flags so why allow it to use them?

That's why I think nss_trusted_users is a good way to avoid accidental
misuse in a similar way as the negative cache. But if you prefer I'll
drop it.

> 
> > > 
> > > I guess DNLSGTM ?
> > 
> > A test-builds with a simplified version of the related patches solved
> > bottleneck issues with 389ds IPA modules. So at lea

[SSSD] Re: Design document: Enhanced NSS API

2017-11-02 Thread Simo Sorce
On Thu, 2017-11-02 at 13:14 +0100, Sumit Bose wrote:
> On Fri, Oct 27, 2017 at 08:43:28AM -0400, Simo Sorce wrote:
> > On Thu, 2017-10-26 at 22:14 +0200, Sumit Bose wrote:
> > > On Thu, Oct 26, 2017 at 02:43:29PM -0400, Simo Sorce wrote:
> > > > On Thu, 2017-10-26 at 12:16 +0200, Jakub Hrozek wrote:
> > > > > On Wed, Oct 25, 2017 at 05:39:21PM +0200, Sumit Bose wrote:
> > > > > > Hi,
> > > > > > 
> > > > > > please find below the design document for the enhanced NSS
> > > > > > API
> > > > > > which
> > > > > > makes e.g. the client side timeouts which where recently
> > > > > > refactored
> > > > > > available to callers.
> > > > > > 
> > > > > > A more visual friendly version can be found at:
> > > > > > https://pagure.io/fork/sbose/SSSD/docs/blob/07514ce52845d47
> > > > > > fe6b
> > > > > > b327
> > > > > > f782a865bfa75628a/f/design_pages/enhanced_nss_api.rst
> > > > > > 
> > > > > > bye,
> > > > > > Sumit
> > > > > 
> > > > > LGTM!
> > > > > 
> > > > 
> > > > Looking at this I have some questions, if you are going to
> > > > create a
> > > > new
> > > > library and just need to set a timeout it seem it would be a
> > > > much
> > > > better interface to use a context handle you allocate and pass
> > > > into
> > > > each call, and then have getters setters to set timeouts or any
> > > > other
> > > > flags that should influence the whole behavior. This will allow
> > > > you
> > > > also to control how many concurrent connections you want to
> > > > have
> > > > against sssd, as each new context will create a  new socket
> > > > connection
> > > > and all.
> > > > 
> > > > In the original libnss_sss.so we could not do that because the
> > > > glibc
> > > > interface does not offer any way to hold a context, but there
> > > > is no
> > > > reason to continue along that line in a *new* API. And not
> > > > using
> > > > contexts in threaded applications is generally a bad idea, as
> > > > you
> > > > end
> > > > up *requiring* te use of mutexes when that is really not always
> > > > a
> > > > need
> > > > (separated threads can open separate connections and not share
> > > > any
> > > > data
> > > > that require mutexes).
> > > 
> > > This sounds like a good 2.0 feature, are you interested in
> > > creating a
> > > more detailed design page for this?
> > 
> > Sure.
> > 
> > >  Currently my goal was to reuse as
> > > much or the current trusted code we already have.
> > > 
> > > > 
> > > > On the responder side I also do not see why new calls are being
> > > > created. You clearly want a client-wide behavior, introduce ONE
> > > > new
> > > > call that sets flags for the rest of the connection, and just
> > > > reuse
> > > > the
> > > > usual commands otherwise.
> > > 
> > > The current flags, like invalidating a cached entry are per-
> > > request,
> > > only the single object address by the current request should be
> > > invalidate not all object which are requested on the same
> > > connection.
> > 
> > I would probably add a command to explicitly invalidate individual
> > caches, this would avoid having special paths on every other call,
> > resulting in cleaner code, at the cost of one more roundtrip
> > though, so
> > I guess it is a matter of figuring out what is the right balance
> > here.
> > 
> > > > 
> > > > I do not understand what is the point of nss_truste_users why a
> > > > force
> > > > reload is a privileged operation ?
> > > 
> > > Since it can force expensive operations on the backends which
> > > will
> > > hit servers I think not everybody should be allowed to do this.
> > 
> > You can already force this by requesting unexisting users/groups, I
> > am
> > not convinced this necessarily needs to be a privileged operation
> > as
> > there are already ways to cause work for SSSD.
> > I would rather drop it. If we really want to deal with potential
> > abuse
> > we should introduce rate-limiting per uid, and basically slow down
> > to a
> > halt abusing clients by giving weights and possibly quotas, doling
> > out
> > privileges is cumbersome anyway and does not really prevent a
> > malicious
> > client to cause hard ATM.
> > 
> > IMHO nss_trusted_users gets a NACK as a concept and should be
> > dropped.
> 
> Of course I can remove it, but since removing it later is easier than
> adding it later I'd like to try to explain again why I think it would
> be useful.
> 
> You are right that it is already possible to send requests to the
> servers via SSSD. But as you said this are "only" searches for
> unexisting user and groups which should be handled by the indexes on
> the server quite efficiently and causes no disk-I/O on the client.
> Additionally we try to avoid accidental misuse of this with the
> negative cache.

You can also call out existing users, especially in large domains if
you have groups big enough you can cause fast cache thrashing very
easily, this is already a big deal for the client. Client disk load is
uninteresting because a process can simply write/fsync locally to ca

[SSSD] Re: Design document: Enhanced NSS API

2017-11-02 Thread Alexander Bokovoy

On to, 02 marras 2017, Simo Sorce wrote:

On Thu, 2017-11-02 at 13:14 +0100, Sumit Bose wrote:

On Fri, Oct 27, 2017 at 08:43:28AM -0400, Simo Sorce wrote:
> On Thu, 2017-10-26 at 22:14 +0200, Sumit Bose wrote:
> > On Thu, Oct 26, 2017 at 02:43:29PM -0400, Simo Sorce wrote:
> > > On Thu, 2017-10-26 at 12:16 +0200, Jakub Hrozek wrote:
> > > > On Wed, Oct 25, 2017 at 05:39:21PM +0200, Sumit Bose wrote:
> > > > > Hi,
> > > > >
> > > > > please find below the design document for the enhanced NSS
> > > > > API
> > > > > which
> > > > > makes e.g. the client side timeouts which where recently
> > > > > refactored
> > > > > available to callers.
> > > > >
> > > > > A more visual friendly version can be found at:
> > > > > https://pagure.io/fork/sbose/SSSD/docs/blob/07514ce52845d47
> > > > > fe6b
> > > > > b327
> > > > > f782a865bfa75628a/f/design_pages/enhanced_nss_api.rst
> > > > >
> > > > > bye,
> > > > > Sumit
> > > >
> > > > LGTM!
> > > >
> > >
> > > Looking at this I have some questions, if you are going to
> > > create a
> > > new
> > > library and just need to set a timeout it seem it would be a
> > > much
> > > better interface to use a context handle you allocate and pass
> > > into
> > > each call, and then have getters setters to set timeouts or any
> > > other
> > > flags that should influence the whole behavior. This will allow
> > > you
> > > also to control how many concurrent connections you want to
> > > have
> > > against sssd, as each new context will create a  new socket
> > > connection
> > > and all.
> > >
> > > In the original libnss_sss.so we could not do that because the
> > > glibc
> > > interface does not offer any way to hold a context, but there
> > > is no
> > > reason to continue along that line in a *new* API. And not
> > > using
> > > contexts in threaded applications is generally a bad idea, as
> > > you
> > > end
> > > up *requiring* te use of mutexes when that is really not always
> > > a
> > > need
> > > (separated threads can open separate connections and not share
> > > any
> > > data
> > > that require mutexes).
> >
> > This sounds like a good 2.0 feature, are you interested in
> > creating a
> > more detailed design page for this?
>
> Sure.
>
> >  Currently my goal was to reuse as
> > much or the current trusted code we already have.
> >
> > >
> > > On the responder side I also do not see why new calls are being
> > > created. You clearly want a client-wide behavior, introduce ONE
> > > new
> > > call that sets flags for the rest of the connection, and just
> > > reuse
> > > the
> > > usual commands otherwise.
> >
> > The current flags, like invalidating a cached entry are per-
> > request,
> > only the single object address by the current request should be
> > invalidate not all object which are requested on the same
> > connection.
>
> I would probably add a command to explicitly invalidate individual
> caches, this would avoid having special paths on every other call,
> resulting in cleaner code, at the cost of one more roundtrip
> though, so
> I guess it is a matter of figuring out what is the right balance
> here.
>
> > >
> > > I do not understand what is the point of nss_truste_users why a
> > > force
> > > reload is a privileged operation ?
> >
> > Since it can force expensive operations on the backends which
> > will
> > hit servers I think not everybody should be allowed to do this.
>
> You can already force this by requesting unexisting users/groups, I
> am
> not convinced this necessarily needs to be a privileged operation
> as
> there are already ways to cause work for SSSD.
> I would rather drop it. If we really want to deal with potential
> abuse
> we should introduce rate-limiting per uid, and basically slow down
> to a
> halt abusing clients by giving weights and possibly quotas, doling
> out
> privileges is cumbersome anyway and does not really prevent a
> malicious
> client to cause hard ATM.
>
> IMHO nss_trusted_users gets a NACK as a concept and should be
> dropped.

Of course I can remove it, but since removing it later is easier than
adding it later I'd like to try to explain again why I think it would
be useful.

You are right that it is already possible to send requests to the
servers via SSSD. But as you said this are "only" searches for
unexisting user and groups which should be handled by the indexes on
the server quite efficiently and causes no disk-I/O on the client.
Additionally we try to avoid accidental misuse of this with the
negative cache.


You can also call out existing users, especially in large domains if
you have groups big enough you can cause fast cache thrashing very
easily, this is already a big deal for the client. Client disk load is
uninteresting because a process can simply write/fsync locally to cause
disk I/O issues, and traffic towards the server is also uninteresting
because a client can simply directly contact the server directly to
cause load.

So what we are left with is uniquely the fact a process might keep the
sssd

[SSSD] Re: Design document: Enhanced NSS API

2017-11-02 Thread Simo Sorce
On Thu, 2017-11-02 at 14:53 +0200, Alexander Bokovoy wrote:
> On to, 02 marras 2017, Simo Sorce wrote:
> > On Thu, 2017-11-02 at 13:14 +0100, Sumit Bose wrote:
> > > On Fri, Oct 27, 2017 at 08:43:28AM -0400, Simo Sorce wrote:
> > > > On Thu, 2017-10-26 at 22:14 +0200, Sumit Bose wrote:
> > > > > On Thu, Oct 26, 2017 at 02:43:29PM -0400, Simo Sorce wrote:
> > > > > > On Thu, 2017-10-26 at 12:16 +0200, Jakub Hrozek wrote:
> > > > > > > On Wed, Oct 25, 2017 at 05:39:21PM +0200, Sumit Bose
> > > > > > > wrote:
> > > > > > > > Hi,
> > > > > > > > 
> > > > > > > > please find below the design document for the enhanced
> > > > > > > > NSS
> > > > > > > > API
> > > > > > > > which
> > > > > > > > makes e.g. the client side timeouts which where
> > > > > > > > recently
> > > > > > > > refactored
> > > > > > > > available to callers.
> > > > > > > > 
> > > > > > > > A more visual friendly version can be found at:
> > > > > > > > https://pagure.io/fork/sbose/SSSD/docs/blob/07514ce5284
> > > > > > > > 5d47
> > > > > > > > fe6b
> > > > > > > > b327
> > > > > > > > f782a865bfa75628a/f/design_pages/enhanced_nss_api.rst
> > > > > > > > 
> > > > > > > > bye,
> > > > > > > > Sumit
> > > > > > > 
> > > > > > > LGTM!
> > > > > > > 
> > > > > > 
> > > > > > Looking at this I have some questions, if you are going to
> > > > > > create a
> > > > > > new
> > > > > > library and just need to set a timeout it seem it would be
> > > > > > a
> > > > > > much
> > > > > > better interface to use a context handle you allocate and
> > > > > > pass
> > > > > > into
> > > > > > each call, and then have getters setters to set timeouts or
> > > > > > any
> > > > > > other
> > > > > > flags that should influence the whole behavior. This will
> > > > > > allow
> > > > > > you
> > > > > > also to control how many concurrent connections you want to
> > > > > > have
> > > > > > against sssd, as each new context will create a  new socket
> > > > > > connection
> > > > > > and all.
> > > > > > 
> > > > > > In the original libnss_sss.so we could not do that because
> > > > > > the
> > > > > > glibc
> > > > > > interface does not offer any way to hold a context, but
> > > > > > there
> > > > > > is no
> > > > > > reason to continue along that line in a *new* API. And not
> > > > > > using
> > > > > > contexts in threaded applications is generally a bad idea,
> > > > > > as
> > > > > > you
> > > > > > end
> > > > > > up *requiring* te use of mutexes when that is really not
> > > > > > always
> > > > > > a
> > > > > > need
> > > > > > (separated threads can open separate connections and not
> > > > > > share
> > > > > > any
> > > > > > data
> > > > > > that require mutexes).
> > > > > 
> > > > > This sounds like a good 2.0 feature, are you interested in
> > > > > creating a
> > > > > more detailed design page for this?
> > > > 
> > > > Sure.
> > > > 
> > > > >  Currently my goal was to reuse as
> > > > > much or the current trusted code we already have.
> > > > > 
> > > > > > 
> > > > > > On the responder side I also do not see why new calls are
> > > > > > being
> > > > > > created. You clearly want a client-wide behavior, introduce
> > > > > > ONE
> > > > > > new
> > > > > > call that sets flags for the rest of the connection, and
> > > > > > just
> > > > > > reuse
> > > > > > the
> > > > > > usual commands otherwise.
> > > > > 
> > > > > The current flags, like invalidating a cached entry are per-
> > > > > request,
> > > > > only the single object address by the current request should
> > > > > be
> > > > > invalidate not all object which are requested on the same
> > > > > connection.
> > > > 
> > > > I would probably add a command to explicitly invalidate
> > > > individual
> > > > caches, this would avoid having special paths on every other
> > > > call,
> > > > resulting in cleaner code, at the cost of one more roundtrip
> > > > though, so
> > > > I guess it is a matter of figuring out what is the right
> > > > balance
> > > > here.
> > > > 
> > > > > > 
> > > > > > I do not understand what is the point of nss_truste_users
> > > > > > why a
> > > > > > force
> > > > > > reload is a privileged operation ?
> > > > > 
> > > > > Since it can force expensive operations on the backends which
> > > > > will
> > > > > hit servers I think not everybody should be allowed to do
> > > > > this.
> > > > 
> > > > You can already force this by requesting unexisting
> > > > users/groups, I
> > > > am
> > > > not convinced this necessarily needs to be a privileged
> > > > operation
> > > > as
> > > > there are already ways to cause work for SSSD.
> > > > I would rather drop it. If we really want to deal with
> > > > potential
> > > > abuse
> > > > we should introduce rate-limiting per uid, and basically slow
> > > > down
> > > > to a
> > > > halt abusing clients by giving weights and possibly quotas,
> > > > doling
> > > > out
> > > > privileges is cumbersome anyway and does not really prevent a
> > > > malicious
> > > > client to cause hard ATM.
> > > > 
> > >

[SSSD] Re: sssd crash on RHEL 7.3

2017-11-02 Thread Steve French
>> one has Samba 4.5 (built the same way). sssd config looks
>> identical, "net ads info" shows the same output, klist -ke output
>> matches - but sssd crashes on one (see below) when starting the
>> service. Ideas?

> You need to rebuild sssd against libldb required by the newer samba (4.7). 
> This was discussed >extensively on the list right before 4.7 RC1 release.

Do you know if more recent RHEL7 (1.15 in RHEL7.4 e.g.) addressed the
libldb incompatibility (and resultant sssd crash) that Alexander
noted?  The version of sssd in RHEL7.5 beta didn't appear any newer
(but is there a RHEL7.x build of 1.16 we could try)?


-- 
Thanks,

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


[SSSD] Re: sssd crash on RHEL 7.3

2017-11-02 Thread smfrench
I am expecting that RHEL7.5 still has problems with Samba and SSSD library 
problems e.g. I see this comment indicating that RHEL 7.5 beta sssd (even with 
default Samba in RHEL) is broken 
https://github.com/cockpit-project/cockpit/issues/7916

The crash is in a similar location (memberof.so+1000):
-- Unit sssd.service has begun starting up.
Nov 03 00:04:03 sfrench-dd1-protocols-021117223937311.lab.com sssd[13557]: 
Starting up
Nov 03 00:04:03 sfrench-dd1-protocols-021117223937311.lab.com kernel: 
sssd[13557]: segfault at 7f7d149a5d78 ip 7f7d149a5d78 sp 7ffe4fc16fd8 
error 15 in memberof.so[7f7d149a5000+1000]
Nov 03 00:04:03 sfrench-dd1-protocols-021117223937311.lab.com systemd[1]: 
sssd.service: control process exited, code=exited status=1
Nov 03 00:04:03 sfrench-dd1-protocols-021117223937311.lab.com systemd[1]: 
Failed to start System Security Services Daemon.
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org


[SSSD] Re: sssd crash on RHEL 7.3

2017-11-02 Thread Alexander Bokovoy

On pe, 03 marras 2017, smfre...@gmail.com wrote:

I am expecting that RHEL7.5 still has problems with Samba and SSSD
library problems e.g. I see this comment indicating that RHEL 7.5 beta
sssd (even with default Samba in RHEL) is broken
https://github.com/cockpit-project/cockpit/issues/7916

There is no RHEL 7.5 beta available yet.

At any given release, samba+libldb in that release is consistent. The
problem only occurs if you are intentionally building Samba (or libldb)
version which is incompatible with what is in the platform.

Since you want to build Samba git master and that one requires libldb
1.3, anything built against 1.2.2 is not compatible with libldb 1.3.
That's the problem.

This means if you truly want to get libldb 1.3 and git master of Samba
on your system, you need to rebuild SSSD yourself.

For development purposes we use COPR repositories for this. This is
what, for example, freeIPA team is doing.



The crash is in a similar location (memberof.so+1000):
-- Unit sssd.service has begun starting up.
Nov 03 00:04:03 sfrench-dd1-protocols-021117223937311.lab.com sssd[13557]: 
Starting up
Nov 03 00:04:03 sfrench-dd1-protocols-021117223937311.lab.com kernel: 
sssd[13557]: segfault at 7f7d149a5d78 ip 7f7d149a5d78 sp 7ffe4fc16fd8 
error 15 in memberof.so[7f7d149a5000+1000]
Nov 03 00:04:03 sfrench-dd1-protocols-021117223937311.lab.com systemd[1]: 
sssd.service: control process exited, code=exited status=1
Nov 03 00:04:03 sfrench-dd1-protocols-021117223937311.lab.com systemd[1]: 
Failed to start System Security Services Daemon.
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org


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