Add mechanism to write protect all unloadable security modules

RFC PATCH New Sargun Dhillon 5 participants 12 messages Mar 26, 2018 – Mar 27, 2018

Subsystems (7)

SELinux security module Security (LSM) Smack security module Apparmor security module +3 more subsystems

lore

Share & tools

lore

Sargun Dhillon

author

2018-03-26 19:24 UTC #0 lore raw

cover Add mechanism to write protect all unloadable security modules

This patchset consists of three features:
1) Not using a circular list for security hook heads.
This has the benefit of allowing us to remove hooks at the tail of the chain, without interfering with the head. In turn, this allows us to always mark the head as read-only.

This also saves space, as there is only one pointer, not two.

2) Adding an intermediate set of mutable hooks.
In the linked-list of hooks, there will be the:
head->write protected hooks->noop mutable hook->mutable hooks

This intermediate hook makes it so that we can prevent built-in LSMs from being modified.

3) Making it so you can load modules after boot time.

Sargun Dhillon (2):
security: convert security hooks to use hlist
security: Add mechanism to (un)load LSMs after boot time

 include/linux/lsm_hooks.h  | 435 +++++++++++++++++++++++----------------------
 security/apparmor/lsm.c    |   2 +-
 security/commoncap.c       |   2 +-
 security/security.c        | 105 +++++++++--
 security/selinux/hooks.c   |   5 +-
 security/smack/smack_lsm.c |   3 +-
 security/tomoyo/tomoyo.c   |   3 +-
 security/yama/yama_lsm.c   |   2 +-
 8 files changed, 315 insertions(+), 242 deletions(-)

Signature · 6 lines

2.14.1

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Sargun Dhillon

author

2018-03-26 19:24 UTC #1 lore raw

RFC PATCH 1/2 security: convert security hooks to use hlist

This changes security_hook_heads to use hlist_heads instead of the circular doubly-linked list heads. This should cut down the size of the struct by about half.

Signed-off-by Sargun Dhillon <[email protected]>

---
 include/linux/lsm_hooks.h | 428 +++++++++++++++++++++++-----------------------
 security/security.c       |  22 +--
 2 files changed, 225 insertions(+), 225 deletions(-)

include/linux/lsm_hooks.h +214 ~~-214~~

index 7161d8e7ee79..09bc60fb35f1 100644
@@ -1730,230 +1730,230 @@ union security_list_options {
 };
 
 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;
