[PATCH] samba-tool group addmembers

Alexander Bokovoy ab at samba.org
Wed Jun 7 11:19:02 UTC 2017


On ke, 07 kesä 2017, Rowland Penny via samba-technical wrote:
> On Wed, 7 Jun 2017 12:56:35 +0300
> Alexander Bokovoy <ab at samba.org> wrote:
> 
> > On ke, 07 kesä 2017, Rowland Penny via samba-technical wrote:
> > > On Wed, 7 Jun 2017 12:02:53 +0300
> > > Alexander Bokovoy <ab at samba.org> wrote:
> > > 
> > > > On ke, 07 kesä 2017, Rowland Penny via samba-technical wrote:
> > > > > 
> > > > > Hi, if you try to add a member to a group and the member exists
> > > > > as a sAMAccountName and a CN, you get this:
> > > > > 
> > > > > root at dc1:~# samba-tool group addmembers group12 rowland
> > > > > ERROR(exception): Failed to add members "rowland" to group
> > > > > "group12"
> > > > > - Unable to find "rowland". Operation cancelled. File
> > > > > "/usr/local/samba/lib/python2.7/site-packages/samba/netcmd/group.py",
> > > > > line 239, in run add_members_operation=True) File
> > > > > "/usr/local/samba/lib/python2.7/site-packages/samba/samdb.py",
> > > > > line 278, in add_remove_group_members raise Exception('Unable
> > > > > to find "%s". Operation cancelled.' % member)
> > > > > 
> > > > > The user 'rowland' exists here:
> > > > > 
> > > > > dn: CN=Rowland Penny,CN=Users,DC=samdom,DC=example,DC=com
> > > > > sAMAccountName: rowland
> > > > > 
> > > > > and here:
> > > > > 
> > > > > dn: CN=rowland,OU=SUDOers,DC=samdom,DC=example,DC=com
> > > > > 
> > > > > The problem isn't that 'rowland' doesn't exist, it is that he
> > > > > exists more than once ;-)
> > > > > 
> > > > > Another user had the same problem, but he created the users with
> > > > > '--use-username-as-cn'
> > > > > 
> > > > > This patch fixed the problem for me and the other user, it just
> > > > > changes the search for the user to use only the sAMAccountName
> > > > > attribute.
> > > > > 
> > > > > Rowland
> > > > 
> > > > > From 8191910cc59e045b94b3779b2b9cddca1b75c230 Mon Sep 17
> > > > > 00:00:00 2001 From: Rowland Penny <rpenny at samba.org>
> > > > > Date: Wed, 7 Jun 2017 09:23:10 +0100
> > > > > Subject: [PATCH] samba-tool: You cannot add members to a group
> > > > > if the member exists as a sAMAccountName and a CN.
> > > > > 
> > > > > Signed-off-by: Rowland Penny <rpenny at samba.org>
> > > > > ---
> > > > >  python/samba/netcmd/group.py | 2 ++
> > > > >  python/samba/samdb.py        | 4 ++--
> > > > >  2 files changed, 4 insertions(+), 2 deletions(-)
> > > > > 
> > > > > diff --git a/python/samba/netcmd/group.py
> > > > > b/python/samba/netcmd/group.py index 11f8773..b9d6add 100644
> > > > > --- a/python/samba/netcmd/group.py
> > > > > +++ b/python/samba/netcmd/group.py
> > > > > @@ -199,6 +199,8 @@ This command adds one or more members to an
> > > > > existing Active Directory group. The 
> > > > >  When a member is added to a group the member may inherit
> > > > > permissions and rights from the group.  Likewise, when
> > > > > permission or rights of a group are changed, the changes may
> > > > > reflect in the members through inheritance. +The member names
> > > > > specified on the command must be the sAMaccountName. + Example1:
> > > > >  samba-tool group addmembers supergroup Group1,Group2,User1 -H
> > > > > ldap://samba.samdom.example.com -Uadministrator%passw0rd 
> > > > > diff --git a/python/samba/samdb.py b/python/samba/samdb.py
> > > > > index 19dd8e9..b4d6768 100644
> > > > > --- a/python/samba/samdb.py
> > > > > +++ b/python/samba/samdb.py
> > > > > @@ -267,8 +267,8 @@ changetype: modify
> > > > >  
> > > > >              for member in members:
> > > > >                  targetmember =
> > > > > self.search(base=self.domain_dn(), scope=ldb.SCOPE_SUBTREE,
> > > > > -
> > > > > expression="(|(sAMAccountName=%s)(CN=%s))" % (
> > > > > -                    ldb.binary_encode(member),
> > > > > ldb.binary_encode(member)), attrs=[])
> > > > > +
> > > > > expression="(sAMAccountName=%s)" % (
> > > > > +                    ldb.binary_encode(member)), attrs=[])
> > > > >  
> > > > >                  if len(targetmember) != 1:
> > > > >                      raise Exception('Unable to find "%s".
> > > > > Operation cancelled.' % member) -- 
> > > > > 2.1.4
> > > > > 
> > > > I think instead of removing CN=%s from the filter it would be
> > > > better to limit the search by objectclass filtering to
> > > > 'objectclass=user'.
> > > > 
> > > > E.g. change expression into
> > > > 
> > > >   expression="(&(|(sAMAccountName=%s)(CN=%s))(objectclass=user))"
> > > > % ...
> > > > 
> > > > 
> > > 
> > > I don't think this will work, the other user created users with
> > > '--use-username-as-cn' and it failed in the same way for him until
> > > he tried my patch, my supposition is that the user is seen twice.
> > > Once by 'sAMAccountName' and again by 'CN'. There is also the
> > > problem of groups, or do you suggest we use something like:
> > > 
> > > expression="(&(|(sAMAccountName=%s)(CN=%s))(|(objectclass=user)(objectclass=group)))"
> > > % ...
> > Yes. At least in python/samba/netcmd/group.py it is clear that members
> > of a group could be user, group, or machine account. This means all of
> > those object classes need to be in the filter. Luckily, that is all
> > covered by (|(objectclass=user)(objectclass=group)).
> > 
> > Leaving out CN=%s would fail hierarchical groups (adding a group as a
> > member of a group). Your problem with sudoers is effectively on
> > matching more than needed objects. If cn=rowland,cn=SUDOers,.. is not
> > a user or a group, then it will not match this new filter and
> > everything will be fine.
> > 
> 
> So, by leaving out 'CN=%s', I will not be able to add a group to a
> group ?
I did small research about it. sAMAccountName is a mandatory part of
Security-Principal class in AD DS schema. Security-Principal is
an auxiliary class for Group class. The latter means that while there
are no instances of Security-Principal object class in AD DS, the
attributes defined as mandatory for an auxiliary class would be
mandatory for the structural classes (like Group) which derive from them.

This means sAMAccountName will exist for Group too. Looks like Windows
populates it by CN value of the group. A semantics of sAMAccountName is
that it never changes:

https://msdn.microsoft.com/en-us/library/ms679635(v=vs.85).aspx
--------------------------------
Update Frequency 

This value should be assigned when the account record is created, and
should not change.
--------------------------------

>From my experiments with the global catalog, it seems that AD DCs don't
ask for CN too when searching a user:

 (&(|(&(samaccounttype=*)
       (!(grouptype:1.2.840.113556.1.4.803:=1))
       (grouptype:1.2.840.113556.1.4.803:=2147483648)
       (grouptype:1.2.840.113556.1.4.804:=10))
    (samaccounttype=805306368))
    (|(name=admin*)
      (samaccountname=admin*)
      (displayName=admin*)
      (msds-phoneticdisplayname=admin*))
 )

Notice that it looks for various name fields here, not just
samaccountname. Not sure we should support this for reasons outlined
below.

So probably you are right, we can drop CN part of the filter.

Further, looking at source4/dsdb/common/util_samr.c, I can see that we
always add sAMAccountName to groups in dsdb_add_domain_group() using CN
attribute's value.

The same file gives you two filters Samba does use to search for users
and groups internally:

        /* check if the group already exists */
        name = samdb_search_string(ldb, tmp_ctx, NULL,
                                   "sAMAccountName",
                                   "(&(sAMAccountName=%s)(objectclass=group))",
                                   ldb_binary_encode_string(tmp_ctx, groupname));

and

        /* check if the user already exists */
        name = samdb_search_string(ldb, tmp_ctx, NULL,
                                   "sAMAccountName",
                                   "(&(sAMAccountName=%s)(objectclass=user))",
                                   ldb_binary_encode_string(tmp_ctx, account_name));

So I think our final filter would be

  expression="(&(sAMAccountName=%s)(|(objectclass=user)(objectclass=group)))"
  % ...


-- 
/ Alexander Bokovoy



More information about the samba-technical mailing list