2017-08-16 22:46:58

by Laura Abbott

[permalink] [raw]
Subject: [PATCHv2 0/2] Command line randomness

Hi,

This is a series to add the kernel command line as a source of randomness.
The first patch is an (old) prepatory patch from me to move the stack canary
initialization later. The second patch is from Daniel Micay to actually
add the command line to the pool.

Kees suggested this go through -mm.

Thanks,
Laura

Daniel Micay (1):
extract early boot entropy from the passed cmdline

Laura Abbott (1):
init: Move stack canary initialization after setup_arch

init/main.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)

--
2.13.0


2017-08-16 22:47:02

by Laura Abbott

[permalink] [raw]
Subject: [PATCHv2 1/2] init: Move stack canary initialization after setup_arch

From: Laura Abbott <[email protected]>

Stack canary intialization involves getting a random number.
Getting this random number may involve accessing caches or other
architectural specific features which are not available until
after the architecture is setup. Move the stack canary initialization
later to accomodate this.

Signed-off-by: Laura Abbott <[email protected]>
Signed-off-by: Laura Abbott <[email protected]>
---
v2: Also moved add_latent_entropy per suggestion of Kees.
---
init/main.c | 11 +++++------
1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/init/main.c b/init/main.c
index 052481fbe363..21d599eaad06 100644
--- a/init/main.c
+++ b/init/main.c
@@ -515,12 +515,6 @@ asmlinkage __visible void __init start_kernel(void)
smp_setup_processor_id();
debug_objects_early_init();

- /*
- * Set up the initial canary ASAP:
- */
- add_latent_entropy();
- boot_init_stack_canary();
-
cgroup_init_early();

local_irq_disable();
@@ -534,6 +528,11 @@ asmlinkage __visible void __init start_kernel(void)
page_address_init();
pr_notice("%s", linux_banner);
setup_arch(&command_line);
+ /*
+ * Set up the the initial canary and entropy after arch
+ */
+ add_latent_entropy();
+ boot_init_stack_canary();
mm_init_cpumask(&init_mm);
setup_command_line(command_line);
setup_nr_cpu_ids();
--
2.13.0

2017-08-16 22:47:13

by Laura Abbott

[permalink] [raw]
Subject: [PATCHv2 2/2] extract early boot entropy from the passed cmdline

From: Daniel Micay <[email protected]>

Existing Android bootloaders usually pass data useful as early entropy
on the kernel command-line. It may also be the case on other embedded
systems. Sample command-line from a Google Pixel running CopperheadOS:

console=ttyHSL0,115200,n8 androidboot.console=ttyHSL0
androidboot.hardware=sailfish user_debug=31 ehci-hcd.park=3
lpm_levels.sleep_disabled=1 cma=32M@0-0xffffffff buildvariant=user
veritykeyid=id:dfcb9db0089e5b3b4090a592415c28e1cb4545ab
androidboot.bootdevice=624000.ufshc androidboot.verifiedbootstate=yellow
androidboot.veritymode=enforcing androidboot.keymaster=1
androidboot.serialno=FA6CE0305299 androidboot.baseband=msm
mdss_mdp.panel=1:dsi:0:qcom,mdss_dsi_samsung_ea8064tg_1080p_cmd:1:none:cfg:single_dsi
androidboot.slot_suffix=_b fpsimd.fpsimd_settings=0
app_setting.use_app_setting=0 kernelflag=0x00000000 debugflag=0x00000000
androidboot.hardware.revision=PVT radioflag=0x00000000
radioflagex1=0x00000000 radioflagex2=0x00000000 cpumask=0x00000000
androidboot.hardware.ddr=4096MB,Hynix,LPDDR4 androidboot.ddrinfo=00000006
androidboot.ddrsize=4GB androidboot.hardware.color=GRA00
androidboot.hardware.ufs=32GB,Samsung androidboot.msm.hw_ver_id=268824801
androidboot.qf.st=2 androidboot.cid=11111111 androidboot.mid=G-2PW4100
androidboot.bootloader=8996-012001-1704121145
androidboot.oem_unlock_support=1 androidboot.fp_src=1
androidboot.htc.hrdump=detected androidboot.ramdump.opt=mem@2g:2g,mem@4g:2g
androidboot.bootreason=reboot androidboot.ramdump_enable=0 ro
root=/dev/dm-0 dm="system none ro,0 1 android-verity /dev/sda34"
rootwait skip_initramfs init=/init androidboot.wificountrycode=US
androidboot.boottime=1BLL:85,1BLE:669,2BLL:0,2BLE:1777,SW:6,KL:8136

Among other things, it contains a value unique to the device
(androidboot.serialno=FA6CE0305299), unique to the OS builds for the
device variant (veritykeyid=id:dfcb9db0089e5b3b4090a592415c28e1cb4545ab)
and timings from the bootloader stages in milliseconds
(androidboot.boottime=1BLL:85,1BLE:669,2BLL:0,2BLE:1777,SW:6,KL:8136).