+	struct hlist_head binder_set_context_mgr;
+	struct hlist_head binder_transaction;
+	struct hlist_head binder_transfer_binder;
+	struct hlist_head binder_transfer_file;
+	struct hlist_head ptrace_access_check;
+	struct hlist_head ptrace_traceme;
+	struct hlist_head capget;
+	struct hlist_head capset;
+	struct hlist_head capable;
+	struct hlist_head quotactl;
+	struct hlist_head quota_on;
+	struct hlist_head syslog;
+	struct hlist_head settime;
+	struct hlist_head vm_enough_memory;
+	struct hlist_head bprm_set_creds;
+	struct hlist_head bprm_check_security;
+	struct hlist_head bprm_committing_creds;
+	struct hlist_head bprm_committed_creds;
+	struct hlist_head sb_alloc_security;
+	struct hlist_head sb_free_security;
+	struct hlist_head sb_copy_data;
+	struct hlist_head sb_remount;
+	struct hlist_head sb_kern_mount;
+	struct hlist_head sb_show_options;
+	struct hlist_head sb_statfs;
+	struct hlist_head sb_mount;
+	struct hlist_head sb_umount;
+	struct hlist_head sb_pivotroot;
+	struct hlist_head sb_set_mnt_opts;
+	struct hlist_head sb_clone_mnt_opts;
+	struct hlist_head sb_parse_opts_str;
+	struct hlist_head dentry_init_security;
+	struct hlist_head 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;
+	struct hlist_head path_unlink;
+	struct hlist_head path_mkdir;
+	struct hlist_head path_rmdir;
+	struct hlist_head path_mknod;
+	struct hlist_head path_truncate;
+	struct hlist_head path_symlink;
+	struct hlist_head path_link;
+	struct hlist_head path_rename;
+	struct hlist_head path_chmod;
+	struct hlist_head path_chown;
+	struct hlist_head 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;
+	struct hlist_head inode_alloc_security;
+	struct hlist_head inode_free_security;
+	struct hlist_head inode_init_security;
+	struct hlist_head inode_create;
+	struct hlist_head inode_link;
+	struct hlist_head inode_unlink;
+	struct hlist_head inode_symlink;
+	struct hlist_head inode_mkdir;
+	struct hlist_head inode_rmdir;
+	struct hlist_head inode_mknod;
+	struct hlist_head inode_rename;
+	struct hlist_head inode_readlink;
+	struct hlist_head inode_follow_link;
+	struct hlist_head inode_permission;
+	struct hlist_head inode_setattr;
+	struct hlist_head inode_getattr;
+	struct hlist_head inode_setxattr;
+	struct hlist_head inode_post_setxattr;
+	struct hlist_head inode_getxattr;
+	struct hlist_head inode_listxattr;
+	struct hlist_head inode_removexattr;
+	struct hlist_head inode_need_killpriv;
+	struct hlist_head inode_killpriv;
+	struct hlist_head inode_getsecurity;
+	struct hlist_head inode_setsecurity;
+	struct hlist_head inode_listsecurity;
+	struct hlist_head inode_getsecid;
+	struct hlist_head inode_copy_up;
+	struct hlist_head inode_copy_up_xattr;
+	struct hlist_head file_permission;
+	struct hlist_head file_alloc_security;
+	struct hlist_head file_free_security;
+	struct hlist_head file_ioctl;
+	struct hlist_head mmap_addr;
+	struct hlist_head mmap_file;
+	struct hlist_head file_mprotect;
+	struct hlist_head file_lock;
+	struct hlist_head file_fcntl;
+	struct hlist_head file_set_fowner;
+	struct hlist_head file_send_sigiotask;
+	struct hlist_head file_receive;
+	struct hlist_head file_open;
+	struct hlist_head task_alloc;
+	struct hlist_head task_free;
+	struct hlist_head cred_alloc_blank;
+	struct hlist_head cred_free;
+	struct hlist_head cred_prepare;
+	struct hlist_head cred_transfer;
+	struct hlist_head kernel_act_as;
+	struct hlist_head kernel_create_files_as;
+	struct hlist_head kernel_read_file;
+	struct hlist_head kernel_post_read_file;
+	struct hlist_head kernel_module_request;
+	struct hlist_head task_fix_setuid;
+	struct hlist_head task_setpgid;
+	struct hlist_head task_getpgid;
+	struct hlist_head task_getsid;
+	struct hlist_head task_getsecid;
+	struct hlist_head task_setnice;
+	struct hlist_head task_setioprio;
+	struct hlist_head task_getioprio;
+	struct hlist_head task_prlimit;
+	struct hlist_head task_setrlimit;
+	struct hlist_head task_setscheduler;
+	struct hlist_head task_getscheduler;
+	struct hlist_head task_movememory;
+	struct hlist_head task_kill;
+	struct hlist_head task_prctl;
+	struct hlist_head task_to_inode;
+	struct hlist_head ipc_permission;
+	struct hlist_head ipc_getsecid;
+	struct hlist_head msg_msg_alloc_security;
+	struct hlist_head msg_msg_free_security;
+	struct hlist_head msg_queue_alloc_security;
+	struct hlist_head msg_queue_free_security;
+	struct hlist_head msg_queue_associate;
+	struct hlist_head msg_queue_msgctl;
+	struct hlist_head msg_queue_msgsnd;
+	struct hlist_head msg_queue_msgrcv;
+	struct hlist_head shm_alloc_security;
+	struct hlist_head shm_free_security;
+	struct hlist_head shm_associate;
+	struct hlist_head shm_shmctl;
+	struct hlist_head shm_shmat;
+	struct hlist_head sem_alloc_security;
+	struct hlist_head sem_free_security;
+	struct hlist_head sem_associate;
+	struct hlist_head sem_semctl;
+	struct hlist_head sem_semop;
+	struct hlist_head netlink_send;
+	struct hlist_head d_instantiate;
+	struct hlist_head getprocattr;
+	struct hlist_head setprocattr;
+	struct hlist_head ismaclabel;
+	struct hlist_head secid_to_secctx;
+	struct hlist_head secctx_to_secid;
+	struct hlist_head release_secctx;
+	struct hlist_head inode_invalidate_secctx;
+	struct hlist_head inode_notifysecctx;
+	struct hlist_head inode_setsecctx;
+	struct hlist_head 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;
+	struct hlist_head unix_stream_connect;
+	struct hlist_head unix_may_send;
+	struct hlist_head socket_create;
+	struct hlist_head socket_post_create;
+	struct hlist_head socket_bind;
+	struct hlist_head socket_connect;
+	struct hlist_head socket_listen;
+	struct hlist_head socket_accept;
+	struct hlist_head socket_sendmsg;
+	struct hlist_head socket_recvmsg;
+	struct hlist_head socket_getsockname;
+	struct hlist_head socket_getpeername;
+	struct hlist_head socket_getsockopt;
+	struct hlist_head socket_setsockopt;
+	struct hlist_head socket_shutdown;
+	struct hlist_head socket_sock_rcv_skb;
+	struct hlist_head socket_getpeersec_stream;
+	struct hlist_head socket_getpeersec_dgram;
+	struct hlist_head sk_alloc_security;
+	struct hlist_head sk_free_security;
+	struct hlist_head sk_clone_security;
+	struct hlist_head sk_getsecid;
+	struct hlist_head sock_graft;
+	struct hlist_head inet_conn_request;
+	struct hlist_head inet_csk_clone;
+	struct hlist_head inet_conn_established;
+	struct hlist_head secmark_relabel_packet;
+	struct hlist_head secmark_refcount_inc;
+	struct hlist_head secmark_refcount_dec;
+	struct hlist_head req_classify_flow;
+	struct hlist_head tun_dev_alloc_security;
+	struct hlist_head tun_dev_free_security;
+	struct hlist_head tun_dev_create;
+	struct hlist_head tun_dev_attach_queue;
+	struct hlist_head tun_dev_attach;
+	struct hlist_head 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;
+	struct hlist_head ib_pkey_access;
+	struct hlist_head ib_endport_manage_subnet;
+	struct hlist_head ib_alloc_security;
+	struct hlist_head 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;
+	struct hlist_head xfrm_policy_alloc_security;
+	struct hlist_head xfrm_policy_clone_security;
+	struct hlist_head xfrm_policy_free_security;
+	struct hlist_head xfrm_policy_delete_security;
+	struct hlist_head xfrm_state_alloc;
+	struct hlist_head xfrm_state_alloc_acquire;
+	struct hlist_head xfrm_state_free_security;
+	struct hlist_head xfrm_state_delete_security;
+	struct hlist_head xfrm_policy_lookup;
+	struct hlist_head xfrm_state_pol_flow_match;
+	struct hlist_head 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;
+	struct hlist_head key_alloc;
+	struct hlist_head key_free;
+	struct hlist_head key_permission;
+	struct hlist_head 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;
+	struct hlist_head audit_rule_init;
+	struct hlist_head audit_rule_known;
+	struct hlist_head audit_rule_match;
+	struct hlist_head 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;
+	struct hlist_head bpf;
+	struct hlist_head bpf_map;
+	struct hlist_head bpf_prog;
+	struct hlist_head bpf_map_alloc_security;
+	struct hlist_head bpf_map_free_security;
+	struct hlist_head bpf_prog_alloc_security;
+	struct hlist_head bpf_prog_free_security;
 #endif /* CONFIG_BPF_SYSCALL */
 } __randomize_layout;

