[PATCH] PTI: unbreak EFI old_memmap

[Jiri Kosina]

From: Jiri Kosina <jkos...@suse.cz>

old_memmap's efi_call_phys_prolog() calls set_pgd() with swapper PGD that 
has PAGE_USER set, which makes PTI set NX on it, and therefore EFI can't 
execute it's code.

Fix that by forcefully clearing _PAGE_NX from the PGD (this can't be done 
by the pgprot API).

_PAGE_NX will be automatically reintroduced in efi_call_phys_epilog(), as 
_set_pgd() will again notice that this is _PAGE_USER, and set _PAGE_NX on 
it.

Signed-off-by: Jiri Kosina <jkos...@suse.cz>
Tested-by: Dimitri Sivanich <sivan...@hpe.com>
Acked-by: Dave Hansen <dave.han...@linux.intel.com>
--- 
 arch/x86/platform/efi/efi_64.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/x86/platform/efi/efi_64.c b/arch/x86/platform/efi/efi_64.c
index d87ac96e37ed..2dd15e967c3f 100644
--- a/arch/x86/platform/efi/efi_64.c
+++ b/arch/x86/platform/efi/efi_64.c
@@ -135,7 +135,9 @@ pgd_t * __init efi_call_phys_prolog(void)
pud[j] = *pud_offset(p4d_k, vaddr);
}
}
+   pgd_offset_k(pgd * PGDIR_SIZE)->pgd &= ~_PAGE_NX;
}
+
 out:
    __flush_tlb_all();
 

-- 
Jiri Kosina
SUSE Labs

--
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 26/30] Lock down ftrace

[Jiri Kosina]

On Fri, 10 Nov 2017, David Howells wrote:

> That would be fine by me.  I have a patch that locks down kprobes in this
> series.  

Which AFAICS renders locking down ftrace as-is unnecessary ...

> Steven says that ftrace might acquire the ability to dump registers in 
> the future.

... and even if that happens, locking down only that particular feature of 
ftrace would be needed.

Thanks,

-- 
Jiri Kosina
SUSE Labs

--
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 26/30] Lock down ftrace

[Jiri Kosina]

On Fri, 10 Nov 2017, David Howells wrote:

> > I fail to see how this fits into the secure boot security model, could you 
> > please explain?
> 
> The idea is to prevent cryptographic data for filesystems and other things
> from being read out of the kernel memory as well as to prevent unauthorised
> modification of kernel memory.

Then it would make sense to actually lock down dumping of registers / 
function arguments (kprobes can currently do that, ftrace eventually could 
as well I guess), but disabling the whole ftrace altogether seems like a 
totally unnecessary overkill.

> > Secure boot is about having a constant proof / verification that the code 
> > you're running in ring0 can be trusted (IOW is the one that has been 
> > signed and verified by the whole boot chain).
> > 
> > Checking execution patterns doesn't seem to fit at all.
> 
> I'll defer this question to Alexei since he suggested I needed to deal 
> with this too.

Thanks.

-- 
Jiri Kosina
SUSE Labs

--
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 26/30] Lock down ftrace

[Jiri Kosina]

On Thu, 9 Nov 2017, David Howells wrote:

> Disallow the use of ftrace when the kernel is locked down.  This patch
> turns off ftrace_enabled late in the kernel boot so that the selftest can
> still be potentially be run.
> 
> The sysctl that controls ftrace_enables is also disallowed when the kernel
> is locked down.  If the lockdown is lifted, then the sysctl can be used to
> reenable ftrace - if ftrace was compiled with CONFIG_DYNAMIC_FTRACE, that
> is; if it wasn't then it won't be possible to reenable it.
> 
> This prevents crypto data theft by analysis of execution patterns, and, if
> in future ftrace also logs the register contents at the time, will prevent
> data theft by that mechanism also.

I fail to see how this fits into the secure boot security model, could you 
please explain?

Secure boot is about having a constant proof / verification that the code 
you're running in ring0 can be trusted (IOW is the one that has been 
signed and verified by the whole boot chain).

Checking execution patterns doesn't seem to fit at all.

Thanks,

-- 
Jiri Kosina
SUSE Labs
--
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 11/24] uswsusp: Disable when the kernel is locked down

