[PATCH] Master fix for bug #9518 - conn->share_access appears not be be reset between users
Jeremy Allison
jra at samba.org
Tue Jan 8 13:14:45 MST 2013
On Tue, Jan 08, 2013 at 10:38:40PM +1100, Andrew Bartlett wrote:
> On Tue, 2013-01-08 at 18:33 +1100, Andrew Bartlett wrote:
> > On Tue, 2013-01-08 at 08:09 +0100, Stefan (metze) Metzmacher wrote:
> > > Am 06.01.2013 15:13, schrieb Volker Lendecke:
> > > > On Fri, Jan 04, 2013 at 04:12:55PM -0800, Jeremy Allison wrote:
> > > >> Please review and apply to master if ok. It includes
> > > >> Andrew's regression test patch also.
> > > >
> > > > Looks very well crafted to me. I haven't tested it, but it
> > > > looks completely sane. Attached find some cosmetic patches
> > > > on top.
> > > >
> > > > With best regards,
> > >
> > > Reviewed-by: Stefan Metzmacher <metze at samba.org>
> >
> > Thanks. All these (Volker's cosmetic patches included) are now on their
> > way to autobuild.
>
> Jermey,
>
> I've got bad news sorry.
>
> source3/printing/nt_printing.c:get_correct_cversion() calls
> become_user_by_session() which uses UID_FIELD_INVALID.
>
> This means that on sn-devel (but not on my Fedora system!) we segfault:
Ok, can you try this revised version. It's the same as the previous
patchset, with Volker's cleanup fixes included, plus this additional
change which should cope with the special become_user_by_session()
case without having to restore all the UID_FIELD_INVALID code.
I'll upload this to the bug report, and also revise the 4.0.x
patchset on the bug report.
Thanks a *lot* for catching that use-case. As you can see,
this code is really tricky to get right (holdovers from having
to support "force user" and "force group" correctly).
Here is the additional change as text so you can see what I did
here:
-----------------------------------------------------------------------
Author: Jeremy Allison <jra at samba.org>
Date: Tue Jan 8 11:02:16 2013 -0800
Fixup the change_to_user_by_session() case as called from become_user_by_session()
Use inside source3/printing/nt_printing.c:get_correct_cversion().
Allow check_user_ok() to be called with vuid==UID_FIELD_INVALID.
All this should do is throw away one entry in the vuid cache.
Signed-off-by: Jeremy Allison <jra at samba.org>
diff --git a/source3/smbd/uid.c b/source3/smbd/uid.c
index b2fe39c..a795eef 100644
--- a/source3/smbd/uid.c
+++ b/source3/smbd/uid.c
@@ -191,6 +191,13 @@ static bool check_user_ok(connection_struct *conn,
for (i=0; i<VUID_CACHE_SIZE; i++) {
ent = &conn->vuid_cache->array[i];
if (ent->vuid == vuid) {
+ if (vuid == UID_FIELD_INVALID) {
+ /*
+ * Slow path, we don't care
+ * about the array traversal.
+ */
+ continue;
+ }
free_conn_session_info_if_unused(conn);
conn->session_info = ent->session_info;
conn->read_only = ent->read_only;
@@ -232,11 +239,26 @@ static bool check_user_ok(connection_struct *conn,
return false;
}
+ /*
+ * It's actually OK to call check_user_ok() with
+ * vuid == UID_FIELD_INVALID as called from change_to_user_by_session().
+ * All this will do is throw away one entry in the cache.
+ */
+
ent->vuid = vuid;
ent->read_only = readonly_share;
ent->share_access = share_access;
free_conn_session_info_if_unused(conn);
conn->session_info = ent->session_info;
+ if (vuid == UID_FIELD_INVALID) {
+ /*
+ * Not strictly needed, just make it really
+ * clear this entry is actually an unused one.
+ */
+ ent->read_only = false;
+ ent->share_access = 0;
+ ent->session_info = NULL;
+ }
conn->read_only = readonly_share;
conn->share_access = share_access;
-------------- next part --------------
>From 07e43c55ed018228153d0e4a20bd212ad94e6489 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Thu, 20 Dec 2012 11:50:25 -0800
Subject: [PATCH 01/23] Start to tidy-up check_user_ok().
Now we have removed "security=share" we cannot be
called with vuid == UID_FIELD_INVALID.
Signed-off-by: Jeremy Allison <jra at samba.org>
---
source3/smbd/uid.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/source3/smbd/uid.c b/source3/smbd/uid.c
index 30c7154..2aa9fff 100644
--- a/source3/smbd/uid.c
+++ b/source3/smbd/uid.c
@@ -90,12 +90,11 @@ static bool check_user_ok(connection_struct *conn,
const struct auth_session_info *session_info,
int snum)
{
- bool valid_vuid = (vuid != UID_FIELD_INVALID);
unsigned int i;
bool readonly_share;
bool admin_user;
- if (valid_vuid) {
+ {
struct vuid_cache_entry *ent;
for (i=0; i<VUID_CACHE_SIZE; i++) {
@@ -145,7 +144,7 @@ static bool check_user_ok(connection_struct *conn,
session_info->info->domain_name,
NULL, session_info->security_token, lp_admin_users(snum));
- if (valid_vuid) {
+ {
struct vuid_cache_entry *ent =
&conn->vuid_cache.array[conn->vuid_cache.next_entry];
--
1.8.1
>From 22d84b3f979184f2639fc81101a1a178b28060f2 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Thu, 20 Dec 2012 11:51:55 -0800
Subject: [PATCH 02/23] Move the definition of struct vuid_cache_entry *ent
outside blocks.
Signed-off-by: Jeremy Allison <jra at samba.org>
---
source3/smbd/uid.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/source3/smbd/uid.c b/source3/smbd/uid.c
index 2aa9fff..9361e49 100644
--- a/source3/smbd/uid.c
+++ b/source3/smbd/uid.c
@@ -93,10 +93,9 @@ static bool check_user_ok(connection_struct *conn,
unsigned int i;
bool readonly_share;
bool admin_user;
+ struct vuid_cache_entry *ent = NULL;
{
- struct vuid_cache_entry *ent;
-
for (i=0; i<VUID_CACHE_SIZE; i++) {
ent = &conn->vuid_cache.array[i];
if (ent->vuid == vuid) {
@@ -145,8 +144,7 @@ static bool check_user_ok(connection_struct *conn,
NULL, session_info->security_token, lp_admin_users(snum));
{
- struct vuid_cache_entry *ent =
- &conn->vuid_cache.array[conn->vuid_cache.next_entry];
+ ent = &conn->vuid_cache.array[conn->vuid_cache.next_entry];
conn->vuid_cache.next_entry =
(conn->vuid_cache.next_entry + 1) % VUID_CACHE_SIZE;
--
1.8.1
>From bdf826992edf5bc2bfde3bc5fbcb0a0bfdf7a257 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Thu, 20 Dec 2012 11:52:27 -0800
Subject: [PATCH 03/23] Remove one set of enclosing {} braces, no longer
needed.
Signed-off-by: Jeremy Allison <jra at samba.org>
---
source3/smbd/uid.c | 16 +++++++---------
1 file changed, 7 insertions(+), 9 deletions(-)
diff --git a/source3/smbd/uid.c b/source3/smbd/uid.c
index 9361e49..9109f05 100644
--- a/source3/smbd/uid.c
+++ b/source3/smbd/uid.c
@@ -95,15 +95,13 @@ static bool check_user_ok(connection_struct *conn,
bool admin_user;
struct vuid_cache_entry *ent = NULL;
- {
- for (i=0; i<VUID_CACHE_SIZE; i++) {
- ent = &conn->vuid_cache.array[i];
- if (ent->vuid == vuid) {
- free_conn_session_info_if_unused(conn);
- conn->session_info = ent->session_info;
- conn->read_only = ent->read_only;
- return(True);
- }
+ for (i=0; i<VUID_CACHE_SIZE; i++) {
+ ent = &conn->vuid_cache.array[i];
+ if (ent->vuid == vuid) {
+ free_conn_session_info_if_unused(conn);
+ conn->session_info = ent->session_info;
+ conn->read_only = ent->read_only;
+ return(True);
}
}
--
1.8.1
>From ff7d8ddce0d667bf9b9021fd8dcff36da15b1ace Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Thu, 20 Dec 2012 11:53:11 -0800
Subject: [PATCH 04/23] Remove the second set of {} braces, no longer needed.
---
source3/smbd/uid.c | 38 ++++++++++++++++++--------------------
1 file changed, 18 insertions(+), 20 deletions(-)
diff --git a/source3/smbd/uid.c b/source3/smbd/uid.c
index 9109f05..9fc5d7c 100644
--- a/source3/smbd/uid.c
+++ b/source3/smbd/uid.c
@@ -141,33 +141,31 @@ static bool check_user_ok(connection_struct *conn,
session_info->info->domain_name,
NULL, session_info->security_token, lp_admin_users(snum));
- {
- ent = &conn->vuid_cache.array[conn->vuid_cache.next_entry];
+ ent = &conn->vuid_cache.array[conn->vuid_cache.next_entry];
- conn->vuid_cache.next_entry =
- (conn->vuid_cache.next_entry + 1) % VUID_CACHE_SIZE;
+ conn->vuid_cache.next_entry =
+ (conn->vuid_cache.next_entry + 1) % VUID_CACHE_SIZE;
- TALLOC_FREE(ent->session_info);
+ TALLOC_FREE(ent->session_info);
- /*
- * If force_user was set, all session_info's are based on the same
- * username-based faked one.
- */
-
- ent->session_info = copy_session_info(
- conn, conn->force_user ? conn->session_info : session_info);
+ /*
+ * If force_user was set, all session_info's are based on the same
+ * username-based faked one.
+ */
- if (ent->session_info == NULL) {
- ent->vuid = UID_FIELD_INVALID;
- return false;
- }
+ ent->session_info = copy_session_info(
+ conn, conn->force_user ? conn->session_info : session_info);
- ent->vuid = vuid;
- ent->read_only = readonly_share;
- free_conn_session_info_if_unused(conn);
- conn->session_info = ent->session_info;
+ if (ent->session_info == NULL) {
+ ent->vuid = UID_FIELD_INVALID;
+ return false;
}
+ ent->vuid = vuid;
+ ent->read_only = readonly_share;
+ free_conn_session_info_if_unused(conn);
+ conn->session_info = ent->session_info;
+
conn->read_only = readonly_share;
if (admin_user) {
DEBUG(2,("check_user_ok: user %s is an admin user. "
--
1.8.1
>From e12713048518a306113534759e73ba95c2235dbd Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Thu, 20 Dec 2012 11:54:07 -0800
Subject: [PATCH 05/23] Remove dead code now vuser can no longer be NULL.
Signed-off-by: Jeremy Allison <jra at samba.org>
---
source3/smbd/uid.c | 8 --------
1 file changed, 8 deletions(-)
diff --git a/source3/smbd/uid.c b/source3/smbd/uid.c
index 9fc5d7c..298fa83 100644
--- a/source3/smbd/uid.c
+++ b/source3/smbd/uid.c
@@ -299,14 +299,6 @@ bool change_to_user(connection_struct *conn, uint64_t vuid)
}
session_info = vuser->session_info;
-
- if (!conn->force_user && vuser == NULL) {
- DEBUG(2,("Invalid vuid used %llu in accessing share %s.\n",
- (unsigned long long)vuid,
- lp_servicename(talloc_tos(), snum)));
- return False;
- }
-
return change_to_user_internal(conn, session_info, vuid);
}
--
1.8.1
>From 7b68212331fbac8b8331c537daf49e6eb7592140 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Thu, 20 Dec 2012 11:55:09 -0800
Subject: [PATCH 06/23] Remove unneeded variable "const struct
auth_session_info *session_info"
Signed-off-by: Jeremy Allison <jra at samba.org>
---
source3/smbd/uid.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/source3/smbd/uid.c b/source3/smbd/uid.c
index 298fa83..9244f29 100644
--- a/source3/smbd/uid.c
+++ b/source3/smbd/uid.c
@@ -271,7 +271,6 @@ static bool change_to_user_internal(connection_struct *conn,
bool change_to_user(connection_struct *conn, uint64_t vuid)
{
- const struct auth_session_info *session_info = NULL;
struct user_struct *vuser;
int snum = SNUM(conn);
@@ -298,8 +297,7 @@ bool change_to_user(connection_struct *conn, uint64_t vuid)
return false;
}
- session_info = vuser->session_info;
- return change_to_user_internal(conn, session_info, vuid);
+ return change_to_user_internal(conn, vuser->session_info, vuid);
}
static bool change_to_user_by_session(connection_struct *conn,
--
1.8.1
>From 1c4b287dbf90f3be66432116fd70338525294b00 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Thu, 20 Dec 2012 14:42:55 -0800
Subject: [PATCH 07/23] Clean up struct connection_struct, make struct
vuid_cache a pointer not inline.
Change VFS ABI to 31 for 4.1.0.
Signed-off-by: Jeremy Allison <jra at samba.org>
---
source3/include/vfs.h | 8 ++++++--
source3/modules/vfs_readonly.c | 4 ++--
source3/smbd/conn.c | 3 ++-
source3/smbd/uid.c | 10 +++++-----
4 files changed, 15 insertions(+), 10 deletions(-)
diff --git a/source3/include/vfs.h b/source3/include/vfs.h
index 2992c1d..7da022e 100644
--- a/source3/include/vfs.h
+++ b/source3/include/vfs.h
@@ -147,7 +147,11 @@
/* Bump to version 30 - Samba 4.0.0 will ship with interface version 30 */
/* Leave at 30 - not yet released. Added conn->cwd to save vfs_GetWd() calls. */
/* Leave at 30 - not yet released. Changed sys_acl_blob_get_file interface to remove type */
-#define SMB_VFS_INTERFACE_VERSION 30
+/* Bump to version 31 - Samba 4.1.0 will ship with interface version 31 */
+/* Leave at 31 - not yet released. Make struct vuid_cache_entry in
+ connection_struct a pointer. */
+
+#define SMB_VFS_INTERFACE_VERSION 31
/*
All intercepted VFS operations must be declared as static functions inside module source
@@ -306,7 +310,7 @@ typedef struct connection_struct {
uint32_t cnum; /* an index passed over the wire */
struct share_params *params;
bool force_user;
- struct vuid_cache vuid_cache;
+ struct vuid_cache *vuid_cache;
bool printer;
bool ipc;
bool read_only; /* Attributes for the current user of the share. */
diff --git a/source3/modules/vfs_readonly.c b/source3/modules/vfs_readonly.c
index 7919dbc..f75db09 100644
--- a/source3/modules/vfs_readonly.c
+++ b/source3/modules/vfs_readonly.c
@@ -82,12 +82,12 @@ static int readonly_connect(vfs_handle_struct *handle,
/* Wipe out the VUID cache. */
for (i=0; i< VUID_CACHE_SIZE; i++) {
- struct vuid_cache_entry *ent = &conn->vuid_cache.array[i];
+ struct vuid_cache_entry *ent = &conn->vuid_cache->array[i];
ent->vuid = UID_FIELD_INVALID;
TALLOC_FREE(ent->session_info);
ent->read_only = false;
}
- conn->vuid_cache.next_entry = 0;
+ conn->vuid_cache->next_entry = 0;
}
return 0;
diff --git a/source3/smbd/conn.c b/source3/smbd/conn.c
index bc5a03b..1d4444f 100644
--- a/source3/smbd/conn.c
+++ b/source3/smbd/conn.c
@@ -63,6 +63,7 @@ connection_struct *conn_new(struct smbd_server_connection *sconn)
if (!(conn=talloc_zero(NULL, connection_struct)) ||
!(conn->params = talloc(conn, struct share_params)) ||
+ !(conn->vuid_cache = talloc_zero(conn, struct vuid_cache)) ||
!(conn->connectpath = talloc_strdup(conn, "")) ||
!(conn->origpath = talloc_strdup(conn, ""))) {
DEBUG(0,("TALLOC_ZERO() failed!\n"));
@@ -89,7 +90,7 @@ static void conn_clear_vuid_cache(connection_struct *conn, uint64_t vuid)
for (i=0; i<VUID_CACHE_SIZE; i++) {
struct vuid_cache_entry *ent;
- ent = &conn->vuid_cache.array[i];
+ ent = &conn->vuid_cache->array[i];
if (ent->vuid == vuid) {
ent->vuid = UID_FIELD_INVALID;
diff --git a/source3/smbd/uid.c b/source3/smbd/uid.c
index 9244f29..f9b5716 100644
--- a/source3/smbd/uid.c
+++ b/source3/smbd/uid.c
@@ -68,7 +68,7 @@ static void free_conn_session_info_if_unused(connection_struct *conn)
for (i = 0; i < VUID_CACHE_SIZE; i++) {
struct vuid_cache_entry *ent;
- ent = &conn->vuid_cache.array[i];
+ ent = &conn->vuid_cache->array[i];
if (ent->vuid != UID_FIELD_INVALID &&
conn->session_info == ent->session_info) {
return;
@@ -96,7 +96,7 @@ static bool check_user_ok(connection_struct *conn,
struct vuid_cache_entry *ent = NULL;
for (i=0; i<VUID_CACHE_SIZE; i++) {
- ent = &conn->vuid_cache.array[i];
+ ent = &conn->vuid_cache->array[i];
if (ent->vuid == vuid) {
free_conn_session_info_if_unused(conn);
conn->session_info = ent->session_info;
@@ -141,10 +141,10 @@ static bool check_user_ok(connection_struct *conn,
session_info->info->domain_name,
NULL, session_info->security_token, lp_admin_users(snum));
- ent = &conn->vuid_cache.array[conn->vuid_cache.next_entry];
+ ent = &conn->vuid_cache->array[conn->vuid_cache->next_entry];
- conn->vuid_cache.next_entry =
- (conn->vuid_cache.next_entry + 1) % VUID_CACHE_SIZE;
+ conn->vuid_cache->next_entry =
+ (conn->vuid_cache->next_entry + 1) % VUID_CACHE_SIZE;
TALLOC_FREE(ent->session_info);
--
1.8.1
>From b32a4be55967a02d5479ff1a3b785e059e684103 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Fri, 21 Dec 2012 09:22:16 -0800
Subject: [PATCH 08/23] Add uint32_t share_access to vuid_cache_entry.
Signed-off-by: Jeremy Allison <jra at samba.org>
---
source3/include/vfs.h | 2 ++
source3/modules/vfs_readonly.c | 1 +
source3/smbd/conn.c | 1 +
3 files changed, 4 insertions(+)
diff --git a/source3/include/vfs.h b/source3/include/vfs.h
index 7da022e..2bce1b7 100644
--- a/source3/include/vfs.h
+++ b/source3/include/vfs.h
@@ -150,6 +150,7 @@
/* Bump to version 31 - Samba 4.1.0 will ship with interface version 31 */
/* Leave at 31 - not yet released. Make struct vuid_cache_entry in
connection_struct a pointer. */
+/* Leave at 31 - not yet released. Add share_access to vuid_cache_entry. */
#define SMB_VFS_INTERFACE_VERSION 31
@@ -279,6 +280,7 @@ struct vuid_cache_entry {
struct auth_session_info *session_info;
uint64_t vuid; /* SMB2 compat */
bool read_only;
+ uint32_t share_access;
};
struct vuid_cache {
diff --git a/source3/modules/vfs_readonly.c b/source3/modules/vfs_readonly.c
index f75db09..445f947 100644
--- a/source3/modules/vfs_readonly.c
+++ b/source3/modules/vfs_readonly.c
@@ -86,6 +86,7 @@ static int readonly_connect(vfs_handle_struct *handle,
ent->vuid = UID_FIELD_INVALID;
TALLOC_FREE(ent->session_info);
ent->read_only = false;
+ ent->share_access = 0;
}
conn->vuid_cache->next_entry = 0;
}
diff --git a/source3/smbd/conn.c b/source3/smbd/conn.c
index 1d4444f..8f472c0 100644
--- a/source3/smbd/conn.c
+++ b/source3/smbd/conn.c
@@ -118,6 +118,7 @@ static void conn_clear_vuid_cache(connection_struct *conn, uint64_t vuid)
TALLOC_FREE(ent->session_info);
}
ent->read_only = False;
+ ent->share_access = 0;
}
}
}
--
1.8.1
>From 27f128101e372afde00b15aab4e2a630e96e0c87 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Fri, 21 Dec 2012 09:35:31 -0800
Subject: [PATCH 09/23] Remove static from create_share_access_mask().
Signed-off-by: Jeremy Allison <jra at samba.org>
---
source3/smbd/proto.h | 1 +
source3/smbd/service.c | 2 +-
2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/source3/smbd/proto.h b/source3/smbd/proto.h
index d7bfa65..5eb899f 100644
--- a/source3/smbd/proto.h
+++ b/source3/smbd/proto.h
@@ -979,6 +979,7 @@ void smbd_exit_server_cleanly(const char *const reason) _NORETURN_;
bool set_conn_connectpath(connection_struct *conn, const char *connectpath);
NTSTATUS set_conn_force_user_group(connection_struct *conn, int snum);
+void create_share_access_mask(connection_struct *conn, int snum);
bool set_current_service(connection_struct *conn, uint16 flags, bool do_chdir);
void load_registry_shares(void);
int add_home_service(const char *service, const char *username, const char *homedir);
diff --git a/source3/smbd/service.c b/source3/smbd/service.c
index 2214ac0..828c036 100644
--- a/source3/smbd/service.c
+++ b/source3/smbd/service.c
@@ -515,7 +515,7 @@ NTSTATUS set_conn_force_user_group(connection_struct *conn, int snum)
Setup the share access mask for a connection.
****************************************************************************/
-static void create_share_access_mask(connection_struct *conn, int snum)
+void create_share_access_mask(connection_struct *conn, int snum)
{
const struct security_token *token = conn->session_info->security_token;
--
1.8.1
>From 9a45e9811cbb5cd96f91eb8594b469a6e6af00ff Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Fri, 21 Dec 2012 09:45:03 -0800
Subject: [PATCH 10/23] Fix API for create_share_access_mask().
Return the uint32_t share_access rather than directly
changing the conn struct.
Signed-off-by: Jeremy Allison <jra at samba.org>
---
source3/smbd/proto.h | 2 +-
source3/smbd/service.c | 19 +++++++++++--------
2 files changed, 12 insertions(+), 9 deletions(-)
diff --git a/source3/smbd/proto.h b/source3/smbd/proto.h
index 5eb899f..abc6b28 100644
--- a/source3/smbd/proto.h
+++ b/source3/smbd/proto.h
@@ -979,7 +979,7 @@ void smbd_exit_server_cleanly(const char *const reason) _NORETURN_;
bool set_conn_connectpath(connection_struct *conn, const char *connectpath);
NTSTATUS set_conn_force_user_group(connection_struct *conn, int snum);
-void create_share_access_mask(connection_struct *conn, int snum);
+uint32_t create_share_access_mask(connection_struct *conn, int snum);
bool set_current_service(connection_struct *conn, uint16 flags, bool do_chdir);
void load_registry_shares(void);
int add_home_service(const char *service, const char *username, const char *homedir);
diff --git a/source3/smbd/service.c b/source3/smbd/service.c
index 828c036..1cd12a6 100644
--- a/source3/smbd/service.c
+++ b/source3/smbd/service.c
@@ -515,34 +515,37 @@ NTSTATUS set_conn_force_user_group(connection_struct *conn, int snum)
Setup the share access mask for a connection.
****************************************************************************/
-void create_share_access_mask(connection_struct *conn, int snum)
+uint32_t create_share_access_mask(connection_struct *conn, int snum)
{
const struct security_token *token = conn->session_info->security_token;
+ uint32_t share_access = 0;
share_access_check(token,
lp_servicename(talloc_tos(), snum),
MAXIMUM_ALLOWED_ACCESS,
- &conn->share_access);
+ &share_access);
if (!CAN_WRITE(conn)) {
- conn->share_access &=
+ share_access &=
~(SEC_FILE_WRITE_DATA | SEC_FILE_APPEND_DATA |
SEC_FILE_WRITE_EA | SEC_FILE_WRITE_ATTRIBUTE |
SEC_DIR_DELETE_CHILD );
}
if (security_token_has_privilege(token, SEC_PRIV_SECURITY)) {
- conn->share_access |= SEC_FLAG_SYSTEM_SECURITY;
+ share_access |= SEC_FLAG_SYSTEM_SECURITY;
}
if (security_token_has_privilege(token, SEC_PRIV_RESTORE)) {
- conn->share_access |= (SEC_RIGHTS_PRIV_RESTORE);
+ share_access |= (SEC_RIGHTS_PRIV_RESTORE);
}
if (security_token_has_privilege(token, SEC_PRIV_BACKUP)) {
- conn->share_access |= (SEC_RIGHTS_PRIV_BACKUP);
+ share_access |= (SEC_RIGHTS_PRIV_BACKUP);
}
if (security_token_has_privilege(token, SEC_PRIV_TAKE_OWNERSHIP)) {
- conn->share_access |= (SEC_STD_WRITE_OWNER);
+ share_access |= (SEC_STD_WRITE_OWNER);
}
+
+ return share_access;
}
/****************************************************************************
@@ -654,7 +657,7 @@ static NTSTATUS make_connection_snum(struct smbd_server_connection *sconn,
*
*/
- create_share_access_mask(conn, snum);
+ conn->share_access = create_share_access_mask(conn, snum);
if ((conn->share_access & FILE_WRITE_DATA) == 0) {
if ((conn->share_access & FILE_READ_DATA) == 0) {
--
1.8.1
>From 638f606e9f14bd9df586bf306be81f48d2c63568 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Fri, 4 Jan 2013 11:43:10 -0800
Subject: [PATCH 11/23] Change API for create_share_access_mask() to pass in
the token.
Don't automatically use the one from conn->session_info->security_token.
Signed-off-by: Jeremy Allison <jra at samba.org>
---
source3/smbd/proto.h | 4 +++-
source3/smbd/service.c | 9 ++++++---
2 files changed, 9 insertions(+), 4 deletions(-)
diff --git a/source3/smbd/proto.h b/source3/smbd/proto.h
index abc6b28..bfaaf6d 100644
--- a/source3/smbd/proto.h
+++ b/source3/smbd/proto.h
@@ -979,7 +979,9 @@ void smbd_exit_server_cleanly(const char *const reason) _NORETURN_;
bool set_conn_connectpath(connection_struct *conn, const char *connectpath);
NTSTATUS set_conn_force_user_group(connection_struct *conn, int snum);
-uint32_t create_share_access_mask(connection_struct *conn, int snum);
+uint32_t create_share_access_mask(connection_struct *conn,
+ int snum,
+ const struct security_token *token);
bool set_current_service(connection_struct *conn, uint16 flags, bool do_chdir);
void load_registry_shares(void);
int add_home_service(const char *service, const char *username, const char *homedir);
diff --git a/source3/smbd/service.c b/source3/smbd/service.c
index 1cd12a6..3e1d87f 100644
--- a/source3/smbd/service.c
+++ b/source3/smbd/service.c
@@ -515,9 +515,10 @@ NTSTATUS set_conn_force_user_group(connection_struct *conn, int snum)
Setup the share access mask for a connection.
****************************************************************************/
-uint32_t create_share_access_mask(connection_struct *conn, int snum)
+uint32_t create_share_access_mask(connection_struct *conn,
+ int snum,
+ const struct security_token *token)
{
- const struct security_token *token = conn->session_info->security_token;
uint32_t share_access = 0;
share_access_check(token,
@@ -657,7 +658,9 @@ static NTSTATUS make_connection_snum(struct smbd_server_connection *sconn,
*
*/
- conn->share_access = create_share_access_mask(conn, snum);
+ conn->share_access = create_share_access_mask(conn,
+ snum,
+ conn->session_info->security_token);
if ((conn->share_access & FILE_WRITE_DATA) == 0) {
if ((conn->share_access & FILE_READ_DATA) == 0) {
--
1.8.1
>From 37fa81de9125385dd788ac659f3a9b5c189f265f Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Fri, 4 Jan 2013 14:24:13 -0800
Subject: [PATCH 12/23] Change API for create_share_access_mask() - remove conn
struct.
Eventually this will be indepentent of conn, just pass in the
readonly flag.
Signed-off-by: Jeremy Allison <jra at samba.org>
---
source3/smbd/proto.h | 4 ++--
source3/smbd/service.c | 10 +++++-----
2 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/source3/smbd/proto.h b/source3/smbd/proto.h
index bfaaf6d..2198ccd 100644
--- a/source3/smbd/proto.h
+++ b/source3/smbd/proto.h
@@ -979,8 +979,8 @@ void smbd_exit_server_cleanly(const char *const reason) _NORETURN_;
bool set_conn_connectpath(connection_struct *conn, const char *connectpath);
NTSTATUS set_conn_force_user_group(connection_struct *conn, int snum);
-uint32_t create_share_access_mask(connection_struct *conn,
- int snum,
+uint32_t create_share_access_mask(int snum,
+ bool readonly_share,
const struct security_token *token);
bool set_current_service(connection_struct *conn, uint16 flags, bool do_chdir);
void load_registry_shares(void);
diff --git a/source3/smbd/service.c b/source3/smbd/service.c
index 3e1d87f..10f4b53 100644
--- a/source3/smbd/service.c
+++ b/source3/smbd/service.c
@@ -515,8 +515,8 @@ NTSTATUS set_conn_force_user_group(connection_struct *conn, int snum)
Setup the share access mask for a connection.
****************************************************************************/
-uint32_t create_share_access_mask(connection_struct *conn,
- int snum,
+uint32_t create_share_access_mask(int snum,
+ bool readonly_share,
const struct security_token *token)
{
uint32_t share_access = 0;
@@ -526,7 +526,7 @@ uint32_t create_share_access_mask(connection_struct *conn,
MAXIMUM_ALLOWED_ACCESS,
&share_access);
- if (!CAN_WRITE(conn)) {
+ if (readonly_share) {
share_access &=
~(SEC_FILE_WRITE_DATA | SEC_FILE_APPEND_DATA |
SEC_FILE_WRITE_EA | SEC_FILE_WRITE_ATTRIBUTE |
@@ -658,8 +658,8 @@ static NTSTATUS make_connection_snum(struct smbd_server_connection *sconn,
*
*/
- conn->share_access = create_share_access_mask(conn,
- snum,
+ conn->share_access = create_share_access_mask(snum,
+ !CAN_WRITE(conn),
conn->session_info->security_token);
if ((conn->share_access & FILE_WRITE_DATA) == 0) {
--
1.8.1
>From 2a75c018f718874ca9b58aebe71f3c35499fe32e Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Fri, 4 Jan 2013 14:25:55 -0800
Subject: [PATCH 13/23] Correctly setup the conn->share_access based on the
current user token.
Also use this to set conn->read_only. Cache the share_access in the
struct vuid_cache_entry struct so we only evaluate this once per new
user access on this share.
Signed-off-by: Jeremy Allison <jra at samba.org>
---
source3/smbd/uid.c | 34 ++++++++++++++++++++++------------
1 file changed, 22 insertions(+), 12 deletions(-)
diff --git a/source3/smbd/uid.c b/source3/smbd/uid.c
index f9b5716..397380c 100644
--- a/source3/smbd/uid.c
+++ b/source3/smbd/uid.c
@@ -94,6 +94,7 @@ static bool check_user_ok(connection_struct *conn,
bool readonly_share;
bool admin_user;
struct vuid_cache_entry *ent = NULL;
+ uint32_t share_access = 0;
for (i=0; i<VUID_CACHE_SIZE; i++) {
ent = &conn->vuid_cache->array[i];
@@ -101,6 +102,7 @@ static bool check_user_ok(connection_struct *conn,
free_conn_session_info_if_unused(conn);
conn->session_info = ent->session_info;
conn->read_only = ent->read_only;
+ conn->share_access = ent->share_access;
return(True);
}
}
@@ -116,11 +118,24 @@ static bool check_user_ok(connection_struct *conn,
session_info->security_token,
conn);
+ share_access = create_share_access_mask(snum,
+ readonly_share,
+ session_info->security_token);
+
+ if ((share_access & FILE_WRITE_DATA) == 0) {
+ if ((share_access & FILE_READ_DATA) == 0) {
+ /* No access, read or write. */
+ DEBUG(0,("user %s connection to %s "
+ "denied due to share security "
+ "descriptor.\n",
+ session_info->unix_info->unix_name,
+ lp_servicename(talloc_tos(), snum)));
+ return false;
+ }
+ }
+
if (!readonly_share &&
- !share_access_check(session_info->security_token,
- lp_servicename(talloc_tos(), snum),
- FILE_WRITE_DATA,
- NULL)) {
+ !(share_access & FILE_WRITE_DATA)) {
/* smb.conf allows r/w, but the security descriptor denies
* write. Fall back to looking at readonly. */
readonly_share = True;
@@ -128,14 +143,6 @@ static bool check_user_ok(connection_struct *conn,
"security descriptor\n"));
}
- if (!share_access_check(session_info->security_token,
- lp_servicename(talloc_tos(), snum),
- readonly_share ?
- FILE_READ_DATA : FILE_WRITE_DATA,
- NULL)) {
- return False;
- }
-
admin_user = token_contains_name_in_list(
session_info->unix_info->unix_name,
session_info->info->domain_name,
@@ -163,10 +170,13 @@ static bool check_user_ok(connection_struct *conn,
ent->vuid = vuid;
ent->read_only = readonly_share;
+ ent->share_access = share_access;
free_conn_session_info_if_unused(conn);
conn->session_info = ent->session_info;
conn->read_only = readonly_share;
+ conn->share_access = share_access;
+
if (admin_user) {
DEBUG(2,("check_user_ok: user %s is an admin user. "
"Setting uid as %d\n",
--
1.8.1
>From 23d97d1db9fcb2c968b4ea7b3a332ffa8015b90d Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Fri, 4 Jan 2013 14:27:18 -0800
Subject: [PATCH 14/23] Add check_user_share_access() which factors out the
share security and read_only flag setting code.
Allows this to be called from both make_connection_snum() as well as check_user_ok().
Gives a consistent share security check function.
Signed-off-by: Jeremy Allison <jra at samba.org>
---
source3/smbd/proto.h | 4 ++++
source3/smbd/uid.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 60 insertions(+)
diff --git a/source3/smbd/proto.h b/source3/smbd/proto.h
index 2198ccd..e8ee873 100644
--- a/source3/smbd/proto.h
+++ b/source3/smbd/proto.h
@@ -1099,6 +1099,10 @@ void reply_transs2(struct smb_request *req);
/* The following definitions come from smbd/uid.c */
bool change_to_guest(void);
+NTSTATUS check_user_share_access(connection_struct *conn,
+ const struct auth_session_info *session_info,
+ uint32_t *p_share_access,
+ bool *p_readonly_share);
bool change_to_user(connection_struct *conn, uint64_t vuid);
bool change_to_root_user(void);
bool smbd_change_to_root_user(void);
diff --git a/source3/smbd/uid.c b/source3/smbd/uid.c
index 397380c..f551e50 100644
--- a/source3/smbd/uid.c
+++ b/source3/smbd/uid.c
@@ -79,6 +79,62 @@ static void free_conn_session_info_if_unused(connection_struct *conn)
}
/*******************************************************************
+ Calculate access mask and if this user can access this share.
+********************************************************************/
+
+NTSTATUS check_user_share_access(connection_struct *conn,
+ const struct auth_session_info *session_info,
+ uint32_t *p_share_access,
+ bool *p_readonly_share)
+{
+ int snum = SNUM(conn);
+ uint32_t share_access = 0;
+ bool readonly_share = false;
+
+ if (!user_ok_token(session_info->unix_info->unix_name,
+ session_info->info->domain_name,
+ session_info->security_token, snum)) {
+ return NT_STATUS_ACCESS_DENIED;
+ }
+
+ readonly_share = is_share_read_only_for_token(
+ session_info->unix_info->unix_name,
+ session_info->info->domain_name,
+ session_info->security_token,
+ conn);
+
+ share_access = create_share_access_mask(snum,
+ readonly_share,
+ session_info->security_token);
+
+ if ((share_access & FILE_WRITE_DATA) == 0) {
+ if ((share_access & FILE_READ_DATA) == 0) {
+ /* No access, read or write. */
+ DEBUG(0,("user %s connection to %s "
+ "denied due to share security "
+ "descriptor.\n",
+ session_info->unix_info->unix_name,
+ lp_servicename(talloc_tos(), snum)));
+ return NT_STATUS_ACCESS_DENIED;
+ }
+ }
+
+ if (!readonly_share &&
+ !(share_access & FILE_WRITE_DATA)) {
+ /* smb.conf allows r/w, but the security descriptor denies
+ * write. Fall back to looking at readonly. */
+ readonly_share = True;
+ DEBUG(5,("falling back to read-only access-evaluation due to "
+ "security descriptor\n"));
+ }
+
+ *p_share_access = share_access;
+ *p_readonly_share = readonly_share;
+
+ return NT_STATUS_OK;
+}
+
+/*******************************************************************
Check if a username is OK.
This sets up conn->session_info with a copy related to this vuser that
--
1.8.1
>From ea16b046a3a9cfe79940a169e68f805e7994bee1 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Thu, 3 Jan 2013 16:06:40 -0800
Subject: [PATCH 15/23] Initialize stack variables. Prelude to factoring out
calls to check_user_share_access().
Signed-off-by: Jeremy Allison <jra at samba.org>
---
source3/smbd/uid.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/source3/smbd/uid.c b/source3/smbd/uid.c
index f551e50..9802056 100644
--- a/source3/smbd/uid.c
+++ b/source3/smbd/uid.c
@@ -147,8 +147,8 @@ static bool check_user_ok(connection_struct *conn,
int snum)
{
unsigned int i;
- bool readonly_share;
- bool admin_user;
+ bool readonly_share = false;
+ bool admin_user = false;
struct vuid_cache_entry *ent = NULL;
uint32_t share_access = 0;
--
1.8.1
>From e3b0ea9e543ee8494abf61a3e73b76853172e253 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Fri, 4 Jan 2013 14:35:46 -0800
Subject: [PATCH 16/23] Factor code out of check_user_ok() into a call to
check_user_share_access().
Signed-off-by: Jeremy Allison <jra at samba.org>
---
source3/smbd/uid.c | 41 +++++++----------------------------------
1 file changed, 7 insertions(+), 34 deletions(-)
diff --git a/source3/smbd/uid.c b/source3/smbd/uid.c
index 9802056..efdd824 100644
--- a/source3/smbd/uid.c
+++ b/source3/smbd/uid.c
@@ -151,6 +151,7 @@ static bool check_user_ok(connection_struct *conn,
bool admin_user = false;
struct vuid_cache_entry *ent = NULL;
uint32_t share_access = 0;
+ NTSTATUS status;
for (i=0; i<VUID_CACHE_SIZE; i++) {
ent = &conn->vuid_cache->array[i];
@@ -163,40 +164,12 @@ static bool check_user_ok(connection_struct *conn,
}
}
- if (!user_ok_token(session_info->unix_info->unix_name,
- session_info->info->domain_name,
- session_info->security_token, snum))
- return(False);
-
- readonly_share = is_share_read_only_for_token(
- session_info->unix_info->unix_name,
- session_info->info->domain_name,
- session_info->security_token,
- conn);
-
- share_access = create_share_access_mask(snum,
- readonly_share,
- session_info->security_token);
-
- if ((share_access & FILE_WRITE_DATA) == 0) {
- if ((share_access & FILE_READ_DATA) == 0) {
- /* No access, read or write. */
- DEBUG(0,("user %s connection to %s "
- "denied due to share security "
- "descriptor.\n",
- session_info->unix_info->unix_name,
- lp_servicename(talloc_tos(), snum)));
- return false;
- }
- }
-
- if (!readonly_share &&
- !(share_access & FILE_WRITE_DATA)) {
- /* smb.conf allows r/w, but the security descriptor denies
- * write. Fall back to looking at readonly. */
- readonly_share = True;
- DEBUG(5,("falling back to read-only access-evaluation due to "
- "security descriptor\n"));
+ status = check_user_share_access(conn,
+ session_info,
+ &share_access,
+ &readonly_share);
+ if (!NT_STATUS_IS_OK(status)) {
+ return false;
}
admin_user = token_contains_name_in_list(
--
1.8.1
>From c0b5b0caa8d5696ef90b830e87f6ee3001f8f575 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Fri, 4 Jan 2013 14:40:05 -0800
Subject: [PATCH 17/23] Fix bug #9518 - conn->share_access appears not be be
reset between users.
Ensure make_connection_snum() uses the same logic as
check_user_ok() to decide if a user can access a share.
Signed-off-by: Jeremy Allison <jra at samba.org>
---
source3/smbd/service.c | 28 ++++++++--------------------
1 file changed, 8 insertions(+), 20 deletions(-)
diff --git a/source3/smbd/service.c b/source3/smbd/service.c
index 10f4b53..fabc5a3 100644
--- a/source3/smbd/service.c
+++ b/source3/smbd/service.c
@@ -652,29 +652,17 @@ static NTSTATUS make_connection_snum(struct smbd_server_connection *sconn,
}
/*
- * New code to check if there's a share security descripter
- * added from NT server manager. This is done after the
- * smb.conf checks are done as we need a uid and token. JRA.
- *
+ * Set up the share security descripter
*/
- conn->share_access = create_share_access_mask(snum,
- !CAN_WRITE(conn),
- conn->session_info->security_token);
-
- if ((conn->share_access & FILE_WRITE_DATA) == 0) {
- if ((conn->share_access & FILE_READ_DATA) == 0) {
- /* No access, read or write. */
- DEBUG(0,("make_connection: connection to %s "
- "denied due to security "
- "descriptor.\n",
- lp_servicename(talloc_tos(), snum)));
- status = NT_STATUS_ACCESS_DENIED;
- goto err_root_exit;
- } else {
- conn->read_only = True;
- }
+ status = check_user_share_access(conn,
+ conn->session_info,
+ &conn->share_access,
+ &conn->read_only);
+ if (!NT_STATUS_IS_OK(status)) {
+ goto err_root_exit;
}
+
/* Initialise VFS function pointers */
if (!smbd_vfs_init(conn)) {
--
1.8.1
>From fa9dc2456aba9058b18e9a67b8dec6f49eef0c16 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Fri, 4 Jan 2013 14:42:23 -0800
Subject: [PATCH 18/23] Move create_share_access_mask() from smbd/service.c to
smbd/uid.c.
Make it static. Only called from uid.c now.
Signed-off-by: Jeremy Allison <jra at samba.org>
---
source3/smbd/proto.h | 3 ---
source3/smbd/service.c | 38 --------------------------------------
source3/smbd/uid.c | 38 ++++++++++++++++++++++++++++++++++++++
3 files changed, 38 insertions(+), 41 deletions(-)
diff --git a/source3/smbd/proto.h b/source3/smbd/proto.h
index e8ee873..7727302 100644
--- a/source3/smbd/proto.h
+++ b/source3/smbd/proto.h
@@ -979,9 +979,6 @@ void smbd_exit_server_cleanly(const char *const reason) _NORETURN_;
bool set_conn_connectpath(connection_struct *conn, const char *connectpath);
NTSTATUS set_conn_force_user_group(connection_struct *conn, int snum);
-uint32_t create_share_access_mask(int snum,
- bool readonly_share,
- const struct security_token *token);
bool set_current_service(connection_struct *conn, uint16 flags, bool do_chdir);
void load_registry_shares(void);
int add_home_service(const char *service, const char *username, const char *homedir);
diff --git a/source3/smbd/service.c b/source3/smbd/service.c
index fabc5a3..8f6d485 100644
--- a/source3/smbd/service.c
+++ b/source3/smbd/service.c
@@ -512,44 +512,6 @@ NTSTATUS set_conn_force_user_group(connection_struct *conn, int snum)
}
/****************************************************************************
- Setup the share access mask for a connection.
-****************************************************************************/
-
-uint32_t create_share_access_mask(int snum,
- bool readonly_share,
- const struct security_token *token)
-{
- uint32_t share_access = 0;
-
- share_access_check(token,
- lp_servicename(talloc_tos(), snum),
- MAXIMUM_ALLOWED_ACCESS,
- &share_access);
-
- if (readonly_share) {
- share_access &=
- ~(SEC_FILE_WRITE_DATA | SEC_FILE_APPEND_DATA |
- SEC_FILE_WRITE_EA | SEC_FILE_WRITE_ATTRIBUTE |
- SEC_DIR_DELETE_CHILD );
- }
-
- if (security_token_has_privilege(token, SEC_PRIV_SECURITY)) {
- share_access |= SEC_FLAG_SYSTEM_SECURITY;
- }
- if (security_token_has_privilege(token, SEC_PRIV_RESTORE)) {
- share_access |= (SEC_RIGHTS_PRIV_RESTORE);
- }
- if (security_token_has_privilege(token, SEC_PRIV_BACKUP)) {
- share_access |= (SEC_RIGHTS_PRIV_BACKUP);
- }
- if (security_token_has_privilege(token, SEC_PRIV_TAKE_OWNERSHIP)) {
- share_access |= (SEC_STD_WRITE_OWNER);
- }
-
- return share_access;
-}
-
-/****************************************************************************
Make a connection, given the snum to connect to, and the vuser of the
connecting user if appropriate.
****************************************************************************/
diff --git a/source3/smbd/uid.c b/source3/smbd/uid.c
index efdd824..b9cebce 100644
--- a/source3/smbd/uid.c
+++ b/source3/smbd/uid.c
@@ -78,6 +78,44 @@ static void free_conn_session_info_if_unused(connection_struct *conn)
TALLOC_FREE(conn->session_info);
}
+/****************************************************************************
+ Setup the share access mask for a connection.
+****************************************************************************/
+
+static uint32_t create_share_access_mask(int snum,
+ bool readonly_share,
+ const struct security_token *token)
+{
+ uint32_t share_access = 0;
+
+ share_access_check(token,
+ lp_servicename(talloc_tos(), snum),
+ MAXIMUM_ALLOWED_ACCESS,
+ &share_access);
+
+ if (readonly_share) {
+ share_access &=
+ ~(SEC_FILE_WRITE_DATA | SEC_FILE_APPEND_DATA |
+ SEC_FILE_WRITE_EA | SEC_FILE_WRITE_ATTRIBUTE |
+ SEC_DIR_DELETE_CHILD );
+ }
+
+ if (security_token_has_privilege(token, SEC_PRIV_SECURITY)) {
+ share_access |= SEC_FLAG_SYSTEM_SECURITY;
+ }
+ if (security_token_has_privilege(token, SEC_PRIV_RESTORE)) {
+ share_access |= (SEC_RIGHTS_PRIV_RESTORE);
+ }
+ if (security_token_has_privilege(token, SEC_PRIV_BACKUP)) {
+ share_access |= (SEC_RIGHTS_PRIV_BACKUP);
+ }
+ if (security_token_has_privilege(token, SEC_PRIV_TAKE_OWNERSHIP)) {
+ share_access |= (SEC_STD_WRITE_OWNER);
+ }
+
+ return share_access;
+}
+
/*******************************************************************
Calculate access mask and if this user can access this share.
********************************************************************/
--
1.8.1
>From 1806a6f4c2b4f409b84150b4dbbcafa3d6e31bc4 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Sun, 6 Jan 2013 14:39:07 +0100
Subject: [PATCH 19/23] smbd: Fix a typo
Reviewed-by: Jeremy Allison <jra at samba.org>
---
source3/smbd/service.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/source3/smbd/service.c b/source3/smbd/service.c
index 8f6d485..4fe3809 100644
--- a/source3/smbd/service.c
+++ b/source3/smbd/service.c
@@ -614,7 +614,7 @@ static NTSTATUS make_connection_snum(struct smbd_server_connection *sconn,
}
/*
- * Set up the share security descripter
+ * Set up the share security descriptor
*/
status = check_user_share_access(conn,
--
1.8.1
>From b57d619068581ad9c4f89d05d0bac1bfaf18cbb5 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Sun, 6 Jan 2013 14:41:24 +0100
Subject: [PATCH 20/23] smbd: Simplify an if-expression
Reviewed-by: Jeremy Allison <jra at samba.org>
---
source3/smbd/uid.c | 17 +++++++----------
1 file changed, 7 insertions(+), 10 deletions(-)
diff --git a/source3/smbd/uid.c b/source3/smbd/uid.c
index b9cebce..5ab0dc4 100644
--- a/source3/smbd/uid.c
+++ b/source3/smbd/uid.c
@@ -145,16 +145,13 @@ NTSTATUS check_user_share_access(connection_struct *conn,
readonly_share,
session_info->security_token);
- if ((share_access & FILE_WRITE_DATA) == 0) {
- if ((share_access & FILE_READ_DATA) == 0) {
- /* No access, read or write. */
- DEBUG(0,("user %s connection to %s "
- "denied due to share security "
- "descriptor.\n",
- session_info->unix_info->unix_name,
- lp_servicename(talloc_tos(), snum)));
- return NT_STATUS_ACCESS_DENIED;
- }
+ if ((share_access & (FILE_READ_DATA|FILE_WRITE_DATA)) == 0) {
+ /* No access, read or write. */
+ DEBUG(0,("user %s connection to %s denied due to share "
+ "security descriptor.\n",
+ session_info->unix_info->unix_name,
+ lp_servicename(talloc_tos(), snum)));
+ return NT_STATUS_ACCESS_DENIED;
}
if (!readonly_share &&
--
1.8.1
>From 2e0469dee641106941ab1497ddd5c88082219f83 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Sun, 6 Jan 2013 14:50:33 +0100
Subject: [PATCH 21/23] smbd: Remove some ()
Reviewed-by: Jeremy Allison <jra at samba.org>
---
source3/smbd/uid.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/source3/smbd/uid.c b/source3/smbd/uid.c
index 5ab0dc4..b2fe39c 100644
--- a/source3/smbd/uid.c
+++ b/source3/smbd/uid.c
@@ -104,13 +104,13 @@ static uint32_t create_share_access_mask(int snum,
share_access |= SEC_FLAG_SYSTEM_SECURITY;
}
if (security_token_has_privilege(token, SEC_PRIV_RESTORE)) {
- share_access |= (SEC_RIGHTS_PRIV_RESTORE);
+ share_access |= SEC_RIGHTS_PRIV_RESTORE;
}
if (security_token_has_privilege(token, SEC_PRIV_BACKUP)) {
- share_access |= (SEC_RIGHTS_PRIV_BACKUP);
+ share_access |= SEC_RIGHTS_PRIV_BACKUP;
}
if (security_token_has_privilege(token, SEC_PRIV_TAKE_OWNERSHIP)) {
- share_access |= (SEC_STD_WRITE_OWNER);
+ share_access |= SEC_STD_WRITE_OWNER;
}
return share_access;
--
1.8.1
>From 64cb552270d9a29179ef6b86ea905a75cc6eee87 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Tue, 8 Jan 2013 11:02:16 -0800
Subject: [PATCH 22/23] Fixup the change_to_user_by_session() case as called
from become_user_by_session()
Use inside source3/printing/nt_printing.c:get_correct_cversion().
Allow check_user_ok() to be called with vuid==UID_FIELD_INVALID.
All this should do is throw away one entry in the vuid cache.
Signed-off-by: Jeremy Allison <jra at samba.org>
---
source3/smbd/uid.c | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
diff --git a/source3/smbd/uid.c b/source3/smbd/uid.c
index b2fe39c..a795eef 100644
--- a/source3/smbd/uid.c
+++ b/source3/smbd/uid.c
@@ -191,6 +191,13 @@ static bool check_user_ok(connection_struct *conn,
for (i=0; i<VUID_CACHE_SIZE; i++) {
ent = &conn->vuid_cache->array[i];
if (ent->vuid == vuid) {
+ if (vuid == UID_FIELD_INVALID) {
+ /*
+ * Slow path, we don't care
+ * about the array traversal.
+ */
+ continue;
+ }
free_conn_session_info_if_unused(conn);
conn->session_info = ent->session_info;
conn->read_only = ent->read_only;
@@ -232,11 +239,26 @@ static bool check_user_ok(connection_struct *conn,
return false;
}
+ /*
+ * It's actually OK to call check_user_ok() with
+ * vuid == UID_FIELD_INVALID as called from change_to_user_by_session().
+ * All this will do is throw away one entry in the cache.
+ */
+
ent->vuid = vuid;
ent->read_only = readonly_share;
ent->share_access = share_access;
free_conn_session_info_if_unused(conn);
conn->session_info = ent->session_info;
+ if (vuid == UID_FIELD_INVALID) {
+ /*
+ * Not strictly needed, just make it really
+ * clear this entry is actually an unused one.
+ */
+ ent->read_only = false;
+ ent->share_access = 0;
+ ent->session_info = NULL;
+ }
conn->read_only = readonly_share;
conn->share_access = share_access;
--
1.8.1
>From 07e4e1308a898b46b3b87fd9f27601bbb4106625 Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Thu, 20 Dec 2012 23:05:55 +1100
Subject: [PATCH 23/23] selftest: show that Samba honours "write list" and
valid users
Reviewed-by: Jeremy Allison <jra at samba.org>
---
selftest/target/Samba3.pm | 7 +++++++
source3/script/tests/test_smbclient_machine_auth.sh | 4 ++++
source3/script/tests/test_smbclient_s3.sh | 8 ++++++--
source3/selftest/tests.py | 5 +++++
4 files changed, 22 insertions(+), 2 deletions(-)
diff --git a/selftest/target/Samba3.pm b/selftest/target/Samba3.pm
index adca52f..8546bd2 100755
--- a/selftest/target/Samba3.pm
+++ b/selftest/target/Samba3.pm
@@ -963,6 +963,13 @@ sub provision($$$$$$)
[ro-tmp]
path = $ro_shrdir
guest ok = yes
+[write-list-tmp]
+ path = $shrdir
+ read only = yes
+ write list = $unix_name
+[valid-users-tmp]
+ path = $shrdir
+ valid users = $unix_name
[msdfs-share]
path = $msdfs_shrdir
msdfs root = yes
diff --git a/source3/script/tests/test_smbclient_machine_auth.sh b/source3/script/tests/test_smbclient_machine_auth.sh
index f67256d..a890d48 100755
--- a/source3/script/tests/test_smbclient_machine_auth.sh
+++ b/source3/script/tests/test_smbclient_machine_auth.sh
@@ -19,3 +19,7 @@ incdir=`dirname $0`/../../../testprogs/blackbox
. $incdir/subunit.sh
testit "smbclient //$SERVER/tmp" $SMBCLIENT //$SERVER/tmp --machine-pass -I $SERVER_IP -p 139 -c quit $ADDARGS
+
+# Testing these here helps because we know the machine account isn't already this user/group
+testit "smbclient //$SERVER/forceuser" $SMBCLIENT //$SERVER/tmp --machine-pass -I $SERVER_IP -p 139 -c quit $ADDARGS
+testit "smbclient //$SERVER/forcegroup" $SMBCLIENT //$SERVER/tmp --machine-pass -I $SERVER_IP -p 139 -c quit $ADDARGS
diff --git a/source3/script/tests/test_smbclient_s3.sh b/source3/script/tests/test_smbclient_s3.sh
index fb518c5..b240da0 100755
--- a/source3/script/tests/test_smbclient_s3.sh
+++ b/source3/script/tests/test_smbclient_s3.sh
@@ -212,7 +212,7 @@ mkdir a_test_dir
quit
EOF
- cmd='CLI_FORCE_INTERACTIVE=yes $SMBCLIENT "$@" -U% //$SERVER/ro-tmp -I $SERVER_IP $ADDARGS < $tmpfile 2>&1'
+ cmd='CLI_FORCE_INTERACTIVE=yes $SMBCLIENT -U% //$SERVER/$1" -I $SERVER_IP $ADDARGS < $tmpfile 2>&1'
eval echo "$cmd"
out=`eval $cmd`
ret=$?
@@ -581,7 +581,11 @@ testit "creating a good symlink and deleting it by path" \
failed=`expr $failed + 1`
testit "writing into a read-only directory fails" \
- test_read_only_dir || \
+ test_read_only_dir ro-tmp || \
+ failed=`expr $failed + 1`
+
+testit "writing into a read-only share fails" \
+ test_read_only_dir valid-users-tmp || \
failed=`expr $failed + 1`
testit "Reading a owner-only file fails" \
diff --git a/source3/selftest/tests.py b/source3/selftest/tests.py
index 57a67ed..44efe18 100755
--- a/source3/selftest/tests.py
+++ b/source3/selftest/tests.py
@@ -333,6 +333,11 @@ for t in tests:
elif t == "smb2.durable-open" or t == "smb2.durable-v2-open":
plansmbtorture4testsuite(t, "s3dc", '//$SERVER_IP/durable -U$USERNAME%$PASSWORD')
plansmbtorture4testsuite(t, "plugin_s4_dc", '//$SERVER_IP/durable -U$USERNAME%$PASSWORD')
+ elif t == "base.rw1":
+ plansmbtorture4testsuite(t, "s3dc", '//$SERVER_IP/tmp -U$USERNAME%$PASSWORD')
+ plansmbtorture4testsuite(t, "s3dc", '//$SERVER_IP/valid-users-tmp -U$USERNAME%$PASSWORD')
+ plansmbtorture4testsuite(t, "s3dc", '//$SERVER_IP/write-list-tmp -U$USERNAME%$PASSWORD')
+ plansmbtorture4testsuite(t, "plugin_s4_dc", '//$SERVER/tmp -U$USERNAME%$PASSWORD')
else:
plansmbtorture4testsuite(t, "s3dc", '//$SERVER_IP/tmp -U$USERNAME%$PASSWORD')
plansmbtorture4testsuite(t, "plugin_s4_dc", '//$SERVER/tmp -U$USERNAME%$PASSWORD')
--
1.8.1
More information about the samba-technical
mailing list