Re: [PATCH 12/12] Add option to automatically set trusted_kernel when in Secure Boot mode

2015-04-22 Thread Dan Carpenter
On Fri, Mar 13, 2015 at 11:38:28AM -1000, Matthew Garrett wrote:
> UEFI Secure Boot provides a mechanism for ensuring that the firmware will
> only load signed bootloaders and kernels. Certain use cases may also
> require that the kernel prevent userspace from inserting untrusted kernel
> code at runtime. Add a configuration option that enforces this automatically
> when enabled.
> 
> Signed-off-by: Matthew Garrett 
> ---
>  Documentation/x86/zero-page.txt   |  2 ++
>  arch/x86/Kconfig  | 13 +
>  arch/x86/boot/compressed/eboot.c  | 33 +
>  arch/x86/include/uapi/asm/bootparam.h |  3 ++-
>  arch/x86/kernel/setup.c   |  6 ++
>  5 files changed, 56 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/x86/zero-page.txt b/Documentation/x86/zero-page.txt
> index 82fbdbc..a811210 100644
> --- a/Documentation/x86/zero-page.txt
> +++ b/Documentation/x86/zero-page.txt
> @@ -30,6 +30,8 @@ Offset  Proto   NameMeaning
>  1E9/001  ALL eddbuf_entries  Number of entries in eddbuf (below)
>  1EA/001  ALL edd_mbr_sig_buf_entries Number of entries in 
> edd_mbr_sig_buffer
>   (below)
> +1EB/001  ALL kbd_status  Numlock is enabled

This line looks like it was included by mistake?

> +1EC/001  ALL secure_boot Secure boot is enabled in the firmware

regards,
dan carpenter

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


[PATCH 12/12] Add option to automatically set trusted_kernel when in Secure Boot mode

2015-03-13 Thread Matthew Garrett
UEFI Secure Boot provides a mechanism for ensuring that the firmware will
only load signed bootloaders and kernels. Certain use cases may also
require that the kernel prevent userspace from inserting untrusted kernel
code at runtime. Add a configuration option that enforces this automatically
when enabled.

Signed-off-by: Matthew Garrett 
---
 Documentation/x86/zero-page.txt   |  2 ++
 arch/x86/Kconfig  | 13 +
 arch/x86/boot/compressed/eboot.c  | 33 +
 arch/x86/include/uapi/asm/bootparam.h |  3 ++-
 arch/x86/kernel/setup.c   |  6 ++
 5 files changed, 56 insertions(+), 1 deletion(-)

diff --git a/Documentation/x86/zero-page.txt b/Documentation/x86/zero-page.txt
index 82fbdbc..a811210 100644
--- a/Documentation/x86/zero-page.txt
+++ b/Documentation/x86/zero-page.txt
@@ -30,6 +30,8 @@ OffsetProto   NameMeaning
 1E9/001ALL eddbuf_entries  Number of entries in eddbuf (below)
 1EA/001ALL edd_mbr_sig_buf_entries Number of entries in 
edd_mbr_sig_buffer
(below)
+1EB/001ALL kbd_status  Numlock is enabled
+1EC/001ALL secure_boot Secure boot is enabled in the firmware
 1EF/001ALL sentinelUsed to detect broken bootloaders
 290/040ALL edd_mbr_sig_buffer EDD MBR signatures
 2D0/A00ALL e820_mapE820 memory map table
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index b7d31ca..0f9ee6f 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -1695,6 +1695,19 @@ config EFI_MIXED
 
   If unsure, say N.
 
+config EFI_SECURE_BOOT_TRUSTED_KERNEL
+def_bool n
+   depends on SECURITY_TRUSTED_KERNEL
+   depends on EFI
+   prompt "Automatically set trusted_kernel with UEFI Secure Boot"
+   ---help---
+ UEFI Secure Boot provides a mechanism for ensuring that the
+ firmware will only load signed bootloaders and kernels. Certain
+ use cases may also require that the kernel restrict any userspace
+ mechanism that could insert untrusted code into the kernel.
+ Say Y here to automatically enable trusted_kernel enforcement
+ when a system boots with UEFI Secure Boot enabled.
+
 config SECCOMP
def_bool y
prompt "Enable seccomp to safely compute untrusted bytecode"
diff --git a/arch/x86/boot/compressed/eboot.c b/arch/x86/boot/compressed/eboot.c
index ef17683..4def2aa 100644
--- a/arch/x86/boot/compressed/eboot.c
+++ b/arch/x86/boot/compressed/eboot.c
@@ -12,6 +12,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "../string.h"
 #include "eboot.h"