@@ -1962,8 +1962,8 @@ struct security_hook_heads {
  * For use with generic list macros for common operations.
  */
 struct security_hook_list {
-	struct list_head		list;
-	struct list_head		*head;
+	struct hlist_node		list;
+	struct hlist_head		*head;
 	union security_list_options	hook;
 	char				*lsm;
 } __randomize_layout;

@@ -2002,7 +2002,7 @@ static inline void security_delete_hooks(struct security_hook_list *hooks,
 	int i;
 
 	for (i = 0; i < count; i++)
-		list_del_rcu(&hooks[i].list);
+		hlist_del_rcu(&hooks[i].list);
 }
 #endif /* CONFIG_SECURITY_SELINUX_DISABLE */

security/security.c +11 ~~-11~~

index 1cd8526cb0b7..3cafff61b049 100644
@@ -61,11 +61,11 @@ static void __init do_security_initcalls(void)
 int __init security_init(void)
 {
 	int i;
-	struct list_head *list = (struct list_head *) &security_hook_heads;
+	struct hlist_head *list = (struct hlist_head *) &security_hook_heads;
 
-	for (i = 0; i < sizeof(security_hook_heads) / sizeof(struct list_head);
+	for (i = 0; i < sizeof(security_hook_heads) / sizeof(struct hlist_head);
 	     i++)
-		INIT_LIST_HEAD(&list[i]);
+		INIT_HLIST_HEAD(&list[i]);
 	pr_info("Security Framework initialized\n");
 
 	/*

@@ -163,7 +163,7 @@ void __init security_add_hooks(struct security_hook_list *hooks, int count,
 
 	for (i = 0; i < count; i++) {
 		hooks[i].lsm = lsm;
-		list_add_tail_rcu(&hooks[i].list, hooks[i].head);
+		hlist_add_tail_rcu(&hooks[i].list, hooks[i].head);
 	}
 	if (lsm_append(lsm, &lsm_names) < 0)
 		panic("%s - Cannot get early memory.\n", __func__);

@@ -201,7 +201,7 @@ EXPORT_SYMBOL(unregister_lsm_notifier);
 	do {							\
 		struct security_hook_list *P;			\
 								\
-		list_for_each_entry(P, &security_hook_heads.FUNC, list)	\
+		hlist_for_each_entry(P, &security_hook_heads.FUNC, list) \
 			P->hook.FUNC(__VA_ARGS__);		\
 	} while (0)

@@ -210,7 +210,7 @@ EXPORT_SYMBOL(unregister_lsm_notifier);
 	do {							\
 		struct security_hook_list *P;			\
 								\
-		list_for_each_entry(P, &security_hook_heads.FUNC, list) { \
+		hlist_for_each_entry(P, &security_hook_heads.FUNC, list) { \
 			RC = P->hook.FUNC(__VA_ARGS__);		\
 			if (RC != 0)				\
 				break;				\

@@ -317,7 +317,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) {
+	hlist_for_each_entry(hp, &security_hook_heads.vm_enough_memory, list) {
 		rc = hp->hook.vm_enough_memory(mm, pages);
 		if (rc <= 0) {
 			cap_sys_admin = 0;

@@ -805,7 +805,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) {
+	hlist_for_each_entry(hp, &security_hook_heads.inode_getsecurity, list) {
 		rc = hp->hook.inode_getsecurity(inode, name, buffer, alloc);
 		if (rc != -EOPNOTSUPP)
 			return rc;

@@ -823,7 +823,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) {
+	hlist_for_each_entry(hp, &security_hook_heads.inode_setsecurity, list) {
 		rc = hp->hook.inode_setsecurity(inode, name, value, size,
 								flags);
 		if (rc != -EOPNOTSUPP)

@@ -1126,7 +1126,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) {
+	hlist_for_each_entry(hp, &security_hook_heads.task_prctl, list) {
 		thisrc = hp->hook.task_prctl(option, arg2, arg3, arg4, arg5);
 		if (thisrc != -ENOSYS) {
 			rc = thisrc;

@@ -1629,7 +1629,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,
+	hlist_for_each_entry(hp, &security_hook_heads.xfrm_state_pol_flow_match,
 				list) {
 		rc = hp->hook.xfrm_state_pol_flow_match(x, xp, fl);
 		break;

Signature · 6 lines

2.14.1

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Sargun Dhillon

author

2018-03-26 19:24 UTC #2 lore raw

RFC PATCH 2/2 security: Add mechanism to (un)load LSMs after boot time

In reply to Sargun Dhillon

This patch introduces a mechanism to add mutable hooks at the end of the callback chain for each LSM hook. It allows for built-in kernel LSMs to be unloaded, as well as modular LSMs to be loaded after boot-time. It also does not compromise the security of hooks which are never meant to be unloaded.

Signed-off-by Sargun Dhillon <[email protected]>

---
 include/linux/lsm_hooks.h  |  7 ++--
 security/apparmor/lsm.c    |  2 +-
 security/commoncap.c       |  2 +-
 security/security.c        | 87 +++++++++++++++++++++++++++++++++++++++++-----
 security/selinux/hooks.c   |  5 +--
 security/smack/smack_lsm.c |  3 +-
 security/tomoyo/tomoyo.c   |  3 +-
 security/yama/yama_lsm.c   |  2 +-
 8 files changed, 92 insertions(+), 19 deletions(-)

include/linux/lsm_hooks.h +4 -3

index 09bc60fb35f1..c0c98758b1bf 100644
@@ -1981,7 +1981,7 @@ extern struct security_hook_heads security_hook_heads;
 extern char *lsm_names;
 
 extern void security_add_hooks(struct security_hook_list *hooks, int count,
-				char *lsm);
+				char *lsm, bool is_mutable);
 
 #ifdef CONFIG_SECURITY_SELINUX_DISABLE
 /*

@@ -2006,11 +2006,12 @@ static inline void security_delete_hooks(struct security_hook_list *hooks,
 }
 #endif /* CONFIG_SECURITY_SELINUX_DISABLE */
 
+#define __lsm_ro_after_init	__ro_after_init
 /* Currently required to handle SELinux runtime hook disable. */
 #ifdef CONFIG_SECURITY_WRITABLE_HOOKS
-#define __lsm_ro_after_init
+#define __lsm_mutable_after_init
 #else
-#define __lsm_ro_after_init	__ro_after_init
+#define __lsm_mutable_after_init __ro_after_init
 #endif /* CONFIG_SECURITY_WRITABLE_HOOKS */
 
 extern int __init security_module_enable(const char *module);

security/apparmor/lsm.c +1 -1

index 9a65eeaf7dfa..d6cca8169df0 100644
@@ -1155,7 +1155,7 @@ static int __init apparmor_init(void)
 		goto buffers_out;
 	}
 	security_add_hooks(apparmor_hooks, ARRAY_SIZE(apparmor_hooks),
-				"apparmor");
+				"apparmor", false);
 
 	/* Report that AppArmor successfully initialized */
 	apparmor_initialized = 1;

security/commoncap.c +1 -1

index 48620c93d697..fe4b0d9d44ce 100644
@@ -1363,7 +1363,7 @@ struct security_hook_list capability_hooks[] __lsm_ro_after_init = {
 void __init capability_add_hooks(void)
 {
 	security_add_hooks(capability_hooks, ARRAY_SIZE(capability_hooks),
-				"capability");
+				"capability", false);
 }
 
 #endif /* CONFIG_SECURITY */

security/security.c +78 -9

174 lines not on this page · raw message

security/selinux/hooks.c +3 -2

index 8644d864e3c1..f05184a3a7a6 100644
@@ -6393,7 +6393,7 @@ static void selinux_bpf_prog_free(struct bpf_prog_aux *aux)
 }
 #endif
 
