[SCM] Samba Shared Repository - branch master updated
Jeremy Allison
jra at samba.org
Sun Aug 28 20:59:01 UTC 2022
The branch, master has been updated
via 0d5016fb3a9 s3: smbd: parse_dfs_path() - Fix comment explaining where this is called from and with what kind of path.
via 3c18b27888d s3: smbd: Remove allow_broken_path parameter from parse_dfs_path().
via 5f0efdfe3cb s3: smbd: Now parse_dfs_path() is only called from dfs_filename_convert() replace allow_broken_path with an SMB1 check.
via 63e569a48cd s3: smbd: Remove allow_broken_path from create_junction().
via 32f6eb2e989 s3: smbd: Remove allow_broken_path from get_referred_path() and it's callers.
via 9d65f1c221f s3: smbd: Remove unneeded NULL check inside msdfs_servicename_matches_connection().
via 2780509a3c8 s3: smbd: In create_junction() don't read hostname from parse_dfs_path_strict().
via b33787fbb00 s3: smbd: In create_junction() remove hostname check. parse_dfs_path_strict() already does this.
via e4045bd7f15 s3: smbd: Change create_junction() to use parse_dfs_path_strict().
via 6869e015d54 s3: smbd: Change get_referred_path() to use parse_dfs_path_strict().
via 88e920491e0 s3: smbd: Add a comment explaing why dfs_filename_convert() must continue to use parse_dfs_path().
via b6afd481a40 s3: smbd: In dfs_filename_convert(), don't ask for hostname, sharename and then just free them.
via 06750a9684b s3: smbd: Add a new function parse_dfs_path_strict().
via 78ef185afdc smbd: add missing check for IPC share for TRANS2_GET_DFS_REFERRAL
from 4ddd277c0b7 smbXcli: Pass negotiate contexts through smbXcli_negprot_send/recv
https://git.samba.org/?p=samba.git;a=shortlog;h=master
- Log -----------------------------------------------------------------
commit 0d5016fb3a906e3b3c7db753d1b1c8f9904a4894
Author: Jeremy Allison <jra at samba.org>
Date: Wed Aug 17 22:51:26 2022 -0700
s3: smbd: parse_dfs_path() - Fix comment explaining where this is called from and with what kind of path.
Signed-off-by: Jeremy Allison <jra at samba.org>
Reviewed-by: Ralph Boehme <slow at samba.org>
Autobuild-User(master): Jeremy Allison <jra at samba.org>
Autobuild-Date(master): Sun Aug 28 20:58:57 UTC 2022 on sn-devel-184
commit 3c18b27888de30c91dce6f38339f0bc3dfb12d3c
Author: Jeremy Allison <jra at samba.org>
Date: Wed Aug 10 21:52:34 2022 -0700
s3: smbd: Remove allow_broken_path parameter from parse_dfs_path().
Nothing now looks at it.
Signed-off-by: Jeremy Allison <jra at samba.org>
Reviewed-by: Ralph Boehme <slow at samba.org>
commit 5f0efdfe3cb3b267f09677659bd0149975382356
Author: Jeremy Allison <jra at samba.org>
Date: Wed Aug 10 21:49:51 2022 -0700
s3: smbd: Now parse_dfs_path() is only called from dfs_filename_convert() replace allow_broken_path with an SMB1 check.
dfs_filename_convert() always sets allow_broken_path = !smb2,
so just move this bool inside of parse_dfs_path().
We can now remove allow_broken_path.
Signed-off-by: Jeremy Allison <jra at samba.org>
Reviewed-by: Ralph Boehme <slow at samba.org>
commit 63e569a48cd977299999e213beff00c2cc5504b5
Author: Jeremy Allison <jra at samba.org>
Date: Wed Aug 10 21:42:31 2022 -0700
s3: smbd: Remove allow_broken_path from create_junction().
We no longer look at it, we know we must have a canonicalized
DFS path here.
Signed-off-by: Jeremy Allison <jra at samba.org>
Reviewed-by: Ralph Boehme <slow at samba.org>
commit 32f6eb2e9891c13cc82455cadf363aea2edfaefc
Author: Jeremy Allison <jra at samba.org>
Date: Wed Aug 10 21:40:47 2022 -0700
s3: smbd: Remove allow_broken_path from get_referred_path() and it's callers.
It no longer looks at this bool, we must already have a
canonicalized path here.
Signed-off-by: Jeremy Allison <jra at samba.org>
Reviewed-by: Ralph Boehme <slow at samba.org>
commit 9d65f1c221fbc7122b48dd1065bb448c799dc670
Author: Jeremy Allison <jra at samba.org>
Date: Wed Aug 10 21:37:41 2022 -0700
s3: smbd: Remove unneeded NULL check inside msdfs_servicename_matches_connection().
This is now only called from is from parse_dfs_path(),
and for that we know conn is non-NULL.
Signed-off-by: Jeremy Allison <jra at samba.org>
Reviewed-by: Ralph Boehme <slow at samba.org>
commit 2780509a3c861215e64db8ccb6d8db5fa7ac7c5d
Author: Jeremy Allison <jra at samba.org>
Date: Wed Aug 17 21:50:19 2022 -0700
s3: smbd: In create_junction() don't read hostname from parse_dfs_path_strict().
It isn't used anymore inside create_junction().
Signed-off-by: Jeremy Allison <jra at samba.org>
Reviewed-by: Ralph Boehme <slow at samba.org>
commit b33787fbb0064780ab16fdd8898742f80fab42a7
Author: Jeremy Allison <jra at samba.org>
Date: Thu Aug 11 11:06:05 2022 -0700
s3: smbd: In create_junction() remove hostname check. parse_dfs_path_strict() already does this.
Signed-off-by: Jeremy Allison <jra at samba.org>
Reviewed-by: Ralph Boehme <slow at samba.org>
commit e4045bd7f155d0b750e2d3f3434d3105a9f0740c
Author: Jeremy Allison <jra at samba.org>
Date: Wed Aug 10 21:34:29 2022 -0700
s3: smbd: Change create_junction() to use parse_dfs_path_strict().
Note we no longer use allow_broken_path.
Signed-off-by: Jeremy Allison <jra at samba.org>
Reviewed-by: Ralph Boehme <slow at samba.org>
commit 6869e015d54c2ec11240c2675d9f840308845a14
Author: Jeremy Allison <jra at samba.org>
Date: Wed Aug 10 21:33:32 2022 -0700
s3: smbd: Change get_referred_path() to use parse_dfs_path_strict().
Remove #ifdef's around parse_dfs_path_strict() as we're
now using it.
Note we no longer use allow_broken_path.
Signed-off-by: Jeremy Allison <jra at samba.org>
Reviewed-by: Ralph Boehme <slow at samba.org>
commit 88e920491e0603821ba615a97a99069e7610a3b1
Author: Jeremy Allison <jra at samba.org>
Date: Wed Aug 17 14:23:45 2022 -0700
s3: smbd: Add a comment explaing why dfs_filename_convert() must continue to use parse_dfs_path().
libsmbclient libraries will always set the FLAGS2_DFS_PATHNAMES
bit when talking to a DFS share, but don't always canonicalize
the incoming pathname to a DFS one (see the code for cli_list()
that puts a non-DFS pathname into SMB2trans2_FindFirst for
example). This is a problem in our client libraries for both
SMB1 and SMB2+
As we still must cope with these older clients we must
keep the lenient parsing for DFS filenames sent over SMB1/2/3.
A future task - change the use of parse_dfs_path() in
dfs_filename_convert() to parse_dfs_path_strict() for SMB2
only and then try and get all our torture tests to pass.
This is not an easy fix (and would still break old clients
out there as well :-( ).
Signed-off-by: Jeremy Allison <jra at samba.org>
Reviewed-by: Ralph Boehme <slow at samba.org>
commit b6afd481a4001593d36230a29f33804dc64b0852
Author: Jeremy Allison <jra at samba.org>
Date: Wed Aug 17 14:21:59 2022 -0700
s3: smbd: In dfs_filename_convert(), don't ask for hostname, sharename and then just free them.
Wastes a talloc/free.
Signed-off-by: Jeremy Allison <jra at samba.org>
Reviewed-by: Ralph Boehme <slow at samba.org>
commit 06750a9684bd9158e7f3b75a7bbce0d8e96c35ce
Author: Jeremy Allison <jra at samba.org>
Date: Wed Aug 17 14:17:23 2022 -0700
s3: smbd: Add a new function parse_dfs_path_strict().
#ifdef'ed out as not yet used. This will replace
parse_dfs_path() for all client sent names via
DFS RPC calls and for SMB_VFS_GET_DFS_REFERRALS().
The paths sent in these calls are guaranteed
to be of canonical form:
\SERVER\share\pathname.
Both for SMB1 and SMB2+ so we can be more strict
when parsing them.
Checks DFS path starts with separator.
Checks hostname is ours.
Ensures servicename (share) is sent, and
if so, terminates the name or is followed by
\pathname.
Errors out if any checks fail.
Reserve parse_dfs_path() for DFS names sent
via "ordinary" SMB 1/2/3 calls where we must
be more lenient in parsing.
Note parse_dfs_path_strict() does not have
bool allow_broken_path or 'struct connection_struct'
as it will not be called from places that use
these.
Signed-off-by: Jeremy Allison <jra at samba.org>
Reviewed-by: Ralph Boehme <slow at samba.org>
commit 78ef185afdcecdba7da10f24b614073f47277725
Author: Ralph Boehme <slow at samba.org>
Date: Sun Aug 28 11:12:52 2022 +0200
smbd: add missing check for IPC share for TRANS2_GET_DFS_REFERRAL
Cf MS-CIFS 3.3.5.58.11.
Signed-off-by: Ralph Boehme <slow at samba.org>
Reviewed-by: Jeremy Allison <jra at samba.org>
-----------------------------------------------------------------------
Summary of changes:
source3/modules/vfs_default.c | 1 -
source3/rpc_server/dfs/srv_dfs_nt.c | 4 -
source3/smbd/msdfs.c | 216 ++++++++++++++++++++++++++++--------
source3/smbd/proto.h | 2 -
source3/smbd/smb1_trans2.c | 5 +
5 files changed, 177 insertions(+), 51 deletions(-)
Changeset truncated at 500 lines:
diff --git a/source3/modules/vfs_default.c b/source3/modules/vfs_default.c
index 48ff174ebbe..9481c9f36d5 100644
--- a/source3/modules/vfs_default.c
+++ b/source3/modules/vfs_default.c
@@ -258,7 +258,6 @@ static NTSTATUS vfswrap_get_dfs_referrals(struct vfs_handle_struct *handle,
pathnamep,
handle->conn->sconn->remote_address,
handle->conn->sconn->local_address,
- !handle->conn->sconn->using_smb2,
junction, &consumedcnt, &self_referral);
if (!NT_STATUS_IS_OK(status)) {
struct smb_filename connectpath_fname = {
diff --git a/source3/rpc_server/dfs/srv_dfs_nt.c b/source3/rpc_server/dfs/srv_dfs_nt.c
index a69494e3902..7b5119bbaf7 100644
--- a/source3/rpc_server/dfs/srv_dfs_nt.c
+++ b/source3/rpc_server/dfs/srv_dfs_nt.c
@@ -90,7 +90,6 @@ WERROR _dfs_Add(struct pipes_struct *p, struct dfs_Add *r)
r->in.path,
remote_address,
local_address,
- true, /*allow_broken_path */
jn, &consumedcnt, &self_ref);
if(!NT_STATUS_IS_OK(status)) {
return ntstatus_to_werror(status);
@@ -172,7 +171,6 @@ WERROR _dfs_Remove(struct pipes_struct *p, struct dfs_Remove *r)
r->in.dfs_entry_path,
remote_address,
local_address,
- true, /*allow_broken_path */
jn, &consumedcnt, &self_ref);
if(!NT_STATUS_IS_OK(status)) {
return WERR_NERR_DFSNOSUCHVOLUME;
@@ -405,7 +403,6 @@ WERROR _dfs_GetInfo(struct pipes_struct *p, struct dfs_GetInfo *r)
}
ret = create_junction(ctx, r->in.dfs_entry_path,
- true, /* allow broken_path */
jn);
if (!ret) {
return WERR_NERR_DFSNOSUCHSERVER;
@@ -417,7 +414,6 @@ WERROR _dfs_GetInfo(struct pipes_struct *p, struct dfs_GetInfo *r)
r->in.dfs_entry_path,
remote_address,
local_address,
- true, /*allow_broken_path */
jn, &consumedcnt, &self_ref);
if(!NT_STATUS_IS_OK(status) ||
consumedcnt < strlen(r->in.dfs_entry_path)) {
diff --git a/source3/smbd/msdfs.c b/source3/smbd/msdfs.c
index 4819df35837..ce7d0f7dd56 100644
--- a/source3/smbd/msdfs.c
+++ b/source3/smbd/msdfs.c
@@ -50,11 +50,6 @@ static bool msdfs_servicename_matches_connection(struct connection_struct *conn,
int snum;
bool match = false;
- if (conn == NULL) {
- /* No connection always matches. */
- return true;
- }
-
snum = SNUM(conn);
conn_servicename = lp_servicename(talloc_tos(), lp_sub, snum);
@@ -80,6 +75,147 @@ done:
return match;
}
+/**********************************************************************
+ Parse a DFS pathname of the form(s)
+
+ \hostname\service - self referral
+ \hostname\service\remainingpath - Windows referral path
+
+ FIXME! Should we also parse:
+ \hostname\service/remainingpath - POSIX referral path
+ as currently nothing uses this ?
+
+ into the dfs_path components. Strict form.
+
+ Checks DFS path starts with separator.
+ Checks hostname is ours.
+ Ensures servicename (share) is sent, and
+ if so, terminates the name or is followed by
+ \pathname.
+
+ If returned, remainingpath is untouched. Caller must call
+ check_path_syntaxXXX() on it.
+
+ Called by all non-fileserver processing (DFS RPC, FSCTL_DFS_GET_REFERRALS)
+ etc. Errors out on any inconsistency in the path.
+**********************************************************************/
+
+static NTSTATUS parse_dfs_path_strict(TALLOC_CTX *ctx,
+ const char *pathname,
+ char **_hostname,
+ char **_servicename,
+ char **_remaining_path)
+{
+ char *pathname_local = NULL;
+ char *p = NULL;
+ const char *hostname = NULL;
+ const char *servicename = NULL;
+ const char *reqpath = NULL;
+ bool my_hostname = false;
+ NTSTATUS status;
+
+ DBG_DEBUG("path = |%s|\n", pathname);
+
+ pathname_local = talloc_strdup(talloc_tos(), pathname);
+ if (pathname_local == NULL) {
+ return NT_STATUS_NO_MEMORY;
+ }
+ /*
+ * parse_dfs_path_strict() is called from
+ * get_referred_path() and create_junction()
+ * which use Windows DFS paths of \server\share.
+ */
+
+ /*
+ * Strict DFS paths *must* start with the
+ * path separator '\\'.
+ */
+
+ if (pathname_local[0] != '\\') {
+ DBG_ERR("path %s doesn't start with \\\n",
+ pathname_local);
+ status = NT_STATUS_NOT_FOUND;
+ goto out;
+ }
+
+ /* Now tokenize. */
+ /* Parse out hostname. */
+ p = strchr(pathname_local + 1, '\\');
+ if (p == NULL) {
+ DBG_ERR("can't parse hostname from path %s\n",
+ pathname_local);
+ status = NT_STATUS_NOT_FOUND;
+ goto out;
+ }
+ *p = '\0';
+ hostname = &pathname_local[1];
+
+ DBG_DEBUG("hostname: %s\n", hostname);
+
+ /* Is this really our hostname ? */
+ my_hostname = is_myname_or_ipaddr(hostname);
+ if (!my_hostname) {
+ DBG_ERR("Hostname %s is not ours.\n",
+ hostname);
+ status = NT_STATUS_NOT_FOUND;
+ goto out;
+ }
+
+ servicename = p + 1;
+
+ /*
+ * Find the end of servicename by looking for
+ * a directory separator character. The character
+ * should be '\\' for a Windows path.
+ * If there is no separator, then this is a self-referral
+ * of "\server\share".
+ */
+
+ p = strchr(servicename, '\\');
+ if (p != NULL) {
+ *p = '\0';
+ }
+
+ DBG_DEBUG("servicename: %s\n", servicename);
+
+ if (p == NULL) {
+ /* Client sent self referral "\server\share". */
+ reqpath = "";
+ } else {
+ /* Step past the '\0' we just replaced '\\' with. */
+ reqpath = p + 1;
+ }
+
+ DBG_DEBUG("rest of the path: %s\n", reqpath);
+
+ if (_hostname != NULL) {
+ *_hostname = talloc_strdup(ctx, hostname);
+ if (*_hostname == NULL) {
+ status = NT_STATUS_NO_MEMORY;
+ goto out;
+ }
+ }
+ if (_servicename != NULL) {
+ *_servicename = talloc_strdup(ctx, servicename);
+ if (*_servicename == NULL) {
+ status = NT_STATUS_NO_MEMORY;
+ goto out;
+ }
+ }
+ if (_remaining_path != NULL) {
+ *_remaining_path = talloc_strdup(ctx, reqpath);
+ if (*_remaining_path == NULL) {
+ status = NT_STATUS_NO_MEMORY;
+ goto out;
+ }
+ }
+
+ status = NT_STATUS_OK;
+out:
+ TALLOC_FREE(pathname_local);
+ return status;
+}
+
/**********************************************************************
Parse a DFS pathname of the form /hostname/service/reqpath
into the dfs_path structure.
@@ -105,7 +241,6 @@ done:
static NTSTATUS parse_dfs_path(TALLOC_CTX *ctx,
connection_struct *conn,
const char *pathname,
- bool allow_broken_path,
char **_hostname,
char **_servicename,
char **_remaining_path)
@@ -117,16 +252,17 @@ static NTSTATUS parse_dfs_path(TALLOC_CTX *ctx,
char *reqpath = NULL;
char *eos_ptr = NULL;
bool servicename_matches = false;
+ bool using_smb1 = !conn->sconn->using_smb2;
pathname_local = talloc_strdup(ctx, pathname);
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.
+ * parse_dfs_path() is only called from
+ * dfs_filename_convert() with SMB1/2/3 DFS
+ * names. Ensure we only have to cope with
+ * '/' separators.
*/
string_replace(pathname_local, '\\', '/');
@@ -135,11 +271,16 @@ static NTSTATUS parse_dfs_path(TALLOC_CTX *ctx,
p = pathname_local;
/*
- * Non-broken DFS paths *must* start with the
- * path separator '/'.
+ * SMB1 DFS paths sent to the fileserver should start with
+ * the path separator '/'. However, libsmbclient libraries
+ * will set the DFS bit on SMB1 calls and then send non-DFS
+ * paths. We must cope with this.
+ *
+ * Note SMB2 paths sent to the fileserver never start with
+ * the path separator '/'.
*/
- if (allow_broken_path && (*p != '/')) {
+ if (using_smb1 && (*p != '/')) {
DBG_ERR("path %s doesn't start with /\n", p);
/*
* Possibly client sent a local path by mistake.
@@ -985,29 +1126,29 @@ NTSTATUS dfs_filename_convert(TALLOC_CTX *ctx,
const char *dfs_path_in,
char **pp_path_out)
{
- char *hostname = NULL;
- char *servicename = NULL;
char *reqpath = NULL;
NTSTATUS status;
+ /*
+ * We must use the non-strict version of parse_dfs_path for
+ * pathnames sent to the fileserver over SMB1/2/3.
+ * libsmbclient callers always set the FLAGS2_DFS_PATHNAMES
+ * but then don't send a DFS path in (for example) FindFirst
+ * or other calls. This is a problem with our client libraries
+ * for both SMB1 and SMB2+ and will remain so whilst broken
+ * versions of libsmbclient are being used.
+ */
+
status = parse_dfs_path(ctx,
conn,
dfs_path_in,
- !conn->sconn->using_smb2,
- &hostname,
- &servicename,
+ NULL, /* hostname */
+ NULL, /* servicename */
&reqpath);
if (!NT_STATUS_IS_OK(status)) {
return status;
}
- /*
- * Caller doesn't care about hostname
- * or servicename.
- */
- TALLOC_FREE(hostname);
- TALLOC_FREE(servicename);
-
/*
* If parse_dfs_path fell back to a local path
* after skipping hostname or servicename, ensure
@@ -1075,7 +1216,6 @@ NTSTATUS get_referred_path(TALLOC_CTX *ctx,
const char *dfs_path,
const struct tsocket_address *remote_address,
const struct tsocket_address *local_address,
- bool allow_broken_path,
struct junction_map *jucn,
size_t *consumedcntp,
bool *self_referralp)
@@ -1092,11 +1232,10 @@ NTSTATUS get_referred_path(TALLOC_CTX *ctx,
*self_referralp = False;
- status = parse_dfs_path(frame,
- NULL,
+ status = parse_dfs_path_strict(
+ frame,
dfs_path,
- allow_broken_path,
- NULL,
+ NULL, /* hostname */
&servicename,
&reqpath);
if (!NT_STATUS_IS_OK(status)) {
@@ -1323,36 +1462,25 @@ int setup_dfs_referral(connection_struct *orig_conn,
bool create_junction(TALLOC_CTX *ctx,
const char *dfs_path,
- bool allow_broken_path,
struct junction_map *jucn)
{
const struct loadparm_substitution *lp_sub =
loadparm_s3_global_substitution();
int snum;
- char *hostname = NULL;
char *servicename = NULL;
char *reqpath = NULL;
NTSTATUS status;
- status = parse_dfs_path(ctx,
- NULL,
+ status = parse_dfs_path_strict(
+ ctx,
dfs_path,
- allow_broken_path,
- &hostname,
+ NULL,
&servicename,
&reqpath);
if (!NT_STATUS_IS_OK(status)) {
return False;
}
- /* check if path is dfs : validate first token */
- if (!is_myname_or_ipaddr(hostname)) {
- DEBUG(4,("create_junction: Invalid hostname %s "
- "in dfs path %s\n",
- hostname, dfs_path));
- return False;
- }
-
/* Check for a non-DFS share */
snum = lp_servicenumber(servicename);
diff --git a/source3/smbd/proto.h b/source3/smbd/proto.h
index c4a33014515..632ef45ffdf 100644
--- a/source3/smbd/proto.h
+++ b/source3/smbd/proto.h
@@ -524,7 +524,6 @@ NTSTATUS get_referred_path(TALLOC_CTX *ctx,
const char *dfs_path,
const struct tsocket_address *remote_address,
const struct tsocket_address *local_address,
- bool allow_broken_path,
struct junction_map *jucn,
size_t *consumedcntp,
bool *self_referralp);
@@ -534,7 +533,6 @@ int setup_dfs_referral(connection_struct *orig_conn,
char **ppdata, NTSTATUS *pstatus);
bool create_junction(TALLOC_CTX *ctx,
const char *dfs_path,
- bool allow_broken_path,
struct junction_map *jucn);
struct referral;
char *msdfs_link_string(TALLOC_CTX *ctx,
diff --git a/source3/smbd/smb1_trans2.c b/source3/smbd/smb1_trans2.c
index bc919815a6b..37c5036fb9c 100644
--- a/source3/smbd/smb1_trans2.c
+++ b/source3/smbd/smb1_trans2.c
@@ -2900,6 +2900,11 @@ static void call_trans2getdfsreferral(connection_struct *conn,
DEBUG(10,("call_trans2getdfsreferral\n"));
+ if (!IS_IPC(conn)) {
+ reply_nterror(req, NT_STATUS_ACCESS_DENIED);
+ return;
+ }
+
if (total_params < 3) {
reply_nterror(req, NT_STATUS_INVALID_PARAMETER);
return;
--
Samba Shared Repository
More information about the samba-cvs
mailing list