* [PATCH v4 0/3] Safe, dynamically loadable LSM hooks
@ 2018-03-07 7:22 Sargun Dhillon
2018-03-07 7:23 ` [PATCH v4 1/3] security: Refactor LSM hooks into an array and enum Sargun Dhillon
` (3 more replies)
0 siblings, 4 replies; 11+ messages in thread
From: Sargun Dhillon @ 2018-03-07 7:22 UTC (permalink / raw)
To: linux-security-module, linux-kernel
Cc: penguin-kernel, keescook, igor.stoppa, casey
This patchset introduces safe dynamic LSM support. These are currently
not unloadable, until we figure out a use case that needs that. Adding
an unload hook is trivial given the way the patch is written.
This exposes a second mechanism of loading hooks which are in modules.
These hooks are behind static keys, so they should come at low performance
overhead. The built-in hook heads are read-only, whereas the dynamic hooks
are mutable.
Not all hooks can be loaded into. Some hooks are blacklisted, and therefore
trying to load a module which plugs into those hooks will fail.
One of the big benefits with loadable security modules is to help with
"unknown unknowns". Although, livepatch is excellent, sometimes, a
surgical LSM is simpler.
It includes an example LSM that prevents specific time travel.
Changes since v3:
* readded hook blacklisted
* return error, rather than panic if unable to allocate memory
Changes since v2:
* inode get/set security is readded
* xfrm singleton hook readded
* Security hooks are turned into an array
* Security hooks and dynamic hooks enum is collapsed
Changes since v1:
* It no longer allows unloading of modules
* prctl is fixed
* inode get/set security is removed
* xfrm singleton hook removed
Sargun Dhillon (3):
security: Refactor LSM hooks into an array and enum
security: Expose a mechanism to load lsm hooks dynamically at runtime
security: Add an example sample dynamic LSM
include/linux/lsm_hooks.h | 459 ++++++++++++++++++++++++----------------------
samples/Kconfig | 6 +
samples/Makefile | 2 +-
samples/lsm/Makefile | 4 +
samples/lsm/lsm_example.c | 33 ++++
security/Kconfig | 9 +
security/inode.c | 13 +-
security/security.c | 222 ++++++++++++++++++++--
8 files changed, 508 insertions(+), 240 deletions(-)
create mode 100644 samples/lsm/Makefile
create mode 100644 samples/lsm/lsm_example.c
--
2.14.1
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v4 1/3] security: Refactor LSM hooks into an array and enum
2018-03-07 7:22 [PATCH v4 0/3] Safe, dynamically loadable LSM hooks Sargun Dhillon
@ 2018-03-07 7:23 ` Sargun Dhillon
2018-03-07 17:45 ` Casey Schaufler
2018-03-07 7:23 ` [PATCH v4 2/3] security: Expose a mechanism to load lsm hooks dynamically at runtime Sargun Dhillon
` (2 subsequent siblings)
3 siblings, 1 reply; 11+ messages in thread
From: Sargun Dhillon @ 2018-03-07 7:23 UTC (permalink / raw)
To: linux-security-module, linux-kernel
Cc: penguin-kernel, keescook, igor.stoppa, casey
This commit should have no functional change. It changes the security hook
list heads struct into an array. Additionally, it exposes all of the hooks
via an enum. This loses memory layout randomization as the enum is not
randomized.
Signed-off-by: Sargun Dhillon <sargun@sargun.me>
---
include/linux/lsm_hooks.h | 433 +++++++++++++++++++++++-----------------------
security/security.c | 30 ++--
2 files changed, 233 insertions(+), 230 deletions(-)
diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index 7161d8e7ee79..d28c7f5b01c1 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -1729,241 +1729,243 @@ union security_list_options {
#endif /* CONFIG_BPF_SYSCALL */
};
-struct security_hook_heads {
- struct list_head binder_set_context_mgr;
- struct list_head binder_transaction;
- struct list_head binder_transfer_binder;
- struct list_head binder_transfer_file;
- struct list_head ptrace_access_check;
- struct list_head ptrace_traceme;
- struct list_head capget;
- struct list_head capset;
- struct list_head capable;
- struct list_head quotactl;
- struct list_head quota_on;
- struct list_head syslog;
- struct list_head settime;
- struct list_head vm_enough_memory;
- struct list_head bprm_set_creds;
- struct list_head bprm_check_security;
- struct list_head bprm_committing_creds;
- struct list_head bprm_committed_creds;
- struct list_head sb_alloc_security;
- struct list_head sb_free_security;
- struct list_head sb_copy_data;
- struct list_head sb_remount;
- struct list_head sb_kern_mount;
- struct list_head sb_show_options;
- struct list_head sb_statfs;
- struct list_head sb_mount;
- struct list_head sb_umount;
- struct list_head sb_pivotroot;
- struct list_head sb_set_mnt_opts;
- struct list_head sb_clone_mnt_opts;
- struct list_head sb_parse_opts_str;
- struct list_head dentry_init_security;
- struct list_head dentry_create_files_as;
+enum lsm_hook {
+ LSM_HOOK_binder_set_context_mgr,
+ LSM_HOOK_binder_transaction,
+ LSM_HOOK_binder_transfer_binder,
+ LSM_HOOK_binder_transfer_file,
+ LSM_HOOK_ptrace_access_check,
+ LSM_HOOK_ptrace_traceme,
+ LSM_HOOK_capget,
+ LSM_HOOK_capset,
+ LSM_HOOK_capable,
+ LSM_HOOK_quotactl,
+ LSM_HOOK_quota_on,
+ LSM_HOOK_syslog,
+ LSM_HOOK_settime,
+ LSM_HOOK_vm_enough_memory,
+ LSM_HOOK_bprm_set_creds,
+ LSM_HOOK_bprm_check_security,
+ LSM_HOOK_bprm_committing_creds,
+ LSM_HOOK_bprm_committed_creds,
+ LSM_HOOK_sb_alloc_security,
+ LSM_HOOK_sb_free_security,
+ LSM_HOOK_sb_copy_data,
+ LSM_HOOK_sb_remount,
+ LSM_HOOK_sb_kern_mount,
+ LSM_HOOK_sb_show_options,
+ LSM_HOOK_sb_statfs,
+ LSM_HOOK_sb_mount,
+ LSM_HOOK_sb_umount,
+ LSM_HOOK_sb_pivotroot,
+ LSM_HOOK_sb_set_mnt_opts,
+ LSM_HOOK_sb_clone_mnt_opts,
+ LSM_HOOK_sb_parse_opts_str,
+ LSM_HOOK_dentry_init_security,
+ LSM_HOOK_dentry_create_files_as,
#ifdef CONFIG_SECURITY_PATH
- struct list_head path_unlink;
- struct list_head path_mkdir;
- struct list_head path_rmdir;
- struct list_head path_mknod;
- struct list_head path_truncate;
- struct list_head path_symlink;
- struct list_head path_link;
- struct list_head path_rename;
- struct list_head path_chmod;
- struct list_head path_chown;
- struct list_head path_chroot;
+ LSM_HOOK_path_unlink,
+ LSM_HOOK_path_mkdir,
+ LSM_HOOK_path_rmdir,
+ LSM_HOOK_path_mknod,
+ LSM_HOOK_path_truncate,
+ LSM_HOOK_path_symlink,
+ LSM_HOOK_path_link,
+ LSM_HOOK_path_rename,
+ LSM_HOOK_path_chmod,
+ LSM_HOOK_path_chown,
+ LSM_HOOK_path_chroot,
#endif
- struct list_head inode_alloc_security;
- struct list_head inode_free_security;
- struct list_head inode_init_security;
- struct list_head inode_create;
- struct list_head inode_link;
- struct list_head inode_unlink;
- struct list_head inode_symlink;
- struct list_head inode_mkdir;
- struct list_head inode_rmdir;
- struct list_head inode_mknod;
- struct list_head inode_rename;
- struct list_head inode_readlink;
- struct list_head inode_follow_link;
- struct list_head inode_permission;
- struct list_head inode_setattr;
- struct list_head inode_getattr;
- struct list_head inode_setxattr;
- struct list_head inode_post_setxattr;
- struct list_head inode_getxattr;
- struct list_head inode_listxattr;
- struct list_head inode_removexattr;
- struct list_head inode_need_killpriv;
- struct list_head inode_killpriv;
- struct list_head inode_getsecurity;
- struct list_head inode_setsecurity;
- struct list_head inode_listsecurity;
- struct list_head inode_getsecid;
- struct list_head inode_copy_up;
- struct list_head inode_copy_up_xattr;
- struct list_head file_permission;
- struct list_head file_alloc_security;
- struct list_head file_free_security;
- struct list_head file_ioctl;
- struct list_head mmap_addr;
- struct list_head mmap_file;
- struct list_head file_mprotect;
- struct list_head file_lock;
- struct list_head file_fcntl;
- struct list_head file_set_fowner;
- struct list_head file_send_sigiotask;
- struct list_head file_receive;
- struct list_head file_open;
- struct list_head task_alloc;
- struct list_head task_free;
- struct list_head cred_alloc_blank;
- struct list_head cred_free;
- struct list_head cred_prepare;
- struct list_head cred_transfer;
- struct list_head kernel_act_as;
- struct list_head kernel_create_files_as;
- struct list_head kernel_read_file;
- struct list_head kernel_post_read_file;
- struct list_head kernel_module_request;
- struct list_head task_fix_setuid;
- struct list_head task_setpgid;
- struct list_head task_getpgid;
- struct list_head task_getsid;
- struct list_head task_getsecid;
- struct list_head task_setnice;
- struct list_head task_setioprio;
- struct list_head task_getioprio;
- struct list_head task_prlimit;
- struct list_head task_setrlimit;
- struct list_head task_setscheduler;
- struct list_head task_getscheduler;
- struct list_head task_movememory;
- struct list_head task_kill;
- struct list_head task_prctl;
- struct list_head task_to_inode;
- struct list_head ipc_permission;
- struct list_head ipc_getsecid;
- struct list_head msg_msg_alloc_security;
- struct list_head msg_msg_free_security;
- struct list_head msg_queue_alloc_security;
- struct list_head msg_queue_free_security;
- struct list_head msg_queue_associate;
- struct list_head msg_queue_msgctl;
- struct list_head msg_queue_msgsnd;
- struct list_head msg_queue_msgrcv;
- struct list_head shm_alloc_security;
- struct list_head shm_free_security;
- struct list_head shm_associate;
- struct list_head shm_shmctl;
- struct list_head shm_shmat;
- struct list_head sem_alloc_security;
- struct list_head sem_free_security;
- struct list_head sem_associate;
- struct list_head sem_semctl;
- struct list_head sem_semop;
- struct list_head netlink_send;
- struct list_head d_instantiate;
- struct list_head getprocattr;
- struct list_head setprocattr;
- struct list_head ismaclabel;
- struct list_head secid_to_secctx;
- struct list_head secctx_to_secid;
- struct list_head release_secctx;
- struct list_head inode_invalidate_secctx;
- struct list_head inode_notifysecctx;
- struct list_head inode_setsecctx;
- struct list_head inode_getsecctx;
+ LSM_HOOK_inode_alloc_security,
+ LSM_HOOK_inode_free_security,
+ LSM_HOOK_inode_init_security,
+ LSM_HOOK_inode_create,
+ LSM_HOOK_inode_link,
+ LSM_HOOK_inode_unlink,
+ LSM_HOOK_inode_symlink,
+ LSM_HOOK_inode_mkdir,
+ LSM_HOOK_inode_rmdir,
+ LSM_HOOK_inode_mknod,
+ LSM_HOOK_inode_rename,
+ LSM_HOOK_inode_readlink,
+ LSM_HOOK_inode_follow_link,
+ LSM_HOOK_inode_permission,
+ LSM_HOOK_inode_setattr,
+ LSM_HOOK_inode_getattr,
+ LSM_HOOK_inode_setxattr,
+ LSM_HOOK_inode_post_setxattr,
+ LSM_HOOK_inode_getxattr,
+ LSM_HOOK_inode_listxattr,
+ LSM_HOOK_inode_removexattr,
+ LSM_HOOK_inode_need_killpriv,
+ LSM_HOOK_inode_killpriv,
+ LSM_HOOK_inode_getsecurity,
+ LSM_HOOK_inode_setsecurity,
+ LSM_HOOK_inode_listsecurity,
+ LSM_HOOK_inode_getsecid,
+ LSM_HOOK_inode_copy_up,
+ LSM_HOOK_inode_copy_up_xattr,
+ LSM_HOOK_file_permission,
+ LSM_HOOK_file_alloc_security,
+ LSM_HOOK_file_free_security,
+ LSM_HOOK_file_ioctl,
+ LSM_HOOK_mmap_addr,
+ LSM_HOOK_mmap_file,
+ LSM_HOOK_file_mprotect,
+ LSM_HOOK_file_lock,
+ LSM_HOOK_file_fcntl,
+ LSM_HOOK_file_set_fowner,
+ LSM_HOOK_file_send_sigiotask,
+ LSM_HOOK_file_receive,
+ LSM_HOOK_file_open,
+ LSM_HOOK_task_alloc,
+ LSM_HOOK_task_free,
+ LSM_HOOK_cred_alloc_blank,
+ LSM_HOOK_cred_free,
+ LSM_HOOK_cred_prepare,
+ LSM_HOOK_cred_transfer,
+ LSM_HOOK_kernel_act_as,
+ LSM_HOOK_kernel_create_files_as,
+ LSM_HOOK_kernel_read_file,
+ LSM_HOOK_kernel_post_read_file,
+ LSM_HOOK_kernel_module_request,
+ LSM_HOOK_task_fix_setuid,
+ LSM_HOOK_task_setpgid,
+ LSM_HOOK_task_getpgid,
+ LSM_HOOK_task_getsid,
+ LSM_HOOK_task_getsecid,
+ LSM_HOOK_task_setnice,
+ LSM_HOOK_task_setioprio,
+ LSM_HOOK_task_getioprio,
+ LSM_HOOK_task_prlimit,
+ LSM_HOOK_task_setrlimit,
+ LSM_HOOK_task_setscheduler,
+ LSM_HOOK_task_getscheduler,
+ LSM_HOOK_task_movememory,
+ LSM_HOOK_task_kill,
+ LSM_HOOK_task_prctl,
+ LSM_HOOK_task_to_inode,
+ LSM_HOOK_ipc_permission,
+ LSM_HOOK_ipc_getsecid,
+ LSM_HOOK_msg_msg_alloc_security,
+ LSM_HOOK_msg_msg_free_security,
+ LSM_HOOK_msg_queue_alloc_security,
+ LSM_HOOK_msg_queue_free_security,
+ LSM_HOOK_msg_queue_associate,
+ LSM_HOOK_msg_queue_msgctl,
+ LSM_HOOK_msg_queue_msgsnd,
+ LSM_HOOK_msg_queue_msgrcv,
+ LSM_HOOK_shm_alloc_security,
+ LSM_HOOK_shm_free_security,
+ LSM_HOOK_shm_associate,
+ LSM_HOOK_shm_shmctl,
+ LSM_HOOK_shm_shmat,
+ LSM_HOOK_sem_alloc_security,
+ LSM_HOOK_sem_free_security,
+ LSM_HOOK_sem_associate,
+ LSM_HOOK_sem_semctl,
+ LSM_HOOK_sem_semop,
+ LSM_HOOK_netlink_send,
+ LSM_HOOK_d_instantiate,
+ LSM_HOOK_getprocattr,
+ LSM_HOOK_setprocattr,
+ LSM_HOOK_ismaclabel,
+ LSM_HOOK_secid_to_secctx,
+ LSM_HOOK_secctx_to_secid,
+ LSM_HOOK_release_secctx,
+ LSM_HOOK_inode_invalidate_secctx,
+ LSM_HOOK_inode_notifysecctx,
+ LSM_HOOK_inode_setsecctx,
+ LSM_HOOK_inode_getsecctx,
#ifdef CONFIG_SECURITY_NETWORK
- struct list_head unix_stream_connect;
- struct list_head unix_may_send;
- struct list_head socket_create;
- struct list_head socket_post_create;
- struct list_head socket_bind;
- struct list_head socket_connect;
- struct list_head socket_listen;
- struct list_head socket_accept;
- struct list_head socket_sendmsg;
- struct list_head socket_recvmsg;
- struct list_head socket_getsockname;
- struct list_head socket_getpeername;
- struct list_head socket_getsockopt;
- struct list_head socket_setsockopt;
- struct list_head socket_shutdown;
- struct list_head socket_sock_rcv_skb;
- struct list_head socket_getpeersec_stream;
- struct list_head socket_getpeersec_dgram;
- struct list_head sk_alloc_security;
- struct list_head sk_free_security;
- struct list_head sk_clone_security;
- struct list_head sk_getsecid;
- struct list_head sock_graft;
- struct list_head inet_conn_request;
- struct list_head inet_csk_clone;
- struct list_head inet_conn_established;
- struct list_head secmark_relabel_packet;
- struct list_head secmark_refcount_inc;
- struct list_head secmark_refcount_dec;
- struct list_head req_classify_flow;
- struct list_head tun_dev_alloc_security;
- struct list_head tun_dev_free_security;
- struct list_head tun_dev_create;
- struct list_head tun_dev_attach_queue;
- struct list_head tun_dev_attach;
- struct list_head tun_dev_open;
+ LSM_HOOK_unix_stream_connect,
+ LSM_HOOK_unix_may_send,
+ LSM_HOOK_socket_create,
+ LSM_HOOK_socket_post_create,
+ LSM_HOOK_socket_bind,
+ LSM_HOOK_socket_connect,
+ LSM_HOOK_socket_listen,
+ LSM_HOOK_socket_accept,
+ LSM_HOOK_socket_sendmsg,
+ LSM_HOOK_socket_recvmsg,
+ LSM_HOOK_socket_getsockname,
+ LSM_HOOK_socket_getpeername,
+ LSM_HOOK_socket_getsockopt,
+ LSM_HOOK_socket_setsockopt,
+ LSM_HOOK_socket_shutdown,
+ LSM_HOOK_socket_sock_rcv_skb,
+ LSM_HOOK_socket_getpeersec_stream,
+ LSM_HOOK_socket_getpeersec_dgram,
+ LSM_HOOK_sk_alloc_security,
+ LSM_HOOK_sk_free_security,
+ LSM_HOOK_sk_clone_security,
+ LSM_HOOK_sk_getsecid,
+ LSM_HOOK_sock_graft,
+ LSM_HOOK_inet_conn_request,
+ LSM_HOOK_inet_csk_clone,
+ LSM_HOOK_inet_conn_established,
+ LSM_HOOK_secmark_relabel_packet,
+ LSM_HOOK_secmark_refcount_inc,
+ LSM_HOOK_secmark_refcount_dec,
+ LSM_HOOK_req_classify_flow,
+ LSM_HOOK_tun_dev_alloc_security,
+ LSM_HOOK_tun_dev_free_security,
+ LSM_HOOK_tun_dev_create,
+ LSM_HOOK_tun_dev_attach_queue,
+ LSM_HOOK_tun_dev_attach,
+ LSM_HOOK_tun_dev_open,
#endif /* CONFIG_SECURITY_NETWORK */
#ifdef CONFIG_SECURITY_INFINIBAND
- struct list_head ib_pkey_access;
- struct list_head ib_endport_manage_subnet;
- struct list_head ib_alloc_security;
- struct list_head ib_free_security;
+ LSM_HOOK_ib_pkey_access,
+ LSM_HOOK_ib_endport_manage_subnet,
+ LSM_HOOK_ib_alloc_security,
+ LSM_HOOK_ib_free_security,
#endif /* CONFIG_SECURITY_INFINIBAND */
#ifdef CONFIG_SECURITY_NETWORK_XFRM
- struct list_head xfrm_policy_alloc_security;
- struct list_head xfrm_policy_clone_security;
- struct list_head xfrm_policy_free_security;
- struct list_head xfrm_policy_delete_security;
- struct list_head xfrm_state_alloc;
- struct list_head xfrm_state_alloc_acquire;
- struct list_head xfrm_state_free_security;
- struct list_head xfrm_state_delete_security;
- struct list_head xfrm_policy_lookup;
- struct list_head xfrm_state_pol_flow_match;
- struct list_head xfrm_decode_session;
+ LSM_HOOK_xfrm_policy_alloc_security,
+ LSM_HOOK_xfrm_policy_clone_security,
+ LSM_HOOK_xfrm_policy_free_security,
+ LSM_HOOK_xfrm_policy_delete_security,
+ LSM_HOOK_xfrm_state_alloc,
+ LSM_HOOK_xfrm_state_alloc_acquire,
+ LSM_HOOK_xfrm_state_free_security,
+ LSM_HOOK_xfrm_state_delete_security,
+ LSM_HOOK_xfrm_policy_lookup,
+ LSM_HOOK_xfrm_state_pol_flow_match,
+ LSM_HOOK_xfrm_decode_session,
#endif /* CONFIG_SECURITY_NETWORK_XFRM */
#ifdef CONFIG_KEYS
- struct list_head key_alloc;
- struct list_head key_free;
- struct list_head key_permission;
- struct list_head key_getsecurity;
+ LSM_HOOK_key_alloc,
+ LSM_HOOK_key_free,
+ LSM_HOOK_key_permission,
+ LSM_HOOK_key_getsecurity,
#endif /* CONFIG_KEYS */
#ifdef CONFIG_AUDIT
- struct list_head audit_rule_init;
- struct list_head audit_rule_known;
- struct list_head audit_rule_match;
- struct list_head audit_rule_free;
+ LSM_HOOK_audit_rule_init,
+ LSM_HOOK_audit_rule_known,
+ LSM_HOOK_audit_rule_match,
+ LSM_HOOK_audit_rule_free,
#endif /* CONFIG_AUDIT */
#ifdef CONFIG_BPF_SYSCALL
- struct list_head bpf;
- struct list_head bpf_map;
- struct list_head bpf_prog;
- struct list_head bpf_map_alloc_security;
- struct list_head bpf_map_free_security;
- struct list_head bpf_prog_alloc_security;
- struct list_head bpf_prog_free_security;
+ LSM_HOOK_bpf,
+ LSM_HOOK_bpf_map,
+ LSM_HOOK_bpf_prog,
+ LSM_HOOK_bpf_map_alloc_security,
+ LSM_HOOK_bpf_map_free_security,
+ LSM_HOOK_bpf_prog_alloc_security,
+ LSM_HOOK_bpf_prog_free_security,
#endif /* CONFIG_BPF_SYSCALL */
-} __randomize_layout;
+ __MAX_LSM_HOOK,
+};
+#define HOOK_IDX(NAME) LSM_HOOK_##NAME
/*
* Security module hook list structure.
* For use with generic list macros for common operations.
*/
struct security_hook_list {
struct list_head list;
- struct list_head *head;
+ enum lsm_hook head_idx;
union security_list_options hook;
char *lsm;
} __randomize_layout;
@@ -1975,9 +1977,8 @@ struct security_hook_list {
* text involved.
*/
#define LSM_HOOK_INIT(HEAD, HOOK) \
- { .head = &security_hook_heads.HEAD, .hook = { .HEAD = HOOK } }
+ { .head_idx = HOOK_IDX(HEAD), .hook = { .HEAD = HOOK } }
-extern struct security_hook_heads security_hook_heads;
extern char *lsm_names;
extern void security_add_hooks(struct security_hook_list *hooks, int count,
diff --git a/security/security.c b/security/security.c
index 1cd8526cb0b7..b9fb297b824e 100644
--- a/security/security.c
+++ b/security/security.c
@@ -35,9 +35,11 @@
/* Maximum number of letters for an LSM name string */
#define SECURITY_NAME_MAX 10
-struct security_hook_heads security_hook_heads __lsm_ro_after_init;
+static struct list_head security_hook_heads[__MAX_LSM_HOOK] __lsm_ro_after_init;
static ATOMIC_NOTIFIER_HEAD(lsm_notifier_chain);
+#define HOOK_HEAD(NAME) (&security_hook_heads[HOOK_IDX(NAME)])
+
char *lsm_names;
/* Boot-time LSM user choice */
static __initdata char chosen_lsm[SECURITY_NAME_MAX + 1] =
@@ -61,11 +63,9 @@ static void __init do_security_initcalls(void)
int __init security_init(void)
{
int i;
- struct list_head *list = (struct list_head *) &security_hook_heads;
- for (i = 0; i < sizeof(security_hook_heads) / sizeof(struct list_head);
- i++)
- INIT_LIST_HEAD(&list[i]);
+ for (i = 0; i < ARRAY_SIZE(security_hook_heads); i++)
+ INIT_LIST_HEAD(&security_hook_heads[i]);
pr_info("Security Framework initialized\n");
/*
@@ -159,11 +159,14 @@ int __init security_module_enable(const char *module)
void __init security_add_hooks(struct security_hook_list *hooks, int count,
char *lsm)
{
+ enum lsm_hook hook_idx;
int i;
for (i = 0; i < count; i++) {
hooks[i].lsm = lsm;
- list_add_tail_rcu(&hooks[i].list, hooks[i].head);
+ hook_idx = hooks[i].head_idx;
+ list_add_tail_rcu(&hooks[i].list,
+ &security_hook_heads[hook_idx]);
}
if (lsm_append(lsm, &lsm_names) < 0)
panic("%s - Cannot get early memory.\n", __func__);
@@ -201,7 +204,7 @@ EXPORT_SYMBOL(unregister_lsm_notifier);
do { \
struct security_hook_list *P; \
\
- list_for_each_entry(P, &security_hook_heads.FUNC, list) \
+ list_for_each_entry(P, HOOK_HEAD(FUNC), list) \
P->hook.FUNC(__VA_ARGS__); \
} while (0)
@@ -210,7 +213,7 @@ EXPORT_SYMBOL(unregister_lsm_notifier);
do { \
struct security_hook_list *P; \
\
- list_for_each_entry(P, &security_hook_heads.FUNC, list) { \
+ list_for_each_entry(P, HOOK_HEAD(FUNC), list) { \
RC = P->hook.FUNC(__VA_ARGS__); \
if (RC != 0) \
break; \
@@ -317,7 +320,7 @@ int security_vm_enough_memory_mm(struct mm_struct *mm, long pages)
* agree that it should be set it will. If any module
* thinks it should not be set it won't.
*/
- list_for_each_entry(hp, &security_hook_heads.vm_enough_memory, list) {
+ list_for_each_entry(hp, HOOK_HEAD(vm_enough_memory), list) {
rc = hp->hook.vm_enough_memory(mm, pages);
if (rc <= 0) {
cap_sys_admin = 0;
@@ -805,7 +808,7 @@ int security_inode_getsecurity(struct inode *inode, const char *name, void **buf
/*
* Only one module will provide an attribute with a given name.
*/
- list_for_each_entry(hp, &security_hook_heads.inode_getsecurity, list) {
+ list_for_each_entry(hp, HOOK_HEAD(inode_getsecurity), list) {
rc = hp->hook.inode_getsecurity(inode, name, buffer, alloc);
if (rc != -EOPNOTSUPP)
return rc;
@@ -823,7 +826,7 @@ int security_inode_setsecurity(struct inode *inode, const char *name, const void
/*
* Only one module will provide an attribute with a given name.
*/
- list_for_each_entry(hp, &security_hook_heads.inode_setsecurity, list) {
+ list_for_each_entry(hp, HOOK_HEAD(inode_setsecurity), list) {
rc = hp->hook.inode_setsecurity(inode, name, value, size,
flags);
if (rc != -EOPNOTSUPP)
@@ -1126,7 +1129,7 @@ int security_task_prctl(int option, unsigned long arg2, unsigned long arg3,
int rc = -ENOSYS;
struct security_hook_list *hp;
- list_for_each_entry(hp, &security_hook_heads.task_prctl, list) {
+ list_for_each_entry(hp, HOOK_HEAD(task_prctl), list) {
thisrc = hp->hook.task_prctl(option, arg2, arg3, arg4, arg5);
if (thisrc != -ENOSYS) {
rc = thisrc;
@@ -1629,8 +1632,7 @@ int security_xfrm_state_pol_flow_match(struct xfrm_state *x,
* For speed optimization, we explicitly break the loop rather than
* using the macro
*/
- list_for_each_entry(hp, &security_hook_heads.xfrm_state_pol_flow_match,
- list) {
+ list_for_each_entry(hp, HOOK_HEAD(xfrm_state_pol_flow_match), list) {
rc = hp->hook.xfrm_state_pol_flow_match(x, xp, fl);
break;
}
--
2.14.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v4 2/3] security: Expose a mechanism to load lsm hooks dynamically at runtime
2018-03-07 7:22 [PATCH v4 0/3] Safe, dynamically loadable LSM hooks Sargun Dhillon
2018-03-07 7:23 ` [PATCH v4 1/3] security: Refactor LSM hooks into an array and enum Sargun Dhillon
@ 2018-03-07 7:23 ` Sargun Dhillon
2018-03-07 17:59 ` Casey Schaufler
2018-03-07 7:23 ` [PATCH v4 3/3] security: Add an example sample dynamic LSM Sargun Dhillon
2018-03-07 17:40 ` [PATCH v4 0/3] Safe, dynamically loadable LSM hooks Casey Schaufler
3 siblings, 1 reply; 11+ messages in thread
From: Sargun Dhillon @ 2018-03-07 7:23 UTC (permalink / raw)
To: linux-security-module, linux-kernel
Cc: penguin-kernel, keescook, igor.stoppa, casey
This patch adds dynamic security hooks. These hooks are designed to allow
for safe runtime loading.
These hooks are only run after all built-in, and major LSMs are run.
The LSMs enabled by this feature must be minor LSMs, but they can poke
at the security blobs, as the blobs should be initialized by the time
their callback happens.
There should be little runtime performance overhead for this feature,
as it's protected behind static_keys, which are disabled by default,
and are only enabled per-hook at runtime, when a module is loaded.
Currently, the hook heads are separated for dynamic hooks, because
it is not read-only like the hooks which are loaded at runtime.
Some hooks are blacklisted, and attempting to load an LSM with any
of them in use will fail.
Signed-off-by: Sargun Dhillon <sargun@sargun.me>
---
include/linux/lsm_hooks.h | 26 +++++-
security/Kconfig | 9 +++
security/inode.c | 13 ++-
security/security.c | 198 ++++++++++++++++++++++++++++++++++++++++++++--
4 files changed, 234 insertions(+), 12 deletions(-)
diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index d28c7f5b01c1..4e6351957dff 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -28,6 +28,7 @@
#include <linux/security.h>
#include <linux/init.h>
#include <linux/rculist.h>
+#include <linux/module.h>
/**
* union security_list_options - Linux Security Module hook function list
@@ -1968,6 +1969,9 @@ struct security_hook_list {
enum lsm_hook head_idx;
union security_list_options hook;
char *lsm;
+#ifdef CONFIG_SECURITY_DYNAMIC_HOOKS
+ struct module *owner;
+#endif
} __randomize_layout;
/*
@@ -1976,11 +1980,29 @@ struct security_hook_list {
* care of the common case and reduces the amount of
* text involved.
*/
+#ifdef CONFIG_SECURITY_DYNAMIC_HOOKS
+#define LSM_HOOK_INIT(HEAD, HOOK) \
+ { \
+ .head_idx = HOOK_IDX(HEAD), \
+ .hook = { .HEAD = HOOK }, \
+ .owner = THIS_MODULE, \
+ }
+
+#else
#define LSM_HOOK_INIT(HEAD, HOOK) \
{ .head_idx = HOOK_IDX(HEAD), .hook = { .HEAD = HOOK } }
+#endif
-extern char *lsm_names;
-
+#ifdef CONFIG_SECURITY_DYNAMIC_HOOKS
+extern int security_add_dynamic_hooks(struct security_hook_list *hooks,
+ int count, char *lsm);
+#else
+static inline int security_add_dynamic_hooks(struct security_hook_list *hooks,
+ int count, char *lsm)
+{
+ return -EOPNOTSUPP;
+}
+#endif
extern void security_add_hooks(struct security_hook_list *hooks, int count,
char *lsm);
diff --git a/security/Kconfig b/security/Kconfig
index c4302067a3ad..481b93b0d4d9 100644
--- a/security/Kconfig
+++ b/security/Kconfig
@@ -36,6 +36,15 @@ config SECURITY_WRITABLE_HOOKS
bool
default n
+config SECURITY_DYNAMIC_HOOKS
+ bool "Runtime loadable (minor) LSMs via LKMs"
+ depends on SECURITY && SRCU
+ help
+ This enables LSMs which live in LKMs, and supports loading, and
+ unloading them safely at runtime. These LSMs must be minor LSMs.
+ They cannot circumvent the built-in LSMs.
+ If you are unsure how to answer this question, answer N.
+
config SECURITYFS
bool "Enable the securityfs filesystem"
help
diff --git a/security/inode.c b/security/inode.c
index 8dd9ca8848e4..89be07b044a5 100644
--- a/security/inode.c
+++ b/security/inode.c
@@ -22,6 +22,10 @@
#include <linux/security.h>
#include <linux/lsm_hooks.h>
#include <linux/magic.h>
+#include <linux/mutex.h>
+
+extern char *lsm_names;
+extern struct mutex lsm_lock;
static struct vfsmount *mount;
static int mount_count;
@@ -312,8 +316,13 @@ static struct dentry *lsm_dentry;
static ssize_t lsm_read(struct file *filp, char __user *buf, size_t count,
loff_t *ppos)
{
- return simple_read_from_buffer(buf, count, ppos, lsm_names,
- strlen(lsm_names));
+ ssize_t ret;
+
+ mutex_lock(&lsm_lock);
+ ret = simple_read_from_buffer(buf, count, ppos, lsm_names,
+ strlen(lsm_names));
+ mutex_unlock(&lsm_lock);
+ return ret;
}
static const struct file_operations lsm_ops = {
diff --git a/security/security.c b/security/security.c
index b9fb297b824e..492d44dd0549 100644
--- a/security/security.c
+++ b/security/security.c
@@ -29,6 +29,7 @@
#include <linux/backing-dev.h>
#include <linux/string.h>
#include <net/flow.h>
+#include <linux/mutex.h>
#define MAX_LSM_EVM_XATTR 2
@@ -36,10 +37,18 @@
#define SECURITY_NAME_MAX 10
static struct list_head security_hook_heads[__MAX_LSM_HOOK] __lsm_ro_after_init;
-static ATOMIC_NOTIFIER_HEAD(lsm_notifier_chain);
-
#define HOOK_HEAD(NAME) (&security_hook_heads[HOOK_IDX(NAME)])
+#ifdef CONFIG_SECURITY_DYNAMIC_HOOKS
+static struct list_head dynamic_security_hook_heads[__MAX_LSM_HOOK];
+static struct srcu_struct dynamic_hook_srcus[__MAX_LSM_HOOK];
+#define DYNAMIC_HOOK_HEAD(NAME) (&dynamic_security_hook_heads[HOOK_IDX(NAME)])
+#define DYNAMIC_HOOK_SRCU(NAME) (&dynamic_hook_srcus[HOOK_IDX(NAME)])
+DEFINE_STATIC_KEY_ARRAY_FALSE(dynamic_hook_keys, __MAX_LSM_HOOK);
+#endif
+static ATOMIC_NOTIFIER_HEAD(lsm_notifier_chain);
+
+DEFINE_MUTEX(lsm_lock);
char *lsm_names;
/* Boot-time LSM user choice */
static __initdata char chosen_lsm[SECURITY_NAME_MAX + 1] =
@@ -55,6 +64,23 @@ static void __init do_security_initcalls(void)
}
}
+#ifdef CONFIG_SECURITY_DYNAMIC_HOOKS
+static void security_init_dynamic_hooks(void)
+{
+ int i, err;
+
+ for (i = 0; i < ARRAY_SIZE(dynamic_security_hook_heads); i++) {
+ INIT_LIST_HEAD(&dynamic_security_hook_heads[i]);
+ err = init_srcu_struct(&dynamic_hook_srcus[i]);
+ if (err)
+ panic("%s: Could not initialize SRCU - %d\n",
+ __func__, err);
+ }
+}
+#else
+static inline void security_init_dynamic_hooks(void) {};
+#endif
+
/**
* security_init - initializes the security framework
*
@@ -66,6 +92,7 @@ int __init security_init(void)
for (i = 0; i < ARRAY_SIZE(security_hook_heads); i++)
INIT_LIST_HEAD(&security_hook_heads[i]);
+ security_init_dynamic_hooks();
pr_info("Security Framework initialized\n");
/*
@@ -172,6 +199,64 @@ void __init security_add_hooks(struct security_hook_list *hooks, int count,
panic("%s - Cannot get early memory.\n", __func__);
}
+#ifdef CONFIG_SECURITY_DYNAMIC_HOOKS
+static int hook_allowed(enum lsm_hook hook_idx)
+{
+ switch (hook_idx) {
+ case HOOK_IDX(inode_getsecurity):
+ case HOOK_IDX(inode_setsecurity):
+ case HOOK_IDX(xfrm_state_pol_flow_match):
+ return -EPERM;
+ default:
+ return 0;
+ }
+}
+/**
+ * security_add_dynamic_hooks:
+ * Register a dynamically loadable module's security hooks.
+ *
+ * @hooks: the hooks to add
+ * @count: the number of hooks to add
+ * @lsm: the name of the security module
+ *
+ * Returns:
+ * 0 if successful
+ * else errno, and no hooks will be installed
+ */
+int security_add_dynamic_hooks(struct security_hook_list *hooks, int count,
+ char *lsm)
+{
+ enum lsm_hook hook_idx;
+ int ret = 0, i;
+
+ for (i = 0; i < count; i++) {
+ ret = hook_allowed(hooks[i].head_idx);
+ if (ret)
+ return ret;
+ }
+
+ mutex_lock(&lsm_lock);
+ ret = lsm_append(lsm, &lsm_names);
+ if (ret)
+ goto out;
+
+ for (i = 0; i < count; i++) {
+ WARN_ON(!try_module_get(hooks[i].owner));
+ hooks[i].lsm = lsm;
+ hook_idx = hooks[i].head_idx;
+ list_add_tail_rcu(&hooks[i].list,
+ &dynamic_security_hook_heads[hook_idx]);
+ static_branch_enable(&dynamic_hook_keys[hook_idx]);
+ }
+
+out:
+ mutex_unlock(&lsm_lock);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(security_add_dynamic_hooks);
+#endif
+
int call_lsm_notifier(enum lsm_event event, void *data)
{
return atomic_notifier_call_chain(&lsm_notifier_chain, event, data);
@@ -200,14 +285,69 @@ EXPORT_SYMBOL(unregister_lsm_notifier);
* This is a hook that returns a value.
*/
-#define call_void_hook(FUNC, ...) \
+#define call_void_hook_builtin(FUNC, ...) do { \
+ struct security_hook_list *P; \
+ list_for_each_entry(P, HOOK_HEAD(FUNC), list) \
+ P->hook.FUNC(__VA_ARGS__); \
+} while (0)
+
+#ifdef CONFIG_SECURITY_DYNAMIC_HOOKS
+#define IS_DYNAMIC_HOOK_ENABLED(FUNC) \
+ (static_branch_unlikely(&dynamic_hook_keys[HOOK_IDX(FUNC)]))
+
+#define call_void_hook_dynamic(FUNC, ...) ({ \
+ struct security_hook_list *P; \
+ int idx; \
+ \
+ idx = srcu_read_lock(DYNAMIC_HOOK_SRCU(FUNC)); \
+ list_for_each_entry_rcu(P, \
+ DYNAMIC_HOOK_HEAD(FUNC), \
+ list) { \
+ P->hook.FUNC(__VA_ARGS__); \
+ } \
+ srcu_read_unlock(DYNAMIC_HOOK_SRCU(FUNC), idx); \
+})
+
+#define call_void_hook(FUNC, ...) \
+ do { \
+ call_void_hook_builtin(FUNC, __VA_ARGS__); \
+ if (!IS_DYNAMIC_HOOK_ENABLED(FUNC)) \
+ break; \
+ call_void_hook_dynamic(FUNC, __VA_ARGS__); \
+ } while (0)
+
+#define call_int_hook(FUNC, IRC, ...) ({ \
+ bool continue_iteration = true; \
+ int RC = IRC, idx; \
do { \
struct security_hook_list *P; \
\
- list_for_each_entry(P, HOOK_HEAD(FUNC), list) \
- P->hook.FUNC(__VA_ARGS__); \
- } while (0)
+ list_for_each_entry(P, HOOK_HEAD(FUNC), list) { \
+ RC = P->hook.FUNC(__VA_ARGS__); \
+ if (RC != 0) { \
+ continue_iteration = false; \
+ break; \
+ } \
+ } \
+ if (!IS_DYNAMIC_HOOK_ENABLED(FUNC)) \
+ break; \
+ if (!continue_iteration) \
+ break; \
+ idx = srcu_read_lock(DYNAMIC_HOOK_SRCU(FUNC)); \
+ list_for_each_entry_rcu(P, \
+ DYNAMIC_HOOK_HEAD(FUNC), \
+ list) { \
+ RC = P->hook.FUNC(__VA_ARGS__); \
+ if (RC != 0) \
+ break; \
+ } \
+ srcu_read_unlock(DYNAMIC_HOOK_SRCU(FUNC), idx); \
+ } while (0); \
+ RC; \
+})
+#else
+#define call_void_hook call_void_hook_builtin
#define call_int_hook(FUNC, IRC, ...) ({ \
int RC = IRC; \
do { \
@@ -221,6 +361,7 @@ EXPORT_SYMBOL(unregister_lsm_notifier);
} while (0); \
RC; \
})
+#endif
/* Security operations */
@@ -312,6 +453,9 @@ int security_vm_enough_memory_mm(struct mm_struct *mm, long pages)
struct security_hook_list *hp;
int cap_sys_admin = 1;
int rc;
+#ifdef CONFIG_SECURITY_DYNAMIC_HOOKS
+ int idx;
+#endif
/*
* The module will respond with a positive value if
@@ -324,9 +468,25 @@ int security_vm_enough_memory_mm(struct mm_struct *mm, long pages)
rc = hp->hook.vm_enough_memory(mm, pages);
if (rc <= 0) {
cap_sys_admin = 0;
- break;
+ goto out;
}
}
+
+#ifdef CONFIG_SECURITY_DYNAMIC_HOOKS
+ if (!IS_DYNAMIC_HOOK_ENABLED(vm_enough_memory))
+ goto out;
+ idx = srcu_read_lock(DYNAMIC_HOOK_SRCU(vm_enough_memory));
+ list_for_each_entry_rcu(hp, DYNAMIC_HOOK_HEAD(vm_enough_memory),
+ list) {
+ rc = hp->hook.vm_enough_memory(mm, pages);
+ if (rc <= 0) {
+ cap_sys_admin = 0;
+ goto out;
+ }
+ }
+ srcu_read_unlock(DYNAMIC_HOOK_SRCU(vm_enough_memory), idx);
+#endif
+out:
return __vm_enough_memory(mm, pages, cap_sys_admin);
}
@@ -1128,15 +1288,36 @@ int security_task_prctl(int option, unsigned long arg2, unsigned long arg3,
int thisrc;
int rc = -ENOSYS;
struct security_hook_list *hp;
+#ifdef CONFIG_SECURITY_DYNAMIC_HOOKS
+ int idx;
+#endif
list_for_each_entry(hp, HOOK_HEAD(task_prctl), list) {
thisrc = hp->hook.task_prctl(option, arg2, arg3, arg4, arg5);
if (thisrc != -ENOSYS) {
rc = thisrc;
if (thisrc != 0)
- break;
+ goto out;
}
}
+
+#ifdef CONFIG_SECURITY_DYNAMIC_HOOKS
+ if (!IS_DYNAMIC_HOOK_ENABLED(task_prctl))
+ goto out;
+ idx = srcu_read_lock(DYNAMIC_HOOK_SRCU(task_prctl));
+ list_for_each_entry_rcu(hp, DYNAMIC_HOOK_HEAD(task_prctl),
+ list) {
+ thisrc = hp->hook.task_prctl(option, arg2, arg3, arg4, arg5);
+ if (thisrc != -ENOSYS) {
+ rc = thisrc;
+ if (thisrc != 0)
+ goto out_unlock;
+ }
+ }
+out_unlock:
+ srcu_read_unlock(DYNAMIC_HOOK_SRCU(task_prctl), idx);
+#endif
+out:
return rc;
}
@@ -1636,6 +1817,7 @@ int security_xfrm_state_pol_flow_match(struct xfrm_state *x,
rc = hp->hook.xfrm_state_pol_flow_match(x, xp, fl);
break;
}
+
return rc;
}
--
2.14.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v4 3/3] security: Add an example sample dynamic LSM
2018-03-07 7:22 [PATCH v4 0/3] Safe, dynamically loadable LSM hooks Sargun Dhillon
2018-03-07 7:23 ` [PATCH v4 1/3] security: Refactor LSM hooks into an array and enum Sargun Dhillon
2018-03-07 7:23 ` [PATCH v4 2/3] security: Expose a mechanism to load lsm hooks dynamically at runtime Sargun Dhillon
@ 2018-03-07 7:23 ` Sargun Dhillon
2018-03-07 17:40 ` [PATCH v4 0/3] Safe, dynamically loadable LSM hooks Casey Schaufler
3 siblings, 0 replies; 11+ messages in thread
From: Sargun Dhillon @ 2018-03-07 7:23 UTC (permalink / raw)
To: linux-security-module, linux-kernel
Cc: penguin-kernel, keescook, igor.stoppa, casey
This adds an example LSM that utilizes the features added by the
dynamically loadable LSMs patch. Once the module is unloaded, the
command is once again allowed. It prevents the user from running:
date --set="October 21 2015 16:29:00 PDT"
Signed-off-by: Sargun Dhillon <sargun@sargun.me>
---
samples/Kconfig | 6 ++++++
samples/Makefile | 2 +-
samples/lsm/Makefile | 4 ++++
samples/lsm/lsm_example.c | 33 +++++++++++++++++++++++++++++++++
4 files changed, 44 insertions(+), 1 deletion(-)
create mode 100644 samples/lsm/Makefile
create mode 100644 samples/lsm/lsm_example.c
diff --git a/samples/Kconfig b/samples/Kconfig
index c332a3b9de05..022242c0b50b 100644
--- a/samples/Kconfig
+++ b/samples/Kconfig
@@ -117,4 +117,10 @@ config SAMPLE_STATX
help
Build example userspace program to use the new extended-stat syscall.
+config SAMPLE_DYNAMIC_LSM
+ tristate "Build LSM examples -- loadable modules only"
+ depends on SECURITY_DYNAMIC_HOOKS && m
+ help
+ This builds an example dynamic LSM
+
endif # SAMPLES
diff --git a/samples/Makefile b/samples/Makefile
index db54e766ddb1..9d23835d6e6d 100644
--- a/samples/Makefile
+++ b/samples/Makefile
@@ -3,4 +3,4 @@
obj-$(CONFIG_SAMPLES) += kobject/ kprobes/ trace_events/ livepatch/ \
hw_breakpoint/ kfifo/ kdb/ hidraw/ rpmsg/ seccomp/ \
configfs/ connector/ v4l/ trace_printk/ blackfin/ \
- vfio-mdev/ statx/
+ vfio-mdev/ statx/ lsm/
diff --git a/samples/lsm/Makefile b/samples/lsm/Makefile
new file mode 100644
index 000000000000..d4ccb940f18b
--- /dev/null
+++ b/samples/lsm/Makefile
@@ -0,0 +1,4 @@
+# builds the loadable LSM example kernel modules;
+# then to use one (as root): insmod <module_name.ko>
+# and to unload: rmmod module_name
+obj-$(CONFIG_SAMPLE_DYNAMIC_LSM) += lsm_example.o
diff --git a/samples/lsm/lsm_example.c b/samples/lsm/lsm_example.c
new file mode 100644
index 000000000000..95c56ebd4d16
--- /dev/null
+++ b/samples/lsm/lsm_example.c
@@ -0,0 +1,33 @@
+/*
+ * This sample hooks into the "settime"
+ *
+ * Once you run it, the following will not be allowed:
+ * date --set="October 21 2015 16:29:00 PDT"
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/lsm_hooks.h>
+
+static int settime_cb(const struct timespec *ts, const struct timezone *tz)
+{
+ /* We aren't allowed to travel to October 21 2015 16:29 PDT */
+ if (ts->tv_sec >= 1445470140 && ts->tv_sec < 1445470200)
+ return -EPERM;
+
+ return 0;
+}
+
+static struct security_hook_list sample_hooks[] = {
+ LSM_HOOK_INIT(settime, settime_cb),
+};
+
+static int __init lsm_init(void)
+{
+ return security_add_dynamic_hooks(sample_hooks,
+ ARRAY_SIZE(sample_hooks),
+ "sample");
+}
+
+module_init(lsm_init)
+MODULE_LICENSE("GPL");
--
2.14.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v4 0/3] Safe, dynamically loadable LSM hooks
2018-03-07 7:22 [PATCH v4 0/3] Safe, dynamically loadable LSM hooks Sargun Dhillon
` (2 preceding siblings ...)
2018-03-07 7:23 ` [PATCH v4 3/3] security: Add an example sample dynamic LSM Sargun Dhillon
@ 2018-03-07 17:40 ` Casey Schaufler
3 siblings, 0 replies; 11+ messages in thread
From: Casey Schaufler @ 2018-03-07 17:40 UTC (permalink / raw)
To: Sargun Dhillon, linux-security-module, linux-kernel
Cc: penguin-kernel, keescook, igor.stoppa
On 3/6/2018 11:22 PM, Sargun Dhillon wrote:
> This patchset introduces safe dynamic LSM support. These are currently
> not unloadable, until we figure out a use case that needs that. Adding
> an unload hook is trivial given the way the patch is written.
If I recall correctly (it's been a decade since the discussion)
one of the reasons for not having loadable security modules was
that no one had a good idea on how they might be unloaded safely.
I'm not 100% convinced that there are no cases where you might
want to load a module that you can't unload, but I can definitely
come up with scenarios where it could be harmful.
> This exposes a second mechanism of loading hooks which are in modules.
> These hooks are behind static keys, so they should come at low performance
> overhead. The built-in hook heads are read-only, whereas the dynamic hooks
> are mutable.
I don't like having two mechanisms and I really don't like the
way you replace the structure of hooks with an array of pointers.
> Not all hooks can be loaded into. Some hooks are blacklisted, and therefore
> trying to load a module which plugs into those hooks will fail.
Which hooks, and why?
> One of the big benefits with loadable security modules is to help with
> "unknown unknowns". Although, livepatch is excellent, sometimes, a
> surgical LSM is simpler.
I could buy this if you could unload the module when the surgery
is done. As it is, you are stuck with whatever restrictions you
have introduced forever.
> It includes an example LSM that prevents specific time travel.
>
> Changes since v3:
> * readded hook blacklisted
> * return error, rather than panic if unable to allocate memory
>
> Changes since v2:
> * inode get/set security is readded
> * xfrm singleton hook readded
> * Security hooks are turned into an array
> * Security hooks and dynamic hooks enum is collapsed
>
> Changes since v1:
> * It no longer allows unloading of modules
> * prctl is fixed
> * inode get/set security is removed
> * xfrm singleton hook removed
>
>
> Sargun Dhillon (3):
> security: Refactor LSM hooks into an array and enum
> security: Expose a mechanism to load lsm hooks dynamically at runtime
> security: Add an example sample dynamic LSM
>
> include/linux/lsm_hooks.h | 459 ++++++++++++++++++++++++----------------------
> samples/Kconfig | 6 +
> samples/Makefile | 2 +-
> samples/lsm/Makefile | 4 +
> samples/lsm/lsm_example.c | 33 ++++
> security/Kconfig | 9 +
> security/inode.c | 13 +-
> security/security.c | 222 ++++++++++++++++++++--
> 8 files changed, 508 insertions(+), 240 deletions(-)
> create mode 100644 samples/lsm/Makefile
> create mode 100644 samples/lsm/lsm_example.c
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v4 1/3] security: Refactor LSM hooks into an array and enum
2018-03-07 7:23 ` [PATCH v4 1/3] security: Refactor LSM hooks into an array and enum Sargun Dhillon
@ 2018-03-07 17:45 ` Casey Schaufler
2018-03-07 19:18 ` Sargun Dhillon
0 siblings, 1 reply; 11+ messages in thread
From: Casey Schaufler @ 2018-03-07 17:45 UTC (permalink / raw)
To: Sargun Dhillon, linux-security-module, linux-kernel
Cc: penguin-kernel, keescook, igor.stoppa
On 3/6/2018 11:23 PM, Sargun Dhillon wrote:
> This commit should have no functional change. It changes the security hook
> list heads struct into an array. Additionally, it exposes all of the hooks
> via an enum. This loses memory layout randomization as the enum is not
> randomized.
Please explain why you want to do this. I still dislike it.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v4 2/3] security: Expose a mechanism to load lsm hooks dynamically at runtime
2018-03-07 7:23 ` [PATCH v4 2/3] security: Expose a mechanism to load lsm hooks dynamically at runtime Sargun Dhillon
@ 2018-03-07 17:59 ` Casey Schaufler
2018-03-07 20:29 ` Sargun Dhillon
0 siblings, 1 reply; 11+ messages in thread
From: Casey Schaufler @ 2018-03-07 17:59 UTC (permalink / raw)
To: Sargun Dhillon, linux-security-module, linux-kernel
Cc: penguin-kernel, keescook, igor.stoppa
On 3/6/2018 11:23 PM, Sargun Dhillon wrote:
> This patch adds dynamic security hooks. These hooks are designed to allow
> for safe runtime loading.
>
> These hooks are only run after all built-in, and major LSMs are run.
> The LSMs enabled by this feature must be minor LSMs, but they can poke
> at the security blobs, as the blobs should be initialized by the time
> their callback happens.
>
> There should be little runtime performance overhead for this feature,
> as it's protected behind static_keys, which are disabled by default,
> and are only enabled per-hook at runtime, when a module is loaded.
>
> Currently, the hook heads are separated for dynamic hooks, because
> it is not read-only like the hooks which are loaded at runtime.
>
> Some hooks are blacklisted, and attempting to load an LSM with any
> of them in use will fail.
>
> Signed-off-by: Sargun Dhillon <sargun@sargun.me>
> ---
> include/linux/lsm_hooks.h | 26 +++++-
> security/Kconfig | 9 +++
> security/inode.c | 13 ++-
> security/security.c | 198 ++++++++++++++++++++++++++++++++++++++++++++--
> 4 files changed, 234 insertions(+), 12 deletions(-)
>
> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> index d28c7f5b01c1..4e6351957dff 100644
> --- a/include/linux/lsm_hooks.h
> +++ b/include/linux/lsm_hooks.h
> @@ -28,6 +28,7 @@
> #include <linux/security.h>
> #include <linux/init.h>
> #include <linux/rculist.h>
> +#include <linux/module.h>
>
> /**
> * union security_list_options - Linux Security Module hook function list
> @@ -1968,6 +1969,9 @@ struct security_hook_list {
> enum lsm_hook head_idx;
> union security_list_options hook;
> char *lsm;
> +#ifdef CONFIG_SECURITY_DYNAMIC_HOOKS
> + struct module *owner;
> +#endif
> } __randomize_layout;
>
> /*
> @@ -1976,11 +1980,29 @@ struct security_hook_list {
> * care of the common case and reduces the amount of
> * text involved.
> */
> +#ifdef CONFIG_SECURITY_DYNAMIC_HOOKS
> +#define LSM_HOOK_INIT(HEAD, HOOK) \
> + { \
> + .head_idx = HOOK_IDX(HEAD), \
> + .hook = { .HEAD = HOOK }, \
> + .owner = THIS_MODULE, \
> + }
> +
> +#else
> #define LSM_HOOK_INIT(HEAD, HOOK) \
> { .head_idx = HOOK_IDX(HEAD), .hook = { .HEAD = HOOK } }
> +#endif
>
> -extern char *lsm_names;
> -
> +#ifdef CONFIG_SECURITY_DYNAMIC_HOOKS
> +extern int security_add_dynamic_hooks(struct security_hook_list *hooks,
> + int count, char *lsm);
> +#else
> +static inline int security_add_dynamic_hooks(struct security_hook_list *hooks,
> + int count, char *lsm)
> +{
> + return -EOPNOTSUPP;
> +}
> +#endif
> extern void security_add_hooks(struct security_hook_list *hooks, int count,
> char *lsm);
>
> diff --git a/security/Kconfig b/security/Kconfig
> index c4302067a3ad..481b93b0d4d9 100644
> --- a/security/Kconfig
> +++ b/security/Kconfig
> @@ -36,6 +36,15 @@ config SECURITY_WRITABLE_HOOKS
> bool
> default n
>
> +config SECURITY_DYNAMIC_HOOKS
> + bool "Runtime loadable (minor) LSMs via LKMs"
> + depends on SECURITY && SRCU
> + help
> + This enables LSMs which live in LKMs, and supports loading, and
> + unloading them safely at runtime. These LSMs must be minor LSMs.
> + They cannot circumvent the built-in LSMs.
> + If you are unsure how to answer this question, answer N.
> +
> config SECURITYFS
> bool "Enable the securityfs filesystem"
> help
> diff --git a/security/inode.c b/security/inode.c
> index 8dd9ca8848e4..89be07b044a5 100644
> --- a/security/inode.c
> +++ b/security/inode.c
> @@ -22,6 +22,10 @@
> #include <linux/security.h>
> #include <linux/lsm_hooks.h>
> #include <linux/magic.h>
> +#include <linux/mutex.h>
> +
> +extern char *lsm_names;
> +extern struct mutex lsm_lock;
>
> static struct vfsmount *mount;
> static int mount_count;
> @@ -312,8 +316,13 @@ static struct dentry *lsm_dentry;
> static ssize_t lsm_read(struct file *filp, char __user *buf, size_t count,
> loff_t *ppos)
> {
> - return simple_read_from_buffer(buf, count, ppos, lsm_names,
> - strlen(lsm_names));
> + ssize_t ret;
> +
> + mutex_lock(&lsm_lock);
> + ret = simple_read_from_buffer(buf, count, ppos, lsm_names,
> + strlen(lsm_names));
> + mutex_unlock(&lsm_lock);
> + return ret;
> }
>
> static const struct file_operations lsm_ops = {
> diff --git a/security/security.c b/security/security.c
> index b9fb297b824e..492d44dd0549 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -29,6 +29,7 @@
> #include <linux/backing-dev.h>
> #include <linux/string.h>
> #include <net/flow.h>
> +#include <linux/mutex.h>
>
> #define MAX_LSM_EVM_XATTR 2
>
> @@ -36,10 +37,18 @@
> #define SECURITY_NAME_MAX 10
I expect that this limit will be too small for loadable modules.
> static struct list_head security_hook_heads[__MAX_LSM_HOOK] __lsm_ro_after_init;
> -static ATOMIC_NOTIFIER_HEAD(lsm_notifier_chain);
> -
> #define HOOK_HEAD(NAME) (&security_hook_heads[HOOK_IDX(NAME)])
>
> +#ifdef CONFIG_SECURITY_DYNAMIC_HOOKS
> +static struct list_head dynamic_security_hook_heads[__MAX_LSM_HOOK];
> +static struct srcu_struct dynamic_hook_srcus[__MAX_LSM_HOOK];
> +#define DYNAMIC_HOOK_HEAD(NAME) (&dynamic_security_hook_heads[HOOK_IDX(NAME)])
> +#define DYNAMIC_HOOK_SRCU(NAME) (&dynamic_hook_srcus[HOOK_IDX(NAME)])
> +DEFINE_STATIC_KEY_ARRAY_FALSE(dynamic_hook_keys, __MAX_LSM_HOOK);
> +#endif
> +static ATOMIC_NOTIFIER_HEAD(lsm_notifier_chain);
> +
> +DEFINE_MUTEX(lsm_lock);
> char *lsm_names;
> /* Boot-time LSM user choice */
> static __initdata char chosen_lsm[SECURITY_NAME_MAX + 1] =
> @@ -55,6 +64,23 @@ static void __init do_security_initcalls(void)
> }
> }
>
> +#ifdef CONFIG_SECURITY_DYNAMIC_HOOKS
> +static void security_init_dynamic_hooks(void)
> +{
> + int i, err;
> +
> + for (i = 0; i < ARRAY_SIZE(dynamic_security_hook_heads); i++) {
> + INIT_LIST_HEAD(&dynamic_security_hook_heads[i]);
> + err = init_srcu_struct(&dynamic_hook_srcus[i]);
> + if (err)
> + panic("%s: Could not initialize SRCU - %d\n",
> + __func__, err);
> + }
> +}
> +#else
> +static inline void security_init_dynamic_hooks(void) {};
> +#endif
> +
> /**
> * security_init - initializes the security framework
> *
> @@ -66,6 +92,7 @@ int __init security_init(void)
>
> for (i = 0; i < ARRAY_SIZE(security_hook_heads); i++)
> INIT_LIST_HEAD(&security_hook_heads[i]);
> + security_init_dynamic_hooks();
> pr_info("Security Framework initialized\n");
>
> /*
> @@ -172,6 +199,64 @@ void __init security_add_hooks(struct security_hook_list *hooks, int count,
> panic("%s - Cannot get early memory.\n", __func__);
> }
>
> +#ifdef CONFIG_SECURITY_DYNAMIC_HOOKS
> +static int hook_allowed(enum lsm_hook hook_idx)
> +{
> + switch (hook_idx) {
> + case HOOK_IDX(inode_getsecurity):
> + case HOOK_IDX(inode_setsecurity):
> + case HOOK_IDX(xfrm_state_pol_flow_match):
> + return -EPERM;
> + default:
> + return 0;
> + }
> +}
Where did this list come from? Why doesn't it include
secid_to_secctx? Why does it include xfrm_stat_pol_flow_match?
> +/**
> + * security_add_dynamic_hooks:
> + * Register a dynamically loadable module's security hooks.
> + *
> + * @hooks: the hooks to add
> + * @count: the number of hooks to add
> + * @lsm: the name of the security module
> + *
> + * Returns:
> + * 0 if successful
> + * else errno, and no hooks will be installed
> + */
> +int security_add_dynamic_hooks(struct security_hook_list *hooks, int count,
> + char *lsm)
> +{
> + enum lsm_hook hook_idx;
> + int ret = 0, i;
> +
> + for (i = 0; i < count; i++) {
> + ret = hook_allowed(hooks[i].head_idx);
> + if (ret)
> + return ret;
> + }
> +
> + mutex_lock(&lsm_lock);
> + ret = lsm_append(lsm, &lsm_names);
> + if (ret)
> + goto out;
> +
> + for (i = 0; i < count; i++) {
> + WARN_ON(!try_module_get(hooks[i].owner));
> + hooks[i].lsm = lsm;
> + hook_idx = hooks[i].head_idx;
> + list_add_tail_rcu(&hooks[i].list,
> + &dynamic_security_hook_heads[hook_idx]);
> + static_branch_enable(&dynamic_hook_keys[hook_idx]);
> + }
> +
> +out:
> + mutex_unlock(&lsm_lock);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(security_add_dynamic_hooks);
> +#endif
> +
> int call_lsm_notifier(enum lsm_event event, void *data)
> {
> return atomic_notifier_call_chain(&lsm_notifier_chain, event, data);
> @@ -200,14 +285,69 @@ EXPORT_SYMBOL(unregister_lsm_notifier);
> * This is a hook that returns a value.
> */
>
> -#define call_void_hook(FUNC, ...) \
> +#define call_void_hook_builtin(FUNC, ...) do { \
> + struct security_hook_list *P; \
> + list_for_each_entry(P, HOOK_HEAD(FUNC), list) \
> + P->hook.FUNC(__VA_ARGS__); \
> +} while (0)
> +
> +#ifdef CONFIG_SECURITY_DYNAMIC_HOOKS
> +#define IS_DYNAMIC_HOOK_ENABLED(FUNC) \
> + (static_branch_unlikely(&dynamic_hook_keys[HOOK_IDX(FUNC)]))
> +
> +#define call_void_hook_dynamic(FUNC, ...) ({ \
> + struct security_hook_list *P; \
> + int idx; \
> + \
> + idx = srcu_read_lock(DYNAMIC_HOOK_SRCU(FUNC)); \
> + list_for_each_entry_rcu(P, \
> + DYNAMIC_HOOK_HEAD(FUNC), \
> + list) { \
> + P->hook.FUNC(__VA_ARGS__); \
> + } \
> + srcu_read_unlock(DYNAMIC_HOOK_SRCU(FUNC), idx); \
There has got to be a better way to do this than setting a lock
for every hook call.
> +})
> +
> +#define call_void_hook(FUNC, ...) \
> + do { \
> + call_void_hook_builtin(FUNC, __VA_ARGS__); \
> + if (!IS_DYNAMIC_HOOK_ENABLED(FUNC)) \
> + break; \
Why not let the list macros do their job?
> + call_void_hook_dynamic(FUNC, __VA_ARGS__); \
> + } while (0)
> +
> +#define call_int_hook(FUNC, IRC, ...) ({ \
> + bool continue_iteration = true; \
> + int RC = IRC, idx; \
> do { \
> struct security_hook_list *P; \
> \
> - list_for_each_entry(P, HOOK_HEAD(FUNC), list) \
> - P->hook.FUNC(__VA_ARGS__); \
> - } while (0)
> + list_for_each_entry(P, HOOK_HEAD(FUNC), list) { \
> + RC = P->hook.FUNC(__VA_ARGS__); \
> + if (RC != 0) { \
> + continue_iteration = false; \
> + break; \
> + } \
> + } \
> + if (!IS_DYNAMIC_HOOK_ENABLED(FUNC)) \
> + break; \
> + if (!continue_iteration) \
> + break; \
> + idx = srcu_read_lock(DYNAMIC_HOOK_SRCU(FUNC)); \
> + list_for_each_entry_rcu(P, \
> + DYNAMIC_HOOK_HEAD(FUNC), \
> + list) { \
> + RC = P->hook.FUNC(__VA_ARGS__); \
> + if (RC != 0) \
> + break; \
> + } \
> + srcu_read_unlock(DYNAMIC_HOOK_SRCU(FUNC), idx); \
> + } while (0); \
> + RC; \
> +})
>
> +#else
> +#define call_void_hook call_void_hook_builtin
I think this is hideous.
> #define call_int_hook(FUNC, IRC, ...) ({ \
> int RC = IRC; \
> do { \
> @@ -221,6 +361,7 @@ EXPORT_SYMBOL(unregister_lsm_notifier);
> } while (0); \
> RC; \
> })
> +#endif
>
> /* Security operations */
>
> @@ -312,6 +453,9 @@ int security_vm_enough_memory_mm(struct mm_struct *mm, long pages)
> struct security_hook_list *hp;
> int cap_sys_admin = 1;
> int rc;
> +#ifdef CONFIG_SECURITY_DYNAMIC_HOOKS
> + int idx;
> +#endif
>
> /*
> * The module will respond with a positive value if
> @@ -324,9 +468,25 @@ int security_vm_enough_memory_mm(struct mm_struct *mm, long pages)
> rc = hp->hook.vm_enough_memory(mm, pages);
> if (rc <= 0) {
> cap_sys_admin = 0;
> - break;
> + goto out;
> }
> }
> +
> +#ifdef CONFIG_SECURITY_DYNAMIC_HOOKS
> + if (!IS_DYNAMIC_HOOK_ENABLED(vm_enough_memory))
> + goto out;
> + idx = srcu_read_lock(DYNAMIC_HOOK_SRCU(vm_enough_memory));
> + list_for_each_entry_rcu(hp, DYNAMIC_HOOK_HEAD(vm_enough_memory),
> + list) {
> + rc = hp->hook.vm_enough_memory(mm, pages);
> + if (rc <= 0) {
> + cap_sys_admin = 0;
> + goto out;
> + }
> + }
> + srcu_read_unlock(DYNAMIC_HOOK_SRCU(vm_enough_memory), idx);
> +#endif
> +out:
> return __vm_enough_memory(mm, pages, cap_sys_admin);
> }
>
> @@ -1128,15 +1288,36 @@ int security_task_prctl(int option, unsigned long arg2, unsigned long arg3,
> int thisrc;
> int rc = -ENOSYS;
> struct security_hook_list *hp;
> +#ifdef CONFIG_SECURITY_DYNAMIC_HOOKS
> + int idx;
> +#endif
>
> list_for_each_entry(hp, HOOK_HEAD(task_prctl), list) {
> thisrc = hp->hook.task_prctl(option, arg2, arg3, arg4, arg5);
> if (thisrc != -ENOSYS) {
> rc = thisrc;
> if (thisrc != 0)
> - break;
> + goto out;
> }
> }
> +
> +#ifdef CONFIG_SECURITY_DYNAMIC_HOOKS
> + if (!IS_DYNAMIC_HOOK_ENABLED(task_prctl))
> + goto out;
> + idx = srcu_read_lock(DYNAMIC_HOOK_SRCU(task_prctl));
> + list_for_each_entry_rcu(hp, DYNAMIC_HOOK_HEAD(task_prctl),
> + list) {
> + thisrc = hp->hook.task_prctl(option, arg2, arg3, arg4, arg5);
> + if (thisrc != -ENOSYS) {
> + rc = thisrc;
> + if (thisrc != 0)
> + goto out_unlock;
> + }
> + }
> +out_unlock:
> + srcu_read_unlock(DYNAMIC_HOOK_SRCU(task_prctl), idx);
> +#endif
> +out:
> return rc;
> }
>
> @@ -1636,6 +1817,7 @@ int security_xfrm_state_pol_flow_match(struct xfrm_state *x,
> rc = hp->hook.xfrm_state_pol_flow_match(x, xp, fl);
> break;
> }
> +
> return rc;
> }
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v4 1/3] security: Refactor LSM hooks into an array and enum
2018-03-07 17:45 ` Casey Schaufler
@ 2018-03-07 19:18 ` Sargun Dhillon
2018-03-07 20:23 ` Casey Schaufler
0 siblings, 1 reply; 11+ messages in thread
From: Sargun Dhillon @ 2018-03-07 19:18 UTC (permalink / raw)
To: Casey Schaufler; +Cc: LSM, LKML, Tetsuo Handa, Kees Cook, Igor Stoppa
On Wed, Mar 7, 2018 at 9:45 AM, Casey Schaufler <casey@schaufler-ca.com> wrote:
> On 3/6/2018 11:23 PM, Sargun Dhillon wrote:
>> This commit should have no functional change. It changes the security hook
>> list heads struct into an array. Additionally, it exposes all of the hooks
>> via an enum. This loses memory layout randomization as the enum is not
>> randomized.
>
> Please explain why you want to do this. I still dislike it.
>
Do you dislike it because of the loss of randomization, or some other reason?
The reason for not just having a second list_heads is that it's
somewhat ugly having to replicate that structure twice -- once for
dynamic hooks, and once for 'static' hooks.
Instead, we have one enum that LSMs can use and two arrays of heads
rather than an entire unrolled set of list_heads.
If we had a way to randomize this, would it make you comfortable?
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v4 1/3] security: Refactor LSM hooks into an array and enum
2018-03-07 19:18 ` Sargun Dhillon
@ 2018-03-07 20:23 ` Casey Schaufler
2018-03-07 22:22 ` Sargun Dhillon
0 siblings, 1 reply; 11+ messages in thread
From: Casey Schaufler @ 2018-03-07 20:23 UTC (permalink / raw)
To: Sargun Dhillon; +Cc: LSM, LKML, Tetsuo Handa, Kees Cook, Igor Stoppa
On 3/7/2018 11:18 AM, Sargun Dhillon wrote:
> On Wed, Mar 7, 2018 at 9:45 AM, Casey Schaufler <casey@schaufler-ca.com> wrote:
>> On 3/6/2018 11:23 PM, Sargun Dhillon wrote:
>>> This commit should have no functional change. It changes the security hook
>>> list heads struct into an array. Additionally, it exposes all of the hooks
>>> via an enum. This loses memory layout randomization as the enum is not
>>> randomized.
>> Please explain why you want to do this. I still dislike it.
>>
> Do you dislike it because of the loss of randomization, or some other reason?
I dislike a huge array of untyped function pointers.
I dislike the loss of type checking in security.c
> The reason for not just having a second list_heads is that it's
> somewhat ugly having to replicate that structure twice -- once for
> dynamic hooks, and once for 'static' hooks.
There was discussion about this some time ago. In the case
where you don't allow dynamic hooks, you mark the lists ro_after_init
whereas in the case with them you don't, but use the locking.
> Instead, we have one enum that LSMs can use and two arrays of heads
> rather than an entire unrolled set of list_heads.
But how is this better? What is the advantage?
>
> If we had a way to randomize this, would it make you comfortable?
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v4 2/3] security: Expose a mechanism to load lsm hooks dynamically at runtime
2018-03-07 17:59 ` Casey Schaufler
@ 2018-03-07 20:29 ` Sargun Dhillon
0 siblings, 0 replies; 11+ messages in thread
From: Sargun Dhillon @ 2018-03-07 20:29 UTC (permalink / raw)
To: Casey Schaufler; +Cc: LSM, LKML, Tetsuo Handa, Kees Cook, Igor Stoppa
On Wed, Mar 7, 2018 at 9:59 AM, Casey Schaufler <casey@schaufler-ca.com> wrote:
> On 3/6/2018 11:23 PM, Sargun Dhillon wrote:
>> This patch adds dynamic security hooks. These hooks are designed to allow
>> for safe runtime loading.
>>
>> These hooks are only run after all built-in, and major LSMs are run.
>> The LSMs enabled by this feature must be minor LSMs, but they can poke
>> at the security blobs, as the blobs should be initialized by the time
>> their callback happens.
>>
>> There should be little runtime performance overhead for this feature,
>> as it's protected behind static_keys, which are disabled by default,
>> and are only enabled per-hook at runtime, when a module is loaded.
>>
>> Currently, the hook heads are separated for dynamic hooks, because
>> it is not read-only like the hooks which are loaded at runtime.
>>
>> Some hooks are blacklisted, and attempting to load an LSM with any
>> of them in use will fail.
>>
>> Signed-off-by: Sargun Dhillon <sargun@sargun.me>
>> ---
>> include/linux/lsm_hooks.h | 26 +++++-
>> security/Kconfig | 9 +++
>> security/inode.c | 13 ++-
>> security/security.c | 198 ++++++++++++++++++++++++++++++++++++++++++++--
>> 4 files changed, 234 insertions(+), 12 deletions(-)
>>
>> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
>> index d28c7f5b01c1..4e6351957dff 100644
>> --- a/include/linux/lsm_hooks.h
>> +++ b/include/linux/lsm_hooks.h
>> @@ -28,6 +28,7 @@
>> #include <linux/security.h>
>> #include <linux/init.h>
>> #include <linux/rculist.h>
>> +#include <linux/module.h>
>>
>> /**
>> * union security_list_options - Linux Security Module hook function list
>> @@ -1968,6 +1969,9 @@ struct security_hook_list {
>> enum lsm_hook head_idx;
>> union security_list_options hook;
>> char *lsm;
>> +#ifdef CONFIG_SECURITY_DYNAMIC_HOOKS
>> + struct module *owner;
>> +#endif
>> } __randomize_layout;
>>
>> /*
>> @@ -1976,11 +1980,29 @@ struct security_hook_list {
>> * care of the common case and reduces the amount of
>> * text involved.
>> */
>> +#ifdef CONFIG_SECURITY_DYNAMIC_HOOKS
>> +#define LSM_HOOK_INIT(HEAD, HOOK) \
>> + { \
>> + .head_idx = HOOK_IDX(HEAD), \
>> + .hook = { .HEAD = HOOK }, \
>> + .owner = THIS_MODULE, \
>> + }
>> +
>> +#else
>> #define LSM_HOOK_INIT(HEAD, HOOK) \
>> { .head_idx = HOOK_IDX(HEAD), .hook = { .HEAD = HOOK } }
>> +#endif
>>
>> -extern char *lsm_names;
>> -
>> +#ifdef CONFIG_SECURITY_DYNAMIC_HOOKS
>> +extern int security_add_dynamic_hooks(struct security_hook_list *hooks,
>> + int count, char *lsm);
>> +#else
>> +static inline int security_add_dynamic_hooks(struct security_hook_list *hooks,
>> + int count, char *lsm)
>> +{
>> + return -EOPNOTSUPP;
>> +}
>> +#endif
>> extern void security_add_hooks(struct security_hook_list *hooks, int count,
>> char *lsm);
>>
>> diff --git a/security/Kconfig b/security/Kconfig
>> index c4302067a3ad..481b93b0d4d9 100644
>> --- a/security/Kconfig
>> +++ b/security/Kconfig
>> @@ -36,6 +36,15 @@ config SECURITY_WRITABLE_HOOKS
>> bool
>> default n
>>
>> +config SECURITY_DYNAMIC_HOOKS
>> + bool "Runtime loadable (minor) LSMs via LKMs"
>> + depends on SECURITY && SRCU
>> + help
>> + This enables LSMs which live in LKMs, and supports loading, and
>> + unloading them safely at runtime. These LSMs must be minor LSMs.
>> + They cannot circumvent the built-in LSMs.
>> + If you are unsure how to answer this question, answer N.
>> +
>> config SECURITYFS
>> bool "Enable the securityfs filesystem"
>> help
>> diff --git a/security/inode.c b/security/inode.c
>> index 8dd9ca8848e4..89be07b044a5 100644
>> --- a/security/inode.c
>> +++ b/security/inode.c
>> @@ -22,6 +22,10 @@
>> #include <linux/security.h>
>> #include <linux/lsm_hooks.h>
>> #include <linux/magic.h>
>> +#include <linux/mutex.h>
>> +
>> +extern char *lsm_names;
>> +extern struct mutex lsm_lock;
>>
>> static struct vfsmount *mount;
>> static int mount_count;
>> @@ -312,8 +316,13 @@ static struct dentry *lsm_dentry;
>> static ssize_t lsm_read(struct file *filp, char __user *buf, size_t count,
>> loff_t *ppos)
>> {
>> - return simple_read_from_buffer(buf, count, ppos, lsm_names,
>> - strlen(lsm_names));
>> + ssize_t ret;
>> +
>> + mutex_lock(&lsm_lock);
>> + ret = simple_read_from_buffer(buf, count, ppos, lsm_names,
>> + strlen(lsm_names));
>> + mutex_unlock(&lsm_lock);
>> + return ret;
>> }
>>
>> static const struct file_operations lsm_ops = {
>> diff --git a/security/security.c b/security/security.c
>> index b9fb297b824e..492d44dd0549 100644
>> --- a/security/security.c
>> +++ b/security/security.c
>> @@ -29,6 +29,7 @@
>> #include <linux/backing-dev.h>
>> #include <linux/string.h>
>> #include <net/flow.h>
>> +#include <linux/mutex.h>
>>
>> #define MAX_LSM_EVM_XATTR 2
>>
>> @@ -36,10 +37,18 @@
>> #define SECURITY_NAME_MAX 10
>
> I expect that this limit will be too small for loadable modules.
>
We can change this. Any suggestion?
>> static struct list_head security_hook_heads[__MAX_LSM_HOOK] __lsm_ro_after_init;
>> -static ATOMIC_NOTIFIER_HEAD(lsm_notifier_chain);
>> -
>> #define HOOK_HEAD(NAME) (&security_hook_heads[HOOK_IDX(NAME)])
>>
>> +#ifdef CONFIG_SECURITY_DYNAMIC_HOOKS
>> +static struct list_head dynamic_security_hook_heads[__MAX_LSM_HOOK];
>> +static struct srcu_struct dynamic_hook_srcus[__MAX_LSM_HOOK];
>> +#define DYNAMIC_HOOK_HEAD(NAME) (&dynamic_security_hook_heads[HOOK_IDX(NAME)])
>> +#define DYNAMIC_HOOK_SRCU(NAME) (&dynamic_hook_srcus[HOOK_IDX(NAME)])
>> +DEFINE_STATIC_KEY_ARRAY_FALSE(dynamic_hook_keys, __MAX_LSM_HOOK);
>> +#endif
>> +static ATOMIC_NOTIFIER_HEAD(lsm_notifier_chain);
>> +
>> +DEFINE_MUTEX(lsm_lock);
>> char *lsm_names;
>> /* Boot-time LSM user choice */
>> static __initdata char chosen_lsm[SECURITY_NAME_MAX + 1] =
>> @@ -55,6 +64,23 @@ static void __init do_security_initcalls(void)
>> }
>> }
>>
>> +#ifdef CONFIG_SECURITY_DYNAMIC_HOOKS
>> +static void security_init_dynamic_hooks(void)
>> +{
>> + int i, err;
>> +
>> + for (i = 0; i < ARRAY_SIZE(dynamic_security_hook_heads); i++) {
>> + INIT_LIST_HEAD(&dynamic_security_hook_heads[i]);
>> + err = init_srcu_struct(&dynamic_hook_srcus[i]);
>> + if (err)
>> + panic("%s: Could not initialize SRCU - %d\n",
>> + __func__, err);
>> + }
>> +}
>> +#else
>> +static inline void security_init_dynamic_hooks(void) {};
>> +#endif
>> +
>> /**
>> * security_init - initializes the security framework
>> *
>> @@ -66,6 +92,7 @@ int __init security_init(void)
>>
>> for (i = 0; i < ARRAY_SIZE(security_hook_heads); i++)
>> INIT_LIST_HEAD(&security_hook_heads[i]);
>> + security_init_dynamic_hooks();
>> pr_info("Security Framework initialized\n");
>>
>> /*
>> @@ -172,6 +199,64 @@ void __init security_add_hooks(struct security_hook_list *hooks, int count,
>> panic("%s - Cannot get early memory.\n", __func__);
>> }
>>
>> +#ifdef CONFIG_SECURITY_DYNAMIC_HOOKS
>> +static int hook_allowed(enum lsm_hook hook_idx)
>> +{
>> + switch (hook_idx) {
>> + case HOOK_IDX(inode_getsecurity):
>> + case HOOK_IDX(inode_setsecurity):
>> + case HOOK_IDX(xfrm_state_pol_flow_match):
>> + return -EPERM;
>> + default:
>> + return 0;
>> + }
>> +}
>
> Where did this list come from? Why doesn't it include
> secid_to_secctx? Why does it include xfrm_stat_pol_flow_match?
>
Because xfrm_stat_pol_flow_match is very perf-sensitive, and it's not
designed to work with multiple LSMs (as the comments say), similarly
with inode_getsecurity / inode_setsecurity. We can block other hooks,
but these were the ones in the code that had explicit comments.
>> +/**
>> + * security_add_dynamic_hooks:
>> + * Register a dynamically loadable module's security hooks.
>> + *
>> + * @hooks: the hooks to add
>> + * @count: the number of hooks to add
>> + * @lsm: the name of the security module
>> + *
>> + * Returns:
>> + * 0 if successful
>> + * else errno, and no hooks will be installed
>> + */
>> +int security_add_dynamic_hooks(struct security_hook_list *hooks, int count,
>> + char *lsm)
>> +{
>> + enum lsm_hook hook_idx;
>> + int ret = 0, i;
>> +
>> + for (i = 0; i < count; i++) {
>> + ret = hook_allowed(hooks[i].head_idx);
>> + if (ret)
>> + return ret;
>> + }
>> +
>> + mutex_lock(&lsm_lock);
>> + ret = lsm_append(lsm, &lsm_names);
>> + if (ret)
>> + goto out;
>> +
>> + for (i = 0; i < count; i++) {
>> + WARN_ON(!try_module_get(hooks[i].owner));
>> + hooks[i].lsm = lsm;
>> + hook_idx = hooks[i].head_idx;
>> + list_add_tail_rcu(&hooks[i].list,
>> + &dynamic_security_hook_heads[hook_idx]);
>> + static_branch_enable(&dynamic_hook_keys[hook_idx]);
>> + }
>> +
>> +out:
>> + mutex_unlock(&lsm_lock);
>> +
>> + return ret;
>> +}
>> +EXPORT_SYMBOL_GPL(security_add_dynamic_hooks);
>> +#endif
>> +
>> int call_lsm_notifier(enum lsm_event event, void *data)
>> {
>> return atomic_notifier_call_chain(&lsm_notifier_chain, event, data);
>> @@ -200,14 +285,69 @@ EXPORT_SYMBOL(unregister_lsm_notifier);
>> * This is a hook that returns a value.
>> */
>>
>> -#define call_void_hook(FUNC, ...) \
>> +#define call_void_hook_builtin(FUNC, ...) do { \
>> + struct security_hook_list *P; \
>> + list_for_each_entry(P, HOOK_HEAD(FUNC), list) \
>> + P->hook.FUNC(__VA_ARGS__); \
>> +} while (0)
>> +
>> +#ifdef CONFIG_SECURITY_DYNAMIC_HOOKS
>> +#define IS_DYNAMIC_HOOK_ENABLED(FUNC) \
>> + (static_branch_unlikely(&dynamic_hook_keys[HOOK_IDX(FUNC)]))
>> +
>> +#define call_void_hook_dynamic(FUNC, ...) ({ \
>> + struct security_hook_list *P; \
>> + int idx; \
>> + \
>> + idx = srcu_read_lock(DYNAMIC_HOOK_SRCU(FUNC)); \
>> + list_for_each_entry_rcu(P, \
>> + DYNAMIC_HOOK_HEAD(FUNC), \
>> + list) { \
>> + P->hook.FUNC(__VA_ARGS__); \
>> + } \
>> + srcu_read_unlock(DYNAMIC_HOOK_SRCU(FUNC), idx); \
>
> There has got to be a better way to do this than setting a lock
> for every hook call.
>
This isn't a "lock" -- it's rcu, so it should just do a ptr
dereference and per cpu counter inc / dec.
>> +})
>> +
>> +#define call_void_hook(FUNC, ...) \
>> + do { \
>> + call_void_hook_builtin(FUNC, __VA_ARGS__); \
>> + if (!IS_DYNAMIC_HOOK_ENABLED(FUNC)) \
>> + break; \
>
> Why not let the list macros do their job?
>
What do you mean? In order to avoid any perf overhead, this uses
static keys -- we have to check those.
>> + call_void_hook_dynamic(FUNC, __VA_ARGS__); \
>> + } while (0)
>> +
>> +#define call_int_hook(FUNC, IRC, ...) ({ \
>> + bool continue_iteration = true; \
>> + int RC = IRC, idx; \
>> do { \
>> struct security_hook_list *P; \
>> \
>> - list_for_each_entry(P, HOOK_HEAD(FUNC), list) \
>> - P->hook.FUNC(__VA_ARGS__); \
>> - } while (0)
>> + list_for_each_entry(P, HOOK_HEAD(FUNC), list) { \
>> + RC = P->hook.FUNC(__VA_ARGS__); \
>> + if (RC != 0) { \
>> + continue_iteration = false; \
>> + break; \
>> + } \
>> + } \
>> + if (!IS_DYNAMIC_HOOK_ENABLED(FUNC)) \
>> + break; \
>> + if (!continue_iteration) \
>> + break; \
>> + idx = srcu_read_lock(DYNAMIC_HOOK_SRCU(FUNC)); \
>> + list_for_each_entry_rcu(P, \
>> + DYNAMIC_HOOK_HEAD(FUNC), \
>> + list) { \
>> + RC = P->hook.FUNC(__VA_ARGS__); \
>> + if (RC != 0) \
>> + break; \
>> + } \
>> + srcu_read_unlock(DYNAMIC_HOOK_SRCU(FUNC), idx); \
>> + } while (0); \
>> + RC; \
>> +})
>>
>> +#else
>> +#define call_void_hook call_void_hook_builtin
>
> I think this is hideous.
>
I can split this up.
>> #define call_int_hook(FUNC, IRC, ...) ({ \
>> int RC = IRC; \
>> do { \
>> @@ -221,6 +361,7 @@ EXPORT_SYMBOL(unregister_lsm_notifier);
>> } while (0); \
>> RC; \
>> })
>> +#endif
>>
>> /* Security operations */
>>
>> @@ -312,6 +453,9 @@ int security_vm_enough_memory_mm(struct mm_struct *mm, long pages)
>> struct security_hook_list *hp;
>> int cap_sys_admin = 1;
>> int rc;
>> +#ifdef CONFIG_SECURITY_DYNAMIC_HOOKS
>> + int idx;
>> +#endif
>>
>> /*
>> * The module will respond with a positive value if
>> @@ -324,9 +468,25 @@ int security_vm_enough_memory_mm(struct mm_struct *mm, long pages)
>> rc = hp->hook.vm_enough_memory(mm, pages);
>> if (rc <= 0) {
>> cap_sys_admin = 0;
>> - break;
>> + goto out;
>> }
>> }
>> +
>> +#ifdef CONFIG_SECURITY_DYNAMIC_HOOKS
>> + if (!IS_DYNAMIC_HOOK_ENABLED(vm_enough_memory))
>> + goto out;
>> + idx = srcu_read_lock(DYNAMIC_HOOK_SRCU(vm_enough_memory));
>> + list_for_each_entry_rcu(hp, DYNAMIC_HOOK_HEAD(vm_enough_memory),
>> + list) {
>> + rc = hp->hook.vm_enough_memory(mm, pages);
>> + if (rc <= 0) {
>> + cap_sys_admin = 0;
>> + goto out;
>> + }
>> + }
>> + srcu_read_unlock(DYNAMIC_HOOK_SRCU(vm_enough_memory), idx);
>> +#endif
>> +out:
>> return __vm_enough_memory(mm, pages, cap_sys_admin);
>> }
>>
>> @@ -1128,15 +1288,36 @@ int security_task_prctl(int option, unsigned long arg2, unsigned long arg3,
>> int thisrc;
>> int rc = -ENOSYS;
>> struct security_hook_list *hp;
>> +#ifdef CONFIG_SECURITY_DYNAMIC_HOOKS
>> + int idx;
>> +#endif
>>
>> list_for_each_entry(hp, HOOK_HEAD(task_prctl), list) {
>> thisrc = hp->hook.task_prctl(option, arg2, arg3, arg4, arg5);
>> if (thisrc != -ENOSYS) {
>> rc = thisrc;
>> if (thisrc != 0)
>> - break;
>> + goto out;
>> }
>> }
>> +
>> +#ifdef CONFIG_SECURITY_DYNAMIC_HOOKS
>> + if (!IS_DYNAMIC_HOOK_ENABLED(task_prctl))
>> + goto out;
>> + idx = srcu_read_lock(DYNAMIC_HOOK_SRCU(task_prctl));
>> + list_for_each_entry_rcu(hp, DYNAMIC_HOOK_HEAD(task_prctl),
>> + list) {
>> + thisrc = hp->hook.task_prctl(option, arg2, arg3, arg4, arg5);
>> + if (thisrc != -ENOSYS) {
>> + rc = thisrc;
>> + if (thisrc != 0)
>> + goto out_unlock;
>> + }
>> + }
>> +out_unlock:
>> + srcu_read_unlock(DYNAMIC_HOOK_SRCU(task_prctl), idx);
>> +#endif
>> +out:
>> return rc;
>> }
>>
>> @@ -1636,6 +1817,7 @@ int security_xfrm_state_pol_flow_match(struct xfrm_state *x,
>> rc = hp->hook.xfrm_state_pol_flow_match(x, xp, fl);
>> break;
>> }
>> +
>> return rc;
>> }
>>
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v4 1/3] security: Refactor LSM hooks into an array and enum
2018-03-07 20:23 ` Casey Schaufler
@ 2018-03-07 22:22 ` Sargun Dhillon
0 siblings, 0 replies; 11+ messages in thread
From: Sargun Dhillon @ 2018-03-07 22:22 UTC (permalink / raw)
To: Casey Schaufler; +Cc: LSM, LKML, Tetsuo Handa, Kees Cook, Igor Stoppa
On Wed, Mar 7, 2018 at 12:23 PM, Casey Schaufler <casey@schaufler-ca.com> wrote:
> On 3/7/2018 11:18 AM, Sargun Dhillon wrote:
>> On Wed, Mar 7, 2018 at 9:45 AM, Casey Schaufler <casey@schaufler-ca.com> wrote:
>>> On 3/6/2018 11:23 PM, Sargun Dhillon wrote:
>>>> This commit should have no functional change. It changes the security hook
>>>> list heads struct into an array. Additionally, it exposes all of the hooks
>>>> via an enum. This loses memory layout randomization as the enum is not
>>>> randomized.
>>> Please explain why you want to do this. I still dislike it.
>>>
>> Do you dislike it because of the loss of randomization, or some other reason?
>
> I dislike a huge array of untyped function pointers.
> I dislike the loss of type checking in security.c
>
Ok, I can go back to that. My previous approach was using two
list_heads, but people suggested otherwise. Would you be okay with the
>> The reason for not just having a second list_heads is that it's
>> somewhat ugly having to replicate that structure twice -- once for
>> dynamic hooks, and once for 'static' hooks.
>
> There was discussion about this some time ago. In the case
> where you don't allow dynamic hooks, you mark the lists ro_after_init
> whereas in the case with them you don't, but use the locking.
>
>
Why expose the compiled-in hooks to being RW? To me, having two
list_heads seems better, because you have the built-in ones be read
only, and the ones at load time be read/write. In fact, we could
protect the r/w with pmalloc?
>> Instead, we have one enum that LSMs can use and two arrays of heads
>> rather than an entire unrolled set of list_heads.
>
> But how is this better? What is the advantage?
>
>>
>> If we had a way to randomize this, would it make you comfortable?
>>
>
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2018-03-07 22:23 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-07 7:22 [PATCH v4 0/3] Safe, dynamically loadable LSM hooks Sargun Dhillon
2018-03-07 7:23 ` [PATCH v4 1/3] security: Refactor LSM hooks into an array and enum Sargun Dhillon
2018-03-07 17:45 ` Casey Schaufler
2018-03-07 19:18 ` Sargun Dhillon
2018-03-07 20:23 ` Casey Schaufler
2018-03-07 22:22 ` Sargun Dhillon
2018-03-07 7:23 ` [PATCH v4 2/3] security: Expose a mechanism to load lsm hooks dynamically at runtime Sargun Dhillon
2018-03-07 17:59 ` Casey Schaufler
2018-03-07 20:29 ` Sargun Dhillon
2018-03-07 7:23 ` [PATCH v4 3/3] security: Add an example sample dynamic LSM Sargun Dhillon
2018-03-07 17:40 ` [PATCH v4 0/3] Safe, dynamically loadable LSM hooks Casey Schaufler
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).