| Message ID | 20170816224650.1089-3-labbott@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Aug 16, 2017 at 3:46 PM, Laura Abbott <labbott@redhat.com> wrote: > From: Daniel Micay <danielmicay@gmail.com> > > 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 <danielmicay@gmail.com> > [labbott: Line-wrapped command line] > Signed-off-by: Laura Abbott <labbott@redhat.com> > --- > 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 >
On 08/16/2017 03:48 PM, Kees Cook wrote: > On Wed, Aug 16, 2017 at 3:46 PM, Laura Abbott <labbott@redhat.com> wrote: >> From: Daniel Micay <danielmicay@gmail.com> >> >> 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 <danielmicay@gmail.com> >> [labbott: Line-wrapped command line] >> Signed-off-by: Laura Abbott <labbott@redhat.com> >> --- >> 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 >> > > >
On Wed, Aug 16, 2017 at 3:46 PM, Laura Abbott <labbott@redhat.com> wrote: > From: Daniel Micay <danielmicay@gmail.com> > > 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
On Wed, Aug 16, 2017 at 9:56 PM, Nick Kralevich <nnk@google.com> wrote: > On Wed, Aug 16, 2017 at 3:46 PM, Laura Abbott <labbott@redhat.com> wrote: >> From: Daniel Micay <danielmicay@gmail.com> >> >> 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
On Wed, 2017-08-16 at 21:58 -0700, Kees Cook wrote: > On Wed, Aug 16, 2017 at 9:56 PM, Nick Kralevich <nnk@google.com> > wrote: > > On Wed, Aug 16, 2017 at 3:46 PM, Laura Abbott <labbott@redhat.com> > > wrote: > > > From: Daniel Micay <danielmicay@gmail.com> > > > > > > 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.
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();