[PATCH] x86: Lock down MSR writing in secure boot

[PATCH] x86: Lock down MSR writing in secure boot

[Kees Cook]

Writing to MSRs should not be allowed unless CAP_COMPROMISE_KERNEL is
set since it could lead to execution of arbitrary code in kernel mode.

Signed-off-by: Kees Cook <keescook@chromium.org>
---
This would be used on top of Matthew Garrett's existing "Secure boot
policy support" patch series.
---
 arch/x86/kernel/msr.c |    7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/arch/x86/kernel/msr.c b/arch/x86/kernel/msr.c
index 4929502..adaab3d 100644
--- a/arch/x86/kernel/msr.c
+++ b/arch/x86/kernel/msr.c
@@ -103,6 +103,9 @@ static ssize_t msr_write(struct file *file, const char __user *buf,
 	int err = 0;
 	ssize_t bytes = 0;
 
+	if (!capable(CAP_COMPROMISE_KERNEL))
+		return -EPERM;
+
 	if (count % 8)
 		return -EINVAL;	/* Invalid chunk size */
 
@@ -150,6 +153,10 @@ static long msr_ioctl(struct file *file, unsigned int ioc, unsigned long arg)
 			err = -EBADF;
 			break;
 		}
+		if (!capable(CAP_COMPROMISE_KERNEL)) {
+			err = -EPERM;
+			break;
+		}
 		if (copy_from_user(&regs, uregs, sizeof regs)) {
 			err = -EFAULT;
 			break;
-- 
1.7.9.5


-- 
Kees Cook
Chrome OS Security

Re: [PATCH] x86: Lock down MSR writing in secure boot

[H. Peter Anvin]

We already have CAP_RAWIO for this in mainline; I am not sure if this should be harder than that...

Kees Cook <keescook@chromium.org> wrote:

>Writing to MSRs should not be allowed unless CAP_COMPROMISE_KERNEL is
>set since it could lead to execution of arbitrary code in kernel mode.
>
>Signed-off-by: Kees Cook <keescook@chromium.org>
>---
>This would be used on top of Matthew Garrett's existing "Secure boot
>policy support" patch series.
>---
> arch/x86/kernel/msr.c |    7 +++++++
> 1 file changed, 7 insertions(+)
>
>diff --git a/arch/x86/kernel/msr.c b/arch/x86/kernel/msr.c
>index 4929502..adaab3d 100644
>--- a/arch/x86/kernel/msr.c
>+++ b/arch/x86/kernel/msr.c
>@@ -103,6 +103,9 @@ static ssize_t msr_write(struct file *file, const
>char __user *buf,
> 	int err = 0;
> 	ssize_t bytes = 0;
> 
>+	if (!capable(CAP_COMPROMISE_KERNEL))
>+		return -EPERM;
>+
> 	if (count % 8)
> 		return -EINVAL;	/* Invalid chunk size */
> 
>@@ -150,6 +153,10 @@ static long msr_ioctl(struct file *file, unsigned
>int ioc, unsigned long arg)
> 			err = -EBADF;
> 			break;
> 		}
>+		if (!capable(CAP_COMPROMISE_KERNEL)) {
>+			err = -EPERM;
>+			break;
>+		}
> 		if (copy_from_user(&regs, uregs, sizeof regs)) {
> 			err = -EFAULT;
> 			break;

-- 
Sent from my mobile phone. Please excuse brevity and lack of formatting.

Re: [PATCH] x86: Lock down MSR writing in secure boot

[Kees Cook]

No. CAP_RAWIO is for reading. Writing needs a much stronger check.

-Kees

On Fri, Feb 8, 2013 at 11:17 AM, H. Peter Anvin <hpa-YMNOUZJC4hwAvxtiuMwx3w@public.gmane.org> wrote:
> We already have CAP_RAWIO for this in mainline; I am not sure if this should be harder than that...
>
> Kees Cook <keescook-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> wrote:
>
>>Writing to MSRs should not be allowed unless CAP_COMPROMISE_KERNEL is
>>set since it could lead to execution of arbitrary code in kernel mode.
>>
>>Signed-off-by: Kees Cook <keescook-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
>>---
>>This would be used on top of Matthew Garrett's existing "Secure boot
>>policy support" patch series.
>>---
>> arch/x86/kernel/msr.c |    7 +++++++
>> 1 file changed, 7 insertions(+)
>>
>>diff --git a/arch/x86/kernel/msr.c b/arch/x86/kernel/msr.c
>>index 4929502..adaab3d 100644
>>--- a/arch/x86/kernel/msr.c
>>+++ b/arch/x86/kernel/msr.c
>>@@ -103,6 +103,9 @@ static ssize_t msr_write(struct file *file, const
>>char __user *buf,
>>       int err = 0;
>>       ssize_t bytes = 0;
>>
>>+      if (!capable(CAP_COMPROMISE_KERNEL))
>>+              return -EPERM;
>>+
>>       if (count % 8)
>>               return -EINVAL; /* Invalid chunk size */
>>
>>@@ -150,6 +153,10 @@ static long msr_ioctl(struct file *file, unsigned
>>int ioc, unsigned long arg)
>>                       err = -EBADF;
>>                       break;
>>               }
>>+              if (!capable(CAP_COMPROMISE_KERNEL)) {
>>+                      err = -EPERM;
>>+                      break;
>>+              }
>>               if (copy_from_user(&regs, uregs, sizeof regs)) {
>>                       err = -EFAULT;
>>                       break;
>
> --
> Sent from my mobile phone. Please excuse brevity and lack of formatting.



--
Kees Cook
Chrome OS Security

Re: [PATCH] x86: Lock down MSR writing in secure boot

[H. Peter Anvin]

On 02/08/2013 11:18 AM, Kees Cook wrote:
> No. CAP_RAWIO is for reading. Writing needs a much stronger check.
>
> -Kees

If so, I suspect we need to do this for *all* raw I/O... but I keep 
wondering how much more sensitive writing really is than reading.

	-hpa

-- 
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.

Re: [PATCH] x86: Lock down MSR writing in secure boot

[Kees Cook]

On Fri, Feb 8, 2013 at 11:42 AM, H. Peter Anvin <hpa@zytor.com> wrote:
> On 02/08/2013 11:18 AM, Kees Cook wrote:
>>
>> No. CAP_RAWIO is for reading. Writing needs a much stronger check.
>
> If so, I suspect we need to do this for *all* raw I/O... but I keep
> wondering how much more sensitive writing really is than reading.

Well, I think there's a reasonable distinction between systems that
expect to strictly enforce user-space/kernel-space separation
(CAP_COMPROMISE_KERNEL) and things that are fiddling with hardware
(CAP_SYS_RAWIO).

For example, even things like /dev/mem already have this separation
(although it is stronger). You can't open /dev/mem without
CAP_SYS_RAWIO, but if you do, you still can't write to RAM in
/dev/mem. This might be one of the earliest examples of this
distinction, actually.

I think it's likely that after a while, we can convert some of these
proposed CAP_COMPROMISE_KERNEL checks in always-deny once we figure
out how to deal with those areas more safely.

-Kees

--
Kees Cook
Chrome OS Security

Re: [PATCH] x86: Lock down MSR writing in secure boot

[H. Peter Anvin]

Analogy fail.  The /dev/mem lockout applies to system RAM, not MMIO.

I fear COMPROMISE_KERNEL is becoming the new SYS_ADMIN, which in turn is the new root.  Why?  Because it is inhebtly about a usage model, not about a specific resource.

Kees Cook <keescook-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> wrote:

>On Fri, Feb 8, 2013 at 11:42 AM, H. Peter Anvin <hpa-YMNOUZJC4hwAvxtiuMwx3w@public.gmane.org> wrote:
>> On 02/08/2013 11:18 AM, Kees Cook wrote:
>>>
>>> No. CAP_RAWIO is for reading. Writing needs a much stronger check.
>>
>> If so, I suspect we need to do this for *all* raw I/O... but I keep
>> wondering how much more sensitive writing really is than reading.
>
>Well, I think there's a reasonable distinction between systems that
>expect to strictly enforce user-space/kernel-space separation
>(CAP_COMPROMISE_KERNEL) and things that are fiddling with hardware
>(CAP_SYS_RAWIO).
>
>For example, even things like /dev/mem already have this separation
>(although it is stronger). You can't open /dev/mem without
>CAP_SYS_RAWIO, but if you do, you still can't write to RAM in
>/dev/mem. This might be one of the earliest examples of this
>distinction, actually.
>
>I think it's likely that after a while, we can convert some of these
>proposed CAP_COMPROMISE_KERNEL checks in always-deny once we figure
>out how to deal with those areas more safely.
>
>-Kees
>
>--
>Kees Cook
>Chrome OS Security

-- 
Sent from my mobile phone. Please excuse brevity and lack of formatting.

Re: [PATCH] x86: Lock down MSR writing in secure boot

[Kees Cook]

On Fri, Feb 8, 2013 at 12:18 PM, H. Peter Anvin <hpa@zytor.com> wrote:
> Analogy fail.  The /dev/mem lockout applies to system RAM, not MMIO.

Well, that's what I meant by it being "stronger".

> I fear COMPROMISE_KERNEL is becoming the new SYS_ADMIN, which in turn is the new root.  Why?  Because it is inhebtly about a usage model, not about a specific resource.

I can see where you're coming from, but I don't agree. Roughly
speaking, uids should be about organization and filesystem DAC (uid 0
shouldn't be special). SYS_ADMIN is about broad-ranging
configuration/resource access (generally still all in userspace).
There hasn't been a strong distinction between user/kernel space for
uid-0 just because it didn't seem valuable. But now it's clearer, and
COMPROMISE_KERNEL can mark where these lines need to be drawn.

Maybe a capability isn't the right way to go, I'm not sure. I'll leave
that to Matthew. Whatever the flag, it should be an immutable state of
the boot. Though, it probably makes sense as a cap just so that
non-secure-boot systems can still remove it from containers, etc.

-Kees

> Kees Cook <keescook@chromium.org> wrote:
>
>>On Fri, Feb 8, 2013 at 11:42 AM, H. Peter Anvin <hpa@zytor.com> wrote:
>>> On 02/08/2013 11:18 AM, Kees Cook wrote:
>>>>
>>>> No. CAP_RAWIO is for reading. Writing needs a much stronger check.
>>>
>>> If so, I suspect we need to do this for *all* raw I/O... but I keep
>>> wondering how much more sensitive writing really is than reading.
>>
>>Well, I think there's a reasonable distinction between systems that
>>expect to strictly enforce user-space/kernel-space separation
>>(CAP_COMPROMISE_KERNEL) and things that are fiddling with hardware
>>(CAP_SYS_RAWIO).
>>
>>For example, even things like /dev/mem already have this separation
>>(although it is stronger). You can't open /dev/mem without
>>CAP_SYS_RAWIO, but if you do, you still can't write to RAM in
>>/dev/mem. This might be one of the earliest examples of this
>>distinction, actually.
>>
>>I think it's likely that after a while, we can convert some of these
>>proposed CAP_COMPROMISE_KERNEL checks in always-deny once we figure
>>out how to deal with those areas more safely.
>>
>>-Kees
>>
>>--
>>Kees Cook
>>Chrome OS Security
>
> --
> Sent from my mobile phone. Please excuse brevity and lack of formatting.



--
Kees Cook
Chrome OS Security

Re: [PATCH] x86: Lock down MSR writing in secure boot

[Matthew Garrett]

On Fri, 2013-02-08 at 12:28 -0800, Kees Cook wrote:

> Maybe a capability isn't the right way to go, I'm not sure. I'll leave
> that to Matthew. Whatever the flag, it should be an immutable state of
> the boot. Though, it probably makes sense as a cap just so that
> non-secure-boot systems can still remove it from containers, etc.

There was interest in ensuring that this wasn't something special-cased
to UEFI Secure Boot, so using a capability seemed like the most
straightforward way - it's fundamentally a restriction on what an
otherwise privileged user is able to do, so it seemed like it fit the
model. But I'm not wed to it in the slightest, and in fact it causes
problems for some userspace (anything that drops all capabilities
suddenly finds itself unable to do something that it expects to be able
to do), so if anyone has any suggestions for a better approach…

Re: [PATCH] x86: Lock down MSR writing in secure boot

[Kees Cook]

On Fri, Feb 8, 2013 at 12:34 PM, Matthew Garrett
<matthew.garrett@nebula.com> wrote:
> On Fri, 2013-02-08 at 12:28 -0800, Kees Cook wrote:
>
>> Maybe a capability isn't the right way to go, I'm not sure. I'll leave
>> that to Matthew. Whatever the flag, it should be an immutable state of
>> the boot. Though, it probably makes sense as a cap just so that
>> non-secure-boot systems can still remove it from containers, etc.
>
> There was interest in ensuring that this wasn't something special-cased
> to UEFI Secure Boot, so using a capability seemed like the most
> straightforward way - it's fundamentally a restriction on what an
> otherwise privileged user is able to do, so it seemed like it fit the
> model. But I'm not wed to it in the slightest, and in fact it causes
> problems for some userspace (anything that drops all capabilities
> suddenly finds itself unable to do something that it expects to be able
> to do), so if anyone has any suggestions for a better approach…

I don't find it unreasonable to drop all caps and lose access to
sensitive things. :) That's sort of the point, really. I think a cap
is the best match. It seems like it should either be a cap or a
namespace flag, but the latter seems messy.

-Kees

--
Kees Cook
Chrome OS Security
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] x86: Lock down MSR writing in secure boot

[Matthew Garrett]

On Fri, 2013-02-08 at 13:02 -0800, Kees Cook wrote:

> I don't find it unreasonable to drop all caps and lose access to
> sensitive things. :) That's sort of the point, really. I think a cap
> is the best match. It seems like it should either be a cap or a
> namespace flag, but the latter seems messy.

Yeah, I think it's an expected outcome, but it means that if (say) qemu
drops privileges, qemu can no longer access PCI resources - even on
non-secure boot systems. That breaks existing userspace.

Re: [PATCH] x86: Lock down MSR writing in secure boot

[Josh Boyer]

On Fri, Feb 8, 2013 at 4:07 PM, Matthew Garrett
<matthew.garrett-05XSO3Yj/JvQT0dZR+AlfA@public.gmane.org> wrote:
> On Fri, 2013-02-08 at 13:02 -0800, Kees Cook wrote:
>
>> I don't find it unreasonable to drop all caps and lose access to
>> sensitive things. :) That's sort of the point, really. I think a cap
>> is the best match. It seems like it should either be a cap or a
>> namespace flag, but the latter seems messy.
>
> Yeah, I think it's an expected outcome, but it means that if (say) qemu
> drops privileges, qemu can no longer access PCI resources - even on
> non-secure boot systems. That breaks existing userspace.