[Jiri Kosina]

On Thu, 6 Apr 2017, Rafael J. Wysocki wrote:

> >>> Your swap partition may be located on an NVDIMM or be encrypted.
> >>
> >> An NVDIMM should be considered the same as any other persistent storage.
> >>
> >> It may be encrypted, but where's the key stored, how easy is it to retrieve
> >> and does the swapout code know this?
> >>
> >>> Isn't this a bit overly drastic?
> >>
> >> Perhaps, but if it's on disk and it's not encrypted, then maybe not.
> >
> > Right.
> >
> > Swap encryption is not mandatory and I'm not sure how the hibernate
> > code can verify whether or not it is in use.
> 
> BTW, SUSE has patches adding secure boot support to the hibernate code
> and Jiri promised me to post them last year even. :-)

Oh, thanks for a friendly ping :) Adding Joey Lee to CC.

-- 
Jiri Kosina
SUSE Labs

--
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 08/16] x86/efi: Carrying hibernation key by setup data

[Jiri Kosina]

On Sat, 15 Aug 2015, Pavel Machek wrote:

  For forwarding hibernation key from EFI stub to boot kernel, this patch
  allocates setup data for carrying hibernation key, size and the status
  of efi operating.
  
  Reviewed-by: Jiri Kosina jkos...@suse.com
 
 Jiri, are you sure you reviewed these? This is not really
 english, afaict, and efi/EFI should be spelled consistently.

Yes, I did review the code and the fact that it is still in compliance 
with my original idea. I think you can blame me on not reviewing 
changelogs super-rigorously though, so all suggestions for improvements 
are absolutely welcome.

Thanks,

-- 
Jiri Kosina
SUSE Labs

--
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] Revert x86/mm/ASLR: Propagate base load address calculation

[Jiri Kosina]

On Mon, 16 Mar 2015, Borislav Petkov wrote:

 On Sat, Mar 14, 2015 at 10:59:23AM +0100, Borislav Petkov wrote:
  In addition to what you've said, I have only one proposal: we revert
  the whole crap and do it again, this time nice and clean in tip. It'll
  stay there as long as it has to. No half-arsed commit messages, no
  misunderstood crap, no lala code f*ckery. Whatever it takes, as long as
  it takes.
 
 And here it is:
 
 ---
 From: Borislav Petkov b...@suse.de
 Date: Mon, 16 Mar 2015 10:57:56 +0100
 Subject: [PATCH] Revert x86/mm/ASLR: Propagate base load address calculation
 
 This reverts commit f47233c2d34f243ecdaac179c3408a39ff9216a7.
 
 The main reason for the revert is that in order to make this work, we
 need non-trivial changes to the x86 boot code which we didn't manage to
 get done in time for merging.
 
 And even if we did, they would've been too risky so instead of rushing
 things and break booting 4.1 on boxes left and right, we will be very
 strict and conservative and will take our time with this to fix and test
 it properly.
 
 Signed-off-by: Borislav Petkov b...@suse.de
 Cc: Jiri Kosina jkos...@suse.cz

Agreed. Let's work on a better refresh for 4.1+. The fix attempts were 
quite chaotic.

Acked-by: Jiri Kosina jkos...@suse.cz

-- 
Jiri Kosina
SUSE Labs
--
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 v3 2/7] x86, boot: Move ZO to end of buffer

[Jiri Kosina]

On Tue, 10 Mar 2015, Borislav Petkov wrote:

 Final patch:
 
 ---
 From: Yinghai Lu ying...@kernel.org
 Date: Sat, 7 Mar 2015 14:07:16 -0800
 Subject: [PATCH] x86/setup: Move compressed kernel to the end of the buffer
[ ... ]
 Signed-off-by: Yinghai Lu ying...@kernel.org
 Cc: H. Peter Anvin h...@zytor.com
 Cc: Matt Fleming matt.flem...@intel.com
 Cc: Kees Cook keesc...@chromium.org
 Cc: Thomas Gleixner t...@linutronix.de
 Cc: Jiri Kosina jkos...@suse.cz
 Cc: linux-efi@vger.kernel.org
 Cc: Ingo Molnar mi...@redhat.com
 Cc: Baoquan He b...@redhat.com
 Fixes: f47233c2d34f (x86/mm/ASLR: Propagate base load address calculation)