Signed-off-by: Daniel Micay <[email protected]>
[labbott: Line-wrapped command line]
Signed-off-by: Laura Abbott <[email protected]>
---
init/main.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/init/main.c b/init/main.c
index 21d599eaad06..cb051aec46ef 100644
--- a/init/main.c
+++ b/init/main.c
@@ -533,6 +533,7 @@ asmlinkage __visible void __init start_kernel(void)
*/
add_latent_entropy();
boot_init_stack_canary();
+ add_device_randomness(command_line, strlen(command_line));
mm_init_cpumask(&init_mm);
setup_command_line(command_line);
setup_nr_cpu_ids();
--
2.13.0

2017-08-16 22:48:49

by Kees Cook

[permalink] [raw]
Subject: Re: [PATCHv2 2/2] extract early boot entropy from the passed cmdline

On Wed, Aug 16, 2017 at 3:46 PM, Laura Abbott <[email protected]> wrote:
> From: Daniel Micay <[email protected]>
>
> Existing Android bootloaders usually pass data useful as early entropy
> on the kernel command-line. It may also be the case on other embedded
> systems. Sample command-line from a Google Pixel running CopperheadOS:
>
> console=ttyHSL0,115200,n8 androidboot.console=ttyHSL0
> androidboot.hardware=sailfish user_debug=31 ehci-hcd.park=3
> lpm_levels.sleep_disabled=1 cma=32M@0-0xffffffff buildvariant=user
> veritykeyid=id:dfcb9db0089e5b3b4090a592415c28e1cb4545ab
> androidboot.bootdevice=624000.ufshc androidboot.verifiedbootstate=yellow
> androidboot.veritymode=enforcing androidboot.keymaster=1
> androidboot.serialno=FA6CE0305299 androidboot.baseband=msm
> mdss_mdp.panel=1:dsi:0:qcom,mdss_dsi_samsung_ea8064tg_1080p_cmd:1:none:cfg:single_dsi
> androidboot.slot_suffix=_b fpsimd.fpsimd_settings=0
> app_setting.use_app_setting=0 kernelflag=0x00000000 debugflag=0x00000000
> androidboot.hardware.revision=PVT radioflag=0x00000000
> radioflagex1=0x00000000 radioflagex2=0x00000000 cpumask=0x00000000
> androidboot.hardware.ddr=4096MB,Hynix,LPDDR4 androidboot.ddrinfo=00000006
> androidboot.ddrsize=4GB androidboot.hardware.color=GRA00
> androidboot.hardware.ufs=32GB,Samsung androidboot.msm.hw_ver_id=268824801
> androidboot.qf.st=2 androidboot.cid=11111111 androidboot.mid=G-2PW4100
> androidboot.bootloader=8996-012001-1704121145
> androidboot.oem_unlock_support=1 androidboot.fp_src=1
> androidboot.htc.hrdump=detected androidboot.ramdump.opt=mem@2g:2g,mem@4g:2g
> androidboot.bootreason=reboot androidboot.ramdump_enable=0 ro
> root=/dev/dm-0 dm="system none ro,0 1 android-verity /dev/sda34"
> rootwait skip_initramfs init=/init androidboot.wificountrycode=US
> androidboot.boottime=1BLL:85,1BLE:669,2BLL:0,2BLE:1777,SW:6,KL:8136
>
> Among other things, it contains a value unique to the device
> (androidboot.serialno=FA6CE0305299), unique to the OS builds for the
> device variant (veritykeyid=id:dfcb9db0089e5b3b4090a592415c28e1cb4545ab)
> and timings from the bootloader stages in milliseconds
> (androidboot.boottime=1BLL:85,1BLE:669,2BLL:0,2BLE:1777,SW:6,KL:8136).
>
> Signed-off-by: Daniel Micay <[email protected]>
> [labbott: Line-wrapped command line]
> Signed-off-by: Laura Abbott <[email protected]>
> ---
> init/main.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/init/main.c b/init/main.c
> index 21d599eaad06..cb051aec46ef 100644
> --- a/init/main.c
> +++ b/init/main.c
> @@ -533,6 +533,7 @@ asmlinkage __visible void __init start_kernel(void)
> */
> add_latent_entropy();
> boot_init_stack_canary();
> + add_device_randomness(command_line, strlen(command_line));

This should be above the add_latent_entropy() line there so the
cmdline entropy can contribute to the canary entropy.

-Kees

> mm_init_cpumask(&init_mm);
> setup_command_line(command_line);
> setup_nr_cpu_ids();
> --
> 2.13.0
>



--
Kees Cook
Pixel Security

2017-08-16 22:50:26

by Kees Cook

