Re: [GIT PULL] Kernel lockdown for secure boot

2018-04-03 Thread Matthew Garrett
On Tue, Apr 3, 2018 at 7:34 PM Alexei Starovoitov <
alexei.starovoi...@gmail.com> wrote:
> If the only thing that folks are paranoid about is reading
> arbitrary kernel memory with bpf_probe_read() helper
> then preferred patch would be to disable it during verification
> when in lockdown mode.
> No run-time overhead and android folks will be happy
> that lockdown doesn't break their work.
> They converted out-of-tree networking accounting
> module and corresponding user daemon to use bpf:

https://www.linuxplumbersconf.org/2017/ocw/system/presentations/4791/original/eBPF%20cgroup%20filters%20for%20data%20usage%20accounting%20on%20Android.pdf

An alternative would be to only disable kernel reads if the kernel contains
secrets that aren't supposed to be readable by root. If the keyring is
configured such that root can read everything, it seems like less of a
concern?
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [GIT PULL] Kernel lockdown for secure boot

2018-04-03 Thread Matthew Garrett
On Tue, Apr 3, 2018 at 6:43 PM Linus Torvalds

wrote:

> On Tue, Apr 3, 2018 at 6:13 PM, Matthew Garrett  wrote:
> >
> > There are four cases:

> No.

> Matthew., stop with the agenda already.

> This shit is what I'm talking about:

> > Verified Boot off, lockdown on: Perception of security improvement
that's
> > trivially circumvented (and so bad)

> You're doing some serious value judgement that is simply bogus.

> If lockdown actually helps avoid CPL0 execution attacks, then it helps
> even if secure more is off.

Bear in mind that I'm talking about defaults here - in more constrained
configurations the answers may change. But the kernel has no way of knowing
whether it's in one of those configurations, and as a result there's an
argument for not overpromising on the security that you're providing users.
If a user has a configuration where you're able to verify that userspace
has some degree of protection (eg, disk encryption keys are in the TPM and
won't unseal if someone's switched out the kernel) then it's reasonable for
userland (or a kernel command line option) to enable the functionality.

What I'm afraid of is this turning into a "security" feature that ends up
being circumvented in most scenarios where it's currently deployed - eg,
module signatures are mostly worthless in the non-lockdown case because you
can just grab the sig_enforce symbol address and then kexec a preamble that
flips it back to N regardless of the kernel config. This is the sort of
thing that's not obvious to most users, and it potentially causes them to
make worse security decisions as a result. The goal for this part of the
patchset isn't to cover every single conceivable case, it's to provide
reasonable defaults in a way that makes life easier for distributions.

> Or think of virtual machines - which people often use on purpose for
> security things. Again, they very much are _not_ going to have secure
> boot, but it's not necessarily even possible to "replace the kernel
> and reboot" at all, because the kernel came from outside the virtual
> machine entirely, and rebooting might just kill the VM rather than
> restart anything.

And where you have a trustworthy external thing providing your kernel,
yeah, that's also an argument - and having a kernel command line argument
that enables it in this case also seems entirely reasonable.

> This is what I mean by having an agenda.  We all know you are a big
> proponent of secure boot. But it seems to cloud your arguments, by
> turning your assumptions and your agenda into an "argument" that is
> simply not even TRUE.

I'm making this argument from the perspective of "What should the kernel do
when it has no additional information". Having the kernel automatically
enable lockdown without the user being aware of which guarantees their
environment is providing risks giving users the impression of security that
they may not have - in that case it makes more sense to have the user make
an explicit decision to enable it.

> See what I'm unhappy about?

> > Verified Boot on, lockdown off: Perception of security improvement
that's
> > trivially circumvented (and so bad), status quo in mainline kernels

> I think this is entirely false too. Again, the "trivial circumvention"
> shows a bias and agenda that isn't really all that true.

> > Of these four options, only two make sense.

> No.

> You say that, because you have that bias and that agenda.

Ok. Only two make sense *in the absence of additional information about
local configuration*. Distributions have to make reasonable choices here,
and where a configuration choice decreases functionality and provides what
may only be a marginal increase in security it's not a good configuration
choice to make by default.

> That said, wouldn't it be equally good to just make the whole thing be
> a protected EFI variable instead? Once you have physical access to the
> EFI shell (to turn off secure boot) you have access to that too.

That's pretty much exactly what mokutil does, without you needing to find a
copy of the UEFI shell to install first. If you think there's a strong
enough need for it, we could definitely add an additional variable that
allowed you to disable lockdown without disabling signature validation.
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [GIT PULL] Kernel lockdown for secure boot

2018-04-03 Thread Alexei Starovoitov
On Tue, Apr 3, 2018 at 9:26 AM, Andy Lutomirski  wrote:
> On Tue, Apr 3, 2018 at 8:41 AM, Alexei Starovoitov
>  wrote:
>> On Tue, Apr 03, 2018 at 08:11:07AM -0700, Andy Lutomirski wrote:
>>> >
>>> >> "bpf: Restrict kernel image access functions when the kernel is locked 
>>> >> down":
>>> >> This patch just sucks in general.
>>> >
>>> > Yes - but that's what Alexei Starovoitov specified.  bpf kind of sucks 
>>> > since
>>> > it gives you unrestricted access to the kernel.
>>>
>>> bpf, in certain contexts, gives you unrestricted access to *reading*
>>> kernel memory.  bpf should, under no circumstances, let you write to
>>> the kernel unless you're using fault injection or similar.
>>>
>>> I'm surprised that Alexei acked this patch.  If something like XDP or
>>> bpfilter starts becoming widely used, this patch will require a lot of
>>> reworking to avoid breaking standard distros.
>>
>> my understanding was that this lockdown set attemps to disallow _reads_
>> of kernel memory from anything, so first version of patch was adding
>> run-time checks for bpf_probe_read() which is no-go
>> and without this helper the bpf for tracing is losing a lot of its power,
>> so the easiest is to disable it all.
>
> Fair enough.

Actually looking at the patch again:
https://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs.git/commit/?h=efi-lock-down=78bb0059c3b8304a8d124b55feebc780fb3e0500

If the only thing that folks are paranoid about is reading
arbitrary kernel memory with bpf_probe_read() helper
then preferred patch would be to disable it during verification
when in lockdown mode.
No run-time overhead and android folks will be happy
that lockdown doesn't break their work.
They converted out-of-tree networking accounting
module and corresponding user daemon to use bpf:
https://www.linuxplumbersconf.org/2017/ocw/system/presentations/4791/original/eBPF%20cgroup%20filters%20for%20data%20usage%20accounting%20on%20Android.pdf
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [GIT PULL] Kernel lockdown for secure boot

2018-04-03 Thread Linus Torvalds
On Tue, Apr 3, 2018 at 6:30 PM, Justin Forbes  wrote:
>>
>> If there actually was a good explanation for the tie-in, it should
>> have been front-and-center and explained as such.
>>
> Honestly, yes, the major distros have been shipping this patch set for years
> now, and every time it comes to upstream, the same damn arguments emerge.

Well, I think it's because the explanations have been bogus.

Just look at this thread. It took closer to a hundred emails (ok, so
I'm exaggerating, but not _that_ much) until the *real* reason for the
tie-in was actually exposed.

For the first 50+ emails, the explanation was "oh, only if you do
secure boot does this make sense".

Which is still pure BULLSHIT. Of _course_ that kind of stuff raises
peoples hackles and makes people not trust the messenger - he's
clearly being evasive and there must be something else going on.

So instead of the bullshit explanations, just explain the purely
_practical_ side.

Because I find it a *lot* more convincing to hear:

   "We'd like to just enable it all the time, but it's known to break
some unusual hardware cases that we can't fix in software, and we
wanted *some* way to disable it that requires explicit and verified
user intervention to do that, and disabling secure boot is the
easiest hack we could come up with".

See? No bullshit. Just straight talk about the *actual* reason why
people decided on this particular tie-in, and admitting that it's a
hack, but also clearly stating the reason for the hack.

Now, I still don't necessarily agree that it's the best possible
option, but when stated in those terms I at least understand why that
option was picked as a reasonable one, and it changes the discussion a
lot, and (at least for me) makes it much more palatable.

Because as long as the explanation is just some "you must use secure
boot or you've already lost and further security is pointless"
hocus-pocus magical thinking, I immediately go "no, that sounds
completely bogus, and it makes testing and coverage much worse, we've
done other things quite like that without this secure boot tie-in".

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


Re: [GIT PULL] Kernel lockdown for secure boot

2018-04-03 Thread Linus Torvalds
On Tue, Apr 3, 2018 at 6:13 PM, Matthew Garrett  wrote:
>
> There are four cases:

No.

Matthew., stop with the agenda already.

This shit is what I'm talking about:

> Verified Boot off, lockdown on: Perception of security improvement that's
> trivially circumvented (and so bad)

You're doing some serious value judgement that is simply bogus.

If lockdown actually helps avoid CPL0 execution attacks, then it helps
even if secure more is off.

Sure, you can do things like try to install kernels and reboot, but
honestly, that's not "trivially circumvented". It can be quite hard to
hide even if you don't have secure boot. Things like disk encryption
(common for a lot of people) for example means that you simply won't
be booting that machine without the user noticing.

Or think of virtual machines - which people often use on purpose for
security things. Again, they very much are _not_ going to have secure
boot, but it's not necessarily even possible to "replace the kernel
and reboot" at all, because the kernel came from outside the virtual
machine entirely, and rebooting might just kill the VM rather than
restart anything.

So I really think you're pushing this whole "not secure boot" means
"trivial circumvention" much much too hard.

To the point of it being an outright lie.

I think the kind of people who run stuff in virtual machines could
easily want to also enable lockdown measures, simply to reduce the
attack window within that VM. Wouldn't you agree? Those are often
security-conscious people.

This is what I mean by having an agenda.  We all know you are a big
proponent of secure boot. But it seems to cloud your arguments, by
turning your assumptions and your agenda into an "argument" that is
simply not even TRUE.

See what I'm unhappy about?

> Verified Boot on, lockdown off: Perception of security improvement that's
> trivially circumvented (and so bad), status quo in mainline kernels

I think this is entirely false too. Again, the "trivial circumvention"
shows a bias and agenda that isn't really all that true.

> Of these four options, only two make sense.

No.

You say that, because you have that bias and that agenda.

But that simply doesn't make it true.

Now, what actually seems to be a real and valid argument is *this* part:

> This makes it easy for a user to switch
> between the two states that make sense by running a single command and then
> following some prompts on the next reboot. The alternative would be to
> provide a signed kernel that always enabled lockdown and an unsigned kernel
> that didn't, which would (a) increase load on distributions and (b) force
> users to both run mokutil --disable-validation and also install a different
> kernel.

THAT is an actual argument. Admittedly I think it's a horrible hack,
but it's a hack that can be explained without outright lying. And it
may be a hack that is "the best we can reasonably do"

See what I'm saying?

One argument is based on your value judgments that not everybody else
believes in.

The other argument is based purely on cold hard particular facts.

Guess which argument is better for people who aren't Matthew Garrett?

That said, wouldn't it be equally good to just make the whole thing be
a protected EFI variable instead? Once you have physical access to the
EFI shell (to turn off secure boot) you have access to that too.

Which would allow the "switch off/on" case even if there are other
reasons why changing secure boot isn't a great option (possibly
because secure boot isn't an option to begin with due to being so
invonvenient).

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


Re: [GIT PULL] Kernel lockdown for secure boot

2018-04-03 Thread Justin Forbes
On Tue, Apr 3, 2018 at 7:56 PM, Linus Torvalds
 wrote:
> On Tue, Apr 3, 2018 at 5:46 PM, Matthew Garrett  wrote:
>>
>> The generic distros have been shipping this policy for the past 5 years.
>
> .. so apparently it doesn't actually break things? Why not enable it
> by default then?
>
> And if "turn off secure boot" really is the accepted - and actuially
> used - workaround for the breakage, then
>

While there is very little breakage in the *years* we have been
shipping this in distro kernels, the accepted and used workaround has
always been "turn off secure boot" or sign/import your own keys,
depending on the problems encountered.

>WHY THE HELL DIDN'T YOU START OFF BY EXPLAINING THAT IN THE FIRST
> PLACE WHEN PEOPLE ASKED WHY THE TIE-IN EXISTED?
>
> Sorry for shouting, but really. We have a thread of just *how* many
> email messages that asked for the explanation for this? All we got was
> incomprehensible and illogical crap explanations.
>
> If there actually was a good explanation for the tie-in, it should
> have been front-and-center and explained as such.
>

Honestly, yes, the major distros have been shipping this patch set for
years now, and every time it comes to upstream, the same damn
arguments emerge.  I do not disagree that there are uses for lockdown
outside of secure boot, provided you have some other mechanism to
verify your chain, I believe chrome OS does. But the tie to secure
boot is because that is the use case that users have been using for
years, it was discussed at kernel summit quite a while ago, plans went
forward there seemed to be agreement, and when it comes time for a
pull request, people come out of the woodwork with an expectation that
it solves every problem or it doesn't need to exist. What is here is a
good starting point. I would expect that if it were merged, others
would build upon that and use much of the code already in place to
extend it. It is tied to secure boot because that is what has been
using this for years as it never seems to get upstream.  I am sure
that once it does finally land, it can and will be extended to other
things, but I don't think I would want to spend a lot of time trying
to leverage another external patch set that has been delayed upstream
so many times until it actually did land.
As for the ties to MS that come up every time, and have here as well,
there is no requirement on the MS signature. You can import your own
keys if you don't want them involved, I keep a "test key" imported for
actually running what I build locally.
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [GIT PULL] Kernel lockdown for secure boot

2018-04-03 Thread Matthew Garrett
On Tue, Apr 3, 2018 at 5:56 PM Linus Torvalds

wrote:

> On Tue, Apr 3, 2018 at 5:46 PM, Matthew Garrett  wrote:
> >
> > The generic distros have been shipping this policy for the past 5 years.

> .. so apparently it doesn't actually break things? Why not enable it
> by default then?

Because it does break things, and the documented fix is "Disable Secure
Boot by running mokutil --disable-validation".

> And if "turn off secure boot" really is the accepted - and actuially
> used - workaround for the breakage, then

> WHY THE HELL DIDN'T YOU START OFF BY EXPLAINING THAT IN THE FIRST
> PLACE WHEN PEOPLE ASKED WHY THE TIE-IN EXISTED?

> Sorry for shouting, but really. We have a thread of just *how* many
> email messages that asked for the explanation for this? All we got was
> incomprehensible and illogical crap explanations.

There are four cases:

Verified Boot off, lockdown off: Status quo in distro and mainline kernels
Verified Boot off, lockdown on: Perception of security improvement that's
trivially circumvented (and so bad)
Verified Boot on, lockdown off: Perception of security improvement that's
trivially circumvented (and so bad), status quo in mainline kernels
Verified Boot on, lockdown on: Security improvement, status quo in distro
kernels

Of these four options, only two make sense. The most common implementation
of Verified Boot on x86 platforms is UEFI Secure Boot, so this patchset
includes an option that (if set) results in the kernel doing the right
thing without user intervention. This makes it easy for a user to switch
between the two states that make sense by running a single command and then
following some prompts on the next reboot. The alternative would be to
provide a signed kernel that always enabled lockdown and an unsigned kernel
that didn't, which would (a) increase load on distributions and (b) force
users to both run mokutil --disable-validation and also install a different
kernel.

I'm sorry if I've appeared tetchy in this discussion - having several of my
coworkers shot has not done wonders for my mood.
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [GIT PULL] Kernel lockdown for secure boot

