| Message ID | 20240315181032.645161-2-cgzones@googlemail.com (mailing list archive) |
|---|---|
| State | Under Review |
| Delegated to: | Paul Moore |
| Headers | show |
| Series | [RFC,1/2] lsm: introduce new hook security_vm_execstack | expand |
On 3/15/2024 11:08 AM, Christian Göttsche wrote: > Add a new hook guarding instantiations of programs with executable > stack. They are being warned about since commit 47a2ebb7f505 ("execve: > warn if process starts with executable stack"). Lets give LSMs the > ability to control their presence on a per application basis. This seems like a hideously expensive way to implement a flag disallowing execution of programs with executable stacks. What's wrong with adding a flag VM_NO_EXECUTABLE_STACK? > > Signed-off-by: Christian Göttsche <cgzones@googlemail.com> > --- > fs/exec.c | 4 ++++ > include/linux/lsm_hook_defs.h | 1 + > include/linux/security.h | 6 ++++++ > security/security.c | 13 +++++++++++++ > 4 files changed, 24 insertions(+) > > diff --git a/fs/exec.c b/fs/exec.c > index 8cdd5b2dd09c..e6f9e980c6b1 100644 > --- a/fs/exec.c > +++ b/fs/exec.c > @@ -829,6 +829,10 @@ int setup_arg_pages(struct linux_binprm *bprm, > BUG_ON(prev != vma); > > if (unlikely(vm_flags & VM_EXEC)) { > + ret = security_vm_execstack(); > + if (ret) > + goto out_unlock; > + > pr_warn_once("process '%pD4' started with executable stack\n", > bprm->file); > } > diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h > index 185924c56378..b31d0744e7e7 100644 > --- a/include/linux/lsm_hook_defs.h > +++ b/include/linux/lsm_hook_defs.h > @@ -49,6 +49,7 @@ LSM_HOOK(int, 0, syslog, int type) > LSM_HOOK(int, 0, settime, const struct timespec64 *ts, > const struct timezone *tz) > LSM_HOOK(int, 1, vm_enough_memory, struct mm_struct *mm, long pages) > +LSM_HOOK(int, 0, vm_execstack, void) > LSM_HOOK(int, 0, bprm_creds_for_exec, struct linux_binprm *bprm) > LSM_HOOK(int, 0, bprm_creds_from_file, struct linux_binprm *bprm, const struct file *file) > LSM_HOOK(int, 0, bprm_check_security, struct linux_binprm *bprm) > diff --git a/include/linux/security.h b/include/linux/security.h > index d0eb20f90b26..084b96814970 100644 > --- a/include/linux/security.h > +++ b/include/linux/security.h > @@ -294,6 +294,7 @@ int security_quota_on(struct dentry *dentry); > int security_syslog(int type); > int security_settime64(const struct timespec64 *ts, const struct timezone *tz); > int security_vm_enough_memory_mm(struct mm_struct *mm, long pages); > +int security_vm_execstack(void); > int security_bprm_creds_for_exec(struct linux_binprm *bprm); > int security_bprm_creds_from_file(struct linux_binprm *bprm, const struct file *file); > int security_bprm_check(struct linux_binprm *bprm); > @@ -624,6 +625,11 @@ static inline int security_vm_enough_memory_mm(struct mm_struct *mm, long pages) > return __vm_enough_memory(mm, pages, cap_vm_enough_memory(mm, pages)); > } > > +static inline int security_vm_execstack(void) > +{ > + return 0; > +} > + > static inline int security_bprm_creds_for_exec(struct linux_binprm *bprm) > { > return 0; > diff --git a/security/security.c b/security/security.c > index 0144a98d3712..f75240d0d99d 100644 > --- a/security/security.c > +++ b/security/security.c > @@ -1125,6 +1125,19 @@ int security_vm_enough_memory_mm(struct mm_struct *mm, long pages) > return __vm_enough_memory(mm, pages, cap_sys_admin); > } > > +/** > + * security_vm_execstack() - Check if starting a program with executable stack > + * is allowed > + * > + * Check whether starting a program with an executable stack is allowed. > + * > + * Return: Returns 0 if permission is granted. > + */ > +int security_vm_execstack(void) > +{ > + return call_int_hook(vm_execstack); > +} > + > /** > * security_bprm_creds_for_exec() - Prepare the credentials for exec() > * @bprm: binary program information
On Fri, 15 Mar 2024 at 19:22, Casey Schaufler <casey@schaufler-ca.com> wrote: > > On 3/15/2024 11:08 AM, Christian Göttsche wrote: > > Add a new hook guarding instantiations of programs with executable > > stack. They are being warned about since commit 47a2ebb7f505 ("execve: > > warn if process starts with executable stack"). Lets give LSMs the > > ability to control their presence on a per application basis. > > This seems like a hideously expensive way to implement a flag > disallowing execution of programs with executable stacks. What's > wrong with adding a flag VM_NO_EXECUTABLE_STACK? That would be global and not on a per application basis. One might want to exempt known legacy programs. Also is performance a concern for this today's rare occurrence? > > > > Signed-off-by: Christian Göttsche <cgzones@googlemail.com> > > --- > > fs/exec.c | 4 ++++ > > include/linux/lsm_hook_defs.h | 1 + > > include/linux/security.h | 6 ++++++ > > security/security.c | 13 +++++++++++++ > > 4 files changed, 24 insertions(+) > > > > diff --git a/fs/exec.c b/fs/exec.c > > index 8cdd5b2dd09c..e6f9e980c6b1 100644 > > --- a/fs/exec.c > > +++ b/fs/exec.c > > @@ -829,6 +829,10 @@ int setup_arg_pages(struct linux_binprm *bprm, > > BUG_ON(prev != vma); > > > > if (unlikely(vm_flags & VM_EXEC)) { > > + ret = security_vm_execstack(); > > + if (ret) > > + goto out_unlock; > > + > > pr_warn_once("process '%pD4' started with executable stack\n", > > bprm->file); > > } > > diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h > > index 185924c56378..b31d0744e7e7 100644 > > --- a/include/linux/lsm_hook_defs.h > > +++ b/include/linux/lsm_hook_defs.h > > @@ -49,6 +49,7 @@ LSM_HOOK(int, 0, syslog, int type) > > LSM_HOOK(int, 0, settime, const struct timespec64 *ts, > > const struct timezone *tz) > > LSM_HOOK(int, 1, vm_enough_memory, struct mm_struct *mm, long pages) > > +LSM_HOOK(int, 0, vm_execstack, void) > > LSM_HOOK(int, 0, bprm_creds_for_exec, struct linux_binprm *bprm) > > LSM_HOOK(int, 0, bprm_creds_from_file, struct linux_binprm *bprm, const struct file *file) > > LSM_HOOK(int, 0, bprm_check_security, struct linux_binprm *bprm) > > diff --git a/include/linux/security.h b/include/linux/security.h > > index d0eb20f90b26..084b96814970 100644 > > --- a/include/linux/security.h > > +++ b/include/linux/security.h > > @@ -294,6 +294,7 @@ int security_quota_on(struct dentry *dentry); > > int security_syslog(int type); > > int security_settime64(const struct timespec64 *ts, const struct timezone *tz); > > int security_vm_enough_memory_mm(struct mm_struct *mm, long pages); > > +int security_vm_execstack(void); > > int security_bprm_creds_for_exec(struct linux_binprm *bprm); > > int security_bprm_creds_from_file(struct linux_binprm *bprm, const struct file *file); > > int security_bprm_check(struct linux_binprm *bprm); > > @@ -624,6 +625,11 @@ static inline int security_vm_enough_memory_mm(struct mm_struct *mm, long pages) > > return __vm_enough_memory(mm, pages, cap_vm_enough_memory(mm, pages)); > > } > > > > +static inline int security_vm_execstack(void) > > +{ > > + return 0; > > +} > > + > > static inline int security_bprm_creds_for_exec(struct linux_binprm *bprm) > > { > > return 0; > > diff --git a/security/security.c b/security/security.c > > index 0144a98d3712..f75240d0d99d 100644 > > --- a/security/security.c > > +++ b/security/security.c > > @@ -1125,6 +1125,19 @@ int security_vm_enough_memory_mm(struct mm_struct *mm, long pages) > > return __vm_enough_memory(mm, pages, cap_sys_admin); > > } > > > > +/** > > + * security_vm_execstack() - Check if starting a program with executable stack > > + * is allowed > > + * > > + * Check whether starting a program with an executable stack is allowed. > > + * > > + * Return: Returns 0 if permission is granted. > > + */ > > +int security_vm_execstack(void) > > +{ > > + return call_int_hook(vm_execstack); > > +} > > + > > /** > > * security_bprm_creds_for_exec() - Prepare the credentials for exec() > > * @bprm: binary program information
On 3/15/2024 11:30 AM, Christian Göttsche wrote: > On Fri, 15 Mar 2024 at 19:22, Casey Schaufler <casey@schaufler-ca.com> wrote: >> On 3/15/2024 11:08 AM, Christian Göttsche wrote: >>> Add a new hook guarding instantiations of programs with executable >>> stack. They are being warned about since commit 47a2ebb7f505 ("execve: >>> warn if process starts with executable stack"). Lets give LSMs the >>> ability to control their presence on a per application basis. >> This seems like a hideously expensive way to implement a flag >> disallowing execution of programs with executable stacks. What's >> wrong with adding a flag VM_NO_EXECUTABLE_STACK? > That would be global and not on a per application basis. > One might want to exempt known legacy programs. OK, I can see that. > Also is performance a concern for this today's rare occurrence? Performance is *always* a concern. You're adding a new hook list for a "rare" case. You're extended SELinux policy to include the case. This really should be a hardening feature, not an SELinux policy feature. The hook makes no sense for an LSM like Smack, which only implements subject+object controls. You could implement a stand alone LSM that implements only this hook, but again, it's not really access control, it's hardening.
On Fri, Mar 15, 2024 at 2:10 PM Christian Göttsche <cgzones@googlemail.com> wrote: > > Add a new hook guarding instantiations of programs with executable > stack. They are being warned about since commit 47a2ebb7f505 ("execve: > warn if process starts with executable stack"). Lets give LSMs the > ability to control their presence on a per application basis. > > Signed-off-by: Christian Göttsche <cgzones@googlemail.com> > --- > fs/exec.c | 4 ++++ > include/linux/lsm_hook_defs.h | 1 + > include/linux/security.h | 6 ++++++ > security/security.c | 13 +++++++++++++ > 4 files changed, 24 insertions(+) Looking at the commit referenced above, I'm guessing the existing security_file_mprotect() hook doesn't catch this? > diff --git a/fs/exec.c b/fs/exec.c > index 8cdd5b2dd09c..e6f9e980c6b1 100644 > --- a/fs/exec.c > +++ b/fs/exec.c > @@ -829,6 +829,10 @@ int setup_arg_pages(struct linux_binprm *bprm, > BUG_ON(prev != vma); > > if (unlikely(vm_flags & VM_EXEC)) { > + ret = security_vm_execstack(); > + if (ret) > + goto out_unlock; > + > pr_warn_once("process '%pD4' started with executable stack\n", > bprm->file); > } Instead of creating a new LSM hook, have you considered calling the existing security_file_mprotect() hook? The existing LSM controls there may not be a great fit in this case, but I'd like to hear if you've tried that, and if you have, what made you decide a new hook was the better option?
On March 15, 2024 1:22:39 PM PDT, Paul Moore <paul@paul-moore.com> wrote: >On Fri, Mar 15, 2024 at 2:10 PM Christian Göttsche ><cgzones@googlemail.com> wrote: >> >> Add a new hook guarding instantiations of programs with executable >> stack. They are being warned about since commit 47a2ebb7f505 ("execve: >> warn if process starts with executable stack"). Lets give LSMs the >> ability to control their presence on a per application basis. >> >> Signed-off-by: Christian Göttsche <cgzones@googlemail.com> >> --- >> fs/exec.c | 4 ++++ >> include/linux/lsm_hook_defs.h | 1 + >> include/linux/security.h | 6 ++++++ >> security/security.c | 13 +++++++++++++ >> 4 files changed, 24 insertions(+) > >Looking at the commit referenced above, I'm guessing the existing >security_file_mprotect() hook doesn't catch this? > >> diff --git a/fs/exec.c b/fs/exec.c >> index 8cdd5b2dd09c..e6f9e980c6b1 100644 >> --- a/fs/exec.c >> +++ b/fs/exec.c >> @@ -829,6 +829,10 @@ int setup_arg_pages(struct linux_binprm *bprm, >> BUG_ON(prev != vma); >> >> if (unlikely(vm_flags & VM_EXEC)) { >> + ret = security_vm_execstack(); >> + if (ret) >> + goto out_unlock; >> + >> pr_warn_once("process '%pD4' started with executable stack\n", >> bprm->file); >> } > >Instead of creating a new LSM hook, have you considered calling the >existing security_file_mprotect() hook? The existing LSM controls >there may not be a great fit in this case, but I'd like to hear if >you've tried that, and if you have, what made you decide a new hook >was the better option? Also, can't MDWE handle this already? https://git.kernel.org/linus/b507808ebce23561d4ff8c2aa1fb949fe402bc61 -Kees
On Fri, Mar 15, 2024 at 11:24 PM Kees Cook <kees@kernel.org> wrote: > On March 15, 2024 1:22:39 PM PDT, Paul Moore <paul@paul-moore.com> wrote: > >On Fri, Mar 15, 2024 at 2:10 PM Christian Göttsche > ><cgzones@googlemail.com> wrote: > >> > >> Add a new hook guarding instantiations of programs with executable > >> stack. They are being warned about since commit 47a2ebb7f505 ("execve: > >> warn if process starts with executable stack"). Lets give LSMs the > >> ability to control their presence on a per application basis. > >> > >> Signed-off-by: Christian Göttsche <cgzones@googlemail.com> > >> --- > >> fs/exec.c | 4 ++++ > >> include/linux/lsm_hook_defs.h | 1 + > >> include/linux/security.h | 6 ++++++ > >> security/security.c | 13 +++++++++++++ > >> 4 files changed, 24 insertions(+) > > > >Looking at the commit referenced above, I'm guessing the existing > >security_file_mprotect() hook doesn't catch this? > > > >> diff --git a/fs/exec.c b/fs/exec.c > >> index 8cdd5b2dd09c..e6f9e980c6b1 100644 > >> --- a/fs/exec.c > >> +++ b/fs/exec.c > >> @@ -829,6 +829,10 @@ int setup_arg_pages(struct linux_binprm *bprm, > >> BUG_ON(prev != vma); > >> > >> if (unlikely(vm_flags & VM_EXEC)) { > >> + ret = security_vm_execstack(); > >> + if (ret) > >> + goto out_unlock; > >> + > >> pr_warn_once("process '%pD4' started with executable stack\n", > >> bprm->file); > >> } > > > >Instead of creating a new LSM hook, have you considered calling the > >existing security_file_mprotect() hook? The existing LSM controls > >there may not be a great fit in this case, but I'd like to hear if > >you've tried that, and if you have, what made you decide a new hook > >was the better option? > > Also, can't MDWE handle this already? > https://git.kernel.org/linus/b507808ebce23561d4ff8c2aa1fb949fe402bc61 It looks like it, but that doesn't mean there isn't also value in an associated LSM hook as the LSM hook would admins and security policy developers/analysts to incorporate this as part of the system's security policy. It's great that we have all of these cool knobs that we can play with independent of each other, but sometimes you really want a single unified security policy that you can look at, analyze, and reason about. Regardless, my previous comments still stand, I'd like to hear verification that the existing security_file_mprotect() hook is not sufficient, and if its current placement is lacking, why calling it from a second location wasn't practical and required the creation of a new LSM hook.
diff --git a/fs/exec.c b/fs/exec.c index 8cdd5b2dd09c..e6f9e980c6b1 100644 --- a/fs/exec.c +++ b/fs/exec.c @@ -829,6 +829,10 @@ int setup_arg_pages(struct linux_binprm *bprm, BUG_ON(prev != vma); if (unlikely(vm_flags & VM_EXEC)) { + ret = security_vm_execstack(); + if (ret) + goto out_unlock; + pr_warn_once("process '%pD4' started with executable stack\n", bprm->file); } diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h index 185924c56378..b31d0744e7e7 100644 --- a/include/linux/lsm_hook_defs.h +++ b/include/linux/lsm_hook_defs.h @@ -49,6 +49,7 @@ LSM_HOOK(int, 0, syslog, int type) LSM_HOOK(int, 0, settime, const struct timespec64 *ts, const struct timezone *tz) LSM_HOOK(int, 1, vm_enough_memory, struct mm_struct *mm, long pages) +LSM_HOOK(int, 0, vm_execstack, void) LSM_HOOK(int, 0, bprm_creds_for_exec, struct linux_binprm *bprm) LSM_HOOK(int, 0, bprm_creds_from_file, struct linux_binprm *bprm, const struct file *file) LSM_HOOK(int, 0, bprm_check_security, struct linux_binprm *bprm) diff --git a/include/linux/security.h b/include/linux/security.h index d0eb20f90b26..084b96814970 100644 --- a/include/linux/security.h +++ b/include/linux/security.h @@ -294,6 +294,7 @@ int security_quota_on(struct dentry *dentry); int security_syslog(int type); int security_settime64(const struct timespec64 *ts, const struct timezone *tz); int security_vm_enough_memory_mm(struct mm_struct *mm, long pages); +int security_vm_execstack(void); int security_bprm_creds_for_exec(struct linux_binprm *bprm); int security_bprm_creds_from_file(struct linux_binprm *bprm, const struct file *file); int security_bprm_check(struct linux_binprm *bprm); @@ -624,6 +625,11 @@ static inline int security_vm_enough_memory_mm(struct mm_struct *mm, long pages) return __vm_enough_memory(mm, pages, cap_vm_enough_memory(mm, pages)); } +static inline int security_vm_execstack(void) +{ + return 0; +} + static inline int security_bprm_creds_for_exec(struct linux_binprm *bprm) { return 0; diff --git a/security/security.c b/security/security.c index 0144a98d3712..f75240d0d99d 100644 --- a/security/security.c +++ b/security/security.c @@ -1125,6 +1125,19 @@ int security_vm_enough_memory_mm(struct mm_struct *mm, long pages) return __vm_enough_memory(mm, pages, cap_sys_admin); } +/** + * security_vm_execstack() - Check if starting a program with executable stack + * is allowed + * + * Check whether starting a program with an executable stack is allowed. + * + * Return: Returns 0 if permission is granted. + */ +int security_vm_execstack(void) +{ + return call_int_hook(vm_execstack); +} + /** * security_bprm_creds_for_exec() - Prepare the credentials for exec() * @bprm: binary program information
Add a new hook guarding instantiations of programs with executable stack. They are being warned about since commit 47a2ebb7f505 ("execve: warn if process starts with executable stack"). Lets give LSMs the ability to control their presence on a per application basis. Signed-off-by: Christian Göttsche <cgzones@googlemail.com> --- fs/exec.c | 4 ++++ include/linux/lsm_hook_defs.h | 1 + include/linux/security.h | 6 ++++++ security/security.c | 13 +++++++++++++ 4 files changed, 24 insertions(+)