Right.  We've had a few reports in Fedora of things breaking on non-SB
systems because of this.  The qemu one is the latest, but the general
problem is people think dropping all caps blindly is making their apps
safer.  Then they find they can't do things they could do before the new
cap was added.  It's messy.

I've thought of treating CAP_COMPROMISE_KERNEL as a hidden cap, where
only the kernel can grant or drop it.  Peter Jones suggested it might
work to allow it to be dropped iff it is the only cap being changed.
Either way, it's a "special" cap and I have no idea how acceptable
something like that would be.

Really though, the main issue is that you cannot introduce new caps to
enforce finer grained access without breaking something.

josh

Re: [PATCH] x86: Lock down MSR writing in secure boot

[Andy Lutomirski]

On 02/08/2013 01:14 PM, Josh Boyer wrote:
> On Fri, Feb 8, 2013 at 4:07 PM, Matthew Garrett
> <matthew.garrett-05XSO3Yj/JvQT0dZR+AlfA@public.gmane.org> wrote:
>> On Fri, 2013-02-08 at 13:02 -0800, Kees Cook wrote:
>>
>>> I don't find it unreasonable to drop all caps and lose access to
>>> sensitive things. :) That's sort of the point, really. I think a cap
>>> is the best match. It seems like it should either be a cap or a
>>> namespace flag, but the latter seems messy.
>>
>> Yeah, I think it's an expected outcome, but it means that if (say) qemu
>> drops privileges, qemu can no longer access PCI resources - even on
>> non-secure boot systems. That breaks existing userspace.
> 
> Right.  We've had a few reports in Fedora of things breaking on non-SB
> systems because of this.  The qemu one is the latest, but the general
> problem is people think dropping all caps blindly is making their apps
> safer.  Then they find they can't do things they could do before the new
> cap was added.  It's messy.