2018-04-03 Thread Linus Torvalds
On Tue, Apr 3, 2018 at 5:25 PM, Linus Torvalds
 wrote:
>
> Honestly, I don't think the patchset is viable at all in that case.

.. or rather, it's probably viable only for distributions that already
have reasons to only care about controlled hardware environments, ie
Chromebooks etc.

But a chome OS install wouldn't care about the whole "secure boot or
not" issue anyway, because they'd also control that side, an they
might as well just enable it unconditionally.

In contrast, the generic distros can't enable it anyway if it breaks
random hardware.  And it wouldn't be about secure boot or not, but
about the random hardware choice.

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


Re: [GIT PULL] Kernel lockdown for secure boot

2018-04-03 Thread Linus Torvalds
On Tue, Apr 3, 2018 at 5:16 PM, Matthew Garrett  wrote:
>
> I ignored it because it's not a viable option. Part of the patchset
> disables various kernel command line options. If there's a kernel command
> line option that disables the patchset then it's pointless.

Honestly, I don't think the patchset is viable at all in that case.

No way will any sane distribution take it, potentially breaking a lot
of machines, and have no way to unbreak them except for "oh, btw, you
have to disable secure boot to get things to work again".

That would be insane.

So you'd better allow some command line options.

One reasonable option may be to just disable lockdown by default (to
make machines work reliably), and then have a "if you're anal about
security, add 'lockdown' to the kernel command line".

People who care about this already need to check the secure boot
status, so this would be just one more thing they'd check.

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


Re: [GIT PULL] Kernel lockdown for secure boot

2018-04-03 Thread Andy Lutomirski
On Tue, Apr 3, 2018 at 5:17 PM, Jann Horn  wrote:
> On Wed, Apr 4, 2018 at 2:06 AM, Linus Torvalds
>  wrote:
>> On Tue, Apr 3, 2018 at 4:59 PM, Matthew Garrett  wrote:
>>>
>>> Ok. So we can build distribution kernels that *always* have this on, and to
>>> turn it off you have to disable Secure Boot and install a different kernel.
>>
>> Bingo.
>>
>> Exactly like EVERY OTHER KERNEL CONFIG OPTION.
>>
>> Just like all the ones that I've mentioned several times.
>>
>> Or, like a lot of other kernel options, maybe have a way to just
>> disable it on the kernel command line, and let the user know about it.
>>
>> That would still be better than disabling secure boot entirely in your
>> world view, so it's (a) more convenient and (b) better.
>>
>> Again, in no case does it make sense to tie it into "how did we boot".
>> Because that's just inconvenient for everybody.
>
> Without taking a stance regarding whether I think that kernel lockdown
> makes sense, I think Matthew's point is this:
> If you don't have lockdown, secure boot doesn't provide a benefit,
> since an attacker could just modify the init binary instead of messing
> with your kernel.
> If you have secure boot, you want lockdown to prevent chainloading
> into a backdoored version of the real OS.

I don't think that's the argument here.  Secure boot can be used to
protect initramfs, since initramfs comes from the secure boot-verified
bootloader.  That verified initramfs can protect the init binary.

As far as I can tell, what's really going on here is that there's a
significant contingent here that wants to prevent Linux from
chainloading something that isn't Linux.  (There doesn't seem to be a
real benefit to preventing Linux from chainloading Linux, since the
chainloaded Linux is unlikely to let the attacker do much that the
original rooted Linux kernel wouldn't have allowed.)  In particular,
Microsoft, which de facto controls most of the secure boot key
ecosystem, doesn't want Windows to be chainloaded without having its
signature verified.

I admit I'm not quite sure why Microsoft considers this important.
They already require a TPM for all new systems, and any important
secrets can be sealed by the TPM such that a maliciously chainloaded
Windows kernel couldn't access those secrets.  But secure boot
predates the WHQL TPM requirement if I remember correctly, and I
suspect that we're seeing leftover requirements from
secure-boot-but-no-TPM era.
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [GIT PULL] Kernel lockdown for secure boot

2018-04-03 Thread David Howells
Linus Torvalds  wrote:

> ...  use the kernel command line to disable things.

An attacker could then modify grub.cfg, say, and cause a reboot (or wait for
the next reboot) to disable lockdown:-/

And whilst we could also distribute a non-locked-down variant of the kernel as
an alternative, the attacker could install and boot that instead since we
can't lock package installation down very easily since it doesn't impinge
directly on the running kernel.

Unfortunately, it's hard to come up with a disablement mechanism in the kernel
that an attacker can't also make use of:-/

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


Re: [GIT PULL] Kernel lockdown for secure boot

2018-04-03 Thread Matthew Garrett
On Tue, Apr 3, 2018 at 5:18 PM Andy Lutomirski  wrote:

> if your secure boot-enabled bootloader can't prevent a bad guy from
> using malicious kernel command line parameters, then fix it.

How is a bootloader supposed to know what the set of malicious kernel
command line parameters is?
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [GIT PULL] Kernel lockdown for secure boot

2018-04-03 Thread Andy Lutomirski
On Tue, Apr 3, 2018 at 5:16 PM, Matthew Garrett  wrote:
> On Tue, Apr 3, 2018 at 5:15 PM Linus Torvalds
> 
> wrote:
>> On Tue, Apr 3, 2018 at 5:10 PM, Matthew Garrett  wrote:
>> >
>> >> Exactly like EVERY OTHER KERNEL CONFIG OPTION.
>> >
>> > So your argument is that we should make the user experience worse?
> Without
>> > some sort of verified boot mechanism, lockdown is just security theater.
>> > There's no good reason to enable it unless you have some mechanism for
>> > verifying that you booted something you trust.
>
>> Wow. Way to snip the rest of the email where I told you what the
>> solution was. Let me repeat it here, since you so conveniently missed
>> it and deleted it:
>
> I ignored it because it's not a viable option. Part of the patchset
> disables various kernel command line options. If there's a kernel command
> line option that disables the patchset then it's pointless.

if your secure boot-enabled bootloader can't prevent a bad guy from
using malicious kernel command line parameters, then fix it.
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [GIT PULL] Kernel lockdown for secure boot

2018-04-03 Thread Jann Horn
On Wed, Apr 4, 2018 at 2:06 AM, Linus Torvalds
 wrote:
> On Tue, Apr 3, 2018 at 4:59 PM, Matthew Garrett  wrote:
>>
>> Ok. So we can build distribution kernels that *always* have this on, and to
>> turn it off you have to disable Secure Boot and install a different kernel.
>
> Bingo.
>
> Exactly like EVERY OTHER KERNEL CONFIG OPTION.
>
> Just like all the ones that I've mentioned several times.
>
> Or, like a lot of other kernel options, maybe have a way to just
> disable it on the kernel command line, and let the user know about it.
>
> That would still be better than disabling secure boot entirely in your
> world view, so it's (a) more convenient and (b) better.
>
> Again, in no case does it make sense to tie it into "how did we boot".
> Because that's just inconvenient for everybody.

Without taking a stance regarding whether I think that kernel lockdown
makes sense, I think Matthew's point is this:
If you don't have lockdown, secure boot doesn't provide a benefit,
since an attacker could just modify the init binary instead of messing
with your kernel.
If you have secure boot, you want lockdown to prevent chainloading
into a backdoored version of the real OS.
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [GIT PULL] Kernel lockdown for secure boot

2018-04-03 Thread Matthew Garrett
On Tue, Apr 3, 2018 at 5:15 PM Linus Torvalds

wrote:
> On Tue, Apr 3, 2018 at 5:10 PM, Matthew Garrett  wrote:
> >
> >> Exactly like EVERY OTHER KERNEL CONFIG OPTION.
> >
> > So your argument is that we should make the user experience worse?
Without
> > some sort of verified boot mechanism, lockdown is just security theater.
> > There's no good reason to enable it unless you have some mechanism for
> > verifying that you booted something you trust.

> Wow. Way to snip the rest of the email where I told you what the
> solution was. Let me repeat it here, since you so conveniently missed
> it and deleted it:

I ignored it because it's not a viable option. Part of the patchset
disables various kernel command line options. If there's a kernel command
line option that disables the patchset then it's pointless.
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [GIT PULL] Kernel lockdown for secure boot

2018-04-03 Thread Linus Torvalds
On Tue, Apr 3, 2018 at 5:10 PM, Matthew Garrett  wrote:
>
>> Exactly like EVERY OTHER KERNEL CONFIG OPTION.
>
> So your argument is that we should make the user experience worse? Without
> some sort of verified boot mechanism, lockdown is just security theater.
> There's no good reason to enable it unless you have some mechanism for
> verifying that you booted something you trust.

Wow. Way to snip the rest of the email where I told you what the
solution was. Let me repeat it here, since you so conveniently missed
it and deleted it:

>> Or, like a lot of other kernel options, maybe have a way to just
>> disable it on the kernel command line, and let the user know about it.
>>
>> That would still be better than disabling secure boot entirely in your
>> world view, so it's (a) more convenient and (b) better.

Matthew, it's simply not worth continuing talking with you.

I'll just not pull this crap, and vendors that you convince to do
stupid things have only themselves to blame.

You clearly have an agenda, and are not willing to look at arguments
against your idiotic choices.

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


Re: [GIT PULL] Kernel lockdown for secure boot

2018-04-03 Thread Matthew Garrett
On Tue, Apr 3, 2018 at 5:08 PM Linus Torvalds

wrote:
> Still better than telling them to disable/enable secure boot, which
> they may or may not even be able to to.

Users who can boot a non-vendor Linux distribution on their platform can
disable Secure Boot 100% of the time.
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [GIT PULL] Kernel lockdown for secure boot

2018-04-03 Thread Matthew Garrett
On Tue, Apr 3, 2018 at 5:06 PM Linus Torvalds

wrote:

> On Tue, Apr 3, 2018 at 4:59 PM, Matthew Garrett  wrote:
> >
> > Ok. So we can build distribution kernels that *always* have this on,
and to
> > turn it off you have to disable Secure Boot and install a different
kernel.

> Bingo.

> Exactly like EVERY OTHER KERNEL CONFIG OPTION.

So your argument is that we should make the user experience worse? Without
some sort of verified boot mechanism, lockdown is just security theater.
There's no good reason to enable it unless you have some mechanism for
verifying that you booted something you trust.
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [GIT PULL] Kernel lockdown for secure boot

2018-04-03 Thread Linus Torvalds
On Tue, Apr 3, 2018 at 5:04 PM, Matthew Garrett  wrote:
>
> How? When there are random DMA-capable PCI devices that are driven by
> userland tools that are mmap()ing the BARs out of sysfs, how do we
> simultaneously avoid breaking those devices while also preventing the
> majority of users from being vulnerable to an attacker just DMAing over the
> kernel?

.. if that ends up being a real problem, then you print a warning and
tell people to use the kernel command line to disable things.

And if it's a big and common problem, then the answer may be that
lockdown has to be entirely OFF by default, and you instead just tell
people to enable it manually with a kernel command line option.

Still better than telling them to disable/enable secure boot, which
they may or may not even be able to to.

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


Re: [GIT PULL] Kernel lockdown for secure boot

2018-04-03 Thread Linus Torvalds
On Tue, Apr 3, 2018 at 4:59 PM, Matthew Garrett  wrote:
>
> Ok. So we can build distribution kernels that *always* have this on, and to
> turn it off you have to disable Secure Boot and install a different kernel.

Bingo.

Exactly like EVERY OTHER KERNEL CONFIG OPTION.

Just like all the ones that I've mentioned several times.

Or, like a lot of other kernel options, maybe have a way to just
disable it on the kernel command line, and let the user know about it.

That would still be better than disabling secure boot entirely in your
world view, so it's (a) more convenient and (b) better.

Again, in no case does it make sense to tie it into "how did we boot".
Because that's just inconvenient for everybody.

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


Re: [GIT PULL] Kernel lockdown for secure boot

2018-04-03 Thread Matthew Garrett
On Tue, Apr 3, 2018 at 5:02 PM Linus Torvalds

wrote:

> On Tue, Apr 3, 2018 at 4:47 PM, Matthew Garrett  wrote:
> >> Another way of looking at this: if lockdown is a good idea to enable
> >> when you booted using secure boot, then why isn't it a good idea when
> >> you *didn't* boot using secure boot?
> >
> > Because it's then trivial to circumvent and the restrictions aren't
worth
> > the benefit.

> Bullshit.

> If there those restrictions cause problems, they need to be fixed
regardless.

How? When there are random DMA-capable PCI devices that are driven by
userland tools that are mmap()ing the BARs out of sysfs, how do we
simultaneously avoid breaking those devices while also preventing the
majority of users from being vulnerable to an attacker just DMAing over the
kernel?
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [GIT PULL] Kernel lockdown for secure boot

2018-04-03 Thread Linus Torvalds
On Tue, Apr 3, 2018 at 4:47 PM, Matthew Garrett  wrote:
>> Another way of looking at this: if lockdown is a good idea to enable
>> when you booted using secure boot, then why isn't it a good idea when
>> you *didn't* boot using secure boot?
>
> Because it's then trivial to circumvent and the restrictions aren't worth
> the benefit.

Bullshit.

If there those restrictions cause problems, they need to be fixed regardless.

In fact, from a debuggability standpoint, you want to find the
problems early, on those kernel development machines that had secure
boot explicitly turned off because it's such a pain.

And if they can't be fixed, then the user is going to disable lockdown
regardless of how he booted the machine.

In no situation is "depending on how you booted" a good choice.

Either you can enable it or you can't. If you can, good. And if you
can't, it has nothing to do with secure boot.

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


Re: [GIT PULL] Kernel lockdown for secure boot

2018-04-03 Thread Matthew Garrett
On Tue, Apr 3, 2018 at 4:55 PM Linus Torvalds

wrote:

> On Tue, Apr 3, 2018 at 4:45 PM, Matthew Garrett  wrote:
> >> Be honest now. It wasn't generally users who clamored for it.
> >
> > If you ask a user whether they want a system that lets an attacker
replace
> > their kernel or one that doesn't, what do you think their answer is
likely
> > to be?

> Goddamnit.

> We both know what the answer will be.

> And it will have *nothing* to do with secure boot.

Right, because they care about outcome rather than mechanism. Secure Boot
is the mechanism we have to make that outcome possible.

> > Again, what is your proposed mechanism for ensuring that off the shelf
> > systems can be configured in a way that makes this possible?

> If you think lockdown is a good idea, and you enabled it, then IT IS
ENABLED.

Ok. So we can build distribution kernels that *always* have this on, and to
turn it off you have to disable Secure Boot and install a different kernel.
Or we can build distribution kernels that only have this on when you're
booting in a context that makes sense, and you can disable it by just
disabling Secure Boot (by running mokutil --disable-validation) and not
have to install a new kernel. Which outcome do you prefer?
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [GIT PULL] Kernel lockdown for secure boot

2018-04-03 Thread Linus Torvalds
On Tue, Apr 3, 2018 at 4:56 PM, David Howells  wrote:
=>
> Most users haven't even given this a moment's thought, aren't even aware of
> the issues, don't even know to ask and, for them, it makes no difference.
> They trust their distribution to deal with stuff they don't know about.

Right.

Like perhaps trusting the distribution to just enable all those
security measures _regaredless_ of whether they booted in using secure
boot or not?

See?

If lockdown breaks something, the distro would need to fix it
regardless of secure boot.

So why is the enablement dependent on it again?

I'm not arguing "lockdown shouldn't be on".

I'm arguing "lockdown being on or off has _nothing_ to do with whether
the machine was booted in EFI mode with secure boot or not".

You don't seem to get it.

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


Re: [GIT PULL] Kernel lockdown for secure boot

