Skip to contentSkip to conversation

Further SELinux restrictions on mprotect

RFC PATCH In review Lorenzo Hernández García-Hierro 2 participants 5 messages Apr 20, 2005 – Apr 28, 2005 1 message not in the archive SELinux security module

lore
Share & tools
lore
#0 lore raw

RFC PATCH 1/2 Further SELinux restrictions on mprotect

Hi,

The following patch set implements additional SELinux permission checks on making memory executable via mprotect based on prior suggestions by Ulrich Drepper and follow-up discussions by Roland McGrath and Ingo Molnar, and some guidance/feedback from Stephen Smalley.

In order to properly test the effects of this patch set (aside from checking the return value of mprotect calls), one needs an underlying no-execute technology, whether software-based like exec-shield or PAX or hardware-based.

As paxtest performs a real test of the executability of the memory and doesn't just bail out on a failed mprotect call, you need the NX mechanism when testing with it.

The patch(es) need testing before deciding about their future regarding mainline merging or whatever else that could be appropriate. This means that we need to identify and test applications where we need to allow execmem e.g. for runtime-code generation but can still prevent making the stack executable or heap executable without loss in
functionality. Note that there may be an issue with respect to the
current handling of PT_GNU_STACK for such applications, e.g. they may be set up with an executable stack in the first place and thus the mprotect restrictions may never come into play.

Example candidates for testing are Valgrind, Java (jit-like compilers), ...

I encourage those interested in these patches or just with some free time to "waste", to make such testing and give back the results, suggest and make any critics or comments.

I've made available a regression test suite, simplistic (doesn't do any fixed outputting, available at
http://pearls.tuxedo-es.org/patches/selinux/execstack-regression-0.1.tar.gz)
which could be used, but I recommend to use a "de-sabotaged" or just fixed, paxtest.

The patches are known to work but have been tested only on i386 (IA-32).

The first patch, included inline below, adds an execstack permission check that controls the ability to make the main process stack executable so that attempts to make the stack executable can still be prevented even if the process is allowed the existing execmem permission in order to e.g. perform runtime code generation. Note that this does not yet address thread stacks. Note also that unlike the execmem check, the execstack check is only applied on mprotect calls, not mmap calls, as the current security_file_mmap hook is not passed the necessary information presently.