-static struct security_hook_list selinux_hooks[] __lsm_ro_after_init = {
+static struct security_hook_list selinux_hooks[] __lsm_mutable_after_init = {
 	LSM_HOOK_INIT(binder_set_context_mgr, selinux_binder_set_context_mgr),
 	LSM_HOOK_INIT(binder_transaction, selinux_binder_transaction),
 	LSM_HOOK_INIT(binder_transfer_binder, selinux_binder_transfer_binder),

@@ -6651,7 +6651,8 @@ static __init int selinux_init(void)
 					    0, SLAB_PANIC, NULL);
 	avc_init();
 
-	security_add_hooks(selinux_hooks, ARRAY_SIZE(selinux_hooks), "selinux");
+	security_add_hooks(selinux_hooks, ARRAY_SIZE(selinux_hooks), "selinux",
+				IS_ENABLED(CONFIG_SECURITY_SELINUX_DISABLE));
 
 	if (avc_add_callback(selinux_netcache_avc_callback, AVC_CALLBACK_RESET))
 		panic("SELinux: Unable to register AVC netcache callback\n");

security/smack/smack_lsm.c +2 -1

index 03fdecba93bb..7a9f1bb06c8e 100644
@@ -4902,7 +4902,8 @@ static __init int smack_init(void)
 	/*
 	 * Register with LSM
 	 */
-	security_add_hooks(smack_hooks, ARRAY_SIZE(smack_hooks), "smack");
+	security_add_hooks(smack_hooks, ARRAY_SIZE(smack_hooks), "smack",
+				false);
 
 	return 0;
 }

security/tomoyo/tomoyo.c +2 -1

index 213b8c593668..ba74fab0e9a5 100644
@@ -543,7 +543,8 @@ static int __init tomoyo_init(void)
 	if (!security_module_enable("tomoyo"))
 		return 0;
 	/* register ourselves with the security framework */
-	security_add_hooks(tomoyo_hooks, ARRAY_SIZE(tomoyo_hooks), "tomoyo");
+	security_add_hooks(tomoyo_hooks, ARRAY_SIZE(tomoyo_hooks), "tomoyo",
+				false);
 	printk(KERN_INFO "TOMOYO Linux initialized\n");
 	cred->security = &tomoyo_kernel_domain;
 	tomoyo_mm_init();

security/yama/yama_lsm.c +1 -1

index ffda91a4a1aa..04c9aed9e951 100644
@@ -480,6 +480,6 @@ static inline void yama_init_sysctl(void) { }
 void __init yama_add_hooks(void)
 {
 	pr_info("Yama: becoming mindful.\n");
-	security_add_hooks(yama_hooks, ARRAY_SIZE(yama_hooks), "yama");
+	security_add_hooks(yama_hooks, ARRAY_SIZE(yama_hooks), "yama", false);
 	yama_init_sysctl();
 }

Signature · 6 lines

2.14.1

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Igor Stoppa

2018-03-26 20:17 UTC #3 lore raw

RFC PATCH 2/2 security: Add mechanism to (un)load LSMs after boot time

