impersonation patchset part2 (Re: FLAG_CASELESS_PATHNAMES/SMBFLG_CASELESS in set_current_service())
Stefan Metzmacher
metze at samba.org
Thu Jun 14 18:01:13 UTC 2018
Hi Jeremy,
>>>> I'm wondering about the mess regarding conn->case_sensitive we have in
>>>> set_current_service(). Is this really needed?
>>>
>>> I hope not. I'd really like to get rid of it.
>>
>> So can we just remove this completely without breaking any existing
>> setups? So we would also completely ignore FLAG_CASELESS_PATHNAMES
>> and also remove the "case sensitive" option. Or just let that
>> option only control the return of FILE_CASE_SENSITIVE_SEARCH.
>
> I think so - at least we need to ensure it passes all
> existing tests of course. Having it affect FILE_CASE_SENSITIVE_SEARCH
> file system attribute return seems to be the right option.
For now I kept it, but only called from switch_message().
All other places only do chdir.
>>>> Windows servers ignore FLAG_CASELESS_PATHNAMES completely,
>>>> but we do some magic just for smbclient and cifs.ko.
>>>> Only [lib]smbclient supports changing the flag on a per request basis,
>>>> but is it really used in practice?
>>>
>>> No.
>>
>> So we can really remove smbc_setOptionCaseSensitive() ?
>> And the "case_sensitive" smbclient command?
>
> Yes, I think we can.
And don't worry about the ABI change?
>> And also don't look at FILE_CASE_SENSITIVE_SEARCH anymore in the client.
Ok, and that won't silently change the behaviour of libsmbclient callers?
>>>> Can't we just set conn->case_sensitive based on, smb1 unix extension
>>>> being used on the share?
>>
>>> Yes, I think so.
>>
>> Similar to req->posix_pathnames?
>
> Yes.
Ok.
>>>> Does it really make sense to have an 'case sensitive' smb.conf option?
>>>> Do we really want to have conn->case_sensitive = true for non-unix
>>>> extensions clients?
>>>
>>> In theory Windows can support this mode (you have to set
>>> a registry switch and reboot).
>>>
>>> https://blogs.msdn.microsoft.com/commandline/2018/02/28/per-directory-case-sensitivity-and-wsl/
>>>
>>> "Case sensitivity in Windows
>>>
>>> The Windows NT family of operating systems (including Windows 10) has
>>> always had the ability to perform case sensitive file system operations.
>>> Applications can pass the FILE_FLAG_POSIX_SEMANTICS flag to the CreateFile
>>> API to indicate that they want the path to be treated as case sensitive.
>>> However, for compatibility reasons, there is a global registry key that
>>> overrides this behavior; when this key is set, all file operations are
>>> case insensitive, even when the FILE_FLAG_POSIX_SEMANTICS flag is
>>> specified. Since Windows XP, this has been the default."
>>>
>>> In practice no one ever does this (changes the registry key).
>>
>> Ok, I thougt FILE_FLAG_POSIX_SEMANTICS would be part of the unix extensions.
>
> It's being used internally to specify a POSIX open to
> the VFS, and currently inside SMB1 NTCreateX if the client sets it
> we change our case semantics, then strip it out of the
> passed in flags.
>
> See the code in source3/smbd/nttrans.c:
>
> 530 if (file_attributes & FILE_FLAG_POSIX_SEMANTICS) {
> 531 case_state = set_posix_case_semantics(ctx, conn);
> 532 if (!case_state) {
> 533 reply_nterror(req, NT_STATUS_NO_MEMORY);
> 534 goto out;
> 535 }
> 536 }
> ...
> 559 /*
> 560 * Bug #6898 - clients using Windows opens should
> 561 * never be able to set this attribute into the
> 562 * VFS.
> 563 */
> 564 file_attributes &= ~FILE_FLAG_POSIX_SEMANTICS;
>
> So currently it does change server behavior if set by
> the client, but this is not what Windows does.
>
> I think we can probably just strip it out if sent from
> the client, and rely on the SMB1 code paths that forcibly
> add it on a POSIX open/mkdir request, and my new code paths
> that forcibly add it on receiving a SMB3+ posix open context.
>
> e.g. remove the lines 530-536 above.
Ok.
>>>> Do we really need a 'nocase'/'ignorecase' mount option?
>>>>
>>>> In libsmbclient it seems that SMBC_server_internal() will
>>>> use cli_get_fs_attr_info() and set may fill in
>>>> FILE_CASE_SENSITIVE_SEARCH if the server supports it
>>>> (samba does).
>>>>
>>>> To me it seems that smbclient and libsmbclient behave differently,
>>>> because smbclient doesn't use SMBC_server[_internal]()
>>>> and doesn't call cli_get_fs_attr_info().
>>>>
>>>> smbclient has the "case_sensitive" operation to toggle
>>>> FILE_CASE_SENSITIVE_SEARCH, which later leads to
>>>> FLAG_CASELESS_PATHNAMES=0.
>>>>
>>>> This makes it really difficult to progress on my impersonation patches,
>>>> because we don't have the per request FLAG_CASELESS_PATHNAMES available
>>>> when we restore the impersonation based on just the connection/vuid.
>>>
>>> Yes, I can believe this is all muddled up, as in SMB1
>>> there are many pathname-based operations - this is what
>>> FLAG_CASELESS_PATHNAMES in an SMB1 request is supposed
>>> to be for. Without the registry key set I'm guessing Windows
>>> servers just ignore it (would be interesting to know if
>>> they ignore it even with the registry key sey).
>>
>> [MS-CIFS] claims that it's completely ignored by windows.
>>
>> <26> Section 2.2.3.1: This bit is ignored by Windows systems, which
>> always handle pathnames as
>> case-insensitive.
> Yes, we should ignore it too and remove from all
> server code I think.
Ok.
>>> Here's what we currently do in the server code:
>>>
>>> 132 /*
>>> 133 * Obey the client case sensitivity requests - only for clients that
>>> 134 * support it. */
>>> 135 switch (lp_case_sensitive(snum)) {
>>> 136 case Auto:
>>> 137 /*
>>> 138 * We need this uglyness due to DOS/Win9x clients that lie
>>> 139 * about case insensitivity. */
>>> 140 ra_type = get_remote_arch();
>>> 141 if (conn->sconn->using_smb2) {
>>> 142 conn->case_sensitive = false;
>>> 143 } else if ((ra_type != RA_SAMBA) && (ra_type != RA_CIFSFS)) {
>>> 144 /*
>>> 145 * Client can't support per-packet case sensitive
>>> 146 * pathnames. */
>>> 147 conn->case_sensitive = false;
>>> 148 } else {
>>> 149 conn->case_sensitive =
>>> 150 !(flags & FLAG_CASELESS_PATHNAMES);
>>> 151 }
>>> 152 break;
>>> 153 case True:
>>> 154 conn->case_sensitive = true;
>>> 155 break;
>>> 156 default:
>>> 157 conn->case_sensitive = false;
>>> 158 break;
>>> 159 }
>>>
>>> If we decide to ignore FLAG_CASELESS_PATHNAMES (which I'm in favor
>>> of) then we can ditch the whole lp_case_sensitive(snum) paths and
>>> just say - Windows opens (SMB1 & SMB2+) means case insensitive unless
>>> FILE_FLAG_POSIX_SEMANTICS set on SMB2_Create or SMB1 NTCreate request.
>>
>> So we remove the whole section?
>
> Yes.
Ok.
>>> POSIX opens (SMB1 global switch or SMB3 POSIX create context)
>>> always mean case sensitive.
>>
>> Good.
>>
>>>> Can this all be done by FILE_FLAG_POSIX_SEMANTICS/ATTR_POSIX_SEMANTICS?
>>>> Where we also have req->posix_pathnames and UCF_POSIX_PATHNAMES?
>>>
>>> Yeah, I think so - see above.
>>>
>>>> Any ideas how to move forward here?
>>>
>>> Does my plan work ? The only issues I can see are when we're
>>> running on a Linux system that actually *has* a case-insensitive
>>> filesystem underneath (XFS or ZFS mounted in case-insensitive mode).
>>
>> Then the filesystem should not announce FILE_CASE_SENSITIVE_SEARCH.
>
> Yes, that makes sense. The client can then know that
> even if it asks for POSIX extensions it will not get
> case-sensitive pathnames.
Should we keep the "case sensitive" option to control this?
Or should we better remove it completely.
And autodetect FILE_CASE_SENSITIVE_SEARCH from the local filesystem
or add a new separate option for it?
>>> We have 2 cases:
>>>
>>> 1). Hide case-sensitivity or case-insensitivity of underlying UNIX
>>> filesystem (always expose case-insensitive semantics to SMB client). This
>>> is the normal SMB1/2/3 case for Windows and Mac clients.
>>>
>>> 2). Expose case-sensitivity or case-insensitivity of underlying UNIX
>>> filesystem (SMB client sees unfiltered view of mounted filesystem).
>>> This is the SMB1-posix extensions, SMB3+-posix extensions view.
>>>
>>> When a client sends FILE_FLAG_POSIX_SEMANTICS in a Windows-style
>>> open, do we want to select case #1 (as Windows does without
>>> registry key change) ? Or do we want to select case #2 (which
>>> I think we do at present in smbd) ?
>>
>> If possible I'd ignore that unless it breaks existing setups
>> and that's where I'm unsure.
>
> You mean select option #1 above ?
I guess so.
>>> Feel free to call and discuss if this is confusing. I'll
>>> hapily review patches that remove 'conn->case_sensitive'.
>>>
>>> I think we need to keep the case-folding code in incoming
>>> pathnames (as this helps a *lot* with large directory optimizations
>>> in the exposing Windows case insensitivity on underlying
>>> UNIX case sensitive filesystems code paths), but these
>>> can be done inside filename_convert without needing
>>> to flip state back and forth on impersonation.
>>
>> Sounds good.
>>
>> Today worked on the attached patches in order to clean up the
>> existing code and set_current_user_info and set_current_service
>> to change_to_user().
>>
>> But adding the chdir_current_service seems to fail.
>> Any idea? It fails with ACCESS_DENIED, but I didn't have time to look
>> at details I'll do that tomorrow.
>
> I'll try and take a look later this week if you
> don't figure it out first.
I found it, during make_connection_snum() we don't have
conn->origpath setup while calling change_to_user(),
so I use a conn->tcon_done = true at the end of make_connection_snum()
and only try chdir_current_service() from chane_to_user().
The patches didn't pass autobuild, but I haven't checked yet if
it was just a flakey test again.
metze
-------------- next part --------------
From 90e5dd54834125ad18df332566718c8882a32f52 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Fri, 25 May 2018 13:40:12 +0200
Subject: [PATCH 01/30] s3:lib: add caching to set_current_user_info()
Currently we do that in the caller, but we use global
cache anyway, so we can simplify the callers.
Signed-off-by: Stefan Metzmacher <metze at samba.org>
Reviewed-by: Ralph Boehme <slow at samba.org>
---
source3/lib/substitute.c | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/source3/lib/substitute.c b/source3/lib/substitute.c
index 9fdc5ca1edc7..ea227c5ab68d 100644
--- a/source3/lib/substitute.c
+++ b/source3/lib/substitute.c
@@ -247,6 +247,17 @@ static const char *get_smb_user_name(void)
void set_current_user_info(const char *smb_name, const char *unix_name,
const char *domain)
{
+ static const void *last_smb_name;
+ static const void *last_unix_name;
+ static const void *last_domain;
+
+ if (likely(last_smb_name == smb_name &&
+ last_unix_name == unix_name &&
+ last_domain == domain))
+ {
+ return;
+ }
+
fstrcpy(current_user_info.smb_name, smb_name);
fstrcpy(current_user_info.unix_name, unix_name);
fstrcpy(current_user_info.domain, domain);
@@ -255,6 +266,10 @@ void set_current_user_info(const char *smb_name, const char *unix_name,
* has already been sanitised in register_existing_vuid. */
sub_set_smb_name(current_user_info.smb_name);
+
+ last_smb_name = smb_name;
+ last_unix_name = unix_name;
+ last_domain = domain;
}
/*******************************************************************
--
2.17.1
From 22fbc8363c64843bd721eb10a933bac6bcb6ea4f Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Fri, 25 May 2018 13:58:04 +0200
Subject: [PATCH 02/30] smbd: remove xconn->client->last_session_id based
set_current_user_info() caching
Signed-off-by: Stefan Metzmacher <metze at samba.org>
Reviewed-by: Ralph Boehme <slow at samba.org>
---
source3/librpc/idl/smbXsrv.idl | 1 -
source3/smbd/process.c | 8 ++------
source3/smbd/smb2_server.c | 9 +++------
3 files changed, 5 insertions(+), 13 deletions(-)
diff --git a/source3/librpc/idl/smbXsrv.idl b/source3/librpc/idl/smbXsrv.idl
index d3f8d30d1e30..26758113af47 100644
--- a/source3/librpc/idl/smbXsrv.idl
+++ b/source3/librpc/idl/smbXsrv.idl
@@ -123,7 +123,6 @@ interface smbXsrv
* this session_table is used for SMB1 and SMB2,
*/
[ignore] struct smbXsrv_session_table *session_table;
- [ignore] hyper last_session_id;
/*
* this tcon_table is only used for SMB1.
*/
diff --git a/source3/smbd/process.c b/source3/smbd/process.c
index 1c54ca491462..29249ea9a759 100644
--- a/source3/smbd/process.c
+++ b/source3/smbd/process.c
@@ -1571,13 +1571,9 @@ static connection_struct *switch_message(uint8_t type, struct smb_request *req)
}
}
- if (session_tag != xconn->client->last_session_id) {
- struct user_struct *vuser = NULL;
+ if (session != NULL) {
+ struct user_struct *vuser = session->compat;
- xconn->client->last_session_id = session_tag;
- if (session) {
- vuser = session->compat;
- }
if (vuser) {
set_current_user_info(
vuser->session_info->unix_info->sanitized_username,
diff --git a/source3/smbd/smb2_server.c b/source3/smbd/smb2_server.c
index 177e5ffc2f2f..a82161879c13 100644
--- a/source3/smbd/smb2_server.c
+++ b/source3/smbd/smb2_server.c
@@ -1964,12 +1964,9 @@ static NTSTATUS smbd_smb2_request_check_session(struct smbd_smb2_request *req)
return NT_STATUS_INVALID_HANDLE;
}
- if (in_session_id != req->xconn->client->last_session_id) {
- req->xconn->client->last_session_id = in_session_id;
- set_current_user_info(session_info->unix_info->sanitized_username,
- session_info->unix_info->unix_name,
- session_info->info->domain_name);
- }
+ set_current_user_info(session_info->unix_info->sanitized_username,
+ session_info->unix_info->unix_name,
+ session_info->info->domain_name);
return NT_STATUS_OK;
}
--
2.17.1
From daeaee05fe2d30cca5c4468cd9d3d5a68c870822 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Wed, 13 Jun 2018 11:03:01 +0200
Subject: [PATCH 03/30] smbd: split out set_current_case_sensitive() and
chdir_current_service() functions
We'll soon use them independend from set_current_service().
Signed-off-by: Stefan Metzmacher <metze at samba.org>
Reviewed-by: Ralph Boehme <slow at samba.org>
---
source3/smbd/proto.h | 2 ++
source3/smbd/service.c | 79 ++++++++++++++++++++++++++++--------------
2 files changed, 55 insertions(+), 26 deletions(-)
diff --git a/source3/smbd/proto.h b/source3/smbd/proto.h
index 2851d6d5f7df..a6b8a05cb4c6 100644
--- a/source3/smbd/proto.h
+++ b/source3/smbd/proto.h
@@ -1061,6 +1061,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);
+void set_current_case_sensitive(connection_struct *conn, uint16_t flags);
+bool chdir_current_service(connection_struct *conn);
bool set_current_service(connection_struct *conn, uint16_t 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 75a47dee0caa..35b1a4617e85 100644
--- a/source3/smbd/service.c
+++ b/source3/smbd/service.c
@@ -90,40 +90,17 @@ bool set_conn_connectpath(connection_struct *conn, const char *connectpath)
Load parameters specific to a connection/service.
****************************************************************************/
-bool set_current_service(connection_struct *conn, uint16_t flags, bool do_chdir)
+void set_current_case_sensitive(connection_struct *conn, uint16_t flags)
{
int snum;
enum remote_arch_types ra_type;
- if (!conn) {
- last_conn = NULL;
- return(False);
- }
-
- conn->lastused_count++;
+ SMB_ASSERT(conn != NULL);
snum = SNUM(conn);
- {
- struct smb_filename connectpath_fname = {
- .base_name = conn->connectpath
- };
- struct smb_filename origpath_fname = {
- .base_name = conn->origpath
- };
-
- if (do_chdir &&
- vfs_ChDir(conn, &connectpath_fname) != 0 &&
- vfs_ChDir(conn, &origpath_fname) != 0) {
- DEBUG(((errno!=EACCES)?0:3),
- ("chdir (%s) failed, reason: %s\n",
- conn->connectpath, strerror(errno)));
- return(False);
- }
- }
-
if ((conn == last_conn) && (last_flags == flags)) {
- return(True);
+ return;
}
last_conn = conn;
@@ -157,6 +134,56 @@ bool set_current_service(connection_struct *conn, uint16_t flags, bool do_chdir)
conn->case_sensitive = false;
break;
}
+ return;
+}
+
+bool chdir_current_service(connection_struct *conn)
+{
+ const struct smb_filename connectpath_fname = {
+ .base_name = conn->connectpath,
+ };
+ const struct smb_filename origpath_fname = {
+ .base_name = conn->origpath,
+ };
+ int ret;
+
+ conn->lastused_count++;
+
+ ret = vfs_ChDir(conn, &connectpath_fname);
+ if (ret != 0) {
+ DEBUG(((errno!=EACCES)?0:3),
+ ("chdir (%s) failed, reason: %s\n",
+ conn->connectpath, strerror(errno)));
+ return false;
+ }
+
+ ret = vfs_ChDir(conn, &origpath_fname);
+ if (ret != 0) {
+ DEBUG(((errno!=EACCES)?0:3),
+ ("chdir (%s) failed, reason: %s\n",
+ conn->origpath, strerror(errno)));
+ return false;
+ }
+
+ return true;
+}
+
+bool set_current_service(connection_struct *conn, uint16_t flags, bool do_chdir)
+{
+ bool ok;
+
+ if (conn == NULL) {
+ return false;
+ }
+
+ if (do_chdir) {
+ ok = chdir_current_service(conn);
+ if (!ok) {
+ return false;
+ }
+ }
+
+ set_current_case_sensitive(conn, flags);
return true;
}
--
2.17.1
From 5578489197a98f69ca4f0c0158eeb6042e6e2ad9 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Wed, 13 Jun 2018 11:23:42 +0200
Subject: [PATCH 04/30] smbd: call set_current_case_sensitive() before
chdir_current_service()
I guess we better setup conn->case_sensitive before doing the
vfs_ChDir() calls, so we have a consistent result everytime.
Otherwise vfs_Chdir() would get conn->case_sensitive from
last request.
Signed-off-by: Stefan Metzmacher <metze at samba.org>
Reviewed-by: Ralph Boehme <slow at samba.org>
---
source3/smbd/service.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/source3/smbd/service.c b/source3/smbd/service.c
index 35b1a4617e85..484461dfee93 100644
--- a/source3/smbd/service.c
+++ b/source3/smbd/service.c
@@ -176,6 +176,8 @@ bool set_current_service(connection_struct *conn, uint16_t flags, bool do_chdir)
return false;
}
+ set_current_case_sensitive(conn, flags);
+
if (do_chdir) {
ok = chdir_current_service(conn);
if (!ok) {
@@ -183,7 +185,6 @@ bool set_current_service(connection_struct *conn, uint16_t flags, bool do_chdir)
}
}
- set_current_case_sensitive(conn, flags);
return true;
}
--
2.17.1
From d0b7edee08a8ce57f9515e202f3c61b4f7500d61 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Thu, 3 May 2018 15:04:30 +0200
Subject: [PATCH 05/30] smbd: let check_user_ok() construct ent->session_info
in one coherent block
We should finish manipulating ent->session_info before filling
conn->session_info. And conn->session_info should be not be changed.
Use git show -U15.
Signed-off-by: Stefan Metzmacher <metze at samba.org>
Reviewed-by: Ralph Boehme <slow at samba.org>
---
source3/smbd/uid.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/source3/smbd/uid.c b/source3/smbd/uid.c
index b24ae3cc3b07..0af47831a4de 100644
--- a/source3/smbd/uid.c
+++ b/source3/smbd/uid.c
@@ -240,6 +240,14 @@ static bool check_user_ok(connection_struct *conn,
return false;
}
+ if (admin_user) {
+ DEBUG(2,("check_user_ok: user %s is an admin user. "
+ "Setting uid as %d\n",
+ ent->session_info->unix_info->unix_name,
+ sec_initial_uid() ));
+ ent->session_info->unix_token->uid = sec_initial_uid();
+ }
+
/*
* It's actually OK to call check_user_ok() with
* vuid == UID_FIELD_INVALID as called from change_to_user_by_session().
@@ -265,14 +273,6 @@ static bool check_user_ok(connection_struct *conn,
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",
- conn->session_info->unix_info->unix_name,
- sec_initial_uid() ));
- conn->session_info->unix_token->uid = sec_initial_uid();
- }
-
return(True);
}
--
2.17.1
From 6fa5049306699ebfd2bac4f1034ccc9db96e6129 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Tue, 12 Jun 2018 15:39:51 +0200
Subject: [PATCH 06/30] smbd: simplify the logic in change_to_user()
We can return early if (vuser == NULL).
Signed-off-by: Stefan Metzmacher <metze at samba.org>
Reviewed-by: Ralph Boehme <slow at samba.org>
---
source3/smbd/uid.c | 17 ++++++++---------
1 file changed, 8 insertions(+), 9 deletions(-)
diff --git a/source3/smbd/uid.c b/source3/smbd/uid.c
index 0af47831a4de..dc675271d279 100644
--- a/source3/smbd/uid.c
+++ b/source3/smbd/uid.c
@@ -378,15 +378,6 @@ bool change_to_user(connection_struct *conn, uint64_t vuid)
}
vuser = get_valid_user_struct(conn->sconn, vuid);
-
- if ((current_user.conn == conn) &&
- (vuser != NULL) && (current_user.vuid == vuid) &&
- (current_user.ut.uid == vuser->session_info->unix_token->uid)) {
- DEBUG(4,("Skipping user change - already "
- "user\n"));
- return(True);
- }
-
if (vuser == NULL) {
/* Invalid vuid sent */
DEBUG(2,("Invalid vuid %llu used on share %s.\n",
@@ -395,6 +386,14 @@ bool change_to_user(connection_struct *conn, uint64_t vuid)
return false;
}
+ if ((current_user.conn == conn) &&
+ (current_user.vuid == vuid) &&
+ (current_user.ut.uid == vuser->session_info->unix_token->uid))
+ {
+ DBG_INFO("Skipping user change - already user\n"));
+ return true;
+ }
+
return change_to_user_internal(conn, vuser->session_info, vuid);
}
--
2.17.1
From 88216117aab9b6cd22938c7e9511e20a9673c2ea Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Tue, 12 Jun 2018 15:39:51 +0200
Subject: [PATCH 07/30] smbd: move current_user caching to
change_to_user_internal()
Note that (current_user.vuid == vuid) also works with
UID_FIELD_INVALID.
Signed-off-by: Stefan Metzmacher <metze at samba.org>
Reviewed-by: Ralph Boehme <slow at samba.org>
---
source3/smbd/uid.c | 23 ++++++++---------------
1 file changed, 8 insertions(+), 15 deletions(-)
diff --git a/source3/smbd/uid.c b/source3/smbd/uid.c
index dc675271d279..af8a60d99eed 100644
--- a/source3/smbd/uid.c
+++ b/source3/smbd/uid.c
@@ -293,6 +293,14 @@ static bool change_to_user_internal(connection_struct *conn,
gid_t *group_list = NULL;
bool ok;
+ if ((current_user.conn == conn) &&
+ (current_user.vuid == vuid) &&
+ (current_user.ut.uid == session_info->unix_token->uid))
+ {
+ DBG_INFO("Skipping user change - already user\n");
+ return true;
+ }
+
snum = SNUM(conn);
ok = check_user_ok(conn, vuid, session_info, snum);
@@ -386,14 +394,6 @@ bool change_to_user(connection_struct *conn, uint64_t vuid)
return false;
}
- if ((current_user.conn == conn) &&
- (current_user.vuid == vuid) &&
- (current_user.ut.uid == vuser->session_info->unix_token->uid))
- {
- DBG_INFO("Skipping user change - already user\n"));
- return true;
- }
-
return change_to_user_internal(conn, vuser->session_info, vuid);
}
@@ -403,13 +403,6 @@ static bool change_to_user_by_session(connection_struct *conn,
SMB_ASSERT(conn != NULL);
SMB_ASSERT(session_info != NULL);
- if ((current_user.conn == conn) &&
- (current_user.ut.uid == session_info->unix_token->uid)) {
- DEBUG(7, ("Skipping user change - already user\n"));
-
- return true;
- }
-
return change_to_user_internal(conn, session_info, UID_FIELD_INVALID);
}
--
2.17.1
From 323f73474ccb1c21853d653e0f9b118472b99695 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Fri, 25 May 2018 14:22:43 +0200
Subject: [PATCH 08/30] smbd: call set_current_user_info() in
change_to_user_internal() and pop_conn_ctx()
change_to_user() should be the one and only function for the whole
impersonation processing. So we also need to stack the
set_current_user_info() information for become_user/unbecome_user.
Signed-off-by: Stefan Metzmacher <metze at samba.org>
Reviewed-by: Ralph Boehme <slow at samba.org>
---
source3/smbd/globals.h | 1 +
source3/smbd/uid.c | 9 +++++++++
2 files changed, 10 insertions(+)
diff --git a/source3/smbd/globals.h b/source3/smbd/globals.h
index 69db07a490ba..384599be1df2 100644
--- a/source3/smbd/globals.h
+++ b/source3/smbd/globals.h
@@ -89,6 +89,7 @@ extern uint16_t fnf_handle;
struct conn_ctx {
connection_struct *conn;
uint64_t vuid;
+ userdom_struct user_info;
};
/* A stack of current_user connection contexts. */
extern struct conn_ctx conn_ctx_stack[MAX_SEC_CTX_DEPTH];
diff --git a/source3/smbd/uid.c b/source3/smbd/uid.c
index af8a60d99eed..913d4f3aa0a1 100644
--- a/source3/smbd/uid.c
+++ b/source3/smbd/uid.c
@@ -301,6 +301,10 @@ static bool change_to_user_internal(connection_struct *conn,
return true;
}
+ set_current_user_info(session_info->unix_info->sanitized_username,
+ session_info->unix_info->unix_name,
+ session_info->info->domain_name);
+
snum = SNUM(conn);
ok = check_user_ok(conn, vuid, session_info, snum);
@@ -467,6 +471,7 @@ bool smbd_unbecome_authenticated_pipe_user(void)
static void push_conn_ctx(void)
{
struct conn_ctx *ctx_p;
+ extern userdom_struct current_user_info;
/* Check we don't overflow our stack */
@@ -480,6 +485,7 @@ static void push_conn_ctx(void)
ctx_p->conn = current_user.conn;
ctx_p->vuid = current_user.vuid;
+ ctx_p->user_info = current_user_info;
DEBUG(4, ("push_conn_ctx(%llu) : conn_ctx_stack_ndx = %d\n",
(unsigned long long)ctx_p->vuid, conn_ctx_stack_ndx));
@@ -501,6 +507,9 @@ static void pop_conn_ctx(void)
conn_ctx_stack_ndx--;
ctx_p = &conn_ctx_stack[conn_ctx_stack_ndx];
+ set_current_user_info(ctx_p->user_info.smb_name,
+ ctx_p->user_info.unix_name,
+ ctx_p->user_info.domain);
current_user.conn = ctx_p->conn;
current_user.vuid = ctx_p->vuid;
--
2.17.1
From 168685144decda59945ea2776d3288551878b413 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Thu, 14 Jun 2018 07:26:14 +0200
Subject: [PATCH 09/30] smbd: make it explicit that make_connection_snum()
returns NT_STATUS_OK on success
Signed-off-by: Stefan Metzmacher <metze at samba.org>
Reviewed-by: Ralph Boehme <slow 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 484461dfee93..d0932c0e9717 100644
--- a/source3/smbd/service.c
+++ b/source3/smbd/service.c
@@ -879,7 +879,7 @@ static NTSTATUS make_connection_snum(struct smbXsrv_connection *xconn,
dbgtext( "(pid %d)\n", (int)getpid() );
}
- return status;
+ return NT_STATUS_OK;
err_root_exit:
--
2.17.1
From 41edce3b342995fce1131834ed37c70e74bc5968 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Thu, 14 Jun 2018 07:27:43 +0200
Subject: [PATCH 10/30] smbd: remember that the tcon completely setup
connection_struct
Signed-off-by: Stefan Metzmacher <metze at samba.org>
Reviewed-by: Ralph Boehme <slow at samba.org>
---
source3/include/vfs.h | 1 +
source3/smbd/msdfs.c | 1 +
source3/smbd/service.c | 1 +
3 files changed, 3 insertions(+)
diff --git a/source3/include/vfs.h b/source3/include/vfs.h
index 81fb6c1f1453..54ab1604b799 100644
--- a/source3/include/vfs.h
+++ b/source3/include/vfs.h
@@ -436,6 +436,7 @@ typedef struct connection_struct {
char *connectpath;
char *origpath;
struct smb_filename *cwd_fname; /* Working directory. */
+ bool tcon_done;
struct vfs_handle_struct *vfs_handles; /* for the new plugins */
diff --git a/source3/smbd/msdfs.c b/source3/smbd/msdfs.c
index 1a6454b5c2be..a143e6d23cac 100644
--- a/source3/smbd/msdfs.c
+++ b/source3/smbd/msdfs.c
@@ -358,6 +358,7 @@ static NTSTATUS create_conn_struct_as_root(TALLOC_CTX *ctx,
}
conn->fs_capabilities = SMB_VFS_FS_CAPABILITIES(conn, &conn->ts_res);
+ conn->tcon_done = true;
*pconn = talloc_move(ctx, &conn);
return NT_STATUS_OK;
diff --git a/source3/smbd/service.c b/source3/smbd/service.c
index d0932c0e9717..612c88357ef7 100644
--- a/source3/smbd/service.c
+++ b/source3/smbd/service.c
@@ -879,6 +879,7 @@ static NTSTATUS make_connection_snum(struct smbXsrv_connection *xconn,
dbgtext( "(pid %d)\n", (int)getpid() );
}
+ conn->tcon_done = true;
return NT_STATUS_OK;
err_root_exit:
--
2.17.1
From 5edfc0ae8e5dfab7fc7d8b5f96fc521748c9034f Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Thu, 14 Jun 2018 11:15:10 +0200
Subject: [PATCH 11/30] smbd: call set_current_case_sensitive() before
change_to_user() in switch_message()
change_to_user() will soon call chdir_current_service() and we should
make sure conn->case_sensitive is prepared before calling vfs_ChDir().
Signed-off-by: Stefan Metzmacher <metze at samba.org>
Reviewed-by: Ralph Boehme <slow at samba.org>
---
source3/smbd/process.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/source3/smbd/process.c b/source3/smbd/process.c
index 29249ea9a759..7babf4d5699f 100644
--- a/source3/smbd/process.c
+++ b/source3/smbd/process.c
@@ -1599,6 +1599,8 @@ static connection_struct *switch_message(uint8_t type, struct smb_request *req)
return NULL;
}
+ set_current_case_sensitive(conn, SVAL(req->inbuf,smb_flg));
+
if (!change_to_user(conn,session_tag)) {
DEBUG(0, ("Error: Could not change to user. Removing "
"deferred open, mid=%llu.\n",
--
2.17.1
From c78c21caccc0cf74e39bfd3c7ef08e5e6c068320 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Wed, 13 Jun 2018 13:30:33 +0200
Subject: [PATCH 12/30] smbd: call chdir_current_service() in
change_to_user_internal() and pop_conn_ctx()
change_to_user() should be the one and only function for the whole
impersonation processing. So we also need to stack the
chdir_current_service() behaviour for become_user/unbecome_user.
Signed-off-by: Stefan Metzmacher <metze at samba.org>
Reviewed-by: Ralph Boehme <slow at samba.org>
---
source3/include/smb.h | 1 +
source3/smbd/globals.h | 1 +
source3/smbd/uid.c | 42 +++++++++++++++++++++++++++++++++++-------
3 files changed, 37 insertions(+), 7 deletions(-)
diff --git a/source3/include/smb.h b/source3/include/smb.h
index 5e83ee90afe2..8ebd1e1eb076 100644
--- a/source3/include/smb.h
+++ b/source3/include/smb.h
@@ -155,6 +155,7 @@ struct sys_notify_context {
struct current_user {
struct connection_struct *conn;
+ bool need_chdir;
uint64_t vuid; /* SMB2 compat */
struct security_unix_token ut;
struct security_token *nt_user_token;
diff --git a/source3/smbd/globals.h b/source3/smbd/globals.h
index 384599be1df2..09f87f90b1b1 100644
--- a/source3/smbd/globals.h
+++ b/source3/smbd/globals.h
@@ -88,6 +88,7 @@ extern uint16_t fnf_handle;
struct conn_ctx {
connection_struct *conn;
+ bool need_chdir;
uint64_t vuid;
userdom_struct user_info;
};
diff --git a/source3/smbd/uid.c b/source3/smbd/uid.c
index 913d4f3aa0a1..28411bcd70b7 100644
--- a/source3/smbd/uid.c
+++ b/source3/smbd/uid.c
@@ -51,6 +51,7 @@ bool change_to_guest(void)
set_sec_ctx(pass->pw_uid, pass->pw_gid, 0, NULL, NULL);
current_user.conn = NULL;
+ current_user.need_chdir = false;
current_user.vuid = UID_FIELD_INVALID;
TALLOC_FREE(pass);
@@ -367,14 +368,27 @@ static bool change_to_user_internal(connection_struct *conn,
current_user.ut.groups,
conn->session_info->security_token);
+ if (conn->tcon_done) {
+ ok = chdir_current_service(conn);
+ if (!ok) {
+ DBG_ERR("chdir_current_service() failed!\n");
+ return false;
+ }
+ }
+
current_user.conn = conn;
+ current_user.need_chdir = conn->tcon_done;
current_user.vuid = vuid;
- DEBUG(5, ("Impersonated user: uid=(%d,%d), gid=(%d,%d)\n",
- (int)getuid(),
- (int)geteuid(),
- (int)getgid(),
- (int)getegid()));
+ if (CHECK_DEBUGLVL(DBGLVL_INFO)) {
+ char cwdbuf[PATH_MAX+1] = { 0, };
+ DBG_INFO("Impersonated user: uid=(%d,%d), gid=(%d,%d), cwd=[%s]\n",
+ (int)getuid(),
+ (int)geteuid(),
+ (int)getgid(),
+ (int)getegid(),
+ getcwd(cwdbuf, sizeof(cwdbuf)));
+ }
return true;
}
@@ -423,6 +437,7 @@ bool smbd_change_to_root_user(void)
(int)getuid(),(int)geteuid(),(int)getgid(),(int)getegid()));
current_user.conn = NULL;
+ current_user.need_chdir = false;
current_user.vuid = UID_FIELD_INVALID;
return(True);
@@ -484,6 +499,7 @@ static void push_conn_ctx(void)
ctx_p = &conn_ctx_stack[conn_ctx_stack_ndx];
ctx_p->conn = current_user.conn;
+ ctx_p->need_chdir = current_user.need_chdir;
ctx_p->vuid = current_user.vuid;
ctx_p->user_info = current_user_info;
@@ -511,10 +527,22 @@ static void pop_conn_ctx(void)
ctx_p->user_info.unix_name,
ctx_p->user_info.domain);
current_user.conn = ctx_p->conn;
+ current_user.need_chdir = ctx_p->need_chdir;
current_user.vuid = ctx_p->vuid;
- ctx_p->conn = NULL;
- ctx_p->vuid = UID_FIELD_INVALID;
+ if (current_user.need_chdir) {
+ bool ok;
+
+ ok = chdir_current_service(current_user.conn);
+ if (!ok) {
+ DBG_ERR("chdir_current_service() failed!\n");
+ smb_panic("chdir_current_service() failed!\n");
+ }
+ }
+
+ *ctx_p = (struct conn_ctx) {
+ .vuid = UID_FIELD_INVALID,
+ };
}
/****************************************************************************
--
2.17.1
From 43985970feb894ffc24a7cd9939a4f9a6d56577b Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Thu, 14 Jun 2018 08:21:21 +0200
Subject: [PATCH 13/30] smbd: use conn->lastused_count++ directly in
process_blocking_lock_queue()
This avoids using set_current_service(), which will be removed shortly.
Signed-off-by: Stefan Metzmacher <metze at samba.org>
Reviewed-by: Ralph Boehme <slow at samba.org>
---
source3/smbd/blocking.c | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/source3/smbd/blocking.c b/source3/smbd/blocking.c
index 1e3a596d11fb..6cbf5c03e932 100644
--- a/source3/smbd/blocking.c
+++ b/source3/smbd/blocking.c
@@ -842,13 +842,10 @@ void process_blocking_lock_queue(struct smbd_server_connection *sconn)
DEBUG(10, ("Processing BLR = %p\n", blr));
- /* We use set_current_service so connections with
- * pending locks are not marked as idle.
+ /*
+ * Connections with pending locks are not marked as idle.
*/
-
- set_current_service(blr->fsp->conn,
- SVAL(blr->req->inbuf,smb_flg),
- false);
+ blr->fsp->conn->lastused_count++;
/*
* Remove the pending lock we're waiting on.
--
2.17.1
From 01421eb2b7b5a06d6c8445e4b11bab20b72526a3 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Thu, 14 Jun 2018 08:23:56 +0200
Subject: [PATCH 14/30] smbd: remove useless set_current_service(NULL,0,True)
from reload_services()
All this does is 'return false' as conn is NULL...
Signed-off-by: Stefan Metzmacher <metze at samba.org>
Reviewed-by: Ralph Boehme <slow at samba.org>
---
source3/smbd/server_reload.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/source3/smbd/server_reload.c b/source3/smbd/server_reload.c
index 3715a51c1529..0f0621e3e813 100644
--- a/source3/smbd/server_reload.c
+++ b/source3/smbd/server_reload.c
@@ -161,8 +161,5 @@ bool reload_services(struct smbd_server_connection *sconn,
reset_stat_cache();
flush_dfree_cache();
- /* this forces service parameters to be flushed */
- set_current_service(NULL,0,True);
-
return(ret);
}
--
2.17.1
From 1b288acfa80df9981f7fb7a36c525db2e0049e24 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Thu, 14 Jun 2018 08:27:50 +0200
Subject: [PATCH 15/30] smbd: call chdir_current_service() directly in
smbXsrv_tcon_disconnect()
There's no need to worry about conn->case_sensitive here.
Signed-off-by: Stefan Metzmacher <metze at samba.org>
Reviewed-by: Ralph Boehme <slow at samba.org>
---
source3/smbd/smbXsrv_tcon.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/source3/smbd/smbXsrv_tcon.c b/source3/smbd/smbXsrv_tcon.c
index 2804311d93c6..5ad557e1f5e6 100644
--- a/source3/smbd/smbXsrv_tcon.c
+++ b/source3/smbd/smbXsrv_tcon.c
@@ -969,11 +969,11 @@ NTSTATUS smbXsrv_tcon_disconnect(struct smbXsrv_tcon *tcon, uint64_t vuid)
if (tcon->compat) {
bool ok;
- ok = set_current_service(tcon->compat, 0, true);
+ ok = chdir_current_service(tcon->compat);
if (!ok) {
status = NT_STATUS_INTERNAL_ERROR;
DEBUG(0, ("smbXsrv_tcon_disconnect(0x%08x, '%s'): "
- "set_current_service() failed: %s\n",
+ "chdir_current_service() failed: %s\n",
tcon->global->tcon_global_id,
tcon->global->share_name,
nt_errstr(status)));
--
2.17.1
From 8ad61e53e92cba55693d6e847602e6869aa20f53 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Thu, 14 Jun 2018 08:29:45 +0200
Subject: [PATCH 16/30] smbd: remove set_current_service() from
defer_rename_done()
The change_to_user() above already called chdir_current_service().
And for smb2 we don't have per packet conn->case_sensitive anyway.
Signed-off-by: Stefan Metzmacher <metze at samba.org>
Reviewed-by: Ralph Boehme <slow at samba.org>
---
source3/smbd/smb2_setinfo.c | 6 ------
1 file changed, 6 deletions(-)
diff --git a/source3/smbd/smb2_setinfo.c b/source3/smbd/smb2_setinfo.c
index 9cd4fdf92024..4958bb8f16e7 100644
--- a/source3/smbd/smb2_setinfo.c
+++ b/source3/smbd/smb2_setinfo.c
@@ -302,12 +302,6 @@ static void defer_rename_done(struct tevent_req *subreq)
return;
}
- ok = set_current_service(state->smb2req->tcon->compat, 0, true);
- if (!ok) {
- tevent_req_nterror(state->req, NT_STATUS_ACCESS_DENIED);
- return;
- }
-
/* Do we still need to wait ? */
lck = get_existing_share_mode_lock(state->req, state->fsp->file_id);
if (lck == NULL) {
--
2.17.1
From 28ee63f87bc05c19e0b01df1bd87cd0f060f8694 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Thu, 14 Jun 2018 08:29:45 +0200
Subject: [PATCH 17/30] smbd: remove set_current_service() from
smbd_smb2_request_check_tcon()
The change_to_user() above already called chdir_current_service().
And for smb2 we don't have per packet conn->case_sensitive anyway.
Signed-off-by: Stefan Metzmacher <metze at samba.org>
Reviewed-by: Ralph Boehme <slow at samba.org>
---
source3/smbd/smb2_server.c | 4 ----
1 file changed, 4 deletions(-)
diff --git a/source3/smbd/smb2_server.c b/source3/smbd/smb2_server.c
index a82161879c13..3ecc1189a877 100644
--- a/source3/smbd/smb2_server.c
+++ b/source3/smbd/smb2_server.c
@@ -1846,10 +1846,6 @@ static NTSTATUS smbd_smb2_request_check_tcon(struct smbd_smb2_request *req)
return NT_STATUS_ACCESS_DENIED;
}
- if (!set_current_service(tcon->compat, 0, true)) {
- return NT_STATUS_ACCESS_DENIED;
- }
-
req->tcon = tcon;
req->last_tid = in_tid;
--
2.17.1
From a1339470dc1bf89c2e7547fb36506e9e9242f1de Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Thu, 14 Jun 2018 10:48:00 +0200
Subject: [PATCH 18/30] smbd: call SMBtdis after calling change_to_user()
followed by change_to_root_user()
This matches the logic we're using for the SMB2 TreeDisconnect.
It ensures we already called set_current_user() and
chdir_current_service() as user, before switching to root.
Signed-off-by: Stefan Metzmacher <metze at samba.org>
Reviewed-by: Ralph Boehme <slow at samba.org>
---
source3/smbd/process.c | 15 ++++++++++++++-
1 file changed, 14 insertions(+), 1 deletion(-)
diff --git a/source3/smbd/process.c b/source3/smbd/process.c
index 7babf4d5699f..d02c587e1120 100644
--- a/source3/smbd/process.c
+++ b/source3/smbd/process.c
@@ -1062,6 +1062,7 @@ force write permissions on print services.
#define CAN_IPC (1<<3) /* Must be paired with AS_USER */
#define AS_GUEST (1<<5) /* Must *NOT* be paired with AS_USER */
#define DO_CHDIR (1<<6)
+#define FORCE_AS_ROOT (1<<7) /* can be combined with AS_USER */
/*
define a list of possible SMB messages and their corresponding
@@ -1187,7 +1188,7 @@ static const struct smb_message_struct {
/* 0x6e */ { NULL, NULL, 0 },
/* 0x6f */ { NULL, NULL, 0 },
/* 0x70 */ { "SMBtcon",reply_tcon,0},
-/* 0x71 */ { "SMBtdis",reply_tdis,DO_CHDIR},
+/* 0x71 */ { "SMBtdis",reply_tdis,AS_USER|FORCE_AS_ROOT},
/* 0x72 */ { "SMBnegprot",reply_negprot,0},
/* 0x73 */ { "SMBsesssetupX",reply_sesssetup_and_X,0},
/* 0x74 */ { "SMBulogoffX",reply_ulogoffX, 0}, /* ulogoff doesn't give a valid TID */
@@ -1496,6 +1497,7 @@ static connection_struct *switch_message(uint8_t type, struct smb_request *req)
NTTIME now = timeval_to_nttime(&req->request_time);
struct smbXsrv_session *session = NULL;
NTSTATUS status;
+ bool as_root = false;
errno = 0;
@@ -1622,6 +1624,10 @@ static connection_struct *switch_message(uint8_t type, struct smb_request *req)
reply_nterror(req, NT_STATUS_ACCESS_DENIED);
return conn;
}
+
+ if (flags & FORCE_AS_ROOT) {
+ as_root = true;
+ }
} else if (flags & AS_GUEST) {
/*
* Does this protocol need to be run as guest? (Only archane
@@ -1632,6 +1638,13 @@ static connection_struct *switch_message(uint8_t type, struct smb_request *req)
return conn;
}
} else {
+ /*
+ * everything else needs to run as root
+ */
+ as_root = true;
+ }
+
+ if (as_root) {
/* This call needs to be run as root */
change_to_root_user();
}
--
2.17.1
From 428bda6fb82719a6534085a56b9c3dad0687d6c5 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Thu, 14 Jun 2018 11:08:56 +0200
Subject: [PATCH 19/30] smbd: don't use DO_CHDIR for SMBexit
This is more or less like SMBulogoffX, but with a different filter
to select files to be closed.
Compare file_close_pid() vs. file_close_user(), both
call close_file(NULL, fsp, SHUTDOWN_CLOSE) for each matching
fsp. As file_close_user() seems to work without calling
chdir_current_service(), file_close_pid() should also be fine.
Signed-off-by: Stefan Metzmacher <metze at samba.org>
Reviewed-by: Ralph Boehme <slow at samba.org>
---
source3/smbd/process.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/source3/smbd/process.c b/source3/smbd/process.c
index d02c587e1120..da3e14e2dd77 100644
--- a/source3/smbd/process.c
+++ b/source3/smbd/process.c
@@ -1092,7 +1092,7 @@ static const struct smb_message_struct {
/* 0x0e */ { "SMBctemp",reply_ctemp,AS_USER },
/* 0x0f */ { "SMBmknew",reply_mknew,AS_USER},
/* 0x10 */ { "SMBcheckpath",reply_checkpath,AS_USER},
-/* 0x11 */ { "SMBexit",reply_exit,DO_CHDIR},
+/* 0x11 */ { "SMBexit",reply_exit,0}, /* compare file_close_{pid,user}() */
/* 0x12 */ { "SMBlseek",reply_lseek,AS_USER},
/* 0x13 */ { "SMBlockread",reply_lockread,AS_USER},
/* 0x14 */ { "SMBwriteunlock",reply_writeunlock,AS_USER},
--
2.17.1
From 4eea0213082e3e1651f74fcc865cd2dc1f5b5845 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Thu, 14 Jun 2018 11:18:13 +0200
Subject: [PATCH 20/30] smbd: remove set_current_service() from
switch_message()
There's no user of DO_CHDIR anymore and all users
of AS_USER already call set_current_case_sensitive()
and chdir_current_service() (via change_to_user()).
There's no need to call them again via set_current_service().
Signed-off-by: Stefan Metzmacher <metze at samba.org>
Reviewed-by: Ralph Boehme <slow at samba.org>
---
source3/smbd/process.c | 7 -------
1 file changed, 7 deletions(-)
diff --git a/source3/smbd/process.c b/source3/smbd/process.c
index da3e14e2dd77..cb38a3d51357 100644
--- a/source3/smbd/process.c
+++ b/source3/smbd/process.c
@@ -1061,7 +1061,6 @@ force write permissions on print services.
#define TIME_INIT (1<<2)
#define CAN_IPC (1<<3) /* Must be paired with AS_USER */
#define AS_GUEST (1<<5) /* Must *NOT* be paired with AS_USER */
-#define DO_CHDIR (1<<6)
#define FORCE_AS_ROOT (1<<7) /* can be combined with AS_USER */
/*
@@ -1667,12 +1666,6 @@ static connection_struct *switch_message(uint8_t type, struct smb_request *req)
}
}
- if (!set_current_service(conn,SVAL(req->inbuf,smb_flg),
- (flags & (AS_USER|DO_CHDIR)
- ?True:False))) {
- reply_nterror(req, NT_STATUS_ACCESS_DENIED);
- return conn;
- }
conn->num_smb_operations++;
}
--
2.17.1
From 6b499a4971113bcee74f86d1da18e6b008204f17 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Thu, 14 Jun 2018 11:22:31 +0200
Subject: [PATCH 21/30] smbd: remove unused set_current_service()
Signed-off-by: Stefan Metzmacher <metze at samba.org>
Reviewed-by: Ralph Boehme <slow at samba.org>
---
source3/smbd/proto.h | 1 -
source3/smbd/service.c | 20 --------------------
2 files changed, 21 deletions(-)
diff --git a/source3/smbd/proto.h b/source3/smbd/proto.h
index a6b8a05cb4c6..0c4c68187c22 100644
--- a/source3/smbd/proto.h
+++ b/source3/smbd/proto.h
@@ -1063,7 +1063,6 @@ bool set_conn_connectpath(connection_struct *conn, const char *connectpath);
NTSTATUS set_conn_force_user_group(connection_struct *conn, int snum);
void set_current_case_sensitive(connection_struct *conn, uint16_t flags);
bool chdir_current_service(connection_struct *conn);
-bool set_current_service(connection_struct *conn, uint16_t flags, bool do_chdir);
void load_registry_shares(void);
int add_home_service(const char *service, const char *username, const char *homedir);
int find_service(TALLOC_CTX *ctx, const char *service, char **p_service_out);
diff --git a/source3/smbd/service.c b/source3/smbd/service.c
index 612c88357ef7..a22a0270cd79 100644
--- a/source3/smbd/service.c
+++ b/source3/smbd/service.c
@@ -168,26 +168,6 @@ bool chdir_current_service(connection_struct *conn)
return true;
}
-bool set_current_service(connection_struct *conn, uint16_t flags, bool do_chdir)
-{
- bool ok;
-
- if (conn == NULL) {
- return false;
- }
-
- set_current_case_sensitive(conn, flags);
-
- if (do_chdir) {
- ok = chdir_current_service(conn);
- if (!ok) {
- return false;
- }
- }
-
- return true;
-}
-
/****************************************************************************
do some basic sainity checks on the share.
This function modifies dev, ecode.
--
2.17.1
From 917490c25fff5c7ad4dc8208a1d6ddc9fddc0f07 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Fri, 25 May 2018 16:30:13 +0200
Subject: [PATCH 22/30] smbd: avoid calling set_current_user_info() twice with
AS_USER (SMB1)
It's already called via change_to_user().
Signed-off-by: Stefan Metzmacher <metze at samba.org>
Reviewed-by: Ralph Boehme <slow at samba.org>
---
source3/smbd/process.c | 22 ++++++++++++++--------
1 file changed, 14 insertions(+), 8 deletions(-)
diff --git a/source3/smbd/process.c b/source3/smbd/process.c
index cb38a3d51357..3041474ff62b 100644
--- a/source3/smbd/process.c
+++ b/source3/smbd/process.c
@@ -1495,6 +1495,7 @@ static connection_struct *switch_message(uint8_t type, struct smb_request *req)
struct smbXsrv_connection *xconn = req->xconn;
NTTIME now = timeval_to_nttime(&req->request_time);
struct smbXsrv_session *session = NULL;
+ bool need_set_current_user_info = false;
NTSTATUS status;
bool as_root = false;
@@ -1573,14 +1574,7 @@ static connection_struct *switch_message(uint8_t type, struct smb_request *req)
}
if (session != NULL) {
- struct user_struct *vuser = session->compat;
-
- if (vuser) {
- set_current_user_info(
- vuser->session_info->unix_info->sanitized_username,
- vuser->session_info->unix_info->unix_name,
- vuser->session_info->info->domain_name);
- }
+ need_set_current_user_info = true;
}
/* Does this call need to be run as the connected user? */
@@ -1609,6 +1603,7 @@ static connection_struct *switch_message(uint8_t type, struct smb_request *req)
reply_force_doserror(req, ERRSRV, ERRbaduid);
return conn;
}
+ need_set_current_user_info = false;
/* All NEED_WRITE and CAN_IPC flags must also have AS_USER. */
@@ -1643,6 +1638,17 @@ static connection_struct *switch_message(uint8_t type, struct smb_request *req)
as_root = true;
}
+ if (need_set_current_user_info) {
+ struct user_struct *vuser = session->compat;
+
+ if (vuser) {
+ set_current_user_info(
+ vuser->session_info->unix_info->sanitized_username,
+ vuser->session_info->unix_info->unix_name,
+ vuser->session_info->info->domain_name);
+ }
+ }
+
if (as_root) {
/* This call needs to be run as root */
change_to_root_user();
--
2.17.1
From 82d1dc8174b36c34066db0207e6ff176b2d00eff Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Fri, 25 May 2018 16:30:13 +0200
Subject: [PATCH 23/30] smbd: avoid calling set_current_user_info() twice with
.need_tcon (SMB2)
It's already called via change_to_user().
Signed-off-by: Stefan Metzmacher <metze at samba.org>
Reviewed-by: Ralph Boehme <slow at samba.org>
---
source3/smbd/smb2_server.c | 26 +++++++++++++++++++-------
1 file changed, 19 insertions(+), 7 deletions(-)
diff --git a/source3/smbd/smb2_server.c b/source3/smbd/smb2_server.c
index 3ecc1189a877..3250bc438562 100644
--- a/source3/smbd/smb2_server.c
+++ b/source3/smbd/smb2_server.c
@@ -1960,10 +1960,6 @@ static NTSTATUS smbd_smb2_request_check_session(struct smbd_smb2_request *req)
return NT_STATUS_INVALID_HANDLE;
}
- set_current_user_info(session_info->unix_info->sanitized_username,
- session_info->unix_info->unix_name,
- session_info->info->domain_name);
-
return NT_STATUS_OK;
}
@@ -2340,9 +2336,7 @@ NTSTATUS smbd_smb2_request_dispatch(struct smbd_smb2_request *req)
}
/*
- * Check if the client provided a valid session id,
- * if so smbd_smb2_request_check_session() calls
- * set_current_user_info().
+ * Check if the client provided a valid session id.
*
* As some command don't require a valid session id
* we defer the check of the session_status
@@ -2507,6 +2501,8 @@ NTSTATUS smbd_smb2_request_dispatch(struct smbd_smb2_request *req)
*
* smbd_smb2_request_check_tcon()
* calls change_to_user() on success.
+ * Which implies set_current_user_info()
+ * and set_current_service().
*/
status = smbd_smb2_request_check_tcon(req);
if (!NT_STATUS_IS_OK(status)) {
@@ -2522,6 +2518,22 @@ NTSTATUS smbd_smb2_request_dispatch(struct smbd_smb2_request *req)
return smbd_smb2_request_error(req,
NT_STATUS_ACCESS_DENIED);
}
+ } else if (call->need_session) {
+ struct auth_session_info *session_info = NULL;
+
+ /*
+ * Unless we also have need_tcon (see above),
+ * we still need to call set_current_user_info().
+ */
+
+ session_info = req->session->global->auth_session_info;
+ if (session_info == NULL) {
+ return NT_STATUS_INVALID_HANDLE;
+ }
+
+ set_current_user_info(session_info->unix_info->sanitized_username,
+ session_info->unix_info->unix_name,
+ session_info->info->domain_name);
}
if (req->was_encrypted || encryption_desired) {
--
2.17.1
From 41e9914e54efe3dce38418440430d51e19cd483f Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Wed, 23 May 2018 11:37:52 +0200
Subject: [PATCH 24/30] vfs_default: remove unused checks which are already
caught by vfs_offload_token_check_handles()
Signed-off-by: Stefan Metzmacher <metze at samba.org>
Reviewed-by: Ralph Boehme <slow at samba.org>
---
source3/modules/vfs_default.c | 10 ----------
1 file changed, 10 deletions(-)
diff --git a/source3/modules/vfs_default.c b/source3/modules/vfs_default.c
index 82afb054a137..58df47ee6b4c 100644
--- a/source3/modules/vfs_default.c
+++ b/source3/modules/vfs_default.c
@@ -1700,16 +1700,6 @@ static struct tevent_req *vfswrap_offload_write_send(
return tevent_req_post(req, ev);
}
- if (src_fsp->op == NULL) {
- tevent_req_nterror(req, NT_STATUS_INTERNAL_ERROR);
- return tevent_req_post(req, ev);
- }
-
- if (dest_fsp->op == NULL) {
- tevent_req_nterror(req, NT_STATUS_INTERNAL_ERROR);
- return tevent_req_post(req, ev);
- }
-
status = vfswrap_offload_write_loop(req);
if (!NT_STATUS_IS_OK(status)) {
tevent_req_nterror(req, status);
--
2.17.1
From c2b165bbf62392092a05a0266f85012521965a92 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Wed, 23 May 2018 11:37:52 +0200
Subject: [PATCH 25/30] vfs_btrfs: remove unused checks which are already
caught by vfs_offload_token_check_handles()
Signed-off-by: Stefan Metzmacher <metze at samba.org>
Reviewed-by: Ralph Boehme <slow at samba.org>
---
source3/modules/vfs_btrfs.c | 5 -----
1 file changed, 5 deletions(-)
diff --git a/source3/modules/vfs_btrfs.c b/source3/modules/vfs_btrfs.c
index 775e5f076116..4c07ebd36ecb 100644
--- a/source3/modules/vfs_btrfs.c
+++ b/source3/modules/vfs_btrfs.c
@@ -298,11 +298,6 @@ static struct tevent_req *btrfs_offload_write_send(struct vfs_handle_struct *han
return tevent_req_post(req, ev);
}
- if (src_fsp->op == NULL || dest_fsp->op == NULL) {
- tevent_req_nterror(req, NT_STATUS_INTERNAL_ERROR);
- return tevent_req_post(req, ev);
- }
-
if (do_locking) {
init_strict_lock_struct(src_fsp,
src_fsp->op->global->open_persistent_id,
--
2.17.1
From 273bccf3382572fa0ae3c3f2f11d89a3a3ecb648 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Wed, 23 May 2018 12:40:21 +0200
Subject: [PATCH 26/30] vfs_btrfs: update
s/btrfs_cc_state/btrfs_offload_write_state/ s/cc_state/state/
This matches our naming conventions used for tevent_req based functions.
Signed-off-by: Stefan Metzmacher <metze at samba.org>
Reviewed-by: Ralph Boehme <slow at samba.org>
---
source3/modules/vfs_btrfs.c | 60 ++++++++++++++++++++-----------------
1 file changed, 33 insertions(+), 27 deletions(-)
diff --git a/source3/modules/vfs_btrfs.c b/source3/modules/vfs_btrfs.c
index 4c07ebd36ecb..cc082f7fbbf3 100644
--- a/source3/modules/vfs_btrfs.c
+++ b/source3/modules/vfs_btrfs.c
@@ -196,7 +196,7 @@ static NTSTATUS btrfs_offload_read_recv(struct tevent_req *req,
return NT_STATUS_OK;
}
-struct btrfs_cc_state {
+struct btrfs_offload_write_state {
struct vfs_handle_struct *handle;
off_t copied;
struct tevent_req *subreq; /* non-null if passed to next VFS fn */
@@ -213,8 +213,8 @@ static struct tevent_req *btrfs_offload_write_send(struct vfs_handle_struct *han
off_t dest_off,
off_t num)
{
- struct tevent_req *req;
- struct btrfs_cc_state *cc_state;
+ struct tevent_req *req = NULL;
+ struct btrfs_offload_write_state *state = NULL;
struct btrfs_ioctl_clone_range_args cr_args;
struct lock_struct src_lck;
struct lock_struct dest_lck;
@@ -225,12 +225,13 @@ static struct tevent_req *btrfs_offload_write_send(struct vfs_handle_struct *han
bool do_locking = false;
NTSTATUS status;
- req = tevent_req_create(mem_ctx, &cc_state, struct btrfs_cc_state);
+ req = tevent_req_create(mem_ctx, &state,
+ struct btrfs_offload_write_state);
if (req == NULL) {
return NULL;
}
- cc_state->handle = handle;
+ state->handle = handle;
status = vfs_offload_token_db_fetch_fsp(btrfs_offload_ctx,
token, &src_fsp);
@@ -263,18 +264,19 @@ static struct tevent_req *btrfs_offload_write_send(struct vfs_handle_struct *han
}
if (!handle_offload_write) {
- cc_state->subreq = SMB_VFS_NEXT_OFFLOAD_WRITE_SEND(handle,
- cc_state, ev,
+ state->subreq = SMB_VFS_NEXT_OFFLOAD_WRITE_SEND(handle,
+ state,
+ ev,
fsctl,
token,
transfer_offset,
dest_fsp,
dest_off,
num);
- if (tevent_req_nomem(cc_state->subreq, req)) {
+ if (tevent_req_nomem(state->subreq, req)) {
return tevent_req_post(req, ev);
}
- tevent_req_set_callback(cc_state->subreq,
+ tevent_req_set_callback(state->subreq,
btrfs_offload_write_done,
req);
return req;
@@ -343,19 +345,20 @@ static struct tevent_req *btrfs_offload_write_send(struct vfs_handle_struct *han
(unsigned long long)cr_args.src_offset,
dest_fsp->fh->fd,
(unsigned long long)cr_args.dest_offset));
- cc_state->subreq = SMB_VFS_NEXT_OFFLOAD_WRITE_SEND(handle,
- cc_state, ev,
+ state->subreq = SMB_VFS_NEXT_OFFLOAD_WRITE_SEND(handle,
+ state,
+ ev,
fsctl,
token,
transfer_offset,
dest_fsp,
dest_off,
num);
- if (tevent_req_nomem(cc_state->subreq, req)) {
+ if (tevent_req_nomem(state->subreq, req)) {
return tevent_req_post(req, ev);
}
/* wait for subreq completion */
- tevent_req_set_callback(cc_state->subreq,
+ tevent_req_set_callback(state->subreq,
btrfs_offload_write_done,
req);
return req;
@@ -363,7 +366,7 @@ static struct tevent_req *btrfs_offload_write_send(struct vfs_handle_struct *han
DEBUG(5, ("BTRFS_IOC_CLONE_RANGE returned %d\n", ret));
/* BTRFS_IOC_CLONE_RANGE is all or nothing */
- cc_state->copied = num;
+ state->copied = num;
tevent_req_done(req);
return tevent_req_post(req, ev);
}
@@ -371,15 +374,17 @@ static struct tevent_req *btrfs_offload_write_send(struct vfs_handle_struct *han
/* only used if the request is passed through to next VFS module */
static void btrfs_offload_write_done(struct tevent_req *subreq)
{
- struct tevent_req *req = tevent_req_callback_data(
- subreq, struct tevent_req);
- struct btrfs_cc_state *cc_state = tevent_req_data(req,
- struct btrfs_cc_state);
+ struct tevent_req *req =
+ tevent_req_callback_data(subreq,
+ struct tevent_req);
+ struct btrfs_offload_write_state *state =
+ tevent_req_data(req,
+ struct btrfs_offload_write_state);
NTSTATUS status;
- status = SMB_VFS_NEXT_OFFLOAD_WRITE_RECV(cc_state->handle,
- cc_state->subreq,
- &cc_state->copied);
+ status = SMB_VFS_NEXT_OFFLOAD_WRITE_RECV(state->handle,
+ state->subreq,
+ &state->copied);
if (tevent_req_nterror(req, status)) {
return;
}
@@ -387,12 +392,13 @@ static void btrfs_offload_write_done(struct tevent_req *subreq)
}
static NTSTATUS btrfs_offload_write_recv(struct vfs_handle_struct *handle,
- struct tevent_req *req,
- off_t *copied)
+ struct tevent_req *req,
+ off_t *copied)
{
+ struct btrfs_offload_write_state *state =
+ tevent_req_data(req,
+ struct btrfs_offload_write_state);
NTSTATUS status;
- struct btrfs_cc_state *cc_state = tevent_req_data(req,
- struct btrfs_cc_state);
if (tevent_req_is_nterror(req, &status)) {
DEBUG(4, ("server side copy chunk failed: %s\n",
@@ -402,8 +408,8 @@ static NTSTATUS btrfs_offload_write_recv(struct vfs_handle_struct *handle,
}
DEBUG(10, ("server side copy chunk copied %llu\n",
- (unsigned long long)cc_state->copied));
- *copied = cc_state->copied;
+ (unsigned long long)state->copied));
+ *copied = state->copied;
tevent_req_received(req);
return NT_STATUS_OK;
}
--
2.17.1
From 8b19900874174305049c2c5d173f91fb3796a0c8 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Wed, 23 May 2018 12:40:21 +0200
Subject: [PATCH 27/30] vfs_btrfs: don't keep state->subreq in
btrfs_offload_write_send/recv()
This can be a local variable as used in most of our tevent_req based
code.
Signed-off-by: Stefan Metzmacher <metze at samba.org>
Reviewed-by: Ralph Boehme <slow at samba.org>
---
source3/modules/vfs_btrfs.c | 49 +++++++++++++++++++------------------
1 file changed, 25 insertions(+), 24 deletions(-)
diff --git a/source3/modules/vfs_btrfs.c b/source3/modules/vfs_btrfs.c
index cc082f7fbbf3..515859d6b90a 100644
--- a/source3/modules/vfs_btrfs.c
+++ b/source3/modules/vfs_btrfs.c
@@ -199,7 +199,6 @@ static NTSTATUS btrfs_offload_read_recv(struct tevent_req *req,
struct btrfs_offload_write_state {
struct vfs_handle_struct *handle;
off_t copied;
- struct tevent_req *subreq; /* non-null if passed to next VFS fn */
};
static void btrfs_offload_write_done(struct tevent_req *subreq);
@@ -215,6 +214,7 @@ static struct tevent_req *btrfs_offload_write_send(struct vfs_handle_struct *han
{
struct tevent_req *req = NULL;
struct btrfs_offload_write_state *state = NULL;
+ struct tevent_req *subreq = NULL;
struct btrfs_ioctl_clone_range_args cr_args;
struct lock_struct src_lck;
struct lock_struct dest_lck;
@@ -264,19 +264,19 @@ static struct tevent_req *btrfs_offload_write_send(struct vfs_handle_struct *han
}
if (!handle_offload_write) {
- state->subreq = SMB_VFS_NEXT_OFFLOAD_WRITE_SEND(handle,
- state,
- ev,
- fsctl,
- token,
- transfer_offset,
- dest_fsp,
- dest_off,
- num);
- if (tevent_req_nomem(state->subreq, req)) {
+ subreq = SMB_VFS_NEXT_OFFLOAD_WRITE_SEND(handle,
+ state,
+ ev,
+ fsctl,
+ token,
+ transfer_offset,
+ dest_fsp,
+ dest_off,
+ num);
+ if (tevent_req_nomem(subreq, req)) {
return tevent_req_post(req, ev);
}
- tevent_req_set_callback(state->subreq,
+ tevent_req_set_callback(subreq,
btrfs_offload_write_done,
req);
return req;
@@ -345,20 +345,20 @@ static struct tevent_req *btrfs_offload_write_send(struct vfs_handle_struct *han
(unsigned long long)cr_args.src_offset,
dest_fsp->fh->fd,
(unsigned long long)cr_args.dest_offset));
- state->subreq = SMB_VFS_NEXT_OFFLOAD_WRITE_SEND(handle,
- state,
- ev,
- fsctl,
- token,
- transfer_offset,
- dest_fsp,
- dest_off,
- num);
- if (tevent_req_nomem(state->subreq, req)) {
+ subreq = SMB_VFS_NEXT_OFFLOAD_WRITE_SEND(handle,
+ state,
+ ev,
+ fsctl,
+ token,
+ transfer_offset,
+ dest_fsp,
+ dest_off,
+ num);
+ if (tevent_req_nomem(subreq, req)) {
return tevent_req_post(req, ev);
}
/* wait for subreq completion */
- tevent_req_set_callback(state->subreq,
+ tevent_req_set_callback(subreq,
btrfs_offload_write_done,
req);
return req;
@@ -383,8 +383,9 @@ static void btrfs_offload_write_done(struct tevent_req *subreq)
NTSTATUS status;
status = SMB_VFS_NEXT_OFFLOAD_WRITE_RECV(state->handle,
- state->subreq,
+ subreq,
&state->copied);
+ TALLOC_FREE(subreq);
if (tevent_req_nterror(req, status)) {
return;
}
--
2.17.1
From 20ec082cf1cdc3b9bca54e4bbecff0445b4e4fbb Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Wed, 23 May 2018 11:54:58 +0200
Subject: [PATCH 28/30] smbd: add {become,change_to}_user_by_fsp() helper
functions
This can be used if a request operates on two fsp's,
e.g. the offload_write_send/recv code.
This is important if (at least) one of
the shares uses "force user".
Signed-off-by: Stefan Metzmacher <metze at samba.org>
Reviewed-by: Ralph Boehme <slow at samba.org>
---
source3/smbd/proto.h | 2 ++
source3/smbd/uid.c | 10 ++++++++++
2 files changed, 12 insertions(+)
diff --git a/source3/smbd/proto.h b/source3/smbd/proto.h
index 0c4c68187c22..f9f29230162c 100644
--- a/source3/smbd/proto.h
+++ b/source3/smbd/proto.h
@@ -1196,6 +1196,7 @@ NTSTATUS check_user_share_access(connection_struct *conn,
uint32_t *p_share_access,
bool *p_readonly_share);
bool change_to_user(connection_struct *conn, uint64_t vuid);
+bool change_to_user_by_fsp(struct files_struct *fsp);
bool smbd_change_to_root_user(void);
bool smbd_become_authenticated_pipe_user(struct auth_session_info *session_info);
bool smbd_unbecome_authenticated_pipe_user(void);
@@ -1204,6 +1205,7 @@ void unbecome_root(void);
void smbd_become_root(void);
void smbd_unbecome_root(void);
bool become_user(connection_struct *conn, uint64_t vuid);
+bool become_user_by_fsp(struct files_struct *fsp);
bool become_user_by_session(connection_struct *conn,
const struct auth_session_info *session_info);
bool unbecome_user(void);
diff --git a/source3/smbd/uid.c b/source3/smbd/uid.c
index 28411bcd70b7..aa6994ac53d0 100644
--- a/source3/smbd/uid.c
+++ b/source3/smbd/uid.c
@@ -415,6 +415,11 @@ bool change_to_user(connection_struct *conn, uint64_t vuid)
return change_to_user_internal(conn, vuser->session_info, vuid);
}
+bool change_to_user_by_fsp(struct files_struct *fsp)
+{
+ return change_to_user(fsp->conn, fsp->vuid);
+}
+
static bool change_to_user_by_session(connection_struct *conn,
const struct auth_session_info *session_info)
{
@@ -592,6 +597,11 @@ bool become_user(connection_struct *conn, uint64_t vuid)
return True;
}
+bool become_user_by_fsp(struct files_struct *fsp)
+{
+ return become_user(fsp->conn, fsp->vuid);
+}
+
bool become_user_by_session(connection_struct *conn,
const struct auth_session_info *session_info)
{
--
2.17.1
From 90c921a62b175034121103055c91c06d054bf06b Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Wed, 23 May 2018 12:03:02 +0200
Subject: [PATCH 29/30] vfs_btrfs: make use of become_user_by_fsp() in order to
switch between src and dst fsp
We can use become_user_by_fsp()/unbecome_user() as it spans only parts of
the btrfs_offload_write_send() function and never goes async in between.
This may matter if at least one share uses "force user".
Signed-off-by: Stefan Metzmacher <metze at samba.org>
Reviewed-by: Ralph Boehme <slow at samba.org>
---
source3/modules/vfs_btrfs.c | 46 +++++++++++++++++++++++++++++++++----
1 file changed, 41 insertions(+), 5 deletions(-)
diff --git a/source3/modules/vfs_btrfs.c b/source3/modules/vfs_btrfs.c
index 515859d6b90a..a11cb66d8e78 100644
--- a/source3/modules/vfs_btrfs.c
+++ b/source3/modules/vfs_btrfs.c
@@ -199,7 +199,26 @@ static NTSTATUS btrfs_offload_read_recv(struct tevent_req *req,
struct btrfs_offload_write_state {
struct vfs_handle_struct *handle;
off_t copied;
+ bool need_unbecome_user;
};
+
+static void btrfs_offload_write_cleanup(struct tevent_req *req,
+ enum tevent_req_state req_state)
+{
+ struct btrfs_offload_write_state *state =
+ tevent_req_data(req,
+ struct btrfs_offload_write_state);
+ bool ok;
+
+ if (!state->need_unbecome_user) {
+ return;
+ }
+
+ ok = unbecome_user();
+ SMB_ASSERT(ok);
+ state->need_unbecome_user = false;
+}
+
static void btrfs_offload_write_done(struct tevent_req *subreq);
static struct tevent_req *btrfs_offload_write_send(struct vfs_handle_struct *handle,
@@ -224,6 +243,7 @@ static struct tevent_req *btrfs_offload_write_send(struct vfs_handle_struct *han
bool handle_offload_write = true;
bool do_locking = false;
NTSTATUS status;
+ bool ok;
req = tevent_req_create(mem_ctx, &state,
struct btrfs_offload_write_state);
@@ -233,6 +253,8 @@ static struct tevent_req *btrfs_offload_write_send(struct vfs_handle_struct *han
state->handle = handle;
+ tevent_req_set_cleanup_fn(req, btrfs_offload_write_cleanup);
+
status = vfs_offload_token_db_fetch_fsp(btrfs_offload_ctx,
token, &src_fsp);
if (tevent_req_nterror(req, status)) {
@@ -289,6 +311,13 @@ static struct tevent_req *btrfs_offload_write_send(struct vfs_handle_struct *han
return tevent_req_post(req, ev);
}
+ ok = become_user_by_fsp(src_fsp);
+ if (!ok) {
+ tevent_req_nterror(req, NT_STATUS_ACCESS_DENIED);
+ return tevent_req_post(req, ev);
+ }
+ state->need_unbecome_user = true;
+
status = vfs_stat_fsp(src_fsp);
if (tevent_req_nterror(req, status)) {
return tevent_req_post(req, ev);
@@ -307,17 +336,24 @@ static struct tevent_req *btrfs_offload_write_send(struct vfs_handle_struct *han
num,
READ_LOCK,
&src_lck);
+ if (!SMB_VFS_STRICT_LOCK_CHECK(src_fsp->conn, src_fsp, &src_lck)) {
+ tevent_req_nterror(req, NT_STATUS_FILE_LOCK_CONFLICT);
+ return tevent_req_post(req, ev);
+ }
+ }
+
+ ok = unbecome_user();
+ SMB_ASSERT(ok);
+ state->need_unbecome_user = false;
+
+ if (do_locking) {
init_strict_lock_struct(dest_fsp,
- dest_fsp->op->global->open_persistent_id,
+ dest_fsp->op->global->open_persistent_id,
dest_off,
num,
WRITE_LOCK,
&dest_lck);
- if (!SMB_VFS_STRICT_LOCK_CHECK(src_fsp->conn, src_fsp, &src_lck)) {
- tevent_req_nterror(req, NT_STATUS_FILE_LOCK_CONFLICT);
- return tevent_req_post(req, ev);
- }
if (!SMB_VFS_STRICT_LOCK_CHECK(dest_fsp->conn, dest_fsp, &dest_lck)) {
tevent_req_nterror(req, NT_STATUS_FILE_LOCK_CONFLICT);
return tevent_req_post(req, ev);
--
2.17.1
From e85c3a09fd016f653380446b7310b24b61cbcb24 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Wed, 23 May 2018 12:03:02 +0200
Subject: [PATCH 30/30] vfs_default: make use of change_to_user_by_fsp() in
order to switch between src and dst fsp
This may matter if at least one share uses "force user".
Signed-off-by: Stefan Metzmacher <metze at samba.org>
Reviewed-by: Ralph Boehme <slow at samba.org>
---
source3/modules/vfs_default.c | 42 +++++++++++++++++++++++++++++++++++
1 file changed, 42 insertions(+)
diff --git a/source3/modules/vfs_default.c b/source3/modules/vfs_default.c
index 58df47ee6b4c..baeb285eeb34 100644
--- a/source3/modules/vfs_default.c
+++ b/source3/modules/vfs_default.c
@@ -1598,6 +1598,22 @@ struct vfswrap_offload_write_state {
size_t next_io_size;
};
+static void vfswrap_offload_write_cleanup(struct tevent_req *req,
+ enum tevent_req_state req_state)
+{
+ struct vfswrap_offload_write_state *state = tevent_req_data(
+ req, struct vfswrap_offload_write_state);
+ bool ok;
+
+ if (state->dst_fsp == NULL) {
+ return;
+ }
+
+ ok = change_to_user_by_fsp(state->dst_fsp);
+ SMB_ASSERT(ok);
+ state->dst_fsp = NULL;
+}
+
static NTSTATUS vfswrap_offload_write_loop(struct tevent_req *req);
static struct tevent_req *vfswrap_offload_write_send(
@@ -1616,6 +1632,7 @@ static struct tevent_req *vfswrap_offload_write_send(
size_t num = MIN(to_copy, COPYCHUNK_MAX_TOTAL_LEN);
files_struct *src_fsp = NULL;
NTSTATUS status;
+ bool ok;
req = tevent_req_create(mem_ctx, &state,
struct vfswrap_offload_write_state);
@@ -1633,6 +1650,8 @@ static struct tevent_req *vfswrap_offload_write_send(
.remaining = to_copy,
};
+ tevent_req_set_cleanup_fn(req, vfswrap_offload_write_cleanup);
+
switch (fsctl) {
case FSCTL_SRV_COPYCHUNK:
case FSCTL_SRV_COPYCHUNK_WRITE:
@@ -1676,6 +1695,12 @@ static struct tevent_req *vfswrap_offload_write_send(
return tevent_req_post(req, ev);
}
+ ok = change_to_user_by_fsp(src_fsp);
+ if (!ok) {
+ tevent_req_nterror(req, NT_STATUS_ACCESS_DENIED);
+ return tevent_req_post(req, ev);
+ }
+
state->buf = talloc_array(state, uint8_t, num);
if (tevent_req_nomem(state->buf, req)) {
return tevent_req_post(req, ev);
@@ -1719,6 +1744,10 @@ static NTSTATUS vfswrap_offload_write_loop(struct tevent_req *req)
struct lock_struct read_lck;
bool ok;
+ /*
+ * This is called under the context of state->src_fsp.
+ */
+
state->next_io_size = MIN(state->remaining, talloc_array_length(state->buf));
init_strict_lock_struct(state->src_fsp,
@@ -1778,6 +1807,12 @@ static void vfswrap_offload_write_read_done(struct tevent_req *subreq)
state->src_off += nread;
+ ok = change_to_user_by_fsp(state->dst_fsp);
+ if (!ok) {
+ tevent_req_nterror(req, NT_STATUS_INTERNAL_ERROR);
+ return;
+ }
+
init_strict_lock_struct(state->dst_fsp,
state->dst_fsp->op->global->open_persistent_id,
state->dst_off,
@@ -1815,6 +1850,7 @@ static void vfswrap_offload_write_write_done(struct tevent_req *subreq)
struct vfs_aio_state aio_state;
ssize_t nwritten;
NTSTATUS status;
+ bool ok;
nwritten = SMB_VFS_PWRITE_RECV(subreq, &aio_state);
TALLOC_FREE(subreq);
@@ -1842,6 +1878,12 @@ static void vfswrap_offload_write_write_done(struct tevent_req *subreq)
return;
}
+ ok = change_to_user_by_fsp(state->src_fsp);
+ if (!ok) {
+ tevent_req_nterror(req, NT_STATUS_INTERNAL_ERROR);
+ return;
+ }
+
status = vfswrap_offload_write_loop(req);
if (!NT_STATUS_IS_OK(status)) {
tevent_req_nterror(req, status);
--
2.17.1
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20180614/b2942084/signature-0001.sig>
More information about the samba-technical
mailing list