From: Björn Baumbach Date: Tue, 6 Nov 2018 14:21:37 +0000 (+0100) Subject: smbd/notify: add option "honor change notify privilege" X-Git-Tag: samba-4.14.0rc1~232 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=6c95e467817b246a0eab626cac10b6120f6c88f8;p=thirdparty%2Fsamba.git smbd/notify: add option "honor change notify privilege" This option can be used to make use of the change notify privilege. By default notify results are not checked against the file system permissions. If "honor change notify privilege" is enabled, a user will only receive notify results, if he has change notify privilege or sufficient file system permissions. If a user has the change notify privilege, he will receive all requested notify results, even if the user does not have the permissions on the file system. Pair-Programmed-With: Stefan Metzmacher Signed-off-by: Björn Baumbach Signed-off-by: Stefan Metzmacher Reviewed-by: Jeremy Allison Autobuild-User(master): Stefan Metzmacher Autobuild-Date(master): Thu Dec 17 15:01:53 UTC 2020 on sn-devel-184 --- diff --git a/docs-xml/smbdotconf/misc/honorchangenotifyprivilege.xml b/docs-xml/smbdotconf/misc/honorchangenotifyprivilege.xml new file mode 100644 index 00000000000..a9c880ce467 --- /dev/null +++ b/docs-xml/smbdotconf/misc/honorchangenotifyprivilege.xml @@ -0,0 +1,20 @@ + + + + This option can be used to make use of the change notify privilege. + By default notify results are not checked against the file system + permissions. + + + If "honor change notify privilege" is enabled, a user will only + receive notify results, if he has change notify privilege or + sufficient file system permissions. If a user has the change notify + privilege, he will receive all requested notify results, even if the + user does not have the permissions on the file system. + + +no + diff --git a/selftest/knownfail.d/notify_privileged b/selftest/knownfail.d/notify_privileged deleted file mode 100644 index 44ff8cbd1b0..00000000000 --- a/selftest/knownfail.d/notify_privileged +++ /dev/null @@ -1 +0,0 @@ -^samba.tests.smb-notify.samba.tests.smb-notify.SMBNotifyTests.test_notify_privileged diff --git a/selftest/target/Samba3.pm b/selftest/target/Samba3.pm index 9c8b657e66d..ee20528a325 100755 --- a/selftest/target/Samba3.pm +++ b/selftest/target/Samba3.pm @@ -2892,6 +2892,7 @@ sub provision($$) [notify_priv] copy = tmp + honor change notify privilege = yes "; close(CONF); diff --git a/source3/param/loadparm.c b/source3/param/loadparm.c index 3de22b350f1..acb4d149f0b 100644 --- a/source3/param/loadparm.c +++ b/source3/param/loadparm.c @@ -250,6 +250,7 @@ static const struct loadparm_service _sDefault = .smbd_search_ask_sharemode = true, .smbd_getinfo_ask_sharemode = true, .spotlight_backend = SPOTLIGHT_BACKEND_NOINDEX, + .honor_change_notify_privilege = false, .dummy = "" }; diff --git a/source3/smbd/notify.c b/source3/smbd/notify.c index 5f18b5cf794..43abef0434d 100644 --- a/source3/smbd/notify.c +++ b/source3/smbd/notify.c @@ -24,6 +24,8 @@ #include "smbd/globals.h" #include "../librpc/gen_ndr/ndr_notify.h" #include "librpc/gen_ndr/ndr_file_id.h" +#include "libcli/security/privileges.h" +#include "libcli/security/security.h" struct notify_change_event { struct timespec when; @@ -597,6 +599,104 @@ void notify_fname(connection_struct *conn, uint32_t action, uint32_t filter, notify_trigger(notify_ctx, action, filter, conn->connectpath, path); } +static bool user_can_stat_name_under_fsp(files_struct *fsp, const char *name) +{ + uint32_t rights; + struct smb_filename fname; + char *filepath = NULL; + NTSTATUS status; + char *p = NULL; + + /* + * Assume we get filepath (relative to the share) + * like this: + * + * 'dir1/dir2/dir3/file' + * + * We start with LIST and TRAVERSE on the + * direct parent ('dir1/dir2/dir3') + * + * Then we switch to just TRAVERSE for + * the rest: 'dir1/dir2', 'dir1', '.' + * + * For a file in the share root, we'll have + * 'file' + * and would just check '.' with LIST and TRAVERSE. + * + * It's important to always check '.' as the last step, + * which means we check the permissions of the share root + * directory. + */ + + if (ISDOT(fsp->fsp_name->base_name)) { + filepath = talloc_strdup(talloc_tos(), name); + } else { + filepath = talloc_asprintf(talloc_tos(), + "%s/%s", + fsp->fsp_name->base_name, + name); + } + if (filepath == NULL) { + DBG_ERR("Memory allocation failed\n"); + return false; + } + + fname = (struct smb_filename) { .base_name = filepath }; + + rights = SEC_DIR_LIST|SEC_DIR_TRAVERSE; + p = strrchr_m(filepath, '/'); + /* + * Check each path component, exluding the share root. + * + * We could check all components including root using + * a do { .. } while() loop, but IMHO the logic is clearer + * having the share root check separately afterwards. + */ + while (p != NULL) { + *p = '\0'; + status = smbd_check_access_rights(fsp->conn, + fsp->conn->cwd_fsp, + &fname, + false, + rights); + if (!NT_STATUS_IS_OK(status)) { + DBG_DEBUG("Access rights for %s/%s: %s\n", + fsp->conn->connectpath, + filepath, + nt_errstr(status)); + TALLOC_FREE(filepath); + return false; + } + + rights = SEC_DIR_TRAVERSE; + p = strrchr_m(filepath, '/'); + } + + TALLOC_FREE(filepath); + + /* Finally check share root. */ + filepath = talloc_strdup(talloc_tos(), "."); + if (filepath == NULL) { + DBG_ERR("Memory allocation failed\n"); + return false; + } + fname = (struct smb_filename) { .base_name = filepath }; + status = smbd_check_access_rights(fsp->conn, + fsp->conn->cwd_fsp, + &fname, + false, + rights); + if (!NT_STATUS_IS_OK(status)) { + DBG_DEBUG("Access rights for %s/.: %s\n", + fsp->conn->connectpath, + nt_errstr(status)); + TALLOC_FREE(filepath); + return false; + } + TALLOC_FREE(filepath); + return true; +} + static void notify_fsp(files_struct *fsp, struct timespec when, uint32_t action, const char *name) { @@ -610,6 +710,35 @@ static void notify_fsp(files_struct *fsp, struct timespec when, return; } + if (lp_honor_change_notify_privilege(SNUM(fsp->conn))) { + bool has_sec_change_notify_privilege; + bool expose = false; + + has_sec_change_notify_privilege = security_token_has_privilege( + fsp->conn->session_info->security_token, + SEC_PRIV_CHANGE_NOTIFY); + + if (has_sec_change_notify_privilege) { + expose = true; + } else { + bool ok; + + ok = become_user_without_service_by_fsp(fsp); + if (ok) { + expose = user_can_stat_name_under_fsp(fsp, name); + unbecome_user_without_service(); + } + } + DBG_DEBUG("has_sec_change_notify_privilege=%s " + "expose=%s for %s notify %s\n", + has_sec_change_notify_privilege ? "true" : "false", + expose ? "true" : "false", + fsp->fsp_name->base_name, name); + if (!expose) { + return; + } + } + /* * Someone has triggered a notify previously, queue the change for * later.