Skip to content

Commit ac5656d

Browse files
Aaron Goidelpcmoore
authored andcommitted
fanotify, inotify, dnotify, security: add security hook for fs notifications
As of now, setting watches on filesystem objects has, at most, applied a check for read access to the inode, and in the case of fanotify, requires CAP_SYS_ADMIN. No specific security hook or permission check has been provided to control the setting of watches. Using any of inotify, dnotify, or fanotify, it is possible to observe, not only write-like operations, but even read access to a file. Modeling the watch as being merely a read from the file is insufficient for the needs of SELinux. This is due to the fact that read access should not necessarily imply access to information about when another process reads from a file. Furthermore, fanotify watches grant more power to an application in the form of permission events. While notification events are solely, unidirectional (i.e. they only pass information to the receiving application), permission events are blocking. Permission events make a request to the receiving application which will then reply with a decision as to whether or not that action may be completed. This causes the issue of the watching application having the ability to exercise control over the triggering process. Without drawing a distinction within the permission check, the ability to read would imply the greater ability to control an application. Additionally, mount and superblock watches apply to all files within the same mount or superblock. Read access to one file should not necessarily imply the ability to watch all files accessed within a given mount or superblock. In order to solve these issues, a new LSM hook is implemented and has been placed within the system calls for marking filesystem objects with inotify, fanotify, and dnotify watches. These calls to the hook are placed at the point at which the target path has been resolved and are provided with the path struct, the mask of requested notification events, and the type of object on which the mark is being set (inode, superblock, or mount). The mask and obj_type have already been translated into common FS_* values shared by the entirety of the fs notification infrastructure. The path struct is passed rather than just the inode so that the mount is available, particularly for mount watches. This also allows for use of the hook by pathname-based security modules. However, since the hook is intended for use even by inode based security modules, it is not placed under the CONFIG_SECURITY_PATH conditional. Otherwise, the inode-based security modules would need to enable all of the path hooks, even though they do not use any of them. This only provides a hook at the point of setting a watch, and presumes that permission to set a particular watch implies the ability to receive all notification about that object which match the mask. This is all that is required for SELinux. If other security modules require additional hooks or infrastructure to control delivery of notification, these can be added by them. It does not make sense for us to propose hooks for which we have no implementation. The understanding that all notifications received by the requesting application are all strictly of a type for which the application has been granted permission shows that this implementation is sufficient in its coverage. Security modules wishing to provide complete control over fanotify must also implement a security_file_open hook that validates that the access requested by the watching application is authorized. Fanotify has the issue that it returns a file descriptor with the file mode specified during fanotify_init() to the watching process on event. This is already covered by the LSM security_file_open hook if the security module implements checking of the requested file mode there. Otherwise, a watching process can obtain escalated access to a file for which it has not been authorized. The selinux_path_notify hook implementation works by adding five new file permissions: watch, watch_mount, watch_sb, watch_reads, and watch_with_perm (descriptions about which will follow), and one new filesystem permission: watch (which is applied to superblock checks). The hook then decides which subset of these permissions must be held by the requesting application based on the contents of the provided mask and the obj_type. The selinux_file_open hook already checks the requested file mode and therefore ensures that a watching process cannot escalate its access through fanotify. The watch, watch_mount, and watch_sb permissions are the baseline permissions for setting a watch on an object and each are a requirement for any watch to be set on a file, mount, or superblock respectively. It should be noted that having either of the other two permissions (watch_reads and watch_with_perm) does not imply the watch, watch_mount, or watch_sb permission. Superblock watches further require the filesystem watch permission to the superblock. As there is no labeled object in view for mounts, there is no specific check for mount watches beyond watch_mount to the inode. Such a check could be added in the future, if a suitable labeled object existed representing the mount. The watch_reads permission is required to receive notifications from read-exclusive events on filesystem objects. These events include accessing a file for the purpose of reading and closing a file which has been opened read-only. This distinction has been drawn in order to provide a direct indication in the policy for this otherwise not obvious capability. Read access to a file should not necessarily imply the ability to observe read events on a file. Finally, watch_with_perm only applies to fanotify masks since it is the only way to set a mask which allows for the blocking, permission event. This permission is needed for any watch which is of this type. Though fanotify requires CAP_SYS_ADMIN, this is insufficient as it gives implicit trust to root, which we do not do, and does not support least privilege. Signed-off-by: Aaron Goidel <[email protected]> Acked-by: Casey Schaufler <[email protected]> Acked-by: Jan Kara <[email protected]> Signed-off-by: Paul Moore <[email protected]>
1 parent 9b80c36 commit ac5656d

