2016-03-29 19:53:59

by Scott Bauer

[permalink] [raw]
Subject: [PATCH v4 0/4] SROP Mitigation: Sigreturn Cookies

Sigreturn-oriented programming is a new attack vector in userland
where an attacker crafts a fake signal frame on the stack and calls
sigreturn. The kernel will extract the fake signal frame, which
contains attacker controlled "saved" registers. The kernel will then
transfer control to the attacker controlled userland instruction pointer.

To prevent SROP attacks the kernel needs to know or be able to dervive
whether a sigreturn it is processing is in response to a legitimate
signal the kernel previously delivered.

Further information and test code can be found in Documentation/security
and this excellent article:
http://lwn.net/Articles/676803/

These patches implement the necessary changes to generate a cookie
which will be placed above signal frame upon signal delivery to userland.
The cookie is generated using a per-process random value xor'd with
the address where the cookie will be stored on the stack.

Upon a sigreturn the kernel will extract the cookie from userland,
recalculate what the original cookie should be and verify that the two
do not differ. If the two differ the kernel will terminate the process
with a SIGSEGV.

This prevents SROP by adding a value that the attacker cannot guess,
but the kernel can verify. Therefore an attacker cannot use sigreturn as
a method to control the flow of a process.


Version changes:

v3->v4
Removed ambiguous __user annotation, added Documentation
and test code.

v2->v3
Changed cookie calculation from using restored regs->sp to
using frame pointer from before restoration.

v1->v2
Miscellaneous nits and code cleanup.

Scott Bauer (4):
SROP Mitigation: Architecture independent code for signal cookies
x86: SROP Mitigation: Implement Signal Cookies
Sysctl: SROP Mitigation: Add Sysctl argument to disable SROP.
Documentation: SROP Mitigation: Add documentation for SROP cookies


Documentation/security/srop-cookies.txt | 203 ++++++++++++++++++++++++++++++++
arch/x86/ia32/ia32_signal.c | 65 +++++++++-
arch/x86/include/asm/fpu/signal.h | 2 +
arch/x86/kernel/fpu/signal.c | 10 ++
arch/x86/kernel/signal.c | 83 +++++++++++--
fs/exec.c | 3 +
include/linux/sched.h | 7 ++
include/linux/signal.h | 3 +
kernel/signal.c | 49 ++++++++
kernel/sysctl.c | 8 ++
10 files changed, 418 insertions(+), 15 deletions(-)


2016-03-29 19:54:05

by Scott Bauer

[permalink] [raw]
Subject: [PATCH v4 1/4] SROP Mitigation: Architecture independent code for signal cookies

This patch adds a per-process secret to the task struct which
will be used during signal delivery and during a sigreturn.
Also, logic is added in signal.c to generate, place, extract,
clear and verify the signal cookie.

Cc: Abhiram Balasubramanian <[email protected]>
Signed-off-by: Scott Bauer <[email protected]>
Signed-off-by: Scott Bauer <[email protected]>
---
fs/exec.c | 3 +++
include/linux/sched.h | 7 +++++++
include/linux/signal.h | 2 ++
kernel/signal.c | 40 ++++++++++++++++++++++++++++++++++++++++
4 files changed, 52 insertions(+)

diff --git a/fs/exec.c b/fs/exec.c
index c4010b8..5d55c01 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -57,6 +57,7 @@
#include <linux/oom.h>
#include <linux/compat.h>
#include <linux/vmalloc.h>
+#include <linux/random.h>

#include <asm/uaccess.h>
#include <asm/mmu_context.h>
@@ -1231,6 +1232,8 @@ void setup_new_exec(struct linux_binprm * bprm)
/* This is the point of no return */
current->sas_ss_sp = current->sas_ss_size = 0;

+ get_random_bytes(&current->sig_cookie, sizeof(current->sig_cookie));
+
if (uid_eq(current_euid(), current_uid()) && gid_eq(current_egid(), current_gid()))
set_dumpable(current->mm, SUID_DUMP_USER);
else
diff --git a/include/linux/sched.h b/include/linux/sched.h
index 60bba7e..1828fb8 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -1502,6 +1502,13 @@ struct task_struct {
unsigned long stack_canary;
#endif
/*
+ * Canary value for signal frames placed on user stack.
+ * This helps mitigate "Signal Return oriented program"
+ * exploits in userland.
+ */
+ unsigned long sig_cookie;
+
+ /*
* pointers to (original) parent process, youngest child, younger sibling,
* older sibling, respectively. (p->father can be replaced with
* p->real_parent->pid)
diff --git a/include/linux/signal.h b/include/linux/signal.h
index 92557bb..fae0618 100644
--- a/include/linux/signal.h
+++ b/include/linux/signal.h
@@ -280,6 +280,8 @@ extern int get_signal(struct ksignal *ksig);
extern void signal_setup_done(int failed, struct ksignal *ksig, int stepping);
extern void exit_signals(struct task_struct *tsk);
extern void kernel_sigaction(int, __sighandler_t);
+extern int set_sigcookie(unsigned long __user *location);
+extern int verify_clear_sigcookie(unsigned long __user *sig_cookie_ptr);

static inline void allow_signal(int sig)
{
diff --git a/kernel/signal.c b/kernel/signal.c
index aa9bf00..1e4f65c 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -2430,6 +2430,46 @@ out:
}
}

+static unsigned long gen_sigcookie(unsigned long __user *location)
+{
+
+ unsigned long sig_cookie;
+ sig_cookie = (unsigned long) location ^ current->sig_cookie;
+
+ return sig_cookie;
+}
+
+int set_sigcookie(unsigned long __user *location)
+{
+
+ unsigned long sig_cookie = gen_sigcookie(location);
+
+ return put_user(sig_cookie, location);
+}
+
+int verify_clear_sigcookie(unsigned long __user *sig_cookie_ptr)
+{
+ unsigned long user_cookie;
+ unsigned long calculated_cookie;
+
+ if (get_user(user_cookie, sig_cookie_ptr))
+ return 1;
+
+ calculated_cookie = gen_sigcookie(sig_cookie_ptr);
+
+ if (user_cookie != calculated_cookie) {
+ pr_warn("Signal protector does not match what kernel set it to"\
+ ". Possible exploit attempt or buggy program!\n");
+ return 1;
+
+ }
+
+ user_cookie = 0;
+ return put_user(user_cookie, sig_cookie_ptr);
+}
+
+EXPORT_SYMBOL(verify_clear_sigcookie);
+EXPORT_SYMBOL(set_sigcookie);
EXPORT_SYMBOL(recalc_sigpending);
EXPORT_SYMBOL_GPL(dequeue_signal);
EXPORT_SYMBOL(flush_signals);
--
1.9.1

2016-03-29 19:54:21

by Scott Bauer

[permalink] [raw]
Subject: [PATCH v4 4/4] Documentation: SROP Mitigation: Add documentation for SROP cookies

This patch adds documentation and test code for SROP mitigation.

Cc: Abhiram Balasubramanian <[email protected]>
Signed-off-by: Scott Bauer <[email protected]>
Signed-off-by: Scott Bauer <[email protected]>
---
Documentation/security/srop-cookies.txt | 203 ++++++++++++++++++++++++++++++++
1 file changed, 203 insertions(+)
create mode 100644 Documentation/security/srop-cookies.txt

diff --git a/Documentation/security/srop-cookies.txt b/Documentation/security/srop-cookies.txt
new file mode 100644
index 0000000..ee17181
--- /dev/null
+++ b/Documentation/security/srop-cookies.txt
@@ -0,0 +1,203 @@
+ Sigreturn-oriented programming and its mitigation
+
+
+A good write up can be found here: https://lwn.net/Articles/676803/
+It covers much of what is outlined in this documentation.
+
+If you're very curious you should read the SROP paper:
+http://www.cs.vu.nl/~herbertb/papers/srop_sp14.pdf
+
+If you're here to learn how to disable SROP issue the following
+Command:
+
+# echo 1 > /proc/sys/kernel/disable-srop-protection
+
+
+============================ Overview ============================
+
+Sigreturn-oriented programming(SROP) is a new technique to control
+a compromised userland process after successfully exploiting a bug.
+
+Signals are delivered to the process during context switches. The
+kernel will setup a signal frame on the process' stack which
+contains the saved state of the process prior to the context switch.
+The kernel then gives control the the process' signal handler. Once
+the process has delt with the signal it will call the sigreturn
+system call. During the context switch into the kernel, the previous
+state of where the process was executing prior to the delivery of
+the signal is overwritten, hence why the kernel must save the the
+state in a signal frame on the user's stack. The kernel will remove
+the signal frame from the process' stack and continue execution
+where the process left off.
+
+The issue with this signal delivery method is if an attacker can
+construct a fake signal frame on the compromised process' stack
+and ROP into the sigreturn system call, they have the ability to
+easily control the flow of execution by having the kernel return
+execution to wherever the signal frame says to continue execution.
+
+More importantly, SROP makes an attackers job easier. Previously
+attackers would have to search for ROP gadgets to get values into
+registers then call mprotect or mmap to get some memory where they
+could copy and execute shellcode. If the ROP gadgets didnt exist
+that allowed setting up a call to those functions then the attackers
+wouldn't be able to fully exploit the bug. With SROP however attackers
+dont' have to search for ROP gadgets to get specific values into
+registers. The attacker would simply lay out the ucontext on the stack
+as they choose then SROP into the mprotect or mmap call.
+
+======================== Mitigating SROP ========================
+
+In order to prevent SROP the kernel must remember or be able to derive
+at runtime whether a sigreturn call it is currently processing is
+in respose to a legitimate signal delivered by the kernel.
+
+During delivery of a signal the kernel will place the signal frame
+with the saved process state on the stack. It will also place a cookie
+above the signal frame.
+
+The cookie is a per-process secret xor'd with the address where it will
+be stored. During a sigreturn the kernel will extract this cookie, and
+then compare the extracted cookie against a new generated cookie:
+(the per process secret xord with address where we extracted cookie from).
+
+If the two cookies match, then the kernel has verified that it is handling
+a sigreturn from a signal that was previously legitimately delivered.
+If the cookies do not match up the kernel sends a SIGSEGV to the process,
+terminating it.
+
+After verifying the cookie, the kernel will zero out the cookie to prevent
+any sort of leaking of the cookie.
+
+This prevents SROP because it forces an attacker to know the cookie in order
+to use SROP as an attack vector.
+
+
+======================== Possible Issues ========================
+SROP protection will probably break any process or application which do
+some sort of checkpoint-restore in user space type things. As well as DOSEMU.
+
+
+
+===============================================================================
+Inlined are two programs that exploit SROP:
+
+For 32bit:
+
+Compile with if you're using a 64bit kernel:
+gcc -O0 -o srop_32 srop_32.c -g -fno-stack-protector -ffixed-ebp -ffixed-esp -m32 -DEMULATED_32
+
+or if you're already on a real 32bit kernel:
+gcc -O0 -o srop_32 srop_32.c -g -fno-stack-protector -ffixed-ebp -ffixed-esp
+
+When run without SROP protection it will exit gracefully, when SROP is
+enabled it will terminate with a SIGSEGV.
+
+#include <stdlib.h>
+#include <stdio.h>
+#include <signal.h>
+
+void syscall(void)
+{
+ exit(1);
+}
+
+void test2(void)
+{
+ register int esp asm("esp");
+ register int ebp asm("ebp");
+ struct sigcontext scon = { 0 };
+
+ scon.eax = 11;
+ scon.ebx = 0x41;
+ scon.ecx = 0;
+ scon.edx = 0;
+ scon.esp = scon.ebp = ebp;
+ scon.eip = (int)syscall+6;
+
+#if defined EMULATED_32
+ scon.fs = 0x00;
+ scon.cs = 0x23;
+ scon.ss = scon.ds = scon.es = 0x2B;
+ scon.gs = 0x63;
+#else
+ scon.fs = 0x00;
+ scon.cs = 0x73;
+ scon.ss = scon.ds = scon.es = 0x7B;
+ scon.gs = 0x33;
+#endif
+ esp = (int) &scon;
+ asm("movl $119, %eax\n");//NR_SIGRETURN;
+ asm("int $0x80\n");
+}
+
+int main(void)
+{
+ test2();
+ return 1;
+}
+
+=====================================================
+
+
+For 64 bit:
+
+gcc -O0 -o srop srop.c -g -fno-stack-protector -ffixed-rsp -ffixed-rbp
+When run the program exits normally, with SROP protetction it terminates
+with a segmentationf fault.
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <signal.h>
+
+enum {
+ REG_R8 = 0,
+ REG_R9,
+ REG_R10,
+ REG_R11,
+ REG_R12,
+ REG_R13,
+ REG_R14,
+ REG_R15,
+ REG_RDI,
+ REG_RSI,
+ REG_RBP,
+ REG_RBX,
+ REG_RDX,
+ REG_RAX,
+ REG_RCX,
+ REG_RSP,
+ REG_RIP,
+ REG_EFL,
+ REG_CSGSFS,/* Actually short cs, gs, fs, __pad0. */
+ REG_ERR,
+ REG_TRAPNO,
+ REG_OLDMASK,
+ REG_CR2
+};
+
+void _exit_(void)
+{
+ exit(1);
+}
+
+void test(void)
+{
+ struct ucontext ctx = { 0 };
+ register unsigned long rsp asm("rsp");
+ register unsigned long rbp asm("rbp");
+ ctx.uc_mcontext.gregs[REG_RIP] = (unsigned long) _exit_ + 4;
+ ctx.uc_mcontext.gregs[REG_RSP] = rsp;
+ ctx.uc_mcontext.gregs[REG_RBP] = rbp;
+ ctx.uc_mcontext.gregs[REG_CSGSFS] = 0x002b000000000033;
+ rsp = (unsigned long) &ctx;
+ asm("movq $0xf,%rax\n");
+ asm("syscall\n");
+}
+
+
+int main(void)
+{
+ test();
+ return 0;
+}
--
1.9.1