2018-04-03 Thread David Howells
Linus Torvalds  wrote:

> Be honest now. It wasn't generally users who clamored for it.
> ...
> If the user actually wanted it, and is asking for it, he can enable it.

>From the distributions' point of view, this is a rubbish argument.

Most users haven't even given this a moment's thought, aren't even aware of
the issues, don't even know to ask and, for them, it makes no difference.
They trust their distribution to deal with stuff they don't know about.

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


Re: [GIT PULL] Kernel lockdown for secure boot

2018-04-03 Thread Linus Torvalds
On Tue, Apr 3, 2018 at 4:45 PM, Matthew Garrett  wrote:
>> Be honest now. It wasn't generally users who clamored for it.
>
> If you ask a user whether they want a system that lets an attacker replace
> their kernel or one that doesn't, what do you think their answer is likely
> to be?

Goddamnit.

We both know what the answer will be.

And it will have *nothing* to do with secure boot.

So *you* be honest now.

Because you clearly aren't.

Seriously. Go ask that question to a random person:

 "Do you want a system that lets an attacker replace their kernel or
one that doesn't?"

and don't ask anything else.

Do you really think they'll answer "no, I don't want an attacker to
replace my kernel, but only if I booted with secure boot"?

Honestly, now.

> Again, what is your proposed mechanism for ensuring that off the shelf
> systems can be configured in a way that makes this possible?

If you think lockdown is a good idea, and you enabled it, then IT IS ENABLED.

No idiotic "secure boot or not" garbage.

Because secure boot or not isn't *relevant*.

Christ, we already have things like

 - CONFIG_STRICT_KERNEL_RWX

 - CONFIG_STRICT_DEVMEM

 - CONFIG_HARDENED_USERCOPY

 - CONFIG_MODULE_SIG_ALL (and friends)

and absolutely *NONE* of them depend on whether the kernel was booted
with secure boot or not.

And I claim that it would be completely idiotic and broken if they did.

And - not entirely unrelated - I claim that it is COMPLETELY IDIOTIC
AND BROKEN to make some new "lockdown" option depend on it.

Comprende?

Really. Your arguments make no sense. They are all fundamentally
broken for the simple reason that all your "but secure boot implies
XYZ" are pure and utter bullshit, because all your arguments are valid
whether secure boot happened or not.

See? Secure boot has *NOTHING* do to with anything.  It has nothing to
do with loading only signed kernel modules. It has nothing to do with
your lockdown patches.

Either lockdown is good or not. It's that simple. But the goodness has
nothing to do with secure boot.

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


Re: [GIT PULL] Kernel lockdown for secure boot

2018-04-03 Thread Andy Lutomirski
On Tue, Apr 3, 2018 at 4:39 PM, David Howells  wrote:
> Linus Torvalds  wrote:
>
>> The same thing is true of some lockdown patch. Maybe it's a good thing
>> in general. But whether it's a good thing is _entirely_ independent of
>> any secure boot issue. I can see using secure boot without it, but I
>> can very much also see using lockdown without secure boot.
>>
>> The two things are simply entirely orthogonal. They have _zero_
>> overlap. I'm not seeing why they'd be linked at all in any way.
>
> I'm not sure I agree.  Here's my reasoning:
>
>  (1) Lockdown mode really needs to activated during kernel boot, before
>  userspace has a chance to run, otherwise there's a window of opportunity
>  in which the kernel *isn't* locked down.

That's simply not true.  A sensible verified boot chain (a la Chrome
OS) is likely to load, as one verified chunk, a kernel and initramfs.
Then initramfs can flip on lockdown all by itself before it enables
networking or any other attack vectors.
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [GIT PULL] Kernel lockdown for secure boot

2018-04-03 Thread Matthew Garrett
On Tue, Apr 3, 2018 at 4:39 PM Linus Torvalds

wrote:

> On Tue, Apr 3, 2018 at 4:26 PM, Linus Torvalds
>  wrote:
> >
> > Magically changing kernel behavior depending on some subtle and often
> > unintentional bootup behavior detail is completely idiotic.

> Another way of looking at this: if lockdown is a good idea to enable
> when you booted using secure boot, then why isn't it a good idea when
> you *didn't* boot using secure boot?

Because it's then trivial to circumvent and the restrictions aren't worth
the benefit.
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [GIT PULL] Kernel lockdown for secure boot

2018-04-03 Thread Matthew Garrett
On Tue, Apr 3, 2018 at 4:26 PM Linus Torvalds

wrote:

> On Tue, Apr 3, 2018 at 4:17 PM, Matthew Garrett  wrote:
> >
> > 1) Secure Boot is intended to permit the construction of a boot chain
that
> > only runs ring 0 code that the user considers trustworthy

> No.

> That may be *one* intention, for some people.

> It's not an a-priori one for the actual user.

Secure Boot is intended to *permit* that. Without Secure Boot you're unable
to do that. Some users want that. Some users don't.

> > 2) Allowing arbitrary user code to run in ring 0 without affirmative
> > consent on the part of the user is therefore incompatible with the
goals of
> > Secure Boot

> Again, that has absolutely zero relevance.

> Those goals are not the *users* goals.

> Be honest now. It wasn't generally users who clamored for it.

If you ask a user whether they want a system that lets an attacker replace
their kernel or one that doesn't, what do you think their answer is likely
to be?

> If the user actually wanted it, and is asking for it, he can enable
> it. Independently of secure boot, which the user generally has little
> control over.

How? If the bootloader will boot kernels that don't impose this restriction
then an attacker just replaces whatever's enabling that feature. And, uh,
seriously, I've been asking for *years* for someone to point me at a PC on
the market that doesn't give the user control over Secure Boot, but Shim
was expressly designed to ensure that the user would have the ability to
enroll additional trusted keys (or disable signature validation entirely),
so which cases are you thinking of where the user doesn't have control?

> > 3) This patchset provides a mechanism to alter the behaviour of the
kernel
> > such that it is significantly more difficult for arbitrary user code to
run
> > in ring 0 without affirmative user consent

> That difficulty already exists, the new thing isn't somehow related to
> that at all.

> Look at our "uyou can only load modules if you're root" rules. Or the
> "you can only load modules if they are signed".

> See a pattern there? They don't magically enable themselves (or
> disable themselves) depending on whether you booted with secure boot
> or not.

What's the benefit of "You can only load modules if they are signed" if
root is able to just overwrite that policy bit in the kernel? The split
between unprivileged users and root is real, but right now module
signatures are theater - there's no significant security benefit from them.
But the reason to tie this to Secure Boot is that without that an attacker
who has root can just replace the kernel on disk (or patch the bootloader
to live-patch the kernel on boot, and yes that's an attack we've seen in
the real world), so while it's a feature that is arguably beneficial under
all circumstances it's a feature that only has significant benefit if you
have some way to actually validate what you're booting in the first place.

> > 4) Providing a mechanism for automatically enabling this behaviour when
> > running in a context that is intended to restrict access to ring 0 is a
> > rational thing to do, because otherwise it is difficult to achieve the
> > objective in (1)

> No. See why it's *NOT* rational, as explained already several times.

> Magically changing kernel behavior depending on some subtle and often
> unintentional bootup behavior detail is completely idiotic.

> It would be idiotic if it was that "check kernel module signatures"
> check. This is no less idiotic.

> Seriously, listen to your own arguments. If they don't make sense for
> checking kernel module signatures, why the hell would they make sense
> for something like lockdown.

> THE TWO THINGS ARE ENTIRELY INDEPENDENT.

Again, what is your proposed mechanism for ensuring that off the shelf
systems can be configured in a way that makes this possible?
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [GIT PULL] Kernel lockdown for secure boot

2018-04-03 Thread Andy Lutomirski
On Tue, Apr 3, 2018 at 4:12 PM, David Howells  wrote:
> Andy Lutomirski  wrote:
>
>> I'm having a very, very hard time coming up with a scenario where I
>> can "trust" something if an attacker can get root but can't modify the
>> running kernel image but I can't "trust" something if the attacker
>> can [modify the running kernel image].
>
> (I think the above is what you meant)
>
> Let's go at this a different way.  How do you decide you can trust something
> in this context?  You compare it to something.  Signing it, keeping a hash
> whitelist, IMA - these are all ways of comparing something.  Do you agree with
> that?

I trust or distrust a system as a whole.  I don't make that decision
by comparing it to anything.  I make it by evaluating how the system
works and deciding whether it's trustworthy.

>
> What use is secure boot if processes run as root can subvert your kernel?
>

Secure boot serves several purposes:

1. Anti-competitive purposes.  It's intentionally difficult to run
non-Windows OSes on Windows ARM machines, for example.

2. Allowing me to use a stock UEFI machine to have a verified boot chain.

The latter has nothing whatsoever to do with CPL0.  The former,
however, does.  If I could easily write some Windows program to run
CPL0 code, then I could chainload Linux using the Windows image, and
I've subverted the purpose.

Cynical?  Yes.

>> > There's no point bothering with UID/GID checking either.
>>
>> Give me a break.  There's a *huge* difference between a system where
>> only root can load unsigned modules and a system where anyone can load
>> unsigned modules.
>
> I don't think we've ever advocated letting just anyone load a module.
>
> But my point is that if you can modify the running kernel, you can nullify all
> security checks, including UID/GID checks.
>
>> > However, if /dev/mem can be read, any root process can extract the session
>> > key for your disk.
>>
>> Any root process can read /dev/mapper/plaintext_disk, lockdown or otherwise.
>
> True - for now - and they can also access the mounted filesystem.  But if they
> get their hands on your powered-off computer, no, they can't.

This is, IMO, a silly argument.  You're saying that some bad guy has
managed to run code as root on my laptop.  Then, next week, the bad
guy steals my laptop while it's powered off, and Lockdown is supposed
to protect me against that bad guy.  If this happens, I've already
lost completely, lockdown or no.
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [GIT PULL] Kernel lockdown for secure boot

2018-04-03 Thread Linus Torvalds
On Tue, Apr 3, 2018 at 4:26 PM, Linus Torvalds
 wrote:
>
> Magically changing kernel behavior depending on some subtle and often
> unintentional bootup behavior detail is completely idiotic.

Another way of looking at this: if lockdown is a good idea to enable
when you booted using secure boot, then why isn't it a good idea when
you *didn't* boot using secure boot?

That's the flip side of this whole argument.

People who boot without secure boot may be wanting all the same
protections. Maybe you have to disable it when you build your own
kernel, for example. Does that suddenly mean that lockdown is now a
bad idea?

And if it does, explain it. Explain why it's a bad idea to enable
without secure boot, but is a good idea to enable *with* secure boot.

In other words: explain the tie-in.

Because I really don't see it. All I see is illogical blathering that
tries to conflate issues that have nothing to do with each other.

Please explain to me why a distro or a user would want lockdown to be
disabled just because the user didn't use secure boot, but suddenly if
it's booted on another machine, it's not just a good idea, but
mandatory in your world view?

Honestly, if I were a distro maintainer, the *last* thing I'd want is
the kernel to act fundamentally differently in some security context
depending on some random bootup condition.

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


Re: [GIT PULL] Kernel lockdown for secure boot

2018-04-03 Thread David Howells
Linus Torvalds  wrote:

> The same thing is true of some lockdown patch. Maybe it's a good thing
> in general. But whether it's a good thing is _entirely_ independent of
> any secure boot issue. I can see using secure boot without it, but I
> can very much also see using lockdown without secure boot.
> 
> The two things are simply entirely orthogonal. They have _zero_
> overlap. I'm not seeing why they'd be linked at all in any way.

I'm not sure I agree.  Here's my reasoning:

 (1) Lockdown mode really needs to activated during kernel boot, before
 userspace has a chance to run, otherwise there's a window of opportunity
 in which the kernel *isn't* locked down.

 (2) If the kernel isn't booted in secure boot mode, then there's the
 opportunity to tamper before the kernel even starts booting.

 (3) There doesn't seem any point in booting in secure boot mode if you don't
 protect the running kernel image against tampering.  What does it mean to
 be in "secure boot mode" in that case?  If the kernel can be tampered
 with, it would seem to be, by definition, insecure.

 (4) You can't validly promise the next OS you kexec that *it* is started in
 secure boot mode if you don't stop your image from being tampered with.
 Note that this doesn't prevent a compromised kernel from lying to the
 next OS.

 (5) Tampering with a running kernel can be achieved in a variety of ways:
 loading of arbitrary modules, loading of modified firmware, direct access
 to devices that can effect DMA, writing to /dev/mem, ...

 (6) We need to be able to load modules and firmware, but these can be signed,
 hashed or measured so we have some idea of their provenance - but signing
 can be worked around if, say, /dev/mem is writable.

 (7) If you told the BIOS[*] that you want to be in secure boot mode, then the
 kernel should honour that and try to prevent tampering with the image.

 (8) Turning lockdown mode on if the kernel is booted in secure boot seems to
 be the way to achieve this.

 (9) BIOS vendors can blacklist any of the components - say the SHIM - to
 prevent an insecure kernel from being used to compromise and kexec
 another OS.

Note that I've provided a kernel command line parameter that will turn
lockdown mode on arbitrarily - but that can be turned off by editing the
parameters in grub.cfg, say.

David

[*] Yeah, I know, this is an x86-centric view.
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [GIT PULL] Kernel lockdown for secure boot

2018-04-03 Thread Linus Torvalds
On Tue, Apr 3, 2018 at 4:12 PM, David Howells  wrote:
>
> What use is secure boot if processes run as root can subvert your kernel?

Stop this idiocy.

The above has now been answered multiple times, several different ways.

The "point" of secure boot may be that you had no choice, or there was
no point at all, it just came that way.

Or the "point" of secure boot may be that you don't trust anybody else
than yourself, but once you've booted you do trust what you booted.

But the *real* point is that this has nothing what-so-ever to do with
secure boot. You may want (or not want) lockdown independently of it.
Don't tie magic boot issues with kernel runtime behavior.

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


Re: [GIT PULL] Kernel lockdown for secure boot

2018-04-03 Thread Linus Torvalds
On Tue, Apr 3, 2018 at 4:17 PM, Matthew Garrett  wrote:
>
> 1) Secure Boot is intended to permit the construction of a boot chain that
> only runs ring 0 code that the user considers trustworthy

No.

That may be *one* intention, for some people.

It's not an a-priori one for the actual user.

> 2) Allowing arbitrary user code to run in ring 0 without affirmative
> consent on the part of the user is therefore incompatible with the goals of
> Secure Boot

Again, that has absolutely zero relevance.

Those goals are not the *users* goals.

Be honest now. It wasn't generally users who clamored for it.

If the user actually wanted it, and is asking for it, he can enable
it. Independently of secure boot, which the user generally has little
control over.

> 3) This patchset provides a mechanism to alter the behaviour of the kernel
> such that it is significantly more difficult for arbitrary user code to run
> in ring 0 without affirmative user consent

That difficulty already exists, the new thing isn't somehow related to
that at all.

Look at our "uyou can only load modules if you're root" rules. Or the
"you can only load modules if they are signed".

See a pattern there? They don't magically enable themselves (or
disable themselves) depending on whether you booted with secure boot
or not.

Yet they are a HELL OF A LOT MORE IMPORTANT than this new patch series.

> 4) Providing a mechanism for automatically enabling this behaviour when
> running in a context that is intended to restrict access to ring 0 is a
> rational thing to do, because otherwise it is difficult to achieve the
> objective in (1)

No. See why it's *NOT* rational, as explained already several times.

Magically changing kernel behavior depending on some subtle and often
unintentional bootup behavior detail is completely idiotic.

It would be idiotic if it was that "check kernel module signatures"
check. This is no less idiotic.

