Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1514258 > unrolled thread
| Started by | Jann Horn <jann@thejh.net> |
|---|---|
| First post | 2016-11-03 04:10 +0100 |
| Last post | 2016-11-03 14:20 +0100 |
| Articles | 2 — 2 participants |
Back to article view | Back to linux.kernel
This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by
below is the oldest one visible, not the original post.
[PATCH v3 3/3] selinux: require EXECMEM for forced ptrace poke Jann Horn <jann@thejh.net> - 2016-11-03 04:10 +0100
Re: [PATCH v3 3/3] selinux: require EXECMEM for forced ptrace poke Stephen Smalley <sds@tycho.nsa.gov> - 2016-11-03 14:20 +0100
| From | Jann Horn <jann@thejh.net> |
|---|---|
| Date | 2016-11-03 04:10 +0100 |
| Subject | [PATCH v3 3/3] selinux: require EXECMEM for forced ptrace poke |
| Message-ID | <szgzE-6A7-19@gated-at.bofh.it> |
This restricts forced writes to private R+X mappings using the EXECMEM
permission. To avoid a breaking change, a new policy capability needs to
be enabled before the new restrictions take effect.
Unlike most other SELinux hooks, this one takes the subject credentials as
an argument instead of looking up current_cred(). This is done because the
security_forced_write() LSM hook can be invoked from within the write
handler of /proc/$pid/mem, where current_cred() is pretty useless.
Changed in v3:
- minor: symmetric comment (Ingo Molnar)
- use helper struct (Ingo Molnar)
- add new policy capability for enabling forced write checks
(Stephen Smalley)
Signed-off-by: Jann Horn <jann@thejh.net>
---
security/selinux/hooks.c | 15 +++++++++++++++
security/selinux/include/security.h | 2 ++
security/selinux/selinuxfs.c | 3 ++-
security/selinux/ss/services.c | 3 +++
4 files changed, 22 insertions(+), 1 deletion(-)
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 09fd6108e421..cdd9c53db2ed 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -2144,6 +2144,20 @@ static int selinux_ptrace_traceme(struct task_struct *parent)
return task_has_perm(parent, current, PROCESS__PTRACE);
}
+static int selinux_forced_write(struct vm_area_struct *vma,
+ const struct gup_creds *creds)
+{
+ /*
+ * Permitting a write to readonly memory is fine - making the readonly
+ * memory executable afterwards would require EXECMOD permission because
+ * anon_vma would be non-NULL.
+ */
+ if (!selinux_policycap_forcedwrite || (vma->vm_flags & VM_EXEC) == 0)
+ return 0;
+
+ return cred_has_perm(creds->subject, creds->object, PROCESS__EXECMEM);
+}
+
static int selinux_capget(struct task_struct *target, kernel_cap_t *effective,
kernel_cap_t *inheritable, kernel_cap_t *permitted)
{
@@ -6085,6 +6099,7 @@ static struct security_hook_list selinux_hooks[] = {
LSM_HOOK_INIT(ptrace_access_check, selinux_ptrace_access_check),
LSM_HOOK_INIT(ptrace_traceme, selinux_ptrace_traceme),
+ LSM_HOOK_INIT(forced_write, selinux_forced_write),
LSM_HOOK_INIT(capget, selinux_capget),
LSM_HOOK_INIT(capset, selinux_capset),
LSM_HOOK_INIT(capable, selinux_capable),
diff --git a/security/selinux/include/security.h b/security/selinux/include/security.h
index 308a286c6cbe..87228f0ff09c 100644
--- a/security/selinux/include/security.h
+++ b/security/selinux/include/security.h
@@ -71,6 +71,7 @@ enum {
POLICYDB_CAPABILITY_OPENPERM,
POLICYDB_CAPABILITY_REDHAT1,
POLICYDB_CAPABILITY_ALWAYSNETWORK,
+ POLICYDB_CAPABILITY_FORCEDWRITE,
__POLICYDB_CAPABILITY_MAX
};
#define POLICYDB_CAPABILITY_MAX (__POLICYDB_CAPABILITY_MAX - 1)
@@ -78,6 +79,7 @@ enum {
extern int selinux_policycap_netpeer;
extern int selinux_policycap_openperm;
extern int selinux_policycap_alwaysnetwork;
+extern int selinux_policycap_forcedwrite;
/*
* type_datum properties
diff --git a/security/selinux/selinuxfs.c b/security/selinux/selinuxfs.c
index 72c145dd799f..a646cb801242 100644
--- a/security/selinux/selinuxfs.c
+++ b/security/selinux/selinuxfs.c
@@ -46,7 +46,8 @@ static char *policycap_names[] = {
"network_peer_controls",
"open_perms",
"redhat1",
- "always_check_network"
+ "always_check_network",
+ "forced_write"
};
unsigned int selinux_checkreqprot = CONFIG_SECURITY_SELINUX_CHECKREQPROT_VALUE;
diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
index 082b20c78363..4017810030d6 100644
--- a/security/selinux/ss/services.c
+++ b/security/selinux/ss/services.c
@@ -73,6 +73,7 @@
int selinux_policycap_netpeer;
int selinux_policycap_openperm;
int selinux_policycap_alwaysnetwork;
+int selinux_policycap_forcedwrite;
static DEFINE_RWLOCK(policy_rwlock);
@@ -1990,6 +1991,8 @@ static void security_load_policycaps(void)
POLICYDB_CAPABILITY_OPENPERM);
selinux_policycap_alwaysnetwork = ebitmap_get_bit(&policydb.policycaps,
POLICYDB_CAPABILITY_ALWAYSNETWORK);
+ selinux_policycap_forcedwrite = ebitmap_get_bit(&policydb.policycaps,
+ POLICYDB_CAPABILITY_FORCEDWRITE);
}
static int security_preserve_bools(struct policydb *p);
--
2.1.4
[toc] | [next] | [standalone]
| From | Stephen Smalley <sds@tycho.nsa.gov> |
|---|---|
| Date | 2016-11-03 14:20 +0100 |
| Message-ID | <szq5X-4mP-3@gated-at.bofh.it> |
| In reply to | #1514258 |
On 11/02/2016 11:04 PM, Jann Horn wrote:
> This restricts forced writes to private R+X mappings using the EXECMEM
> permission. To avoid a breaking change, a new policy capability needs to
> be enabled before the new restrictions take effect.
>
> Unlike most other SELinux hooks, this one takes the subject credentials as
> an argument instead of looking up current_cred(). This is done because the
> security_forced_write() LSM hook can be invoked from within the write
> handler of /proc/$pid/mem, where current_cred() is pretty useless.
>
> Changed in v3:
> - minor: symmetric comment (Ingo Molnar)
> - use helper struct (Ingo Molnar)
> - add new policy capability for enabling forced write checks
> (Stephen Smalley)
>
> Signed-off-by: Jann Horn <jann@thejh.net>
> ---
> security/selinux/hooks.c | 15 +++++++++++++++
> security/selinux/include/security.h | 2 ++
> security/selinux/selinuxfs.c | 3 ++-
> security/selinux/ss/services.c | 3 +++
> 4 files changed, 22 insertions(+), 1 deletion(-)
>
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 09fd6108e421..cdd9c53db2ed 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -2144,6 +2144,20 @@ static int selinux_ptrace_traceme(struct task_struct *parent)
> return task_has_perm(parent, current, PROCESS__PTRACE);
> }
>
> +static int selinux_forced_write(struct vm_area_struct *vma,
> + const struct gup_creds *creds)
> +{
> + /*
> + * Permitting a write to readonly memory is fine - making the readonly
> + * memory executable afterwards would require EXECMOD permission because
> + * anon_vma would be non-NULL.
> + */
> + if (!selinux_policycap_forcedwrite || (vma->vm_flags & VM_EXEC) == 0)
> + return 0;
> +
> + return cred_has_perm(creds->subject, creds->object, PROCESS__EXECMEM);
> +}
> +
> static int selinux_capget(struct task_struct *target, kernel_cap_t *effective,
> kernel_cap_t *inheritable, kernel_cap_t *permitted)
> {
> @@ -6085,6 +6099,7 @@ static struct security_hook_list selinux_hooks[] = {
>
> LSM_HOOK_INIT(ptrace_access_check, selinux_ptrace_access_check),
> LSM_HOOK_INIT(ptrace_traceme, selinux_ptrace_traceme),
> + LSM_HOOK_INIT(forced_write, selinux_forced_write),
> LSM_HOOK_INIT(capget, selinux_capget),
> LSM_HOOK_INIT(capset, selinux_capset),
> LSM_HOOK_INIT(capable, selinux_capable),
> diff --git a/security/selinux/include/security.h b/security/selinux/include/security.h
> index 308a286c6cbe..87228f0ff09c 100644
> --- a/security/selinux/include/security.h
> +++ b/security/selinux/include/security.h
> @@ -71,6 +71,7 @@ enum {
> POLICYDB_CAPABILITY_OPENPERM,
> POLICYDB_CAPABILITY_REDHAT1,
> POLICYDB_CAPABILITY_ALWAYSNETWORK,
> + POLICYDB_CAPABILITY_FORCEDWRITE,
> __POLICYDB_CAPABILITY_MAX
> };
> #define POLICYDB_CAPABILITY_MAX (__POLICYDB_CAPABILITY_MAX - 1)
> @@ -78,6 +79,7 @@ enum {
> extern int selinux_policycap_netpeer;
> extern int selinux_policycap_openperm;
> extern int selinux_policycap_alwaysnetwork;
> +extern int selinux_policycap_forcedwrite;
>
> /*
> * type_datum properties
> diff --git a/security/selinux/selinuxfs.c b/security/selinux/selinuxfs.c
> index 72c145dd799f..a646cb801242 100644
> --- a/security/selinux/selinuxfs.c
> +++ b/security/selinux/selinuxfs.c
> @@ -46,7 +46,8 @@ static char *policycap_names[] = {
> "network_peer_controls",
> "open_perms",
> "redhat1",
> - "always_check_network"
> + "always_check_network",
> + "forced_write"
This is a nit, but can you provide a more descriptive capability name
that would be meaningful to policy writers and signifies that this
policy capability enables checking execmem in these situations?
> };
>
> unsigned int selinux_checkreqprot = CONFIG_SECURITY_SELINUX_CHECKREQPROT_VALUE;
> diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
> index 082b20c78363..4017810030d6 100644
> --- a/security/selinux/ss/services.c
> +++ b/security/selinux/ss/services.c
> @@ -73,6 +73,7 @@
> int selinux_policycap_netpeer;
> int selinux_policycap_openperm;
> int selinux_policycap_alwaysnetwork;
> +int selinux_policycap_forcedwrite;
>
> static DEFINE_RWLOCK(policy_rwlock);
>
> @@ -1990,6 +1991,8 @@ static void security_load_policycaps(void)
> POLICYDB_CAPABILITY_OPENPERM);
> selinux_policycap_alwaysnetwork = ebitmap_get_bit(&policydb.policycaps,
> POLICYDB_CAPABILITY_ALWAYSNETWORK);
> + selinux_policycap_forcedwrite = ebitmap_get_bit(&policydb.policycaps,
> + POLICYDB_CAPABILITY_FORCEDWRITE);
> }
>
> static int security_preserve_bools(struct policydb *p);
>
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web