[Samba] Group mapping bugs + PATCH
Ganael LAPLANCHE
ganael.laplanche at edfgdf.fr
Tue Jan 27 10:26:55 GMT 2004
Hi all !
I've been using group mapping for a few weeks and found some bugs... For
three of them, I tried to develop a small patch. I don't know Samba code
very well and this is my first patch, so there could be errors :-p I hope
my investigations will be useful :)
* Platform :
Here is what I used :
Debian 3.0r1, Samba 3.0.2rc1 - PDC, OpenLDAP 2.1.23 as backend
smb.conf extract :
[...]
passdb backend = ldapsam:ldap://ldapserver.mydomain.com, guest
ldap admin dn = "cn=Manager,dc=mydomain,dc=com"
ldap ssl = off
ldap delete dn = no
ldap user suffix = ou=Users
ldap machine suffix = ou=Machines
ldap suffix = dc=mydomain,dc=com
workgroup = MYDOMAIN
netbios name = MYPDC
encrypt passwords = yes
os level = 65
domain logons = Yes
domain master = Yes
local master = Yes
[...]
* Bugs :
1) Security issue : net_groupmap doesn't update users' sambaPrimaryGroupSID
when adding/modifying/deleting a mapping that involves the users' Unix
primary group. Users keep getting rights provided by the "old"
sambaPrimaryGroupSID. We should scan the users to update their
sambaPrimaryGroupSIDs (this will unfortunately decrease performances).
2) pdb_ldap allows to map the same SID twice (with two distinct gids) while
creating/modifying a mapping. So we don't have exactly 1 SID <=> 1 GID. It
is impossible to delete such a mapping (mapped twice) after its creation.
3) pdb_ldap.c also allows to use a gid already mapped while modifying a
mapping (there is no check).
4) ldapsam_update_group_mapping_entry doesn't add the sambaGroupMapping
objectClass when MODIFYING a mapping that involves a "simple" posixGroup
entry in LDAP, like this one :
dn: cn=myusers,ou=Users,dc=mydomain,dc=com
objectClass: posixGroup
gidNumber: 1012
cn: myusers
memberUid: foouser
The result is the mapping can't be modified, here is the error :
ldapsam_update_group_mapping_entry: failed to modify group 1012 error:
attribute 'sambaSID' not allowed (Object class violation)
Could not update group database
We don't get this error when we CREATE a mapping because the objectClass is
automatically added.
5) A last bug was found while modifying an existing mapping :
/usr/local/samba/bin# ./net groupmap list
domadm (S-1-5-21-3746524048-3196343125-3229652583-512) -> domadm
test (S-1-5-21-3746524048-3196343125-3229652583-520) -> newdomadm
opuspdc1:/usr/local/samba/bin# ./net groupmap modify
sid=S-1-5-21-3746524048-3196343125-3229652583-512 unixgroup=newdomadm
Updated mapping entry for
/usr/local/samba/bin# ./net groupmap list
domadm (S-1-5-21-3746524048-3196343125-3229652583-512) -> domadm
domadm (S-1-5-21-3746524048-3196343125-3229652583-512) -> newdomadm
The result of this command is unexpected ! Here is what (I think) happens :
- first, the command doesn't check for the existence of a mapping involving
newdomadm -> we should get an error here (same error as in 3)
- second, net_groupmap.c/net_groupmap_modify looks for the mapping using
the SID (it finds S-1-5-21-3746524048-3196343125-3229652583-512/domadm) and
modifies it in memory (to get
S-1-5-21-3746524048-3196343125-3229652583-512/newdomadm). BUT, when the
mapping is to be stored in LDAP via
pdb_ldap.c/ldapsam_update_group_mapping_entry, it is passed ALREADY
MODIFIED to the function that looks for the mapping but using the new GID
(the one of newdomadm) ! Since the search keys aren't the same (SID, then
Gid), we may get in trouble here. This is what happens, it doesn't return
the same record, it returns the second, existing one
(S-1-5-21-3746524048-3196343125-3229652583-520/newdomadm) which is modified
to S-1-5-21-3746524048-3196343125-3229652583-512/newdomadm. This should not
need to be corrected if we had 1 SID <=> 1 Gid, but 2) and 3) allow this
error.
* Patch :
Here is a small patch for bugs #2, #3, and the half of #5 !
--------------------------------------------------------- beginning of
pdb_ldap.c.patch ---------------------------------------------------------
--- passdb/pdb_ldap.c.orig 2004-01-06 22:08:40.000000000 +0100
+++ passdb/pdb_ldap.c 2004-01-19 12:44:41.000000000 +0100
@@ -1910,10 +1910,16 @@
if (NT_STATUS_IS_OK(ldapsam_getgrgid(methods, &dummy,
map->gid))) {
- DEBUG(0, ("ldapsam_add_group_mapping_entry: Group %ld
already exists in LDAP\n", (unsigned long)map->gid));
+ DEBUG(0, ("ldapsam_add_group_mapping_entry: Unix group %ld
already mapped in LDAP\n", (unsigned long)map->gid));
return NT_STATUS_UNSUCCESSFUL;
}
+ if (NT_STATUS_IS_OK(ldapsam_getgrsid(methods, &dummy,
+ map->sid))) {
+ DEBUG(0, ("ldapsam_add_group_mapping_entry: SID already
mapped in LDAP\n"));
+ return NT_STATUS_UNSUCCESSFUL;
+ }
+
rc = ldapsam_search_one_group_by_gid(ldap_state, map->gid,
&result);
if (rc != LDAP_SUCCESS) {
ldap_msgfree(result);
@@ -1991,6 +1997,14 @@
LDAPMessage *entry = NULL;
LDAPMod **mods = NULL;
+ GROUP_MAP dummy;
+
+ if (NT_STATUS_IS_OK(ldapsam_getgrgid(methods, &dummy,
+ map->gid))) {
+ DEBUG(0, ("ldapsam_update_group_mapping_entry: Unix group %ld
already mapped in LDAP\n", (unsigned long)map->gid));
+ return NT_STATUS_UNSUCCESSFUL;
+ }
+
rc = ldapsam_search_one_group_by_gid(ldap_state, map->gid,
&result);
if (rc != LDAP_SUCCESS) {
--------------------------------------------------------- end of
pdb_ldap.c.patch ---------------------------------------------------------
Note : I also modified debug info to make them more explicit...
* Final note
Hope this helps, any question/comment welcome !
Ganael LAPLANCHE
ganael.laplanche at martymac.com
http://www.martymac.com
More information about the samba
mailing list