The original author of the code that makes the distinction of the stack region, is Ingo Molnar, who wrote it within his patch for /proc/<pid>/maps markers.
(http://marc.theaimsgroup.com/?l=linux-kernel&m=110719881508591&w=2)

The patches also can be found at:
http://pearls.tuxedo-es.org/patches/selinux/policy-execstack.patch
http://pearls.tuxedo-es.org/patches/selinux/kernel-execstack.patch

policy-execstack.patch is the patch that needs to be applied to the policy in order to support the execstack permission and exclude it
from general_domain_access within macros/core_macros.te.

kernel-execstack.patch adds such permission to the SELinux code within the kernel and adds the proper permission check to the selinux_file_mprotect() hook.

---

 linux-2.6-20050404-lorenzo/security/selinux/hooks.c                     |   10 ++++++++++
 linux-2.6-20050404-lorenzo/security/selinux/include/av_perm_to_string.h |    1 +
 linux-2.6-20050404-lorenzo/security/selinux/include/av_permissions.h    |    1 +
 3 files changed, 12 insertions(+)
security/selinux/include/av_permissions.h renamed from security/selinux/include/av_permissions.h~kernel-execstack +1 -0
diff -puN security/selinux/include/av_permissions.h~kernel-execstack security/selinux/include/av_permissions.h
@@ -465,6 +465,7 @@
 #define PROCESS__DYNTRANSITION                    0x00800000UL
 #define PROCESS__SETCURRENT                       0x01000000UL
 #define PROCESS__EXECMEM                          0x02000000UL
+#define PROCESS__EXECSTACK                        0x04000000UL
 
 #define IPC__CREATE                               0x00000001UL
 #define IPC__DESTROY                              0x00000002UL
security/selinux/include/av_perm_to_string.h renamed from security/selinux/include/av_perm_to_string.h~kernel-execstack +1 -0
diff -puN security/selinux/include/av_perm_to_string.h~kernel-execstack security/selinux/include/av_perm_to_string.h
@@ -70,6 +70,7 @@
    S_(SECCLASS_PROCESS, PROCESS__DYNTRANSITION, "dyntransition")
    S_(SECCLASS_PROCESS, PROCESS__SETCURRENT, "setcurrent")
    S_(SECCLASS_PROCESS, PROCESS__EXECMEM, "execmem")
+   S_(SECCLASS_PROCESS, PROCESS__EXECSTACK, "execstack")
    S_(SECCLASS_MSGQ, MSGQ__ENQUEUE, "enqueue")
    S_(SECCLASS_MSG, MSG__SEND, "send")
    S_(SECCLASS_MSG, MSG__RECEIVE, "receive")
security/selinux/hooks.c renamed from security/selinux/hooks.c~kernel-execstack +10 -0
diff -puN security/selinux/hooks.c~kernel-execstack security/selinux/hooks.c
@@ -2478,6 +2478,16 @@ static int selinux_file_mprotect(struct 
 		if (rc)
 			return rc;
 	}
+	if (!vma->vm_file && (prot & PROT_EXEC) &&
+		vma->vm_start <= vma->vm_mm->start_stack &&
+		vma->vm_end >= vma->vm_mm->start_stack) {
+		/* Attempt to make the process stack executable.
+		 * This has an additional execstack check.
+		 */
+		rc = task_has_perm(current, current, PROCESS__EXECSTACK);
+		if (rc)
+			return rc;
+	}
 #endif
 
 	return file_map_prot_check(vma->vm_file, prot, vma->vm_flags&VM_SHARED);

_

Signature · 2 lines
Lorenzo Hernández García-Hierro <[email protected]> 
[1024D/6F2B2DEC] & [2048g/9AE91A22][http://tuxedo-es.org]
#1 lore raw
Quoted 9 lines · Lorenzo Hernández García-Hierro

The first patch, included inline below, adds an execstack permission check that controls the ability to make the main process stack executable so that attempts to make the stack executable can still be prevented even if the process is allowed the existing execmem permission in order to e.g. perform runtime code generation. Note that this does not yet address thread stacks. Note also that unlike the execmem check, the execstack check is only applied on mprotect calls, not mmap calls, as the current security_file_mmap hook is not passed the necessary information presently.

Thanks for working on these patches. I built a 2.6.12-rc3 kernel with execshield and both of these patches applied, and then performed a little experimentation, yielding the following results:

The execstack check kills the mprotstack test of the modified paxtest as expected even if execmem is allowed. Likewise, the execheap check kills the mprotheap test of the modified paxtest.

As expected, X requires execheap permission presently.

Running firefox with the Sun java plugin installed yields an execstack denial on firefox-bin, and the plugin doesn't seem to work then. Hence, we don't seem to gain any benefit in that particular case.

Running valgrind does not yield an execstack denial and only appears to require execmem permission.

Running OpenOffice generates audit messages for both execstack and execheap denials on soffice.bin, but still appears to function. Not sure what is occurring there.

What do others think about these additional permission checks, particularly in terms of being able to practically apply them? Has anyone else experimented with them?

Signature · 8 lines
Stephen Smalley <[email protected]>
National Security Agency


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to [email protected] with
the words "unsubscribe selinux" without quotes as the message.
#2 lore raw
Quoted 1 line · Stephen Smalley

Thanks for working on these patches.

Thanks to you for the guidance and feedback.

Quoted 9 lines

I built a 2.6.12-rc3 kernel with
execshield and both of these patches applied, and then performed a little experimentation, yielding the following results:

The execstack check kills the mprotstack test of the modified paxtest as expected even if execmem is allowed. Likewise, the execheap check kills the mprotheap test of the modified paxtest.

As expected, X requires execheap permission presently.

Yes.

Quoted 3 lines

Running firefox with the Sun java plugin installed yields an execstack denial on firefox-bin, and the plugin doesn't seem to work then. Hence, we don't seem to gain any benefit in that particular case.

Related with the issue regarding OOo.

Quoted 2 lines

Running valgrind does not yield an execstack denial and only appears to require execmem permission.

It's a special case as Ulrich notes in the latter reply. I was wrong about it, I apologize for any inconvenience.

Quoted 3 lines

Running OpenOffice generates audit messages for both execstack and execheap denials on soffice.bin, but still appears to function. Not sure what is occurring there.

OOo may use a JVM, that depends slightly on your installation, and it's not something *imperative* to have, thus, it can cause the denials but still work, of course without the features relying in the JVM. It could be good to check (using Ingo's /proc/PID/maps markers patch) where's located the executable heap segment and where's the executable stack, for use in a later debugging (indeed, OOo is a big beast in that terms...).

Cheers and many thanks for testing,

Signature · 2 lines
Lorenzo Hernández García-Hierro <[email protected]> 
[1024D/6F2B2DEC] & [2048g/9AE91A22][http://tuxedo-es.org]
#3 lore raw
Quoted 5 lines · Lorenzo Hernández García-Hierro
Quoted 2 lines

Running valgrind does not yield an execstack denial and only appears to require execmem permission.

It's a special case as Ulrich notes in the latter reply. I was wrong about it, I apologize for any inconvenience.

Just to clarify, the valgrind behavior is what I expected to happen, i.e. it needs to perform runtime code generation (thus needs execmem) but does not need an executable stack (thus does not need execstack). Thus, that particular test went as expected; it has the desired behavior
- we can allow it execmem without allowing it execstack. What was surprising/disheartening was that we didn't get the expected behavior from the Sun jre, where it seemed to require execstack as well as execmem in order to function. But hopefully that can be fixed.

Signature · 8 lines
Stephen Smalley <[email protected]>
National Security Agency


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to [email protected] with
the words "unsubscribe selinux" without quotes as the message.

A message that is not in the archive (sent off-list or not yet received): [email protected]

#5 lore raw
Quoted 9 lines · Ulrich Drepper
Quoted 3 lines · Stephen Smalley

What do others think about these additional permission checks, particularly in terms of being able to practically apply them? Has anyone else experimented with them?

I certainly like all the lockdown in this area we can get. The other approaches to achieve this (i.e., pax) are simply unsuitable for a general OS release. The fact that some programs need relaxed policies now doesn't mean this cannot be changed.

I agree, it's a matter of doing it well, and doing it simple, at least from the users side.
That would ensure a future and wide deployment in an huge variety of distributions (ie. Debian, Ubuntu...), and more important, in the "Desktop world", generally.

Quoted 2 lines

For instance, the X issue you saw is iirc changed upstream. Maybe upstream == R7, but I think the people are receptive.

I was looking around it, and seems that is patched right now. I think it was also on RH's bugzilla, submitted by Ingo, but I dunno certainly.

Quoted 2 lines

As for the JVM, I have contacts at Sun and I'm sure they have an equal interest in making the JVM secure.

Good to know, personally I don't care much on Java but we must provide support for the most possible wide range of users, and that would help a lot.

Quoted 1 line

valgrind is a special tool, I'm not worried.

:)

Cheers and thanks for your comments,

Signature · 2 lines
Lorenzo Hernández García-Hierro <[email protected]> 
[1024D/6F2B2DEC] & [2048g/9AE91A22][http://tuxedo-es.org]