[permalink] [raw]
Subject: Re: [PATCHv2 1/2] init: Move stack canary initialization after setup_arch

On Wed, Aug 16, 2017 at 3:46 PM, Laura Abbott <[email protected]> wrote:
> From: Laura Abbott <[email protected]>
>
> Stack canary intialization involves getting a random number.
> Getting this random number may involve accessing caches or other
> architectural specific features which are not available until
> after the architecture is setup. Move the stack canary initialization
> later to accomodate this.
>
> Signed-off-by: Laura Abbott <[email protected]>
> Signed-off-by: Laura Abbott <[email protected]>

Acked-by: Kees Cook <[email protected]>

-Kees

> ---
> v2: Also moved add_latent_entropy per suggestion of Kees.
> ---
> init/main.c | 11 +++++------
> 1 file changed, 5 insertions(+), 6 deletions(-)
>
> diff --git a/init/main.c b/init/main.c
> index 052481fbe363..21d599eaad06 100644
> --- a/init/main.c
> +++ b/init/main.c
> @@ -515,12 +515,6 @@ asmlinkage __visible void __init start_kernel(void)
> smp_setup_processor_id();
> debug_objects_early_init();
>
> - /*
> - * Set up the initial canary ASAP:
> - */
> - add_latent_entropy();
> - boot_init_stack_canary();
> -
> cgroup_init_early();
>
> local_irq_disable();
> @@ -534,6 +528,11 @@ asmlinkage __visible void __init start_kernel(void)
> page_address_init();
> pr_notice("%s", linux_banner);
> setup_arch(&command_line);
> + /*
> + * Set up the the initial canary and entropy after arch
> + */
> + add_latent_entropy();
> + boot_init_stack_canary();
> mm_init_cpumask(&init_mm);
> setup_command_line(command_line);
> setup_nr_cpu_ids();
> --
> 2.13.0
>



--
Kees Cook
Pixel Security

2017-08-16 22:53:29

by Laura Abbott

[permalink] [raw]
Subject: Re: [PATCHv2 2/2] extract early boot entropy from the passed cmdline

On 08/16/2017 03:48 PM, Kees Cook wrote:
> On Wed, Aug 16, 2017 at 3:46 PM, Laura Abbott <[email protected]> wrote:
>> From: Daniel Micay <[email protected]>
>>
>> Existing Android bootloaders usually pass data useful as early entropy
>> on the kernel command-line. It may also be the case on other embedded
>> systems. Sample command-line from a Google Pixel running CopperheadOS:
>>
>> console=ttyHSL0,115200,n8 androidboot.console=ttyHSL0
>> androidboot.hardware=sailfish user_debug=31 ehci-hcd.park=3
>> lpm_levels.sleep_disabled=1 cma=32M@0-0xffffffff buildvariant=user
>> veritykeyid=id:dfcb9db0089e5b3b4090a592415c28e1cb4545ab
>> androidboot.bootdevice=624000.ufshc androidboot.verifiedbootstate=yellow
>> androidboot.veritymode=enforcing androidboot.keymaster=1
>> androidboot.serialno=FA6CE0305299 androidboot.baseband=msm
>> mdss_mdp.panel=1:dsi:0:qcom,mdss_dsi_samsung_ea8064tg_1080p_cmd:1:none:cfg:single_dsi
>> androidboot.slot_suffix=_b fpsimd.fpsimd_settings=0
>> app_setting.use_app_setting=0 kernelflag=0x00000000 debugflag=0x00000000
>> androidboot.hardware.revision=PVT radioflag=0x00000000
>> radioflagex1=0x00000000 radioflagex2=0x00000000 cpumask=0x00000000
>> androidboot.hardware.ddr=4096MB,Hynix,LPDDR4 androidboot.ddrinfo=00000006
>> androidboot.ddrsize=4GB androidboot.hardware.color=GRA00
>> androidboot.hardware.ufs=32GB,Samsung androidboot.msm.hw_ver_id=268824801
>> androidboot.qf.st=2 androidboot.cid=11111111 androidboot.mid=G-2PW4100
>> androidboot.bootloader=8996-012001-1704121145
>> androidboot.oem_unlock_support=1 androidboot.fp_src=1
>> androidboot.htc.hrdump=detected androidboot.ramdump.opt=mem@2g:2g,mem@4g:2g
>> androidboot.bootreason=reboot androidboot.ramdump_enable=0 ro
>> root=/dev/dm-0 dm="system none ro,0 1 android-verity /dev/sda34"
>> rootwait skip_initramfs init=/init androidboot.wificountrycode=US
>> androidboot.boottime=1BLL:85,1BLE:669,2BLL:0,2BLE:1777,SW:6,KL:8136
>>
>> Among other things, it contains a value unique to the device
>> (androidboot.serialno=FA6CE0305299), unique to the OS builds for the
>> device variant (veritykeyid=id:dfcb9db0089e5b3b4090a592415c28e1cb4545ab)
>> and timings from the bootloader stages in milliseconds
>> (androidboot.boottime=1BLL:85,1BLE:669,2BLL:0,2BLE:1777,SW:6,KL:8136).
>>
>> Signed-off-by: Daniel Micay <[email protected]>
>> [labbott: Line-wrapped command line]
>> Signed-off-by: Laura Abbott <[email protected]>
>> ---
>> init/main.c | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/init/main.c b/init/main.c
>> index 21d599eaad06..cb051aec46ef 100644
>> --- a/init/main.c
>> +++ b/init/main.c
>> @@ -533,6 +533,7 @@ asmlinkage __visible void __init start_kernel(void)
>> */
>> add_latent_entropy();
>> boot_init_stack_canary();
>> + add_device_randomness(command_line, strlen(command_line));
>
> This should be above the add_latent_entropy() line there so the
> cmdline entropy can contribute to the canary entropy.
>