Seriously, listen to your own arguments. If they don't make sense for
checking kernel module signatures, why the hell would they make sense
for something like lockdown.

THE TWO THINGS ARE ENTIRELY INDEPENDENT.

I'm done with you. You're not listening, and you're repeating bogus
arguments that make no sense.

No way in hell will I merge anything like this.

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


Re: [GIT PULL] Kernel lockdown for secure boot

2018-04-03 Thread Matthew Garrett
On Tue, Apr 3, 2018 at 4:08 PM Linus Torvalds

wrote:

> That's not the right approach to begin with, Matthew.  The onus is on
> *you* to explain why you tied them together, not on others to explain
> to you - over and over - that they have nothing to do with each other.

1) Secure Boot is intended to permit the construction of a boot chain that
only runs ring 0 code that the user considers trustworthy
2) Allowing arbitrary user code to run in ring 0 without affirmative
consent on the part of the user is therefore incompatible with the goals of
Secure Boot
3) This patchset provides a mechanism to alter the behaviour of the kernel
such that it is significantly more difficult for arbitrary user code to run
in ring 0 without affirmative user consent
4) Providing a mechanism for automatically enabling this behaviour when
running in a context that is intended to restrict access to ring 0 is a
rational thing to do, because otherwise it is difficult to achieve the
objective in (1)

Alternative approaches to achieve (1) rely on severely constraining
userland - ChromeOS, for instance, doesn't impose these restrictions at
present but also doesn't allow users to run arbitrary applications (you're
stuck inside either the Chrome or Android sandbox). So, if the goal is to
achieve (1) when the platform is in this state, what's a more reasonable
alternative?
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [GIT PULL] Kernel lockdown for secure boot

2018-04-03 Thread David Howells
Andy Lutomirski  wrote:

> I'm having a very, very hard time coming up with a scenario where I
> can "trust" something if an attacker can get root but can't modify the
> running kernel image but I can't "trust" something if the attacker
> can [modify the running kernel image].

(I think the above is what you meant)

Let's go at this a different way.  How do you decide you can trust something
in this context?  You compare it to something.  Signing it, keeping a hash
whitelist, IMA - these are all ways of comparing something.  Do you agree with
that?

However, the comparison can be subverted if the running kernel image (I might
be better saying running kernel state here since I'm not talking about the
source bzImage file) can be modified arbitrarily by userspace, either by
modifying the data against which the comparison is made - e.g. the public key
set or the hash list - or by modifying the code that makes the comparison.

/dev/mem, direct access to DMA, bpf, etc. all provide ways of modifying the
kernel image arbitrarily, which leads me to this:

> I *don't* buy into the party line about why signed modules should be needed
> for Secure Boot.

Modules are just another way of modifying the kernel image.  If I can just
create an arbitrary module and load it, then I can modify the kernel image
from within the module.

Locking down modules by signing, hashing or IMA practically prevents the
loading of arbitrarily constructed modules and only permits modules from a set
that the provider of the modules somewhat trusts.

What use is secure boot if processes run as root can subvert your kernel?

> > There's no point bothering with UID/GID checking either.
> 
> Give me a break.  There's a *huge* difference between a system where
> only root can load unsigned modules and a system where anyone can load
> unsigned modules.

I don't think we've ever advocated letting just anyone load a module.

But my point is that if you can modify the running kernel, you can nullify all
security checks, including UID/GID checks.

> > However, if /dev/mem can be read, any root process can extract the session
> > key for your disk.
> 
> Any root process can read /dev/mapper/plaintext_disk, lockdown or otherwise.

True - for now - and they can also access the mounted filesystem.  But if they
get their hands on your powered-off computer, no, they can't.

> But I don't think the upstream kernel should apply a patch that ties any of
> this to Secure Boot without a genuine technical reason why it makes sense.

Because unless you turn lockdown on during kernel boot, there exists a window
of opportunity where the kernel isn't locked down and can be accessed, thereby
obviating the fact that you started in Secure Boot mode.

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


Re: [GIT PULL] Kernel lockdown for secure boot

2018-04-03 Thread Linus Torvalds
On Tue, Apr 3, 2018 at 4:08 PM, Linus Torvalds
 wrote:
>
> This discussion is over until you give an actual honest-to-goodness
> reason for why you tied the two features together. No more "Why not?"
> crap.

Side note: I suspect the reason is something along the lines of "there
are political reasons".

But dammit, if that's the case, those should be documented and
explained, not answered with "why not" when people ask why something
is the case.

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


Re: [GIT PULL] Kernel lockdown for secure boot

2018-04-03 Thread Matthew Garrett
On Tue, Apr 3, 2018 at 3:53 PM Andy Lutomirski  wrote:
> On Tue, Apr 3, 2018 at 3:51 PM, Matthew Garrett  wrote:
> > Lockdown is clearly useful without Secure Boot (and I intend to deploy
it
> > that way for various things), but I still don't understand why you feel
> > that the common case of booting a kernel from a boot chain that's widely
> > trusted derives no benefit from it being harder to subvert that kernel
into
> > subverting that boot chain. For cases where you're self-signing and feel
> > happy about that, you just set CONFIG_LOCK_DOWN_IN_EFI_SECURE_BOOT to n
and
> > everyone's happy?

> I would like to see distros that want Secure Boot to annoy users by
> enabling Lockdown be honest about the fact that it's an annoyance and
> adds very little value by having to carry a patch that was rejected by
> the upstream kernel.

I disagree with the assertion that it adds very little value, but if you
want to reject a technically useful patch for political reasons then I'm
well beyond the point of caring.
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [GIT PULL] Kernel lockdown for secure boot

2018-04-03 Thread Linus Torvalds
On Tue, Apr 3, 2018 at 3:51 PM, Matthew Garrett  wrote:
>
> Lockdown is clearly useful without Secure Boot (and I intend to deploy it
> that way for various things), but I still don't understand why you feel
> that the common case of booting a kernel from a boot chain that's widely
> trusted derives no benefit from it being harder to subvert that kernel into
> subverting that boot chain.

It has NOTHING TO DO WITH "HARDER TO SUBVERT".

THE TWO FEATURES HAVE NOTHING TO DO WITH EACH OTHER WHAT-SO-EVER.

I do not want my kernel to act differently depending on some really
esoteric detail in how it was booted. That is fundamentally wrong.

Is that really so hard to understand?

Look at it this way: maybe lockdown breaks some application because
that app does something odd. I get a report of that happening, and it
so happens that the reporter is running the same distro I am, so I try
it with his exact kernel configuration, and it works for me.

It is *entirely* non-obvious that the reporter happened to run a
distro kernel that had secure boot enabled, and I obviously do not.

See what the problem is? Tying these things magically together IS A BAD IDEA.

And when people ask you why you did it, YOU HAVE YET TO COME UP WITH A
SINGLE ACTUAL RESPONSE.

Instead, you just ask people why they care, or tell people to not enable it.

Seriously, Matthew, it's WRONG to tie things together in magic ways
when they have nothing what-so-ever to do with each other.

So no. The answer is simply "don't tie the two things together".

And dammit, if you tie them together, you had damn well have a good
reason. So far, your reasons have _literally_ been "Why not?" and
tried to make the onus be on others to explain to you why not.

That's not the right approach to begin with, Matthew.  The onus is on
*you* to explain why you tied them together, not on others to explain
to you - over and over - that they have nothing to do with each other.

This discussion is over until you give an actual honest-to-goodness
reason for why you tied the two features together. No more "Why not?"
crap.

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


Re: [GIT PULL] Kernel lockdown for secure boot

2018-04-03 Thread Andy Lutomirski
On Tue, Apr 3, 2018 at 3:51 PM, Matthew Garrett  wrote:
> On Tue, Apr 3, 2018 at 3:46 PM Linus Torvalds
> 
> wrote:
>
>> For example, I love signed kernel modules. The fact that I love them
>> has absolutely zero to do with secure boot, though. There is
>> absolutely no linkage between the two issues: I use (self-)signed
>> kernel modules simply because I think it's a good thing in general.
>
>> The same thing is true of some lockdown patch. Maybe it's a good thing
>> in general. But whether it's a good thing is _entirely_ independent of
>> any secure boot issue. I can see using secure boot without it, but I
>> can very much also see using lockdown without secure boot.
>
>> The two things are simply entirely orthogonal. They have _zero_
>> overlap. I'm not seeing why they'd be linked at all in any way.
>
> Lockdown is clearly useful without Secure Boot (and I intend to deploy it
> that way for various things), but I still don't understand why you feel
> that the common case of booting a kernel from a boot chain that's widely
> trusted derives no benefit from it being harder to subvert that kernel into
> subverting that boot chain. For cases where you're self-signing and feel
> happy about that, you just set CONFIG_LOCK_DOWN_IN_EFI_SECURE_BOOT to n and
> everyone's happy?

I would like to see distros that want Secure Boot to annoy users by
enabling Lockdown be honest about the fact that it's an annoyance and
adds very little value by having to carry a patch that was rejected by
the upstream kernel.

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


Re: [GIT PULL] Kernel lockdown for secure boot

2018-04-03 Thread Matthew Garrett
On Tue, Apr 3, 2018 at 3:46 PM Linus Torvalds

wrote:

> For example, I love signed kernel modules. The fact that I love them
> has absolutely zero to do with secure boot, though. There is
> absolutely no linkage between the two issues: I use (self-)signed
> kernel modules simply because I think it's a good thing in general.

> The same thing is true of some lockdown patch. Maybe it's a good thing
> in general. But whether it's a good thing is _entirely_ independent of
> any secure boot issue. I can see using secure boot without it, but I
> can very much also see using lockdown without secure boot.

> The two things are simply entirely orthogonal. They have _zero_
> overlap. I'm not seeing why they'd be linked at all in any way.

Lockdown is clearly useful without Secure Boot (and I intend to deploy it
that way for various things), but I still don't understand why you feel
that the common case of booting a kernel from a boot chain that's widely
trusted derives no benefit from it being harder to subvert that kernel into
subverting that boot chain. For cases where you're self-signing and feel
happy about that, you just set CONFIG_LOCK_DOWN_IN_EFI_SECURE_BOOT to n and
everyone's happy?
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [GIT PULL] Kernel lockdown for secure boot

2018-04-03 Thread Linus Torvalds
On Tue, Apr 3, 2018 at 3:39 PM, Andy Lutomirski  wrote:
>
> Sure.  I have no problem with having an upstream kernel have a
> lockdown feature, although I think that feature should distinguish
> between reads and writes.  But I don't think the upstream kernel
> should apply a patch that ties any of this to Secure Boot without a
> genuine technical reason why it makes sense.

So this is where I violently agree with Andy.

For example, I love signed kernel modules. The fact that I love them
has absolutely zero to do with secure boot, though. There is
absolutely no linkage between the two issues: I use (self-)signed
kernel modules simply because I think it's a good thing in general.

The same thing is true of some lockdown patch. Maybe it's a good thing
in general. But whether it's a good thing is _entirely_ independent of
any secure boot issue. I can see using secure boot without it, but I
can very much also see using lockdown without secure boot.

The two things are simply entirely orthogonal. They have _zero_
overlap. I'm not seeing why they'd be linked at all in any way.

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


Re: [GIT PULL] Kernel lockdown for secure boot

2018-04-03 Thread Andy Lutomirski
On Tue, Apr 3, 2018 at 3:32 PM, David Howells  wrote:
> Andy Lutomirski  wrote:
>
>> > If the user can arbitrarily modify the running kernel image, you cannot
>> > trust anything.  You cannot determine the trustworthiness of something
>> > because your basis for determining that trust can be compromised.
>>
>> I'm having a very, very hard time coming up with a scenario where I
>> can "trust" something if an attacker can get root but can't modify the
>> running kernel image but I can't "trust" something if the attacker
>> can't.
>
> Eh?  If the attacker can't what?  Did you mean to put "can" at the end of that
> rather than "can't"?  I don't see why the kernel-level trust would be
> compromised if an attacker can't get root and can't modify the running kernel
> image.

Whoops, yes.

>
> Here's a simple scenario: You boot your machine.  You have module verification
> keys in your kernel.  You have /dev/mem available for root to read/write.  A
> program running as root can modify the keys in your kernel or just disable the
> checking code entirely.  It can now insmod any module it likes.  You may as
> well not bother with signed modules.  In fact, it can modify the running
> kernel image in any way it likes, without even having to load modules.

I don't particularly disagree with any of this, but you seem to be
saying "if you've bought into the party line wrt signed modules, you
had better enable lockdown, too".  I *don't* buy into the party line
about why signed modules should be needed for Secure Boot.

> There's no point bothering with UID/GID checking either.

Give me a break.  There's a *huge* difference between a system where
only root can load unsigned modules and a system where anyone can load
unsigned modules.

>
>> > Stopping the kernel from being arbitrarily read stops any encryption keys 
>> > it
>> > may be using from being retrieved.
>>
>> If I build a server that runs Panera Bread 2.0's website, and the
>> attacker exploits my machine to steal tens of millions of customer
>> records by getting the machine to talk to some database server using
>> keys that are securely stored in the kernel keyring, ...
>
> I was thinking more in terms of preventing access to the encrypted data on
> your own disk.  The key for that could be unlocked using a TPM, but the
> session key then has to be retained in RAM for performance reasons unless you
> can transfer the session key to, say, your SATA controller without it going
> through the CPU.
>
> However, if /dev/mem can be read, any root process can extract the session key
> for your disk.

Any root process can read /dev/mapper/plaintext_disk, lockdown or otherwise.

>
> But, as you suggest, they could also protect secrets used in communications.
> However, the communications themselves have to be exposed to userspace for
> userspace to be able to use them.  That is unavoidable.  The kernel keyring,
> for example, tries to restrict who can even see a key, much less use it as
> much as possible - but ptrace() exists...  You are no less vulnerable if the
> key is held in a userspace process; then the attacker can get the key and the
> data.
>
> If the kernel is locked down, the aim is to try and make sure that keys
> stashed in the kernel cannot be read, though they have to be able to be used,
> or there's no point to them.

Sure.  I have no problem with having an upstream kernel have a
lockdown feature, although I think that feature should distinguish
between reads and writes.  But I don't think the upstream kernel
should apply a patch that ties any of this to Secure Boot without a
genuine technical reason why it makes sense.
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [GIT PULL] Kernel lockdown for secure boot

2018-04-03 Thread David Howells
Andy Lutomirski  wrote:

> > If the user can arbitrarily modify the running kernel image, you cannot
> > trust anything.  You cannot determine the trustworthiness of something
> > because your basis for determining that trust can be compromised.
> 
> I'm having a very, very hard time coming up with a scenario where I
> can "trust" something if an attacker can get root but can't modify the
> running kernel image but I can't "trust" something if the attacker
> can't.

Eh?  If the attacker can't what?  Did you mean to put "can" at the end of that
rather than "can't"?  I don't see why the kernel-level trust would be
compromised if an attacker can't get root and can't modify the running kernel
image.

Here's a simple scenario: You boot your machine.  You have module verification
keys in your kernel.  You have /dev/mem available for root to read/write.  A
program running as root can modify the keys in your kernel or just disable the
checking code entirely.  It can now insmod any module it likes.  You may as
well not bother with signed modules.  In fact, it can modify the running
kernel image in any way it likes, without even having to load modules.
There's no point bothering with UID/GID checking either.

> > Stopping the kernel from being arbitrarily read stops any encryption keys it
> > may be using from being retrieved.
> 
> If I build a server that runs Panera Bread 2.0's website, and the
> attacker exploits my machine to steal tens of millions of customer
> records by getting the machine to talk to some database server using
> keys that are securely stored in the kernel keyring, ...

