[PATCH] samba-tool group addmembers
Rowland Penny
rpenny at samba.org
Wed Jun 7 15:02:32 UTC 2017
On Wed, 7 Jun 2017 12:25:51 +0100
Rowland Penny via samba-technical <samba-technical at lists.samba.org>
wrote:
> On Wed, 7 Jun 2017 14:19:02 +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: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)))"
> > % ...
> >
> >
>
> OK, I can live with that, I will produce a new patch based on that
> expression ;-)
>
> Rowland
>
OK, here is the new patch with the objectclass changes.
Rowland
-------------- next part --------------
A non-text attachment was scrubbed...
Name: samba-tool-You-cannot-add-members-to-a-group-if-the-.patch
Type: text/x-patch
Size: 2227 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20170607/f4c0ef9c/samba-tool-You-cannot-add-members-to-a-group-if-the-.bin>
More information about the samba-technical
mailing list