Yeah, bad merge on my fault. Will spin a v3.

> -Kees
>
>> mm_init_cpumask(&init_mm);
>> setup_command_line(command_line);
>> setup_nr_cpu_ids();
>> --
>> 2.13.0
>>
>
>
>

2017-08-17 04:56:19

by Nick Kralevich

[permalink] [raw]
Subject: Re: [kernel-hardening] [PATCHv2 2/2] extract early boot entropy from the passed cmdline

On Wed, Aug 16, 2017 at 3:46 PM, Laura Abbott <[email protected]> wrote:
> From: Daniel Micay <[email protected]>
>
> Existing Android bootloaders usually pass data useful as early entropy
> on the kernel command-line. It may also be the case on other embedded
> systems. Sample command-line from a Google Pixel running CopperheadOS:
>

Why is it better to put this into the kernel, rather than just rely on
the existing userspace functionality which does exactly the same
thing? This is what Android already does today:
https://android-review.googlesource.com/198113

-- Nick

2017-08-17 04:58:29

by Kees Cook

[permalink] [raw]
Subject: Re: [kernel-hardening] [PATCHv2 2/2] extract early boot entropy from the passed cmdline

On Wed, Aug 16, 2017 at 9:56 PM, Nick Kralevich <[email protected]> wrote:
> On Wed, Aug 16, 2017 at 3:46 PM, Laura Abbott <[email protected]> wrote:
>> From: Daniel Micay <[email protected]>
>>
>> Existing Android bootloaders usually pass data useful as early entropy
>> on the kernel command-line. It may also be the case on other embedded
>> systems. Sample command-line from a Google Pixel running CopperheadOS:
>>
>
> Why is it better to put this into the kernel, rather than just rely on
> the existing userspace functionality which does exactly the same
> thing? This is what Android already does today:
> https://android-review.googlesource.com/198113

That's too late for setting up the kernel stack canary, among other
things. The kernel will also be generating some early secrets for slab
cache canaries, etc. That all needs to happen well before init is
started.

-Kees

--
Kees Cook
Pixel Security

2017-08-17 05:10:14

by Daniel Micay

[permalink] [raw]
Subject: Re: [kernel-hardening] [PATCHv2 2/2] extract early boot entropy from the passed cmdline

On Wed, 2017-08-16 at 21:58 -0700, Kees Cook wrote:
> On Wed, Aug 16, 2017 at 9:56 PM, Nick Kralevich <[email protected]>
> wrote:
> > On Wed, Aug 16, 2017 at 3:46 PM, Laura Abbott <[email protected]>
> > wrote:
> > > From: Daniel Micay <[email protected]>
> > >
> > > Existing Android bootloaders usually pass data useful as early
> > > entropy
> > > on the kernel command-line. It may also be the case on other
> > > embedded
> > > systems. Sample command-line from a Google Pixel running
> > > CopperheadOS:
> > >
> >
> > Why is it better to put this into the kernel, rather than just rely
> > on
> > the existing userspace functionality which does exactly the same
> > thing? This is what Android already does today:
> > https://android-review.googlesource.com/198113
>
> That's too late for setting up the kernel stack canary, among other
> things. The kernel will also be generating some early secrets for slab
> cache canaries, etc. That all needs to happen well before init is
> started.
>
> -Kees
>

It's also unfortunately the kernel's global stack canary for the entire
boot since unlike x86 there aren't per-task canaries. GCC / Clang access
it via a segment register on x86 vs. a global on other architectures.

In theory it could be task-local elsewhere but doing it efficiently
would imply reserving a register to store the random value. I think that
may actually end up helping performance more than it hurts by not
needing to read the global stack canary value from cache repeatedly. If
stack canaries were augmented into something more (XOR in the retaddr
and offer the option of more coverage than STRONG) it would be more
important.