Why not require CAP_COMPROMISE_KERNEL to open (with O_RDWR or O_WRONLY)
/dev/msr?  After all, sudo </dev/null >/dev/msr will cause a privileged
write() call on the fd as long as the capability is in your bounding set.

--Andy

Re: [PATCH] x86: Lock down MSR writing in secure boot

[H. Peter Anvin]

On 02/08/2013 01:02 PM, Kees Cook wrote:
> On Fri, Feb 8, 2013 at 12:34 PM, Matthew Garrett
> <matthew.garrett-05XSO3Yj/JvQT0dZR+AlfA@public.gmane.org> wrote:
>> On Fri, 2013-02-08 at 12:28 -0800, Kees Cook wrote:
>>
>>> Maybe a capability isn't the right way to go, I'm not sure. I'll leave
>>> that to Matthew. Whatever the flag, it should be an immutable state of
>>> the boot. Though, it probably makes sense as a cap just so that
>>> non-secure-boot systems can still remove it from containers, etc.
>>
>> There was interest in ensuring that this wasn't something special-cased
>> to UEFI Secure Boot, so using a capability seemed like the most
>> straightforward way - it's fundamentally a restriction on what an
>> otherwise privileged user is able to do, so it seemed like it fit the
>> model. But I'm not wed to it in the slightest, and in fact it causes
>> problems for some userspace (anything that drops all capabilities
>> suddenly finds itself unable to do something that it expects to be able
>> to do), so if anyone has any suggestions for a better approach…
> 
> I don't find it unreasonable to drop all caps and lose access to
> sensitive things. :) That's sort of the point, really. I think a cap
> is the best match. It seems like it should either be a cap or a
> namespace flag, but the latter seems messy.
> 

