Message-ID: <53C8319A.8090108@amacapital.net>
Date:	Thu, 17 Jul 2014 13:27:06 -0700
From:	Andy Lutomirski <luto@...capital.net>
To:	Theodore Ts'o <tytso@....edu>, linux-kernel@...r.kernel.org
CC:	linux-abi@...r.kernel.org, linux-crypto@...r.kernel.org,
	beck@...nbsd.org
Subject: Re: [PATCH, RFC] random: introduce getrandom(2) system call

On 07/17/2014 02:18 AM, Theodore Ts'o wrote:
> The getrandom(2) system call was requested by the LibreSSL Portable
> developers.  It is analoguous to the getentropy(2) system call in
> OpenBSD.
> 
> The rationale of this system call is to provide resiliance against
> file descriptor exhaustion attacks, where the attacker consumes all
> available file descriptors, forcing the use of the fallback code where
> /dev/[u]random is not available.  Since the fallback code is often not
> well-tested, it is better to eliminate this potential failure mode
> entirely.
> 
> The other feature provided by this new system call is the ability to
> request randomness from the /dev/urandom entropy pool, but to block
> until at least 128 bits of entropy has been accumulated in the
> /dev/urandom entropy pool.  Historically, the emphasis in the
> /dev/urandom development has been to ensure that urandom pool is
> initialized as quickly as possible after system boot, and preferably
> before the init scripts start execution.  This is because changing
> /dev/urandom reads to block represents an interface change that could
> potentially break userspace which is not acceptable.  In practice, on
> most x86 desktop and server systems, in general the entropy pool can
> be initialized before it is needed (and in modern kernels, we will
> printk a warning message if not).  However, on an embedded system,
> this may not be hte case.  And so with a new interface, we can provide
> this requested functionality of blocking until the urandom pool has
> been initialized.  Any userspace program which uses this new
> functionality must make sure that if it is used in early boot, that it
> will not cause the boot up scripts or other portions of the system
> startup to hang indefinitely.
> 
> SYNOPSIS
> 	#include <linux/random.h>
> 
> 	int getrandom(void *buf, size_t buflen, unsigned int flags);
> 
> DESCRIPTION
> 
> 	The system call getrandom() fills the buffer pointed to by buf
> 	with up to buflen random bytes which can be used to seed user
> 	space random number generators (i.e., DRBG's) or for other
> 	cryptographic processes.  It should not be used Monte Carlo
> 	simulations or for other probabilistic sampling applications.
> 
> 	If the GRND_RANDOM flags bit is set, then draw from the
> 	/dev/random pool instead of /dev/urandom pool.  The
> 	/dev/random pool is limited based on the entropy that can be
> 	obtained from environmental noise, so if there is insufficient
> 	entropy, the requested number of bytes may not be returned.
> 	If there is no entropy available at all, getrandom(2) will
> 	either return an error with errno set to EAGAIN, or block if
> 	the GRND_BLOCK flags bit is set.
> 
> 	If the GRND_RANDOM flags bit is not set, then the /dev/raundom
> 	pool will be used.  Unlike reading from the /dev/urandom, if
> 	the urandom pool has not been sufficiently initialized,
> 	getrandom(2) will either return an error with errno set to
> 	EGAIN, or block if the GRND_BLOCK flags bit is set.
> 
> RETURN VALUE
>        On success, the number of bytes that was returned is returned.
> 
>        On error, -1 is returned, and errno is set appropriately
> 
> ERRORS
> 	EINVAL		The buflen value was invalid.
> 
> 	EFAULT		buf is outside your accessible address space.
> 
> 	EAGAIN		The requested entropy was not available, and the
> 			getentropy(2) would have blocked if GRND_BLOCK flag
> 			was set.
> 
> Signed-off-by: Theodore Ts'o <tytso@....edu>
> ---
>  arch/x86/syscalls/syscall_32.tbl  |  1 +
>  arch/x86/syscalls/syscall_64.tbl  |  1 +
>  drivers/char/random.c             | 35 +++++++++++++++++++++++++++++++++--
>  include/linux/syscalls.h          |  3 +++
>  include/uapi/asm-generic/unistd.h |  4 +++-
>  include/uapi/linux/random.h       |  9 +++++++++
>  6 files changed, 50 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/x86/syscalls/syscall_32.tbl b/arch/x86/syscalls/syscall_32.tbl
> index d6b8679..f484e39 100644
> --- a/arch/x86/syscalls/syscall_32.tbl
> +++ b/arch/x86/syscalls/syscall_32.tbl
> @@ -360,3 +360,4 @@
>  351	i386	sched_setattr		sys_sched_setattr
>  352	i386	sched_getattr		sys_sched_getattr
>  353	i386	renameat2		sys_renameat2
> +354	i386	getrandom		sys_getrandom
> diff --git a/arch/x86/syscalls/syscall_64.tbl b/arch/x86/syscalls/syscall_64.tbl
> index ec255a1..6705032 100644
> --- a/arch/x86/syscalls/syscall_64.tbl
> +++ b/arch/x86/syscalls/syscall_64.tbl
> @@ -323,6 +323,7 @@
>  314	common	sched_setattr		sys_sched_setattr
>  315	common	sched_getattr		sys_sched_getattr
>  316	common	renameat2		sys_renameat2
> +317	common	getrandom		sys_getrandom
>  
>  #
>  # x32-specific system call numbers start at 512 to avoid cache impact
> diff --git a/drivers/char/random.c b/drivers/char/random.c
> index aa22fe5..76a56f6 100644
> --- a/drivers/char/random.c
> +++ b/drivers/char/random.c
> @@ -258,6 +258,8 @@
>  #include <linux/kmemcheck.h>
>  #include <linux/workqueue.h>
>  #include <linux/irq.h>
> +#include <linux/syscalls.h>
> +#include <linux/completion.h>
>  
>  #include <asm/processor.h>
>  #include <asm/uaccess.h>
> @@ -469,6 +471,8 @@ static struct entropy_store nonblocking_pool = {
>  					push_to_pool),
>  };
>  
> +DECLARE_COMPLETION(urandom_initialized);
> +
>  static __u32 const twist_table[8] = {
>  	0x00000000, 0x3b6e20c8, 0x76dc4190, 0x4db26158,
>  	0xedb88320, 0xd6d6a3e8, 0x9b64c2b0, 0xa00ae278 };
> @@ -657,6 +661,7 @@ retry:
>  		r->entropy_total = 0;
>  		if (r == &nonblocking_pool) {
>  			prandom_reseed_late();
> +			complete_all(&urandom_initialized);
>  			pr_notice("random: %s pool is initialized\n", r->name);
>  		}
>  	}
> @@ -1355,7 +1360,7 @@ static int arch_random_refill(void)
>  }
>  
>  static ssize_t
> -random_read(struct file *file, char __user *buf, size_t nbytes, loff_t *ppos)
> +_random_read(int nonblock, char __user *buf, size_t nbytes)
>  {
>  	ssize_t n;
>  
> @@ -1379,7 +1384,7 @@ random_read(struct file *file, char __user *buf, size_t nbytes, loff_t *ppos)
>  		if (arch_random_refill())
>  			continue;
>  
> -		if (file->f_flags & O_NONBLOCK)
> +		if (nonblock)
>  			return -EAGAIN;
>  
>  		wait_event_interruptible(random_read_wait,
> @@ -1391,6 +1396,12 @@ random_read(struct file *file, char __user *buf, size_t nbytes, loff_t *ppos)
>  }
>  
>  static ssize_t
> +random_read(struct file *file, char __user *buf, size_t nbytes, loff_t *ppos)
> +{
> +	return _random_read(file->f_flags & O_NONBLOCK, buf, nbytes);
> +}
> +
> +static ssize_t
>  urandom_read(struct file *file, char __user *buf, size_t nbytes, loff_t *ppos)
>  {
>  	int ret;
> @@ -1533,6 +1544,26 @@ const struct file_operations urandom_fops = {
>  	.llseek = noop_llseek,
>  };
>  
> +SYSCALL_DEFINE3(getrandom, char __user *, buf, size_t, count,
> +		unsigned int, flags)
> +{
> +	int r;
> +

As previously noted, this needs to validate flags.

> +	if (count > 256)
> +		return -EINVAL;

I think I'd rather see this allow any length, at least when using urandom.

> +
> +	if (flags & GRND_RANDOM) {
> +		return _random_read(!(flags & GRND_BLOCK), buf, count);
> +	}
> +	if (flags & GRND_BLOCK) {
> +		r = wait_for_completion_interruptible(&urandom_initialized);
> +		if (r)
> +			return r;
> +	} else if (!completion_done(&urandom_initialized))
> +		return -EAGAIN;
> +	return urandom_read(NULL, buf, count, NULL);

This can return -ERESTARTSYS.  Does it need any logic to restart correctly?

I don't think it's possible for the urandom case to succeed without
filling the buffer.  If that's true, can we document it and add a
corresponding BUG_ON/WARN_ON in the syscall implementation?

--Andy

> +/*
> + * Flags for getrandom(2)
> + *
> + * GAND_BLOCK 		Allow getrandom(2) to block
> + * GAND_RANDOM		Use the /dev/random pool instead of /dev/urandom

GRND?

> + */
> +#define GRND_BLOCK	0x0001
> +#define GRND_RANDOM	0x0002
> +
>  #endif /* _UAPI_LINUX_RANDOM_H */
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists