linux-kernel - Re: [PATCH v25 01/12] Linux Random Number Generator

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Sat, 16 Nov 2019 19:13:23 +0100
From:   Nicolai Stange <nstange@...e.de>
To:     Stephan Müller <smueller@...onox.de>
Cc:     Arnd Bergmann <arnd@...db.de>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        linux-crypto@...r.kernel.org, LKML <linux-kernel@...r.kernel.org>,
        linux-api@...r.kernel.org,
        "Eric W. Biederman" <ebiederm@...ssion.com>,
        "Alexander E. Patrakov" <patrakov@...il.com>,
        "Ahmed S. Darwish" <darwish.07@...il.com>,
        "Theodore Y. Ts'o" <tytso@....edu>, Willy Tarreau <w@....eu>,
        Matthew Garrett <mjg59@...f.ucam.org>,
        Vito Caputo <vcaputo@...garu.com>,
        Andreas Dilger <adilger.kernel@...ger.ca>,
        Jan Kara <jack@...e.cz>, Ray Strode <rstrode@...hat.com>,
        William Jon McCann <mccann@....edu>,
        zhangjs <zachary@...shancloud.com>,
        Andy Lutomirski <luto@...nel.org>,
        Florian Weimer <fweimer@...hat.com>,
        Lennart Poettering <mzxreary@...inter.de>,
        Nicolai Stange <nstange@...e.de>,
        "Peter\, Matthias" <matthias.peter@....bund.de>,
        Marcelo Henrique Cerri <marcelo.cerri@...onical.com>,
        Roman Drahtmueller <draht@...altsekun.de>,
        Neil Horman <nhorman@...hat.com>
Subject: Re: [PATCH v25 01/12] Linux Random Number Generator

Hi Stephan,

Stephan Müller <smueller@...onox.de> writes:

> +/* Initialize the default DRNG during boot */

I think that this can get called a bit too early through the
get_random_bytes() invoked from e.g. boot_init_stack_canary(): in
start_kernel(), there is

	boot_init_stack_canary();

	time_init();

On ARM (at least with arm_arch_timer.c), get_cycles() would return 0
until

  time_init() => timer_probe() => arch_timer_of_init() =>
  arch_timer_common_init() => arch_timer_arch_init() =>
  arch_timer_delay_timer_register() => register_current_timer_delay()

has executed and thus, ...

> +void lrng_drngs_init_cc20(void)
> +{
> +	unsigned long flags = 0;
> +
> +	if (lrng_get_available())
> +		return;
> +
> +	lrng_sdrng_lock(&lrng_sdrng_init, &flags);
> +	if (lrng_get_available()) {
> +		lrng_sdrng_unlock(&lrng_sdrng_init, &flags);
> +		return;
> +	}
> +
> +	if (random_get_entropy() || random_get_entropy()) {
> +		/*
> +		 * As the highres timer is identified here, previous interrupts
> +		 * obtained during boot time are treated like a lowres-timer
> +		 * would have been present.
> +		 */
> +		lrng_pool_configure(true, LRNG_IRQ_ENTROPY_BITS);
> +	} else {
> +		lrng_health_disable();
> +		lrng_pool_configure(false, LRNG_IRQ_ENTROPY_BITS *
> +					   LRNG_IRQ_OVERSAMPLING_FACTOR);
> +		pr_warn("operating without high-resolution timer and applying "
> +			"IRQ oversampling factor %u\n",
> +			LRNG_IRQ_OVERSAMPLING_FACTOR);


... LRNG thinks that no high-res timer is available even though there
is:

[    0.000000] lrng_sdrng: operating without high-resolution timer and applying IRQ oversampling factor 10
[    0.000000] lrng_chacha20: ChaCha20 core initialized
[    0.000000] lrng_chacha20: ChaCha20 core initialized
[    0.000014] sched_clock: 32 bits at 1000kHz, resolution 1000ns, wraps every 2147483647500ns
[    0.000036] clocksource: timer: mask: 0xffffffff max_cycles: 0xffffffff, max_idle_ns: 1911260446275 ns
[    0.000114] bcm2835: system timer (irq = 27)
[    0.000594] arch_timer: cp15 timer(s) running at 19.20MHz (phys).
[    0.000613] clocksource: arch_sys_counter: mask: 0xffffffffffffff max_cycles: 0x46d987e47, max_idle_ns: 440795202767 ns
[    0.000631] sched_clock: 56 bits at 19MHz, resolution 52ns, wraps every 4398046511078ns
[    0.000645] Switching to timer-based delay loop, resolution 52ns

Note that this last line comes from aforementioned
register_current_timer_delay().

Similarly, get_random_bytes() can get called quite early through
WARN() => warn_slowpath_fmt() => __warn() => print_oops_end_marker() =>
init_oops_id().

Perhaps it would make sense not to do the (pool + health test)
initalization "on-demand", but rather make sure it happens at some
well-defined point after time_init()? Or at least that the pool +
the health tests get reconfigured eventually?


Thanks,

Nicolai

P.S: include/linux/lrng.h needs an #include <linux/errno.h> for
     CONFIG_LRNG_DRNG_SWITCH=n


> +	}
> +
> +	lrng_sdrng_reset(&lrng_sdrng_init);
> +	lrng_cc20_init_state(&secondary_chacha20);
> +	lrng_state_init_seed_work();
> +	lrng_sdrng_unlock(&lrng_sdrng_init, &flags);
> +
> +	lrng_sdrng_lock(&lrng_sdrng_atomic, &flags);
> +	lrng_sdrng_reset(&lrng_sdrng_atomic);
> +	/*
> +	 * We do not initialize the state of the atomic DRNG as it is identical
> +	 * to the secondary DRNG at this point.
> +	 */
> +	lrng_sdrng_unlock(&lrng_sdrng_atomic, &flags);
> +
> +	lrng_trng_init();
> +
> +	lrng_set_available();
> +}
> +
> +/* Reset LRNG such that all existing entropy is gone */

-- 
SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg), GF: Felix Imendörffer