I was thinking more in terms of preventing access to the encrypted data on
your own disk.  The key for that could be unlocked using a TPM, but the
session key then has to be retained in RAM for performance reasons unless you
can transfer the session key to, say, your SATA controller without it going
through the CPU.

However, if /dev/mem can be read, any root process can extract the session key
for your disk.

But, as you suggest, they could also protect secrets used in communications.
However, the communications themselves have to be exposed to userspace for
userspace to be able to use them.  That is unavoidable.  The kernel keyring,
for example, tries to restrict who can even see a key, much less use it as
much as possible - but ptrace() exists...  You are no less vulnerable if the
key is held in a userspace process; then the attacker can get the key and the
data.

If the kernel is locked down, the aim is to try and make sure that keys
stashed in the kernel cannot be read, though they have to be able to be used,
or there's no point to them.

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


Re: [GIT PULL] Kernel lockdown for secure boot

2018-04-03 Thread Andy Lutomirski
On Tue, Apr 3, 2018 at 12:49 PM, David Howells  wrote:
> Andy Lutomirski  wrote:
>
>> >>> A kernel that allows users arbitrary access to ring 0 is just an
>> >>> overfeatured bootloader. Why would you want secure boot in that case?
>> >>
>> >> To get a chain of trust.
>> >
>> > You don't have a chain of trust that you can trust in that case.
>> >
>> Please elaborate on why I can’t trust it.
>
> If the user can arbitrarily modify the running kernel image, you cannot trust
> anything.  You cannot determine the trustworthiness of something because your
> basis for determining that trust can be compromised.

I'm having a very, very hard time coming up with a scenario where I
can "trust" something if an attacker can get root but can't modify the
running kernel image but I can't "trust" something if the attacker
can't.  About the only think I can come up with is that root generally
has a hard time directly reading kernel keyring data.

But if we really think that kernel keyring data is so sacred, let's
please solve it properly using SGX or some hypervisor doodad like
Microsoft's Credential Guard.  Protecting the keyring with lockdown is
a whole lot of annoyance without all that much gain.

>
>> Please also elaborate on how lockdown helps at all.
>
> Stopping the kernel from being arbitrarily modified allows you to preserve
> your trust.

If I build a voting machine, an ATM, or a server that runs Panera
Bread's website, I can certainly issue a press release that says "hey,
the bad guy just downloaded tens of millions of customer records, but
they didn't actually get to run CPL0 code, so all is well."



>
> Stopping the kernel from being arbitrarily read stops any encryption keys it
> may be using from being retrieved.

If I build a server that runs Panera Bread 2.0's website, and the
attacker exploits my machine to steal tens of millions of customer
records by getting the machine to talk to some database server using
keys that are securely stored in the kernel keyring, I would feel
sooo much better knowing that the attacker didn't also manage to
extract the key that lets the machine talk to the database server.
Never mind that the attacker already stole the entire contents of the
database.


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


Re: [GIT PULL] Kernel lockdown for secure boot

2018-04-03 Thread Andy Lutomirski
On Tue, Apr 3, 2018 at 12:29 PM, Matthew Garrett  wrote:
> On Tue, Apr 3, 2018 at 9:46 AM Andy Lutomirski  wrote:
>> On Tue, Apr 3, 2018 at 9:29 AM, Matthew Garrett  wrote:
>> > A kernel that allows users arbitrary access to ring 0 is just an
>> > overfeatured bootloader. Why would you want secure boot in that case?
>
>> To get a chain of trust.  I can provision a system with some public
>> keys, stored in UEFI authenticated variables, such that the system
>> will only boot a signed image.  That signed image, can, in turn, load
>> a signed (or hashed or otherwise verfified) kernel and a verified
>> initramfs.  The initramfs can run a full system from a verified (using
>> dm-verity or similar) filesystem, for example.  Now it's very hard to
>> persistently attack this system.  Chromium OS does something very much
>> like this, except that it doesn't use UEFI as far as I know.  So does
>> iOS, and so do some Android versions.  None of this requires lockdown,
>> or even a separation between usermode and kernelmode, to work
>> correctly.  One could even do this on an MMU-less system if one really
>> cared to.  More usefully, someone probably has done this using a
>> unikernel.
>
> That's only viable if you're the only person with the ability to sign stuff
> for your machine - the moment there are generic distributions that your
> machine trusts, an attacker can use one as a bootloader to compromise your
> trust chain.


If you removed "as a bootloader", then I agree with that sentence.

Can someone please explain why the UEFI crowd cares so much about "as
a bootloader"?  Once I'm able to install an OS (Linux kernel +
bootloader, Windows embedded doodad, OpenBSD, whatever) on your
machine, I can use your peripherals, read your data, write your data,
see your keystrokes, use your network connection, re-flash your BIOS
(at least as well as any OS can), run VMs, and generally own your
system.  Somehow you all seem fine with all of this, except that the
fact that I can chainload something else gives UEFI people the
willies.

Can someone explain why?
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [GIT PULL] Kernel lockdown for secure boot

2018-04-03 Thread James Morris
On Tue, 3 Apr 2018, Ard Biesheuvel wrote:

>  [snip]

Thanks for the input -- there are obviously still issues to be resolved.

I'll now not be pushing these to Linus for v4.17.


-- 
James Morris


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


Re: [GIT PULL] Kernel lockdown for secure boot

2018-04-03 Thread Matthew Garrett
On Tue, Apr 3, 2018 at 2:21 PM Al Viro  wrote:

> On Tue, Apr 03, 2018 at 09:08:54PM +, Matthew Garrett wrote:
> > If you don't want Secure Boot, turn it off. If you want Secure Boot,
use a
> > kernel that behaves in a way that actually increases your security.

> That assumes you *can* turn that shit off.  On the hardware where
manufacturer
> has installed firmware that doesn't allow that SB is a misfeature that has
> to be worked around.  Making that harder might improve the value of SB to
> said manufacturers, but what's the benefit for everybody else?

This is why Shim has support for its own key database, as well as allowing
you to disable further signature validation. If the hardware supports third
party code at all, you can just use Shim to sidestep any unreasonable
restrictions the vendor has imposed.

