sin/samba-mirror
1
0
Fork 0
mirror of https://github.com/samba-team/samba.git synced 2025-01-11 05:18:09 +03:00
Code

s3: smbd: Change srvstr_get_path_internal() to always call check_path_syntaxXXX(), even on DFS pathnames.

Browse Source 
The original design decision to just copy a DFS path and let
parse_dfs_path() take care of it was a horrible mistake.

Fix srvstr_get_path_internal() to always return a
/server/share/path (i.e. a path separated with '/', not '\').

This is a more complex change than I like to allow
DFS path procesing in srvstr_get_path_internal() but
needed as clients (including Samba smbclient) have a
rather "fuzzy" idea of what constitutes a valid DFS path.
If we detect the DFS path isn't valid here we have to
fall back to treating it as a local path.

I also need to modify the DFS parsing in
filename_convert_smb1_search_path() to cope with only '/'
separators.

This also means parse_dfs_path() needs changing to
cope.

The changes here are best reviewed by just applying
the fix and looking at the modified functions:

srvstr_get_path_internal()
parse_dfs_path()

For parse_dfs_path() it's mostly removing bad code
and makes parse_dfs_path() much easier to read.

These changes will enable me to remove some ugly mistakes made
adding ucf_flags to extract_snapshot_token(), as
we can now always assume canonicalized paths.

This is a little messy, but has to be done in
one chunk as the change to srvstr_get_path_internal()
depends on the change to parse_dfs_path().

Thanks to Volker for the insight that made this
cleanup possible.

Signed-off-by: Jeremy Allison <jra@samba.org>
Reviewed-by: Volker Lendecke <vl@samba.org>
This commit is contained in:
Jeremy Allison 2022-08-04 09:52:17 -07:00 committed by Volker Lendecke
parent 972dd999b8
commit f24ef117cf
3 changed files with 118 additions and 79 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
source3/smbd/filename.c
View File

@ -1932,7 +1932,6 @@ NTSTATUS filename_convert_smb1_search_path(TALLOC_CTX *ctx,
char *p = NULL;
char *mask = NULL;
struct smb_filename *smb_fname = NULL;
bool posix_pathnames = (ucf_flags & UCF_POSIX_PATHNAMES);
NTTIME twrp = 0;
*_smb_fname_out = NULL;
@ -1949,10 +1948,7 @@ NTSTATUS filename_convert_smb1_search_path(TALLOC_CTX *ctx,
if (ucf_flags & UCF_DFS_PATHNAME) {
/*
* We've been given a raw DFS pathname.
* In Windows mode this is separated by '\'
* characters, in POSIX by '/' characters.
*/
char path_sep = posix_pathnames ? '/' : '\\';
char *fname = NULL;
char *name_in_copy = NULL;
char *last_component = NULL;
@ -1967,7 +1963,7 @@ NTSTATUS filename_convert_smb1_search_path(TALLOC_CTX *ctx,
* Now we know that the last component is the
* wildcard. Copy it and truncate to remove it.
*/
p = strrchr_m(name_in_copy, path_sep);
p = strrchr(name_in_copy, '/');
if (p == NULL) {
last_component = talloc_strdup(ctx, name_in_copy);
name_in_copy[0] = '\0';

113
source3/smbd/msdfs.c
View File

@ -37,11 +37,13 @@
#include "source3/lib/substitute.h"
/**********************************************************************
Parse a DFS pathname of the form \hostname\service\reqpath
Parse a DFS pathname of the form /hostname/service/reqpath
into the dfs_path structure.
If POSIX pathnames is true, the pathname may also be of the
form /hostname/service/reqpath.
We cope with either here.
NB. srvstr_get_path_internal() now *always* calls
check_path_syntax_XXX() on an incoming name, so
the path separator is now always '/', even from
Windows clients.
Unfortunately, due to broken clients who might set the
SVAL(inbuf,smb_flg2) & FLAGS2_DFS_PATHNAMES bit and then
@ -65,11 +67,9 @@ static NTSTATUS parse_dfs_path(connection_struct *conn,
const struct loadparm_substitution *lp_sub =
loadparm_s3_global_substitution();
char *pathname_local;
char *p,*temp;
char *p;
char *servicename;
char *eos_ptr;
NTSTATUS status = NT_STATUS_OK;
char sepchar;
ZERO_STRUCTP(pdp);
@ -80,30 +80,28 @@ static NTSTATUS parse_dfs_path(connection_struct *conn,
*/
pathname_local = talloc_strdup(pdp, pathname);
if (!pathname_local) {
if (pathname_local == NULL) {
return NT_STATUS_NO_MEMORY;
}
/*
* parse_dfs_path() can be called from
* get_referred_path() and create_junction()
* which use Windows DFS paths of \server\share.
* Ensure we only have to cope with '/' separators.
*/
string_replace(pathname_local, '\\', '/');
/* Get a pointer to the terminating '\0' */
eos_ptr = &pathname_local[strlen(pathname_local)];
p = temp = pathname_local;
p = pathname_local;
/*
* Non-broken DFS paths *must* start with the
* path separator. For Windows this is always '\\',
* for posix paths this is always '/'.
* path separator '/'.
*/
if (*pathname == '/') {
pdp->posix_path = true;
sepchar = '/';
} else {
pdp->posix_path = false;
sepchar = '\\';
}
if (allow_broken_path && (*pathname != sepchar)) {
DEBUG(10,("parse_dfs_path: path %s doesn't start with %c\n",
pathname, sepchar ));
if (allow_broken_path && (*p != '/')) {
DBG_ERR("path %s doesn't start with /\n", p);
/*
* Possibly client sent a local path by mistake.
* Try and convert to a local path.
@ -114,12 +112,7 @@ static NTSTATUS parse_dfs_path(connection_struct *conn,
pdp->hostname = eos_ptr; /* "" */
pdp->servicename = eos_ptr; /* "" */
/* We've got no info about separators. */
pdp->posix_path = lp_posix_pathnames();
p = temp;
DEBUG(10,("parse_dfs_path: trying to convert %s to a "
"local path\n",
temp));
DBG_ERR("trying to convert %s to a local path\n", p);
goto local_path;
}
@ -127,17 +120,15 @@ static NTSTATUS parse_dfs_path(connection_struct *conn,
* Safe to use on talloc'ed string as it only shrinks.
* It also doesn't affect the eos_ptr.
*/
trim_char(temp,sepchar,sepchar);
trim_char(p, '/', '/');
DEBUG(10,("parse_dfs_path: temp = |%s| after trimming %c's\n",
temp, sepchar));
DBG_ERR("p = |%s| after trimming /'s\n", p);
/* Now tokenize. */
/* Parse out hostname. */
p = strchr_m(temp,sepchar);
p = strchr(p,'/');
if(p == NULL) {
DEBUG(10,("parse_dfs_path: can't parse hostname from path %s\n",
temp));
DBG_ERR("can't parse hostname from path %s\n", pathname_local);
/*
* Possibly client sent a local path by mistake.
* Try and convert to a local path.
@ -146,20 +137,18 @@ static NTSTATUS parse_dfs_path(connection_struct *conn,
pdp->hostname = eos_ptr; /* "" */
pdp->servicename = eos_ptr; /* "" */
p = temp;
DEBUG(10,("parse_dfs_path: trying to convert %s "
"to a local path\n",
temp));
p = pathname_local;
DBG_ERR("trying to convert %s to a local path\n", p);
goto local_path;
}
*p = '\0';
pdp->hostname = temp;
pdp->hostname = pathname_local;
DEBUG(10,("parse_dfs_path: hostname: %s\n",pdp->hostname));
DBG_ERR("hostname: %s\n",pdp->hostname);
/* Parse out servicename. */
servicename = p+1;
p = strchr_m(servicename,sepchar);
p = strchr(servicename, '/');
if (p) {
*p = '\0';
}
@ -169,8 +158,7 @@ static NTSTATUS parse_dfs_path(connection_struct *conn,
|| (strequal(servicename, HOMES_NAME)
&& strequal(lp_servicename(talloc_tos(), lp_sub, SNUM(conn)),
get_current_username()) )) ) {
DEBUG(10,("parse_dfs_path: %s is not our servicename\n",
servicename));
DBG_ERR("%s is not our servicename\n", servicename);
/*
* Possibly client sent a local path by mistake.
@ -183,21 +171,20 @@ static NTSTATUS parse_dfs_path(connection_struct *conn,
/* Repair the path - replace the sepchar's
we nulled out */
servicename--;
*servicename = sepchar;
*servicename = '/';
if (p) {
*p = sepchar;
*p = '/';
}
p = temp;
DEBUG(10,("parse_dfs_path: trying to convert %s "
"to a local path\n",
temp));
p = pathname_local;
DBG_ERR("trying to convert %s to a local path\n",
pathname_local);
goto local_path;
}
pdp->servicename = servicename;
DEBUG(10,("parse_dfs_path: servicename: %s\n",pdp->servicename));
DBG_ERR("servicename: %s\n", pdp->servicename);
if(p == NULL) {
/* Client sent self referral \server\share. */
@ -209,22 +196,14 @@ static NTSTATUS parse_dfs_path(connection_struct *conn,
local_path:
/*
* As check_path_syntax_XXX() has already been
* called we know this is a normal path containing
* '/' separators.
*/
pdp->reqpath = p;
/* Rest is reqpath. */
if (pdp->posix_path) {
status = check_path_syntax_posix(pdp->reqpath);
} else {
status = check_path_syntax(pdp->reqpath);
}
if (!NT_STATUS_IS_OK(status)) {
DEBUG(10,("parse_dfs_path: '%s' failed with %s\n",
p, nt_errstr(status) ));
return status;
}
DEBUG(10,("parse_dfs_path: rest of the path: %s\n",pdp->reqpath));
DBG_ERR("rest of the path: %s\n", pdp->reqpath);
return NT_STATUS_OK;
}
@ -765,9 +744,7 @@ static NTSTATUS dfs_path_lookup(TALLOC_CTX *ctx,
status = NT_STATUS_NO_MEMORY;
goto out;
}
if (!pdp->posix_path) {
string_replace(canon_dfspath, '\\', '/');
}
string_replace(canon_dfspath, '\\', '/');
/*
* localpath comes out of unix_convert, so it has

78
source3/smbd/smb2_reply.c
View File

@ -259,6 +259,7 @@ static size_t srvstr_get_path_internal(TALLOC_CTX *ctx,
NTSTATUS *err)
{
size_t ret;
char *dst = NULL;
*pp_dest = NULL;
@ -270,19 +271,84 @@ static size_t srvstr_get_path_internal(TALLOC_CTX *ctx,
return ret;
}
dst = *pp_dest;
if (smb_flags2 & FLAGS2_DFS_PATHNAMES) {
/*
* For a DFS path the function parse_dfs_path()
* will do the path processing, just make a copy.
* A valid DFS path looks either like
* /server/share
* \server\share
* (there may be more components after).
* Either way it must have at least two separators.
*
* Ensure we end up as /server/share
* so we don't need to special case
* separator characters elsewhere in
* the code.
*/
*err = NT_STATUS_OK;
return ret;
char *server = NULL;
char *share = NULL;
char *remaining_path = NULL;
char path_sep = 0;
if (posix_pathnames && (dst[0] == '/')) {
path_sep = dst[0];
} else if (dst[0] == '\\') {
path_sep = dst[0];
}
if (path_sep == 0) {
goto local_path;
}
/*
* May be a DFS path.
* We need some heuristics here,
* as clients differ on what constitutes
* a well-formed DFS path. If the path
* appears malformed, just fall back to
* processing as a local path.
*/
server = dst;
/*
* Look to see if we also have /share following.
*/
share = strchr(server+1, path_sep);
if (share == NULL) {
goto local_path;
}
/*
* It's a well formed DFS path with
* at least server and share components.
* Replace the slashes with '/' and
* pass the remainder to local_path.
*/
*server = '/';
*share = '/';
/*
* Skip past share so we don't pass the
* sharename into check_path_syntax().
*/
remaining_path = strchr(share+1, path_sep);
if (remaining_path == NULL) {
/*
* If no remaining path this was
* a bare /server/share path. Just return.
*/
*err = NT_STATUS_OK;
return ret;
}
*remaining_path = '/';
dst = remaining_path + 1;
/* dst now points at any following components. */
}
local_path:
if (posix_pathnames) {
*err = check_path_syntax_posix(*pp_dest);
*err = check_path_syntax_posix(dst);
} else {
*err = check_path_syntax(*pp_dest);
*err = check_path_syntax(dst);
}
return ret;