File tree

8 files changed

+113
-12
lines changed

8 files changed

+113
-12
lines changed

fs/notify/dnotify/dnotify.c

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
#include <linux/sched/signal.h>
2323
#include <linux/dnotify.h>
2424
#include <linux/init.h>
25+
#include <linux/security.h>
2526
#include <linux/spinlock.h>
2627
#include <linux/slab.h>
2728
#include <linux/fdtable.h>
@@ -288,6 +289,17 @@ int fcntl_dirnotify(int fd, struct file *filp, unsigned long arg)
288289
goto out_err;
289290
}
290291

292+
/*
293+
* convert the userspace DN_* "arg" to the internal FS_*
294+
* defined in fsnotify
295+
*/
296+
mask = convert_arg(arg);
297+
298+
error = security_path_notify(&filp->f_path, mask,
299+
FSNOTIFY_OBJ_TYPE_INODE);
300+
if (error)
301+
goto out_err;
302+
291303
/* expect most fcntl to add new rather than augment old */
292304
dn = kmem_cache_alloc(dnotify_struct_cache, GFP_KERNEL);
293305
if (!dn) {
@@ -302,9 +314,6 @@ int fcntl_dirnotify(int fd, struct file *filp, unsigned long arg)
302314
goto out_err;
303315
}
304316

305-
/* convert the userspace DN_* "arg" to the internal FS_* defines in fsnotify */
306-
mask = convert_arg(arg);
307-
308317
/* set up the new_fsn_mark and new_dn_mark */
309318
new_fsn_mark = &new_dn_mark->fsn_mark;
310319
fsnotify_init_mark(new_fsn_mark, dnotify_group);

fs/notify/fanotify/fanotify_user.c

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -528,7 +528,8 @@ static const struct file_operations fanotify_fops = {
528528
};
529529