Caps are fine; the problem is the "putting it all under one cap".  The
semi-problem here is that to preserve backwards compatibility we really
should have a way to have hierarchical caps in Linux (which we currently
don't), but it is not really an issue for this.

Also, keep in mind that there is a very simple way to deny MSR access
completely, which is to not include the driver in your kernel (and not
allow module loading, but if you can load modules you can just load a
module to muck with whatever MSR you want.)

I am still wondering if there are any legitimate uses of CAP_RAWIO &
~CAP_COMPROMISE_KERNEL that can't be used to subvert the latter.  I am
not sure there are.

	-hpa

Re: [PATCH] x86: Lock down MSR writing in secure boot

[Borislav Petkov]

On Fri, Feb 08, 2013 at 02:30:52PM -0800, H. Peter Anvin wrote:
> Also, keep in mind that there is a very simple way to deny MSR access
> completely, which is to not include the driver in your kernel (and not
> allow module loading, but if you can load modules you can just load a
> module to muck with whatever MSR you want.)

I was contemplating that too. What is the use case of having
msr.ko in a secure boot environment? Isn't that an all-no-tools,
you-can't-do-sh*t-except-what-you're-explicitly-allowed-to environment which
simply doesn't need to write MSRs in the first place?

-- 
Regards/Gruss,
    Boris.

Sent from a fat crate under my desk. Formatting is fine.
--

Re: [PATCH] x86: Lock down MSR writing in secure boot

[Matthew Garrett]

On Sat, 2013-02-09 at 00:06 +0100, Borislav Petkov wrote:
> On Fri, Feb 08, 2013 at 02:30:52PM -0800, H. Peter Anvin wrote:
> > Also, keep in mind that there is a very simple way to deny MSR access
> > completely, which is to not include the driver in your kernel (and not
> > allow module loading, but if you can load modules you can just load a
> > module to muck with whatever MSR you want.)
> 
> I was contemplating that too. What is the use case of having
> msr.ko in a secure boot environment? Isn't that an all-no-tools,
> you-can't-do-sh*t-except-what-you're-explicitly-allowed-to environment which
> simply doesn't need to write MSRs in the first place?

Well, sure, distributions could build every kernel twice. That seems a
little excessive, though.

Re: [PATCH] x86: Lock down MSR writing in secure boot

[H. Peter Anvin]

On 02/08/2013 03:26 PM, Matthew Garrett wrote:
> On Sat, 2013-02-09 at 00:06 +0100, Borislav Petkov wrote:
>> On Fri, Feb 08, 2013 at 02:30:52PM -0800, H. Peter Anvin wrote:
>>> Also, keep in mind that there is a very simple way to deny MSR access
>>> completely, which is to not include the driver in your kernel (and not
>>> allow module loading, but if you can load modules you can just load a
>>> module to muck with whatever MSR you want.)
>>
>> I was contemplating that too. What is the use case of having
>> msr.ko in a secure boot environment? Isn't that an all-no-tools,
>> you-can't-do-sh*t-except-what-you're-explicitly-allowed-to environment which
>> simply doesn't need to write MSRs in the first place?
> 
> Well, sure, distributions could build every kernel twice. That seems a
> little excessive, though.
> 

You don't have to build the kernel twice to exclude a loadable module.

	-hpa

Re: [PATCH] x86: Lock down MSR writing in secure boot

[Matthew Garrett]

On Fri, 2013-02-08 at 17:22 -0800, H. Peter Anvin wrote:

> You don't have to build the kernel twice to exclude a loadable module.

I guess you could just strip the signatures off any modules you don't
want to support under Secure Boot, but that breaks some other use cases.

Re: [PATCH] x86: Lock down MSR writing in secure boot

[Kees Cook]

On Fri, Feb 8, 2013 at 5:29 PM, Matthew Garrett
<matthew.garrett-05XSO3Yj/JvQT0dZR+AlfA@public.gmane.org> wrote:
> On Fri, 2013-02-08 at 17:22 -0800, H. Peter Anvin wrote:
>
>> You don't have to build the kernel twice to exclude a loadable module.
>
> I guess you could just strip the signatures off any modules you don't
> want to support under Secure Boot, but that breaks some other use cases.

Also, _reading_ MSRs from userspace arguably has utility that doesn't
compromise ring-0. So excluding the driver entirely seems like
overkill.

-Kees

--
Kees Cook
Chrome OS Security

Re: [PATCH] x86: Lock down MSR writing in secure boot

[Borislav Petkov]

On Fri, Feb 08, 2013 at 10:45:35PM -0800, Kees Cook wrote:
> Also, _reading_ MSRs from userspace arguably has utility that doesn't
> compromise ring-0.

And to come back to the original question: what is that utility, who
would need it on a secure boot system and why?

-- 
Regards/Gruss,
    Boris.

Sent from a fat crate under my desk. Formatting is fine.
--

Re: [PATCH] x86: Lock down MSR writing in secure boot

[Kees Cook]

On Sat, Feb 9, 2013 at 1:29 AM, Borislav Petkov <bp@alien8.de> wrote:
> On Fri, Feb 08, 2013 at 10:45:35PM -0800, Kees Cook wrote:
>> Also, _reading_ MSRs from userspace arguably has utility that doesn't
>> compromise ring-0.
>
> And to come back to the original question: what is that utility, who
> would need it on a secure boot system and why?

I have used it for double-checking the state of VMX (is it disabled
and locked), and for making sure that XD isn't masked, checking
thermal stuff, etc. I'm not arguing to keep the MSR driver while in
secure mode, mind you, I was just pointing out that like /dev/mem
there could be some utility to keeping the read half around. That
said, it could also be seen as an info leak. *shrug* My goal was just
to make sure it wasn't writable in the secure boot mode.

-Kees

--
Kees Cook
Chrome OS Security

Re: [PATCH] x86: Lock down MSR writing in secure boot

[Matthew Garrett]

On Sat, 2013-02-09 at 10:29 +0100, Borislav Petkov wrote:
> On Fri, Feb 08, 2013 at 10:45:35PM -0800, Kees Cook wrote:
> > Also, _reading_ MSRs from userspace arguably has utility that doesn't
> > compromise ring-0.
> 
> And to come back to the original question: what is that utility, who
> would need it on a secure boot system and why?

Things like Turbostat are useful, although perhaps that information
should be exposed in a better way.

Re: [PATCH] x86: Lock down MSR writing in secure boot

[H. Peter Anvin]

On 02/09/2013 07:11 AM, Matthew Garrett wrote:
> On Sat, 2013-02-09 at 10:29 +0100, Borislav Petkov wrote:
>> On Fri, Feb 08, 2013 at 10:45:35PM -0800, Kees Cook wrote:
>>> Also, _reading_ MSRs from userspace arguably has utility that doesn't
>>> compromise ring-0.
>>
>> And to come back to the original question: what is that utility, who
>> would need it on a secure boot system and why?
> 
> Things like Turbostat are useful, although perhaps that information
> should be exposed in a better way.
> 

OK... what none of this gets into:

Why should CAP_RAWIO be allowed on a secure boot system, when there are
2^n known ways of compromise a system with CAP_RAWIO?

	-hpa

Re: [PATCH] x86: Lock down MSR writing in secure boot

[Matthew Garrett]

On Tue, 2013-02-12 at 16:48 -0800, H. Peter Anvin wrote:

> OK... what none of this gets into:
> 
> Why should CAP_RAWIO be allowed on a secure boot system, when there are
> 2^n known ways of compromise a system with CAP_RAWIO?

CAP_SYS_RAWIO seems to have ended up being a catchall of "Maybe someone
who isn't entirely root should be able to do this", and not everything
it covers is equivalent to being able to compromise the running kernel.
I wouldn't argue with the idea that maybe we should just reappraise most
of the current uses of CAP_SYS_RAWIO, but removing capability checks
from places that currently have them seems like an invitation for
userspace breakage.

Re: [PATCH] x86: Lock down MSR writing in secure boot

[H. Peter Anvin]

On 02/12/2013 09:39 PM, Matthew Garrett wrote:
> On Tue, 2013-02-12 at 16:48 -0800, H. Peter Anvin wrote:
>
>> OK... what none of this gets into:
>>
>> Why should CAP_RAWIO be allowed on a secure boot system, when there are
>> 2^n known ways of compromise a system with CAP_RAWIO?
>
> CAP_SYS_RAWIO seems to have ended up being a catchall of "Maybe someone
> who isn't entirely root should be able to do this", and not everything
> it covers is equivalent to being able to compromise the running kernel.
> I wouldn't argue with the idea that maybe we should just reappraise most
> of the current uses of CAP_SYS_RAWIO, but removing capability checks
> from places that currently have them seems like an invitation for
> userspace breakage.
>

Sounds like you are thinking of CAP_SYS_ADMIN, but I don't really see a 
huge difference between MSRs and I/O control registers... just different 
address spaces.

	-hpa


-- 
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.

Re: [PATCH] x86: Lock down MSR writing in secure boot

[Matthew Garrett]

On Tue, 2013-02-12 at 22:12 -0800, H. Peter Anvin wrote:

> Sounds like you are thinking of CAP_SYS_ADMIN, but I don't really see a 
> huge difference between MSRs and I/O control registers... just different 
> address spaces.

Not having CAP_SYS_RAWIO blocks various SCSI commands, for instance.
These might result in the ability to write individual blocks or destroy
the device firmware, but do any of them permit modifying the running
kernel?

Re: [PATCH] x86: Lock down MSR writing in secure boot

[H. Peter Anvin]

On 02/12/2013 10:27 PM, Matthew Garrett wrote:
> On Tue, 2013-02-12 at 22:12 -0800, H. Peter Anvin wrote:
>
>> Sounds like you are thinking of CAP_SYS_ADMIN, but I don't really see a
>> huge difference between MSRs and I/O control registers... just different
>> address spaces.
>
> Not having CAP_SYS_RAWIO blocks various SCSI commands, for instance.
> These might result in the ability to write individual blocks or destroy
> the device firmware, but do any of them permit modifying the running
> kernel?

That is just batshit crazy.  If you have CAP_SYS_RAWIO you can do iopl() 
which means you can reprogram your northbridge, at which point you most 
definitely *can* modify the running kernel.

And some SCSI driver requires this??!

	-hpa

-- 
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.

Re: [PATCH] x86: Lock down MSR writing in secure boot

[Matthew Garrett]

On Tue, 2013-02-12 at 22:33 -0800, H. Peter Anvin wrote:

> That is just batshit crazy.  If you have CAP_SYS_RAWIO you can do iopl() 
> which means you can reprogram your northbridge, at which point you most 
> definitely *can* modify the running kernel.

Well right, that's the point of this patchset - it adds some extra
permission checks to some of the existing CAP_SYS_RAWIO checks.
CAP_SYS_RAWIO hasn't meant "I can perform arbitrary pio and mmio" for
years - it means "I can do things that might maybe break something
somehow". So sure, removing CAP_SYS_RAWIO would give us basically all
the security we want in a secure boot environment, but it would also
block things that we *want* to work.

Re: [PATCH] x86: Lock down MSR writing in secure boot

[H. Peter Anvin]

On 02/12/2013 10:41 PM, Matthew Garrett wrote:
> On Tue, 2013-02-12 at 22:33 -0800, H. Peter Anvin wrote:
>
>> That is just batshit crazy.  If you have CAP_SYS_RAWIO you can do iopl()
>> which means you can reprogram your northbridge, at which point you most
>> definitely *can* modify the running kernel.
>
> Well right, that's the point of this patchset - it adds some extra
> permission checks to some of the existing CAP_SYS_RAWIO checks.
> CAP_SYS_RAWIO hasn't meant "I can perform arbitrary pio and mmio" for
> years - it means "I can do things that might maybe break something
> somehow". So sure, removing CAP_SYS_RAWIO would give us basically all
> the security we want in a secure boot environment, but it would also
> block things that we *want* to work.
>

So, let 's see...

Problem:

Someone adds SYS_CAP_RAWIO to some places it definitely does not
belong.

Solution:

Break all the *appropriate* (as defined)uses of SYS_CAP_RAWIO?

What the heck?

	-hpa



-- 
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.

Re: [PATCH] x86: Lock down MSR writing in secure boot

[Matthew Garrett]

On Wed, 2013-02-13 at 09:20 -0800, H. Peter Anvin wrote:

> Problem:
> 
> Someone adds SYS_CAP_RAWIO to some places it definitely does not
> belong.
> 
> Solution:
> 
> Break all the *appropriate* (as defined)uses of SYS_CAP_RAWIO?

Problem:

CAP_SYS_RAWIO has been used in a bunch of arguably inappropriate places.
Removing CAP_SYS_RAWIO from the set of possible capabilities on a system
will prevent userspace from doing things that userspace should be
permitted to do. Removing CAP_SYS_RAWIO from the places that it
currently exists will allow userspace to do too much. Replacing
CAP_SYS_RAWIO with CAP_SYS_ADMIN will prevent userspace from doing
things that it can currently do.

Solution:

Admit that CAP_SYS_RAWIO is fucked up beyond rescue. Add a new
capability with well-defined semantics.

Re: [PATCH] x86: Lock down MSR writing in secure boot

[Casey Schaufler]

On 2/13/2013 9:26 AM, Matthew Garrett wrote:
> On Wed, 2013-02-13 at 09:20 -0800, H. Peter Anvin wrote:
>
>> Problem:
>>
>> Someone adds SYS_CAP_RAWIO to some places it definitely does not
>> belong.
>>
>> Solution:
>>
>> Break all the *appropriate* (as defined)uses of SYS_CAP_RAWIO?
> Problem:
>
> CAP_SYS_RAWIO has been used in a bunch of arguably inappropriate places.
> Removing CAP_SYS_RAWIO from the set of possible capabilities on a system
> will prevent userspace from doing things that userspace should be
> permitted to do. Removing CAP_SYS_RAWIO from the places that it
> currently exists will allow userspace to do too much. Replacing
> CAP_SYS_RAWIO with CAP_SYS_ADMIN will prevent userspace from doing
> things that it can currently do.
>
> Solution:
>
> Admit that CAP_SYS_RAWIO is fucked up beyond rescue. Add a new
> capability with well-defined semantics.

You can't add a new capability where there is an existing capability
that can be remotely argued to be appropriate.

If you tried to "fix" CAP_SYS_RAWIO and/or CAP_SYS_ADMIN you'd end
up with hundreds of capabilities.

Your particular problem is *not* so important that you get a
capability all to yourself.


> N�����r��y���b�X��ǧv�^�)޺{.n�+����{���.�+r��n�觶\x17��ܨ}���Ơz�&j:+v���\a����zZ+��+zf���h���~����i���z�\x1e�w���?����&�)ߢ^[fl===

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] x86: Lock down MSR writing in secure boot

