| Message ID | 1466069891-28577-1-git-send-email-champetier.etienne@gmail.com |
|---|---|
| State | Changes Requested |
| Headers | show |
On 16/06/2016 11:38, Etienne CHAMPETIER wrote: > This commit: > 1) seed /dev/urandom with a saved seed as early as possible > (see /lib/preinit/81_urandom_seed) > 2) save a new seed if system.@system[0].write_urandom_seed_on_boot == 1 > or if none exists. We use getrandom() so we are sure /dev/urandom > pool is initialized (see /etc/init.d/urandom_seed) > > seed size is 512 bytes (ie /proc/sys/kernel/random/poolsize / 8) > it's the same size as in ubuntu 14.04 and all systemd systems > > seed file is /etc/urandom.seed (need a writable path) > > seeding /dev/urandom doesn't change entropy estimation, so we still have > "random: ubus urandom read with 4 bits of entropy available" > messages in the logs, but we can now ignore them if > after "urandom-seed: Seeding with ..." message > > for now saving a new seed is disabled by default as Felix and John > are concerned we might write too much and destroy the flash > > v2: log preinit messages to /dev/kmsg > v3: use non generic function name for logging, as /lib/preinit/ files > are all sourced together in /etc/preinit > v4: after a lot of discussion on the ML, use a config param > > Signed-off-by: Etienne CHAMPETIER <champetier.etienne@gmail.com> > --- > package/base-files/files/bin/config_generate | 1 + > package/base-files/files/etc/init.d/urandom_seed | 28 ++++++++++++++++++++++ > .../base-files/files/lib/preinit/81_urandom_seed | 19 +++++++++++++++ > 3 files changed, 48 insertions(+) > create mode 100755 package/base-files/files/etc/init.d/urandom_seed > create mode 100644 package/base-files/files/lib/preinit/81_urandom_seed > > diff --git a/package/base-files/files/bin/config_generate b/package/base-files/files/bin/config_generate > index 8002bc4..9bccead 100755 > --- a/package/base-files/files/bin/config_generate > +++ b/package/base-files/files/bin/config_generate > @@ -230,6 +230,7 @@ generate_static_system() { > set system.@system[-1].timezone='UTC' > set system.@system[-1].ttylogin='0' > set system.@system[-1].log_size='64' > + set system.@system[-1].write_urandom_seed_on_boot='0' > > delete system.ntp > set system.ntp='timeserver' > diff --git a/package/base-files/files/etc/init.d/urandom_seed b/package/base-files/files/etc/init.d/urandom_seed > new file mode 100755 > index 0000000..f950685 > --- /dev/null > +++ b/package/base-files/files/etc/init.d/urandom_seed > @@ -0,0 +1,28 @@ > +#!/bin/sh /etc/rc.common > + > +START=99 > + > +EXTRA_COMMANDS="save" > + > +SEED=/etc/urandom.seed > + > +error_exit() { > + logger -t urandom_seed "$1" > + exit 1 > +} > + > +save() { > + touch $SEED.tmp || error_exit "touch failed" > + chown root:root $SEED.tmp || error_exit "chown failed" > + chmod 600 $SEED.tmp || error_exit "chmod failed" > + getrandom 512 > $SEED.tmp || error_exit "getrandom failed" > + mv $SEED.tmp $SEED || error_exit "mv failed" > +} > + > +boot() { > + [ -f $SEED ] || { > + save > + exit 0 > + } > + [ "$(uci get system.@system[0].write_urandom_seed_on_boot)" == "1" ] && save > +} write_urandom_seed_on_boot was a placeholder for what the option should be named as i could not think of a good one ;) please try to find a shorter one John > diff --git a/package/base-files/files/lib/preinit/81_urandom_seed b/package/base-files/files/lib/preinit/81_urandom_seed > new file mode 100644 > index 0000000..a1457aa > --- /dev/null > +++ b/package/base-files/files/lib/preinit/81_urandom_seed > @@ -0,0 +1,19 @@ > +#!/bin/sh > + > +log_urandom_seed() { > + echo "urandom-seed: $1" > /dev/kmsg > +} > + > +do_urandom_seed() { > + S=/etc/urandom.seed > + U=/dev/urandom > + > + [ -c $U ] || { log_urandom_seed "Something is wrong with $U"; return; } > + [ -f $S ] || { log_urandom_seed "Seed file not found ($S)"; return; } > + [ -O $S -a -G $S -a ! -x $S ] || { log_urandom_seed "Wrong owner / permissions for $S"; return; } > + > + log_urandom_seed "Seeding with $S" > + cat $S > $U > +} > + > +boot_hook_add preinit_main do_urandom_seed >
On 22/06/16 08:50, John Crispin wrote: > <snip> > write_urandom_seed_on_boot was a placeholder for what the option should > be named as i could not think of a good one ;) please try to find a > shorter one > > John May I offer "preserveurandomseed" or 'saveurandom' as some 'random' ideas :-) Kevin
Hi all, 2016-06-22 10:12 GMT+02:00 Kevin Darbyshire-Bryant <kevin@darbyshire-bryant.me.uk>: > > > On 22/06/16 08:50, John Crispin wrote: >> >> > <snip> >> >> write_urandom_seed_on_boot was a placeholder for what the option should >> be named as i could not think of a good one ;) please try to find a >> shorter one >> >> John > > May I offer "preserveurandomseed" or 'saveurandom' as some 'random' ideas > :-) is 'saveurandomseed' short enough ? else what about 'urandomseed' ? (my first version was 'write_new_urandom_seed_on_boot' ;) ) > > Kevin > >
Hi, if we plan to attach multiple functions to the option (e.g. persistent lease file) then it would probably make sense to give it a generic name, e.g. "option persist 0/1" or "option save_state 0/1" ~ Jo
2016-06-23 16:08 GMT+02:00 Jo-Philipp Wich <jo@mein.io>: > Hi, > > if we plan to attach multiple functions to the option (e.g. persistent > lease file) then it would probably make sense to give it a generic name, > e.g. > > "option persist 0/1" > > or > > "option save_state 0/1" why put multiple possibly unrelated functions under one option? I might want to save urandom seed but not persistant lease file > > ~ Jo > Etienne
Hi.
> why put multiple possibly unrelated functions under one option?
To have a global "write to flash is okay" option instead of needing to
change different options with different semantics in different places.
For example if I want to make the lease file persistent, I would change
the path in the config from /tmp/ to /etc/. But yes, no need to group stuff.
What about following the dnsmasq approach though?
Setting "option randomseed /etc/random.seed" would make it persistent,
the default would be empty or something in /var.
~ Jo
On Thu, 2016-06-16 at 09:38 +0000, Etienne CHAMPETIER wrote: > +SEED=/etc/urandom.seed > + > +error_exit() { > + logger -t urandom_seed "$1" > + exit 1 > +} > + > +save() { > + touch $SEED.tmp || error_exit "touch failed" > + chown root:root $SEED.tmp || error_exit "chown failed" > + chmod 600 $SEED.tmp || error_exit "chmod failed" > + getrandom 512 > $SEED.tmp || error_exit "getrandom failed" > + mv $SEED.tmp $SEED || error_exit "mv failed" > +} The .tmp file should go in /tmp, and maybe SEED would be better off in /etc/config, so that it gets picked up by sysupgrade?
Hi Daniel, 2016-06-23 17:19 GMT+02:00 Daniel Gimpelevich <daniel@gimpelevich.san-francisco.ca.us>: > On Thu, 2016-06-16 at 09:38 +0000, Etienne CHAMPETIER wrote: >> +SEED=/etc/urandom.seed >> + >> +error_exit() { >> + logger -t urandom_seed "$1" >> + exit 1 >> +} >> + >> +save() { >> + touch $SEED.tmp || error_exit "touch failed" >> + chown root:root $SEED.tmp || error_exit "chown failed" >> + chmod 600 $SEED.tmp || error_exit "chmod failed" >> + getrandom 512 > $SEED.tmp || error_exit "getrandom failed" >> + mv $SEED.tmp $SEED || error_exit "mv failed" >> +} > > The .tmp file should go in /tmp, no, else you loose the atomic part of the 'mv' command > and maybe SEED would be better off > in /etc/config, so that it gets picked up by sysupgrade? and no because if you restore the same config on multiple router you end up with the same seed, which is exactly what this patch is about (not having the same state) > >
2016-06-23 17:05 GMT+02:00 Jo-Philipp Wich <jo@mein.io>: > Hi. > >> why put multiple possibly unrelated functions under one option? > > To have a global "write to flash is okay" option instead of needing to > change different options with different semantics in different places. if it's disabled by default i don't see the need for a global "write to flash is okay". Also do we have that much different options? > > For example if I want to make the lease file persistent, I would change > the path in the config from /tmp/ to /etc/. But yes, no need to group stuff. if we have a global and a specific config, how do you mix them up (0, 1, undef)? > > What about following the dnsmasq approach though? > Setting "option randomseed /etc/random.seed" would make it persistent, > the default would be empty or something in /var. no need to write it if we lose it on reboot ... > > ~ Jo >
diff --git a/package/base-files/files/bin/config_generate b/package/base-files/files/bin/config_generate index 8002bc4..9bccead 100755 --- a/package/base-files/files/bin/config_generate +++ b/package/base-files/files/bin/config_generate @@ -230,6 +230,7 @@ generate_static_system() { set system.@system[-1].timezone='UTC' set system.@system[-1].ttylogin='0' set system.@system[-1].log_size='64' + set system.@system[-1].write_urandom_seed_on_boot='0' delete system.ntp set system.ntp='timeserver' diff --git a/package/base-files/files/etc/init.d/urandom_seed b/package/base-files/files/etc/init.d/urandom_seed new file mode 100755 index 0000000..f950685 --- /dev/null +++ b/package/base-files/files/etc/init.d/urandom_seed @@ -0,0 +1,28 @@ +#!/bin/sh /etc/rc.common + +START=99 + +EXTRA_COMMANDS="save" + +SEED=/etc/urandom.seed + +error_exit() { + logger -t urandom_seed "$1" + exit 1 +} + +save() { + touch $SEED.tmp || error_exit "touch failed" + chown root:root $SEED.tmp || error_exit "chown failed" + chmod 600 $SEED.tmp || error_exit "chmod failed" + getrandom 512 > $SEED.tmp || error_exit "getrandom failed" + mv $SEED.tmp $SEED || error_exit "mv failed" +} + +boot() { + [ -f $SEED ] || { + save + exit 0 + } + [ "$(uci get system.@system[0].write_urandom_seed_on_boot)" == "1" ] && save +} diff --git a/package/base-files/files/lib/preinit/81_urandom_seed b/package/base-files/files/lib/preinit/81_urandom_seed new file mode 100644 index 0000000..a1457aa --- /dev/null +++ b/package/base-files/files/lib/preinit/81_urandom_seed @@ -0,0 +1,19 @@ +#!/bin/sh + +log_urandom_seed() { + echo "urandom-seed: $1" > /dev/kmsg +} + +do_urandom_seed() { + S=/etc/urandom.seed + U=/dev/urandom + + [ -c $U ] || { log_urandom_seed "Something is wrong with $U"; return; } + [ -f $S ] || { log_urandom_seed "Seed file not found ($S)"; return; } + [ -O $S -a -G $S -a ! -x $S ] || { log_urandom_seed "Wrong owner / permissions for $S"; return; } + + log_urandom_seed "Seeding with $S" + cat $S > $U +} + +boot_hook_add preinit_main do_urandom_seed
This commit: 1) seed /dev/urandom with a saved seed as early as possible (see /lib/preinit/81_urandom_seed) 2) save a new seed if system.@system[0].write_urandom_seed_on_boot == 1 or if none exists. We use getrandom() so we are sure /dev/urandom pool is initialized (see /etc/init.d/urandom_seed) seed size is 512 bytes (ie /proc/sys/kernel/random/poolsize / 8) it's the same size as in ubuntu 14.04 and all systemd systems seed file is /etc/urandom.seed (need a writable path) seeding /dev/urandom doesn't change entropy estimation, so we still have "random: ubus urandom read with 4 bits of entropy available" messages in the logs, but we can now ignore them if after "urandom-seed: Seeding with ..." message for now saving a new seed is disabled by default as Felix and John are concerned we might write too much and destroy the flash v2: log preinit messages to /dev/kmsg v3: use non generic function name for logging, as /lib/preinit/ files are all sourced together in /etc/preinit v4: after a lot of discussion on the ML, use a config param Signed-off-by: Etienne CHAMPETIER <champetier.etienne@gmail.com> --- package/base-files/files/bin/config_generate | 1 + package/base-files/files/etc/init.d/urandom_seed | 28 ++++++++++++++++++++++ .../base-files/files/lib/preinit/81_urandom_seed | 19 +++++++++++++++ 3 files changed, 48 insertions(+) create mode 100755 package/base-files/files/etc/init.d/urandom_seed create mode 100644 package/base-files/files/lib/preinit/81_urandom_seed