2016-03-29 19:54:14

by Scott Bauer

[permalink] [raw]
Subject: [PATCH v4 2/4] x86: SROP Mitigation: Implement Signal Cookies

This patch adds SROP mitigation logic to the x86 signal delivery
and sigreturn code. The cookie is placed in the unused alignment
space above the saved FP state, if it exists. If there is no FP
state to save then the cookie is placed in the alignment space above
the sigframe.

Cc: Abhiram Balasubramanian <[email protected]>
Signed-off-by: Scott Bauer <[email protected]>
Signed-off-by: Scott Bauer <[email protected]>
---
arch/x86/ia32/ia32_signal.c | 65 +++++++++++++++++++++++++++---
arch/x86/include/asm/fpu/signal.h | 2 +
arch/x86/kernel/fpu/signal.c | 10 +++++
arch/x86/kernel/signal.c | 83 ++++++++++++++++++++++++++++++++++-----
4 files changed, 145 insertions(+), 15 deletions(-)

diff --git a/arch/x86/ia32/ia32_signal.c b/arch/x86/ia32/ia32_signal.c
index 0552884..85ec799 100644
--- a/arch/x86/ia32/ia32_signal.c
+++ b/arch/x86/ia32/ia32_signal.c
@@ -68,7 +68,8 @@
}