[Matthew Garrett]

On Wed, 2013-02-13 at 09:51 -0800, Casey Schaufler wrote:
> On 2/13/2013 9:26 AM, Matthew Garrett wrote:
> > Admit that CAP_SYS_RAWIO is fucked up beyond rescue. Add a new
> > capability with well-defined semantics.
> 
> You can't add a new capability where there is an existing capability
> that can be remotely argued to be appropriate.

CAP_SYS_RAWIO can't be argued to be appropriate. It covers a range of
functionality that doesn't permit the running kernel to be modified and
which is required to provide a functional Linux system. Using it would
require redefining its existing usage, which would break existing
userspace.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH] x86: Lock down MSR writing in secure boot

[H. Peter Anvin]

On 02/13/2013 09:56 AM, Matthew Garrett wrote:
> On Wed, 2013-02-13 at 09:51 -0800, Casey Schaufler wrote:
>> On 2/13/2013 9:26 AM, Matthew Garrett wrote:
>>> Admit that CAP_SYS_RAWIO is fucked up beyond rescue. Add a new
>>> capability with well-defined semantics.
>>
>> You can't add a new capability where there is an existing capability
>> that can be remotely argued to be appropriate.
>
> CAP_SYS_RAWIO can't be argued to be appropriate. It covers a range of
> functionality that doesn't permit the running kernel to be modified and
> which is required to provide a functional Linux system. Using it would
> require redefining its existing usage, which would break existing
> userspace.
>