@@ -1030,6 +1031,36 @@ void setup_graphics(struct boot_params *boot_params)
}
 }
 
+static int get_secure_boot(void)
+{
+   u8 sb, setup;
+   unsigned long datasize = sizeof(sb);
+   efi_guid_t var_guid = EFI_GLOBAL_VARIABLE_GUID;
+   efi_status_t status;
+
+   status = efi_call_phys5(sys_table->runtime->get_variable,
+   L"SecureBoot", &var_guid, NULL, &datasize, &sb);
+
+   if (status != EFI_SUCCESS)
+   return 0;
+
+   if (sb == 0)
+   return 0;
+
+
+   status = efi_call_phys5(sys_table->runtime->get_variable,
+   L"SetupMode", &var_guid, NULL, &datasize,
+   &setup);
+
+   if (status != EFI_SUCCESS)
+   return 0;
+
+   if (setup == 1)
+   return 0;
+
+   return 1;
+}
+
 /*
  * Because the x86 boot code expects to be passed a boot_params we
  * need to create one ourselves (usually the bootloader would create
@@ -1406,6 +1437,8 @@ struct boot_params *efi_main(struct efi_config *c,
else
setup_boot_services32(efi_early);
 
+   boot_params->secure_boot = get_secure_boot();
+
setup_graphics(boot_params);
 
setup_efi_pci(boot_params);
diff --git a/arch/x86/include/uapi/asm/bootparam.h 
b/arch/x86/include/uapi/asm/bootparam.h
index 44e6dd7..3ddf415 100644
--- a/arch/x86/include/uapi/asm/bootparam.h
+++ b/arch/x86/include/uapi/asm/bootparam.h
@@ -134,7 +134,8 @@ struct boot_params {
__u8  eddbuf_entries;   /* 0x1e9 */
__u8  edd_mbr_sig_buf_entries;  /* 0x1ea */
__u8  kbd_status;   /* 0x1eb */
-   __u8  _pad5[3]; /* 0x1ec */
+   __u8  secure_boot;  /* 0x1ec */
+   __u8  _pad5[2]; /* 0x1ed */
/*
 * The sentinel is set to a nonzero value (0xff) in header.S.
 *
diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index 98dc931..980e308 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -50,6 +50,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -1165,6 +1166,11 @@ v

Re: [PATCH 12/12] Add option to automatically set trusted_kernel when in Secure Boot mode

2014-02-27 Thread Kees Cook
On Wed, Feb 26, 2014 at 2:48 PM, Matthew Garrett
 wrote:
> On Wed, 2014-02-26 at 22:41 +, One Thousand Gnomes wrote:
>> Another issue that needs addressing is firmware. Quite a few of our
>> request_firmware cases load device firmware which is not signed into DMA
>> capable hardware. Probably also worth checking what the
>> architectural guarantees on bogus microcode updates is. Maybe we need
>> firmware signing for such cases to match the mod signing ?
>
> Vendors keep telling me that they're validating firmware for new
> hardware, and I keep tending not to believe them. Meh. The big problem
> with firmware signatures is that we don't necessarily have the right to
> distribute modified versions of the firmware, so we'd need detached
> signature support. I'm certainly not against this.

I have been working on a patch series for this. It will have LSM hooks
for validating firmware origin (via fd) and contents (via blob),
similar to the changes I made for validating module origins. It just
need to finish testing, and I'll post the series. If you want to check
it out in its current state, it's here:

http://git.kernel.org/cgit/linux/kernel/git/kees/linux.git/log/?h=fw-restrict

-Kees

-- 
Kees Cook
Chrome OS Security
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 12/12] Add option to automatically set trusted_kernel when in Secure Boot mode

2014-02-26 Thread Matthew Garrett
On Wed, 2014-02-26 at 22:41 +, One Thousand Gnomes wrote:

> I think you have a load more cases to attempt to paper over before you
> even pretend to achieve that goal. Firewire for example. Also it only
> remotely begins to work if you also force CAP_SYS_RAWIO off globally as
> you need to force off things like raw command issuing to various
> controllers (especially as some of that code is written on the basis that
> 'its RAWIO, screw making it secure and doing all the checks we could
> bother with'.

Physical presence is required to do anything meaningful with firewire,
and UEFI secure boot isn't intended to protect against that. Which
controllers will trigger arbitrary DMA in response to raw commands?

> RAWIO also disables things like CPU msr access - which is also quite
> adequate for subverting a kernel.

Patch 7.

> Another issue that needs addressing is firmware. Quite a few of our
> request_firmware cases load device firmware which is not signed into DMA
> capable hardware. Probably also worth checking what the
> architectural guarantees on bogus microcode updates is. Maybe we need
> firmware signing for such cases to match the mod signing ?

Vendors keep telling me that they're validating firmware for new
hardware, and I keep tending not to believe them. Meh. The big problem
with firmware signatures is that we don't necessarily have the right to
distribute modified versions of the firmware, so we'd need detached
signature support. I'm certainly not against this.

> I'm trying to think what else. Possibly disabling it on Pentium-M with
> the rep movs erratum (Y19) as it's quite possible to set up suitable
> adjacent page sets in user space via the graphics.

Quirking this out when the hardware makes it impossible to provide any
guarantees seems reasonable.

-- 
Matthew Garrett 


Re: [PATCH 12/12] Add option to automatically set trusted_kernel when in Secure Boot mode

2014-02-26 Thread H. Peter Anvin
On 02/26/2014 02:41 PM, One Thousand Gnomes wrote:
> On Wed, 26 Feb 2014 15:11:13 -0500
> Matthew Garrett  wrote:
> 
>> UEFI Secure Boot provides a mechanism for ensuring that the firmware will
>> only load signed bootloaders and kernels. Certain use cases may also
>> require that the kernel prevent userspace from inserting untrusted kernel
>> code at runtime. Add a configuration option that enforces this automatically
>> when enabled.
> 
> I think you have a load more cases to attempt to paper over before you
> even pretend to achieve that goal. Firewire for example. Also it only
> remotely begins to work if you also force CAP_SYS_RAWIO off globally as
> you need to force off things like raw command issuing to various
> controllers (especially as some of that code is written on the basis that
> 'its RAWIO, screw making it secure and doing all the checks we could
> bother with'.
> 
> RAWIO also disables things like CPU msr access - which is also quite
> adequate for subverting a kernel.
> 
> Another issue that needs addressing is firmware. Quite a few of our
> request_firmware cases load device firmware which is not signed into DMA
> capable hardware. Probably also worth checking what the
> architectural guarantees on bogus microcode updates is. Maybe we need
> firmware signing for such cases to match the mod signing ?
> 
> I'm trying to think what else. Possibly disabling it on Pentium-M with
> the rep movs erratum (Y19) as it's quite possible to set up suitable
> adjacent page sets in user space via the graphics.
> 

I have been arguing for a long time that this should disable RAWIO.  The
argument was that apparently some SCSI controllers started requiring
RAWIO to do things like update firmware... which is arguably equally
problematic, but either way is (a) clearly wrong and (b) started a long,
never-ending discussion...

-hpa


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


Re: [PATCH 12/12] Add option to automatically set trusted_kernel when in Secure Boot mode

2014-02-26 Thread One Thousand Gnomes
On Wed, 26 Feb 2014 15:11:13 -0500
Matthew Garrett  wrote:

> UEFI Secure Boot provides a mechanism for ensuring that the firmware will
> only load signed bootloaders and kernels. Certain use cases may also
> require that the kernel prevent userspace from inserting untrusted kernel
> code at runtime. Add a configuration option that enforces this automatically
> when enabled.

I think you have a load more cases to attempt to paper over before you
even pretend to achieve that goal. Firewire for example. Also it only
remotely begins to work if you also force CAP_SYS_RAWIO off globally as
you need to force off things like raw command issuing to various
controllers (especially as some of that code is written on the basis that
'its RAWIO, screw making it secure and doing all the checks we could
bother with'.

RAWIO also disables things like CPU msr access - which is also quite
adequate for subverting a kernel.

Another issue that needs addressing is firmware. Quite a few of our
request_firmware cases load device firmware which is not signed into DMA
capable hardware. Probably also worth checking what the
architectural guarantees on bogus microcode updates is. Maybe we need
firmware signing for such cases to match the mod signing ?

I'm trying to think what else. Possibly disabling it on Pentium-M with
the rep movs erratum (Y19) as it's quite possible to set up suitable
adjacent page sets in user space via the graphics.

Alan
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH 12/12] Add option to automatically set trusted_kernel when in Secure Boot mode

2014-02-26 Thread Matthew Garrett
UEFI Secure Boot provides a mechanism for ensuring that the firmware will
only load signed bootloaders and kernels. Certain use cases may also
require that the kernel prevent userspace from inserting untrusted kernel
code at runtime. Add a configuration option that enforces this automatically
when enabled.

Signed-off-by: Matthew Garrett 
---
 Documentation/x86/zero-page.txt   |  2 ++
 arch/x86/Kconfig  | 13 +
 arch/x86/boot/compressed/eboot.c  | 36 +++
 arch/x86/include/uapi/asm/bootparam.h |  3 ++-
 arch/x86/kernel/setup.c   |  6 ++
 5 files changed, 59 insertions(+), 1 deletion(-)

diff --git a/Documentation/x86/zero-page.txt b/Documentation/x86/zero-page.txt
index 199f453..ec38acf 100644
--- a/Documentation/x86/zero-page.txt
+++ b/Documentation/x86/zero-page.txt
@@ -30,6 +30,8 @@ OffsetProto   NameMeaning
 1E9/001ALL eddbuf_entries  Number of entries in eddbuf (below)
 1EA/001ALL edd_mbr_sig_buf_entries Number of entries in 
edd_mbr_sig_buffer
(below)
+1EB/001ALL kbd_status  Numlock is enabled
+1EC/001ALL secure_boot Secure boot is enabled in the firmware
 1EF/001ALL sentinelUsed to detect broken bootloaders
 290/040ALL edd_mbr_sig_buffer EDD MBR signatures
 2D0/A00ALL e820_mapE820 memory map table
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 0af5250..3856dfb 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -1585,6 +1585,19 @@ config EFI_STUB
 
  See Documentation/efi-stub.txt for more information.
 
+config EFI_SECURE_BOOT_TRUSTED_KERNEL
+def_bool n
+   depends on SECURITY_TRUSTED_KERNEL
+   depends on EFI
+   prompt "Automatically set trusted_kernel with UEFI Secure Boot"
+   ---help---
+ UEFI Secure Boot provides a mechanism for ensuring that the
+ firmware will only load signed bootloaders and kernels. Certain
+ use cases may also require that the kernel restrict any userspace
+ mechanism that could insert untrusted code into the kernel.
+ Say Y here to automatically enable trusted_kernel enforcement
+ when a system boots with UEFI Secure Boot enabled.
+
 config SECCOMP
def_bool y
prompt "Enable seccomp to safely compute untrusted bytecode"
diff --git a/arch/x86/boot/compressed/eboot.c b/arch/x86/boot/compressed/eboot.c
index a7677ba..4836551 100644
--- a/arch/x86/boot/compressed/eboot.c
+++ b/arch/x86/boot/compressed/eboot.c
@@ -12,6 +12,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #undef memcpy  /* Use memcpy from misc.c */
 