Quoted 5 lines · Sargun Dhillon

This patch introduces a mechanism to add mutable hooks at the end of the callback chain for each LSM hook. It allows for built-in kernel LSMs to be unloaded, as well as modular LSMs to be loaded after boot-time. It also does not compromise the security of hooks which are never meant to be unloaded.

Looking at this from the perspective of really convincing people to use other modules, there is a problem, imho.

[...]

security/smack/smack_lsm.c static __init int smack_init(void) · line 4906

Quoted 6 lines

security/smack/smack_lsm.c +2 -1

 	/*
 	 * Register with LSM
 	 */
-	security_add_hooks(smack_hooks, ARRAY_SIZE(smack_hooks), "smack");
+	security_add_hooks(smack_hooks, ARRAY_SIZE(smack_hooks), "smack",
+				false);

Hardcoding what is (im)mutable will never satisfy everyone. If, instead, this decision was delegated to the kernel command line, it would be possible to have any module to become immutable -or not- depending on the default values and the configuration received at boot.

A distro could ship with its defaults and then any user could reconfigure it, without having to recompile or install anything, just editing the command line.

--
igor
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majordomo at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

Sargun Dhillon

author

2018-03-26 20:24 UTC #4 lore raw

RFC PATCH 2/2 security: Add mechanism to (un)load LSMs after boot time

Quoted 26 lines · Igor Stoppa

Quoted 5 lines · Sargun Dhillon
This patch introduces a mechanism to add mutable hooks at the end of the callback chain for each LSM hook. It allows for built-in kernel LSMs to be unloaded, as well as modular LSMs to be loaded after boot-time. It also does not compromise the security of hooks which are never meant to be unloaded.
Looking at this from the perspective of really convincing people to use other modules, there is a problem, imho.
[...]
security/smack/smack_lsm.c static __init int smack_init(void) · line 4906
Quoted 6 lines
      /*
       * Register with LSM
       */
security/smack/smack_lsm.c +2 -1
-     security_add_hooks(smack_hooks, ARRAY_SIZE(smack_hooks), "smack");
+     security_add_hooks(smack_hooks, ARRAY_SIZE(smack_hooks), "smack",
+                             false);
Hardcoding what is (im)mutable will never satisfy everyone. If, instead, this decision was delegated to the kernel command line, it would be possible to have any module to become immutable -or not- depending on the default values and the configuration received at boot.

Quoted 6 lines

A distro could ship with its defaults and then any user could reconfigure it, without having to recompile or install anything, just editing the command line.
--
igor

Today, the only "mutable" module we have is SELinux. It has a kernel config flag which determines if it is unloadable (mutable) or not. If you look at the patchset, it, in fact, sets mutability based on that config flag:

- security_add_hooks(selinux_hooks, ARRAY_SIZE(selinux_hooks), "selinux");
+ security_add_hooks(selinux_hooks, ARRAY_SIZE(selinux_hooks), "selinux",
+ IS_ENABLED(CONFIG_SECURITY_SELINUX_DISABLE));

Similarly, modules can change this behaviour based on their own choices, whether that be config flags, boot parameters, or similar. In my opinion, most LSMs should never be unloadable. In fact, we don't really have a safe way (in this patchset) to unload the security modules' code safely. Doing this would either require usage of (s)rcu, or a similar concurrency control mechanism.
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majordomo at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

Casey Schaufler

maintainer

2018-03-26 21:04 UTC #5 lore raw

RFC PATCH 2/2 security: Add mechanism to (un)load LSMs after boot time

Quoted 24 lines · Sargun Dhillon

Quoted 23 lines · Igor Stoppa
Quoted 5 lines · Sargun Dhillon
This patch introduces a mechanism to add mutable hooks at the end of the callback chain for each LSM hook. It allows for built-in kernel LSMs to be unloaded, as well as modular LSMs to be loaded after boot-time. It also does not compromise the security of hooks which are never meant to be unloaded.
Looking at this from the perspective of really convincing people to use other modules, there is a problem, imho.
[...]
security/smack/smack_lsm.c static __init int smack_init(void) · line 4906
Quoted 6 lines
      /*
       * Register with LSM
       */
security/smack/smack_lsm.c +2 -1
-     security_add_hooks(smack_hooks, ARRAY_SIZE(smack_hooks), "smack");
+     security_add_hooks(smack_hooks, ARRAY_SIZE(smack_hooks), "smack",
+                             false);
Hardcoding what is (im)mutable will never satisfy everyone. If, instead, this decision was delegated to the kernel command line, it would be possible to have any module to become immutable -or not- depending on the default values and the configuration received at boot.

Quoted 7 lines

Quoted 6 lines
A distro could ship with its defaults and then any user could reconfigure it, without having to recompile or install anything, just editing the command line.
--
igor
Today, the only "mutable" module we have is SELinux.

Paul Moore has threatened to remove the "mutable" option from SELinux should it bump up high enough on his priority list. The conditions under which you can disable SELinux are very limited, you can't unload it after the policy has been loaded.

Quoted 10 lines

It has a kernel
config flag which determines if it is unloadable (mutable) or not. If you look at the patchset, it, in fact, sets mutability based on that config flag:
security/selinux/hooks.c +2 -1
-       security_add_hooks(selinux_hooks, ARRAY_SIZE(selinux_hooks), "selinux");
+       security_add_hooks(selinux_hooks, ARRAY_SIZE(selinux_hooks), "selinux",
+                               IS_ENABLED(CONFIG_SECURITY_SELINUX_DISABLE));

Quoted 6 lines

