From 5af635da76780806744f1868fb6f6e775aec3f61 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Wed, 27 Jul 2022 16:04:24 +0200 Subject: [PATCH 01/14] smdb: use fsp_is_alternate_stream() in open_file() No change in behaviour. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15126 MR: https://gitlab.com/samba-team/samba/-/merge_requests/2643 Signed-off-by: Ralph Boehme Reviewed-by: Volker Lendecke (cherry picked from commit 0d3995cec10c5fae8c8b6a1df312062e38437e6f) --- source3/smbd/open.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/source3/smbd/open.c b/source3/smbd/open.c index d39d2d619cd..ece9ba38d94 100644 --- a/source3/smbd/open.c +++ b/source3/smbd/open.c @@ -1406,7 +1406,7 @@ static NTSTATUS open_file(struct smb_request *req, #endif /* Don't create files with Microsoft wildcard characters. */ - if (fsp->base_fsp) { + if (fsp_is_alternate_stream(fsp)) { /* * wildcard characters are allowed in stream names * only test the basefilename @@ -1422,7 +1422,7 @@ static NTSTATUS open_file(struct smb_request *req, } /* Can we access this file ? */ - if (!fsp->base_fsp) { + if (!fsp_is_alternate_stream(fsp)) { /* Only do this check on non-stream open. */ if (file_existed) { status = smbd_check_access_rights_fsp( -- 2.34.1 From 8decde5ad1ebeb3cdcfbfeccd46279eb3f7c2124 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Wed, 27 Jul 2022 12:43:01 +0200 Subject: [PATCH 02/14] vfs_xattr_tdb: move close_xattr_db() This just makes the diff of the next commit smaller and easier to digest. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15126 MR: https://gitlab.com/samba-team/samba/-/merge_requests/2643 Signed-off-by: Ralph Boehme Reviewed-by: Volker Lendecke (cherry picked from commit b26dc252aaf3f4b960bdfdb6a3dfe612b89fcdd5) --- source3/modules/vfs_xattr_tdb.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/source3/modules/vfs_xattr_tdb.c b/source3/modules/vfs_xattr_tdb.c index 07b95899d80..beaaf0d12d9 100644 --- a/source3/modules/vfs_xattr_tdb.c +++ b/source3/modules/vfs_xattr_tdb.c @@ -321,6 +321,16 @@ static int xattr_tdb_fremovexattr(struct vfs_handle_struct *handle, return ret; } +/* + * Destructor for the VFS private data + */ + +static void close_xattr_db(void **data) +{ + struct db_context **p_db = (struct db_context **)data; + TALLOC_FREE(*p_db); +} + /* * Open the tdb file upon VFS_CONNECT */ @@ -565,16 +575,6 @@ static int xattr_tdb_unlinkat(vfs_handle_struct *handle, return ret; } -/* - * Destructor for the VFS private data - */ - -static void close_xattr_db(void **data) -{ - struct db_context **p_db = (struct db_context **)data; - TALLOC_FREE(*p_db); -} - static int xattr_tdb_connect(vfs_handle_struct *handle, const char *service, const char *user) { -- 2.34.1 From d30b3abac6288ca324f301c11bed55a13f57595c Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Wed, 27 Jul 2022 11:59:54 +0200 Subject: [PATCH 03/14] vfs_xattr_tdb: add a module config BUG: https://bugzilla.samba.org/show_bug.cgi?id=15126 MR: https://gitlab.com/samba-team/samba/-/merge_requests/2643 Signed-off-by: Ralph Boehme Reviewed-by: Volker Lendecke (cherry picked from commit 451ad315a9bf32c627e1966ec30185542701c87e) --- source3/modules/vfs_xattr_tdb.c | 196 ++++++++++++++++---------------- 1 file changed, 99 insertions(+), 97 deletions(-) diff --git a/source3/modules/vfs_xattr_tdb.c b/source3/modules/vfs_xattr_tdb.c index beaaf0d12d9..6d459fd4c98 100644 --- a/source3/modules/vfs_xattr_tdb.c +++ b/source3/modules/vfs_xattr_tdb.c @@ -29,7 +29,12 @@ #undef DBGC_CLASS #define DBGC_CLASS DBGC_VFS -static bool xattr_tdb_init(int snum, TALLOC_CTX *mem_ctx, struct db_context **p_db); +struct xattr_tdb_config { + struct db_context *db; +}; + +static bool xattr_tdb_init(struct vfs_handle_struct *handle, + struct xattr_tdb_config **_config); static int xattr_tdb_get_file_id(struct vfs_handle_struct *handle, const char *path, struct file_id *id) @@ -77,16 +82,20 @@ static struct tevent_req *xattr_tdb_getxattrat_send( const char *xattr_name, size_t alloc_hint) { + struct xattr_tdb_config *config = NULL; struct tevent_req *req = NULL; struct xattr_tdb_getxattrat_state *state = NULL; struct smb_filename *cwd = NULL; - struct db_context *db = NULL; struct file_id id; int ret; int error; int cwd_ret; DATA_BLOB xattr_blob; + if (!xattr_tdb_init(handle, &config)) { + return NULL; + } + req = tevent_req_create(mem_ctx, &state, struct xattr_tdb_getxattrat_state); if (req == NULL) { @@ -94,11 +103,6 @@ static struct tevent_req *xattr_tdb_getxattrat_send( } state->xattr_size = -1; - SMB_VFS_HANDLE_GET_DATA(handle, db, struct db_context, - if (!xattr_tdb_init(-1, state, &db)) { - tevent_req_error(req, EIO); - return tevent_req_post(req, ev); - }); cwd = SMB_VFS_GETWD(dir_fsp->conn, state); if (tevent_req_nomem(cwd, req)) { @@ -122,7 +126,7 @@ static struct tevent_req *xattr_tdb_getxattrat_send( return tevent_req_post(req, ev); } - state->xattr_size = xattr_tdb_getattr(db, + state->xattr_size = xattr_tdb_getattr(config->db, state, &id, xattr_name, @@ -194,27 +198,27 @@ static ssize_t xattr_tdb_fgetxattr(struct vfs_handle_struct *handle, struct files_struct *fsp, const char *name, void *value, size_t size) { + struct xattr_tdb_config *config = NULL; SMB_STRUCT_STAT sbuf; struct file_id id; - struct db_context *db; ssize_t xattr_size; DATA_BLOB blob; - TALLOC_CTX *frame = talloc_stackframe(); + TALLOC_CTX *frame = NULL; + + if (!xattr_tdb_init(handle, &config)) { + return -1; + } - SMB_VFS_HANDLE_GET_DATA(handle, db, struct db_context, - if (!xattr_tdb_init(-1, frame, &db)) - { - TALLOC_FREE(frame); return -1; - }); if (SMB_VFS_NEXT_FSTAT(handle, fsp, &sbuf) == -1) { - TALLOC_FREE(frame); return -1; } + frame = talloc_stackframe(); + id = SMB_VFS_NEXT_FILE_ID_CREATE(handle, &sbuf); - xattr_size = xattr_tdb_getattr(db, frame, &id, name, &blob); + xattr_size = xattr_tdb_getattr(config->db, frame, &id, name, &blob); if (xattr_size < 0) { errno = ENOATTR; TALLOC_FREE(frame); @@ -241,27 +245,22 @@ static int xattr_tdb_fsetxattr(struct vfs_handle_struct *handle, const char *name, const void *value, size_t size, int flags) { + struct xattr_tdb_config *config = NULL; SMB_STRUCT_STAT sbuf; struct file_id id; - struct db_context *db; int ret; - TALLOC_CTX *frame = talloc_stackframe(); - SMB_VFS_HANDLE_GET_DATA(handle, db, struct db_context, - if (!xattr_tdb_init(-1, frame, &db)) - { - TALLOC_FREE(frame); return -1; - }); + if (!xattr_tdb_init(handle, &config)) { + return -1; + } if (SMB_VFS_NEXT_FSTAT(handle, fsp, &sbuf) == -1) { - TALLOC_FREE(frame); return -1; } id = SMB_VFS_NEXT_FILE_ID_CREATE(handle, &sbuf); - ret = xattr_tdb_setattr(db, &id, name, value, size, flags); - TALLOC_FREE(frame); + ret = xattr_tdb_setattr(config->db, &id, name, value, size, flags); return ret; } @@ -270,105 +269,120 @@ static ssize_t xattr_tdb_flistxattr(struct vfs_handle_struct *handle, struct files_struct *fsp, char *list, size_t size) { + struct xattr_tdb_config *config = NULL; SMB_STRUCT_STAT sbuf; struct file_id id; - struct db_context *db; - int ret; - TALLOC_CTX *frame = talloc_stackframe(); - SMB_VFS_HANDLE_GET_DATA(handle, db, struct db_context, - if (!xattr_tdb_init(-1, frame, &db)) - { - TALLOC_FREE(frame); return -1; - }); + if (!xattr_tdb_init(handle, &config)) { + return -1; + } if (SMB_VFS_NEXT_FSTAT(handle, fsp, &sbuf) == -1) { - TALLOC_FREE(frame); return -1; } id = SMB_VFS_NEXT_FILE_ID_CREATE(handle, &sbuf); - ret = xattr_tdb_listattr(db, &id, list, size); - TALLOC_FREE(frame); - return ret; + return xattr_tdb_listattr(config->db, &id, list, size); } static int xattr_tdb_fremovexattr(struct vfs_handle_struct *handle, struct files_struct *fsp, const char *name) { + struct xattr_tdb_config *config = NULL; SMB_STRUCT_STAT sbuf; struct file_id id; - struct db_context *db; - int ret; - TALLOC_CTX *frame = talloc_stackframe(); - SMB_VFS_HANDLE_GET_DATA(handle, db, struct db_context, - if (!xattr_tdb_init(-1, frame, &db)) - { - TALLOC_FREE(frame); return -1; - }); + if (!xattr_tdb_init(handle, &config)) { + return -1; + } if (SMB_VFS_NEXT_FSTAT(handle, fsp, &sbuf) == -1) { - TALLOC_FREE(frame); return -1; } id = SMB_VFS_NEXT_FILE_ID_CREATE(handle, &sbuf); - ret = xattr_tdb_removeattr(db, &id, name); - TALLOC_FREE(frame); - return ret; + return xattr_tdb_removeattr(config->db, &id, name); } /* * Destructor for the VFS private data */ -static void close_xattr_db(void **data) +static void config_destructor(void **data) { - struct db_context **p_db = (struct db_context **)data; - TALLOC_FREE(*p_db); + struct xattr_tdb_config **config = (struct xattr_tdb_config **)data; + TALLOC_FREE((*config)->db); } /* * Open the tdb file upon VFS_CONNECT */ -static bool xattr_tdb_init(int snum, TALLOC_CTX *mem_ctx, struct db_context **p_db) +static bool xattr_tdb_init(struct vfs_handle_struct *handle, + struct xattr_tdb_config **_config) { - struct db_context *db; + struct xattr_tdb_config *config = NULL; const char *dbname; char *def_dbname; + if (SMB_VFS_HANDLE_TEST_DATA(handle)) { + SMB_VFS_HANDLE_GET_DATA(handle, config, struct xattr_tdb_config, + return false); + if (_config != NULL) { + *_config = config; + } + return true; + } + + config = talloc_zero(handle->conn, struct xattr_tdb_config); + if (config == NULL) { + errno = ENOMEM; + goto error; + } + def_dbname = state_path(talloc_tos(), "xattr.tdb"); if (def_dbname == NULL) { errno = ENOSYS; - return false; + goto error; } - dbname = lp_parm_const_string(snum, "xattr_tdb", "file", def_dbname); + dbname = lp_parm_const_string(SNUM(handle->conn), + "xattr_tdb", + "file", + def_dbname); /* now we know dbname is not NULL */ become_root(); - db = db_open(NULL, dbname, 0, TDB_DEFAULT, O_RDWR|O_CREAT, 0600, - DBWRAP_LOCK_ORDER_2, DBWRAP_FLAG_NONE); + config->db = db_open(handle, dbname, 0, TDB_DEFAULT, O_RDWR|O_CREAT, 0600, + DBWRAP_LOCK_ORDER_2, DBWRAP_FLAG_NONE); unbecome_root(); - if (db == NULL) { + if (config->db == NULL) { #if defined(ENOTSUP) errno = ENOTSUP; #else errno = ENOSYS; #endif TALLOC_FREE(def_dbname); - return false; + goto error; } - - *p_db = db; TALLOC_FREE(def_dbname); + + SMB_VFS_HANDLE_SET_DATA(handle, config, config_destructor, + struct xattr_tdb_config, return false); + + if (_config != NULL) { + *_config = config; + } return true; + +error: + DBG_WARNING("Failed to initialize config: %s\n", strerror(errno)); + lp_do_parameter(SNUM(handle->conn), "ea support", "False"); + return false; } static int xattr_tdb_openat(struct vfs_handle_struct *handle, @@ -377,12 +391,15 @@ static int xattr_tdb_openat(struct vfs_handle_struct *handle, struct files_struct *fsp, const struct vfs_open_how *how) { - struct db_context *db = NULL; - TALLOC_CTX *frame = NULL; + struct xattr_tdb_config *config = NULL; SMB_STRUCT_STAT sbuf; int fd; int ret; + if (!xattr_tdb_init(handle, &config)) { + return -1; + } + fd = SMB_VFS_NEXT_OPENAT(handle, dirfsp, smb_fname, @@ -414,17 +431,8 @@ static int xattr_tdb_openat(struct vfs_handle_struct *handle, fsp->file_id = SMB_VFS_FILE_ID_CREATE(fsp->conn, &sbuf); - frame = talloc_stackframe(); - - SMB_VFS_HANDLE_GET_DATA(handle, db, struct db_context, - if (!xattr_tdb_init(-1, frame, &db)) - { - TALLOC_FREE(frame); return -1; - }); - - xattr_tdb_remove_all_attrs(db, &fsp->file_id); + xattr_tdb_remove_all_attrs(config->db, &fsp->file_id); - TALLOC_FREE(frame); return fd; } @@ -433,12 +441,16 @@ static int xattr_tdb_mkdirat(vfs_handle_struct *handle, const struct smb_filename *smb_fname, mode_t mode) { - struct db_context *db = NULL; + struct xattr_tdb_config *config = NULL; TALLOC_CTX *frame = NULL; struct file_id fileid; int ret; struct smb_filename *full_fname = NULL; + if (!xattr_tdb_init(handle, &config)) { + return -1; + } + ret = SMB_VFS_NEXT_MKDIRAT(handle, dirfsp, smb_fname, @@ -472,13 +484,7 @@ static int xattr_tdb_mkdirat(vfs_handle_struct *handle, fileid = SMB_VFS_FILE_ID_CREATE(handle->conn, &full_fname->st); - SMB_VFS_HANDLE_GET_DATA(handle, db, struct db_context, - if (!xattr_tdb_init(-1, frame, &db)) - { - TALLOC_FREE(frame); return -1; - }); - - xattr_tdb_remove_all_attrs(db, &fileid); + xattr_tdb_remove_all_attrs(config->db, &fileid); TALLOC_FREE(frame); return 0; } @@ -491,19 +497,19 @@ static int xattr_tdb_unlinkat(vfs_handle_struct *handle, const struct smb_filename *smb_fname, int flags) { + struct xattr_tdb_config *config = NULL; struct smb_filename *smb_fname_tmp = NULL; struct smb_filename *full_fname = NULL; struct file_id id; - struct db_context *db; int ret = -1; bool remove_record = false; - TALLOC_CTX *frame = talloc_stackframe(); + TALLOC_CTX *frame = NULL; + + if (!xattr_tdb_init(handle, &config)) { + return -1; + } - SMB_VFS_HANDLE_GET_DATA(handle, db, struct db_context, - if (!xattr_tdb_init(-1, frame, &db)) - { - TALLOC_FREE(frame); return -1; - }); + frame = talloc_stackframe(); smb_fname_tmp = cp_smb_filename(frame, smb_fname); if (smb_fname_tmp == NULL) { @@ -568,7 +574,7 @@ static int xattr_tdb_unlinkat(vfs_handle_struct *handle, id = SMB_VFS_NEXT_FILE_ID_CREATE(handle, &smb_fname_tmp->st); - xattr_tdb_remove_all_attrs(db, &id); + xattr_tdb_remove_all_attrs(config->db, &id); out: TALLOC_FREE(frame); @@ -580,7 +586,6 @@ static int xattr_tdb_connect(vfs_handle_struct *handle, const char *service, { char *sname = NULL; int res, snum; - struct db_context *db; res = SMB_VFS_NEXT_CONNECT(handle, service, user); if (res < 0) { @@ -595,7 +600,7 @@ static int xattr_tdb_connect(vfs_handle_struct *handle, const char *service, return 0; } - if (!xattr_tdb_init(snum, NULL, &db)) { + if (!xattr_tdb_init(handle, NULL)) { DEBUG(5, ("Could not init xattr tdb\n")); lp_do_parameter(snum, "ea support", "False"); return 0; @@ -603,9 +608,6 @@ static int xattr_tdb_connect(vfs_handle_struct *handle, const char *service, lp_do_parameter(snum, "ea support", "True"); - SMB_VFS_HANDLE_SET_DATA(handle, db, close_xattr_db, - struct db_context, return -1); - return 0; } -- 2.34.1 From ab418144bee747b5ea17ab8bafc8fbd3a403289c Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Wed, 27 Jul 2022 12:47:21 +0200 Subject: [PATCH 04/14] vfs_xattr_tdb: add "xattr_tdb:ignore_user_xattr" option Allows passing on "user." xattr to the backend. This can be useful for testing specific aspects of operation on streams when "streams_xattr" is configured as stream filesystem backend. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15126 MR: https://gitlab.com/samba-team/samba/-/merge_requests/2643 Signed-off-by: Ralph Boehme Reviewed-by: Volker Lendecke (cherry picked from commit 92e0045d7ca7c0b94efd0244ba0e426cad0a05b6) --- source3/modules/vfs_xattr_tdb.c | 85 ++++++++++++++++++++++++++++++++- 1 file changed, 84 insertions(+), 1 deletion(-) diff --git a/source3/modules/vfs_xattr_tdb.c b/source3/modules/vfs_xattr_tdb.c index 6d459fd4c98..261ebc2b213 100644 --- a/source3/modules/vfs_xattr_tdb.c +++ b/source3/modules/vfs_xattr_tdb.c @@ -31,11 +31,20 @@ struct xattr_tdb_config { struct db_context *db; + bool ignore_user_xattr; }; static bool xattr_tdb_init(struct vfs_handle_struct *handle, struct xattr_tdb_config **_config); +static bool is_user_xattr(const char *xattr_name) +{ + int match; + + match = strncmp(xattr_name, "user.", strlen("user.")); + return (match == 0); +} + static int xattr_tdb_get_file_id(struct vfs_handle_struct *handle, const char *path, struct file_id *id) { @@ -73,6 +82,8 @@ struct xattr_tdb_getxattrat_state { uint8_t *xattr_value; }; +static void xattr_tdb_getxattrat_done(struct tevent_req *subreq); + static struct tevent_req *xattr_tdb_getxattrat_send( TALLOC_CTX *mem_ctx, struct tevent_context *ev, @@ -84,6 +95,7 @@ static struct tevent_req *xattr_tdb_getxattrat_send( { struct xattr_tdb_config *config = NULL; struct tevent_req *req = NULL; + struct tevent_req *subreq = NULL; struct xattr_tdb_getxattrat_state *state = NULL; struct smb_filename *cwd = NULL; struct file_id id; @@ -103,6 +115,20 @@ static struct tevent_req *xattr_tdb_getxattrat_send( } state->xattr_size = -1; + if (config->ignore_user_xattr && is_user_xattr(xattr_name)) { + subreq = SMB_VFS_NEXT_GETXATTRAT_SEND(state, + ev, + handle, + dir_fsp, + smb_fname, + xattr_name, + alloc_hint); + if (tevent_req_nomem(subreq, req)) { + return tevent_req_post(req, ev); + } + tevent_req_set_callback(subreq, xattr_tdb_getxattrat_done, req); + return req; + } cwd = SMB_VFS_GETWD(dir_fsp->conn, state); if (tevent_req_nomem(cwd, req)) { @@ -170,6 +196,27 @@ static struct tevent_req *xattr_tdb_getxattrat_send( return tevent_req_post(req, ev); } +static void xattr_tdb_getxattrat_done(struct tevent_req *subreq) +{ + struct tevent_req *req = tevent_req_callback_data( + subreq, struct tevent_req); + struct xattr_tdb_getxattrat_state *state = tevent_req_data( + req, struct xattr_tdb_getxattrat_state); + + state->xattr_size = SMB_VFS_NEXT_GETXATTRAT_RECV(subreq, + &state->vfs_aio_state, + state, + &state->xattr_value); + TALLOC_FREE(subreq); + if (state->xattr_size == -1) { + tevent_req_error(req, state->vfs_aio_state.error); + return; + } + + tevent_req_done(req); +} + + static ssize_t xattr_tdb_getxattrat_recv(struct tevent_req *req, struct vfs_aio_state *aio_state, TALLOC_CTX *mem_ctx, @@ -209,6 +256,10 @@ static ssize_t xattr_tdb_fgetxattr(struct vfs_handle_struct *handle, return -1; } + if (config->ignore_user_xattr && is_user_xattr(name)) { + return SMB_VFS_NEXT_FGETXATTR( + handle, fsp, name, value, size); + } if (SMB_VFS_NEXT_FSTAT(handle, fsp, &sbuf) == -1) { return -1; @@ -254,6 +305,11 @@ static int xattr_tdb_fsetxattr(struct vfs_handle_struct *handle, return -1; } + if (config->ignore_user_xattr && is_user_xattr(name)) { + return SMB_VFS_NEXT_FSETXATTR( + handle, fsp, name, value, size, flags); + } + if (SMB_VFS_NEXT_FSTAT(handle, fsp, &sbuf) == -1) { return -1; } @@ -272,6 +328,8 @@ static ssize_t xattr_tdb_flistxattr(struct vfs_handle_struct *handle, struct xattr_tdb_config *config = NULL; SMB_STRUCT_STAT sbuf; struct file_id id; + ssize_t backend_size; + ssize_t ret; if (!xattr_tdb_init(handle, &config)) { return -1; @@ -283,7 +341,25 @@ static ssize_t xattr_tdb_flistxattr(struct vfs_handle_struct *handle, id = SMB_VFS_NEXT_FILE_ID_CREATE(handle, &sbuf); - return xattr_tdb_listattr(config->db, &id, list, size); + ret = xattr_tdb_listattr(config->db, &id, list, size); + if (ret == -1) { + return -1; + } + if (ret == size) { + return ret; + } + if (!config->ignore_user_xattr) { + return ret; + } + SMB_ASSERT(ret < size); + + backend_size = SMB_VFS_NEXT_FLISTXATTR( + handle, fsp, list + ret, size - ret); + if (backend_size == -1) { + return -1; + } + + return ret + backend_size; } static int xattr_tdb_fremovexattr(struct vfs_handle_struct *handle, @@ -297,6 +373,10 @@ static int xattr_tdb_fremovexattr(struct vfs_handle_struct *handle, return -1; } + if (config->ignore_user_xattr && is_user_xattr(name)) { + return SMB_VFS_NEXT_FREMOVEXATTR(handle, fsp, name); + } + if (SMB_VFS_NEXT_FSTAT(handle, fsp, &sbuf) == -1) { return -1; } @@ -371,6 +451,9 @@ static bool xattr_tdb_init(struct vfs_handle_struct *handle, } TALLOC_FREE(def_dbname); + config->ignore_user_xattr = lp_parm_bool( + SNUM(handle->conn), "xattr_tdb", "ignore_user_xattr", false); + SMB_VFS_HANDLE_SET_DATA(handle, config, config_destructor, struct xattr_tdb_config, return false); -- 2.34.1 From 94821a7a4a7b6441255da253092aa8a81ae7a3da Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Wed, 27 Jul 2022 13:37:32 +0200 Subject: [PATCH 05/14] CI: add a test trying to delete a stream on a pathref ("stat open") handle When using vfs_streams_xattr, for a pathref handle of a stream the system fd will be a fake fd created by pipe() in vfs_fake_fd(). For the following callchain we wrongly pass a stream fsp to SMB_VFS_FGET_NT_ACL(): SMB_VFS_CREATE_FILE(..., "file:stream", ...) => open_file(): if (open_fd): -> taking the else branch: -> smbd_check_access_rights_fsp(stream_fsp) -> SMB_VFS_FGET_NT_ACL(stream_fsp) This is obviously wrong and can lead to strange permission errors when using vfs_acl_xattr: in vfs_acl_xattr we will try to read the stored ACL by calling fgetxattr(fake-fd) which of course faild with EBADF. Now unfortunately the vfs_acl_xattr code ignores the specific error and handles this as if there was no ACL stored and subsequently runs the code to synthesize a default ACL according to the setting of "acl:default acl style". As the correct access check for streams has already been carried out by calling check_base_file_access() from create_file_unixpath(), the above problem is not a security issue: it can only lead to "decreased" permissions resulting in unexpected ACCESS_DENIED errors. The fix is obviously going to be calling smbd_check_access_rights_fsp(stream_fsp->base_fsp). This test verifies that deleting a file works when the stored NT ACL grants DELETE_FILE while the basic POSIX permissions (used in the acl_xattr fallback code) do not. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15126 MR: https://gitlab.com/samba-team/samba/-/merge_requests/2643 Signed-off-by: Ralph Boehme Reviewed-by: Volker Lendecke (cherry picked from commit 23bc760ec5d61208c2d8778991e3d7e202eab352) --- .../knownfail.d/samba3.blackbox.delete_stream | 1 + selftest/target/Samba3.pm | 7 + source3/script/tests/test_delete_stream.sh | 123 ++++++++++++++++++ source3/selftest/tests.py | 1 + 4 files changed, 132 insertions(+) create mode 100644 selftest/knownfail.d/samba3.blackbox.delete_stream create mode 100755 source3/script/tests/test_delete_stream.sh diff --git a/selftest/knownfail.d/samba3.blackbox.delete_stream b/selftest/knownfail.d/samba3.blackbox.delete_stream new file mode 100644 index 00000000000..22c996aa63b --- /dev/null +++ b/selftest/knownfail.d/samba3.blackbox.delete_stream @@ -0,0 +1 @@ +^samba3.blackbox.delete_stream.delete stream\(fileserver\) diff --git a/selftest/target/Samba3.pm b/selftest/target/Samba3.pm index 1a4bf7439ef..2313f6fce36 100755 --- a/selftest/target/Samba3.pm +++ b/selftest/target/Samba3.pm @@ -3366,6 +3366,13 @@ sub provision($$) copy = tmp vfs objects = streams_xattr xattr_tdb +[acl_streams_xattr] + copy = tmp + vfs objects = acl_xattr streams_xattr fake_acls xattr_tdb + acl_xattr:ignore system acls = yes + acl_xattr:security_acl_name = user.acl + xattr_tdb:ignore_user_xattr = yes + [compound_find] copy = tmp smbd:find async delay usec = 10000 diff --git a/source3/script/tests/test_delete_stream.sh b/source3/script/tests/test_delete_stream.sh new file mode 100755 index 00000000000..43d591ecfdf --- /dev/null +++ b/source3/script/tests/test_delete_stream.sh @@ -0,0 +1,123 @@ +#!/bin/sh +# +# this verifies that deleting a stream uses the correct ACL +# when using vfs_acl_xattr. +# + +if [ $# -lt 9 ]; then + echo "Usage: $0 SERVER SERVER_IP USERNAME PASSWORD PREFIX SMBCLIENT SMBCACLS NET SHARE" + exit 1 +fi + +SERVER="$1" +SERVER_IP="$2" +USERNAME="$3" +PASSWORD="$4" +PREFIX="$5" +SMBCLIENT="$6" +SMBCACLS="$7" +NET="$8" +SHARE="$9" + +SMBCLIENT="$VALGRIND ${SMBCLIENT}" +SMBCACLS="$VALGRIND ${SMBCACLS}" +NET="$VALGRIND ${NET}" + +incdir=$(dirname $0)/../../../testprogs/blackbox +. $incdir/subunit.sh + + +setup_testfile() +{ + touch $PREFIX/file + echo stream > $PREFIX/stream + + $SMBCLIENT //$SERVER/$SHARE -U $USERNAME%$PASSWORD -c "mkdir dir" || return 1 + $SMBCLIENT //$SERVER/$SHARE -U $USERNAME%$PASSWORD -c "lcd $PREFIX; put file dir/file" || return 1 + $SMBCLIENT //$SERVER/$SHARE -U $USERNAME%$PASSWORD -c "lcd $PREFIX; put stream dir/file:stream" || return 1 + + rm $PREFIX/file + rm $PREFIX/stream + + # + # Add full control ACE to the file and an ACL without "DELETE" on the + # parent directory + # + + $SMBCACLS //$SERVER/$SHARE -U $USERNAME%$PASSWORD -S "ACL:Everyone:ALLOWED/0x0/0x1bf" dir || return 1 + $SMBCACLS //$SERVER/$SHARE -U $USERNAME%$PASSWORD -a "ACL:Everyone:ALLOWED/0x0/0x101ff" dir/file || return 1 +} + +remove_testfile() +{ + $SMBCACLS //$SERVER/$SHARE -U $USERNAME%$PASSWORD -S "ACL:Everyone:ALLOWED/0x0/0x101ff" dir/file || return 1 + $SMBCACLS //$SERVER/$SHARE -U $USERNAME%$PASSWORD -S "ACL:Everyone:ALLOWED/0x0/0x101ff" dir || return 1 + $SMBCLIENT //$SERVER/$SHARE -U $USERNAME%$PASSWORD -c "rm dir/file" || return 1 + $SMBCLIENT //$SERVER/$SHARE -U $USERNAME%$PASSWORD -c "rmdir dir" || return 1 +} + +set_win_owner() +{ + local owner=$1 + + $SMBCACLS //$SERVER/$SHARE dir/file -U $USERNAME%$PASSWORD -C "$owner" || return 1 +} + +delete_stream() +{ + # + # Setup a file with a stream where we're not the owner and + # have delete rights. Bug 15126 would trigger a fallback to + # "acl_xattr:default acl style" because fetching the stored + # ACL would fail. The stored ACL allows deleting the stream + # but the synthesized default ACL does not, so the deletion + # of the stream should work, but it fails if we have the bug. + # + + # Now try deleting the stream + out=$($SMBCLIENT //$SERVER/$SHARE -U $USERNAME%$PASSWORD -c "wdel 0x20 dir/file:stream") || return 1 + + # + # Bail out in case we get any sort of NT_STATUS_* error, should be + # NT_STATUS_ACCESS_DENIED, but let's not slip through any other error. + # + echo "$out" | grep NT_STATUS_ && return 1 + + return 0 +} + +win_owner_is() +{ + local expected_owner=$1 + local actual_owner + + $SMBCACLS //$SERVER/$SHARE dir/file -U $USERNAME%$PASSWORD + actual_owner=$($SMBCACLS //$SERVER/$SHARE dir/file -U $USERNAME%$PASSWORD | sed -rn 's/^OWNER:(.*)/\1/p') + echo "actual_owner = $actual_owner" + if ! test "x$actual_owner" = "x$expected_owner"; then + echo "Actual owner of dir/file is $actual_owner', expected $expected_owner" + return 1 + fi + return 0 +} + +# Create a testfile +testit "create testfile" setup_testfile $SHARE || exit 1 + +# Grant SeRestorePrivilege to the user so we can change the owner +testit "grant SeRestorePrivilege" $NET rpc rights grant $USERNAME SeRestorePrivilege -U $USERNAME%$PASSWORD -I $SERVER_IP || exit 1 + +# We have SeRestorePrivilege, so both give and take ownership must succeed +testit "give owner with SeRestorePrivilege" set_win_owner "$SERVER\user1" || exit 1 +testit "verify owner" win_owner_is "$SERVER/user1" || exit 1 + +# Now try to remove the stream on the testfile +testit "delete stream" delete_stream $SHARE afile || exit 1 + +# Remove testfile +testit "remove testfile" remove_testfile $SHARE || exit 1 + +# Revoke SeRestorePrivilege, give ownership must fail now with NT_STATUS_INVALID_OWNER +testit "revoke SeRestorePrivilege" $NET rpc rights revoke $USERNAME SeRestorePrivilege -U $USERNAME%$PASSWORD -I $SERVER_IP || exit 1 + +exit 0 diff --git a/source3/selftest/tests.py b/source3/selftest/tests.py index a8f833d52a3..6b8e7f774f2 100755 --- a/source3/selftest/tests.py +++ b/source3/selftest/tests.py @@ -564,6 +564,7 @@ for env in ["fileserver"]: plantestsuite("samba3.blackbox.large_acl.NT1", env + "_smb1_done", [os.path.join(samba3srcdir, "script/tests/test_large_acl.sh"), '$SERVER', '$USERNAME', '$PASSWORD', smbclient3, smbcacls, '-m', 'NT1']) plantestsuite("samba3.blackbox.large_acl.SMB3", env, [os.path.join(samba3srcdir, "script/tests/test_large_acl.sh"), '$SERVER', '$USERNAME', '$PASSWORD', smbclient3, smbcacls, '-m', 'SMB3']) plantestsuite("samba3.blackbox.give_owner", env, [os.path.join(samba3srcdir, "script/tests/test_give_owner.sh"), '$SERVER', '$SERVER_IP', '$USERNAME', '$PASSWORD', '$PREFIX', smbclient3, smbcacls, net, 'tmp']) + plantestsuite("samba3.blackbox.delete_stream", env, [os.path.join(samba3srcdir, "script/tests/test_delete_stream.sh"), '$SERVER', '$SERVER_IP', '$USERNAME', '$PASSWORD', '$PREFIX', smbclient3, smbcacls, net, 'acl_streams_xattr']) plantestsuite("samba3.blackbox.homes", env, [os.path.join(samba3srcdir, "script/tests/test_homes.sh"), '$SERVER', '$USERNAME', '$PASSWORD', '$LOCAL_PATH', '$PREFIX', smbclient3, configuration]) plantestsuite("samba3.blackbox.force_group_change", env, [os.path.join(samba3srcdir, "script/tests/test_force_group_change.sh"), -- 2.34.1 From af3d8487cbfe184b367999935d952f42773b7a83 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Fri, 29 Jul 2022 14:49:56 +0200 Subject: [PATCH 06/14] smbd: use metadata_fsp() with SMB_VFS_FGET_NT_ACL() BUG: https://bugzilla.samba.org/show_bug.cgi?id=15126 MR: https://gitlab.com/samba-team/samba/-/merge_requests/2643 Signed-off-by: Ralph Boehme Reviewed-by: Volker Lendecke (cherry picked from commit c949e4b2a42423ac3851e86e489fd0c5d46d7f1f) --- selftest/knownfail.d/samba3.blackbox.delete_stream | 1 - source3/rpc_server/srvsvc/srv_srvsvc_nt.c | 2 +- source3/smbd/dir.c | 2 +- source3/smbd/file_access.c | 2 +- source3/smbd/open.c | 4 ++-- source3/smbd/pysmbd.c | 2 +- source3/torture/cmd_vfs.c | 2 +- 7 files changed, 7 insertions(+), 8 deletions(-) delete mode 100644 selftest/knownfail.d/samba3.blackbox.delete_stream diff --git a/selftest/knownfail.d/samba3.blackbox.delete_stream b/selftest/knownfail.d/samba3.blackbox.delete_stream deleted file mode 100644 index 22c996aa63b..00000000000 --- a/selftest/knownfail.d/samba3.blackbox.delete_stream +++ /dev/null @@ -1 +0,0 @@ -^samba3.blackbox.delete_stream.delete stream\(fileserver\) diff --git a/source3/rpc_server/srvsvc/srv_srvsvc_nt.c b/source3/rpc_server/srvsvc/srv_srvsvc_nt.c index c5b39ed9cb6..07bfb759147 100644 --- a/source3/rpc_server/srvsvc/srv_srvsvc_nt.c +++ b/source3/rpc_server/srvsvc/srv_srvsvc_nt.c @@ -2579,7 +2579,7 @@ WERROR _srvsvc_NetGetFileSecurity(struct pipes_struct *p, goto error_exit; } - nt_status = SMB_VFS_FGET_NT_ACL(fsp, + nt_status = SMB_VFS_FGET_NT_ACL(metadata_fsp(fsp), (SECINFO_OWNER |SECINFO_GROUP |SECINFO_DACL), sd_buf, &sd_buf->sd); diff --git a/source3/smbd/dir.c b/source3/smbd/dir.c index 9aa0a768c7a..04dfef00d95 100644 --- a/source3/smbd/dir.c +++ b/source3/smbd/dir.c @@ -1263,7 +1263,7 @@ static bool user_can_read_fsp(struct files_struct *fsp) return false; } - status = SMB_VFS_FGET_NT_ACL(fsp, + status = SMB_VFS_FGET_NT_ACL(metadata_fsp(fsp), (SECINFO_OWNER | SECINFO_GROUP | SECINFO_DACL), diff --git a/source3/smbd/file_access.c b/source3/smbd/file_access.c index e3c8a1e2b67..476aead590d 100644 --- a/source3/smbd/file_access.c +++ b/source3/smbd/file_access.c @@ -158,7 +158,7 @@ bool directory_has_default_acl_fsp(struct files_struct *fsp) unsigned int i; NTSTATUS status; - status = SMB_VFS_FGET_NT_ACL(fsp, + status = SMB_VFS_FGET_NT_ACL(metadata_fsp(fsp), SECINFO_DACL, talloc_tos(), &secdesc); diff --git a/source3/smbd/open.c b/source3/smbd/open.c index ece9ba38d94..30349864f05 100644 --- a/source3/smbd/open.c +++ b/source3/smbd/open.c @@ -296,7 +296,7 @@ NTSTATUS smbd_check_access_rights_fsp(struct files_struct *dirfsp, return status; } - status = SMB_VFS_FGET_NT_ACL(fsp, + status = SMB_VFS_FGET_NT_ACL(metadata_fsp(fsp), (SECINFO_OWNER | SECINFO_GROUP | SECINFO_DACL), @@ -3253,7 +3253,7 @@ static NTSTATUS smbd_calculate_maximum_allowed_access_fsp( return NT_STATUS_OK; } - status = SMB_VFS_FGET_NT_ACL(fsp, + status = SMB_VFS_FGET_NT_ACL(metadata_fsp(fsp), (SECINFO_OWNER | SECINFO_GROUP | SECINFO_DACL), diff --git a/source3/smbd/pysmbd.c b/source3/smbd/pysmbd.c index ed079b5e938..6209c6626d2 100644 --- a/source3/smbd/pysmbd.c +++ b/source3/smbd/pysmbd.c @@ -317,7 +317,7 @@ static NTSTATUS get_nt_acl_conn(TALLOC_CTX *mem_ctx, return status; } - status = SMB_VFS_FGET_NT_ACL(smb_fname->fsp, + status = SMB_VFS_FGET_NT_ACL(metadata_fsp(smb_fname->fsp), security_info_wanted, mem_ctx, sd); diff --git a/source3/torture/cmd_vfs.c b/source3/torture/cmd_vfs.c index 896197fbd5c..8d38a7a9ee7 100644 --- a/source3/torture/cmd_vfs.c +++ b/source3/torture/cmd_vfs.c @@ -1624,7 +1624,7 @@ static NTSTATUS cmd_fget_nt_acl(struct vfs_state *vfs, TALLOC_CTX *mem_ctx, return NT_STATUS_OK; } - status = SMB_VFS_FGET_NT_ACL(vfs->files[fd], + status = SMB_VFS_FGET_NT_ACL(metadata_fsp(vfs->files[fd]), SECINFO_OWNER | SECINFO_GROUP | SECINFO_DACL, talloc_tos(), &sd); if (!NT_STATUS_IS_OK(status)) { -- 2.34.1 From e1c60799c0c5c6f54e832aac10c9e32345b65c69 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Fri, 29 Jul 2022 14:54:07 +0200 Subject: [PATCH 07/14] smbd: use metadata_fsp() with SMB_VFS_FSET_NT_ACL() BUG: https://bugzilla.samba.org/show_bug.cgi?id=15126 MR: https://gitlab.com/samba-team/samba/-/merge_requests/2643 Signed-off-by: Ralph Boehme Reviewed-by: Volker Lendecke (cherry picked from commit 4ab29e2a345b48ebba652d5154e96adf954a6757) --- source3/smbd/open.c | 2 +- source3/smbd/pysmbd.c | 2 +- source3/torture/cmd_vfs.c | 10 ++++++++-- 3 files changed, 10 insertions(+), 4 deletions(-) diff --git a/source3/smbd/open.c b/source3/smbd/open.c index 30349864f05..89541e7be15 100644 --- a/source3/smbd/open.c +++ b/source3/smbd/open.c @@ -5251,7 +5251,7 @@ static NTSTATUS inherit_new_acl(files_struct *dirfsp, files_struct *fsp) /* We need to be root to force this. */ become_root(); } - status = SMB_VFS_FSET_NT_ACL(fsp, + status = SMB_VFS_FSET_NT_ACL(metadata_fsp(fsp), security_info_sent, psd); if (inherit_owner) { diff --git a/source3/smbd/pysmbd.c b/source3/smbd/pysmbd.c index 6209c6626d2..658de43d2c0 100644 --- a/source3/smbd/pysmbd.c +++ b/source3/smbd/pysmbd.c @@ -281,7 +281,7 @@ static NTSTATUS set_nt_acl_conn(const char *fname, return status; } - status = SMB_VFS_FSET_NT_ACL(fsp, security_info_sent, sd); + status = SMB_VFS_FSET_NT_ACL(metadata_fsp(fsp), security_info_sent, sd); if (!NT_STATUS_IS_OK(status)) { DEBUG(0,("set_nt_acl_no_snum: fset_nt_acl returned %s.\n", nt_errstr(status))); } diff --git a/source3/torture/cmd_vfs.c b/source3/torture/cmd_vfs.c index 8d38a7a9ee7..3be450b97bd 100644 --- a/source3/torture/cmd_vfs.c +++ b/source3/torture/cmd_vfs.c @@ -1717,7 +1717,10 @@ static NTSTATUS cmd_fset_nt_acl(struct vfs_state *vfs, TALLOC_CTX *mem_ctx, return NT_STATUS_INVALID_PARAMETER; } - status = SMB_VFS_FSET_NT_ACL(vfs->files[fd], SECINFO_OWNER | SECINFO_GROUP | SECINFO_DACL, sd); + status = SMB_VFS_FSET_NT_ACL( + metadata_fsp(vfs->files[fd]), + SECINFO_OWNER | SECINFO_GROUP | SECINFO_DACL, + sd); if (!NT_STATUS_IS_OK(status)) { printf("fset_nt_acl returned (%s)\n", nt_errstr(status)); return status; @@ -1823,7 +1826,10 @@ static NTSTATUS cmd_set_nt_acl(struct vfs_state *vfs, TALLOC_CTX *mem_ctx, int a goto out; } - status = SMB_VFS_FSET_NT_ACL(fsp, SECINFO_OWNER | SECINFO_GROUP | SECINFO_DACL, sd); + status = SMB_VFS_FSET_NT_ACL( + metadata_fsp(fsp), + SECINFO_OWNER | SECINFO_GROUP | SECINFO_DACL, + sd); if (!NT_STATUS_IS_OK(status)) { printf("fset_nt_acl returned (%s)\n", nt_errstr(status)); goto out; -- 2.34.1 From 5aec2e7f6a8a4add4a92e21c05a689733095e062 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Fri, 29 Jul 2022 14:55:08 +0200 Subject: [PATCH 08/14] smbd: use metadata_fsp() with SMB_VFS_FGET_DOS_ATTRIBUTES() BUG: https://bugzilla.samba.org/show_bug.cgi?id=15126 MR: https://gitlab.com/samba-team/samba/-/merge_requests/2643 Signed-off-by: Ralph Boehme Reviewed-by: Volker Lendecke (cherry picked from commit 03b9ce84736d536ab2dd8a5ce1a2656e6a90c8c8) --- source3/smbd/dosmode.c | 3 ++- source3/smbd/open.c | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/source3/smbd/dosmode.c b/source3/smbd/dosmode.c index b8ceb8e148f..5fc130ac669 100644 --- a/source3/smbd/dosmode.c +++ b/source3/smbd/dosmode.c @@ -727,7 +727,8 @@ uint32_t fdos_mode(struct files_struct *fsp) } /* Get the DOS attributes via the VFS if we can */ - status = SMB_VFS_FGET_DOS_ATTRIBUTES(fsp->conn, fsp, &result); + status = SMB_VFS_FGET_DOS_ATTRIBUTES( + fsp->conn, metadata_fsp(fsp), &result); if (!NT_STATUS_IS_OK(status)) { /* * Only fall back to using UNIX modes if we get NOT_IMPLEMENTED. diff --git a/source3/smbd/open.c b/source3/smbd/open.c index 89541e7be15..b06b6f06485 100644 --- a/source3/smbd/open.c +++ b/source3/smbd/open.c @@ -3615,7 +3615,8 @@ static NTSTATUS open_file_ntcreate(connection_struct *conn, */ uint32_t attr = 0; - status = SMB_VFS_FGET_DOS_ATTRIBUTES(conn, smb_fname->fsp, &attr); + status = SMB_VFS_FGET_DOS_ATTRIBUTES( + conn, metadata_fsp(smb_fname->fsp), &attr); if (NT_STATUS_IS_OK(status)) { existing_dos_attributes = attr; } -- 2.34.1 From 5a186925e6f6f5598e0e12a3426adaba23b2790b Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Fri, 29 Jul 2022 14:56:21 +0200 Subject: [PATCH 09/14] smbd: use metadata_fsp() with SMB_VFS_FSET_DOS_ATTRIBUTES() BUG: https://bugzilla.samba.org/show_bug.cgi?id=15126 MR: https://gitlab.com/samba-team/samba/-/merge_requests/2643 Signed-off-by: Ralph Boehme Reviewed-by: Volker Lendecke (cherry picked from commit 55e55804bb2d0f21c1bbe207257bb40555f3b7a2) --- source3/smbd/dosmode.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/source3/smbd/dosmode.c b/source3/smbd/dosmode.c index 5fc130ac669..dd07b5f3882 100644 --- a/source3/smbd/dosmode.c +++ b/source3/smbd/dosmode.c @@ -933,9 +933,8 @@ int file_set_dosmode(connection_struct *conn, if (smb_fname->fsp != NULL) { /* Store the DOS attributes in an EA by preference. */ - status = SMB_VFS_FSET_DOS_ATTRIBUTES(conn, - smb_fname->fsp, - dosmode); + status = SMB_VFS_FSET_DOS_ATTRIBUTES( + conn, metadata_fsp(smb_fname->fsp), dosmode); } else { status = NT_STATUS_OBJECT_NAME_NOT_FOUND; } -- 2.34.1 From a936bb732d90be918138924a2700fb0adf642a7e Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Fri, 29 Jul 2022 14:56:41 +0200 Subject: [PATCH 10/14] smbd: ignore request to set the SPARSE attribute on streams As per MS-FSA 2.1.1.5 this is a per stream attribute, but our backends don't support it in a consistent way, therefor just pretend success and ignore the request. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15126 MR: https://gitlab.com/samba-team/samba/-/merge_requests/2643 Signed-off-by: Ralph Boehme Reviewed-by: Volker Lendecke (cherry picked from commit 3af8f8e8741cc8c889bbf416ccd38a1b702917ec) --- source3/smbd/dosmode.c | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/source3/smbd/dosmode.c b/source3/smbd/dosmode.c index dd07b5f3882..3d69a35b366 100644 --- a/source3/smbd/dosmode.c +++ b/source3/smbd/dosmode.c @@ -1102,6 +1102,19 @@ NTSTATUS file_set_sparse(connection_struct *conn, return NT_STATUS_INVALID_PARAMETER; } + if (fsp_is_alternate_stream(fsp)) { + /* + * MS-FSA 2.1.1.5 IsSparse + * + * This is a per stream attribute, but our backends don't + * support it a consistent way, therefor just pretend + * success and ignore the request. + */ + DBG_DEBUG("Ignoring request to set FILE_ATTRIBUTE_SPARSE on " + "[%s]\n", fsp_str_dbg(fsp)); + return NT_STATUS_OK; + } + DEBUG(10,("file_set_sparse: setting sparse bit %u on file %s\n", sparse, smb_fname_str_dbg(fsp->fsp_name))); -- 2.34.1 From d61ef6ca583552baa03d4a73bcba615fedcab3fb Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Wed, 27 Jul 2022 19:05:26 +0200 Subject: [PATCH 11/14] smbd: use metadata_fsp() in get_acl_group_bits() BUG: https://bugzilla.samba.org/show_bug.cgi?id=15126 MR: https://gitlab.com/samba-team/samba/-/merge_requests/2643 Signed-off-by: Ralph Boehme Reviewed-by: Volker Lendecke (cherry picked from commit 06555c6bcb5644fc9eea35b3cbae8d8801c65ab6) --- source3/smbd/posix_acls.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source3/smbd/posix_acls.c b/source3/smbd/posix_acls.c index 62e399ba2fb..8cef0fee024 100644 --- a/source3/smbd/posix_acls.c +++ b/source3/smbd/posix_acls.c @@ -3844,7 +3844,7 @@ int get_acl_group_bits(connection_struct *conn, SMB_ACL_T posix_acl; int result = -1; - posix_acl = SMB_VFS_SYS_ACL_GET_FD(fsp, + posix_acl = SMB_VFS_SYS_ACL_GET_FD(metadata_fsp(fsp), SMB_ACL_TYPE_ACCESS, talloc_tos()); if (posix_acl == (SMB_ACL_T)NULL) -- 2.34.1 From aa635f780cc33f8271d4e3acf9365fcd38e8195f Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Wed, 27 Jul 2022 15:58:37 +0200 Subject: [PATCH 12/14] smbd: skip access checks for stat-opens on streams in open_file() For streams, access is already checked in create_file_unixpath() by check_base_file_access(). We already skip the access check in this function when doing an IO open of a file, see above in open_file(), also skip it for "stat opens". BUG: https://bugzilla.samba.org/show_bug.cgi?id=15126 MR: https://gitlab.com/samba-team/samba/-/merge_requests/2643 Signed-off-by: Ralph Boehme Reviewed-by: Volker Lendecke (cherry picked from commit f0299abf1b28a14518328710d9f84bef17fd2ecf) --- source3/smbd/open.c | 51 ++++++++++++++++++++++++++------------------- 1 file changed, 29 insertions(+), 22 deletions(-) diff --git a/source3/smbd/open.c b/source3/smbd/open.c index b06b6f06485..7877f0e5aa5 100644 --- a/source3/smbd/open.c +++ b/source3/smbd/open.c @@ -1611,29 +1611,36 @@ static NTSTATUS open_file(struct smb_request *req, } } - status = smbd_check_access_rights_fsp(dirfsp, - fsp, - false, - access_mask); - - if (NT_STATUS_EQUAL(status, NT_STATUS_OBJECT_NAME_NOT_FOUND) && - posix_open && - S_ISLNK(smb_fname->st.st_ex_mode)) { - /* This is a POSIX stat open for delete - * or rename on a symlink that points - * nowhere. Allow. */ - DEBUG(10,("open_file: allowing POSIX " - "open on bad symlink %s\n", - smb_fname_str_dbg(smb_fname))); - status = NT_STATUS_OK; - } + /* + * Access to streams is checked by checking the basefile and + * that has alreay been checked by check_base_file_access() + * in create_file_unixpath(). + */ + if (!fsp_is_alternate_stream(fsp)) { + status = smbd_check_access_rights_fsp(dirfsp, + fsp, + false, + access_mask); + + if (NT_STATUS_EQUAL(status, NT_STATUS_OBJECT_NAME_NOT_FOUND) && + posix_open && + S_ISLNK(smb_fname->st.st_ex_mode)) { + /* This is a POSIX stat open for delete + * or rename on a symlink that points + * nowhere. Allow. */ + DEBUG(10,("open_file: allowing POSIX " + "open on bad symlink %s\n", + smb_fname_str_dbg(smb_fname))); + status = NT_STATUS_OK; + } - if (!NT_STATUS_IS_OK(status)) { - DBG_DEBUG("smbd_check_access_rights_fsp on file " - "%s returned %s\n", - fsp_str_dbg(fsp), - nt_errstr(status)); - return status; + if (!NT_STATUS_IS_OK(status)) { + DBG_DEBUG("smbd_check_access_rights_fsp on file " + "%s returned %s\n", + fsp_str_dbg(fsp), + nt_errstr(status)); + return status; + } } } -- 2.34.1 From bd904b94311bb93bf522e86c21f8b0b173e4526f Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Fri, 29 Jul 2022 07:07:25 +0200 Subject: [PATCH 13/14] vfs_streams_xattr: restrict which fcntl's are allowed on streams BUG: https://bugzilla.samba.org/show_bug.cgi?id=15126 MR: https://gitlab.com/samba-team/samba/-/merge_requests/2643 Signed-off-by: Ralph Boehme Reviewed-by: Volker Lendecke (cherry picked from commit 51243e3849736acbbf1d8f52cc02cdec5995fde4) --- source3/modules/vfs_streams_xattr.c | 33 +++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/source3/modules/vfs_streams_xattr.c b/source3/modules/vfs_streams_xattr.c index 3dfb894a800..f3371ca9b7e 100644 --- a/source3/modules/vfs_streams_xattr.c +++ b/source3/modules/vfs_streams_xattr.c @@ -1539,6 +1539,38 @@ static bool streams_xattr_strict_lock_check(struct vfs_handle_struct *handle, return true; } +static int streams_xattr_fcntl(vfs_handle_struct *handle, + files_struct *fsp, + int cmd, + va_list cmd_arg) +{ + va_list dup_cmd_arg; + void *arg; + int ret; + + if (fsp_is_alternate_stream(fsp)) { + switch (cmd) { + case F_GETFL: + case F_SETFL: + break; + default: + DBG_ERR("Unsupported fcntl() cmd [%d] on [%s]\n", + cmd, fsp_str_dbg(fsp)); + errno = EINVAL; + return -1; + } + } + + va_copy(dup_cmd_arg, cmd_arg); + arg = va_arg(dup_cmd_arg, void *); + + ret = SMB_VFS_NEXT_FCNTL(handle, fsp, cmd, arg); + + va_end(dup_cmd_arg); + + return ret; +} + static struct vfs_fn_pointers vfs_streams_xattr_fns = { .fs_capabilities_fn = streams_xattr_fs_capabilities, .connect_fn = streams_xattr_connect, @@ -1567,6 +1599,7 @@ static struct vfs_fn_pointers vfs_streams_xattr_fns = { .filesystem_sharemode_fn = streams_xattr_filesystem_sharemode, .linux_setlease_fn = streams_xattr_linux_setlease, .strict_lock_check_fn = streams_xattr_strict_lock_check, + .fcntl_fn = streams_xattr_fcntl, .fchown_fn = streams_xattr_fchown, .fchmod_fn = streams_xattr_fchmod, -- 2.34.1 From 97a547965b1fc5581204abc2f6c1f47a25f1891f Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Wed, 27 Jul 2022 18:40:21 +0200 Subject: [PATCH 14/14] vfs_default: assert all passed in fsp's and names are non-stream type Enforce fsp is a non-stream one in as many VFS operations as possible in vfs_default. We really need an assert here instead of returning an error, as otherwise he can have very hard to diagnose bugs. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15126 MR: https://gitlab.com/samba-team/samba/-/merge_requests/2643 Signed-off-by: Ralph Boehme Reviewed-by: Volker Lendecke Autobuild-User(master): Volker Lendecke Autobuild-Date(master): Wed Aug 10 16:32:35 UTC 2022 on sn-devel-184 (cherry picked from commit fc45fcfde51b0b0bdcd524c82a0f9eabf7273045) --- source3/modules/vfs_default.c | 92 ++++++++++++++++++++++------------- 1 file changed, 58 insertions(+), 34 deletions(-) diff --git a/source3/modules/vfs_default.c b/source3/modules/vfs_default.c index 9cf70fd84ce..dee8ff50df4 100644 --- a/source3/modules/vfs_default.c +++ b/source3/modules/vfs_default.c @@ -707,11 +707,7 @@ static int vfswrap_openat(vfs_handle_struct *handle, goto out; } - if (is_named_stream(smb_fname)) { - errno = ENOENT; - result = -1; - goto out; - } + SMB_ASSERT(!is_named_stream(smb_fname)); #ifdef O_PATH have_opath = true; @@ -1258,17 +1254,14 @@ static int vfswrap_renameat(vfs_handle_struct *handle, START_PROFILE(syscall_renameat); - if (is_named_stream(smb_fname_src) || is_named_stream(smb_fname_dst)) { - errno = ENOENT; - goto out; - } + SMB_ASSERT(!is_named_stream(smb_fname_src)); + SMB_ASSERT(!is_named_stream(smb_fname_dst)); result = renameat(fsp_get_pathref_fd(srcfsp), smb_fname_src->base_name, fsp_get_pathref_fd(dstfsp), smb_fname_dst->base_name); - out: END_PROFILE(syscall_renameat); return result; } @@ -1280,14 +1273,11 @@ static int vfswrap_stat(vfs_handle_struct *handle, START_PROFILE(syscall_stat); - if (is_named_stream(smb_fname)) { - errno = ENOENT; - goto out; - } + SMB_ASSERT(!is_named_stream(smb_fname)); result = sys_stat(smb_fname->base_name, &smb_fname->st, lp_fake_directory_create_times(SNUM(handle->conn))); - out: + END_PROFILE(syscall_stat); return result; } @@ -1310,14 +1300,11 @@ static int vfswrap_lstat(vfs_handle_struct *handle, START_PROFILE(syscall_lstat); - if (is_named_stream(smb_fname)) { - errno = ENOENT; - goto out; - } + SMB_ASSERT(!is_named_stream(smb_fname)); result = sys_lstat(smb_fname->base_name, &smb_fname->st, lp_fake_directory_create_times(SNUM(handle->conn))); - out: + END_PROFILE(syscall_lstat); return result; } @@ -1333,10 +1320,7 @@ static int vfswrap_fstatat( START_PROFILE(syscall_fstatat); - if (is_named_stream(smb_fname)) { - errno = ENOENT; - goto out; - } + SMB_ASSERT(!is_named_stream(smb_fname)); result = sys_fstatat( fsp_get_pathref_fd(dirfsp), @@ -1344,7 +1328,7 @@ static int vfswrap_fstatat( sbuf, flags, lp_fake_directory_create_times(SNUM(handle->conn))); - out: + END_PROFILE(syscall_fstatat); return result; } @@ -1441,6 +1425,8 @@ static NTSTATUS vfswrap_fsctl(struct vfs_handle_struct *handle, char **out_data = (char **)_out_data; NTSTATUS status; + SMB_ASSERT(!fsp_is_alternate_stream(fsp)); + switch (function) { case FSCTL_SET_SPARSE: { @@ -1805,6 +1791,8 @@ static struct tevent_req *vfswrap_get_dos_attributes_send( struct tevent_req *subreq = NULL; struct vfswrap_get_dos_attributes_state *state = NULL; + SMB_ASSERT(!is_named_stream(smb_fname)); + req = tevent_req_create(mem_ctx, &state, struct vfswrap_get_dos_attributes_state); if (req == NULL) { @@ -1961,6 +1949,8 @@ static NTSTATUS vfswrap_fget_dos_attributes(struct vfs_handle_struct *handle, { bool offline; + SMB_ASSERT(!fsp_is_alternate_stream(fsp)); + offline = vfswrap_is_offline(handle->conn, fsp->fsp_name); if (offline) { *dosmode |= FILE_ATTRIBUTE_OFFLINE; @@ -1973,6 +1963,8 @@ static NTSTATUS vfswrap_fset_dos_attributes(struct vfs_handle_struct *handle, struct files_struct *fsp, uint32_t dosmode) { + SMB_ASSERT(!fsp_is_alternate_stream(fsp)); + return set_ea_dos_attribute(handle->conn, fsp->fsp_name, dosmode); } @@ -2649,15 +2641,12 @@ static int vfswrap_unlinkat(vfs_handle_struct *handle, START_PROFILE(syscall_unlinkat); - if (is_named_stream(smb_fname)) { - errno = ENOENT; - goto out; - } + SMB_ASSERT(!is_named_stream(smb_fname)); + result = unlinkat(fsp_get_pathref_fd(dirfsp), smb_fname->base_name, flags); - out: END_PROFILE(syscall_unlinkat); return result; } @@ -3142,6 +3131,8 @@ static int vfswrap_linux_setlease(vfs_handle_struct *handle, files_struct *fsp, START_PROFILE(syscall_linux_setlease); + SMB_ASSERT(!fsp_is_alternate_stream(fsp)); + #ifdef HAVE_KERNEL_OPLOCKS_LINUX result = linux_setlease(fsp_get_io_fd(fsp), leasetype); #else @@ -3160,6 +3151,8 @@ static int vfswrap_symlinkat(vfs_handle_struct *handle, START_PROFILE(syscall_symlinkat); + SMB_ASSERT(!is_named_stream(new_smb_fname)); + result = symlinkat(link_target->base_name, fsp_get_pathref_fd(dirfsp), new_smb_fname->base_name); @@ -3177,6 +3170,8 @@ static int vfswrap_readlinkat(vfs_handle_struct *handle, START_PROFILE(syscall_readlinkat); + SMB_ASSERT(!is_named_stream(smb_fname)); + result = readlinkat(fsp_get_pathref_fd(dirfsp), smb_fname->base_name, buf, @@ -3197,6 +3192,9 @@ static int vfswrap_linkat(vfs_handle_struct *handle, START_PROFILE(syscall_linkat); + SMB_ASSERT(!is_named_stream(old_smb_fname)); + SMB_ASSERT(!is_named_stream(new_smb_fname)); + result = linkat(fsp_get_pathref_fd(srcfsp), old_smb_fname->base_name, fsp_get_pathref_fd(dstfsp), @@ -3217,6 +3215,8 @@ static int vfswrap_mknodat(vfs_handle_struct *handle, START_PROFILE(syscall_mknodat); + SMB_ASSERT(!is_named_stream(smb_fname)); + result = sys_mknodat(fsp_get_pathref_fd(dirfsp), smb_fname->base_name, mode, @@ -3255,6 +3255,8 @@ static int vfswrap_fchflags(vfs_handle_struct *handle, #ifdef HAVE_FCHFLAGS int fd = fsp_get_pathref_fd(fsp); + SMB_ASSERT(!fsp_is_alternate_stream(fsp)); + if (!fsp->fsp_flags.is_pathref) { return fchflags(fd, flags); } @@ -3326,6 +3328,8 @@ static NTSTATUS vfswrap_fstreaminfo(vfs_handle_struct *handle, struct stream_struct *streams = *pstreams; NTSTATUS status; + SMB_ASSERT(!fsp_is_alternate_stream(fsp)); + if (fsp->fsp_flags.is_directory) { /* * No default streams on directories @@ -3426,6 +3430,9 @@ static NTSTATUS vfswrap_fget_nt_acl(vfs_handle_struct *handle, NTSTATUS result; START_PROFILE(fget_nt_acl); + + SMB_ASSERT(!fsp_is_alternate_stream(fsp)); + result = posix_fget_nt_acl(fsp, security_info, mem_ctx, ppdesc); END_PROFILE(fget_nt_acl); @@ -3437,6 +3444,9 @@ static NTSTATUS vfswrap_fset_nt_acl(vfs_handle_struct *handle, files_struct *fsp NTSTATUS result; START_PROFILE(fset_nt_acl); + + SMB_ASSERT(!fsp_is_alternate_stream(fsp)); + result = set_nt_acl(fsp, security_info_sent, psd); END_PROFILE(fset_nt_acl); return result; @@ -3456,6 +3466,8 @@ static SMB_ACL_T vfswrap_sys_acl_get_fd(vfs_handle_struct *handle, SMB_ACL_TYPE_T type, TALLOC_CTX *mem_ctx) { + SMB_ASSERT(!fsp_is_alternate_stream(fsp)); + return sys_acl_get_fd(handle, fsp, type, mem_ctx); } @@ -3464,12 +3476,16 @@ static int vfswrap_sys_acl_set_fd(vfs_handle_struct *handle, SMB_ACL_TYPE_T type, SMB_ACL_T theacl) { + SMB_ASSERT(!fsp_is_alternate_stream(fsp)); + return sys_acl_set_fd(handle, fsp, type, theacl); } static int vfswrap_sys_acl_delete_def_fd(vfs_handle_struct *handle, files_struct *fsp) { + SMB_ASSERT(!fsp_is_alternate_stream(fsp)); + return sys_acl_delete_def_fd(handle, fsp); } @@ -3485,6 +3501,8 @@ static ssize_t vfswrap_fgetxattr(struct vfs_handle_struct *handle, { int fd = fsp_get_pathref_fd(fsp); + SMB_ASSERT(!fsp_is_alternate_stream(fsp)); + if (!fsp->fsp_flags.is_pathref) { return fgetxattr(fd, name, value, size); } @@ -3555,6 +3573,8 @@ static struct tevent_req *vfswrap_getxattrat_send( bool have_per_thread_creds = false; bool do_async = false; + SMB_ASSERT(!is_named_stream(smb_fname)); + req = tevent_req_create(mem_ctx, &state, struct vfswrap_getxattrat_state); if (req == NULL) { @@ -3661,10 +3681,9 @@ static void vfswrap_getxattrat_do_sync(struct tevent_req *req) { struct vfswrap_getxattrat_state *state = tevent_req_data( req, struct vfswrap_getxattrat_state); - struct files_struct *fsp = metadata_fsp(state->smb_fname->fsp); state->xattr_size = vfswrap_fgetxattr(state->handle, - fsp, + state->smb_fname->fsp, state->xattr_name, state->xattr_value, talloc_array_length(state->xattr_value)); @@ -3684,7 +3703,6 @@ static void vfswrap_getxattrat_do_async(void *private_data) struct timespec start_time; struct timespec end_time; int ret; - struct files_struct *fsp = metadata_fsp(state->smb_fname->fsp); PROFILE_TIMESTAMP(&start_time); SMBPROFILE_BYTES_ASYNC_SET_BUSY(state->profile_bytes); @@ -3708,7 +3726,7 @@ static void vfswrap_getxattrat_do_async(void *private_data) } state->xattr_size = vfswrap_fgetxattr(state->handle, - fsp, + state->smb_fname->fsp, state->xattr_name, state->xattr_value, talloc_array_length(state->xattr_value)); @@ -3809,6 +3827,8 @@ static ssize_t vfswrap_flistxattr(struct vfs_handle_struct *handle, struct files { int fd = fsp_get_pathref_fd(fsp); + SMB_ASSERT(!fsp_is_alternate_stream(fsp)); + if (!fsp->fsp_flags.is_pathref) { return flistxattr(fd, list, size); } @@ -3835,6 +3855,8 @@ static int vfswrap_fremovexattr(struct vfs_handle_struct *handle, struct files_s { int fd = fsp_get_pathref_fd(fsp); + SMB_ASSERT(!fsp_is_alternate_stream(fsp)); + if (!fsp->fsp_flags.is_pathref) { return fremovexattr(fd, name); } @@ -3861,6 +3883,8 @@ static int vfswrap_fsetxattr(struct vfs_handle_struct *handle, struct files_stru { int fd = fsp_get_pathref_fd(fsp); + SMB_ASSERT(!fsp_is_alternate_stream(fsp)); + if (!fsp->fsp_flags.is_pathref) { return fsetxattr(fd, name, value, size, flags); } -- 2.34.1