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(a)ad18.ipa18.devel
> > > > ACL:posix_test_group@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@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@BUILTIN
> > > > GROUP:domain users(a)ad18.ipa18.devel
> > > > ACL:posix_test_group@ad18.ipa18.devel:ALLOWED/0x0/FULL
> > > > ACL:CONSOLE LOGON:ALLOWED/0x0/READ
> > > > ACL:Network Configuration Operators@BUILTIN:ALLOWED/0x0/READ
> > > > ACL:SYSTEM@NT AUTHORITY:ALLOWED/I/FULL
> > > > ACL:Administrators@BUILTIN:ALLOWED/I/FULL
> > > > ACL:administrator@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(a)ad18.ipa18.devel
> > > > ACL:posix_test_group@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@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.