(This doesn't help with systems that don't support third party code at all,
but this patchset does nothing to make that worse - that hardware wouldn't
boot your own kernel before this patchset, and it won't afterwards either)
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [GIT PULL] Kernel lockdown for secure boot

2018-04-03 Thread Matthew Garrett
On Tue, Apr 3, 2018 at 2:26 PM Linus Torvalds

wrote:

> On Tue, Apr 3, 2018 at 2:08 PM, Matthew Garrett  wrote:
> >
> > Secure Boot ensures that the firmware will only load signed
bootloaders. If
> > a signed bootloader loads a kernel that's effectively an unsigned
> > bootloader, there's no point in using Secure Boot

> Bullshit.

> I may want to know that I'm running *my* kernel, but once that is the
> case, I trust it.

If you don't believe that your self-signed kernel is going to be a threat
against your security model then great! Don't turn this on when you build
it. But if you built a kernel that didn't have this lockdown functionality
and got it signed with, say, Red Hat's signing keys, anyone could take Red
Hat's bootloader chain and that kernel and subvert the Secure Boot chain on
any machine that trusts the third party signing key (ie, basically all of
them)

> Yes, on x86 hardware at least at some point MS actually had the rule
> that it has to be something you can turn off. That rule is apparently
> not true on ARM, though.

Correct - there's no requirement that it be something you can disable on
ARM, but since Microsoft won't sign any third-party code for ARM anyway it
makes no difference to this discussion.

> If you want lockdown, fine, enable it. But what the F*CK does that
> have to do with whether you had secure boot or not?

Because a kernel signed with a generally trusted key that doesn't implement
any lockdown functionality is effectively a bootloader that will load
unsigned material on most machines on the market, which reduces the
security of users running those machines with Secure Boot enabled.
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [GIT PULL] Kernel lockdown for secure boot

2018-04-03 Thread Linus Torvalds
On Tue, Apr 3, 2018 at 2:08 PM, Matthew Garrett  wrote:
>
> Secure Boot ensures that the firmware will only load signed bootloaders. If
> a signed bootloader loads a kernel that's effectively an unsigned
> bootloader, there's no point in using Secure Boot

Bullshit.

I may want to know that I'm running *my* kernel, but once that is the
case, I trust it.

In fact, I tend to trust it more than some random vendor key. You should too.

Your whole argument is FUNDAMENTALLY garbage. It's the Disney kind of
garbage. It was garbage back then, and it's garbage now.

It is also garbage for a simple technical reason: secure boot can be
hard to turn off. Sometimes "turn off" means "you just have to add
your own keys".

Yes, on x86 hardware at least at some point MS actually had the rule
that it has to be something you can turn off. That rule is apparently
not true on ARM, though.

Seriously. You sound like you're parroting some party line, not like
you are answering the actual question.

So again: why do you conflate the two issues?

If you want lockdown, fine, enable it. But what the F*CK does that
have to do with whether you had secure boot or not?

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


Re: [GIT PULL] Kernel lockdown for secure boot

2018-04-03 Thread Al Viro
On Tue, Apr 03, 2018 at 09:08:54PM +, Matthew Garrett wrote:

> > The fact is, some hardware pushes secure boot pretty hard. That has
> > *nothing* to do with some "lockdown" mode.
> 
> Secure Boot ensures that the firmware will only load signed bootloaders. If
> a signed bootloader loads a kernel that's effectively an unsigned
> bootloader, there's no point in using Secure Boot - you should just turn it
> off instead, because it's not giving you any meaningful security. Andy's
> example gives a scenario where by constraining your *userland* sufficiently
> you can get close to having the same guarantees, but that involves you
> having a read-only filesystem and takes you even further away from having a
> general purpose computer.
> 
> If you don't want Secure Boot, turn it off. If you want Secure Boot, use a
> kernel that behaves in a way that actually increases your security.

That assumes you *can* turn that shit off.  On the hardware where manufacturer
has installed firmware that doesn't allow that SB is a misfeature that has
to be worked around.  Making that harder might improve the value of SB to
said manufacturers, but what's the benefit for everybody else?
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [GIT PULL] Kernel lockdown for secure boot

2018-04-03 Thread Matthew Garrett
On Tue, Apr 3, 2018 at 2:01 PM Linus Torvalds

wrote:

> On Tue, Apr 3, 2018 at 1:54 PM, Matthew Garrett  wrote:
> >
> >> .. maybe you don't *want* secure boot, but it's been pushed in your
> >> face by people with an agenda?
> >
> > Then turn it off, or build a self-signed kernel that doesn't do this?

> Umm. So you asked a question, and then when you got an answer you said
> "don't do that then".

> The fact is, some hardware pushes secure boot pretty hard. That has
> *nothing* to do with some "lockdown" mode.

Secure Boot ensures that the firmware will only load signed bootloaders. If
a signed bootloader loads a kernel that's effectively an unsigned
bootloader, there's no point in using Secure Boot - you should just turn it
off instead, because it's not giving you any meaningful security. Andy's
example gives a scenario where by constraining your *userland* sufficiently
you can get close to having the same guarantees, but that involves you
having a read-only filesystem and takes you even further away from having a
general purpose computer.

If you don't want Secure Boot, turn it off. If you want Secure Boot, use a
kernel that behaves in a way that actually increases your security.
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [GIT PULL] Kernel lockdown for secure boot

2018-04-03 Thread Linus Torvalds
On Tue, Apr 3, 2018 at 1:54 PM, Matthew Garrett  wrote:
>
>> .. maybe you don't *want* secure boot, but it's been pushed in your
>> face by people with an agenda?
>
> Then turn it off, or build a self-signed kernel that doesn't do this?

Umm. So you asked a question, and then when you got an answer you said
"don't do that then".

The fact is, some hardware pushes secure boot pretty hard. That has
*nothing* to do with some "lockdown" mode.

Why do you conflate the two? That was the original question. You
replied with another question. People answered yours.

NOW ANSWER THE ORIGINAL QUESTION, DAMMIT.

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


Re: [GIT PULL] Kernel lockdown for secure boot

2018-04-03 Thread Matthew Garrett
On Tue, Apr 3, 2018 at 1:53 PM Linus Torvalds

wrote:

> On Tue, Apr 3, 2018 at 9:29 AM, Matthew Garrett  wrote:
> > On Tue, Apr 3, 2018 at 8:11 AM Andy Lutomirski  wrote:
> >> Can you explain that much more clearly?  I'm asking why booting via
> >> UEFI Secure Boot should enable lockdown, and I don't see what this has
> >> to do with kexec.  And "someone blacklist[ing] your key in the
> >> bootloader" sounds like a political issue, not a technical issue.
> >
> > A kernel that allows users arbitrary access to ring 0 is just an
> > overfeatured bootloader. Why would you want secure boot in that case?

> .. maybe you don't *want* secure boot, but it's been pushed in your
> face by people with an agenda?

Then turn it off, or build a self-signed kernel that doesn't do this?
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [GIT PULL] Kernel lockdown for secure boot

2018-04-03 Thread Linus Torvalds
On Tue, Apr 3, 2018 at 9:29 AM, Matthew Garrett  wrote:
> On Tue, Apr 3, 2018 at 8:11 AM Andy Lutomirski  wrote:
>> Can you explain that much more clearly?  I'm asking why booting via
>> UEFI Secure Boot should enable lockdown, and I don't see what this has
>> to do with kexec.  And "someone blacklist[ing] your key in the
>> bootloader" sounds like a political issue, not a technical issue.
>
> A kernel that allows users arbitrary access to ring 0 is just an
> overfeatured bootloader. Why would you want secure boot in that case?

.. maybe you don't *want* secure boot, but it's been pushed in your
face by people with an agenda?

Seriously.

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


Re: [PATCH 2/2] efi: Add embedded peripheral firmware support

2018-04-03 Thread Peter Jones
On Sat, Mar 31, 2018 at 02:19:44PM +0200, Hans de Goede wrote:
> diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
> index fddc5f706fd2..1a5ea950f58f 100644
> --- a/drivers/firmware/efi/efi.c
> +++ b/drivers/firmware/efi/efi.c
> @@ -455,6 +455,7 @@ int __init efi_mem_desc_lookup(u64 phys_addr, 
> efi_memory_desc_t *out_md)
>   u64 end;
>  
>   if (!(md->attribute & EFI_MEMORY_RUNTIME) &&
> + md->type != EFI_BOOT_SERVICES_CODE &&
>   md->type != EFI_BOOT_SERVICES_DATA &&
>   md->type != EFI_RUNTIME_SERVICES_DATA) {
>   continue;

Might be worth adding a comment here to ensure nobody comes along later
and adds something like EFI_BOOT_LOADER_DATA or other stuff that's
allocated later here.  I don't want to accidentally patch our way into
having the ability to stumble across a firmware blob somebody dumped
into the middle of a grub config file, especially since you only need to
collide crc32 (within the same length) to pre-alias a match.

...
> +static int __init efi_check_md_for_embedded_firmware(
> + efi_memory_desc_t *md, const struct embedded_fw_desc *desc)
> +{
...
> + if (found_fw_count >= MAX_EMBEDDED_FIRMWARES) {
> + pr_err("Error already have %d embedded firmwares\n",
> +MAX_EMBEDDED_FIRMWARES);
> + return -ENOSPC;
> + }

Doesn't seem like this needs to be pr_err(); after all we have already
found a valid match, so the firmware vendor has done something
moderately stupid, but we have a firmware that will probably work.  Of
course it still needs to return != 0, but pr_warn() or even pr_info()
seems more reasonable.

Aside from those nits, looks good to me.

Reviewed-by: Peter Jones 

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


Re: [GIT PULL] Kernel lockdown for secure boot

2018-04-03 Thread David Howells
Andy Lutomirski  wrote:

> >>> A kernel that allows users arbitrary access to ring 0 is just an
> >>> overfeatured bootloader. Why would you want secure boot in that case?
> >>
> >> To get a chain of trust.
> >
> > You don't have a chain of trust that you can trust in that case.
> >
> Please elaborate on why I can’t trust it.

If the user can arbitrarily modify the running kernel image, you cannot trust
anything.  You cannot determine the trustworthiness of something because your
basis for determining that trust can be compromised.

> Please also elaborate on how lockdown helps at all.

Stopping the kernel from being arbitrarily modified allows you to preserve
your trust.

Stopping the kernel from being arbitrarily read stops any encryption keys it
may be using from being retrieved.

And, if you can't guarantee the trustworthiness of your own image, you can't
pass the trust onto the next image that you kexec.

Now, I can't guarantee that my patches close every hole, they just close all
the holes I know about - including some obscure ones like using DMA-capable
ISA devices to hack/access the kernel image.

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


Re: [GIT PULL] Kernel lockdown for secure boot

2018-04-03 Thread Kees Cook
On Tue, Apr 3, 2018 at 12:01 PM, Andy Lutomirski  wrote:
> On Tue, Apr 3, 2018 at 11:45 AM, Kees Cook  wrote:
>> On Tue, Apr 3, 2018 at 9:45 AM, Andy Lutomirski  wrote:
>>> On Tue, Apr 3, 2018 at 9:29 AM, Matthew Garrett  wrote:
 On Tue, Apr 3, 2018 at 8:11 AM Andy Lutomirski  wrote:
> Can you explain that much more clearly?  I'm asking why booting via
> UEFI Secure Boot should enable lockdown, and I don't see what this has
> to do with kexec.  And "someone blacklist[ing] your key in the
> bootloader" sounds like a political issue, not a technical issue.

 A kernel that allows users arbitrary access to ring 0 is just an
 overfeatured bootloader. Why would you want secure boot in that case?
>>>
>>> To get a chain of trust.  I can provision a system with some public
>>> keys, stored in UEFI authenticated variables, such that the system
>>> will only boot a signed image.  That signed image, can, in turn, load
>>> a signed (or hashed or otherwise verfified) kernel and a verified
>>> initramfs.  The initramfs can run a full system from a verified (using
>>> dm-verity or similar) filesystem, for example.  Now it's very hard to
>>> persistently attack this system.  Chromium OS does something very much
>>> like this, except that it doesn't use UEFI as far as I know.  So does
>>> iOS, and so do some Android versions.
>>
>> Correct, Chrome OS does not use UEFI, and we still want this patch
>> series, as it plugs all the known "intentional" escalation paths from
>> uid-0 to ring-0. Happily, that means all the politics around the UEFI
>> and Secure Boot case can be ignored, because those issues are specific
>> to Secure Boot, not the lockdown series. (They are _related_, sure,
>> but lockdown isn't only about Secure Boot -- it's just that SB is one
>> of the widely deployed implementations of this kind of
>> trust-chain-booting-thing. Chrome OS and Android's Verified Boot do
>> similar things and have the same expectations about the uid-0/ring-0
>> separation.)
>>
>> The goal for that bright line on Chrome OS and Android is to stop
>> attack persistence. We want to know that a reboot onto a new kernel
>> and OS image will actually result in getting the desired system state,
>> and that any attack on persistent system data (even for things running
>> with full root privileges) can't result in using kernel interfaces to
>> gain kernel control. This isn't expected to be _perfect_, since
>> nothing is. But it creates a place to work from. The idea that uid-0
>> is NOT ring-0 is still relatively new, so the existing designs in the
>> kernel aren't well suited to building that distinction. I view this
>> series as a solid first step to getting there, though.
>
> But wouldn't Chrome OS possibly want to lock down kernel memory write
> vectors but not read vectors?  After all, debugging is useful even on
> Chrome OS.

Chrome OS absolutely wants to block writing. We also want to block
reading as much as we possibly can, though yes we bump up against
debugging in that quest. But those cases are manageable and specific,
IMO.

-Kees

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


Re: [GIT PULL] Kernel lockdown for secure boot

2018-04-03 Thread Andy Lutomirski
On Tue, Apr 3, 2018 at 11:45 AM, Kees Cook  wrote:
> On Tue, Apr 3, 2018 at 9:45 AM, Andy Lutomirski  wrote:
>> On Tue, Apr 3, 2018 at 9:29 AM, Matthew Garrett  wrote:
>>> On Tue, Apr 3, 2018 at 8:11 AM Andy Lutomirski  wrote:
 Can you explain that much more clearly?  I'm asking why booting via
 UEFI Secure Boot should enable lockdown, and I don't see what this has
 to do with kexec.  And "someone blacklist[ing] your key in the
 bootloader" sounds like a political issue, not a technical issue.
>>>
>>> A kernel that allows users arbitrary access to ring 0 is just an
>>> overfeatured bootloader. Why would you want secure boot in that case?
>>
>> To get a chain of trust.  I can provision a system with some public
>> keys, stored in UEFI authenticated variables, such that the system
>> will only boot a signed image.  That signed image, can, in turn, load
>> a signed (or hashed or otherwise verfified) kernel and a verified
>> initramfs.  The initramfs can run a full system from a verified (using
>> dm-verity or similar) filesystem, for example.  Now it's very hard to
>> persistently attack this system.  Chromium OS does something very much
>> like this, except that it doesn't use UEFI as far as I know.  So does
>> iOS, and so do some Android versions.
>
> Correct, Chrome OS does not use UEFI, and we still want this patch
> series, as it plugs all the known "intentional" escalation paths from
> uid-0 to ring-0. Happily, that means all the politics around the UEFI
> and Secure Boot case can be ignored, because those issues are specific
> to Secure Boot, not the lockdown series. (They are _related_, sure,
> but lockdown isn't only about Secure Boot -- it's just that SB is one
> of the widely deployed implementations of this kind of
> trust-chain-booting-thing. Chrome OS and Android's Verified Boot do
> similar things and have the same expectations about the uid-0/ring-0
> separation.)
>
> The goal for that bright line on Chrome OS and Android is to stop
> attack persistence. We want to know that a reboot onto a new kernel
> and OS image will actually result in getting the desired system state,
> and that any attack on persistent system data (even for things running
> with full root privileges) can't result in using kernel interfaces to
> gain kernel control. This isn't expected to be _perfect_, since
> nothing is. But it creates a place to work from. The idea that uid-0
> is NOT ring-0 is still relatively new, so the existing designs in the
> kernel aren't well suited to building that distinction. I view this
> series as a solid first step to getting there, though.
>

But wouldn't Chrome OS possibly want to lock down kernel memory write
vectors but not read vectors?  After all, debugging is useful even on
Chrome OS.

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


Re: [GIT PULL] Kernel lockdown for secure boot

2018-04-03 Thread Andy Lutomirski
> On Apr 3, 2018, at 10:16 AM, David Howells  wrote:
>
> Andy Lutomirski  wrote:
>
>>> A kernel that allows users arbitrary access to ring 0 is just an
>>> overfeatured bootloader. Why would you want secure boot in that case?
>>
>> To get a chain of trust.
>
> You don't have a chain of trust that you can trust in that case.
>

Please elaborate on why I can’t trust it. Please also elaborate on how
lockdown helps at all.
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/2] efi: Add embedded peripheral firmware support

2018-04-03 Thread Luis R. Rodriguez
On Tue, Apr 03, 2018 at 08:07:11PM +0200, Lukas Wunner wrote:
> On Tue, Apr 03, 2018 at 10:33:25AM +0200, Hans de Goede wrote:
> > I asked Peter Jones for suggestions how to extract this during boot and
> > he suggested seeing if there was a copy of the firmware in the
> > EFI_BOOT_SERVICES_CODE memory segment, which it turns out there is.
> > 
> > My patch to add support for this contains a table of device-model (dmi
> > strings), firmware header (first 64 bits), length and crc32 and then if
> > we boot on a device-model which is in the table the code scans the
> > EFI_BOOT_SERVICES_CODE for the prefix, if found checks the crc and
> > caches the firmware for later use by request-firmware.
> > 
> > So I just do a brute-force search for the firmware, this really is hack,
> > nothing standard about it I'm afraid. But it works on 4 different x86
> > tablets I have and makes the touchscreen work OOTB on them, so I believe
> > it is a worthwhile hack to have.
> 
> The EFI Firmware Volume contains a kind of filesystem with files
> identified by GUIDs.  Those files include EFI drivers, ACPI tables,
> DMI data and so on.  It is actually quite common for vendors to
> also include device firmware on the Firmware Volume.  Apple is doing
> this to ship firmware updates e.g. for the GMUX controller found on
> dual GPU MacBook Pros.  If they want to update the controller's
> firmware, they include it in a BIOS update, and an EFI driver checks
> on boot if the firmware update for the controller is necessary and
> if so, flashes it.
> 
> The firmware files you're looking for are almost certainly included
> on the Firmware Volume as individual files.

What Hans implemented seems to have been for a specific x86 hack, best if we
confirm if indeed they are present on the Firmware Volume.

> Rather than scraping
> the EFI memory for firmware, I think it would be cleaner and more
> elegant if you just retrieve the files you're interested in from
> the Firmware Volume.
> 
> We're doing something similar with Apple EFI properties, see
> 58c5475aba67 and c9cc3aaa0281.
> 
> Basically what you need to do to implement this approach is:
> 
> * Determine the GUIDs used by vendors for the files you're interested
>   in.  Either dump the Firmware Volume or take an EFI update as
>   shipped by the vendor, then feed it to UEFIExtract:
>   https://github.com/LongSoft/UEFITool
>   
> * Add the EFI Firmware Volume Protocol to include/linux/efi.h:
>   
> https://www.intel.com/content/dam/doc/reference-guide/efi-firmware-file-volume-specification.pdf
> 
> * Amend arch/x86/boot/compressed/eboot.c to read the files with the
>   GUIDs you're interested in into memory and pass the files to the
>   kernel as setup_data payloads.
> 
> * Once the kernel has booted, make the files you've retrieved
>   available to device drivers as firmware blobs.

Happen to know if devices using Firmware Volumes also sign their firmware
and if hw checks the firmware at load time?

  Luis

> The end result is mostly the same as what you're doing here,
> and you'll also have a similar array of structs, but instead
> of hardcoding 8-byte signatures of firmware files, you'll be
> using GUIDs.  I envision lots of interesting use cases for
> a generic Firmware Volume file retrieval mechanism.  What you
> need to keep in mind though is that this approach only works
> if the kernel is booted via the EFI stub.
> 
> Thanks,
> 
> Lukas
> 

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


Re: [PATCH 2/2] efi: Add embedded peripheral firmware support

2018-04-03 Thread Luis R. Rodriguez
In your next patches please Cc the folks I added for future review as well.
We don't have a mailing list for the firmware API so I just tend to Cc
who I think should help review when needed.

On Tue, Apr 03, 2018 at 10:33:25AM +0200, Hans de Goede wrote:
> Hi Luis,
> 
> Thank you for the review.
> 
> On 03-04-18 01:23, Luis R. Rodriguez wrote:
> > On Sat, Mar 31, 2018 at 02:19:44PM +0200, Hans de Goede wrote:
> > > Just like with PCI options ROMs, which we save in the setup_efi_pci*
> > > functions from arch/x86/boot/compressed/eboot.c, the EFI code / ROM itself
> > > sometimes may contain data which is useful/necessary for peripheral 
> > > drivers
> > > to have access to.
> > > 
> > > Specifically the EFI code may contain an embedded copy of firmware which
> > > needs to be (re)loaded into the peripheral. Normally such firmware would 
> > > be
> > > part of linux-firmware, but in some cases this is not feasible, for 2
> > > reasons:
> > > 
> > > 1) The firmware is customized for a specific use-case of the chipset / use
> > > with a specific hardware model, so we cannot have a single firmware file
> > > for the chipset. E.g. touchscreen controller firmwares are compiled
> > > specifically for the hardware model they are used with, as they are
> > > calibrated for a specific model digitizer.
> > 
> > Some devices have OTP and use this sort of calibration data,
> 
> Right, I'm not sure it really is OTP and not flash, but many touchscreen
> controllers do come with their firmware embedded into the controller, 

It varies, sometimes firmware has default fluff calibration data as well which 
can be used if the device does not have specific calibration data.

> but not all unfortunately.

Indeed.

> > I was unaware of
> > the use of EFI to stash firmware. Good to know, but can you also provide
> > references to what part of what standard should be followed for it in
> > documentation?
> 
> This is not part of the standard. There has been a long(ish) standing issue
> with us not being able to get re-distribute permission for the firmware for
> some touchscreen controllers found on cheap x86 devices. Which means that
> we cannot put it in Linux firmware.

BTW do these cheap x86 devices have hw signing support? Just curious thinking
long term here. Because of it is not-standard then perhaps wen can partner
later up with a vendor to this properly and actually support hw firmware
singing.

> Dave Olsthoorn (in the Cc) noticed that the touchscreen did work in the
> refind bootload UI, so the EFI code must have a copy of the firmware.

:)

> I asked Peter Jones for suggestions how to extract this during boot and
> he suggested seeing if there was a copy of the firmware in the
> EFI_BOOT_SERVICES_CODE memory segment, which it turns out there is.

Sneaky Pete, nice. So essentially we're reverse engineering support for this.

Anyway please mention that this is not part of standard in the documentation,
and we've just found out in practice some vendors are doing this. That would
avoid having people ask later.

> My patch to add support for this contains a table of device-model (dmi
> strings), firmware header (first 64 bits), length and crc32 and then if
> we boot on a device-model which is in the table the code scans the
> EFI_BOOT_SERVICES_CODE for the prefix, if found checks the crc and
> caches the firmware for later use by request-firmware.

Neat, best to add proper docs for this.

> So I just do a brute-force search for the firmware, this really is hack,
> nothing standard about it I'm afraid. But it works on 4 different x86
> tablets I have and makes the touchscreen work OOTB on them, so I believe
> it is a worthwhile hack to have.

Absolutely, just not to shove an entire fallback firmware path to all users.

> > > 2) Despite repeated attempts we have failed to get permission to
> > > redistribute the firmware. This is especially a problem with customized
> > > firmwares, these get created by the chip vendor for a specific ODM and the
> > > copyright may partially belong with the ODM, so the chip vendor cannot
> > > give a blanket permission to distribute these.
> > > 
> > > This commit adds support for finding peripheral firmware embedded in the
> > > EFI code and making this available to peripheral drivers through the
> > > standard firmware loading mechanism.
> > 
> > Neat.
> > 
> > > Note we check the EFI_BOOT_SERVICES_CODE for embedded firmware pretty
> > > late in the init sequence,
> > 
> > This also creates a technical limitation on use for the API that users
> > should be aware of. Its important to document such limitation.
> 
> I don't think this is a problem for any normal drivers, when I say pretty
> late I mean late in init/main.c: start_kernel(), so still before any normal
> drivers load.
> 
> The first idea was to scan for the firmware at the same time we check for
> things as the ACPI BGRT logo stuff, but as mentioned that requires using
> early_mmap() which does not work for the amount of memory 

Re: [GIT PULL] Kernel lockdown for secure boot