Thanks a lot for fixing my oversight.

Acked-by: Jiri Kosina jkos...@suse.cz

 Link: 
 http://lkml.kernel.org/r/1425766041-6551-3-git-send-email-ying...@kernel.org
 [ Commit message massively rewritten ]
 Signed-off-by: 
 ---
  arch/x86/boot/compressed/head_32.S | 11 +--
  arch/x86/boot/compressed/head_64.S |  8 ++--
  arch/x86/boot/compressed/mkpiggy.c |  7 ++-
  arch/x86/boot/compressed/vmlinux.lds.S |  1 +
  arch/x86/boot/header.S |  2 +-
  arch/x86/kernel/asm-offsets.c  |  1 +
  arch/x86/kernel/vmlinux.lds.S  |  1 +
  7 files changed, 21 insertions(+), 10 deletions(-)
 
 diff --git a/arch/x86/boot/compressed/head_32.S 
 b/arch/x86/boot/compressed/head_32.S
 index cbed1407a5cd..a9b56f1d8e75 100644
 --- a/arch/x86/boot/compressed/head_32.S
 +++ b/arch/x86/boot/compressed/head_32.S
 @@ -147,7 +147,9 @@ preferred_addr:
  1:
  
   /* Target address to relocate to for decompression */
 - addl$z_extract_offset, %ebx
 + movlBP_init_size(%esi), %eax
 + subl$_end, %eax
 + addl%eax, %ebx
  
   /* Set up the stack */
   lealboot_stack_end(%ebx), %esp
 @@ -208,8 +210,13 @@ relocated:
   */
   /* push arguments for decompress_kernel: */
   pushl   $z_output_len   /* decompressed length */
 - lealz_extract_offset_negative(%ebx), %ebp
 +
 + movlBP_init_size(%esi), %eax
 + subl$_end, %eax
 + movl%ebx, %ebp
 + subl%eax, %ebp
   pushl   %ebp/* output address */
 +
   pushl   $z_input_len/* input_len */
   lealinput_data(%ebx), %eax
   pushl   %eax/* input_data */
 diff --git a/arch/x86/boot/compressed/head_64.S 
 b/arch/x86/boot/compressed/head_64.S
 index 2884e0c3e8a5..69015b576cf6 100644
 --- a/arch/x86/boot/compressed/head_64.S
 +++ b/arch/x86/boot/compressed/head_64.S
 @@ -101,7 +101,9 @@ ENTRY(startup_32)
  1:
  
   /* Target address to relocate to for decompression */
 - addl$z_extract_offset, %ebx
 + movlBP_init_size(%esi), %eax
 + subl$_end, %eax
 + addl%eax, %ebx
  
  /*
   * Prepare for entering 64 bit mode
 @@ -329,7 +331,9 @@ preferred_addr:
  1:
  
   /* Target address to relocate to for decompression */
 - leaqz_extract_offset(%rbp), %rbx
 + movlBP_init_size(%rsi), %ebx
 + subl$_end, %ebx
 + addq%rbp, %rbx
  
   /* Set up the stack */
   leaqboot_stack_end(%rbx), %rsp
 diff --git a/arch/x86/boot/compressed/mkpiggy.c 
 b/arch/x86/boot/compressed/mkpiggy.c
 index b669ab65bf6c..c03b0097ce58 100644
 --- a/arch/x86/boot/compressed/mkpiggy.c
 +++ b/arch/x86/boot/compressed/mkpiggy.c
 @@ -80,11 +80,8 @@ int main(int argc, char *argv[])
   printf(z_input_len = %lu\n, ilen);
   printf(.globl z_output_len\n);
   printf(z_output_len = %lu\n, (unsigned long)olen);
 - printf(.globl z_extract_offset\n);
 - printf(z_extract_offset = 0x%lx\n, offs);
 - /* z_extract_offset_negative allows simplification of head_32.S */
 - printf(.globl z_extract_offset_negative\n);
 - printf(z_extract_offset_negative = -0x%lx\n, offs);
 + printf(.globl z_min_extract_offset\n);
 + printf(z_min_extract_offset = 0x%lx\n, offs);
  
   printf(.globl input_data, input_data_end\n);
   printf(input_data:\n);
 diff --git a/arch/x86/boot/compressed/vmlinux.lds.S 
 b/arch/x86/boot/compressed/vmlinux.lds.S
 index 34d047c98284..a80acabb80ec 100644
 --- a/arch/x86/boot/compressed/vmlinux.lds.S
 +++ b/arch/x86/boot/compressed/vmlinux.lds.S
 @@ -70,5 +70,6 @@ SECTIONS
   _epgtable = . ;
   }
  #endif
 + . = ALIGN(PAGE_SIZE);   /* keep size page-aligned */
   _end = .;
  }
 diff --git a/arch/x86/boot/header.S b/arch/x86/boot/header.S
 index 16ef02596db2..9bfab22efdf7 100644
 --- a/arch/x86/boot/header.S
 +++ b/arch/x86/boot/header.S
 @@ -440,7 +440,7 @@ setup_data:   .quad 0 # 
 64-bit physical pointer to
  
  pref_address:.quad LOAD_PHYSICAL_ADDR# preferred 
 load addr
  
 -#define ZO_INIT_SIZE (ZO__end - ZO_startup_32 + ZO_z_extract_offset)
 +#define ZO_INIT_SIZE (ZO__end - ZO_startup_32 + ZO_z_min_extract_offset)
  #define VO_INIT_SIZE (VO__end - VO__text)
  #if ZO_INIT_SIZE  VO_INIT_SIZE
  #define

Re: [RFC V4 PATCH 00/15] Signature verification of hibernate snapshot

[Jiri Kosina]

On Wed, 25 Sep 2013, James Bottomley wrote:

  I don't get this. Why is it important that current kernel can't
  recreate the signature?
 
 The thread model is an attack on the saved information (i.e. the suspend
 image) between it being saved by the old kernel and used by the new one.
 The important point isn't that the new kernel doesn't have access to
 K_{N-1} it's that no-one does: the key is destroyed as soon as the old
 kernel terminates however the verification public part P_{N-1} survives.

James,

could you please describe the exact scenario you think that the symmetric 
keys aproach doesn't protect against, while the assymetric key aproach 
does?

The crucial points, which I believe make the symmetric key aproach work 
(and I feel quite embarassed by the fact that I haven't realized this 
initially when coming up with the assymetric keys aproach) are:

- the kernel that is performing the actual resumption is trusted in the 
  secure boot model, i.e. you trust it to perform proper verification

- potentially malicious userspace (which is what we are protecting against 
  -- malicious root creating fake hibernation image and issuing reboot) 
  doesn't have access to the symmetric key

-- 
Jiri Kosina
SUSE Labs
--
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 0/4] EFI 1:1 mapping

[Jiri Kosina]

On Thu, 20 Jun 2013, Matthew Garrett wrote:

  This will break the Macs so maybe we can do
  
  efi=no_11_map
  
  so the Macs can still boot but use the 1:1 map by default.
 
 I'm going to guess that there are more people running unmodified Linux 
 kernels on Macs than there are people using kexec, 

I'd be careful in order not to underestimate how much kexec is being used. 
[At least some] distros are using it during installation process.

-- 
Jiri Kosina
SUSE Labs

--
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 0/4] EFI 1:1 mapping

[Jiri Kosina]

On Thu, 20 Jun 2013, Matthew Garrett wrote:

  Can't we detect Macs from some of the UEFI strings at boot time and do
  the right thing with the boot switch (which can be overriden from the
  kernel command line if we get it wrong)?
 
 Yes, and then our behaviour differs from Windows

How so? Windows don't work on those older Macs as well, do they?

So if we properly detect those (and only those), we mimic Windows 
completely, right?

-- 
Jiri Kosina
SUSE Labs

--
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: [regression, bisected] x86: efi: Pass boot services variable info to runtime code

[Jiri Kosina]

On Fri, 31 May 2013, Ingo Molnar wrote:

 So this change needs to be reverted or fixed.

I don't think anyone is arguing against that.

My remark was purely to describe the current status quo and help to 
understand what exactly is happening, i.e.:

- QueryVariableInfo() should be a valid thing to do from inside boot  
  environment, according to the spec
- now we see that at least SGI bios (an probably other incarnations) 
  think otherwise
- if we are not able to fix / work around the bug in BIOS (*), we have to 
  make a choice between two evils -- either increase likelyhood of 
  bricking certain machines due to filling the EFI storage space, or 
  break booting on broken BIOSen
- the theory is that Borislav's 1:1 mapping patches will work this around; 
  one of the supporting arguments being that it's probably what Windows is 
  doing. I believe Borislav is in the process of testing this. But the 
  patches are not ready for mainline yet.

(*) If one would be naive enough, he'd believe that the pressure should be 
put on the BIOS writers instead of OS to fix the bug :)

-- 
Jiri Kosina
SUSE Labs
--
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: [regression, bisected] x86: efi: Pass boot services variable info to runtime code

[Jiri Kosina]

On Thu, 30 May 2013, Russ Anderson wrote:

 Yes, but this call is clearly happening way before ExitBootServices() 
 -- 
 see the surrounding code, see for example this in efi_main():
 
 [ ... snip ... ]
   setup_efi_vars(boot_params);
 
   setup_efi_pci(boot_params);
 
   status = efi_call_phys3(sys_table-boottime-allocate_pool,
   EFI_LOADER_DATA, sizeof(*gdt),
   (void **)gdt);
   if (status != EFI_SUCCESS) {
   efi_printk(Failed to alloc mem for gdt structure\n);
   goto fail;
   }
 [ ... snip ... ]

Yes.  Note the failing call is sys_table-runtime while all the
other calls are sys_table-boottime and seem to work.  Not sure
why the sys_table-runtime call has a problem but it may be
a clue.  Could something in the runtime path not be set up???
   
   That was my original idea early today as well. My understanding of the 
   UEFI spec is admittedly limited, but afaics calling runtime method from 
   boot environment should be a valid thing to do ... ?
  
  QueryVariableInfo() is a runtime services, all runtime services should
  available bother on boot time and runtime:
  
  UEFI spec 2.3.1 P.109:
Runtime Services
Functions that are available before and after any call to  
ExitBootServices(). These functions are described in Section 7.
 
 That's a great idea.  This patch moves the QueryVariableInfo()
 call from bootime to runtime, in efi_late_init().  The attached
 patch is consistent with the UEFI spec and avoids the problem.

Unfortunately that means that you can as well throw the patch away 
completely.

The sole point is to run the QueryVariableInfo() from the boot 
environment, in order to obtain more accurate information.
And it's a valid thing to do, according to UEFI specification.

-- 
Jiri Kosina
SUSE Labs
--
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] x86,efi: Implement efi_no_storage_paranoia parameter

[Jiri Kosina]

On Wed, 17 Apr 2013, Richard Weinberger wrote:

 Using this parameter one can disable the storage_size/2 check if
 he is really sure that the UEFI does sane gc and fulfills the spec.
 
 This parameter is useful if a devices uses more than 50% of the
 storage by default.
 The Intel DQSW67 desktop board is such a sucker for exmaple.
 
 Signed-off-by: Richard Weinberger rich...@nod.at
 ---
  Documentation/kernel-parameters.txt |  6 ++
  arch/x86/platform/efi/efi.c | 15 ++-
  2 files changed, 20 insertions(+), 1 deletion(-)
 
 diff --git a/Documentation/kernel-parameters.txt 
 b/Documentation/kernel-parameters.txt
 index 4609e81..d1cc3a9 100644
 --- a/Documentation/kernel-parameters.txt
 +++ b/Documentation/kernel-parameters.txt
 @@ -788,6 +788,12 @@ bytes respectively. Such letter suffixes can also be 
 entirely omitted.
   edd=[EDD]
   Format: {off | on | skip[mbr]}
  
 + efi_no_storage_paranoia [EFI; X86]
 + Using this parameter you can use more than 50% of
 + your efi variable storage. Use this parameter only if
 + you are really sure that your UEFI does sane gc and
 + fulfills the spec otherwise your board may brick.
 +
   eisa_irq_edge=  [PARISC,HW]
   See header of drivers/parisc/eisa.c.
  
 diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c
 index 4959e3f..07524e1 100644
 --- a/arch/x86/platform/efi/efi.c
 +++ b/arch/x86/platform/efi/efi.c
 @@ -113,6 +113,16 @@ static int __init setup_add_efi_memmap(char *arg)
  }
  early_param(add_efi_memmap, setup_add_efi_memmap);
  
 +static bool efi_no_storage_paranoia;
 +EXPORT_SYMBOL_GPL(efi_no_storage_paranoia);

