FLAG_CASELESS_PATHNAMES/SMBFLG_CASELESS in set_current_service()
Stefan Metzmacher
metze at samba.org
Wed Jun 13 19:30:54 UTC 2018
Am 13.06.2018 um 18:30 schrieb Jeremy Allison via samba-technical:
> On Wed, Jun 13, 2018 at 10:17:31AM +0200, Stefan Metzmacher wrote:
>> Hi Jeremy,
>> hi Steve,
>>
>> 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.
>> 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?
And also don't look at FILE_CASE_SENSITIVE_SEARCH anymore in the client.
>> 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?
>> 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.
>> 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.
> 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?
> 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.
> 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.
> 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.
metze
metze
-------------- next part --------------
From 7f27b96e531e9ab730ca338b323a85d904a3cf9f 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 1/9] 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>
---
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 156a32e165dc49d803d222cc79ced6f870e38eea 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 2/9] smbd: remove xconn->client->last_session_id based
set_current_user_info() caching
Signed-off-by: Stefan Metzmacher <metze 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 5cb5edc173d9151cb8c670b6e9af2bf891919f40 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 3/9] 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>
---
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 8c832bc40b0dfe4b6e578e922b5b12f9a3c9035b 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 4/9] 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>
---
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 0e0a847f1a4f3e4d939f19f334209624ed9759aa 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 5/9] 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>
---
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 cabcf5f4184142d4a8a308cacea4807c8a55d5b7 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 6/9] smbd: simplify the logic in change_to_user()
We can return early if (vuser == NULL).
Signed-off-by: Stefan Metzmacher <metze 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..e100c45a6069 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 d0eeeb903bf88352faf5cd8ffd99a2ff99f5b22e 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 7/9] 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>
---
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 e100c45a6069..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 946b640b35b0b83de7d3d135935880b3923869df 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 8/9] 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>
---
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 84a30fd49c8dbc0d32b50f0bb0ae2ebad2293225 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 9/9] FAILS: 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.
TDB_NO_FSYNC=1 buildnice make -j test FAIL_IMMEDIATELY=1 SOCKET_WRAPPER_KEEP_PCAP=1 TESTS="samba3.ntlm_auth.krb5.with.old.ccache.*ktest:local"
---
source3/smbd/uid.c | 34 +++++++++++++++++++++++++++-------
1 file changed, 27 insertions(+), 7 deletions(-)
diff --git a/source3/smbd/uid.c b/source3/smbd/uid.c
index 913d4f3aa0a1..a72fbfa1e643 100644
--- a/source3/smbd/uid.c
+++ b/source3/smbd/uid.c
@@ -370,11 +370,21 @@ static bool change_to_user_internal(connection_struct *conn,
current_user.conn = conn;
current_user.vuid = vuid;
- DEBUG(5, ("Impersonated user: uid=(%d,%d), gid=(%d,%d)\n",
- (int)getuid(),
- (int)geteuid(),
- (int)getgid(),
- (int)getegid()));
+ ok = chdir_current_service(conn);
+ if (!ok) {
+ DBG_ERR("chdir_current_service() failed!\n");
+ return false;
+ }
+
+ 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;
}
@@ -496,6 +506,7 @@ static void push_conn_ctx(void)
static void pop_conn_ctx(void)
{
struct conn_ctx *ctx_p;
+ bool ok;
/* Check for stack underflow. */
@@ -513,8 +524,17 @@ static void pop_conn_ctx(void)
current_user.conn = ctx_p->conn;
current_user.vuid = ctx_p->vuid;
- ctx_p->conn = NULL;
- ctx_p->vuid = UID_FIELD_INVALID;
+ if (current_user.conn != NULL) {
+ 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
-------------- 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/20180613/9bb4df9f/signature.sig>
More information about the samba-technical
mailing list