static int ia32_restore_sigcontext(struct pt_regs *regs,
- struct sigcontext_32 __user *sc)
+ struct sigcontext_32 __user *sc,
+ void **user_cookie_ptr)
{
unsigned int tmpflags, err = 0;
void __user *buf;
@@ -105,6 +106,16 @@ static int ia32_restore_sigcontext(struct pt_regs *regs,
buf = compat_ptr(tmp);
} get_user_catch(err);

+ /*
+ * If there is fp state get cookie from the top of the fp state,
+ * else get it from the top of the sig frame.
+ */
+
+ if (tmp != 0)
+ *user_cookie_ptr = compat_ptr(tmp + fpu__getsize(1));
+ else
+ *user_cookie_ptr = NULL;
+
err |= fpu__restore_sig(buf, 1);

force_iret();
@@ -117,6 +128,7 @@ asmlinkage long sys32_sigreturn(void)
struct pt_regs *regs = current_pt_regs();
struct sigframe_ia32 __user *frame = (struct sigframe_ia32 __user *)(regs->sp-8);
sigset_t set;
+ void __user *user_cookie;

if (!access_ok(VERIFY_READ, frame, sizeof(*frame)))
goto badframe;
@@ -129,8 +141,16 @@ asmlinkage long sys32_sigreturn(void)

set_current_blocked(&set);

- if (ia32_restore_sigcontext(regs, &frame->sc))
+ if (ia32_restore_sigcontext(regs, &frame->sc, &user_cookie))
goto badframe;
+
+ if (user_cookie == NULL)
+ user_cookie = compat_ptr(((unsigned long) frame) +
+ sizeof(frame));
+
+ if (verify_clear_sigcookie(user_cookie))
+ goto badframe;
+
return regs->ax;

badframe:
@@ -143,6 +163,7 @@ asmlinkage long sys32_rt_sigreturn(void)
struct pt_regs *regs = current_pt_regs();
struct rt_sigframe_ia32 __user *frame;
sigset_t set;
+ void __user *user_cookie;

frame = (struct rt_sigframe_ia32 __user *)(regs->sp - 4);

@@ -153,9 +174,17 @@ asmlinkage long sys32_rt_sigreturn(void)

set_current_blocked(&set);

- if (ia32_restore_sigcontext(regs, &frame->uc.uc_mcontext))
+ if (ia32_restore_sigcontext(regs, &frame->uc.uc_mcontext, &user_cookie))
goto badframe;

+ if (user_cookie == NULL)
+ user_cookie = compat_ptr(((unsigned long) frame) +
+ sizeof(*frame));
+
+ if (verify_clear_sigcookie(user_cookie))
+ goto badframe;
+
+
if (compat_restore_altstack(&frame->uc.uc_stack))
goto badframe;

@@ -213,7 +242,8 @@ static int ia32_setup_sigcontext(struct sigcontext_32 __user *sc,
*/
static void __user *get_sigframe(struct ksignal *ksig, struct pt_regs *regs,
size_t frame_size,
- void __user **fpstate)
+ void __user **fpstate,
+ void __user **cookie_ptr)
{
struct fpu *fpu = &current->thread.fpu;
unsigned long sp;
@@ -230,11 +260,20 @@ static void __user *get_sigframe(struct ksignal *ksig, struct pt_regs *regs,
ksig->ka.sa.sa_restorer)
sp = (unsigned long) ksig->ka.sa.sa_restorer;

+ /*
+ * Allocate space for cookie above FP/Frame. It will sit in
+ * the padding of the saved FP state, or if there is no FP
+ * state it will sit in the padding of the sig frame.
+ */
+ sp -= sizeof(unsigned long);
+
if (fpu->fpstate_active) {
unsigned long fx_aligned, math_size;

sp = fpu__alloc_mathframe(sp, 1, &fx_aligned, &math_size);
*fpstate = (struct _fpstate_32 __user *) sp;
+ *cookie_ptr = (void __user *) sp + math_size;
+
if (copy_fpstate_to_sigframe(*fpstate, (void __user *)fx_aligned,
math_size) < 0)
return (void __user *) -1L;
@@ -244,6 +283,10 @@ static void __user *get_sigframe(struct ksignal *ksig, struct pt_regs *regs,
/* Align the stack pointer according to the i386 ABI,
* i.e. so that on function entry ((sp + 4) & 15) == 0. */
sp = ((sp + 4) & -16ul) - 4;
+
+ if (!fpu->fpstate_active)
+ *cookie_ptr = (void __user *) (sp + frame_size);
+
return (void __user *) sp;
}

@@ -254,6 +297,7 @@ int ia32_setup_frame(int sig, struct ksignal *ksig,
void __user *restorer;
int err = 0;
void __user *fpstate = NULL;
+ void __user *cookie_location;

/* copy_to_user optimizes that into a single 8 byte store */
static const struct {
@@ -266,7 +310,8 @@ int ia32_setup_frame(int sig, struct ksignal *ksig,
0x80cd, /* int $0x80 */
};

- frame = get_sigframe(ksig, regs, sizeof(*frame), &fpstate);
+ frame = get_sigframe(ksig, regs, sizeof(*frame),
+ &fpstate, &cookie_location);

if (!access_ok(VERIFY_WRITE, frame, sizeof(*frame)))
return -EFAULT;
@@ -274,6 +319,9 @@ int ia32_setup_frame(int sig, struct ksignal *ksig,
if (__put_user(sig, &frame->sig))
return -EFAULT;

+ if (set_sigcookie(cookie_location))
+ return -EFAULT;
+
if (ia32_setup_sigcontext(&frame->sc, fpstate, regs, set->sig[0]))
return -EFAULT;

@@ -332,6 +380,7 @@ int ia32_setup_rt_frame(int sig, struct ksignal *ksig,
void __user *restorer;
int err = 0;
void __user *fpstate = NULL;
+ void __user *cookie_location;

/* __copy_to_user optimizes that into a single 8 byte store */
static const struct {
@@ -346,11 +395,15 @@ int ia32_setup_rt_frame(int sig, struct ksignal *ksig,
0,
};

- frame = get_sigframe(ksig, regs, sizeof(*frame), &fpstate);
+ frame = get_sigframe(ksig, regs, sizeof(*frame),
+ &fpstate, &cookie_location);

if (!access_ok(VERIFY_WRITE, frame, sizeof(*frame)))
return -EFAULT;

+ if (set_sigcookie(cookie_location))
+ return -EFAULT;
+
put_user_try {
put_user_ex(sig, &frame->sig);
put_user_ex(ptr_to_compat(&frame->info), &frame->pinfo);
diff --git a/arch/x86/include/asm/fpu/signal.h b/arch/x86/include/asm/fpu/signal.h
index 0e970d0..0c279cd 100644
--- a/arch/x86/include/asm/fpu/signal.h
+++ b/arch/x86/include/asm/fpu/signal.h
@@ -28,6 +28,8 @@ unsigned long
fpu__alloc_mathframe(unsigned long sp, int ia32_frame,
unsigned long *buf_fx, unsigned long *size);

+unsigned long fpu__getsize(int ia32_frame);
+
extern void fpu__init_prepare_fx_sw_frame(void);

#endif /* _ASM_X86_FPU_SIGNAL_H */
diff --git a/arch/x86/kernel/fpu/signal.c b/arch/x86/kernel/fpu/signal.c
index 31c6a60..50535d7 100644
--- a/arch/x86/kernel/fpu/signal.c
+++ b/arch/x86/kernel/fpu/signal.c
@@ -344,6 +344,16 @@ static inline int xstate_sigframe_size(void)
return use_xsave() ? xstate_size + FP_XSTATE_MAGIC2_SIZE : xstate_size;
}

+unsigned long fpu__getsize(int ia32_frame)
+{
+ unsigned long frame_size = xstate_sigframe_size();
+
+ if (ia32_frame && use_fxsr())
+ frame_size += sizeof(struct fregs_state);
+
+ return frame_size;
+}
+
/*
* Restore FPU state from a sigframe:
*/
diff --git a/arch/x86/kernel/signal.c b/arch/x86/kernel/signal.c
index 548ddf7..87aa6f3 100644
--- a/arch/x86/kernel/signal.c
+++ b/arch/x86/kernel/signal.c
@@ -23,6 +23,7 @@
#include <linux/user-return-notifier.h>
#include <linux/uprobes.h>
#include <linux/context_tracking.h>
+#include <linux/hash.h>

#include <asm/processor.h>
#include <asm/ucontext.h>
@@ -92,7 +93,8 @@ static void force_valid_ss(struct pt_regs *regs)

static int restore_sigcontext(struct pt_regs *regs,
struct sigcontext __user *sc,
- unsigned long uc_flags)
+ unsigned long uc_flags,
+ void **user_cookie_ptr)
{
unsigned long buf_val;
void __user *buf;
@@ -146,6 +148,12 @@ static int restore_sigcontext(struct pt_regs *regs,
buf = (void __user *)buf_val;
} get_user_catch(err);

+ if (buf_val != 0)
+ *user_cookie_ptr = (void __user *)
+ (buf_val + fpu__getsize(config_enabled(CONFIG_X86_32)));
+ else
+ *user_cookie_ptr = NULL;
+
err |= fpu__restore_sig(buf, config_enabled(CONFIG_X86_32));

force_iret();
@@ -235,7 +243,7 @@ static unsigned long align_sigframe(unsigned long sp)

static void __user *
get_sigframe(struct k_sigaction *ka, struct pt_regs *regs, size_t frame_size,
- void __user **fpstate)
+ void __user **fpstate, void __user **cookie_ptr)
{
/* Default to using normal stack */
unsigned long math_size = 0;
@@ -262,14 +270,25 @@ get_sigframe(struct k_sigaction *ka, struct pt_regs *regs, size_t frame_size,
}
}

+ /*
+ * Allocate space for cookie above FP/Frame. It will sit in
+ * the padding of the saved FP state, or if there is no FP
+ * state it will sit in the padding of the sig frame.
+ */
+ sp -= sizeof(unsigned long);
+
if (fpu->fpstate_active) {
sp = fpu__alloc_mathframe(sp, config_enabled(CONFIG_X86_32),
&buf_fx, &math_size);
*fpstate = (void __user *)sp;
+ *cookie_ptr = (void __user *)sp + math_size;
}

sp = align_sigframe(sp - frame_size);

+ if (!fpu->fpstate_active)
+ *cookie_ptr = (void __user *) (sp + frame_size);
+
/*
* If we are on the alternate signal stack and would overflow it, don't.
* Return an always-bogus address instead so we will die with SIGSEGV.
@@ -316,8 +335,10 @@ __setup_frame(int sig, struct ksignal *ksig, sigset_t *set,
void __user *restorer;
int err = 0;
void __user *fpstate = NULL;
+ void __user *cookie_location;

- frame = get_sigframe(&ksig->ka, regs, sizeof(*frame), &fpstate);
+ frame = get_sigframe(&ksig->ka, regs, sizeof(*frame),
+ &fpstate, &cookie_location);

if (!access_ok(VERIFY_WRITE, frame, sizeof(*frame)))
return -EFAULT;
@@ -325,6 +346,9 @@ __setup_frame(int sig, struct ksignal *ksig, sigset_t *set,
if (__put_user(sig, &frame->sig))
return -EFAULT;

+ if (set_sigcookie(cookie_location))
+ return -EFAULT;
+
if (setup_sigcontext(&frame->sc, fpstate, regs, set->sig[0]))
return -EFAULT;

@@ -379,12 +403,17 @@ static int __setup_rt_frame(int sig, struct ksignal *ksig,
void __user *restorer;
int err = 0;
void __user *fpstate = NULL;
+ void __user *cookie_location;

- frame = get_sigframe(&ksig->ka, regs, sizeof(*frame), &fpstate);
+ frame = get_sigframe(&ksig->ka, regs, sizeof(*frame),
+ &fpstate, &cookie_location);

if (!access_ok(VERIFY_WRITE, frame, sizeof(*frame)))
return -EFAULT;

+ if (set_sigcookie(cookie_location))
+ return -EFAULT;
+
put_user_try {
put_user_ex(sig, &frame->sig);
put_user_ex(&frame->info, &frame->pinfo);
@@ -458,9 +487,11 @@ static int __setup_rt_frame(int sig, struct ksignal *ksig,
{
struct rt_sigframe __user *frame;
void __user *fp = NULL;
+ void __user *cookie_location;
int err = 0;

- frame = get_sigframe(&ksig->ka, regs, sizeof(struct rt_sigframe), &fp);
+ frame = get_sigframe(&ksig->ka, regs, sizeof(struct rt_sigframe),
+ &fp, &cookie_location);

if (!access_ok(VERIFY_WRITE, frame, sizeof(*frame)))
return -EFAULT;
@@ -470,6 +501,9 @@ static int __setup_rt_frame(int sig, struct ksignal *ksig,
return -EFAULT;
}

+ if (set_sigcookie(cookie_location))
+ return -EFAULT;
+
put_user_try {
/* Create the ucontext. */
put_user_ex(frame_uc_flags(regs), &frame->uc.uc_flags);
@@ -541,8 +575,10 @@ static int x32_setup_rt_frame(struct ksignal *ksig,
void __user *restorer;
int err = 0;
void __user *fpstate = NULL;
+ void __user *cookie_location;

- frame = get_sigframe(&ksig->ka, regs, sizeof(*frame), &fpstate);
+ frame = get_sigframe(&ksig->ka, regs, sizeof(*frame),
+ &fpstate, &cookie_location);

if (!access_ok(VERIFY_WRITE, frame, sizeof(*frame)))
return -EFAULT;
@@ -552,6 +588,9 @@ static int x32_setup_rt_frame(struct ksignal *ksig,
return -EFAULT;
}

+ if (set_sigcookie(cookie_location))
+ return -EFAULT;
+
put_user_try {
/* Create the ucontext. */
put_user_ex(frame_uc_flags(regs), &frame->uc.uc_flags);
@@ -603,6 +642,7 @@ asmlinkage unsigned long sys_sigreturn(void)
{
struct pt_regs *regs = current_pt_regs();
struct sigframe __user *frame;
+ void __user *user_cookie;
sigset_t set;

frame = (struct sigframe __user *)(regs->sp - 8);
@@ -620,8 +660,16 @@ asmlinkage unsigned long sys_sigreturn(void)
* x86_32 has no uc_flags bits relevant to restore_sigcontext.
* Save a few cycles by skipping the __get_user.
*/
- if (restore_sigcontext(regs, &frame->sc, 0))
+ if (restore_sigcontext(regs, &frame->sc, 0, &user_cookie))
+ goto badframe;
+
+ if (user_cookie == NULL)
+ user_cookie = (void __user *)
+ ((unsigned long) frame) + sizeof(*frame);
+
+ if (verify_clear_sigcookie(user_cookie))
goto badframe;
+
return regs->ax;

badframe:
@@ -635,6 +683,7 @@ asmlinkage long sys_rt_sigreturn(void)
{
struct pt_regs *regs = current_pt_regs();
struct rt_sigframe __user *frame;
+ void __user *user_cookie;
sigset_t set;
unsigned long uc_flags;

@@ -648,7 +697,14 @@ asmlinkage long sys_rt_sigreturn(void)

set_current_blocked(&set);

- if (restore_sigcontext(regs, &frame->uc.uc_mcontext, uc_flags))
+ if (restore_sigcontext(regs, &frame->uc.uc_mcontext, uc_flags, &user_cookie))
+ goto badframe;
+
+ if (user_cookie == NULL)
+ user_cookie = (void __user *)
+ ((unsigned long) frame) + sizeof(*frame);
+
+ if (verify_clear_sigcookie(user_cookie))
goto badframe;

if (restore_altstack(&frame->uc.uc_stack))
@@ -834,6 +890,7 @@ asmlinkage long sys32_x32_rt_sigreturn(void)
{
struct pt_regs *regs = current_pt_regs();
struct rt_sigframe_x32 __user *frame;
+ void __user *user_cookie;
sigset_t set;
unsigned long uc_flags;

@@ -848,7 +905,15 @@ asmlinkage long sys32_x32_rt_sigreturn(void)

set_current_blocked(&set);

- if (restore_sigcontext(regs, &frame->uc.uc_mcontext, uc_flags))
+ if (restore_sigcontext(regs, &frame->uc.uc_mcontext,
+ uc_flags, &user_cookie))
+ goto badframe;
+
+ if (user_cookie == NULL)
+ user_cookie = (void __user *)
+ ((unsigned long) frame) + sizeof(*frame);
+
+ if (verify_clear_sigcookie(user_cookie))
goto badframe;

if (compat_restore_altstack(&frame->uc.uc_stack))
--
1.9.1

2016-03-29 19:54:25

by Scott Bauer

[permalink] [raw]
Subject: [PATCH v4 3/4] Sysctl: SROP Mitigation: Add Sysctl argument to disable SROP.

This patch adds a sysctl argument to disable SROP protection.

Cc: Abhiram Balasubramanian <[email protected]>
Signed-off-by: Scott Bauer <[email protected]>
Signed-off-by: Scott Bauer <[email protected]>
---
include/linux/signal.h | 1 +
kernel/signal.c | 13 +++++++++++--
kernel/sysctl.c | 8 ++++++++
3 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/include/linux/signal.h b/include/linux/signal.h
index fae0618..cd7f152 100644
--- a/include/linux/signal.h
+++ b/include/linux/signal.h
@@ -9,6 +9,7 @@ struct task_struct;

/* for sysctl */
extern int print_fatal_signals;
+extern int srop_disabled;
/*
* Real Time signals may be queued.
*/
diff --git a/kernel/signal.c b/kernel/signal.c
index 1e4f65c..fbe61d6 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -52,6 +52,7 @@
static struct kmem_cache *sigqueue_cachep;

int print_fatal_signals __read_mostly;
+int srop_disabled __read_mostly;

static void __user *sig_handler(struct task_struct *t, int sig)
{
@@ -2452,18 +2453,26 @@ int verify_clear_sigcookie(unsigned long __user *sig_cookie_ptr)
unsigned long user_cookie;
unsigned long calculated_cookie;

+ if (srop_disabled)
+ goto out;
+
if (get_user(user_cookie, sig_cookie_ptr))
return 1;

calculated_cookie = gen_sigcookie(sig_cookie_ptr);

if (user_cookie != calculated_cookie) {
- pr_warn("Signal protector does not match what kernel set it to"\
- ". Possible exploit attempt or buggy program!\n");
+ pr_warn("kernel/signal.c: Signal protector does not match what"\
+ " kernel set it to.\n" \
+ "Possible exploit attempt or buggy program!\nIf you"\
+ " believe this is an error you can disable SROP "\
+ " Protection by #echo 1 > /proc/sys/kernel/"\
+ "disable-srop-protection\n");
return 1;

}

+out:
user_cookie = 0;
return put_user(user_cookie, sig_cookie_ptr);
}
diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index 725587f..7886634 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -536,6 +536,14 @@ static struct ctl_table kern_table[] = {
.mode = 0644,
.proc_handler = proc_dointvec,
},
+ {
+ .procname = "disable-srop-protection",
+ .data = &srop_disabled,
+ .maxlen = sizeof(int),
+ .mode = 0644,
+ .proc_handler = proc_dointvec,
+
+ },
#ifdef CONFIG_SPARC
{
.procname = "reboot-cmd",
--
1.9.1

2016-03-29 19:59:09

by Andi Kleen

[permalink] [raw]
Subject: Re: [PATCH v4 3/4] Sysctl: SROP Mitigation: Add Sysctl argument to disable SROP.

On Tue, Mar 29, 2016 at 01:53:26PM -0600, Scott Bauer wrote:
> This patch adds a sysctl argument to disable SROP protection.

Sysctl needs to be documented in Documentation/sysctl/

Also negated sysctl is weird, normally they are positive (enable-xxx)

-Andi

2016-03-29 20:13:18

by Brian Gerst

[permalink] [raw]
Subject: Re: [PATCH v4 4/4] Documentation: SROP Mitigation: Add documentation for SROP cookies

On Tue, Mar 29, 2016 at 3:53 PM, Scott Bauer <[email protected]> wrote:
> This patch adds documentation and test code for SROP mitigation.
>
> Cc: Abhiram Balasubramanian <[email protected]>
> Signed-off-by: Scott Bauer <[email protected]>
> Signed-off-by: Scott Bauer <[email protected]>
> ---
> Documentation/security/srop-cookies.txt | 203 ++++++++++++++++++++++++++++++++
> 1 file changed, 203 insertions(+)
> create mode 100644 Documentation/security/srop-cookies.txt
>
> diff --git a/Documentation/security/srop-cookies.txt b/Documentation/security/srop-cookies.txt
> new file mode 100644
> index 0000000..ee17181
> --- /dev/null
> +++ b/Documentation/security/srop-cookies.txt
> @@ -0,0 +1,203 @@
> + Sigreturn-oriented programming and its mitigation
> +
> +
> +A good write up can be found here: https://lwn.net/Articles/676803/
> +It covers much of what is outlined in this documentation.
> +
> +If you're very curious you should read the SROP paper:
> +http://www.cs.vu.nl/~herbertb/papers/srop_sp14.pdf
> +
> +If you're here to learn how to disable SROP issue the following
> +Command:
> +
> +# echo 1 > /proc/sys/kernel/disable-srop-protection
> +
> +
> +============================ Overview ============================
> +
> +Sigreturn-oriented programming(SROP) is a new technique to control
> +a compromised userland process after successfully exploiting a bug.
> +
> +Signals are delivered to the process during context switches. The
> +kernel will setup a signal frame on the process' stack which
> +contains the saved state of the process prior to the context switch.
> +The kernel then gives control the the process' signal handler. Once
> +the process has delt with the signal it will call the sigreturn
> +system call. During the context switch into the kernel, the previous
> +state of where the process was executing prior to the delivery of
> +the signal is overwritten, hence why the kernel must save the the
> +state in a signal frame on the user's stack. The kernel will remove
> +the signal frame from the process' stack and continue execution
> +where the process left off.
> +
> +The issue with this signal delivery method is if an attacker can
> +construct a fake signal frame on the compromised process' stack
> +and ROP into the sigreturn system call, they have the ability to
> +easily control the flow of execution by having the kernel return
> +execution to wherever the signal frame says to continue execution.
> +
> +More importantly, SROP makes an attackers job easier. Previously
> +attackers would have to search for ROP gadgets to get values into
> +registers then call mprotect or mmap to get some memory where they
> +could copy and execute shellcode. If the ROP gadgets didnt exist
> +that allowed setting up a call to those functions then the attackers
> +wouldn't be able to fully exploit the bug. With SROP however attackers
> +dont' have to search for ROP gadgets to get specific values into
> +registers. The attacker would simply lay out the ucontext on the stack
> +as they choose then SROP into the mprotect or mmap call.
> +
> +======================== Mitigating SROP ========================
> +
> +In order to prevent SROP the kernel must remember or be able to derive
> +at runtime whether a sigreturn call it is currently processing is
> +in respose to a legitimate signal delivered by the kernel.
> +
> +During delivery of a signal the kernel will place the signal frame
> +with the saved process state on the stack. It will also place a cookie
> +above the signal frame.
> +
> +The cookie is a per-process secret xor'd with the address where it will
> +be stored. During a sigreturn the kernel will extract this cookie, and
> +then compare the extracted cookie against a new generated cookie:
> +(the per process secret xord with address where we extracted cookie from).
> +
> +If the two cookies match, then the kernel has verified that it is handling
> +a sigreturn from a signal that was previously legitimately delivered.
> +If the cookies do not match up the kernel sends a SIGSEGV to the process,
> +terminating it.
> +
> +After verifying the cookie, the kernel will zero out the cookie to prevent
> +any sort of leaking of the cookie.
> +
> +This prevents SROP because it forces an attacker to know the cookie in order
> +to use SROP as an attack vector.
> +
> +
> +======================== Possible Issues ========================
> +SROP protection will probably break any process or application which do
> +some sort of checkpoint-restore in user space type things. As well as DOSEMU.
> +
> +
> +
> +===============================================================================
> +Inlined are two programs that exploit SROP:
> +
> +For 32bit:
> +
> +Compile with if you're using a 64bit kernel:
> +gcc -O0 -o srop_32 srop_32.c -g -fno-stack-protector -ffixed-ebp -ffixed-esp -m32 -DEMULATED_32
> +
> +or if you're already on a real 32bit kernel:
> +gcc -O0 -o srop_32 srop_32.c -g -fno-stack-protector -ffixed-ebp -ffixed-esp
> +
> +When run without SROP protection it will exit gracefully, when SROP is
> +enabled it will terminate with a SIGSEGV.
> +
> +#include <stdlib.h>
> +#include <stdio.h>
> +#include <signal.h>
> +
> +void syscall(void)
> +{
> + exit(1);
> +}
> +
> +void test2(void)
> +{
> + register int esp asm("esp");
> + register int ebp asm("ebp");
> + struct sigcontext scon = { 0 };
> +
> + scon.eax = 11;
> + scon.ebx = 0x41;
> + scon.ecx = 0;
> + scon.edx = 0;
> + scon.esp = scon.ebp = ebp;
> + scon.eip = (int)syscall+6;
> +
> +#if defined EMULATED_32
> + scon.fs = 0x00;
> + scon.cs = 0x23;
> + scon.ss = scon.ds = scon.es = 0x2B;
> + scon.gs = 0x63;
> +#else
> + scon.fs = 0x00;
> + scon.cs = 0x73;
> + scon.ss = scon.ds = scon.es = 0x7B;
> + scon.gs = 0x33;
> +#endif
> + esp = (int) &scon;
> + asm("movl $119, %eax\n");//NR_SIGRETURN;
> + asm("int $0x80\n");
> +}
> +
> +int main(void)
> +{
> + test2();
> + return 1;
> +}
> +
> +=====================================================
> +
> +
> +For 64 bit:
> +
> +gcc -O0 -o srop srop.c -g -fno-stack-protector -ffixed-rsp -ffixed-rbp
> +When run the program exits normally, with SROP protetction it terminates
> +with a segmentationf fault.
> +
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <signal.h>
> +
> +enum {
> + REG_R8 = 0,
> + REG_R9,
> + REG_R10,
> + REG_R11,
> + REG_R12,
> + REG_R13,
> + REG_R14,
> + REG_R15,
> + REG_RDI,
> + REG_RSI,
> + REG_RBP,
> + REG_RBX,
> + REG_RDX,
> + REG_RAX,
> + REG_RCX,
> + REG_RSP,
> + REG_RIP,
> + REG_EFL,
> + REG_CSGSFS,/* Actually short cs, gs, fs, __pad0. */
> + REG_ERR,
> + REG_TRAPNO,
> + REG_OLDMASK,
> + REG_CR2
> +};
> +
> +void _exit_(void)
> +{
> + exit(1);
> +}
> +
> +void test(void)
> +{
> + struct ucontext ctx = { 0 };
> + register unsigned long rsp asm("rsp");
> + register unsigned long rbp asm("rbp");
> + ctx.uc_mcontext.gregs[REG_RIP] = (unsigned long) _exit_ + 4;
> + ctx.uc_mcontext.gregs[REG_RSP] = rsp;
> + ctx.uc_mcontext.gregs[REG_RBP] = rbp;
> + ctx.uc_mcontext.gregs[REG_CSGSFS] = 0x002b000000000033;
> + rsp = (unsigned long) &ctx;
> + asm("movq $0xf,%rax\n");
> + asm("syscall\n");
> +}
> +
> +
> +int main(void)
> +{
> + test();
> + return 0;
> +}
> --
> 1.9.1
>

These test programs should go in tools/testing/selftests/x86.

--
Brian Gerst

2016-03-29 20:46:25

by Scott Bauer

[permalink] [raw]
Subject: Re: [PATCH v4 3/4] Sysctl: SROP Mitigation: Add Sysctl argument to disable SROP.




On 03/29/2016 01:59 PM, Andi Kleen wrote:
> On Tue, Mar 29, 2016 at 01:53:26PM -0600, Scott Bauer wrote:
>> This patch adds a sysctl argument to disable SROP protection.
>
> Sysctl needs to be documented in Documentation/sysctl/
>
> Also negated sysctl is weird, normally they are positive (enable-xxx)
>

Sure, I can change it. This may be a dumb question: I want SROP to be enabled by default, and thus the new
enable-xxx will be initialized to 1, that's fine, right?

2016-03-29 20:53:25

by Andi Kleen

[permalink] [raw]
Subject: Re: [PATCH v4 3/4] Sysctl: SROP Mitigation: Add Sysctl argument to disable SROP.

On Tue, Mar 29, 2016 at 02:46:03PM -0600, Scotty Bauer wrote:
>
>
>
> On 03/29/2016 01:59 PM, Andi Kleen wrote:
> > On Tue, Mar 29, 2016 at 01:53:26PM -0600, Scott Bauer wrote:
> >> This patch adds a sysctl argument to disable SROP protection.
> >
> > Sysctl needs to be documented in Documentation/sysctl/
> >
> > Also negated sysctl is weird, normally they are positive (enable-xxx)
> >
>
> Sure, I can change it. This may be a dumb question: I want SROP to be enabled by default, and thus the new
> enable-xxx will be initialized to 1, that's fine, right?

Yes that's fine.

-Andi

--
[email protected] -- Speaking for myself only

2016-03-29 21:29:23

by Andy Lutomirski

[permalink] [raw]
Subject: Re: [PATCH v4 0/4] SROP Mitigation: Sigreturn Cookies

On Tue, Mar 29, 2016 at 12:53 PM, Scott Bauer <[email protected]> wrote:
> Sigreturn-oriented programming is a new attack vector in userland
> where an attacker crafts a fake signal frame on the stack and calls
> sigreturn. The kernel will extract the fake signal frame, which
> contains attacker controlled "saved" registers. The kernel will then
> transfer control to the attacker controlled userland instruction pointer.
>
> To prevent SROP attacks the kernel needs to know or be able to dervive
> whether a sigreturn it is processing is in response to a legitimate
> signal the kernel previously delivered.
>
> Further information and test code can be found in Documentation/security
> and this excellent article:
> http://lwn.net/Articles/676803/
>
> These patches implement the necessary changes to generate a cookie
> which will be placed above signal frame upon signal delivery to userland.
> The cookie is generated using a per-process random value xor'd with
> the address where the cookie will be stored on the stack.
>
> Upon a sigreturn the kernel will extract the cookie from userland,
> recalculate what the original cookie should be and verify that the two
> do not differ. If the two differ the kernel will terminate the process
> with a SIGSEGV.
>
> This prevents SROP by adding a value that the attacker cannot guess,
> but the kernel can verify. Therefore an attacker cannot use sigreturn as
> a method to control the flow of a process.
>

Has anyone verified that this doesn't break CRIU cross-machine (or
cross-boot) migration and that this doesn't break dosemu? You're
changing the ABI here.

--Andy

>
> Version changes:
>
> v3->v4
> Removed ambiguous __user annotation, added Documentation
> and test code.
>
> v2->v3
> Changed cookie calculation from using restored regs->sp to
> using frame pointer from before restoration.
>
> v1->v2
> Miscellaneous nits and code cleanup.
>
> Scott Bauer (4):
> SROP Mitigation: Architecture independent code for signal cookies
> x86: SROP Mitigation: Implement Signal Cookies
> Sysctl: SROP Mitigation: Add Sysctl argument to disable SROP.
> Documentation: SROP Mitigation: Add documentation for SROP cookies
>
>
> Documentation/security/srop-cookies.txt | 203 ++++++++++++++++++++++++++++++++
> arch/x86/ia32/ia32_signal.c | 65 +++++++++-
> arch/x86/include/asm/fpu/signal.h | 2 +
> arch/x86/kernel/fpu/signal.c | 10 ++
> arch/x86/kernel/signal.c | 83 +++++++++++--
> fs/exec.c | 3 +
> include/linux/sched.h | 7 ++
> include/linux/signal.h | 3 +
> kernel/signal.c | 49 ++++++++
> kernel/sysctl.c | 8 ++
> 10 files changed, 418 insertions(+), 15 deletions(-)
>



--
Andy Lutomirski
AMA Capital Management, LLC

2016-03-29 21:37:06

by Scott Bauer

[permalink] [raw]
Subject: Re: [PATCH v4 0/4] SROP Mitigation: Sigreturn Cookies



On 03/29/2016 03:29 PM, Andy Lutomirski wrote:
> On Tue, Mar 29, 2016 at 12:53 PM, Scott Bauer <[email protected]> wrote:
>> Sigreturn-oriented programming is a new attack vector in userland
>> where an attacker crafts a fake signal frame on the stack and calls
>> sigreturn. The kernel will extract the fake signal frame, which
>> contains attacker controlled "saved" registers. The kernel will then
>> transfer control to the attacker controlled userland instruction pointer.
>>
>> To prevent SROP attacks the kernel needs to know or be able to dervive
>> whether a sigreturn it is processing is in response to a legitimate
>> signal the kernel previously delivered.
>>
>> Further information and test code can be found in Documentation/security
>> and this excellent article:
>> http://lwn.net/Articles/676803/
>>
>> These patches implement the necessary changes to generate a cookie
>> which will be placed above signal frame upon signal delivery to userland.
>> The cookie is generated using a per-process random value xor'd with
>> the address where the cookie will be stored on the stack.
>>
>> Upon a sigreturn the kernel will extract the cookie from userland,
>> recalculate what the original cookie should be and verify that the two
>> do not differ. If the two differ the kernel will terminate the process
>> with a SIGSEGV.
>>
>> This prevents SROP by adding a value that the attacker cannot guess,
>> but the kernel can verify. Therefore an attacker cannot use sigreturn as
>> a method to control the flow of a process.
>>
>
> Has anyone verified that this doesn't break CRIU cross-machine (or
> cross-boot) migration and that this doesn't break dosemu? You're
> changing the ABI here.
>

I haven't yet I'll do that to verify it breaks -- I'm pretty sure under some
conditions it will break CRIU. That's why we added the sysctl to turn it off.
Should I have mentioned this in the main commit that it possibly breaks CRIU/DOSEMU?
I went ahead and added that to the Documentation.


2016-03-29 21:39:03

by Andy Lutomirski

[permalink] [raw]
Subject: Re: [PATCH v4 0/4] SROP Mitigation: Sigreturn Cookies

On Tue, Mar 29, 2016 at 2:36 PM, Scotty Bauer <[email protected]> wrote:
>
>
> On 03/29/2016 03:29 PM, Andy Lutomirski wrote:
>> On Tue, Mar 29, 2016 at 12:53 PM, Scott Bauer <[email protected]> wrote:
>>> Sigreturn-oriented programming is a new attack vector in userland
>>> where an attacker crafts a fake signal frame on the stack and calls
>>> sigreturn. The kernel will extract the fake signal frame, which
>>> contains attacker controlled "saved" registers. The kernel will then
>>> transfer control to the attacker controlled userland instruction pointer.
>>>
>>> To prevent SROP attacks the kernel needs to know or be able to dervive
>>> whether a sigreturn it is processing is in response to a legitimate
>>> signal the kernel previously delivered.
>>>
>>> Further information and test code can be found in Documentation/security
>>> and this excellent article:
>>> http://lwn.net/Articles/676803/
>>>
>>> These patches implement the necessary changes to generate a cookie
>>> which will be placed above signal frame upon signal delivery to userland.
>>> The cookie is generated using a per-process random value xor'd with
>>> the address where the cookie will be stored on the stack.
>>>
>>> Upon a sigreturn the kernel will extract the cookie from userland,
>>> recalculate what the original cookie should be and verify that the two
>>> do not differ. If the two differ the kernel will terminate the process
>>> with a SIGSEGV.
>>>
>>> This prevents SROP by adding a value that the attacker cannot guess,
>>> but the kernel can verify. Therefore an attacker cannot use sigreturn as
>>> a method to control the flow of a process.
>>>
>>
>> Has anyone verified that this doesn't break CRIU cross-machine (or
>> cross-boot) migration and that this doesn't break dosemu? You're
>> changing the ABI here.
>>
>
> I haven't yet I'll do that to verify it breaks -- I'm pretty sure under some
> conditions it will break CRIU. That's why we added the sysctl to turn it off.
> Should I have mentioned this in the main commit that it possibly breaks CRIU/DOSEMU?
> I went ahead and added that to the Documentation.
>
>

Then there's an unanswered question: is this patch acceptable given
that it's an ABI break? Security fixes are sometimes an exception to
the "no ABI breaks" rule, but it's by no means an automatic exception.

--Andy

2016-03-29 22:34:38

by Linus Torvalds

[permalink] [raw]
Subject: Re: [PATCH v4 0/4] SROP Mitigation: Sigreturn Cookies

On Tue, Mar 29, 2016 at 4:38 PM, Andy Lutomirski <[email protected]> wrote:
>
> Then there's an unanswered question: is this patch acceptable given
> that it's an ABI break? Security fixes are sometimes an exception to
> the "no ABI breaks" rule, but it's by no means an automatic exception.

So there isn't any "no ABI break" rule - there is only a "does it
break real applications" rule.

(This can also be re-stated as: "Talk is cheap", aka "reality trumps
documentation".

Documentation is meaningless if it doesn't match reality, and what we
actually *do* is what matters.

So the ABI isn't about some theoretical interface documentation, the
ABI is about what people use and have tested.

On the one hand, that means that that our ABI is _stricter_ than any
documentatiuon, and that "but we can make this change that breaks app
XYZ, because XYZ is depending on undocumented behavior" is not an
acceptable excuse.

But on the other hand it *also* means that since the ABI is about real
programs, not theoretical issues, we can also change things as long as
we don't actually break anything that people can notice and depend
on).

And while *acute* security holes will be fixed regardless of ABI
issues, something like this that is only hardening rather than fixing
a particular security hole, really needs to not break any
applications.

Because if it does break anything, it needs to be turned off by
default. That's a hard rule. And since that would be largely defeating
the whole point o fthe series, I think we really need to have made
sure nothing breaks before a patch series like this can be accepted.

That said, if this is done right, I don't think it will break
anything. CRIU may indeed be a special case, but CRIU isn't really a
normal application, and the CRIU people may need to turn this off
explicitly, if it does break.

But yes, dosemu needs to be tested, and needs to just continue
working. But does dosemu actually create a signal stack, as opposed to
just playing with one that has been created for it? I thought it was
just the latter case, which should be ok even with a magic cookie in
there.

Linus

2016-03-29 22:54:27

by Linus Torvalds

[permalink] [raw]
Subject: Re: [PATCH v4 0/4] SROP Mitigation: Sigreturn Cookies

On Tue, Mar 29, 2016 at 2:53 PM, Scott Bauer <[email protected]> wrote:
>
> These patches implement the necessary changes to generate a cookie
> which will be placed above signal frame upon signal delivery to userland.
> The cookie is generated using a per-process random value xor'd with
> the address where the cookie will be stored on the stack.

Side note: wouldn't it be better to make the cookie something that
doesn't make it trivial to figure out the random value in case you
already have access to a signal stack?

Maybe there could be a stronger variation of this that makes the
cookie be something like a single md5 round (not a full md5).
Something fast, and not necessarily secure, but something that needs
more than one single CPU instruction to figure out.

So you could do 4 32

- the random value
- the low 32 bits of the address of the cookie
- the low 32 bits of the return point stack and instruction pointer

Yes, yes, md5 is not cryptographically secure, and making it a single
iteration rather than the full four makes it even less so, but if the
attacker can generate long arbitrary code, then the whole SROP is
pointless to begin with, no?

In contrast, with the plain xor, the SROP would be a trivial operation
if you can just force it to happen within the context of a signal, so
that you can just re-use the signal return stack as-is. But mixing in
the returning IP and SP would make it *much* harder to use the
sigreturn as an attack vector.

I realize that this would likely need to be a separate and non-default
extra hardening mode, because there are *definitely* applications that
take signals and then update the return address (maybe single-stepping
over instructions etc). But for a *lot* of applications, signal return
implies changing no signal state at all, and mixing in the returning
IP and SP would seem to be a fundamentally stronger cookie.

No?

Linus

2016-03-29 22:55:21

by Daniel Micay

[permalink] [raw]
Subject: Re: [kernel-hardening] Re: [PATCH v4 0/4] SROP Mitigation: Sigreturn Cookies

> Then there's an unanswered question: is this patch acceptable given
> that it's an ABI break?  Security fixes are sometimes an exception to
> the "no ABI breaks" rule, but it's by no means an automatic exception.
>
> --Andy

It seems this could be worked around in general. Processes can have a
bit tracking whether this is enabled, and CRIU can save/restore it. It
would just leave it off for resuming old saved processes.

Should CRIU really be covered by the kernel's ABI guarantee though? It
seems like this was meant to be extensible, so it's adding an extra ABI
guarantee that wasn't there before. It makes sense to freeze this ABI
for CRIU, but a version field should be added first in one final ABI
break if it's not too late.


Attachments:
signature.asc (819.00 B)
This is a digitally signed message part

2016-03-29 22:55:43

by Linus Torvalds

[permalink] [raw]
Subject: Re: [PATCH v4 0/4] SROP Mitigation: Sigreturn Cookies

On Tue, Mar 29, 2016 at 5:54 PM, Linus Torvalds
<[email protected]> wrote:
>
> So you could do 4 32
>
> - the random value
> - the low 32 bits of the address of the cookie
> - the low 32 bits of the return point stack and instruction pointer

Oops, editing mishap. That was supposed to be about the 128-bit md5
chunk, which uses 4 32-bit values, but then I edited things and didn't
get back to it.

Linus

2016-03-29 23:04:25

by Linus Torvalds

[permalink] [raw]
Subject: Re: [PATCH v4 1/4] SROP Mitigation: Architecture independent code for signal cookies

On Tue, Mar 29, 2016 at 2:53 PM, Scott Bauer <[email protected]> wrote:
> @@ -1231,6 +1232,8 @@ void setup_new_exec(struct linux_binprm * bprm)
> /* This is the point of no return */
> current->sas_ss_sp = current->sas_ss_size = 0;
>
> + get_random_bytes(&current->sig_cookie, sizeof(current->sig_cookie));
> +

This should probably just be

current->sig_cookie = get_random_long();

instead. That will use hardware random numbers if available, and be
*much* faster.

I realize that some people don't like the hardware random number
generators because they don't trust them, but quite frankly, for
something like this it's fine. If the attacker is in collusion with
the hardware manufacturer, you have way bigger problems than a SROP
attack.

Linus

2016-03-29 23:05:28

by Andy Lutomirski

[permalink] [raw]
Subject: Re: [PATCH v4 0/4] SROP Mitigation: Sigreturn Cookies

On Tue, Mar 29, 2016 at 3:54 PM, Linus Torvalds
<[email protected]> wrote:
> On Tue, Mar 29, 2016 at 2:53 PM, Scott Bauer <[email protected]> wrote:
>>
>> These patches implement the necessary changes to generate a cookie
>> which will be placed above signal frame upon signal delivery to userland.
>> The cookie is generated using a per-process random value xor'd with
>> the address where the cookie will be stored on the stack.
>

> I realize that this would likely need to be a separate and non-default
> extra hardening mode, because there are *definitely* applications that
> take signals and then update the return address (maybe single-stepping
> over instructions etc). But for a *lot* of applications, signal return
> implies changing no signal state at all, and mixing in the returning
> IP and SP would seem to be a fundamentally stronger cookie.

Like selftests/x86? :)

If we wanted to increase confidence that this wouldn't break existing
applications, I've been thinking about adding an extensible bit mask
of backwards compatibility breaks that an and/or libc is okay with.
One of these would be "I don't use vsyscalls", in which case the
vsyscall page would be unmapped entirely. Another could be
"sigcontext cookies are okay". These could potentially be programmed
by syscall and/or ELF notes.

--Andy

2016-03-29 23:11:25

by Scott Bauer

[permalink] [raw]
Subject: Re: [PATCH v4 0/4] SROP Mitigation: Sigreturn Cookies



On 03/29/2016 04:54 PM, Linus Torvalds wrote:
> On Tue, Mar 29, 2016 at 2:53 PM, Scott Bauer <[email protected]> wrote:
>>
>> These patches implement the necessary changes to generate a cookie
>> which will be placed above signal frame upon signal delivery to userland.
>> The cookie is generated using a per-process random value xor'd with
>> the address where the cookie will be stored on the stack.
>
> Side note: wouldn't it be better to make the cookie something that
> doesn't make it trivial to figure out the random value in case you
> already have access to a signal stack?
>
> Maybe there could be a stronger variation of this that makes the
> cookie be something like a single md5 round (not a full md5).
> Something fast, and not necessarily secure, but something that needs
> more than one single CPU instruction to figure out.
>
> So you could do 4 32
>
> - the random value
> - the low 32 bits of the address of the cookie
> - the low 32 bits of the return point stack and instruction pointer
>
> Yes, yes, md5 is not cryptographically secure, and making it a single
> iteration rather than the full four makes it even less so, but if the
> attacker can generate long arbitrary code, then the whole SROP is
> pointless to begin with, no?
>

Yeah I had toyed with using hashes, I used hash_64 not md5 which is like 14
extra instructions or something. Anyway Daniel Micay pointed out we could use SipHash
https://131002.net/siphash/, but there's no siphash for me to use in the kernel
and I'm the *last* person on earth to start porting/implementing 'crypto' algos.

Anyway, we all sort of agreed that if you have enough arbitrary execution already
to cause a signal, leak the cookie, do some xor magic to get the per-process
secret then you probably don't really need to SROP in your exploit. Although
you did mention an interesting attack which is force a signal then muck with
an existing legitimate frame, which I would like to protect against now.

> In contrast, with the plain xor, the SROP would be a trivial operation
> if you can just force it to happen within the context of a signal, so
> that you can just re-use the signal return stack as-is. But mixing in
> the returning IP and SP would make it *much* harder to use the
> sigreturn as an attack vector.
>
> I realize that this would likely need to be a separate and non-default
> extra hardening mode, because there are *definitely* applications that
> take signals and then update the return address (maybe single-stepping
> over instructions etc). But for a *lot* of applications, signal return
> implies changing no signal state at all, and mixing in the returning
> IP and SP would seem to be a fundamentally stronger cookie.
>
> No?

It's not hard to implement So I can try it. When you say an extra hardening
mode do you mean hide it behind a sysctl or some sort of compile time CONFIG?

2016-03-29 23:15:02

by Scott Bauer

[permalink] [raw]
Subject: Re: [PATCH v4 0/4] SROP Mitigation: Sigreturn Cookies



On 03/29/2016 04:34 PM, Linus Torvalds wrote:
> On Tue, Mar 29, 2016 at 4:38 PM, Andy Lutomirski <[email protected]> wrote:
>>
>> Then there's an unanswered question: is this patch acceptable given
>> that it's an ABI break? Security fixes are sometimes an exception to
>> the "no ABI breaks" rule, but it's by no means an automatic exception.
>
> So there isn't any "no ABI break" rule - there is only a "does it
> break real applications" rule.
>
> (This can also be re-stated as: "Talk is cheap", aka "reality trumps
> documentation".
>
> Documentation is meaningless if it doesn't match reality, and what we
> actually *do* is what matters.
>
> So the ABI isn't about some theoretical interface documentation, the
> ABI is about what people use and have tested.
>
> On the one hand, that means that that our ABI is _stricter_ than any
> documentatiuon, and that "but we can make this change that breaks app
> XYZ, because XYZ is depending on undocumented behavior" is not an
> acceptable excuse.
>
> But on the other hand it *also* means that since the ABI is about real
> programs, not theoretical issues, we can also change things as long as
> we don't actually break anything that people can notice and depend
> on).
>
> And while *acute* security holes will be fixed regardless of ABI
> issues, something like this that is only hardening rather than fixing
> a particular security hole, really needs to not break any
> applications.
>
> Because if it does break anything, it needs to be turned off by
> default. That's a hard rule. And since that would be largely defeating
> the whole point o fthe series, I think we really need to have made
> sure nothing breaks before a patch series like this can be accepted.
>
> That said, if this is done right, I don't think it will break
> anything. CRIU may indeed be a special case, but CRIU isn't really a
> normal application, and the CRIU people may need to turn this off
> explicitly, if it does break.
>
> But yes, dosemu needs to be tested, and needs to just continue
> working. But does dosemu actually create a signal stack, as opposed to
> just playing with one that has been created for it? I thought it was
> just the latter case, which should be ok even with a magic cookie in
> there.
>
> Linus
>


For what it's worth this series is breaking CRIU, I just tested:

root@node0:/mnt/criu# criu restore -vvvv -o restore.log --shell-job
root@node0:/mnt/criu# tail -3 /var/log/syslog
Mar 29 17:12:08 localhost kernel: [ 3554.625535] Possible exploit attempt or buggy program!
Mar 29 17:12:08 localhost kernel: [ 3554.625535] If you believe this is an error you can disable SROP Protection by #echo 1 > /proc/sys/kernel/disable-srop-protection
Mar 29 17:12:08 localhost kernel: [ 3554.625545] test_[25305] bad frame in rt_sigreturn frame:000000000001e540 ip:7f561542cf20 sp:7ffe004ecfd8 orax:ffffffffffffffff in libc-2.19.so[7f561536c000+1bb0]
root@node0:/mnt/criu# echo 1 > /proc/sys/kernel/disable-srop-protection
root@node0:/mnt/criu# criu restore -vvvv -o restore.log --shell-job
slept for one second
slept for one second
slept for one second
slept for one second
root@node0:/mnt/criu#


I'm working on getting dosemu up and running-- are there any other applications
off the top of your head that I should be testing with?



2016-03-29 23:25:29

by Linus Torvalds

[permalink] [raw]
Subject: Re: [PATCH v4 0/4] SROP Mitigation: Sigreturn Cookies

On Tue, Mar 29, 2016 at 6:11 PM, Scotty Bauer <[email protected]> wrote:
>
> Yeah I had toyed with using hashes, I used hash_64 not md5 which is like 14
> extra instructions or something.

That sounds fine. Anything that requires enough code to undo that it
kind of defeats the purpose of a SROP should be enough. It's not about
encryption, I'd just think that if you can force the buffer overflow
while already in a signal handler, you'd want something that is at
least *slightly* harder to defeat than a single "xor" instruction.

> It's not hard to implement So I can try it. When you say an extra hardening
> mode do you mean hide it behind a sysctl or some sort of compile time CONFIG?

Since there already is a sysctl, I'd just assume that.

The important part is that the *default* value for that sysctl can't
break real applications. I don't really count CRIU as a real app, if
only because once you start doing checkpoint-restore you are going to
do some amount of system maintenance anyway, so somebody doing CRIU is
kind of expected to have a certain amount of system expertise, I would
say.

But dosemu - or Wine - is very much something that "normal people" run
- people who we do *not* expect to have to know about new sysctl's
etc. They already have one (mmap at zero), but that is very directly
related to what vm86 mode and Wine does, and people have had time to
learn about it. Let's not add another.

So testing dosemu and wine would be good. I wonder what else has shown
issues with signal stack layout changes. Debuggers and some JIT
engines, I suspect.

Linus

2016-03-29 23:34:10

by Scott Bauer

[permalink] [raw]
Subject: Re: [PATCH v4 0/4] SROP Mitigation: Sigreturn Cookies



On 03/29/2016 05:25 PM, Linus Torvalds wrote:
> On Tue, Mar 29, 2016 at 6:11 PM, Scotty Bauer <[email protected]> wrote:
>>
>> Yeah I had toyed with using hashes, I used hash_64 not md5 which is like 14
>> extra instructions or something.
>
> That sounds fine. Anything that requires enough code to undo that it
> kind of defeats the purpose of a SROP should be enough. It's not about
> encryption, I'd just think that if you can force the buffer overflow
> while already in a signal handler, you'd want something that is at
> least *slightly* harder to defeat than a single "xor" instruction.
>
>> It's not hard to implement So I can try it. When you say an extra hardening
>> mode do you mean hide it behind a sysctl or some sort of compile time CONFIG?
>
> Since there already is a sysctl, I'd just assume that.
>
> The important part is that the *default* value for that sysctl can't
> break real applications. I don't really count CRIU as a real app, if
> only because once you start doing checkpoint-restore you are going to
> do some amount of system maintenance anyway, so somebody doing CRIU is
> kind of expected to have a certain amount of system expertise, I would
> say.
>
> But dosemu - or Wine - is very much something that "normal people" run
> - people who we do *not* expect to have to know about new sysctl's
> etc. They already have one (mmap at zero), but that is very directly
> related to what vm86 mode and Wine does, and people have had time to
> learn about it. Let's not add another.
>
> So testing dosemu and wine would be good. I wonder what else has shown
> issues with signal stack layout changes. Debuggers and some JIT
> engines, I suspect.
>
> Linus
>


Alright I'll test Wine/Mono, Dosemu, some random languages/debuggers see if
there is anything that breaks.

Thanks.


2016-03-31 20:32:53

by Eric W. Biederman

[permalink] [raw]
Subject: Re: [PATCH v4 0/4] SROP Mitigation: Sigreturn Cookies


Cc' the Criu list to attempt to give them a heads up.

Scotty Bauer <[email protected]> writes:

> On 03/29/2016 04:34 PM, Linus Torvalds wrote:
>> On Tue, Mar 29, 2016 at 4:38 PM, Andy Lutomirski <[email protected]> wrote:
>>>
>>> Then there's an unanswered question: is this patch acceptable given
>>> that it's an ABI break? Security fixes are sometimes an exception to
>>> the "no ABI breaks" rule, but it's by no means an automatic exception.
>>
>> So there isn't any "no ABI break" rule - there is only a "does it
>> break real applications" rule.
>>
>> (This can also be re-stated as: "Talk is cheap", aka "reality trumps
>> documentation".
>>
>> Documentation is meaningless if it doesn't match reality, and what we
>> actually *do* is what matters.
>>
>> So the ABI isn't about some theoretical interface documentation, the
>> ABI is about what people use and have tested.
>>
>> On the one hand, that means that that our ABI is _stricter_ than any
>> documentatiuon, and that "but we can make this change that breaks app
>> XYZ, because XYZ is depending on undocumented behavior" is not an
>> acceptable excuse.
>>
>> But on the other hand it *also* means that since the ABI is about real
>> programs, not theoretical issues, we can also change things as long as
>> we don't actually break anything that people can notice and depend
>> on).
>>
>> And while *acute* security holes will be fixed regardless of ABI
>> issues, something like this that is only hardening rather than fixing
>> a particular security hole, really needs to not break any
>> applications.
>>
>> Because if it does break anything, it needs to be turned off by
>> default. That's a hard rule. And since that would be largely defeating
>> the whole point o fthe series, I think we really need to have made
>> sure nothing breaks before a patch series like this can be accepted.
>>
>> That said, if this is done right, I don't think it will break
>> anything. CRIU may indeed be a special case, but CRIU isn't really a
>> normal application, and the CRIU people may need to turn this off
>> explicitly, if it does break.
>>
>> But yes, dosemu needs to be tested, and needs to just continue
>> working. But does dosemu actually create a signal stack, as opposed to
>> just playing with one that has been created for it? I thought it was
>> just the latter case, which should be ok even with a magic cookie in
>> there.
>>
>> Linus
>>
>
>
> For what it's worth this series is breaking CRIU, I just tested:
>
> root@node0:/mnt/criu# criu restore -vvvv -o restore.log --shell-job
> root@node0:/mnt/criu# tail -3 /var/log/syslog
> Mar 29 17:12:08 localhost kernel: [ 3554.625535] Possible exploit attempt or buggy program!
> Mar 29 17:12:08 localhost kernel: [ 3554.625535] If you believe this is an error you can disable SROP Protection by #echo 1 > /proc/sys/kernel/disable-srop-protection
> Mar 29 17:12:08 localhost kernel: [ 3554.625545] test_[25305] bad frame in rt_sigreturn frame:000000000001e540 ip:7f561542cf20 sp:7ffe004ecfd8 orax:ffffffffffffffff in libc-2.19.so[7f561536c000+1bb0]
> root@node0:/mnt/criu# echo 1 > /proc/sys/kernel/disable-srop-protection
> root@node0:/mnt/criu# criu restore -vvvv -o restore.log --shell-job
> slept for one second
> slept for one second
> slept for one second
> slept for one second
> root@node0:/mnt/criu#

Which means that if checkpoint/restart is going to continue to be
something that is possible in Linux it should be possible to
save/restore the per process sig_cookie. Perhaps with a prctl?

This should be addressed as part of this patchset as making that
information too easily accessible/changable will defeat the security
guarantees. Making it too difficult to do destroys the ability to
migrate a process from one kernel to another. As the existence of CRIU
attests it is desirable to have a checkpoint/restart capability in the
kernel.

> I'm working on getting dosemu up and running-- are there any other applications
> off the top of your head that I should be testing with?

There are a set of POSIX functions setcontext, getcontext, makecontext
and swapcontext that to the best of my knowledge deal in signal stacks.
Although I don't know that they use sigreturn. Anything that makes use
of those is potentially affected.

Perhaps you can find binaries that care by looking for libraries and
executables that import those elf symbols. glibc certainly provides
them.


Eric

2016-03-31 20:35:35

by Eric W. Biederman

[permalink] [raw]
Subject: Re: [PATCH v4 1/4] SROP Mitigation: Architecture independent code for signal cookies

Scott Bauer <[email protected]> writes:

> This patch adds a per-process secret to the task struct which
> will be used during signal delivery and during a sigreturn.
> Also, logic is added in signal.c to generate, place, extract,
> clear and verify the signal cookie.
>
> Cc: Abhiram Balasubramanian <[email protected]>
> Signed-off-by: Scott Bauer <[email protected]>
> Signed-off-by: Scott Bauer <[email protected]>

> diff --git a/include/linux/sched.h b/include/linux/sched.h
> index 60bba7e..1828fb8 100644
> --- a/include/linux/sched.h
> +++ b/include/linux/sched.h
> @@ -1502,6 +1502,13 @@ struct task_struct {
> unsigned long stack_canary;
> #endif
> /*
> + * Canary value for signal frames placed on user stack.
> + * This helps mitigate "Signal Return oriented program"
> + * exploits in userland.
> + */
> + unsigned long sig_cookie;


I suspect we want this to be a per-mm attribute rather than a per-thread
attribute.

Otherwise you are breaking anything that uses a N-M threading model.
Which I suspect means that this implementation choice breaks all go
programs on linux.

> + /*
> * pointers to (original) parent process, youngest child, younger sibling,
> * older sibling, respectively. (p->father can be replaced with
> * p->real_parent->pid)

Eric

2016-03-31 22:00:37

by Linus Torvalds

[permalink] [raw]
Subject: Re: [PATCH v4 1/4] SROP Mitigation: Architecture independent code for signal cookies

On Thu, Mar 31, 2016 at 3:25 PM, Eric W. Biederman
<[email protected]> wrote:
>
> I suspect we want this to be a per-mm attribute rather than a per-thread
> attribute.
>
> Otherwise you are breaking anything that uses a N-M threading model.
> Which I suspect means that this implementation choice breaks all go
> programs on linux.

That sounds like a good point, but wouldn't it make more conceptual
sense to make it part of "struct sighand_struct" instead?

That is also shared for threads.

Linus

2016-03-31 22:28:08

by Eric W. Biederman

[permalink] [raw]
Subject: Re: [PATCH v4 1/4] SROP Mitigation: Architecture independent code for signal cookies

Linus Torvalds <[email protected]> writes:

> On Thu, Mar 31, 2016 at 3:25 PM, Eric W. Biederman
> <[email protected]> wrote:
>>
>> I suspect we want this to be a per-mm attribute rather than a per-thread
>> attribute.
>>
>> Otherwise you are breaking anything that uses a N-M threading model.
>> Which I suspect means that this implementation choice breaks all go
>> programs on linux.
>
> That sounds like a good point, but wouldn't it make more conceptual
> sense to make it part of "struct sighand_struct" instead?
>
> That is also shared for threads.

Good point. Given this is a signal handling feature
struct sighand_struct is the natural place to put this.

Eric

2016-04-01 12:57:54

by Cyrill Gorcunov

[permalink] [raw]
Subject: Re: [PATCH v4 0/4] SROP Mitigation: Sigreturn Cookies

On Thu, Mar 31, 2016 at 03:22:38PM -0500, Eric W. Biederman wrote:
>
> Cc' the Criu list to attempt to give them a heads up.

Thanks Eric! I managed to miss this thread (I try to scan
lkml descussions one a day in my inbox, but this one somehow
escaped, thank you!)

> Scotty Bauer <[email protected]> writes:
...
> >> Because if it does break anything, it needs to be turned off by
> >> default. That's a hard rule. And since that would be largely defeating
> >> the whole point o fthe series, I think we really need to have made
> >> sure nothing breaks before a patch series like this can be accepted.
> >>
> >> That said, if this is done right, I don't think it will break
> >> anything. CRIU may indeed be a special case, but CRIU isn't really a
> >> normal application, and the CRIU people may need to turn this off
> >> explicitly, if it does break.
> >>
> >> But yes, dosemu needs to be tested, and needs to just continue
> >> working. But does dosemu actually create a signal stack, as opposed to
> >> just playing with one that has been created for it? I thought it was
> >> just the latter case, which should be ok even with a magic cookie in
> >> there.
...
> > For what it's worth this series is breaking CRIU, I just tested:
> >
> > root@node0:/mnt/criu# criu restore -vvvv -o restore.log --shell-job
> > root@node0:/mnt/criu# tail -3 /var/log/syslog
> > Mar 29 17:12:08 localhost kernel: [ 3554.625535] Possible exploit attempt or buggy program!
> > Mar 29 17:12:08 localhost kernel: [ 3554.625535] If you believe this is an error you can disable SROP Protection by #echo 1 > /proc/sys/kernel/disable-srop-protection
> > Mar 29 17:12:08 localhost kernel: [ 3554.625545] test_[25305] bad frame in rt_sigreturn frame:000000000001e540 ip:7f561542cf20 sp:7ffe004ecfd8 orax:ffffffffffffffff in libc-2.19.so[7f561536c000+1bb0]
> > root@node0:/mnt/criu# echo 1 > /proc/sys/kernel/disable-srop-protection
> > root@node0:/mnt/criu# criu restore -vvvv -o restore.log --shell-job
> > slept for one second
> > slept for one second
> > slept for one second
> > slept for one second
> > root@node0:/mnt/criu#
>
> Which means that if checkpoint/restart is going to continue to be
> something that is possible in Linux it should be possible to
> save/restore the per process sig_cookie. Perhaps with a prctl?

Yes please. Currently (together with other aims) we're trying to
remove "root-only" requirement from criu, so user would be able
to c/r non-privileged processes without sudo/su. Thus I presume
such prctl will be cap-sysadmin only and we will have to run some
suid'ed daemon for it or something.

> This should be addressed as part of this patchset as making that
> information too easily accessible/changable will defeat the security
> guarantees. Making it too difficult to do destroys the ability to
> migrate a process from one kernel to another. As the existence of CRIU
> attests it is desirable to have a checkpoint/restart capability in the
> kernel.

To change sigframe an attacked process must have had some code
already injected and this cookie guard will help but not _that_
much I think.

> > I'm working on getting dosemu up and running-- are there any other applications
> > off the top of your head that I should be testing with?
>
> There are a set of POSIX functions setcontext, getcontext, makecontext
> and swapcontext that to the best of my knowledge deal in signal stacks.
> Although I don't know that they use sigreturn. Anything that makes use
> of those is potentially affected.
>
> Perhaps you can find binaries that care by looking for libraries and
> executables that import those elf symbols. glibc certainly provides
> them.

Cyrill

2016-04-24 16:14:45

by Pavel Machek

[permalink] [raw]
Subject: Re: [PATCH v4 0/4] SROP Mitigation: Sigreturn Cookies

Hi!

> > To prevent SROP attacks the kernel needs to know or be able to dervive

derive?


--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

2016-04-24 16:27:38

by Pavel Machek

[permalink] [raw]
Subject: Re: [PATCH v4 4/4] Documentation: SROP Mitigation: Add documentation for SROP cookies

Hi!

> index 0000000..ee17181
> --- /dev/null
> +++ b/Documentation/security/srop-cookies.txt
> @@ -0,0 +1,203 @@
> + Sigreturn-oriented programming and its mitigation
> +
> +
> +A good write up can be found here: https://lwn.net/Articles/676803/
> +It covers much of what is outlined in this documentation.

Umm. I'd rather have good documentation here than on the web
somewhere.

> +If you're very curious you should read the SROP paper:
> +http://www.cs.vu.nl/~herbertb/papers/srop_sp14.pdf
> +
> +If you're here to learn how to disable SROP issue the following

", issue"?

> +Command:
> +
> +# echo 1 > /proc/sys/kernel/disable-srop-protection

Can we have reverse logic, having '.../srop-protection' file? Double
negatives are not intuitive...

> +============================ Overview ============================
> +
> +Sigreturn-oriented programming(SROP) is a new technique to control
> +a compromised userland process after successfully exploiting a bug.
> +
> +Signals are delivered to the process during context switches. The
> +kernel will setup a signal frame on the process' stack which
> +contains the saved state of the process prior to the context switch.
> +The kernel then gives control the the process' signal handler. Once
> +the process has delt with the signal it will call the sigreturn

dealt

> +system call. During the context switch into the kernel, the previous
> +state of where the process was executing prior to the delivery of
> +the signal is overwritten, hence why the kernel must save the the

hence why? the the?

> +state in a signal frame on the user's stack. The kernel will remove
> +the signal frame from the process' stack and continue execution
> +where the process left off.
> +
> +The issue with this signal delivery method is if an attacker can

"is: if"?

> +construct a fake signal frame on the compromised process' stack
> +and ROP into the sigreturn system call, they have the ability to
> +easily control the flow of execution by having the kernel return
> +execution to wherever the signal frame says to continue execution.
> +
> +More importantly, SROP makes an attackers job easier. Previously

attacker's?

> +attackers would have to search for ROP gadgets to get values into
> +registers then call mprotect or mmap to get some memory where they
> +could copy and execute shellcode. If the ROP gadgets didnt exist

didn't

> +that allowed setting up a call to those functions then the attackers
> +wouldn't be able to fully exploit the bug. With SROP however attackers

, however,

> +dont' have to search for ROP gadgets to get specific values into

don't

> +registers. The attacker would simply lay out the ucontext on the stack
> +as they choose then SROP into the mprotect or mmap call.

choose,

> +======================== Mitigating SROP ========================
> +
> +In order to prevent SROP the kernel must remember or be able to derive
> +at runtime whether a sigreturn call it is currently processing is
> +in respose to a legitimate signal delivered by the kernel.

response.

> +During delivery of a signal the kernel will place the signal frame

, the kernel

> +with the saved process state on the stack. It will also place a cookie
> +above the signal frame.
> +
> +The cookie is a per-process secret xor'd with the address where it will
> +be stored. During a sigreturn the kernel will extract this cookie, and
> +then compare the extracted cookie against a new generated cookie:
> +(the per process secret xord with address where we extracted cookie from).

kill the ()s?

> +If the two cookies match, then the kernel has verified that it is handling
> +a sigreturn from a signal that was previously legitimately delivered.
> +If the cookies do not match up the kernel sends a SIGSEGV to the process,
> +terminating it.

up, the kernel.

Hmm. SIGSEGV does not neccessarily terminate a process, right? Which
register values will it use for the SIGSEGV handler?

> +After verifying the cookie, the kernel will zero out the cookie to prevent
> +any sort of leaking of the cookie.

delete 'any sort', because we know that cookie can leak, for example
due to threads sharing the mm?

> +This prevents SROP because it forces an attacker to know the cookie in order
> +to use SROP as an attack vector.

-> This makes SROP harder... ?

> +
> +
> +======================== Possible Issues ========================

you may want to search for " " in this document.

> +SROP protection will probably break any process or application which do

does.

> +some sort of checkpoint-restore in user space type things. As well as DOSEMU.
> +
> +
> +
> +===============================================================================
> +Inlined are two programs that exploit SROP:
> +
> +For 32bit:
> +
> +Compile with if you're using a 64bit kernel:
> +gcc -O0 -o srop_32 srop_32.c -g -fno-stack-protector -ffixed-ebp -ffixed-esp -m32 -DEMULATED_32
> +
> +or if you're already on a real 32bit kernel:
> +gcc -O0 -o srop_32 srop_32.c -g -fno-stack-protector -ffixed-ebp -ffixed-esp
> +

this should go to tools/ somewhere, no?

> +When run without SROP protection it will exit gracefully, when SROP is
> +enabled it will terminate with a SIGSEGV.
> +
> +#include <stdlib.h>
> +#include <stdio.h>
> +#include <signal.h>
> +
> +void syscall(void)
> +{
> + exit(1);
> +}
> +
> +void test2(void)
> +{
> + register int esp asm("esp");
> + register int ebp asm("ebp");
> + struct sigcontext scon = { 0 };
> +
> + scon.eax = 11;
> + scon.ebx = 0x41;
> + scon.ecx = 0;
> + scon.edx = 0;
> + scon.esp = scon.ebp = ebp;
> + scon.eip = (int)syscall+6;
> +
> +#if defined EMULATED_32
> + scon.fs = 0x00;
> + scon.cs = 0x23;
> + scon.ss = scon.ds = scon.es = 0x2B;
> + scon.gs = 0x63;
> +#else
> + scon.fs = 0x00;
> + scon.cs = 0x73;
> + scon.ss = scon.ds = scon.es = 0x7B;
> + scon.gs = 0x33;
> +#endif
> + esp = (int) &scon;
> + asm("movl $119, %eax\n");//NR_SIGRETURN;
> + asm("int $0x80\n");
> +}

Coding style. //.

> +For 64 bit:
> +
> +gcc -O0 -o srop srop.c -g -fno-stack-protector -ffixed-rsp -ffixed-rbp
> +When run the program exits normally, with SROP protetction it terminates
> +with a segmentationf fault.

segmentation fault.


> + REG_RIP,
> + REG_EFL,
> + REG_CSGSFS,/* Actually short cs, gs, fs, __pad0. */

", "

> +void _exit_(void)
> +{
> + exit(1);
> +}
> +
> +void test(void)
> +{
> + struct ucontext ctx = { 0 };
> + register unsigned long rsp asm("rsp");
> + register unsigned long rbp asm("rbp");
> + ctx.uc_mcontext.gregs[REG_RIP] = (unsigned long) _exit_ + 4;

Why + 4?

> + ctx.uc_mcontext.gregs[REG_RSP] = rsp;
> + ctx.uc_mcontext.gregs[REG_RBP] = rbp;

Umm. Does this work?

> + ctx.uc_mcontext.gregs[REG_CSGSFS] = 0x002b000000000033;
> + rsp = (unsigned long) &ctx;
> + asm("movq $0xf,%rax\n");

", "? asm volatile? Clobbers?

> + asm("syscall\n");

Make it single asm statemtent so that gcc does not play with you? Add
checking if syscall just returns failure?

Aha, -O0 above. So you already know the code is buggy...?
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html