Similarly, modules can change this behaviour based on their own choices, whether that be config flags, boot parameters, or similar. In my opinion, most LSMs should never be unloadable. In fact, we don't really have a safe way (in this patchset) to unload the security modules' code safely. Doing this would either require usage of (s)rcu, or a similar concurrency control mechanism.

If you can't safely unload the code, what is the point of making a module unloadable? Start every hook with:

	if (!gonkulator_enabled)
		return appropriately;

and provide mechanism for flipping the switch.

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majordomo at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

Sargun Dhillon

author

2018-03-26 21:10 UTC #6 lore raw

RFC PATCH 2/2 security: Add mechanism to (un)load LSMs after boot time

Quoted 55 lines · Casey Schaufler

Quoted 24 lines · Sargun Dhillon
Quoted 23 lines · Igor Stoppa
Quoted 5 lines · Sargun Dhillon
This patch introduces a mechanism to add mutable hooks at the end of the callback chain for each LSM hook. It allows for built-in kernel LSMs to be unloaded, as well as modular LSMs to be loaded after boot-time. It also does not compromise the security of hooks which are never meant to be unloaded.
Looking at this from the perspective of really convincing people to use other modules, there is a problem, imho.
[...]
security/smack/smack_lsm.c static __init int smack_init(void) · line 4906
Quoted 6 lines
      /*
       * Register with LSM
       */
security/smack/smack_lsm.c +2 -1
-     security_add_hooks(smack_hooks, ARRAY_SIZE(smack_hooks), "smack");
+     security_add_hooks(smack_hooks, ARRAY_SIZE(smack_hooks), "smack",
+                             false);
Hardcoding what is (im)mutable will never satisfy everyone. If, instead, this decision was delegated to the kernel command line, it would be possible to have any module to become immutable -or not- depending on the default values and the configuration received at boot.
Quoted 7 lines
Quoted 6 lines
A distro could ship with its defaults and then any user could reconfigure it, without having to recompile or install anything, just editing the command line.
--
igor
Today, the only "mutable" module we have is SELinux.
Paul Moore has threatened to remove the "mutable" option from SELinux should it bump up high enough on his priority list. The conditions under which you can disable SELinux are very limited, you can't unload it after the policy has been loaded.
Quoted 10 lines
It has a kernel
config flag which determines if it is unloadable (mutable) or not. If you look at the patchset, it, in fact, sets mutability based on that config flag:
security/selinux/hooks.c +2 -1
-       security_add_hooks(selinux_hooks, ARRAY_SIZE(selinux_hooks), "selinux");
+       security_add_hooks(selinux_hooks, ARRAY_SIZE(selinux_hooks), "selinux",
+                               IS_ENABLED(CONFIG_SECURITY_SELINUX_DISABLE));
Quoted 6 lines
Similarly, modules can change this behaviour based on their own choices, whether that be config flags, boot parameters, or similar. In my opinion, most LSMs should never be unloadable. In fact, we don't really have a safe way (in this patchset) to unload the security modules' code safely. Doing this would either require usage of (s)rcu, or a similar concurrency control mechanism.

Quoted 2 lines

If you can't safely unload the code, what is the point of making a module unloadable? Start every hook with:

s/module/hook/
Ask the SELinux folk?

Quoted 6 lines

        if (!gonkulator_enabled)
                return appropriately;
and provide mechanism for flipping the switch.

I intend to eventually follow-up this patch with a way to safely unload the code, but I'd prefer to get (safe) dynamic *hooking* and *unhooking* first, and then later on we can make it so the code can get unloaded.
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majordomo at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

Casey Schaufler

maintainer

2018-03-26 21:46 UTC #7 lore raw

RFC PATCH 2/2 security: Add mechanism to (un)load LSMs after boot time

Quoted 53 lines · Sargun Dhillon

Quoted 52 lines · Casey Schaufler
Quoted 23 lines · Sargun Dhillon
Quoted 22 lines · Igor Stoppa
Quoted 5 lines · Sargun Dhillon
This patch introduces a mechanism to add mutable hooks at the end of the callback chain for each LSM hook. It allows for built-in kernel LSMs to be unloaded, as well as modular LSMs to be loaded after boot-time. It also does not compromise the security of hooks which are never meant to be unloaded.
Looking at this from the perspective of really convincing people to use other modules, there is a problem, imho.
[...]
security/smack/smack_lsm.c static __init int smack_init(void) · line 4906
Quoted 6 lines
      /*
       * Register with LSM
       */
security/smack/smack_lsm.c +2 -1
-     security_add_hooks(smack_hooks, ARRAY_SIZE(smack_hooks), "smack");
+     security_add_hooks(smack_hooks, ARRAY_SIZE(smack_hooks), "smack",
+                             false);
Hardcoding what is (im)mutable will never satisfy everyone. If, instead, this decision was delegated to the kernel command line, it would be possible to have any module to become immutable -or not- depending on the default values and the configuration received at boot.
Quoted 7 lines
Quoted 6 lines
A distro could ship with its defaults and then any user could reconfigure it, without having to recompile or install anything, just editing the command line.
--
igor
Today, the only "mutable" module we have is SELinux.
Paul Moore has threatened to remove the "mutable" option from SELinux should it bump up high enough on his priority list. The conditions under which you can disable SELinux are very limited, you can't unload it after the policy has been loaded.
Quoted 16 lines
It has a kernel
config flag which determines if it is unloadable (mutable) or not. If you look at the patchset, it, in fact, sets mutability based on that config flag:
security/selinux/hooks.c +2 -1
-       security_add_hooks(selinux_hooks, ARRAY_SIZE(selinux_hooks), "selinux");
+       security_add_hooks(selinux_hooks, ARRAY_SIZE(selinux_hooks), "selinux",
+                               IS_ENABLED(CONFIG_SECURITY_SELINUX_DISABLE));
Similarly, modules can change this behaviour based on their own choices, whether that be config flags, boot parameters, or similar. In my opinion, most LSMs should never be unloadable. In fact, we don't really have a safe way (in this patchset) to unload the security modules' code safely. Doing this would either require usage of (s)rcu, or a similar concurrency control mechanism.

