[SSSD] [PATCHES] nss: check for Well-Known SIDs in SID based requests

Jakub Hrozek jhrozek at redhat.com
Fri Nov 15 17:08:30 UTC 2013


On Fri, Nov 15, 2013 at 06:01:14PM +0100, Sumit Bose wrote:
> On Fri, Nov 15, 2013 at 05:47:08PM +0100, Jakub Hrozek wrote:
> > On Wed, Nov 13, 2013 at 01:12:02PM +0100, Sumit Bose wrote:
> > > On Mon, Nov 11, 2013 at 06:28:33PM -0500, Simo Sorce wrote:
> > > > On Mon, 2013-11-11 at 22:03 +0100, Sumit Bose wrote:
> > > > > On Mon, Nov 11, 2013 at 10:41:46AM -0500, Simo Sorce wrote: 
> > > > > > Why should we reserve these names ?
> > > > > > These SID are never used for actual users anyway, so I do not understand
> > > > > > why would you care about their names ?
> > > > > > How would you create a collision ?
> > > > > 
> > > > > The main purpose is to resolve the SIDs for [gs]etcifsacl. Currently the
> > > > > output for getcifsacl with SSSD looks like:
> > > > > 
> > > > > REVISION:0x1
> > > > > CONTROL:0x8404
> > > > > OWNER:S-1-5-32-544
> > > > > GROUP:domain users at ad18.ipa18.devel
> > > > > ACL:posix_test_group at ad18.ipa18.devel:ALLOWED/0x0/FULL
> > > > > ACL:S-1-2-1:ALLOWED/0x0/READ
> > > > > ACL:S-1-5-32-556:ALLOWED/0x0/READ
> > > > > ACL:S-1-5-18:ALLOWED/I/FULL
> > > > > ACL:S-1-5-32-544:ALLOWED/I/FULL
> > > > > ACL:administrator at ad18.ipa18.devel:ALLOWED/I/FULL
> > > > > 
> > > > > the well-Known SIDs are not resolved. With the patches it looks like:
> > > > > 
> > > > > REVISION:0x1
> > > > > CONTROL:0x8404
> > > > > OWNER:Administrators at BUILTIN
> > > > > GROUP:domain users at ad18.ipa18.devel
> > > > > ACL:posix_test_group at ad18.ipa18.devel:ALLOWED/0x0/FULL
> > > > > ACL:CONSOLE LOGON:ALLOWED/0x0/READ
> > > > > ACL:Network Configuration Operators at BUILTIN:ALLOWED/0x0/READ
> > > > > ACL:SYSTEM at NT AUTHORITY:ALLOWED/I/FULL
> > > > > ACL:Administrators at BUILTIN:ALLOWED/I/FULL
> > > > > ACL:administrator at ad18.ipa18.devel:ALLOWED/I/FULL
> > > > > 
> > > > > Everything is resolved. Maybe I've gone a little bit to far becasuse I
> > > > > just realized that winbind only resolves the BUILTIN SIDs:
> > > > > 
> > > > > REVISION:0x1
> > > > > CONTROL:0x8404
> > > > > OWNER:BUILTIN\Administrators
> > > > > GROUP:domain users at ad18.ipa18.devel
> > > > > ACL:posix_test_group at ad18.ipa18.devel:ALLOWED/0x0/FULL
> > > > > ACL:S-1-2-1:ALLOWED/0x0/READ
> > > > > ACL:BUILTIN\Network Configuration Operators:ALLOWED/0x0/READ
> > > > > ACL:S-1-5-18:ALLOWED/I/FULL
> > > > > ACL:BUILTIN\Administrators:ALLOWED/I/FULL
> > > > > ACL:administrator at ad18.ipa18.devel:ALLOWED/I/FULL
> > > > > 
> > > > > setcifsacl can use the names instead of the SIDs to modify the ACLs,
> > > > > hence it is useful to support the name-to-SID translation as well.  A
> > > > > collision might be possible if for some reason a called e.g. 'CONSOLE
> > > > > LOGIN' is created. In this case this user is only accessible by its
> > > > > fully qualified name.
> > > > 
> > > > Ok I think it is fair to require the fully qualified name in that case,
> > > > and not try to black list potentially valid names ?
> > > 
> > > ok, thanks. The new version always uses fully qualified names, add a man
> > > page section and updates the cifs-utils plugin to reflect the changes.
> > > 
> > > bye,
> > > Sumit
> > 
> > These patches work really well and I haven't found any issues. Also
> > Coverity scan didn't bring any new defects. I tested mostly with
> > pysss_nss_idmap, both user and group sids and various builtin sids.
> > 
> > Thanks for providing detailed unit tests!
> > 
> > I only have one question about return codes. Currently if the first part
> > of the SID matches any of the well known categories but the RID is not
> > known, we return EINVAL. This return code is then passed all the way to
> > the client. Would it make sense to return ENOENT instead?
> 
> The idea was that if ENOENT is returned SSSD can continue to consult
> his backends to see if the given SID can be resolved. Since the backends
> do not know how to handle SIDs from those categories we can error out
> if we cannot find the matching RID.
> 
> bye,
> Sumit

Ah, right. That makese sense.

ACK.



More information about the sssd-devel mailing list