So people have piggybacked complete inappropriate junk onto 
CAP_SYS_RAWIO.  Great.  What the hell do we do now?  We can't break 
apart CAP_SYS_RAWIO because we don't have hierarchical capabilities.

We thus have a bunch of unpalatable choices, **all of which are wrong**.

This, incidentally, is *exactly* the reason I object to 
CAP_COMPROMISE_KERNEL as well... it describes a usage model, not a resource.

	-hpa

-- 
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.

Re: [PATCH] x86: Lock down MSR writing in secure boot

[Matthew Garrett]

On Wed, 2013-02-13 at 10:44 -0800, H. Peter Anvin wrote:

> So people have piggybacked complete inappropriate junk onto 
> CAP_SYS_RAWIO.  Great.  What the hell do we do now?  We can't break 
> apart CAP_SYS_RAWIO because we don't have hierarchical capabilities.

Yeah. Like I said, it's approximately useless.

> We thus have a bunch of unpalatable choices, **all of which are wrong**.
> 
> This, incidentally, is *exactly* the reason I object to 
> CAP_COMPROMISE_KERNEL as well... it describes a usage model, not a resource.

Like I said, I'm not wed to a capability-based model. However, it does
seem marginally more attractive than sprinkling if (!secure_boot) all
over the place. If anyone has alternatives, this would be a great time
to raise them.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH] x86: Lock down MSR writing in secure boot

[H. Peter Anvin]

On 02/13/2013 09:51 AM, Casey Schaufler wrote:
>
> You can't add a new capability where there is an existing capability
> that can be remotely argued to be appropriate.
>
> If you tried to "fix" CAP_SYS_RAWIO and/or CAP_SYS_ADMIN you'd end
> up with hundreds of capabilities.
>
> Your particular problem is *not* so important that you get a
> capability all to yourself.
>

{facepalm}

This is exactly the kind of thinking which has led to the capability 
system being so bloody useless.

Capabilities need to be associated with resources, not use cases.

	-hpa


-- 
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.

Re: [PATCH] x86: Lock down MSR writing in secure boot

[Casey Schaufler]

On 2/13/2013 2:26 PM, H. Peter Anvin wrote:
> On 02/13/2013 09:51 AM, Casey Schaufler wrote:
>>
>> You can't add a new capability where there is an existing capability
>> that can be remotely argued to be appropriate.
>>
>> If you tried to "fix" CAP_SYS_RAWIO and/or CAP_SYS_ADMIN you'd end
>> up with hundreds of capabilities.
>>
>> Your particular problem is *not* so important that you get a
>> capability all to yourself.
>>
>
> {facepalm}
>
> This is exactly the kind of thinking which has led to the capability
> system being so bloody useless.