Quoted 4 lines

Quoted 2 lines
If you can't safely unload the code, what is the point of making a module unloadable? Start every hook with:
s/module/hook/
Ask the SELinux folk?

SELinux can safely unload the hooks because they limit the ability to unload to a point before they've started allocating data. They aren't proud of it. I seriously doubt they'd have if if you could run will a NULL policy.

Quoted 9 lines

Quoted 5 lines
        if (!gonkulator_enabled)
                return appropriately;
and provide mechanism for flipping the switch.
I intend to eventually follow-up this patch with a way to safely unload the code, but I'd prefer to get (safe) dynamic *hooking* and *unhooking* first, and then later on we can make it so the code can get unloaded.

Unless you want to restrict the unhooking and unloading to a point where no data has been allocated by the module it's going to be difficult to argue that it's safe to unhook and/or unload a module. Sure, SELinux has that restriction, but the use case is a bit questionable.

Sargun Dhillon

author

2018-03-26 21:51 UTC #8 lore raw

RFC PATCH 2/2 security: Add mechanism to (un)load LSMs after boot time

Quoted 74 lines · Casey Schaufler

Quoted 53 lines · Sargun Dhillon
Quoted 52 lines · Casey Schaufler
Quoted 23 lines · Sargun Dhillon
Quoted 22 lines · Igor Stoppa
Quoted 5 lines · Sargun Dhillon
This patch introduces a mechanism to add mutable hooks at the end of the callback chain for each LSM hook. It allows for built-in kernel LSMs to be unloaded, as well as modular LSMs to be loaded after boot-time. It also does not compromise the security of hooks which are never meant to be unloaded.
Looking at this from the perspective of really convincing people to use other modules, there is a problem, imho.
[...]
security/smack/smack_lsm.c static __init int smack_init(void) · line 4906
Quoted 6 lines
      /*
       * Register with LSM
       */
security/smack/smack_lsm.c +2 -1
-     security_add_hooks(smack_hooks, ARRAY_SIZE(smack_hooks), "smack");
+     security_add_hooks(smack_hooks, ARRAY_SIZE(smack_hooks), "smack",
+                             false);
Hardcoding what is (im)mutable will never satisfy everyone. If, instead, this decision was delegated to the kernel command line, it would be possible to have any module to become immutable -or not- depending on the default values and the configuration received at boot.
Quoted 7 lines
Quoted 6 lines
A distro could ship with its defaults and then any user could reconfigure it, without having to recompile or install anything, just editing the command line.
--
igor
Today, the only "mutable" module we have is SELinux.
Paul Moore has threatened to remove the "mutable" option from SELinux should it bump up high enough on his priority list. The conditions under which you can disable SELinux are very limited, you can't unload it after the policy has been loaded.
Quoted 16 lines
It has a kernel
config flag which determines if it is unloadable (mutable) or not. If you look at the patchset, it, in fact, sets mutability based on that config flag:
security/selinux/hooks.c +2 -1
-       security_add_hooks(selinux_hooks, ARRAY_SIZE(selinux_hooks), "selinux");
+       security_add_hooks(selinux_hooks, ARRAY_SIZE(selinux_hooks), "selinux",
+                               IS_ENABLED(CONFIG_SECURITY_SELINUX_DISABLE));
Similarly, modules can change this behaviour based on their own choices, whether that be config flags, boot parameters, or similar. In my opinion, most LSMs should never be unloadable. In fact, we don't really have a safe way (in this patchset) to unload the security modules' code safely. Doing this would either require usage of (s)rcu, or a similar concurrency control mechanism.
Quoted 4 lines
Quoted 2 lines
If you can't safely unload the code, what is the point of making a module unloadable? Start every hook with:
s/module/hook/
Ask the SELinux folk?
SELinux can safely unload the hooks because they limit the ability to unload to a point before they've started allocating data. They aren't proud of it. I seriously doubt they'd have if if you could run will a NULL policy.
Quoted 9 lines
Quoted 5 lines
        if (!gonkulator_enabled)
                return appropriately;
and provide mechanism for flipping the switch.
I intend to eventually follow-up this patch with a way to safely unload the code, but I'd prefer to get (safe) dynamic *hooking* and *unhooking* first, and then later on we can make it so the code can get unloaded.

Quoted 7 lines

Unless you want to restrict the unhooking and unloading to a point where no data has been allocated by the module it's going to be difficult to argue that it's safe to unhook and/or unload a module. Sure, SELinux has that restriction, but the use case is a bit questionable.

What's wrong for this in the case of during development, and for minor LSMs? --
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majordomo at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

Casey Schaufler

maintainer

2018-03-26 22:17 UTC #9 lore raw

RFC PATCH 2/2 security: Add mechanism to (un)load LSMs after boot time

Quoted 73 lines · Sargun Dhillon

Quoted 72 lines · Casey Schaufler
Quoted 53 lines · Sargun Dhillon
Quoted 52 lines · Casey Schaufler
Quoted 23 lines · Sargun Dhillon
Quoted 22 lines · Igor Stoppa
Quoted 5 lines · Sargun Dhillon
This patch introduces a mechanism to add mutable hooks at the end of the callback chain for each LSM hook. It allows for built-in kernel LSMs to be unloaded, as well as modular LSMs to be loaded after boot-time. It also does not compromise the security of hooks which are never meant to be unloaded.
Looking at this from the perspective of really convincing people to use other modules, there is a problem, imho.
[...]
Review
Quoted 6 lines
      /*
       * Register with LSM
       */
