Read and write list ordering
Uri Simchoni
uri at samba.org
Sat Jul 16 18:55:33 UTC 2016
On 07/16/2016 02:46 AM, Justin Maggard wrote:
> I'd like to get other people's opinions on this.
>
> Currently, objects in the write list always override the same objects
> in the read list.
>
> This includes cases where you have an individual user account in the
> read list, and a group he belongs to in the write list. This seems
> unintuitive, as most of us are used to user account settings taking
> precedence over group account settings.
>
> The attached patch keeps track of the object type, and gives
> preference to user account objects if they exist.
>
> Thoughts?
>
> Thanks,
> -Justin
>
If I were to design an authorization system from scratch, I would agree
with you. If the admin has gone through the trouble of specifying what
this user's access is, then the SMB server should look no further.
However, the patch would break compatibility with existing setups, so
such a change must be brought in a way that doesn't change the behavior
of any existing setup.
Please note that this also breaks compatibility with Windows because in
Windows, everything is a SID, there's no special weight to user-specific
rules.
One idea of how to do this is a via a new smb.conf share parameter which
is a list of user:perm tuples. A perm can be "rw/ro/deny" or something
like that. This list can be processed before the other lists, and
whatever "verdict" found there is final (also stops on first match).
If people object to adding this (e.g. on account of "too many competing
features"), I also think it's possible to keep this as a private patch
that only touches upon share_access.c, and hence has little maintenance
cost, see attached.
Thanks,
Uri.
-------------- next part --------------
From dedd4bd56fdf1dafc071646dfc46dfbd7d4193b3 Mon Sep 17 00:00:00 2001
From: Uri Simchoni <uri at samba.org>
Date: Sat, 16 Jul 2016 21:48:37 +0300
Subject: [PATCH] smbd: modify share access rule precedence
When evaluating share access rules, evaluate user-spcific
rule first and consider them as "terminal" if they apply.
---
source3/smbd/share_access.c | 126 +++++++++++++++++++++++++++++++++-----------
1 file changed, 95 insertions(+), 31 deletions(-)
diff --git a/source3/smbd/share_access.c b/source3/smbd/share_access.c
index fa56063..2a5367e 100644
--- a/source3/smbd/share_access.c
+++ b/source3/smbd/share_access.c
@@ -67,6 +67,17 @@ static bool do_group_checks(const char **name, const char **pattern)
return False;
}
+static bool is_principal_group(const char *name)
+{
+ if (name[0] == '@' || (name[0] == '+' && name[1] == '&') ||
+ name[0] == '+' || (name[0] == '&' && name[1] == '+') ||
+ name[0] == '&') {
+ return true;
+ }
+
+ return false;
+}
+
static bool token_contains_name(TALLOC_CTX *mem_ctx,
const char *username,
const char *domain,
@@ -155,31 +166,51 @@ static bool token_contains_name(TALLOC_CTX *mem_ctx,
*
* The other use is the netgroup check when using @group or &group.
*/
-
-bool token_contains_name_in_list(const char *username,
- const char *domain,
- const char *sharename,
- const struct security_token *token,
- const char **list)
+#define PRINCIPAL_TYPE_USER 1
+#define PRINCIPAL_TYPE_GROUP 2
+#define PRINCIPAL_TYPE_ALL (PRINCIPAL_TYPE_USER | PRINCIPAL_TYPE_GROUP)
+
+static bool token_contains_name_in_list_internal(
+ const char *username, const char *domain, const char *sharename,
+ const struct security_token *token, const char **list,
+ unsigned principals_mask)
{
if (list == NULL) {
return False;
}
- while (*list != NULL) {
- TALLOC_CTX *frame = talloc_stackframe();
+ for (;*list != NULL; ++list) {
+ TALLOC_CTX *frame;
bool ret;
+ bool is_group = is_principal_group(*list);
+ if (is_group && !(principals_mask & PRINCIPAL_TYPE_GROUP)) {
+ continue;
+ }
+
+ if (!is_group && !(principals_mask & PRINCIPAL_TYPE_USER)) {
+ continue;
+ }
+
+ frame = talloc_stackframe();
ret = token_contains_name(frame, username, domain, sharename,
token, *list);
TALLOC_FREE(frame);
if (ret) {
return true;
}
- list += 1;
}
return False;
}
+bool token_contains_name_in_list(const char *username, const char *domain,
+ const char *sharename,
+ const struct security_token *token,
+ const char **list)
+{
+ return token_contains_name_in_list_internal(
+ username, domain, sharename, token, list, PRINCIPAL_TYPE_ALL);
+}
+
/*
* Check whether the user described by "token" has access to share snum.
*
@@ -197,23 +228,38 @@ bool user_ok_token(const char *username, const char *domain,
const struct security_token *token, int snum)
{
if (lp_invalid_users(snum) != NULL) {
- if (token_contains_name_in_list(username, domain,
- lp_servicename(talloc_tos(), snum),
- token,
- lp_invalid_users(snum))) {
+ if (token_contains_name_in_list_internal(
+ username, domain, lp_servicename(talloc_tos(), snum),
+ token, lp_invalid_users(snum), PRINCIPAL_TYPE_USER)) {
DEBUG(10, ("User %s in 'invalid users'\n", username));
- return False;
+ return false;
}
}
if (lp_valid_users(snum) != NULL) {
- if (!token_contains_name_in_list(username, domain,
- lp_servicename(talloc_tos(), snum),
- token,
- lp_valid_users(snum))) {
- DEBUG(10, ("User %s not in 'valid users'\n",
- username));
- return False;
+ if (token_contains_name_in_list_internal(
+ username, domain, lp_servicename(talloc_tos(), snum),
+ token, lp_valid_users(snum), PRINCIPAL_TYPE_USER)) {
+ DEBUG(10, ("User %s in 'valid users'\n", username));
+ return true;
+ }
+ }
+
+ if (lp_invalid_users(snum) != NULL) {
+ if (token_contains_name_in_list_internal(
+ username, domain, lp_servicename(talloc_tos(), snum),
+ token, lp_invalid_users(snum), PRINCIPAL_TYPE_GROUP)) {
+ DEBUG(10, ("User %s in 'invalid users'\n", username));
+ return false;
+ }
+ }
+
+ if (lp_valid_users(snum) != NULL) {
+ if (!token_contains_name_in_list_internal(
+ username, domain, lp_servicename(talloc_tos(), snum),
+ token, lp_valid_users(snum), PRINCIPAL_TYPE_GROUP)) {
+ DEBUG(10, ("User %s not in 'valid users'\n", username));
+ return false;
}
}
@@ -245,24 +291,42 @@ bool is_share_read_only_for_token(const char *username,
int snum = SNUM(conn);
bool result = conn->read_only;
+ if (lp_write_list(snum) != NULL) {
+ if (token_contains_name_in_list_internal(
+ username, domain, lp_servicename(talloc_tos(), snum),
+ token, lp_write_list(snum), PRINCIPAL_TYPE_USER)) {
+ result = false;
+ goto done;
+ }
+ }
+
if (lp_read_list(snum) != NULL) {
- if (token_contains_name_in_list(username, domain,
- lp_servicename(talloc_tos(), snum),
- token,
- lp_read_list(snum))) {
- result = True;
+ if (token_contains_name_in_list_internal(
+ username, domain, lp_servicename(talloc_tos(), snum),
+ token, lp_read_list(snum), PRINCIPAL_TYPE_USER)) {
+ result = true;
+ goto done;
}
}
if (lp_write_list(snum) != NULL) {
- if (token_contains_name_in_list(username, domain,
- lp_servicename(talloc_tos(), snum),
- token,
- lp_write_list(snum))) {
- result = False;
+ if (token_contains_name_in_list_internal(
+ username, domain, lp_servicename(talloc_tos(), snum),
+ token, lp_write_list(snum), PRINCIPAL_TYPE_GROUP)) {
+ result = false;
+ goto done;
+ }
+ }
+
+ if (lp_read_list(snum) != NULL) {
+ if (token_contains_name_in_list_internal(
+ username, domain, lp_servicename(talloc_tos(), snum),
+ token, lp_read_list(snum), PRINCIPAL_TYPE_GROUP)) {
+ result = true;
}
}
+done:
DEBUG(10,("is_share_read_only_for_user: share %s is %s for unix user "
"%s\n", lp_servicename(talloc_tos(), snum),
result ? "read-only" : "read-write", username));
--
2.5.5
More information about the samba-technical
mailing list