The reason the capability system is "bloody useless" is
that no one wants to update the core system applications to
use it in favor of good old fashioned worked for dad and
works for me too superuser.

>
> Capabilities need to be associated with resources, not use cases.

There is no such thing as a "resource" in the Linux security
policy model. The Linux security policy model is based on
subjects (tasks) accessing objects (e.g. files). Capabilities
provide a granular mechanism for granting privilege to
violate the Linux security policy.

Because in the Bad Old days of Unix "superuser privilege"
also granted rights to preform configuration activities it
was not possible to eliminate the superuser without extending
the capability mechanism to include these. Thus, there are two
sorts of capabilities, those controlling privileged access and
those controlling restricted activities.

In both cases what you are controlling is an activity. Sorry,
but that is the way it is defined.

I understand that you want capabilities to be associated with
resources. That is *not* what we have, and arguing that its
what we should have is pointless because Linux does not even
have a concept of resources.

>
>     -hpa
>
>

Re: [PATCH] x86: Lock down MSR writing in secure boot

[H. Peter Anvin]

On 02/13/2013 02:58 PM, Casey Schaufler wrote:
>>
>> This is exactly the kind of thinking which has led to the capability
>> system being so bloody useless.
> 
> The reason the capability system is "bloody useless" is
> that no one wants to update the core system applications to
> use it in favor of good old fashioned worked for dad and
> works for me too superuser.
> 

Because, in large part because a bunch of the capabilities are so close
to equivalent to "superuser" that the distinction is meaningless... so
why go through the hassle?  (This is especially so since it was a *long*
time before the filesystem had any notion of capabilities, and so you
had to make your app run as root anyway before you could drop the
capabilities... and some of the people who tried failed spectacularly
and opened up new security holes.)

> 
> I understand that you want capabilities to be associated with
> resources. That is *not* what we have, and arguing that its
> what we should have is pointless because Linux does not even
> have a concept of resources.
> 

OK, fine, call it "activities".  This is still distinct from "usage
models", and that's where we're just broken.

I may look into seeing if there is any sane way we can use the existing
API to define hierarchical capabilities, which at least should let us
split existing capabilities just like we used capabilities to split root.

	-hpa

Re: [PATCH] x86: Lock down MSR writing in secure boot

[Casey Schaufler]

On 2/13/2013 4:25 PM, H. Peter Anvin wrote:
> On 02/13/2013 02:58 PM, Casey Schaufler wrote:
>>> This is exactly the kind of thinking which has led to the capability
>>> system being so bloody useless.
>> The reason the capability system is "bloody useless" is
>> that no one wants to update the core system applications to
>> use it in favor of good old fashioned worked for dad and
>> works for me too superuser.
>>
> Because, in large part because a bunch of the capabilities are so close
> to equivalent to "superuser" that the distinction is meaningless... so
> why go through the hassle?  (This is especially so since it was a *long*
> time before the filesystem had any notion of capabilities, and so you
> had to make your app run as root anyway before you could drop the
> capabilities... and some of the people who tried failed spectacularly
> and opened up new security holes.)

Yes. I know. I was one of the people who wrote the P1003.1e/2c spec.
I implemented a rootless Unix system. The infamous sendmail capability
blowup was not my code, but it was my fault. In the Linux world we have
had a terrible problem with no one implementing the next step because
no one is eager to have the next bit because you can't do everything
without that next step.

>> I understand that you want capabilities to be associated with
>> resources. That is *not* what we have, and arguing that its
>> what we should have is pointless because Linux does not even
>> have a concept of resources.
>>
> OK, fine, call it "activities".  This is still distinct from "usage
> models", and that's where we're just broken.
>
> I may look into seeing if there is any sane way we can use the existing
> API to define hierarchical capabilities, which at least should let us
> split existing capabilities just like we used capabilities to split root.

If you want that sort of granularity throw yourself on the SELinux
bandwagon. Fine grained capabilities are insane and unmanageable
and will only lead to tears. Security is despised because of the
notion that making systems impossible to use is a good thing.

>
> 	-hpa
>
>

Re: [PATCH] x86: Lock down MSR writing in secure boot

[Matthew Garrett]

On Wed, 2013-02-13 at 16:44 -0800, Casey Schaufler wrote:

> If you want that sort of granularity throw yourself on the SELinux
> bandwagon. Fine grained capabilities are insane and unmanageable
> and will only lead to tears. Security is despised because of the
> notion that making systems impossible to use is a good thing.

SELinux is completely unusable for this specific case.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH] x86: Lock down MSR writing in secure boot

[H. Peter Anvin]

On 02/13/2013 05:04 PM, Matthew Garrett wrote:
> On Wed, 2013-02-13 at 16:44 -0800, Casey Schaufler wrote:
> 
>> If you want that sort of granularity throw yourself on the SELinux
>> bandwagon. Fine grained capabilities are insane and unmanageable
>> and will only lead to tears. Security is despised because of the
>> notion that making systems impossible to use is a good thing.
> 
> SELinux is completely unusable for this specific case.
> 

Well, for at least things with device nodes (/dev/mem, /dev/msr and so
on) it should be possible, no?  ioperm() and iopl() are another matter.

	-hpa

Re: [PATCH] x86: Lock down MSR writing in secure boot

[Matthew Garrett]

On Wed, 2013-02-13 at 17:08 -0800, H. Peter Anvin wrote:

> Well, for at least things with device nodes (/dev/mem, /dev/msr and so
> on) it should be possible, no?  ioperm() and iopl() are another matter.

Sure, if we can guarantee that a signed userspace loads a signed SELinux
policy before any unsigned code runs. But, realistically, that's not
going to be possible.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH] x86: Lock down MSR writing in secure boot

[Casey Schaufler]

On 2/13/2013 5:04 PM, Matthew Garrett wrote:
> On Wed, 2013-02-13 at 16:44 -0800, Casey Schaufler wrote:
>
>> If you want that sort of granularity throw yourself on the SELinux
>> bandwagon. Fine grained capabilities are insane and unmanageable
>> and will only lead to tears. Security is despised because of the
>> notion that making systems impossible to use is a good thing.
> SELinux is completely unusable for this specific case.
>
Well, you'll get no argument from me there.