Is there any particular reason to export this symbol?

-- 
Jiri Kosina
SUSE Labs

--
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: [RFC] Second attempt at kernel secure boot support

[Jiri Kosina]

On Wed, 31 Oct 2012, Matthew Garrett wrote:

  Reading stored memory image (potentially tampered before reboot) from disk 
  is basically DMA-ing arbitrary data over the whole RAM. I am currently not 
  able to imagine a scenario how this could be made secure (without 
  storing private keys to sign the hibernation image on the machine itself 
  which, well, doesn't sound secure either).
 
 shim generates a public and private key. 

It seems to me that this brings quite a huge delay into the boot process 
both for regular and resume cases (as shim has no way to know what is 
going to happen next). Mostly because obtaining enough entropy is 
generally very difficult when we have just shim running, right?

 It hands the kernel the private key in a boot parameter and stores the 
 public key in a boot variable. On suspend, the kernel signs the suspend 
 image with that private key and discards it. On the next boot, shim 
 generates a new key pair and hands the new private key to the kernel 
 along with the old public key. The kernel verifies the suspend image 
 before resuming it. The only way to subvert this would be to be able to 
 access kernel memory directly, which means the attacker has already won.

I like this protocol, but after some off-line discussions, I still have 
doubts about it. Namely: how do we make sure that there is noone tampering 
with the variable?

Obvious step towards solving this is making the variable inaccessible 
after ExitBootServices() has been called (by not setting runtime access 
flag on it).

Now how about this scenario:

- consider securely booted win8 (no Linux installed on that machine, so 
  the variable for storing public key doesn't exist yet), possibly being 
  taken over by a malicious user
- he/she creates this secure variable from within the win8 and stores 
  his/her own public key into it
- he/she supplies a signed shim (as provided by some Linux distro vendor), 
  signed kernel (as provided by some Linux distro vendor) and specially 
  crafted resume image, signed by his/her own private key
- he/she reboots the machine in a way that shim+distro kernel+hacker's S4 
  image is used to resume
- distro kernel verifies the signature of the S4 image against the 
  attacker's public key stored in the variable; the signature is OK
- he/she has won, as he has managed to run an arbitrary kernel code 
  (stored in the S4 image) in a trusted mode

No?

Basically, once the machine is already populated with the secure version 
of Linux, this can't happen, as we will (as far as I understand) set the 
variable for storing the public key in a way that it can't be accessed 
from runtime environment. But how can we prevent it being *created* before 
the machine is ever touched by Linux?

Thanks,

-- 
Jiri Kosina
SUSE Labs
--
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: [RFC] Second attempt at kernel secure boot support

[Jiri Kosina]

On Sun, 4 Nov 2012, Eric W. Biederman wrote:

  Why is when kernel has been securely booted, the in-kernel kexec 
  mechanism has to verify the signature of the supplied image before 
  kexecing it not enough? (basically the same thing we are doing for signed 
  modules already).
 
 For modules the only untrusted part of their environment are the command
 line parameters, and several of those have already been noted for
 needing to be ignored in a non-trusted root scenario.
 
 For kexec there is a bunch of glue code and data that takes care of
 transitioning from the environment provided by kexec and the environment
 that the linux kernel or memtest86 or whatever we are booting is
 expecting.
 
 Figuring out what glue code and data we need and supplying that glue
 code and data is what kexec-tools does.  The situation is a bit like
 dealing with the modules before most of the work of insmod was moved
 into the kernel.
 
 For kexec-tools it is desirable to have glue layers outside of the
 kernel because every boot system in existence has a different set of
 parameter passing rules.
 
 So signing in the kernel gets us into how do we sign the glue code and
 how dow we verify the glue code will jump to our signed and verified
 kernel image.

Do I understand you correctly that by the 'glue' stuff you actually mean 
the division of the kexec image into segments?

Of course, when we are dividing the image into segments and then passing 
those individually (even more so if some transformations are performed on 
those segments, which I don't know whether that's the case or not), then 
we can't do any signature verification of the image any more.

But I still don't fully understand what is so magical about taking the 
kernel image as is, and passing the whole lot to the running kernel as-is, 
allowing for signature verification.

Yes, it couldn't be sys_kexec_load(), as that would be ABI breakage, so 
it'd mean sys_kexec_raw_load(), or whatever ... but I fail to see why that 
would be problem in principle.

If you can point me to the code where all the magic that prevents this 
easy handling is happening, I'd appreciate it.

Thanks,

-- 
Jiri Kosina
SUSE Labs
--
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: [RFC] Second attempt at kernel secure boot support

[Jiri Kosina]

On Mon, 5 Nov 2012, Jiri Kosina wrote:

 Do I understand you correctly that by the 'glue' stuff you actually mean 
 the division of the kexec image into segments?
 
 Of course, when we are dividing the image into segments and then passing 
 those individually (even more so if some transformations are performed on 
 those segments, which I don't know whether that's the case or not), then 
 we can't do any signature verification of the image any more.
 
 But I still don't fully understand what is so magical about taking the 
 kernel image as is, and passing the whole lot to the running kernel as-is, 
 allowing for signature verification.
 
 Yes, it couldn't be sys_kexec_load(), as that would be ABI breakage, so 
 it'd mean sys_kexec_raw_load(), or whatever ... but I fail to see why that 
 would be problem in principle.
 
 If you can point me to the code where all the magic that prevents this 
 easy handling is happening, I'd appreciate it.

OK, so after wandering through kexec-tools sources for a while, I am 
starting to get your point. I wasn't actually aware of the fact that it 
supports such a wide variety of binary formats etc. (multiboot, nbi, etc).

I had a naive idea of just putting in-kernel verification of a complete 
ELF binary passed to kernel by userspace, and if the signature matches, 
jumping to it.
Would work for elf-x86_64 nicely I guess, but we'd lose a lot of other 
functionality currently being provided by kexec-tools.

Bah. This is a real pandora's box.

-- 
Jiri Kosina
SUSE Labs
--
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: [RFC] Second attempt at kernel secure boot support

[Jiri Kosina]

On Mon, 29 Oct 2012, Matthew Garrett wrote:

   This is pretty much identical to the first patchset, but with the 
   capability
   renamed (CAP_COMPROMISE_KERNEL) and the kexec patch dropped. If anyone 
   wants
   to deploy these then they should disable kexec until support for signed
   kexec payloads has been merged.
  
  Apparently your patchset currently doesn't handle device firmware loading, 
  nor do you seem to mention in in the comments.
 
 Correct.
 
  I believe signed firmware loading should be put on plate as well, right?
 
 I think that's definitely something that should be covered. I hadn't 
 worried about it immediately as any attack would be limited to machines 
 with a specific piece of hardware, and the attacker would need to expend 
 a significant amount of reverse engineering work on the firmware - and 
 we'd probably benefit from them doing that in the long run...

Now -- how about resuming from S4?

Reading stored memory image (potentially tampered before reboot) from disk 
is basically DMA-ing arbitrary data over the whole RAM. I am currently not 
able to imagine a scenario how this could be made secure (without 
storing private keys to sign the hibernation image on the machine itself 
which, well, doesn't sound secure either).

-- 
Jiri Kosina
SUSE Labs
--
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: [RFC] Second attempt at kernel secure boot support

[Jiri Kosina]

On Wed, 31 Oct 2012, Alan Cox wrote:

 All this depends on your threat model. If I have physical access to
 suspend/resume your machine then you already lost. If I don't have
 physical access then I can't boot my unsigned OS to patch your S4 image
 so it doesn't matter.

Prepare (as a root) a hand-crafted image, reboot, let the kernel resume 
from that artificial image.

It can be viewed as a very obscure way of rewriting the kernel through 
/dev/mem (which is obviously not possible when in 'secure boot' 
environment).

-- 
Jiri Kosina
SUSE Labs
--
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