2018-04-03 Thread Kees Cook
On Tue, Apr 3, 2018 at 9:45 AM, Andy Lutomirski  wrote:
> On Tue, Apr 3, 2018 at 9:29 AM, Matthew Garrett  wrote:
>> On Tue, Apr 3, 2018 at 8:11 AM Andy Lutomirski  wrote:
>>> Can you explain that much more clearly?  I'm asking why booting via
>>> UEFI Secure Boot should enable lockdown, and I don't see what this has
>>> to do with kexec.  And "someone blacklist[ing] your key in the
>>> bootloader" sounds like a political issue, not a technical issue.
>>
>> A kernel that allows users arbitrary access to ring 0 is just an
>> overfeatured bootloader. Why would you want secure boot in that case?
>
> To get a chain of trust.  I can provision a system with some public
> keys, stored in UEFI authenticated variables, such that the system
> will only boot a signed image.  That signed image, can, in turn, load
> a signed (or hashed or otherwise verfified) kernel and a verified
> initramfs.  The initramfs can run a full system from a verified (using
> dm-verity or similar) filesystem, for example.  Now it's very hard to
> persistently attack this system.  Chromium OS does something very much
> like this, except that it doesn't use UEFI as far as I know.  So does
> iOS, and so do some Android versions.

Correct, Chrome OS does not use UEFI, and we still want this patch
series, as it plugs all the known "intentional" escalation paths from
uid-0 to ring-0. Happily, that means all the politics around the UEFI
and Secure Boot case can be ignored, because those issues are specific
to Secure Boot, not the lockdown series. (They are _related_, sure,
but lockdown isn't only about Secure Boot -- it's just that SB is one
of the widely deployed implementations of this kind of
trust-chain-booting-thing. Chrome OS and Android's Verified Boot do
similar things and have the same expectations about the uid-0/ring-0
separation.)

The goal for that bright line on Chrome OS and Android is to stop
attack persistence. We want to know that a reboot onto a new kernel
and OS image will actually result in getting the desired system state,
and that any attack on persistent system data (even for things running
with full root privileges) can't result in using kernel interfaces to
gain kernel control. This isn't expected to be _perfect_, since
nothing is. But it creates a place to work from. The idea that uid-0
is NOT ring-0 is still relatively new, so the existing designs in the
kernel aren't well suited to building that distinction. I view this
series as a solid first step to getting there, though.

-Kees

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


Re: [PATCH 2/2] efi: Add embedded peripheral firmware support

2018-04-03 Thread Lukas Wunner
On Tue, Apr 03, 2018 at 10:33:25AM +0200, Hans de Goede wrote:
> I asked Peter Jones for suggestions how to extract this during boot and
> he suggested seeing if there was a copy of the firmware in the
> EFI_BOOT_SERVICES_CODE memory segment, which it turns out there is.
> 
> My patch to add support for this contains a table of device-model (dmi
> strings), firmware header (first 64 bits), length and crc32 and then if
> we boot on a device-model which is in the table the code scans the
> EFI_BOOT_SERVICES_CODE for the prefix, if found checks the crc and
> caches the firmware for later use by request-firmware.
> 
> So I just do a brute-force search for the firmware, this really is hack,
> nothing standard about it I'm afraid. But it works on 4 different x86
> tablets I have and makes the touchscreen work OOTB on them, so I believe
> it is a worthwhile hack to have.

The EFI Firmware Volume contains a kind of filesystem with files
identified by GUIDs.  Those files include EFI drivers, ACPI tables,
DMI data and so on.  It is actually quite common for vendors to
also include device firmware on the Firmware Volume.  Apple is doing
this to ship firmware updates e.g. for the GMUX controller found on
dual GPU MacBook Pros.  If they want to update the controller's
firmware, they include it in a BIOS update, and an EFI driver checks
on boot if the firmware update for the controller is necessary and
if so, flashes it.

The firmware files you're looking for are almost certainly included
on the Firmware Volume as individual files.  Rather than scraping
the EFI memory for firmware, I think it would be cleaner and more
elegant if you just retrieve the files you're interested in from
the Firmware Volume.

We're doing something similar with Apple EFI properties, see
58c5475aba67 and c9cc3aaa0281.

Basically what you need to do to implement this approach is:

* Determine the GUIDs used by vendors for the files you're interested
  in.  Either dump the Firmware Volume or take an EFI update as
  shipped by the vendor, then feed it to UEFIExtract:
  https://github.com/LongSoft/UEFITool
  
* Add the EFI Firmware Volume Protocol to include/linux/efi.h:
  
https://www.intel.com/content/dam/doc/reference-guide/efi-firmware-file-volume-specification.pdf

* Amend arch/x86/boot/compressed/eboot.c to read the files with the
  GUIDs you're interested in into memory and pass the files to the
  kernel as setup_data payloads.

* Once the kernel has booted, make the files you've retrieved
  available to device drivers as firmware blobs.

The end result is mostly the same as what you're doing here,
and you'll also have a similar array of structs, but instead
of hardcoding 8-byte signatures of firmware files, you'll be
using GUIDs.  I envision lots of interesting use cases for
a generic Firmware Volume file retrieval mechanism.  What you
need to keep in mind though is that this approach only works
if the kernel is booted via the EFI stub.

Thanks,

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


Re: [GIT PULL] Kernel lockdown for secure boot

2018-04-03 Thread David Howells
Andy Lutomirski  wrote:

> > A kernel that allows users arbitrary access to ring 0 is just an
> > overfeatured bootloader. Why would you want secure boot in that case?
> 
> To get a chain of trust.

You don't have a chain of trust that you can trust in that case.

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


Re: [PATCH v2] x86/xen/efi: Initialize UEFI secure boot state during dom0 boot

2018-04-03 Thread James Bottomley
On Tue, 2018-04-03 at 18:07 +0200, Daniel Kiper wrote:
> On Tue, Apr 03, 2018 at 08:44:41AM -0700, James Bottomley wrote:
> > 
> > On Tue, 2018-04-03 at 16:39 +0200, Daniel Kiper wrote:
> > > 
> > > Initialize UEFI secure boot state during dom0 boot. Otherwise the
> > > kernel may not even know that it runs on secure boot enabled
> > > platform.
> > > 
> > > Signed-off-by: Daniel Kiper 
> > > ---
> > >  arch/x86/xen/efi.c|   57
> > > +
> > >  drivers/firmware/efi/libstub/secureboot.c |3 ++
> > >  2 files changed, 60 insertions(+)
> > > 
> > > diff --git a/arch/x86/xen/efi.c b/arch/x86/xen/efi.c
> > > index a18703b..1804b27 100644
> > > --- a/arch/x86/xen/efi.c
> > > +++ b/arch/x86/xen/efi.c
> > > @@ -115,6 +115,61 @@ static efi_system_table_t __init
> > > *xen_efi_probe(void)
> > >   return _systab_xen;
> > >  }
> > >  
> > > +/*
> > > + * Determine whether we're in secure boot mode.
> > > + *
> > > + * Please keep the logic in sync with
> > > + *
> > > drivers/firmware/efi/libstub/secureboot.c:efi_get_secureboot().
> > > + */
> > > +static enum efi_secureboot_mode xen_efi_get_secureboot(void)
> > > +{
> > > + static efi_guid_t efi_variable_guid =
> > > EFI_GLOBAL_VARIABLE_GUID;
> > > + static efi_guid_t shim_guid = EFI_SHIM_LOCK_GUID;
> > > + efi_status_t status;
> > > + u8 moksbstate, secboot, setupmode;
> > > + unsigned long size;
> > > +
> > > + size = sizeof(secboot);
> > > + status = efi.get_variable(L"SecureBoot",
> > > _variable_guid,
> > > +   NULL, , );
> > > +
> > > + if (status == EFI_NOT_FOUND)
> > > + return efi_secureboot_mode_disabled;
> > > +
> > > + if (status != EFI_SUCCESS)
> > > + goto out_efi_err;
> > > +
> > > + size = sizeof(setupmode);
> > > + status = efi.get_variable(L"SetupMode",
> > > _variable_guid,
> > > +   NULL, , );
> > > +
> > > + if (status != EFI_SUCCESS)
> > > + goto out_efi_err;
> > > +
> > > + if (secboot == 0 || setupmode == 1)
> > > + return efi_secureboot_mode_disabled;
> > > +
> > > + /* See if a user has put the shim into insecure mode. */
> > > + size = sizeof(moksbstate);
> > > + status = efi.get_variable(L"MokSBStateRT", _guid,
> > > +   NULL, , );
> > > +
> > > + /* If it fails, we don't care why. Default to secure. */
> > > + if (status != EFI_SUCCESS)
> > > + goto secure_boot_enabled;
> > > +
> > > + if (moksbstate == 1)
> > > + return efi_secureboot_mode_disabled;
> > > +
> > > + secure_boot_enabled:
> > > + pr_info("UEFI Secure Boot is enabled.\n");
> > > + return efi_secureboot_mode_enabled;
> > > +
> > > + out_efi_err:
> > > + pr_err("Could not determine UEFI Secure Boot
> > > status.\n");
> > > + return efi_secureboot_mode_unknown;
> > > +}
> > > +
> > 
> > This looks like a bad idea: you're duplicating the secure boot
> > check in
> > 
> > drivers/firmware/efi/libstub/secureboot.c
> > 
> > Which is an implementation of policy.  If we have to have policy in
> > the kernel, it should really only be in one place to prevent drift;
> > why can't you simply use the libstub efi_get_secureboot() so we're
> > not duplicating the implementation of policy?
> 
> Well, here is the first version of this patch:
> https://lkml.org/lkml/2018/1/9/496 Ard did not like it. I was not
> happy too. In general both approaches are not perfect. More you can
> find in the discussion around this patchset. If you have better idea
> how to do that I am happy to implement it.

One way might be simply to have the pre exit-boot-services code lay
down a variable containing the state which you pick up, rather than you
calling efi code separately and trying to use the insecure RT
variables.  That way there's a uniform view of the internal kernel
secure boot state that everyone can use.

James

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


Re: [GIT PULL] Kernel lockdown for secure boot

2018-04-03 Thread Andy Lutomirski
On Tue, Apr 3, 2018 at 9:29 AM, Matthew Garrett  wrote:
> On Tue, Apr 3, 2018 at 8:11 AM Andy Lutomirski  wrote:
>> Can you explain that much more clearly?  I'm asking why booting via
>> UEFI Secure Boot should enable lockdown, and I don't see what this has
>> to do with kexec.  And "someone blacklist[ing] your key in the
>> bootloader" sounds like a political issue, not a technical issue.
>
> A kernel that allows users arbitrary access to ring 0 is just an
> overfeatured bootloader. Why would you want secure boot in that case?

To get a chain of trust.  I can provision a system with some public
keys, stored in UEFI authenticated variables, such that the system
will only boot a signed image.  That signed image, can, in turn, load
a signed (or hashed or otherwise verfified) kernel and a verified
initramfs.  The initramfs can run a full system from a verified (using
dm-verity or similar) filesystem, for example.  Now it's very hard to
persistently attack this system.  Chromium OS does something very much
like this, except that it doesn't use UEFI as far as I know.  So does
iOS, and so do some Android versions.  None of this requires lockdown,
or even a separation between usermode and kernelmode, to work
correctly.  One could even do this on an MMU-less system if one really
cared to.  More usefully, someone probably has done this using a
unikernel.

If I had to guess at a motivation that makes this patchset work, it
would be that there is an uneasy truce between Microsoft and the
various vendors of signed Linux bootloaders.  That truce could
conceivably require that the signed bootloaders not knowingly ship a
system that allows a non-physically-present user to chainload Windows.
If so, the patchset should say that loud and clear in its description
and the parts that block bpf should go away.
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [GIT PULL] Kernel lockdown for secure boot

2018-04-03 Thread Matthew Garrett
On Tue, Apr 3, 2018 at 8:11 AM Andy Lutomirski  wrote:
> Can you explain that much more clearly?  I'm asking why booting via
> UEFI Secure Boot should enable lockdown, and I don't see what this has
> to do with kexec.  And "someone blacklist[ing] your key in the
> bootloader" sounds like a political issue, not a technical issue.

A kernel that allows users arbitrary access to ring 0 is just an
overfeatured bootloader. Why would you want secure boot in that case?
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [GIT PULL] Kernel lockdown for secure boot

2018-04-03 Thread Andy Lutomirski
On Tue, Apr 3, 2018 at 8:41 AM, Alexei Starovoitov
 wrote:
> On Tue, Apr 03, 2018 at 08:11:07AM -0700, Andy Lutomirski wrote:
>> >
>> >> "bpf: Restrict kernel image access functions when the kernel is locked 
>> >> down":
>> >> This patch just sucks in general.
>> >
>> > Yes - but that's what Alexei Starovoitov specified.  bpf kind of sucks 
>> > since
>> > it gives you unrestricted access to the kernel.
>>
>> bpf, in certain contexts, gives you unrestricted access to *reading*
>> kernel memory.  bpf should, under no circumstances, let you write to
>> the kernel unless you're using fault injection or similar.
>>
>> I'm surprised that Alexei acked this patch.  If something like XDP or
>> bpfilter starts becoming widely used, this patch will require a lot of
>> reworking to avoid breaking standard distros.
>
> my understanding was that this lockdown set attemps to disallow _reads_
> of kernel memory from anything, so first version of patch was adding
> run-time checks for bpf_probe_read() which is no-go
> and without this helper the bpf for tracing is losing a lot of its power,
> so the easiest is to disable it all.

Fair enough.

> I think lockdown suppose to disable xdp, bpfilter, nflog, raw sockets + pcap 
> too
> otherwise even cap_net_admin can see traffic coming into host.
> Similarly kprobe, perf_event, ftrace should be off as well?
>

I'm reasonably sure that lockdown is not intended to be this far
reaching.  cap_net_admin can see traffic coming into the host, and I
don't think lockdown is intended to change that.

David, I think this is exactly why you need to define what "lockdown"
means.  As it stands, the best argument I've seen involves
"blacklisting", but that's a political thing and almost no one
involved has any ability to evaluate it.  Right now there's a series
of patches that check for "lockdown" and seem to disable things that
make someone uncomfortable.  That's not a good way to design a
security feature.
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2] x86/xen/efi: Initialize UEFI secure boot state during dom0 boot

