[PATCH] sethostname: dump new hostname into RNG

[PATCH] sethostname: dump new hostname into RNG

[Jason A. Donenfeld]

On some small machines with little entropy, a quasi-unique hostname is
sometimes a relevant factor. I've seen, for example, 8 character
alpha-numeric serial numbers. In addition, the time at which the hostname
is set is usually a decent measurement of how long early boot took. So,
call add_device_randomness() on new hostnames, which feeds its arguments
to the RNG in addition to a fresh cycle counter.

Low cost hooks like this never hurt and can only ever help, and since
this costs basically nothing for an operation that is never a fast path,
this is an overall easy win.

Cc: Kees Cook <keescook@chromium.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Dominik Brodowski <linux@dominikbrodowski.net>
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
---
 kernel/sys.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/kernel/sys.c b/kernel/sys.c
index b911fa6d81ab..7b7f973ea585 100644
--- a/kernel/sys.c
+++ b/kernel/sys.c
@@ -1366,6 +1366,7 @@ SYSCALL_DEFINE2(sethostname, char __user *, name, int, len)
 	if (!copy_from_user(tmp, name, len)) {
 		struct new_utsname *u;
 
+		add_device_randomness(tmp, len);
 		down_write(&uts_sem);
 		u = utsname();
 		memcpy(u->nodename, tmp, len);
-- 
2.37.3

[PATCH v2] utsname: contribute changes to RNG

[Jason A. Donenfeld]

On some small machines with little entropy, a quasi-unique hostname is
sometimes a relevant factor. I've seen, for example, 8 character
alpha-numeric serial numbers. In addition, the time at which the hostname
is set is usually a decent measurement of how long early boot took. So,
call add_device_randomness() on new hostnames, which feeds its arguments
to the RNG in addition to a fresh cycle counter.

Low cost hooks like this never hurt and can only ever help, and since
this costs basically nothing for an operation that is never a fast path,
this is an overall easy win.

Cc: Kees Cook <keescook@chromium.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Dominik Brodowski <linux@dominikbrodowski.net>
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
---
 kernel/sys.c            | 2 ++
 kernel/utsname_sysctl.c | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/kernel/sys.c b/kernel/sys.c
index b911fa6d81ab..b4b40ccf0949 100644
--- a/kernel/sys.c
+++ b/kernel/sys.c
@@ -25,6 +25,7 @@
 #include <linux/times.h>
 #include <linux/posix-timers.h>
 #include <linux/security.h>
+#include <linux/random.h>
 #include <linux/suspend.h>
 #include <linux/tty.h>
 #include <linux/signal.h>
@@ -1366,6 +1367,7 @@ SYSCALL_DEFINE2(sethostname, char __user *, name, int, len)
 	if (!copy_from_user(tmp, name, len)) {
 		struct new_utsname *u;
 
+		add_device_randomness(tmp, len);
 		down_write(&uts_sem);
 		u = utsname();
 		memcpy(u->nodename, tmp, len);
diff --git a/kernel/utsname_sysctl.c b/kernel/utsname_sysctl.c
index 4ca61d49885b..de16bcf14b03 100644
--- a/kernel/utsname_sysctl.c
+++ b/kernel/utsname_sysctl.c
@@ -8,6 +8,7 @@
 #include <linux/export.h>
 #include <linux/uts.h>
 #include <linux/utsname.h>
+#include <linux/random.h>
 #include <linux/sysctl.h>
 #include <linux/wait.h>
 #include <linux/rwsem.h>
@@ -57,6 +58,7 @@ static int proc_do_uts_string(struct ctl_table *table, int write,
 		 * theoretically be incorrect if there are two parallel writes
 		 * at non-zero offsets to the same sysctl.
 		 */
+		add_device_randomness(tmp_data, sizeof(tmp_data));
 		down_write(&uts_sem);
 		memcpy(get_uts(table), tmp_data, sizeof(tmp_data));
 		up_write(&uts_sem);
-- 
2.37.3

Re: [PATCH] sethostname: dump new hostname into RNG

[Kees Cook]

On Tue, Sep 27, 2022 at 11:40:39AM +0200, Jason A. Donenfeld wrote:
> On some small machines with little entropy, a quasi-unique hostname is
> sometimes a relevant factor. I've seen, for example, 8 character
> alpha-numeric serial numbers. In addition, the time at which the hostname
> is set is usually a decent measurement of how long early boot took. So,
> call add_device_randomness() on new hostnames, which feeds its arguments
> to the RNG in addition to a fresh cycle counter.
> 
> Low cost hooks like this never hurt and can only ever help, and since
> this costs basically nothing for an operation that is never a fast path,
> this is an overall easy win.

Seems reasonable!

Reviewed-by: Kees Cook <keescook@chromium.org>

> 
> Cc: Kees Cook <keescook@chromium.org>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Dominik Brodowski <linux@dominikbrodowski.net>
> Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
> ---
>  kernel/sys.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/kernel/sys.c b/kernel/sys.c
> index b911fa6d81ab..7b7f973ea585 100644
> --- a/kernel/sys.c
> +++ b/kernel/sys.c
> @@ -1366,6 +1366,7 @@ SYSCALL_DEFINE2(sethostname, char __user *, name, int, len)
>  	if (!copy_from_user(tmp, name, len)) {
>  		struct new_utsname *u;
>  
> +		add_device_randomness(tmp, len);
>  		down_write(&uts_sem);
>  		u = utsname();
>  		memcpy(u->nodename, tmp, len);
> -- 
> 2.37.3
> 

-- 
Kees Cook

Re: [PATCH v2] utsname: contribute changes to RNG

[Kees Cook]

On Tue, Sep 27, 2022 at 11:53:54AM +0200, Jason A. Donenfeld wrote:
> On some small machines with little entropy, a quasi-unique hostname is
> sometimes a relevant factor. I've seen, for example, 8 character
> alpha-numeric serial numbers. In addition, the time at which the hostname
> is set is usually a decent measurement of how long early boot took. So,
> call add_device_randomness() on new hostnames, which feeds its arguments
> to the RNG in addition to a fresh cycle counter.
> 
> Low cost hooks like this never hurt and can only ever help, and since
> this costs basically nothing for an operation that is never a fast path,
> this is an overall easy win.
> 
> Cc: Kees Cook <keescook@chromium.org>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Dominik Brodowski <linux@dominikbrodowski.net>
> Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
> ---
>  kernel/sys.c            | 2 ++
>  kernel/utsname_sysctl.c | 2 ++
>  2 files changed, 4 insertions(+)
> 
> diff --git a/kernel/sys.c b/kernel/sys.c
> index b911fa6d81ab..b4b40ccf0949 100644
> --- a/kernel/sys.c
> +++ b/kernel/sys.c
> @@ -25,6 +25,7 @@
>  #include <linux/times.h>
>  #include <linux/posix-timers.h>
>  #include <linux/security.h>
> +#include <linux/random.h>
>  #include <linux/suspend.h>
>  #include <linux/tty.h>
>  #include <linux/signal.h>
> @@ -1366,6 +1367,7 @@ SYSCALL_DEFINE2(sethostname, char __user *, name, int, len)
>  	if (!copy_from_user(tmp, name, len)) {
>  		struct new_utsname *u;
>  
> +		add_device_randomness(tmp, len);
>  		down_write(&uts_sem);
>  		u = utsname();
>  		memcpy(u->nodename, tmp, len);
> diff --git a/kernel/utsname_sysctl.c b/kernel/utsname_sysctl.c
> index 4ca61d49885b..de16bcf14b03 100644
> --- a/kernel/utsname_sysctl.c
> +++ b/kernel/utsname_sysctl.c
> @@ -8,6 +8,7 @@
>  #include <linux/export.h>
>  #include <linux/uts.h>
>  #include <linux/utsname.h>
> +#include <linux/random.h>
>  #include <linux/sysctl.h>
>  #include <linux/wait.h>
>  #include <linux/rwsem.h>
> @@ -57,6 +58,7 @@ static int proc_do_uts_string(struct ctl_table *table, int write,
>  		 * theoretically be incorrect if there are two parallel writes
>  		 * at non-zero offsets to the same sysctl.
>  		 */
> +		add_device_randomness(tmp_data, sizeof(tmp_data));

Does this matter that we're dumping the same same trailing bytes into
the RNG? (i.e. only min(lenp, table->maxlen) has changed.)

If that's okay (and maybe a comment should be added):

Reviewed-by: Kees Cook <keescook@chromium.org>


>  		down_write(&uts_sem);
>  		memcpy(get_uts(table), tmp_data, sizeof(tmp_data));
>  		up_write(&uts_sem);
> -- 
> 2.37.3
> 

-- 
Kees Cook

Re: [PATCH v2] utsname: contribute changes to RNG

[Jason A. Donenfeld]

On Tue, Sep 27, 2022 at 4:25 PM Kees Cook <keescook@chromium.org>
wrote:> > +             add_device_randomness(tmp_data,
sizeof(tmp_data));
>
> Does this matter that we're dumping the same same trailing bytes into
> the RNG? (i.e. only min(lenp, table->maxlen) has changed.)
>
> If that's okay (and maybe a comment should be added):
>
> Reviewed-by: Kees Cook <keescook@chromium.org>

It doesn't matter. You can call add_device_randomness() as often as
you want on whatever data you want, and it is never bad.

I mentioned in my Linux Plumbers talk that I'd like to see various
subsystems add whatever they can find, provided it's not a performance
bottleneck. add_device_randomness() is good for things that /could/ be
random, but might not be. Right after my talk, somebody posted this,
which I was happy to see:
https://lore.kernel.org/all/20220915004117.1562703-2-bryan.odonoghue@linaro.org/

There's also this commit from a few months ago that I was quite happy
about: https://lore.kernel.org/all/20220425205442.1347837-1-linus.walleij@linaro.org/

This one here is part of that same general trend. If you have more
ideas on odd drivers or hooks or whatever else where that might be
useful, feel free to submit patches.

Jason