530530
static int fanotify_find_path(int dfd, const char __user *filename,
531-
struct path *path, unsigned int flags)
531+
struct path *path, unsigned int flags, __u64 mask,
532+
unsigned int obj_type)
532533
{
533534
int ret;
534535

@@ -567,8 +568,15 @@ static int fanotify_find_path(int dfd, const char __user *filename,
567568

568569
/* you can only watch an inode if you have read permissions on it */
569570
ret = inode_permission(path->dentry->d_inode, MAY_READ);
571+
if (ret) {
572+
path_put(path);
573+
goto out;
574+
}
575+
576+
ret = security_path_notify(path, mask, obj_type);
570577
if (ret)
571578
path_put(path);
579+
572580
out:
573581
return ret;
574582
}
@@ -931,6 +939,7 @@ static int do_fanotify_mark(int fanotify_fd, unsigned int flags, __u64 mask,
931939
__kernel_fsid_t __fsid, *fsid = NULL;
932940
u32 valid_mask = FANOTIFY_EVENTS | FANOTIFY_EVENT_FLAGS;
933941
unsigned int mark_type = flags & FANOTIFY_MARK_TYPE_BITS;
942+
unsigned int obj_type;
934943
int ret;
935944

936945
pr_debug("%s: fanotify_fd=%d flags=%x dfd=%d pathname=%p mask=%llx\n",
@@ -945,8 +954,13 @@ static int do_fanotify_mark(int fanotify_fd, unsigned int flags, __u64 mask,
945954

946955
switch (mark_type) {
947956
case FAN_MARK_INODE:
957+
obj_type = FSNOTIFY_OBJ_TYPE_INODE;
958+
break;
948959
case FAN_MARK_MOUNT:
960+
obj_type = FSNOTIFY_OBJ_TYPE_VFSMOUNT;
961+
break;
949962
case FAN_MARK_FILESYSTEM:
963+
obj_type = FSNOTIFY_OBJ_TYPE_SB;
950964
break;
951965
default:
952966
return -EINVAL;
@@ -1014,7 +1028,8 @@ static int do_fanotify_mark(int fanotify_fd, unsigned int flags, __u64 mask,
10141028
goto fput_and_out;
10151029
}
10161030

1017-
ret = fanotify_find_path(dfd, pathname, &path, flags);
1031+
ret = fanotify_find_path(dfd, pathname, &path, flags,
1032+
(mask & ALL_FSNOTIFY_EVENTS), obj_type);
10181033
if (ret)
10191034
goto fput_and_out;
10201035

fs/notify/inotify/inotify_user.c

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@
3939
#include <linux/poll.h>
4040
#include <linux/wait.h>
4141
#include <linux/memcontrol.h>
42+
#include <linux/security.h>
4243

4344
#include "inotify.h"
4445
#include "../fdinfo.h"
@@ -342,7 +343,8 @@ static const struct file_operations inotify_fops = {
342343
/*
343344
* find_inode - resolve a user-given path to a specific inode
344345
*/
345-
static int inotify_find_inode(const char __user *dirname, struct path *path, unsigned flags)
346+
static int inotify_find_inode(const char __user *dirname, struct path *path,
347+
unsigned int flags, __u64 mask)
346348
{
347349
int error;
348350

@@ -351,8 +353,15 @@ static int inotify_find_inode(const char __user *dirname, struct path *path, uns
351353
return error;
352354
/* you can only watch an inode if you have read permissions on it */
353355
error = inode_permission(path->dentry->d_inode, MAY_READ);
356+
if (error) {
357+
path_put(path);
358+
return error;
359+
}
360+
error = security_path_notify(path, mask,
361+
FSNOTIFY_OBJ_TYPE_INODE);
354362
if (error)
355363
path_put(path);
364+
356365
return error;
357366
}
358367

@@ -744,7 +753,8 @@ SYSCALL_DEFINE3(inotify_add_watch, int, fd, const char __user *, pathname,
744753
if (mask & IN_ONLYDIR)
745754
flags |= LOOKUP_DIRECTORY;
746755

747-
ret = inotify_find_inode(pathname, &path, flags);
756+
ret = inotify_find_inode(pathname, &path, flags,
757+
(mask & IN_ALL_EVENTS));
748758
if (ret)
749759
goto fput_and_out;
750760

include/linux/lsm_hooks.h

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -339,6 +339,9 @@
339339
* Check for permission to change root directory.
340340
* @path contains the path structure.
341341
* Return 0 if permission is granted.
342+
* @path_notify:
343+
* Check permissions before setting a watch on events as defined by @mask,
344+
* on an object at @path, whose type is defined by @obj_type.
342345
* @inode_readlink:
343346
* Check the permission to read the symbolic link.
344347
* @dentry contains the dentry structure for the file link.
@@ -1535,7 +1538,9 @@ union security_list_options {
15351538
int (*path_chown)(const struct path *path, kuid_t uid, kgid_t gid);
15361539
int (*path_chroot)(const struct path *path);
15371540
#endif
1538-
1541+
/* Needed for inode based security check */
1542+
int (*path_notify)(const struct path *path, u64 mask,
1543+
unsigned int obj_type);
15391544
int (*inode_alloc_security)(struct inode *inode);
15401545
void (*inode_free_security)(struct inode *inode);
15411546
int (*inode_init_security)(struct inode *inode, struct inode *dir,
@@ -1860,6 +1865,8 @@ struct security_hook_heads {
18601865
struct hlist_head path_chown;
18611866
struct hlist_head path_chroot;
18621867
#endif
1868+
/* Needed for inode based modules as well */
1869+
struct hlist_head path_notify;
18631870
struct hlist_head inode_alloc_security;
18641871
struct hlist_head inode_free_security;
18651872
struct hlist_head inode_init_security;

include/linux/security.h

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -259,7 +259,8 @@ int security_dentry_create_files_as(struct dentry *dentry, int mode,
259259
struct qstr *name,
260260
const struct cred *old,
261261
struct cred *new);
262-
262+
int security_path_notify(const struct path *path, u64 mask,
263+
unsigned int obj_type);
263264
int security_inode_alloc(struct inode *inode);
264265
void security_inode_free(struct inode *inode);
265266
int security_inode_init_security(struct inode *inode, struct inode *dir,
@@ -387,7 +388,6 @@ int security_ismaclabel(const char *name);
387388
int security_secid_to_secctx(u32 secid, char **secdata, u32 *seclen);
388389
int security_secctx_to_secid(const char *secdata, u32 seclen, u32 *secid);
389390
void security_release_secctx(char *secdata, u32 seclen);
390-
391391
void security_inode_invalidate_secctx(struct inode *inode);
392392
int security_inode_notifysecctx(struct inode *inode, void *ctx, u32 ctxlen);
393393
int security_inode_setsecctx(struct dentry *dentry, void *ctx, u32 ctxlen);
@@ -621,6 +621,12 @@ static inline int security_move_mount(const struct path *from_path,
621621
return 0;
622622
}
623623

624+
static inline int security_path_notify(const struct path *path, u64 mask,
625+
unsigned int obj_type)
626+
{
627+
return 0;
628+
}
629+
624630
static inline int security_inode_alloc(struct inode *inode)
625631
{
626632
return 0;

security/security.c

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -871,6 +871,12 @@ int security_move_mount(const struct path *from_path, const struct path *to_path
871871
return call_int_hook(move_mount, 0, from_path, to_path);
872872
}
873873

874+
int security_path_notify(const struct path *path, u64 mask,
875+
unsigned int obj_type)
876+
{
877+
return call_int_hook(path_notify, 0, path, mask, obj_type);
878+
}
879+
874880
int security_inode_alloc(struct inode *inode)
875881
{
876882
int rc = lsm_inode_alloc(inode);

security/selinux/hooks.c

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,8 @@
9292
#include <linux/kernfs.h>
9393
#include <linux/stringhash.h> /* for hashlen_string() */
9494
#include <uapi/linux/mount.h>
95+
#include <linux/fsnotify.h>
96+
#include <linux/fanotify.h>
9597

9698
#include "avc.h"
9799
#include "objsec.h"
@@ -3261,6 +3263,50 @@ static int selinux_inode_removexattr(struct dentry *dentry, const char *name)
32613263
return -EACCES;
32623264
}
32633265

3266+
static int selinux_path_notify(const struct path *path, u64 mask,
3267+
unsigned int obj_type)
3268+
{
3269+
int ret;
3270+
u32 perm;
3271+
3272+
struct common_audit_data ad;
3273+
3274+
ad.type = LSM_AUDIT_DATA_PATH;
3275+
ad.u.path = *path;
3276+
3277+
/*
3278+
* Set permission needed based on the type of mark being set.
3279+
* Performs an additional check for sb watches.
3280+
*/
3281+
switch (obj_type) {
3282+
case FSNOTIFY_OBJ_TYPE_VFSMOUNT:
3283+
perm = FILE__WATCH_MOUNT;
3284+
break;
3285+
case FSNOTIFY_OBJ_TYPE_SB:
3286+
perm = FILE__WATCH_SB;
3287+
ret = superblock_has_perm(current_cred(), path->dentry->d_sb,
3288+
FILESYSTEM__WATCH, &ad);
3289+
if (ret)
3290+
return ret;
3291+
break;
3292+
case FSNOTIFY_OBJ_TYPE_INODE:
3293+
perm = FILE__WATCH;
3294+
break;
3295+
default:
3296+
return -EINVAL;
3297+
}
3298+
3299+
/* blocking watches require the file:watch_with_perm permission */
3300+
if (mask & (ALL_FSNOTIFY_PERM_EVENTS))
3301+
perm |= FILE__WATCH_WITH_PERM;
3302+
3303+
/* watches on read-like events need the file:watch_reads permission */
3304+
if (mask & (FS_ACCESS | FS_ACCESS_PERM | FS_CLOSE_NOWRITE))
3305+
perm |= FILE__WATCH_READS;
3306+
3307+
return path_has_perm(current_cred(), path, perm);
3308+
}
3309+
32643310
/*
32653311
* Copy the inode security context value to the user.
32663312
*
@@ -6798,6 +6844,7 @@ static struct security_hook_list selinux_hooks[] __lsm_ro_after_init = {
67986844
LSM_HOOK_INIT(inode_getsecid, selinux_inode_getsecid),
67996845
LSM_HOOK_INIT(inode_copy_up, selinux_inode_copy_up),
68006846
LSM_HOOK_INIT(inode_copy_up_xattr, selinux_inode_copy_up_xattr),
6847+
LSM_HOOK_INIT(path_notify, selinux_path_notify),
68016848

68026849
LSM_HOOK_INIT(kernfs_init_security, selinux_kernfs_init_security),
68036850

security/selinux/include/classmap.h

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,8 @@
77

88
#define COMMON_FILE_PERMS COMMON_FILE_SOCK_PERMS, "unlink", "link", \
99
"rename", "execute", "quotaon", "mounton", "audit_access", \
10-
"open", "execmod"
10+
"open", "execmod", "watch", "watch_mount", "watch_sb", \
11+
"watch_with_perm", "watch_reads"
1112

1213
#define COMMON_SOCK_PERMS COMMON_FILE_SOCK_PERMS, "bind", "connect", \
1314
"listen", "accept", "getopt", "setopt", "shutdown", "recvfrom", \
@@ -60,7 +61,7 @@ struct security_class_mapping secclass_map[] = {
6061
{ "filesystem",
6162
{ "mount", "remount", "unmount", "getattr",
6263
"relabelfrom", "relabelto", "associate", "quotamod",
63-
"quotaget", NULL } },
64+
"quotaget", "watch", NULL } },
6465
{ "file",
6566
{ COMMON_FILE_PERMS,
6667
"execute_no_trans", "entrypoint", NULL } },

0 commit comments

Comments
 (0)