From: Matthew Garrett <matthew.garrett@nebula.com> To: "gnomes@lxorguk.ukuu.org.uk" <gnomes@lxorguk.ukuu.org.uk> Cc: "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>, "jmorris@namei.org" <jmorris@namei.org>, "keescook@chromium.org" <keescook@chromium.org>, "linux-security-module@vger.kernel.org" <linux-security-module@vger.kernel.org>, "akpm@linux-foundation.org" <akpm@linux-foundation.org>, "hpa@zytor.com" <hpa@zytor.com>, "jwboyer@fedoraproject.org" <jwboyer@fedoraproject.org>, "linux-efi@vger.kernel.org" <linux-efi@vger.kernel.org>, "gregkh@linuxfoundation.org" <gregkh@linuxfoundation.org> Subject: Re: Trusted kernel patchset for Secure Boot lockdown Date: Fri, 14 Mar 2014 12:51:58 +0000 [thread overview] Message-ID: <1394801518.6416.38.camel@x230.lan> (raw) In-Reply-To: <20140314122231.17b9ca8a@alan.etchedpixels.co.uk> [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #1: Type: text/plain; charset="utf-8", Size: 4395 bytes --] On Fri, 2014-03-14 at 12:22 +0000, One Thousand Gnomes wrote: > > Have you actually looked at these patches? I've looked at every case of > > RAWIO in the kernel. For cases that are hardware specific and tied to > > fairly old hardware, I've ignored them. For cases which provide an > > Yes I have - and it's not exactly localised: it modifies stuff all over > the tree when it shouldn't need to. It's a security policy. If it leaks > policy all over the kernel then the implementation model is *wrong*. But you keep talking about MSRs despite there being a patch that limits access to MSRs. If you have specific examples of privilege escalations that are possible even with these patches then please, mention them. > > >default behaviour for capable(x) in fact be > > > > > > capable(x & ~secure_forbidden) > > > > > >for a measured kernel and add a > > > > > > capable_always() > > > > > >for the cases you want to not break. > > > > We could do that, but now the behaviour of the patchset is far less > > obvious. capable(CAP_SYS_RAWIO) now means something different to every > > other use of capable(), and we still need get_trusted_kernel() calls for > > cases where the checks have nothing to do with processes and so > > capabilities can't be used. > > You don't need get_measured_kernel() calls because thats hidden within > capable/capable_always. And if the interaction is a complicated as you > imply that again says to me the model is wrong. Complicated multi-way > security interactions lead to complicated exploits leveraging the > disconnects. How is capable() going to be any use when deciding whether or not to interpret some kernel command line options? > > We can do this without unnecessarily breaking any userspace. We just > > can't do it by fiddling with capabilities. > > It should still be a security model nto spreading measured_kernel() > checks about. Now if that means > > capable(CAP_SYS_RAWIO) > > should become security-> interface hooks that pass something meaningful > to the security layer then I'd rather we did the job properly in the first > place and put the policy in one spot not all over the kernel. We still need a mechanism to differentiate between cases where CAP_SYS_RAWIO should be forbidden and cases where it should be permitted, which means that we need to add additional policy. We can modify capable(), but that means that capable() no longer does what you expect it to - it's not a transparent interface, and that's likely to result in people accidentally misusing it. > The question then becomes what do we need to pass beyond "CAP_SYS_RAWIO" > so the policy can be in the right place - even for example imposed by > SELinux rules. It needs to be possible for this policy to be imposed without userspace being involved, so using selinux would mean baking it into the kernel (along with an additional implementation for apparmor, and presumably one for tomoyo as well). > > > > I don't think we need to break any userspace for "normal" mode to do > > > this. Userspace in measured mode is going to change anyway. It already > > > has just for things like module signing. > > > > This has been discussed at length. Nobody who's actually spent time > > working on the problem wants to use capabilities. CAP_SYS_RAWIO is not > > semantically identical to the trusted kernel bit. Trying to make them > > semantically identical will break existing userspace. > > I never said it was. I said that getting rid of CAP_SYS_RAWIO and then > dealing with *just* the exceptions to this is a lot cleaner, and likely > to be more secure. And will give us the choice between complicating a simple interface or breaking userspace. If the maintainer believes that's a better approach then I can rewrite all of this again, but so far you seem to be in a minority on this front. > In addition several of the cases that you point out could be fixed by > replacing the CAP_SYS_RAWIO using stuff with a proper interface should > probably just be *fixed* to provide that. ...thus breaking userspace outside this use case. I mean, sure, I'm absolutely fine with deleting /dev/mem entirely. I don't see Linus agreeing. -- Matthew Garrett <matthew.garrett@nebula.com> ÿôèº{.nÇ+·®+%Ëÿ±éݶ\x17¥wÿº{.nÇ+·¥{±þG«éÿ{ayº\x1dÊÚë,j\a¢f£¢·hïêÿêçz_è®\x03(éÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?¨èÚ&£ø§~á¶iOæ¬z·vØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?I¥
WARNING: multiple messages have this Message-ID (diff)
From: Matthew Garrett <matthew.garrett-05XSO3Yj/JvQT0dZR+AlfA@public.gmane.org> To: "gnomes-qBU/x9rampVanCEyBjwyrvXRex20P6io@public.gmane.org" <gnomes-qBU/x9rampVanCEyBjwyrvXRex20P6io@public.gmane.org> Cc: "linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" <linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>, "jmorris-gx6/JNMH7DfYtjvyW6yDsg@public.gmane.org" <jmorris-gx6/JNMH7DfYtjvyW6yDsg@public.gmane.org>, "keescook-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org" <keescook-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>, "linux-security-module-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" <linux-security-module-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>, "akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org" <akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org>, "hpa-YMNOUZJC4hwAvxtiuMwx3w@public.gmane.org" <hpa-YMNOUZJC4hwAvxtiuMwx3w@public.gmane.org>, "jwboyer-rxtnV0ftBwyoClj4AeEUq9i2O/JbrIOy@public.gmane.org" <jwboyer-rxtnV0ftBwyoClj4AeEUq9i2O/JbrIOy@public.gmane.org>, "linux-efi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" <linux-efi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>, "gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org" <gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org> Subject: Re: Trusted kernel patchset for Secure Boot lockdown Date: Fri, 14 Mar 2014 12:51:58 +0000 [thread overview] Message-ID: <1394801518.6416.38.camel@x230.lan> (raw) In-Reply-To: <20140314122231.17b9ca8a-mUKnrFFms3BCCTY1wZZT65JpZx93mCW/@public.gmane.org> On Fri, 2014-03-14 at 12:22 +0000, One Thousand Gnomes wrote: > > Have you actually looked at these patches? I've looked at every case of > > RAWIO in the kernel. For cases that are hardware specific and tied to > > fairly old hardware, I've ignored them. For cases which provide an > > Yes I have - and it's not exactly localised: it modifies stuff all over > the tree when it shouldn't need to. It's a security policy. If it leaks > policy all over the kernel then the implementation model is *wrong*. But you keep talking about MSRs despite there being a patch that limits access to MSRs. If you have specific examples of privilege escalations that are possible even with these patches then please, mention them. > > >default behaviour for capable(x) in fact be > > > > > > capable(x & ~secure_forbidden) > > > > > >for a measured kernel and add a > > > > > > capable_always() > > > > > >for the cases you want to not break. > > > > We could do that, but now the behaviour of the patchset is far less > > obvious. capable(CAP_SYS_RAWIO) now means something different to every > > other use of capable(), and we still need get_trusted_kernel() calls for > > cases where the checks have nothing to do with processes and so > > capabilities can't be used. > > You don't need get_measured_kernel() calls because thats hidden within > capable/capable_always. And if the interaction is a complicated as you > imply that again says to me the model is wrong. Complicated multi-way > security interactions lead to complicated exploits leveraging the > disconnects. How is capable() going to be any use when deciding whether or not to interpret some kernel command line options? > > We can do this without unnecessarily breaking any userspace. We just > > can't do it by fiddling with capabilities. > > It should still be a security model nto spreading measured_kernel() > checks about. Now if that means > > capable(CAP_SYS_RAWIO) > > should become security-> interface hooks that pass something meaningful > to the security layer then I'd rather we did the job properly in the first > place and put the policy in one spot not all over the kernel. We still need a mechanism to differentiate between cases where CAP_SYS_RAWIO should be forbidden and cases where it should be permitted, which means that we need to add additional policy. We can modify capable(), but that means that capable() no longer does what you expect it to - it's not a transparent interface, and that's likely to result in people accidentally misusing it. > The question then becomes what do we need to pass beyond "CAP_SYS_RAWIO" > so the policy can be in the right place - even for example imposed by > SELinux rules. It needs to be possible for this policy to be imposed without userspace being involved, so using selinux would mean baking it into the kernel (along with an additional implementation for apparmor, and presumably one for tomoyo as well). > > > > I don't think we need to break any userspace for "normal" mode to do > > > this. Userspace in measured mode is going to change anyway. It already > > > has just for things like module signing. > > > > This has been discussed at length. Nobody who's actually spent time > > working on the problem wants to use capabilities. CAP_SYS_RAWIO is not > > semantically identical to the trusted kernel bit. Trying to make them > > semantically identical will break existing userspace. > > I never said it was. I said that getting rid of CAP_SYS_RAWIO and then > dealing with *just* the exceptions to this is a lot cleaner, and likely > to be more secure. And will give us the choice between complicating a simple interface or breaking userspace. If the maintainer believes that's a better approach then I can rewrite all of this again, but so far you seem to be in a minority on this front. > In addition several of the cases that you point out could be fixed by > replacing the CAP_SYS_RAWIO using stuff with a proper interface should > probably just be *fixed* to provide that. ...thus breaking userspace outside this use case. I mean, sure, I'm absolutely fine with deleting /dev/mem entirely. I don't see Linus agreeing. -- Matthew Garrett <matthew.garrett@nebula.com>
next prev parent reply other threads:[~2014-03-14 12:52 UTC|newest] Thread overview: 128+ messages / expand[flat|nested] mbox.gz Atom feed top 2014-02-26 20:11 Trusted kernel patchset for Secure Boot lockdown Matthew Garrett 2014-02-26 20:11 ` [PATCH 01/12] Add support for indicating that the booted kernel is externally trusted Matthew Garrett 2014-02-27 19:02 ` Kees Cook 2014-02-27 19:02 ` Kees Cook 2014-03-31 14:49 ` Pavel Machek 2014-02-26 20:11 ` [PATCH 02/12] Enforce module signatures when trusted kernel is enabled Matthew Garrett 2014-02-26 20:11 ` Matthew Garrett 2014-02-26 20:11 ` [PATCH 03/12] PCI: Lock down BAR access when trusted_kernel is true Matthew Garrett 2014-02-26 20:11 ` [PATCH 04/12] x86: Lock down IO port " Matthew Garrett 2014-02-26 20:11 ` [PATCH 05/12] Restrict /dev/mem and /dev/kmem " Matthew Garrett 2014-02-26 20:11 ` [PATCH 06/12] acpi: Limit access to custom_method if " Matthew Garrett 2014-02-26 20:11 ` [PATCH 07/12] acpi: Ignore acpi_rsdp kernel parameter when " Matthew Garrett 2014-02-26 20:11 ` [PATCH 08/12] kexec: Disable at runtime if " Matthew Garrett 2014-02-26 20:11 ` [PATCH 09/12] uswsusp: Disable when " Matthew Garrett 2014-02-26 20:11 ` Matthew Garrett 2014-03-31 14:49 ` Pavel Machek 2014-02-26 20:11 ` [PATCH 10/12] x86: Restrict MSR access " Matthew Garrett 2014-02-26 20:11 ` [PATCH 11/12] asus-wmi: Restrict debugfs interface " Matthew Garrett 2014-02-26 20:11 ` [PATCH 12/12] Add option to automatically set trusted_kernel when in Secure Boot mode Matthew Garrett 2014-02-26 20:11 ` Matthew Garrett 2014-02-26 22:41 ` One Thousand Gnomes 2014-02-26 22:41 ` One Thousand Gnomes 2014-02-26 22:47 ` H. Peter Anvin 2014-02-26 22:48 ` Matthew Garrett 2014-02-26 22:48 ` Matthew Garrett 2014-02-27 18:48 ` Kees Cook 2014-02-27 18:48 ` Kees Cook 2014-02-26 21:11 ` Trusted kernel patchset for Secure Boot lockdown Kees Cook 2014-02-26 22:21 ` One Thousand Gnomes 2014-02-26 22:21 ` One Thousand Gnomes 2014-02-27 9:54 ` Alon Ziv 2014-03-19 17:42 ` Florian Weimer 2014-03-19 17:42 ` Florian Weimer 2014-02-27 18:04 ` Josh Boyer 2014-02-27 18:04 ` Josh Boyer 2014-02-27 19:07 ` Greg KH 2014-02-27 19:11 ` Josh Boyer 2014-02-27 19:11 ` Josh Boyer 2014-02-28 12:50 ` Josh Boyer 2014-02-28 3:03 ` James Morris 2014-02-28 4:52 ` Matthew Garrett 2014-02-28 4:52 ` Matthew Garrett 2014-03-13 5:01 ` Matthew Garrett 2014-03-13 5:01 ` Matthew Garrett 2014-03-13 6:22 ` Kees Cook 2014-03-13 6:22 ` Kees Cook 2014-03-13 9:33 ` James Morris 2014-03-13 9:33 ` James Morris 2014-03-13 10:12 ` One Thousand Gnomes 2014-03-13 10:12 ` One Thousand Gnomes 2014-03-13 15:54 ` H. Peter Anvin 2014-03-13 15:54 ` H. Peter Anvin 2014-03-13 15:59 ` Matthew Garrett 2014-03-13 15:59 ` Matthew Garrett 2014-03-13 21:24 ` One Thousand Gnomes 2014-03-13 21:24 ` One Thousand Gnomes 2014-03-13 21:28 ` H. Peter Anvin 2014-03-13 21:28 ` H. Peter Anvin 2014-03-13 21:32 ` Matthew Garrett 2014-03-13 21:32 ` Matthew Garrett 2014-03-13 21:30 ` Matthew Garrett 2014-03-13 21:30 ` Matthew Garrett 2014-03-13 23:21 ` One Thousand Gnomes 2014-03-13 23:21 ` One Thousand Gnomes 2014-03-14 1:57 ` Matthew Garrett 2014-03-14 1:57 ` Matthew Garrett 2014-03-14 12:22 ` One Thousand Gnomes 2014-03-14 12:22 ` One Thousand Gnomes 2014-03-14 12:51 ` Matthew Garrett [this message] 2014-03-14 12:51 ` Matthew Garrett 2014-03-14 15:23 ` Kees Cook 2014-03-14 15:23 ` Kees Cook 2014-03-14 15:46 ` Matthew Garrett 2014-03-14 15:46 ` Matthew Garrett 2014-03-14 15:54 ` Kees Cook 2014-03-14 15:54 ` Kees Cook 2014-03-14 15:58 ` Matthew Garrett 2014-03-14 15:58 ` Matthew Garrett 2014-03-14 16:28 ` One Thousand Gnomes 2014-03-14 16:28 ` One Thousand Gnomes 2014-03-14 17:06 ` One Thousand Gnomes 2014-03-14 17:06 ` One Thousand Gnomes 2014-03-14 18:11 ` Matthew Garrett 2014-03-14 18:11 ` Matthew Garrett 2014-03-14 19:24 ` Matthew Garrett 2014-03-14 19:24 ` Matthew Garrett 2014-03-14 20:37 ` David Lang 2014-03-14 20:37 ` David Lang 2014-03-14 20:43 ` Matthew Garrett 2014-03-14 20:43 ` Matthew Garrett 2014-03-14 21:58 ` One Thousand Gnomes 2014-03-14 21:58 ` One Thousand Gnomes 2014-03-14 22:04 ` Matthew Garrett 2014-03-14 22:04 ` Matthew Garrett 2014-03-14 21:48 ` One Thousand Gnomes 2014-03-14 21:48 ` One Thousand Gnomes 2014-03-14 21:56 ` Matthew Garrett 2014-03-14 21:56 ` Matthew Garrett 2014-03-14 22:08 ` One Thousand Gnomes 2014-03-14 22:08 ` One Thousand Gnomes 2014-03-14 22:15 ` Matthew Garrett 2014-03-14 22:15 ` Matthew Garrett 2014-03-14 22:31 ` One Thousand Gnomes 2014-03-14 22:31 ` One Thousand Gnomes 2014-03-14 22:52 ` Matthew Garrett 2014-03-14 22:52 ` Matthew Garrett 2014-03-19 19:50 ` Kees Cook 2014-03-19 19:50 ` Kees Cook 2014-03-14 23:18 ` Theodore Ts'o 2014-03-14 23:18 ` Theodore Ts'o 2014-03-15 0:15 ` One Thousand Gnomes 2014-03-15 0:15 ` One Thousand Gnomes 2014-03-19 17:49 ` Florian Weimer 2014-03-19 17:49 ` Florian Weimer 2014-03-19 20:16 ` Kees Cook 2014-03-19 20:16 ` Kees Cook 2014-03-20 14:47 ` One Thousand Gnomes 2014-03-20 14:47 ` One Thousand Gnomes 2014-03-20 14:55 ` tytso 2014-03-20 14:55 ` tytso 2014-03-20 17:12 ` Matthew Garrett 2014-03-20 17:12 ` Matthew Garrett 2014-03-20 18:13 ` One Thousand Gnomes 2014-03-20 18:13 ` One Thousand Gnomes 2014-03-13 21:26 ` One Thousand Gnomes 2014-03-13 21:26 ` One Thousand Gnomes 2014-03-13 21:31 ` Matthew Garrett 2014-03-13 21:31 ` Matthew Garrett
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=1394801518.6416.38.camel@x230.lan \ --to=matthew.garrett@nebula.com \ --cc=akpm@linux-foundation.org \ --cc=gnomes@lxorguk.ukuu.org.uk \ --cc=gregkh@linuxfoundation.org \ --cc=hpa@zytor.com \ --cc=jmorris@namei.org \ --cc=jwboyer@fedoraproject.org \ --cc=keescook@chromium.org \ --cc=linux-efi@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-security-module@vger.kernel.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.