[LEDE-DEV] [PATCH v5] base-files: seed /dev/urandom
Etienne Champetier
champetier.etienne at gmail.com
Tue Jul 19 08:45:39 PDT 2016
2016-07-19 2:44 GMT+02:00 Daniel Golle <daniel at makrotopia.org>:
> Hi Etienne,
> Hi Arjen,
> Hi John,
> Hi Jo,
Hi Daniel
>
> while wondering which process is blocking my rootfs from being
> unmounted for sysupgrade I discovered that getrandom story going
> on and while waiting for blocking I/O read from /dev/urandom, it
> even ignored any TERM or KILL signals.
In my test (x86_64 vm), all INT TERM KILL are enough to kill getrandom
(i've also checked with strace)
Can you verify that "killall getrandom" is enough on your device
> This then prevented the rootfs from unmounting and thus the ubi
> device from detaching. The sysupgrade script, however, seems not to
> care about that and just started ubiformat anyway and managed to
> format and flash most of the NAND -- and by doing to, filling up the
> random pool, which then resulted in /etc/urandom.seed being created
> and ruining what was just written. If it wasn't for the serial
> console being already hooked up, the device would have been bricked.
> Please be more careful when making such fundamental changes such
> as this one. Btw, it also prevents reboot until random seed is
> written.
The init process is blocked waiting for getrandom which break many
things (including halt/reboot)
I'm sending a patch to use procd so it doesn't block anymore
> Please fix getrandom to be more graceful and use non-blocking reads
> with (delayed) retries or something like that.
>
> Cheers
>
> Daniel
>
>
> On Tue, Jun 28, 2016 at 12:55:57PM +0200, Etienne Champetier wrote:
>> Hi Arjen,
>>
>> 2016-06-28 11:52 GMT+02:00 Arjen de Korte <arjen+lede at de-korte.org>:
>> > Citeren John Crispin <john at phrozen.org>:
>> >
>> >> On 28/06/2016 10:28, Jo-Philipp Wich wrote:
>> >>>
>> >>> Hi Etienne,
>> >>>
>> >>> I like this approach, fine with me now.
>> >>>
>> >>
>> >> same here, we could not also consider adding a uci-defaults script that
>> >> check if rootfs is on a mtd or real lbock device and change the default
>> >> during firstboot, but i guess that would be a new patch. i have just
>> >> pulled this into my staging tree
>> >
>> >
>> > Would it be useful to list /etc/urandom.seed in
>> > package/base-files/files/lib/upgrade/keep.d/base-files-essential to keep on
>> > sysupgrade? Or would this break if the file does not exist? In that case, it
>> > might be useful to make a note to add it to /etc/sysupgrade.conf if the
>> > contents should be kept on upgrades.
>> >
>>
>> We can keep /etc/urandom.seed on upgrade, be we shouldn't include it
>> in config backup
>>
>> >
>> >>> On 06/27/2016 05:53 PM, Etienne CHAMPETIER wrote:
>> >>>>
>> >>>> This commit:
>> >>>> 1) seed /dev/urandom with the saved seeds as early as possible
>> >>>> (see /lib/preinit/81_urandom_seed)
>> >>>> 2) save a seed at /etc/urandom.seed if it doesn't exists
>> >>>> 3) save a new seed each boot at "system. at system[0].urandom_seed"
>> >>>> (see /etc/init.d/urandom_seed)
>> >>>>
>> >>>> We use getrandom() so we are sure /dev/urandom pool is initialized
>> >>>>
>> >>>> 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
>> >>>>
>> >>>> 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
>> >>>>
>> >>>> Saving a new seed on each boot is disabled by default to avoid too much
>> >>>> writes without user consent
>> >>>>
>> >>>> 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 uci config param
>> >>>> v5: config param is now the path of the seed
>> >>>>
>> >>>> Signed-off-by: Etienne CHAMPETIER <champetier.etienne at gmail.com>
>> >>>
>> >>> Acked-by: Jo-Philipp Wich <jo at mein.io>
>> >>>>
>> >>>> ---
>> >>>> package/base-files/files/bin/config_generate | 1 +
>> >>>> package/base-files/files/etc/init.d/urandom_seed | 29
>> >>>> ++++++++++++++++++++++
>> >>>> .../base-files/files/lib/preinit/81_urandom_seed | 24
>> >>>> ++++++++++++++++++
>> >>>> 3 files changed, 54 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..c0ba0fb 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. at system[-1].timezone='UTC'
>> >>>> set system. at system[-1].ttylogin='0'
>> >>>> set system. at system[-1].log_size='64'
>> >>>> + set system. at system[-1].urandom_seed='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..cb2eb44
>> >>>> --- /dev/null
>> >>>> +++ b/package/base-files/files/etc/init.d/urandom_seed
>> >>>> @@ -0,0 +1,29 @@
>> >>>> +#!/bin/sh /etc/rc.common
>> >>>> +
>> >>>> +START=99
>> >>>> +
>> >>>> +EXTRA_COMMANDS="save"
>> >>>> +
>> >>>> +_log() {
>> >>>> + logger -t urandom_seed "$1"
>> >>>> +}
>> >>>> +
>> >>>> +_save() {
>> >>>> + touch $1.tmp || { _log "touch $1 failed"; return; }
>> >>>> + chown root:root $1.tmp || { _log "chown $1 failed"; return; }
>> >>>> + chmod 600 $1.tmp || { _log "chmod $1 failed"; return; }
>> >>>> + getrandom 512 > $1.tmp || { _log "getrandom failed"; return; }
>> >>>> + mv $1.tmp $1 || { _log "mv $1 failed"; return; }
>> >>>> +}
>> >>>> +
>> >>>> +save() {
>> >>>> + SEED="$(uci -q get system. at system[0].urandom_seed)"
>> >>>> + [ "${SEED:0:1}" == "/" ] && _save "$SEED" && _log "Seed saved
>> >>>> ($SEED)"
>> >>>> +
>> >>>> + SEED=/etc/urandom.seed
>> >>>> + [ ! -f $SEED ] && _save "$SEED" && _log "Seed saved ($SEED)"
>> >>>> +}
>> >>>> +
>> >>>> +boot() {
>> >>>> + 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..10878f3
>> >>>> --- /dev/null
>> >>>> +++ b/package/base-files/files/lib/preinit/81_urandom_seed
>> >>>> @@ -0,0 +1,24 @@
>> >>>> +#!/bin/sh
>> >>>> +
>> >>>> +log_urandom_seed() {
>> >>>> + echo "urandom-seed: $1" > /dev/kmsg
>> >>>> +}
>> >>>> +
>> >>>> +_do_urandom_seed() {
>> >>>> + [ -f "$1" ] || { log_urandom_seed "Seed file not found ($1)";
>> >>>> return; }
>> >>>> + [ -O "$1" -a -G "$1" -a ! -x "$1" ] || { log_urandom_seed "Wrong
>> >>>> owner / permissions for $1"; return; }
>> >>>> +
>> >>>> + log_urandom_seed "Seeding with $1"
>> >>>> + cat "$1" > /dev/urandom
>> >>>> +}
>> >>>> +
>> >>>> +do_urandom_seed() {
>> >>>> + [ -c /dev/urandom ] || { log_urandom_seed "Something is wrong with
>> >>>> /dev/urandom"; return; }
>> >>>> +
>> >>>> + _do_urandom_seed "/etc/urandom.seed"
>> >>>> +
>> >>>> + SEED="$(uci -q get system. at system[0].urandom_seed)"
>> >>>> + [ "${SEED:0:1}" == "/" -a "$SEED" != "/etc/urandom.seed" ] &&
>> >>>> _do_urandom_seed "$SEED"
>> >>>> +}
>> >>>> +
>> >>>> +boot_hook_add preinit_main do_urandom_seed
>> >>>>
>> >>>
>> >>>
>>
>> _______________________________________________
>> Lede-dev mailing list
>> Lede-dev at lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/lede-dev
More information about the Lede-dev
mailing list