@@ -421,6 +422,37 @@ void setup_graphics(struct boot_params *boot_params)
 }
 
 
+static int get_secure_boot(void)
+{
+   u8 sb, setup;
+   unsigned long datasize = sizeof(sb);
+   efi_guid_t var_guid = EFI_GLOBAL_VARIABLE_GUID;
+   efi_status_t status;
+
+   status = efi_call_phys5(sys_table->runtime->get_variable,
+   L"SecureBoot", &var_guid, NULL, &datasize, &sb);
+
+   if (status != EFI_SUCCESS)
+   return 0;
+
+   if (sb == 0)
+   return 0;
+
+
+   status = efi_call_phys5(sys_table->runtime->get_variable,
+   L"SetupMode", &var_guid, NULL, &datasize,
+   &setup);
+
+   if (status != EFI_SUCCESS)
+   return 0;
+
+   if (setup == 1)
+   return 0;
+
+   return 1;
+}
+
+
 /*
  * Because the x86 boot code expects to be passed a boot_params we
  * need to create one ourselves (usually the bootloader would create
@@ -760,6 +792,10 @@ struct boot_params *efi_main(void *handle, 
efi_system_table_t *_table,
if (sys_table->hdr.signature != EFI_SYSTEM_TABLE_SIGNATURE)
goto fail;
 
+   sanitize_boot_params(boot_params);
+
+   boot_params->secure_boot = get_secure_boot();
+
setup_graphics(boot_params);
 
setup_efi_pci(boot_params);
diff --git a/arch/x86/include/uapi/asm/bootparam.h 
b/arch/x86/include/uapi/asm/bootparam.h
index 225b098..90dbfb7 100644
--- a/arch/x86/include/uapi/asm/bootparam.h
+++ b/arch/x86/include/uapi/asm/bootparam.h
@@ -133,7 +133,8 @@ struct boot_params {
__u8  eddbuf_entries;   /* 0x1e9 */
__u8  edd_mbr_sig_buf_entries;  /* 0x1ea */
__u8  kbd_status;   /* 0x1eb */
-   __u8  _pad5[3]; /* 0x1ec */
+   __u8  secure_boot;  /* 0x1ec */
+   __u8  _pad5[2]; /* 0x1ed */
/*
 * The sentinel is set to a nonzero value (0xff) in header.S.
 *
diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index 06853e6..875258e 100644
--- a/arch/x86/kernel/setup