| Message ID | 1500416736-49829-5-git-send-email-keescook@chromium.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Jul 18, 2017 at 6:25 PM, Kees Cook <keescook@chromium.org> wrote: > The SELinux bprm_secureexec hook can be merged with the bprm_set_creds > hook since it's dealing with the same information, and all of the details > are finalized during the first call to the bprm_set_creds hook via > prepare_binprm() (subsequent calls due to binfmt_script, etc, are ignored > via bprm->called_set_creds). > > Here, the test can just happen at the end of the bprm_set_creds hook, > and the bprm_secureexec hook can be dropped. > > Cc: Paul Moore <paul@paul-moore.com> > Cc: Stephen Smalley <sds@tycho.nsa.gov> > Signed-off-by: Kees Cook <keescook@chromium.org> > --- > security/selinux/hooks.c | 24 +++++------------------- > 1 file changed, 5 insertions(+), 19 deletions(-) This seems reasonable in the context of the other changes. Stephen just posted an AT_SECURE test for the selinux-testsuite on the SELinux mailing list, it would be nice to ensure that this patchset doesn't run afoul of that. Acked-by: Paul Moore <paul@paul-moore.com> > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c > index 0f1450a06b02..18038f73a2f7 100644 > --- a/security/selinux/hooks.c > +++ b/security/selinux/hooks.c > @@ -2413,30 +2413,17 @@ static int selinux_bprm_set_creds(struct linux_binprm *bprm) > > /* Clear any possibly unsafe personality bits on exec: */ > bprm->per_clear |= PER_CLEAR_ON_SETID; > - } > - > - return 0; > -} > - > -static int selinux_bprm_secureexec(struct linux_binprm *bprm) > -{ > - const struct task_security_struct *tsec = current_security(); > - u32 sid, osid; > - int atsecure = 0; > - > - sid = tsec->sid; > - osid = tsec->osid; > > - if (osid != sid) { > /* Enable secure mode for SIDs transitions unless > the noatsecure permission is granted between > the two SIDs, i.e. ahp returns 0. */ > - atsecure = avc_has_perm(osid, sid, > - SECCLASS_PROCESS, > - PROCESS__NOATSECURE, NULL); > + rc = avc_has_perm(old_tsec->sid, new_tsec->sid, > + SECCLASS_PROCESS, PROCESS__NOATSECURE, > + NULL); > + bprm->secureexec |= !!rc; > } > > - return !!atsecure; > + return 0; > } > > static int match_file(const void *p, struct file *file, unsigned fd) > @@ -6151,7 +6138,6 @@ static struct security_hook_list selinux_hooks[] __lsm_ro_after_init = { > LSM_HOOK_INIT(bprm_set_creds, selinux_bprm_set_creds), > LSM_HOOK_INIT(bprm_committing_creds, selinux_bprm_committing_creds), > LSM_HOOK_INIT(bprm_committed_creds, selinux_bprm_committed_creds), > - LSM_HOOK_INIT(bprm_secureexec, selinux_bprm_secureexec), > > LSM_HOOK_INIT(sb_alloc_security, selinux_sb_alloc_security), > LSM_HOOK_INIT(sb_free_security, selinux_sb_free_security), > -- > 2.7.4
On Wed, Jul 19, 2017 at 8:03 PM, Paul Moore <paul@paul-moore.com> wrote: > On Tue, Jul 18, 2017 at 6:25 PM, Kees Cook <keescook@chromium.org> wrote: >> The SELinux bprm_secureexec hook can be merged with the bprm_set_creds >> hook since it's dealing with the same information, and all of the details >> are finalized during the first call to the bprm_set_creds hook via >> prepare_binprm() (subsequent calls due to binfmt_script, etc, are ignored >> via bprm->called_set_creds). >> >> Here, the test can just happen at the end of the bprm_set_creds hook, >> and the bprm_secureexec hook can be dropped. >> >> Cc: Paul Moore <paul@paul-moore.com> >> Cc: Stephen Smalley <sds@tycho.nsa.gov> >> Signed-off-by: Kees Cook <keescook@chromium.org> >> --- >> security/selinux/hooks.c | 24 +++++------------------- >> 1 file changed, 5 insertions(+), 19 deletions(-) > > This seems reasonable in the context of the other changes. > > Stephen just posted an AT_SECURE test for the selinux-testsuite on the > SELinux mailing list, it would be nice to ensure that this patchset > doesn't run afoul of that. Quick follow-up: I just merged Stephen's test into the test suite: * https://github.com/SELinuxProject/selinux-testsuite > Acked-by: Paul Moore <paul@paul-moore.com> > >> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c >> index 0f1450a06b02..18038f73a2f7 100644 >> --- a/security/selinux/hooks.c >> +++ b/security/selinux/hooks.c >> @@ -2413,30 +2413,17 @@ static int selinux_bprm_set_creds(struct linux_binprm *bprm) >> >> /* Clear any possibly unsafe personality bits on exec: */ >> bprm->per_clear |= PER_CLEAR_ON_SETID; >> - } >> - >> - return 0; >> -} >> - >> -static int selinux_bprm_secureexec(struct linux_binprm *bprm) >> -{ >> - const struct task_security_struct *tsec = current_security(); >> - u32 sid, osid; >> - int atsecure = 0; >> - >> - sid = tsec->sid; >> - osid = tsec->osid; >> >> - if (osid != sid) { >> /* Enable secure mode for SIDs transitions unless >> the noatsecure permission is granted between >> the two SIDs, i.e. ahp returns 0. */ >> - atsecure = avc_has_perm(osid, sid, >> - SECCLASS_PROCESS, >> - PROCESS__NOATSECURE, NULL); >> + rc = avc_has_perm(old_tsec->sid, new_tsec->sid, >> + SECCLASS_PROCESS, PROCESS__NOATSECURE, >> + NULL); >> + bprm->secureexec |= !!rc; >> } >> >> - return !!atsecure; >> + return 0; >> } >> >> static int match_file(const void *p, struct file *file, unsigned fd) >> @@ -6151,7 +6138,6 @@ static struct security_hook_list selinux_hooks[] __lsm_ro_after_init = { >> LSM_HOOK_INIT(bprm_set_creds, selinux_bprm_set_creds), >> LSM_HOOK_INIT(bprm_committing_creds, selinux_bprm_committing_creds), >> LSM_HOOK_INIT(bprm_committed_creds, selinux_bprm_committed_creds), >> - LSM_HOOK_INIT(bprm_secureexec, selinux_bprm_secureexec), >> >> LSM_HOOK_INIT(sb_alloc_security, selinux_sb_alloc_security), >> LSM_HOOK_INIT(sb_free_security, selinux_sb_free_security), >> -- >> 2.7.4 > > -- > paul moore > www.paul-moore.com
On Wed, Jul 19, 2017 at 5:19 PM, Paul Moore <paul@paul-moore.com> wrote: > On Wed, Jul 19, 2017 at 8:03 PM, Paul Moore <paul@paul-moore.com> wrote: >> On Tue, Jul 18, 2017 at 6:25 PM, Kees Cook <keescook@chromium.org> wrote: >>> The SELinux bprm_secureexec hook can be merged with the bprm_set_creds >>> hook since it's dealing with the same information, and all of the details >>> are finalized during the first call to the bprm_set_creds hook via >>> prepare_binprm() (subsequent calls due to binfmt_script, etc, are ignored >>> via bprm->called_set_creds). >>> >>> Here, the test can just happen at the end of the bprm_set_creds hook, >>> and the bprm_secureexec hook can be dropped. >>> >>> Cc: Paul Moore <paul@paul-moore.com> >>> Cc: Stephen Smalley <sds@tycho.nsa.gov> >>> Signed-off-by: Kees Cook <keescook@chromium.org> >>> --- >>> security/selinux/hooks.c | 24 +++++------------------- >>> 1 file changed, 5 insertions(+), 19 deletions(-) >> >> This seems reasonable in the context of the other changes. >> >> Stephen just posted an AT_SECURE test for the selinux-testsuite on the >> SELinux mailing list, it would be nice to ensure that this patchset >> doesn't run afoul of that. > > Quick follow-up: I just merged Stephen's test into the test suite: > > * https://github.com/SELinuxProject/selinux-testsuite Is there a quick how-to on just running the AT_SECURE test? -Kees
On Wed, Jul 19, 2017 at 9:37 PM, Kees Cook <keescook@chromium.org> wrote: > On Wed, Jul 19, 2017 at 5:19 PM, Paul Moore <paul@paul-moore.com> wrote: >> On Wed, Jul 19, 2017 at 8:03 PM, Paul Moore <paul@paul-moore.com> wrote: >>> On Tue, Jul 18, 2017 at 6:25 PM, Kees Cook <keescook@chromium.org> wrote: >>>> The SELinux bprm_secureexec hook can be merged with the bprm_set_creds >>>> hook since it's dealing with the same information, and all of the details >>>> are finalized during the first call to the bprm_set_creds hook via >>>> prepare_binprm() (subsequent calls due to binfmt_script, etc, are ignored >>>> via bprm->called_set_creds). >>>> >>>> Here, the test can just happen at the end of the bprm_set_creds hook, >>>> and the bprm_secureexec hook can be dropped. >>>> >>>> Cc: Paul Moore <paul@paul-moore.com> >>>> Cc: Stephen Smalley <sds@tycho.nsa.gov> >>>> Signed-off-by: Kees Cook <keescook@chromium.org> >>>> --- >>>> security/selinux/hooks.c | 24 +++++------------------- >>>> 1 file changed, 5 insertions(+), 19 deletions(-) >>> >>> This seems reasonable in the context of the other changes. >>> >>> Stephen just posted an AT_SECURE test for the selinux-testsuite on the >>> SELinux mailing list, it would be nice to ensure that this patchset >>> doesn't run afoul of that. >> >> Quick follow-up: I just merged Stephen's test into the test suite: >> >> * https://github.com/SELinuxProject/selinux-testsuite > > Is there a quick how-to on just running the AT_SECURE test? You'll need a functional SELinux system to start, I run it against Fedora Rawhide regularly* with various development kernels, but recent stable Fedora releases should work too. Occasionally I hear of people running it on Debian, but I haven't had a Debian SELinux system in some time so I can't say for certain everything is 100% working there. Once you've gotten a working system in enforcing mode, read the README file in the test suite to install the necessary dependencies (look in the "Userland and Base Policy" section), then build the tests/policy (you should be able to skip this step, as the make dependencies will handle it, but it is nice to do it separately to make sure you have the build dependencies sorted): # make ... load the test policy # make -C policy load ... run the tests: # cd tests/atsecure # ./test ... optionally uninstall the test policy: # make -C policy unload In some ways it is easier to just run the entire test suite: # make # make test Alternatively, if you've got a fairly recent git repo with all the patches merged I can build a test kernel and give it a shot for you, although fair warning it may take a day or two for me to get to it. * It is worth noting that the current 4.13-rcX releases have two bugs that affect the selinux-testsuite. The worst is a kernel panic due to a bug in overlayfs' xattr code, there is a patch available to fix it, but as of yesterday it hadn't yet hit Linus tree (I can dig it up if you need it). The second issue related to IPsec and getting peer label information over UDP connections, I haven't had a chance to sort that out yet, but at least it isn't a kernel panic.
On Thu, Jul 20, 2017 at 6:42 AM, Paul Moore <paul@paul-moore.com> wrote: > Alternatively, if you've got a fairly recent git repo with all the > patches merged I can build a test kernel and give it a shot for you, > although fair warning it may take a day or two for me to get to it. Hurm, I think this will take quite a bit of time for me to set up. :P If you have a chance, I'd appreciate it if you could test the series. It's currently based on v4.12: https://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git/log/?h=kspp/setuid-rlimits/secureexec-no-hook If it doesn't work out or takes too much time I can work on setting up the test environment next week (travelling at the moment). Thanks for the details! -Kees
On Thu, Jul 20, 2017 at 1:06 PM, Kees Cook <keescook@chromium.org> wrote: > On Thu, Jul 20, 2017 at 6:42 AM, Paul Moore <paul@paul-moore.com> wrote: >> Alternatively, if you've got a fairly recent git repo with all the >> patches merged I can build a test kernel and give it a shot for you, >> although fair warning it may take a day or two for me to get to it. > > Hurm, I think this will take quite a bit of time for me to set up. :P > If you have a chance, I'd appreciate it if you could test the series. > It's currently based on v4.12: > https://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git/log/?h=kspp/setuid-rlimits/secureexec-no-hook > > If it doesn't work out or takes too much time I can work on setting up > the test environment next week (travelling at the moment). Building a kernel now, in case anyone on Fedora wants to play with it, you can find it here (when it finishes): * https://copr.fedorainfracloud.org/coprs/pcmoore/kernel-testing/build/581947
On Thu, Jul 20, 2017 at 4:42 PM, Paul Moore <paul@paul-moore.com> wrote: > On Thu, Jul 20, 2017 at 1:06 PM, Kees Cook <keescook@chromium.org> wrote: >> On Thu, Jul 20, 2017 at 6:42 AM, Paul Moore <paul@paul-moore.com> wrote: >>> Alternatively, if you've got a fairly recent git repo with all the >>> patches merged I can build a test kernel and give it a shot for you, >>> although fair warning it may take a day or two for me to get to it. >> >> Hurm, I think this will take quite a bit of time for me to set up. :P >> If you have a chance, I'd appreciate it if you could test the series. >> It's currently based on v4.12: >> https://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git/log/?h=kspp/setuid-rlimits/secureexec-no-hook >> >> If it doesn't work out or takes too much time I can work on setting up >> the test environment next week (travelling at the moment). > > Building a kernel now, in case anyone on Fedora wants to play with it, > you can find it here (when it finishes): > > * https://copr.fedorainfracloud.org/coprs/pcmoore/kernel-testing/build/581947 Quick follow up, the kernel above passes the selinux-testsuite atsecure test.
On Fri, Jul 21, 2017 at 8:40 AM, Paul Moore <paul@paul-moore.com> wrote: > On Thu, Jul 20, 2017 at 4:42 PM, Paul Moore <paul@paul-moore.com> wrote: >> On Thu, Jul 20, 2017 at 1:06 PM, Kees Cook <keescook@chromium.org> wrote: >>> On Thu, Jul 20, 2017 at 6:42 AM, Paul Moore <paul@paul-moore.com> wrote: >>>> Alternatively, if you've got a fairly recent git repo with all the >>>> patches merged I can build a test kernel and give it a shot for you, >>>> although fair warning it may take a day or two for me to get to it. >>> >>> Hurm, I think this will take quite a bit of time for me to set up. :P >>> If you have a chance, I'd appreciate it if you could test the series. >>> It's currently based on v4.12: >>> https://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git/log/?h=kspp/setuid-rlimits/secureexec-no-hook >>> >>> If it doesn't work out or takes too much time I can work on setting up >>> the test environment next week (travelling at the moment). >> >> Building a kernel now, in case anyone on Fedora wants to play with it, >> you can find it here (when it finishes): >> >> * https://copr.fedorainfracloud.org/coprs/pcmoore/kernel-testing/build/581947 > > Quick follow up, the kernel above passes the selinux-testsuite atsecure test. Awesome, thanks for taking the time to test it. :) Can I add your Tested-by? -Kees
On Fri, Jul 21, 2017 at 1:37 PM, Kees Cook <keescook@chromium.org> wrote: > On Fri, Jul 21, 2017 at 8:40 AM, Paul Moore <paul@paul-moore.com> wrote: >> On Thu, Jul 20, 2017 at 4:42 PM, Paul Moore <paul@paul-moore.com> wrote: >>> On Thu, Jul 20, 2017 at 1:06 PM, Kees Cook <keescook@chromium.org> wrote: >>>> On Thu, Jul 20, 2017 at 6:42 AM, Paul Moore <paul@paul-moore.com> wrote: >>>>> Alternatively, if you've got a fairly recent git repo with all the >>>>> patches merged I can build a test kernel and give it a shot for you, >>>>> although fair warning it may take a day or two for me to get to it. >>>> >>>> Hurm, I think this will take quite a bit of time for me to set up. :P >>>> If you have a chance, I'd appreciate it if you could test the series. >>>> It's currently based on v4.12: >>>> https://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git/log/?h=kspp/setuid-rlimits/secureexec-no-hook >>>> >>>> If it doesn't work out or takes too much time I can work on setting up >>>> the test environment next week (travelling at the moment). >>> >>> Building a kernel now, in case anyone on Fedora wants to play with it, >>> you can find it here (when it finishes): >>> >>> * https://copr.fedorainfracloud.org/coprs/pcmoore/kernel-testing/build/581947 >> >> Quick follow up, the kernel above passes the selinux-testsuite atsecure test. > > Awesome, thanks for taking the time to test it. :) Can I add your Tested-by? Sorry, I should have included that, here ya go: Tested-by: Paul Moore <paul@paul-moore.com>
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 0f1450a06b02..18038f73a2f7 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -2413,30 +2413,17 @@ static int selinux_bprm_set_creds(struct linux_binprm *bprm) /* Clear any possibly unsafe personality bits on exec: */ bprm->per_clear |= PER_CLEAR_ON_SETID; - } - - return 0; -} - -static int selinux_bprm_secureexec(struct linux_binprm *bprm) -{ - const struct task_security_struct *tsec = current_security(); - u32 sid, osid; - int atsecure = 0; - - sid = tsec->sid; - osid = tsec->osid; - if (osid != sid) { /* Enable secure mode for SIDs transitions unless the noatsecure permission is granted between the two SIDs, i.e. ahp returns 0. */ - atsecure = avc_has_perm(osid, sid, - SECCLASS_PROCESS, - PROCESS__NOATSECURE, NULL); + rc = avc_has_perm(old_tsec->sid, new_tsec->sid, + SECCLASS_PROCESS, PROCESS__NOATSECURE, + NULL); + bprm->secureexec |= !!rc; } - return !!atsecure; + return 0; } static int match_file(const void *p, struct file *file, unsigned fd) @@ -6151,7 +6138,6 @@ static struct security_hook_list selinux_hooks[] __lsm_ro_after_init = { LSM_HOOK_INIT(bprm_set_creds, selinux_bprm_set_creds), LSM_HOOK_INIT(bprm_committing_creds, selinux_bprm_committing_creds), LSM_HOOK_INIT(bprm_committed_creds, selinux_bprm_committed_creds), - LSM_HOOK_INIT(bprm_secureexec, selinux_bprm_secureexec), LSM_HOOK_INIT(sb_alloc_security, selinux_sb_alloc_security), LSM_HOOK_INIT(sb_free_security, selinux_sb_free_security),
The SELinux bprm_secureexec hook can be merged with the bprm_set_creds hook since it's dealing with the same information, and all of the details are finalized during the first call to the bprm_set_creds hook via prepare_binprm() (subsequent calls due to binfmt_script, etc, are ignored via bprm->called_set_creds). Here, the test can just happen at the end of the bprm_set_creds hook, and the bprm_secureexec hook can be dropped. Cc: Paul Moore <paul@paul-moore.com> Cc: Stephen Smalley <sds@tycho.nsa.gov> Signed-off-by: Kees Cook <keescook@chromium.org> --- security/selinux/hooks.c | 24 +++++------------------- 1 file changed, 5 insertions(+), 19 deletions(-)