New approach to "valid users" fix
Gerald (Jerry) Carter
jerry at samba.org
Fri Aug 11 19:01:38 GMT 2006
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Gerald (Jerry) Carter wrote:
> Jeremy and Volker,
>
> I think we've been dealing with the "valid users" problem
> in smb.conf the wrong way. The latest bug report made
> it pretty obvious. The name2sid() and gid2sid() methods
> are not symmetric.
New patch. Alternative approach yielding the same solution.
The main reason that we have struggled with 3.0.23 is that
we ignored smbpasswd. The code for tdbsam and ldapsam
has been solid. Therefore, this patch goes the distance
and removes algorithmic mapping altogether.
pdb_rid_algorithm() is really just a checked now to see if
a backend can allocate RIDs.
The danger is that not the security descriptor issue
we were so concerned about (that no on has complained
about) will now be more widespread. But the upside
is that you can always move from a name, uid/gid, or a
SID to another other the other representations and be
guaranteed to have a reversible mapping.
The real problem was that smbpasswd was the exception.
This patch fixes the token creation by mainly throwing
away algorithmic RIDs for non-mapped groups in gid_to_sid().
The lookup_name() can also throw away the LOOKUP_NAME_GROUP
check because we always qualify the name coming from
lookup_name_smbconf().
As an example, this is a domain user's token from a
Samba PDC. Notice Domain Users, BUILTIN\users, and
Unix Group\users. This is an LDAP backend.
NT user token of user S-1-5-21-2547222302-1596225915-2414751004-2560
contains 13 SIDs
SID[ 0]: S-1-5-21-2547222302-1596225915-2414751004-2560
SID[ 1]: S-1-22-2-100
SID[ 2]: S-1-1-0
SID[ 3]: S-1-5-2
SID[ 4]: S-1-5-11
SID[ 5]: S-1-5-21-2547222302-1596225915-2414751004-3003
SID[ 6]: S-1-5-21-2547222302-1596225915-2414751004-512
SID[ 7]: S-1-5-21-2547222302-1596225915-2414751004-513
SID[ 8]: S-1-22-2-1042
SID[ 9]: S-1-22-2-1046
SID[ 10]: S-1-22-2-1047
SID[ 11]: S-1-5-32-544
SID[ 12]: S-1-5-32-545
The smbpasswd case is similar.
NT user token of user S-1-5-21-391507597-2097566357-2340928898-3008
contains 7 SIDs
SID[ 0]: S-1-5-21-391507597-2097566357-2340928898-3008
SID[ 1]: S-1-22-2-100
SID[ 2]: S-1-1-0
SID[ 3]: S-1-5-2
SID[ 4]: S-1-5-11
SID[ 5]: S-1-22-2-1042
SID[ 6]: S-1-22-2-1045
I've sent the patch out to a few people on the samba ml
for testing. Other feedback welcome.
cheers, jerry
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.4 (MingW32)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org
iD8DBQFE3NQSIR7qMdg1EfYRAiG5AJoCD8CEyyr+Ff5FaeWaWfZP+TbarACgwcac
4QOat2JZK7HAfu4Ib6t3tMw=
=ylr2
-----END PGP SIGNATURE-----
-------------- next part --------------
Index: groupdb/mapping.c
===================================================================
--- groupdb/mapping.c (revision 17493)
+++ groupdb/mapping.c (working copy)
@@ -195,7 +195,7 @@
fstrcpy(map.nt_name, grpname);
if (pdb_rid_algorithm()) {
- rid = pdb_gid_to_group_rid( grp->gr_gid );
+ rid = algorithmic_pdb_gid_to_group_rid( grp->gr_gid );
} else {
if (!pdb_new_rid(&rid)) {
DEBUG(3, ("Could not get a new RID for %s\n",
Index: passdb/util_unixsids.c
===================================================================
--- passdb/util_unixsids.c (revision 17493)
+++ passdb/util_unixsids.c (working copy)
@@ -42,6 +42,12 @@
return sid_append_rid(sid, uid);
}
+BOOL uid_to_unix_groups_sid(gid_t gid, DOM_SID *sid)
+{
+ sid_copy(sid, &global_sid_Unix_Groups);
+ return sid_append_rid(sid, gid);
+}
+
const char *unix_users_domain_name(void)
{
return "Unix User";
Index: passdb/lookup_sid.c
===================================================================
--- passdb/lookup_sid.c (revision 17493)
+++ passdb/lookup_sid.c (working copy)
@@ -43,7 +43,6 @@
DOM_SID sid;
enum SID_NAME_USE type;
TALLOC_CTX *tmp_ctx = talloc_new(mem_ctx);
- struct group *grp;
if (tmp_ctx == NULL) {
DEBUG(0, ("talloc_new failed\n"));
@@ -120,63 +119,6 @@
goto failed;
}
- /*
- * Nasty hack necessary for too common scenarios:
- *
- * For 'valid users = +users' we know "users" is most probably not
- * BUILTIN\users but the unix group users. This hack requires the
- * admin to explicitly qualify BUILTIN if BUILTIN\users is meant.
- *
- * Please note that LOOKUP_NAME_GROUP can not be requested via for
- * example lsa_lookupnames, it only comes into this routine via
- * the expansion of group names coming in from smb.conf
- */
-
- if ((flags & LOOKUP_NAME_GROUP) && ((grp = getgrnam(name)) != NULL)) {
-
- GROUP_MAP map;
-
- if (pdb_getgrgid(&map, grp->gr_gid)) {
- /* The hack gets worse. Handle the case where we have
- * 'force group = +unixgroup' but "unixgroup" has a
- * group mapping */
-
- if (sid_check_is_in_builtin(&map.sid)) {
- domain = talloc_strdup(
- tmp_ctx, builtin_domain_name());
- } else {
- domain = talloc_strdup(
- tmp_ctx, get_global_sam_name());
- }
-
- sid_copy(&sid, &map.sid);
- type = map.sid_name_use;
- goto ok;
- }
-
- /* If we are using the smbpasswd backend, we need to use the
- * algorithmic mapping for the unix group we find. This is
- * necessary because when creating the NT token from the unix
- * gid list we got from initgroups() we use gid_to_sid() that
- * uses algorithmic mapping if pdb_rid_algorithm() is true. */
-
- if (pdb_rid_algorithm() &&
- (grp->gr_gid < max_algorithmic_gid())) {
- domain = talloc_strdup(tmp_ctx, get_global_sam_name());
- sid_compose(&sid, get_global_sam_sid(),
- pdb_gid_to_group_rid(grp->gr_gid));
- type = SID_NAME_DOM_GRP;
- goto ok;
- }
-
- if (lookup_unix_group_name(name, &sid)) {
- domain = talloc_strdup(tmp_ctx,
- unix_groups_domain_name());
- type = SID_NAME_DOM_GRP;
- goto ok;
- }
- }
-
/* Now the guesswork begins, we haven't been given an explicit
* domain. Try the sequence as documented on
* http://msdn.microsoft.com/library/en-us/secmgmt/security/lsalookupnames.asp
@@ -1138,15 +1080,10 @@
goto done;
}
- if (pdb_rid_algorithm() && (uid < max_algorithmic_uid())) {
- sid_copy(psid, get_global_sam_sid());
- sid_append_rid(psid, algorithmic_pdb_uid_to_user_rid(uid));
- goto done;
- } else {
- uid_to_unix_users_sid(uid, psid);
- goto done;
- }
+ /* This is an unmapped user */
+ uid_to_unix_users_sid(uid, psid);
+
done:
DEBUG(10,("uid_to_sid: local %u -> %s\n", (unsigned int)uid,
sid_string_static(psid)));
@@ -1180,16 +1117,10 @@
/* This is a mapped group */
goto done;
}
+
+ /* This is an unmapped group */
- if (pdb_rid_algorithm() && (gid < max_algorithmic_gid())) {
- sid_copy(psid, get_global_sam_sid());
- sid_append_rid(psid, pdb_gid_to_group_rid(gid));
- goto done;
- } else {
- sid_copy(psid, &global_sid_Unix_Groups);
- sid_append_rid(psid, gid);
- goto done;
- }
+ uid_to_unix_groups_sid(gid, psid);
done:
DEBUG(10,("gid_to_sid: local %u -> %s\n", (unsigned int)gid,
@@ -1235,14 +1166,9 @@
*puid = id.uid;
goto done;
}
- if (pdb_rid_algorithm() &&
- algorithmic_pdb_rid_is_user(rid)) {
- *puid = algorithmic_pdb_user_rid_to_uid(rid);
- goto done;
- }
- /* This was ours, but it was neither mapped nor
- * algorithmic. Fail */
+ /* This was ours, but it was not mapped. Fail */
+
return False;
}
@@ -1323,14 +1249,9 @@
*pgid = id.gid;
goto done;
}
- if (pdb_rid_algorithm() &&
- !algorithmic_pdb_rid_is_user(rid)) {
- /* This must be a group, presented as alias */
- *pgid = pdb_group_rid_to_gid(rid);
- goto done;
- }
- /* This was ours, but it was neither mapped nor
- * algorithmic. Fail. */
+
+ /* This was ours, but it was not mapped. Fail */
+
return False;
}
Index: passdb/passdb.c
===================================================================
--- passdb/passdb.c (revision 17493)
+++ passdb/passdb.c (working copy)
@@ -505,7 +505,7 @@
there is not anymore a direct link between the gid and the rid.
********************************************************************/
-uint32 pdb_gid_to_group_rid(gid_t gid)
+uint32 algorithmic_pdb_gid_to_group_rid(gid_t gid)
{
int rid_offset = algorithmic_rid_base();
return (((((uint32)gid)*RID_MULTIPLIER) + rid_offset) | GROUP_RID_TYPE);
Index: passdb/pdb_interface.c
===================================================================
--- passdb/pdb_interface.c (revision 17493)
+++ passdb/pdb_interface.c (working copy)
@@ -595,7 +595,7 @@
}
if (pdb_rid_algorithm()) {
- *rid = pdb_gid_to_group_rid( grp->gr_gid );
+ *rid = algorithmic_pdb_gid_to_group_rid( grp->gr_gid );
} else {
if (!pdb_new_rid(rid)) {
return NT_STATUS_ACCESS_DENIED;
Index: include/smb.h
===================================================================
--- include/smb.h (revision 17493)
+++ include/smb.h (working copy)
@@ -272,7 +272,7 @@
#define LOOKUP_NAME_REMOTE 2 /* Ask others */
#define LOOKUP_NAME_ALL (LOOKUP_NAME_ISOLATED|LOOKUP_NAME_REMOTE)
-#define LOOKUP_NAME_GROUP 4 /* This is a NASTY hack for valid users = @foo
+#define LOOKUP_NAME_GROUP 4 /* (unused) This is a NASTY hack for valid users = @foo
* where foo also exists in as user. */
/**
Index: utils/net_groupmap.c
===================================================================
--- utils/net_groupmap.c (revision 17493)
+++ utils/net_groupmap.c (working copy)
@@ -275,7 +275,7 @@
if ( (rid == 0) && (string_sid[0] == '\0') ) {
d_printf("No rid or sid specified, choosing a RID\n");
if (pdb_rid_algorithm()) {
- rid = pdb_gid_to_group_rid(gid);
+ rid = algorithmic_pdb_gid_to_group_rid(gid);
} else {
if (!pdb_new_rid(&rid)) {
d_printf("Could not get new RID\n");
@@ -555,7 +555,14 @@
map.gid = grp->gr_gid;
if (opt_rid == 0) {
- opt_rid = pdb_gid_to_group_rid(map.gid);
+ if ( pdb_rid_algorithm() )
+ opt_rid = algorithmic_pdb_gid_to_group_rid(map.gid);
+ else {
+ if ( !pdb_new_rid((uint32*)&opt_rid) ) {
+ d_fprintf( stderr, "Could not allocate new RID\n");
+ return -1;
+ }
+ }
}
sid_copy(&map.sid, get_global_sam_sid());
More information about the samba-technical
mailing list