(quoted diff) +2 -1
-     security_add_hooks(smack_hooks, ARRAY_SIZE(smack_hooks), "smack");
+     security_add_hooks(smack_hooks, ARRAY_SIZE(smack_hooks), "smack",
+                             false);
Hardcoding what is (im)mutable will never satisfy everyone. If, instead, this decision was delegated to the kernel command line, it would be possible to have any module to become immutable -or not- depending on the default values and the configuration received at boot.
Quoted 7 lines
Quoted 6 lines
A distro could ship with its defaults and then any user could reconfigure it, without having to recompile or install anything, just editing the command line.
--
igor
Today, the only "mutable" module we have is SELinux.
Paul Moore has threatened to remove the "mutable" option from SELinux should it bump up high enough on his priority list. The conditions under which you can disable SELinux are very limited, you can't unload it after the policy has been loaded.
Quoted 16 lines
It has a kernel
config flag which determines if it is unloadable (mutable) or not. If you look at the patchset, it, in fact, sets mutability based on that config flag:
(quoted diff) +2 -1
-       security_add_hooks(selinux_hooks, ARRAY_SIZE(selinux_hooks), "selinux");
+       security_add_hooks(selinux_hooks, ARRAY_SIZE(selinux_hooks), "selinux",
+                               IS_ENABLED(CONFIG_SECURITY_SELINUX_DISABLE));
Similarly, modules can change this behaviour based on their own choices, whether that be config flags, boot parameters, or similar. In my opinion, most LSMs should never be unloadable. In fact, we don't really have a safe way (in this patchset) to unload the security modules' code safely. Doing this would either require usage of (s)rcu, or a similar concurrency control mechanism.
Quoted 4 lines
Quoted 2 lines
If you can't safely unload the code, what is the point of making a module unloadable? Start every hook with:
s/module/hook/
Ask the SELinux folk?
SELinux can safely unload the hooks because they limit the ability to unload to a point before they've started allocating data. They aren't proud of it. I seriously doubt they'd have if if you could run will a NULL policy.
Quoted 9 lines
Quoted 5 lines
        if (!gonkulator_enabled)
                return appropriately;
and provide mechanism for flipping the switch.
I intend to eventually follow-up this patch with a way to safely unload the code, but I'd prefer to get (safe) dynamic *hooking* and *unhooking* first, and then later on we can make it so the code can get unloaded.

Quoted 8 lines

Quoted 7 lines
Unless you want to restrict the unhooking and unloading to a point where no data has been allocated by the module it's going to be difficult to argue that it's safe to unhook and/or unload a module. Sure, SELinux has that restriction, but the use case is a bit questionable.
What's wrong for this in the case of during development, and for minor LSMs?

You can always add ifdefs and unnatural runtime options during development. You don't need infrastructure support for that. I will grant you minor LSMs that don't allocate and/or manage data. As you included in your patchset, you have to identify if a module is "mutable" for this to be a safe feature. My recollection from the early days of LSM that there were objections to unloadable modules for other reasons, but that was long ago and I don't recall them clearly.

James Morris

maintainer

2018-03-27 08:08 UTC #10 lore raw

RFC PATCH 2/2 security: Add mechanism to (un)load LSMs after boot time

In reply to Sargun Dhillon

security/selinux/hooks.c static __init int selinux_init(void) · line 6655

Quoted 10 lines · Sargun Dhillon

Today, the only "mutable" module we have is SELinux. It has a kernel config flag which determines if it is unloadable (mutable) or not. If you look at the patchset, it, in fact, sets mutability based on that config flag:
security/selinux/hooks.c +2 -1
-       security_add_hooks(selinux_hooks, ARRAY_SIZE(selinux_hooks), "selinux");
+       security_add_hooks(selinux_hooks, ARRAY_SIZE(selinux_hooks), "selinux",
+                               IS_ENABLED(CONFIG_SECURITY_SELINUX_DISABLE));

There has been discussion about removing the ability to unload SELinux -- not sure what the current status of that is.

Regardless, it's a special case for historical reasons and should not be thought of as an example for future use.

Quoted 3 lines

Similarly, modules can change this behaviour based on their own choices, whether that be config flags, boot parameters, or similar. In my opinion, most LSMs should never be unloadable.

All, probably.

Signature · 7 lines

James Morris
<[email protected]>

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Paul Moore

maintainer

2018-03-27 11:52 UTC #11 lore raw

RFC PATCH 2/2 security: Add mechanism to (un)load LSMs after boot time

Quoted 7 lines · James Morris

Quoted 6 lines · Sargun Dhillon
Today, the only "mutable" module we have is SELinux. It has a kernel config flag which determines if it is unloadable (mutable) or not. If you look at the patchset, it, in fact, sets mutability based on that config flag:

Quoted 6 lines

security/selinux/hooks.c static __init int selinux_init(void) · line 6655
Quoted 3 lines
security/selinux/hooks.c +2 -1
-       security_add_hooks(selinux_hooks, ARRAY_SIZE(selinux_hooks), "selinux");
+       security_add_hooks(selinux_hooks, ARRAY_SIZE(selinux_hooks), "selinux",
+                               IS_ENABLED(CONFIG_SECURITY_SELINUX_DISABLE));
There has been discussion about removing the ability to unload SELinux -- not sure what the current status of that is.

It is something I would still like to do, but there is some work that needs to be done to allow a smooth transition for those people who are currently disabling/unloading SELinux at runtime.

Signature · 6 lines

paul moore
www.paul-moore.com
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html