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

Jakub Hrozek jhrozek at redhat.com
Fri Nov 15 16:47:08 UTC 2013


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?



More information about the sssd-devel mailing list