2018-04-03 Thread Daniel Kiper
On Tue, Apr 03, 2018 at 08:44:41AM -0700, James Bottomley wrote:
> On Tue, 2018-04-03 at 16:39 +0200, Daniel Kiper wrote:
> > Initialize UEFI secure boot state during dom0 boot. Otherwise the
> > kernel
> > may not even know that it runs on secure boot enabled platform.
> >
> > Signed-off-by: Daniel Kiper 
> > ---
> >  arch/x86/xen/efi.c|   57
> > +
> >  drivers/firmware/efi/libstub/secureboot.c |3 ++
> >  2 files changed, 60 insertions(+)
> >
> > diff --git a/arch/x86/xen/efi.c b/arch/x86/xen/efi.c
> > index a18703b..1804b27 100644
> > --- a/arch/x86/xen/efi.c
> > +++ b/arch/x86/xen/efi.c
> > @@ -115,6 +115,61 @@ static efi_system_table_t __init
> > *xen_efi_probe(void)
> >     return _systab_xen;
> >  }
> >  
> > +/*
> > + * Determine whether we're in secure boot mode.
> > + *
> > + * Please keep the logic in sync with
> > + * drivers/firmware/efi/libstub/secureboot.c:efi_get_secureboot().
> > + */
> > +static enum efi_secureboot_mode xen_efi_get_secureboot(void)
> > +{
> > +   static efi_guid_t efi_variable_guid =
> > EFI_GLOBAL_VARIABLE_GUID;
> > +   static efi_guid_t shim_guid = EFI_SHIM_LOCK_GUID;
> > +   efi_status_t status;
> > +   u8 moksbstate, secboot, setupmode;
> > +   unsigned long size;
> > +
> > +   size = sizeof(secboot);
> > +   status = efi.get_variable(L"SecureBoot", _variable_guid,
> > +     NULL, , );
> > +
> > +   if (status == EFI_NOT_FOUND)
> > +   return efi_secureboot_mode_disabled;
> > +
> > +   if (status != EFI_SUCCESS)
> > +   goto out_efi_err;
> > +
> > +   size = sizeof(setupmode);
> > +   status = efi.get_variable(L"SetupMode", _variable_guid,
> > +     NULL, , );
> > +
> > +   if (status != EFI_SUCCESS)
> > +   goto out_efi_err;
> > +
> > +   if (secboot == 0 || setupmode == 1)
> > +   return efi_secureboot_mode_disabled;
> > +
> > +   /* See if a user has put the shim into insecure mode. */
> > +   size = sizeof(moksbstate);
> > +   status = efi.get_variable(L"MokSBStateRT", _guid,
> > +     NULL, , );
> > +
> > +   /* If it fails, we don't care why. Default to secure. */
> > +   if (status != EFI_SUCCESS)
> > +   goto secure_boot_enabled;
> > +
> > +   if (moksbstate == 1)
> > +   return efi_secureboot_mode_disabled;
> > +
> > + secure_boot_enabled:
> > +   pr_info("UEFI Secure Boot is enabled.\n");
> > +   return efi_secureboot_mode_enabled;
> > +
> > + out_efi_err:
> > +   pr_err("Could not determine UEFI Secure Boot status.\n");
> > +   return efi_secureboot_mode_unknown;
> > +}
> > +
>
> This looks like a bad idea: you're duplicating the secure boot check in
>
> drivers/firmware/efi/libstub/secureboot.c
>
> Which is an implementation of policy.  If we have to have policy in the
> kernel, it should really only be in one place to prevent drift; why
> can't you simply use the libstub efi_get_secureboot() so we're not
> duplicating the implementation of policy?

Well, here is the first version of this patch: 
https://lkml.org/lkml/2018/1/9/496
Ard did not like it. I was not happy too. In general both approaches are not 
perfect.
More you can find in the discussion around this patchset. If you have better 
idea
how to do that I am happy to implement it.

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


Re: [GIT PULL] Kernel lockdown for secure boot

2018-04-03 Thread Alexei Starovoitov
On Tue, Apr 03, 2018 at 08:11:07AM -0700, Andy Lutomirski wrote:
> >
> >> "bpf: Restrict kernel image access functions when the kernel is locked 
> >> down":
> >> This patch just sucks in general.
> >
> > Yes - but that's what Alexei Starovoitov specified.  bpf kind of sucks since
> > it gives you unrestricted access to the kernel.
> 
> bpf, in certain contexts, gives you unrestricted access to *reading*
> kernel memory.  bpf should, under no circumstances, let you write to
> the kernel unless you're using fault injection or similar.
> 
> I'm surprised that Alexei acked this patch.  If something like XDP or
> bpfilter starts becoming widely used, this patch will require a lot of
> reworking to avoid breaking standard distros.

my understanding was that this lockdown set attemps to disallow _reads_
of kernel memory from anything, so first version of patch was adding
run-time checks for bpf_probe_read() which is no-go
and without this helper the bpf for tracing is losing a lot of its power,
so the easiest is to disable it all.
I think lockdown suppose to disable xdp, bpfilter, nflog, raw sockets + pcap too
otherwise even cap_net_admin can see traffic coming into host.
Similarly kprobe, perf_event, ftrace should be off as well?

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


Re: [GIT PULL] Kernel lockdown for secure boot

2018-04-03 Thread Andy Lutomirski
[re-added cc's, I think.  Sorry, I think I failed to use the gmane
gateway correctly there.]

On Tue, Apr 3, 2018 at 12:06 AM, David Howells  wrote:
> Andy Lutomirski  wrote:
>
>> This is an attempt at a review.  I'm replying here because I can't find the
>> actual relevant patch emails.
>
> This was the latest post:
>
> https://lkml.org/lkml/2017/11/9/660
>
> and they were posted multiple times before that, plus distributions, such as
> Fedora, have been carrying them for a long while.
>
>> For the rest of this review, I'm going to pretend that you actually want two
>> features: "try-prevent-root-from-corrupting-the-kernel" and
>> "try-to-prevent-root-from-reading-kernel-memory".
>
> It theoretically boils down into those two, but the line is blurrier than you
> think.
>
> Further, some of the vectors that can be used to do one can potentially do the
> other also and it starts getting to be a lot of extra work to distinguish the
> two.
>
>> I do *not* see why the mere act of using Secure Boot should have this
>> effect.
>
> To be able to pass secure boot mode over kexec, you have to make sure that the
> kernel image doesn't get corrupted, lest someone blacklist your signing key in
> the bootloader.

Can you explain that much more clearly?  I'm asking why booting via
UEFI Secure Boot should enable lockdown, and I don't see what this has
to do with kexec.  And "someone blacklist[ing] your key in the
bootloader" sounds like a political issue, not a technical issue.

What is the actual purpose of these patches?

>
>> In particular, UEFI Secure Boot should *not* enable
>> "try-to-prevent-root-from-reading-kernel-memory", which means that, unless
>> you actually implement the split, you should drop a bunch of the patches.
>
> Yes it should.  If someone can read your kernel image, they can steal the
> crypto keys you use to encrypt your filesystem.

Can you please explain the actual attack that is avoided by doing this?

Suppose I'm a bad guy attacking someone's laptop.  If I just have
normal uid!=0 access, then these patches have no effect.  Instead,
we're talking about an attacker who is somehow able to become global
root and bypass all LSM restrictions but has not gained kernel code
execution.  It is indeed the case that your patches make it harder to
simply read the dm-crypt encryption key out of main memory.  But root
can attack the disk encryption in many other ways.  They can
persistently compromise the machine by adding services or user
accounts or intentionally misconfiguring something.  They can directly
read the entire contents of the disk.  They can modify the initrd so
that the next time the machine reboots and the user types the
password, the attacker gets the key (unless the TPM is involved, but
getting *that* right on a standard distro is difficult or impossible).

And I'm not even sure why an attacker who manages to become root wants
your disk encryption key.  That key is worth nothing unless the
attacker makes its attack persistent, but, if the attacker can install
a persistent user-level backdoor, then they can read the cleartext off
your disk just as easily as they can read the ciphertext.

>
>> "Restrict /dev/{mem,kmem,port} when the kernel is locked down": this should
>> probably split into one restriction for read and one for write.
>
> Not so for /dev/port.  Read & Write here are _not_ the same as Read & Write
> on, say, /dev/mem.  In fact, if /dev/mem gives you access to mmio ports, then
> the same applies there.  Btw, Fedora hasn't even provided /dev/kmem for a
> while.

Then split /dev/mem and turn off /dev/port for all locked-down modes.

>
>> "bpf: Restrict kernel image access functions when the kernel is locked down":
>> This patch just sucks in general.
>
> Yes - but that's what Alexei Starovoitov specified.  bpf kind of sucks since
> it gives you unrestricted access to the kernel.

bpf, in certain contexts, gives you unrestricted access to *reading*
kernel memory.  bpf should, under no circumstances, let you write to
the kernel unless you're using fault injection or similar.

I'm surprised that Alexei acked this patch.  If something like XDP or
bpfilter starts becoming widely used, this patch will require a lot of
reworking to avoid breaking standard distros.
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2] x86/xen/efi: Initialize UEFI secure boot state during dom0 boot

2018-04-03 Thread Daniel Kiper
Initialize UEFI secure boot state during dom0 boot. Otherwise the kernel
may not even know that it runs on secure boot enabled platform.

Signed-off-by: Daniel Kiper 
---
 arch/x86/xen/efi.c|   57 +
 drivers/firmware/efi/libstub/secureboot.c |3 ++
 2 files changed, 60 insertions(+)

diff --git a/arch/x86/xen/efi.c b/arch/x86/xen/efi.c
index a18703b..1804b27 100644
--- a/arch/x86/xen/efi.c
+++ b/arch/x86/xen/efi.c
@@ -115,6 +115,61 @@ static efi_system_table_t __init *xen_efi_probe(void)
return _systab_xen;
 }
 
+/*
+ * Determine whether we're in secure boot mode.
+ *
+ * Please keep the logic in sync with
+ * drivers/firmware/efi/libstub/secureboot.c:efi_get_secureboot().
+ */
+static enum efi_secureboot_mode xen_efi_get_secureboot(void)
+{
+   static efi_guid_t efi_variable_guid = EFI_GLOBAL_VARIABLE_GUID;
+   static efi_guid_t shim_guid = EFI_SHIM_LOCK_GUID;
+   efi_status_t status;
+   u8 moksbstate, secboot, setupmode;
+   unsigned long size;
+
+   size = sizeof(secboot);
+   status = efi.get_variable(L"SecureBoot", _variable_guid,
+ NULL, , );
+
+   if (status == EFI_NOT_FOUND)
+   return efi_secureboot_mode_disabled;
+
+   if (status != EFI_SUCCESS)
+   goto out_efi_err;
+
+   size = sizeof(setupmode);
+   status = efi.get_variable(L"SetupMode", _variable_guid,
+ NULL, , );
+
+   if (status != EFI_SUCCESS)
+   goto out_efi_err;
+
+   if (secboot == 0 || setupmode == 1)
+   return efi_secureboot_mode_disabled;
+
+   /* See if a user has put the shim into insecure mode. */
+   size = sizeof(moksbstate);
+   status = efi.get_variable(L"MokSBStateRT", _guid,
+ NULL, , );
+
+   /* If it fails, we don't care why. Default to secure. */
+   if (status != EFI_SUCCESS)
+   goto secure_boot_enabled;
+
+   if (moksbstate == 1)
+   return efi_secureboot_mode_disabled;
+
+ secure_boot_enabled:
+   pr_info("UEFI Secure Boot is enabled.\n");
+   return efi_secureboot_mode_enabled;
+
+ out_efi_err:
+   pr_err("Could not determine UEFI Secure Boot status.\n");
+   return efi_secureboot_mode_unknown;
+}
+
 void __init xen_efi_init(void)
 {
efi_system_table_t *efi_systab_xen;
@@ -129,6 +184,8 @@ void __init xen_efi_init(void)
boot_params.efi_info.efi_systab = (__u32)__pa(efi_systab_xen);
boot_params.efi_info.efi_systab_hi = (__u32)(__pa(efi_systab_xen) >> 
32);
 
+   boot_params.secure_boot = xen_efi_get_secureboot();
+
set_bit(EFI_BOOT, );
set_bit(EFI_PARAVIRT, );
set_bit(EFI_64BIT, );
diff --git a/drivers/firmware/efi/libstub/secureboot.c 
b/drivers/firmware/efi/libstub/secureboot.c
index 8f07eb4..72d9dfb 100644
--- a/drivers/firmware/efi/libstub/secureboot.c
+++ b/drivers/firmware/efi/libstub/secureboot.c
@@ -30,6 +30,9 @@
 
 /*
  * Determine whether we're in secure boot mode.
+ *
+ * Please keep the logic in sync with
+ * arch/x86/xen/efi.c:xen_efi_get_secureboot().
  */
 enum efi_secureboot_mode efi_get_secureboot(efi_system_table_t *sys_table_arg)
 {
-- 
1.7.10.4

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


Re: [PATCH 2/2] efi: Add embedded peripheral firmware support

2018-04-03 Thread Hans de Goede

Hi Luis,

Thank you for the review.

On 03-04-18 01:23, Luis R. Rodriguez wrote:

On Sat, Mar 31, 2018 at 02:19:44PM +0200, Hans de Goede wrote:

Just like with PCI options ROMs, which we save in the setup_efi_pci*
functions from arch/x86/boot/compressed/eboot.c, the EFI code / ROM itself
sometimes may contain data which is useful/necessary for peripheral drivers
to have access to.

Specifically the EFI code may contain an embedded copy of firmware which
needs to be (re)loaded into the peripheral. Normally such firmware would be
part of linux-firmware, but in some cases this is not feasible, for 2
reasons:

1) The firmware is customized for a specific use-case of the chipset / use
with a specific hardware model, so we cannot have a single firmware file
for the chipset. E.g. touchscreen controller firmwares are compiled
specifically for the hardware model they are used with, as they are
calibrated for a specific model digitizer.


Some devices have OTP and use this sort of calibration data,


Right, I'm not sure it really is OTP and not flash, but many touchscreen
controllers do come with their firmware embedded into the controller, but
not all unfortunately.


I was unaware of
the use of EFI to stash firmware. Good to know, but can you also provide
references to what part of what standard should be followed for it in
documentation?


This is not part of the standard. There has been a long(ish) standing issue
with us not being able to get re-distribute permission for the firmware for
some touchscreen controllers found on cheap x86 devices. Which means that
we cannot put it in Linux firmware.

Dave Olsthoorn (in the Cc) noticed that the touchscreen did work in the
refind bootload UI, so the EFI code must have a copy of the firmware.

I asked Peter Jones for suggestions how to extract this during boot and
he suggested seeing if there was a copy of the firmware in the
EFI_BOOT_SERVICES_CODE memory segment, which it turns out there is.

My patch to add support for this contains a table of device-model (dmi
strings), firmware header (first 64 bits), length and crc32 and then if
we boot on a device-model which is in the table the code scans the
EFI_BOOT_SERVICES_CODE for the prefix, if found checks the crc and
caches the firmware for later use by request-firmware.

So I just do a brute-force search for the firmware, this really is hack,
nothing standard about it I'm afraid. But it works on 4 different x86
tablets I have and makes the touchscreen work OOTB on them, so I believe
it is a worthwhile hack to have.


2) Despite repeated attempts we have failed to get permission to
redistribute the firmware. This is especially a problem with customized
firmwares, these get created by the chip vendor for a specific ODM and the
copyright may partially belong with the ODM, so the chip vendor cannot
give a blanket permission to distribute these.

This commit adds support for finding peripheral firmware embedded in the
EFI code and making this available to peripheral drivers through the
standard firmware loading mechanism.


Neat.


Note we check the EFI_BOOT_SERVICES_CODE for embedded firmware pretty
late in the init sequence,


This also creates a technical limitation on use for the API that users
should be aware of. Its important to document such limitation.


I don't think this is a problem for any normal drivers, when I say pretty
late I mean late in init/main.c: start_kernel(), so still before any normal
drivers load.

The first idea was to scan for the firmware at the same time we check for
things as the ACPI BGRT logo stuff, but as mentioned that requires using
early_mmap() which does not work for the amount of memory we want to map.


Also if we can address the limitation that would be even better.

For instance, on what part of the driver is the call to request firmware
being made? Note that we support async probe now, so if the call was done
on probe, it may be wise to use async probe, however, can we be *certain*
that the EFI firmware would have been parsed and ready by then? Please
check. It just may be the case.

Or, if we use late_initcall() would that suffice on the driver, if they
used a request firmware call on init or probe?


As said I think we still do it early enough for any driver use, when
I wrote "late in the init sequence" I should have probably written something
else, like "near the end of start_kernel() instead of from setup_arch()"




this is on purpose because the typical
EFI_BOOT_SERVICES_CODE memory-segment is too large for early_memremap().


To be clear you neede to use memremap()


Yes.


What mechanism would have in place to ensure that a driver which expects
firmware to be on EFI data to be already available prior to its driver's
call to initialize?


See above, this still runs before start_kernel() calls rest_init() which is
where any normal init calls (and driver probing) happens so still early
enough for any users I can think of. I think my poorly worded commit
message is causing a