Re: [PATCH] x86: Lock down MSR writing in secure boot

[Paolo Bonzini]

Il 13/02/2013 07:33, H. Peter Anvin ha scritto:
>>
>>> Sounds like you are thinking of CAP_SYS_ADMIN, but I don't really see a
>>> huge difference between MSRs and I/O control registers... just different
>>> address spaces.
>>
>> Not having CAP_SYS_RAWIO blocks various SCSI commands, for instance.
>> These might result in the ability to write individual blocks or destroy
>> the device firmware, but do any of them permit modifying the running
>> kernel?

No, they cannot.

> That is just batshit crazy.  If you have CAP_SYS_RAWIO you can do iopl()
> which means you can reprogram your northbridge, at which point you most
> definitely *can* modify the running kernel.
> 
> And some SCSI driver requires this??!

No, and that's why there is a patchset floating that lets you toggle
this ability with a sysfs control.  This way you do not need
CAP_SYS_RAWIO anymore.

On non-x86 machines CAP_SYS_RAWIO is much less dangerous, especially
when coupled with file DAC.

Paolo

Re: [PATCH] x86: Lock down MSR writing in secure boot

[H. Peter Anvin]

On 02/13/2013 12:27 AM, Paolo Bonzini wrote:
>
> On non-x86 machines CAP_SYS_RAWIO is much less dangerous, especially
> when coupled with file DAC.
>

"File DAC"?

	-hpa

-- 
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.

Re: [PATCH] x86: Lock down MSR writing in secure boot

[H. Peter Anvin]

On 02/13/2013 12:27 AM, Paolo Bonzini wrote:
>
> On non-x86 machines CAP_SYS_RAWIO is much less dangerous, especially
> when coupled with file DAC.
>

Either way, I think you are at best deluded and more like you just 
completely wrong about CAP_SYS_RAWIO being "less dangerous on non-x86 
machines".  With the possible exception of s390 I suspect it is, in 
fact, more dangerous.

	-hpa


-- 
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.

Re: [PATCH] x86: Lock down MSR writing in secure boot

[Paolo Bonzini]

Il 13/02/2013 18:22, H. Peter Anvin ha scritto:
>>
>> On non-x86 machines CAP_SYS_RAWIO is much less dangerous, especially
>> when coupled with file DAC.

Discretionary Access Control.

> Either way, I think you are at best deluded and more like you just
> completely wrong about CAP_SYS_RAWIO being "less dangerous on non-x86
> machines".  With the possible exception of s390 I suspect it is, in
> fact, more dangerous.

I may well be wrong, but as a quick data point CAP_SYS_RAWIO has no
occurrences in arch/ except arch/x86.  Of course a lot of driver
functionality will be limited to CAP_SYS_RAWIO, but usually this
requires having a file descriptor for some file.

Paolo

Re: [PATCH] x86: Lock down MSR writing in secure boot

[H. Peter Anvin]

On 02/13/2013 11:55 AM, Paolo Bonzini wrote:
> Il 13/02/2013 18:22, H. Peter Anvin ha scritto:
>>>
>>> On non-x86 machines CAP_SYS_RAWIO is much less dangerous, especially
>>> when coupled with file DAC.
>
> Discretionary Access Control.
>
>> Either way, I think you are at best deluded and more like you just
>> completely wrong about CAP_SYS_RAWIO being "less dangerous on non-x86
>> machines".  With the possible exception of s390 I suspect it is, in
>> fact, more dangerous.
>
> I may well be wrong, but as a quick data point CAP_SYS_RAWIO has no
> occurrences in arch/ except arch/x86.  Of course a lot of driver
> functionality will be limited to CAP_SYS_RAWIO, but usually this
> requires having a file descriptor for some file.
>

Well, yes, although that could include /dev/mem.

	-hpa


-- 
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.

Re: [PATCH] x86: Lock down MSR writing in secure boot

[Matthew Garrett]

On Fri, 2013-02-08 at 11:12 -0800, Kees Cook wrote:
> Writing to MSRs should not be allowed unless CAP_COMPROMISE_KERNEL is
> set since it could lead to execution of arbitrary code in kernel mode.

Willing to buy this, but do you have a description of one potential
approach? We should probably also figure out what's writing to MSRs at
the moment (anything other than energy_perf_bias?) and decide what the
best thing to do there is.

Re: [PATCH] x86: Lock down MSR writing in secure boot

[Kees Cook]

On Fri, Feb 8, 2013 at 11:17 AM, Matthew Garrett
<matthew.garrett@nebula.com> wrote:
> On Fri, 2013-02-08 at 11:12 -0800, Kees Cook wrote:
>> Writing to MSRs should not be allowed unless CAP_COMPROMISE_KERNEL is
>> set since it could lead to execution of arbitrary code in kernel mode.
>
> Willing to buy this, but do you have a description of one potential
> approach? We should probably also figure out what's writing to MSRs at
> the moment (anything other than energy_perf_bias?) and decide what the
> best thing to do there is.

Yes, change the SYSENTER entry point to where-ever you like. There are
examples already written:
http://grsecurity.net/~spender/msr32.c

IMO, _writing_ an MSR from userspace should be considered a bug. If
writing is needed, a kernel driver should be mediating the change.
wrmsr (and rdmsr) are ring-0 only for good reason. :)

-Kees

--
Kees Cook
Chrome OS Security

Re: [PATCH] x86: Lock down MSR writing in secure boot

[Matthew Garrett]

On Fri, 2013-02-08 at 11:21 -0800, Kees Cook wrote:
> On Fri, Feb 8, 2013 at 11:17 AM, Matthew Garrett
> <matthew.garrett@nebula.com> wrote:
> > On Fri, 2013-02-08 at 11:12 -0800, Kees Cook wrote:
> >> Writing to MSRs should not be allowed unless CAP_COMPROMISE_KERNEL is
> >> set since it could lead to execution of arbitrary code in kernel mode.
> >
> > Willing to buy this, but do you have a description of one potential
> > approach? We should probably also figure out what's writing to MSRs at
> > the moment (anything other than energy_perf_bias?) and decide what the
> > best thing to do there is.
> 
> Yes, change the SYSENTER entry point to where-ever you like. There are
> examples already written:
> http://grsecurity.net/~spender/msr32.c

Cool. Yup, this sounds like a good plan.