[PATCH, RFC] random: introduce getrandom(2) system call

[PATCH, RFC] random: introduce getrandom(2) system call

[Theodore Ts'o]

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@mit.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;
+
+	if (count > 256)
+		return -EINVAL;
+
+	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);
+}
+
 /***************************************************************
  * Random UUID interface
  *
diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
index b0881a0..cd82f72f 100644
--- a/include/linux/syscalls.h
+++ b/include/linux/syscalls.h
@@ -866,4 +866,7 @@ asmlinkage long sys_process_vm_writev(pid_t pid,
 asmlinkage long sys_kcmp(pid_t pid1, pid_t pid2, int type,
 			 unsigned long idx1, unsigned long idx2);
 asmlinkage long sys_finit_module(int fd, const char __user *uargs, int flags);
+asmlinkage long sys_getrandom(char __user * buf, size_t count,
+			      unsigned int flags);
+
 #endif
diff --git a/include/uapi/asm-generic/unistd.h b/include/uapi/asm-generic/unistd.h
index 3336406..2926b1d 100644
--- a/include/uapi/asm-generic/unistd.h
+++ b/include/uapi/asm-generic/unistd.h
@@ -699,9 +699,11 @@ __SYSCALL(__NR_sched_setattr, sys_sched_setattr)
 __SYSCALL(__NR_sched_getattr, sys_sched_getattr)
 #define __NR_renameat2 276
 __SYSCALL(__NR_renameat2, sys_renameat2)
+#define __NR_getrandom 277
+__SYSCALL(__NR_getrandom, sys_getrandom)
 
 #undef __NR_syscalls
-#define __NR_syscalls 277
+#define __NR_syscalls 278
 
 /*
  * All syscalls below here should go away really,
diff --git a/include/uapi/linux/random.h b/include/uapi/linux/random.h
index fff3528..6c13442 100644
--- a/include/uapi/linux/random.h
+++ b/include/uapi/linux/random.h
@@ -40,4 +40,13 @@ struct rand_pool_info {
 	__u32	buf[0];
 };
 
+/*
+ * Flags for getrandom(2)
+ *
+ * GAND_BLOCK 		Allow getrandom(2) to block
+ * GAND_RANDOM		Use the /dev/random pool instead of /dev/urandom
+ */
+#define GRND_BLOCK	0x0001
+#define GRND_RANDOM	0x0002
+
 #endif /* _UAPI_LINUX_RANDOM_H */
-- 
2.0.0

Re: [PATCH, RFC] random: introduce getrandom(2) system call

[Hannes Frederic Sowa]

On Do, 2014-07-17 at 05:18 -0400, Theodore Ts'o wrote:
> SYNOPSIS
> 	#include <linux/random.h>
> 
> 	int getrandom(void *buf, size_t buflen, unsigned int flags);

Cool, I think the interface is sane.

Btw. couldn't libressl etc. fall back to binary_sysctl
kernel.random.uuid and seed with that as a last resort? We have it
available for few more years.

> +SYSCALL_DEFINE3(getrandom, char __user *, buf, size_t, count,
> +		unsigned int, flags)
> +{
> +	int r;
> +
> +	if (count > 256)
> +		return -EINVAL;
> +

Why this "arbitrary" limitation? Couldn't we just check for > SSIZE_MAX
or to be more conservative to INT_MAX?

> +	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);
> +}
> +

Great, thanks Ted,
Hannes

Re: [PATCH, RFC] random: introduce getrandom(2) system call

[Theodore Ts'o]

On Thu, Jul 17, 2014 at 12:57:07PM +0200, Hannes Frederic Sowa wrote:
> 
> Btw. couldn't libressl etc. fall back to binary_sysctl
> kernel.random.uuid and seed with that as a last resort? We have it
> available for few more years.

Yes, they could.  But trying to avoid more uses of binary_sysctl seems
to be a good thing, I think.  The other thing is that is that this
interface provides is the ability to block until the entropy pool is
initialized, which isn't a big deal for x86 systems, but might be
useful as a gentle forcing function to force ARM systems to figure out
good ways of making sure the entropy pools are initialized (i.e., by
actually providing !@#!@ cycle counter) without breaking userspace
compatibility --- since this is a new interface.

> > +	if (count > 256)
> > +		return -EINVAL;
> > +
> 
> Why this "arbitrary" limitation? Couldn't we just check for > SSIZE_MAX
> or to be more conservative to INT_MAX?

I'm not wedded to this limitation.  OpenBSD's getentropy(2) has an
architected arbitrary limit of 128 bytes.  I haven't made a final
decision if the right answer is to hard code some value, or make this
limit be configurable, or remote the limit entirely (which in practice
would be SSIZE_MAX or INT_MAX).

The main argument I can see for putting in a limit is to encourage the
"proper" use of the interface.  In practice, anything larger than 128
probably means the interface is getting misused, either due to a bug
or some other kind of oversight.

For example, when I started instrumenting /dev/urandom, I caught
Google Chrome pulling 4k out of /dev/urandom --- twice --- at startup
time.  It turns out it was the fault of the NSS library, which was
using fopen() to access /dev/urandom.  (Sigh.)

						- Ted

Re: [PATCH, RFC] random: introduce getrandom(2) system call

[Hannes Frederic Sowa]

On Do, 2014-07-17 at 08:52 -0400, Theodore Ts'o wrote:
> On Thu, Jul 17, 2014 at 12:57:07PM +0200, Hannes Frederic Sowa wrote:
> > 
> > Btw. couldn't libressl etc. fall back to binary_sysctl
> > kernel.random.uuid and seed with that as a last resort? We have it
> > available for few more years.
> 
> Yes, they could.  But trying to avoid more uses of binary_sysctl seems
> to be a good thing, I think.  The other thing is that is that this
> interface provides is the ability to block until the entropy pool is
> initialized, which isn't a big deal for x86 systems, but might be
> useful as a gentle forcing function to force ARM systems to figure out
> good ways of making sure the entropy pools are initialized (i.e., by
> actually providing !@#!@ cycle counter) without breaking userspace
> compatibility --- since this is a new interface.

I am not questioning this new interface - I like it - just wanted to
mention there is already a safe fallback for LibreSSL in the way they
already seem to do it in OpenBSD (via sysctl).

> 
> > > +	if (count > 256)
> > > +		return -EINVAL;
> > > +
> > 
> > Why this "arbitrary" limitation? Couldn't we just check for > SSIZE_MAX
> > or to be more conservative to INT_MAX?
> 
> I'm not wedded to this limitation.  OpenBSD's getentropy(2) has an
> architected arbitrary limit of 128 bytes.  I haven't made a final
> decision if the right answer is to hard code some value, or make this
> limit be configurable, or remote the limit entirely (which in practice
> would be SSIZE_MAX or INT_MAX).
> 
> The main argument I can see for putting in a limit is to encourage the
> "proper" use of the interface.  In practice, anything larger than 128
> probably means the interface is getting misused, either due to a bug
> or some other kind of oversight.
> 
> For example, when I started instrumenting /dev/urandom, I caught
> Google Chrome pulling 4k out of /dev/urandom --- twice --- at startup
> time.  It turns out it was the fault of the NSS library, which was
> using fopen() to access /dev/urandom.  (Sigh.)

In the end people would just recall getentropy in a loop and fetch 256
bytes each time. I don't think the artificial limit does make any sense.
I agree that this allows a potential misuse of the interface, but
doesn't a warning in dmesg suffice?

It also makes it easier to port applications from open("/dev/*random"),
read(...) to getentropy() by reusing the same limits.

I would vote for warning (at about 256 bytes) + no limit.

Thanks,
Hannes

Re: [PATCH, RFC] random: introduce getrandom(2) system call

[Tobias Klauser]

On 2014-07-17 at 11:18:15 +0200, Theodore Ts'o <tytso@mit.edu> wrote:

[...]

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

Very minor nitpick: These should probably read GRND_BLOCK/GRND_RANDOM as
well, no?

> +#define GRND_BLOCK	0x0001
> +#define GRND_RANDOM	0x0002
> +
>  #endif /* _UAPI_LINUX_RANDOM_H */
> -- 
> 2.0.0
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 

Re: [PATCH, RFC] random: introduce getrandom(2) system call

[Theodore Ts'o]

On Thu, Jul 17, 2014 at 02:09:49PM +0200, Tobias Klauser wrote:
> 
> > +/*
> > + * Flags for getrandom(2)
> > + *
> > + * GAND_BLOCK 		Allow getrandom(2) to block
> > + * GAND_RANDOM		Use the /dev/random pool instead of /dev/urandom
> > + */
> 
> Very minor nitpick: These should probably read GRND_BLOCK/GRND_RANDOM as
> well, no?

Thanks, typo.  I'll get this fixed up.

	       	    	     	   - Ted

Re: [PATCH, RFC] random: introduce getrandom(2) system call

[Christoph Hellwig]

On Thu, Jul 17, 2014 at 05:18:15AM -0400, 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.

What's the reason to not implement exactly the same system call OpenBSD
does?  Having slightly different names and semantics for the same
functionality is highly annoying.

Re: [PATCH, RFC] random: introduce getrandom(2) system call

[Theodore Ts'o]

On Thu, Jul 17, 2014 at 09:12:15AM -0700, Christoph Hellwig wrote:
> On Thu, Jul 17, 2014 at 05:18:15AM -0400, 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.
> 
> What's the reason to not implement exactly the same system call OpenBSD
> does?  Having slightly different names and semantics for the same
> functionality is highly annoying.

The getrandom(2) system call is a superset of getentropy(2).  When we
add the support for this into glibc, it won't be terribly difficult
nor annoying to drop the following in alongside the standard support
needed for any new system call:

int getentropy(void *buf, size_t buflen)
{
	int	ret;

	ret = getentropy(buf, buflen, 0);
	return (ret > 0) ? 0 : ret;
}

The reason for the additional flags is that I'm trying to solve more
problems than just getentropy()'s raison d'etre.  The discussion of
this is in the commit description; let me know if there bits that I
could make clearer.

Cheers,

					- Ted

Re: [PATCH, RFC] random: introduce getrandom(2) system call

[Bob Beck]

Hi Ted, yeah I understand the reasoning, it would be good if there was
a way to influence the various libc people to
ensure they manage to provide a getentropy().


On Thu, Jul 17, 2014 at 11:01 AM, Theodore Ts'o <tytso@mit.edu> wrote:
> On Thu, Jul 17, 2014 at 09:12:15AM -0700, Christoph Hellwig wrote:
>> On Thu, Jul 17, 2014 at 05:18:15AM -0400, 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.
>>
>> What's the reason to not implement exactly the same system call OpenBSD
>> does?  Having slightly different names and semantics for the same
>> functionality is highly annoying.
>
> The getrandom(2) system call is a superset of getentropy(2).  When we
> add the support for this into glibc, it won't be terribly difficult
> nor annoying to drop the following in alongside the standard support
> needed for any new system call:
>
> int getentropy(void *buf, size_t buflen)
> {
>         int     ret;
>
>         ret = getentropy(buf, buflen, 0);
>         return (ret > 0) ? 0 : ret;
> }
>
> The reason for the additional flags is that I'm trying to solve more
> problems than just getentropy()'s raison d'etre.  The discussion of
> this is in the commit description; let me know if there bits that I
> could make clearer.
>
> Cheers,
>
>                                         - Ted

Re: [PATCH, RFC] random: introduce getrandom(2) system call

[Theodore Ts'o]

On Thu, Jul 17, 2014 at 11:05:01AM -0600, Bob Beck wrote:
> Hi Ted, yeah I understand the reasoning, it would be good if there was
> a way to influence the various libc people to
> ensure they manage to provide a getentropy().

I don't anticipate that to be a problem.  And before they do, and/or
if you are dealing with a system where the kernel has been upgraded,
but not libc, you have your choice of either sticking with the
binary_sysctl approach, or calling getrandom directly using the
syscall method; and in that case, whether we use getrandom() or
provide an exact getentropy() replacement system call isn't that much
difference, since you'd have to have Linux-specific workaround code
anyway....

				- Ted

Re: [PATCH, RFC] random: introduce getrandom(2) system call

[Bob Beck]

we have diffs pending that will do the syscall method until we start
to see it in libc :)

So basically we're going to put that in right away :)

On Thu, Jul 17, 2014 at 11:34 AM, Theodore Ts'o <tytso@mit.edu> wrote:
> On Thu, Jul 17, 2014 at 11:05:01AM -0600, Bob Beck wrote:
>> Hi Ted, yeah I understand the reasoning, it would be good if there was
>> a way to influence the various libc people to
>> ensure they manage to provide a getentropy().
>
> I don't anticipate that to be a problem.  And before they do, and/or
> if you are dealing with a system where the kernel has been upgraded,
> but not libc, you have your choice of either sticking with the
> binary_sysctl approach, or calling getrandom directly using the
> syscall method; and in that case, whether we use getrandom() or
> provide an exact getentropy() replacement system call isn't that much
> difference, since you'd have to have Linux-specific workaround code
> anyway....
>
>                                 - Ted

Re: [PATCH, RFC] random: introduce getrandom(2) system call

[Bob Beck]

And thanks btw.

I don't suppose you guys know who we should talk to about possibly
getting MAP_INHERIT_ZERO minherit() support?

On Thu, Jul 17, 2014 at 11:45 AM, Bob Beck <beck@openbsd.org> wrote:
> we have diffs pending that will do the syscall method until we start
> to see it in libc :)
>
> So basically we're going to put that in right away :)
>
> On Thu, Jul 17, 2014 at 11:34 AM, Theodore Ts'o <tytso@mit.edu> wrote:
>> On Thu, Jul 17, 2014 at 11:05:01AM -0600, Bob Beck wrote:
>>> Hi Ted, yeah I understand the reasoning, it would be good if there was
>>> a way to influence the various libc people to
>>> ensure they manage to provide a getentropy().
>>
>> I don't anticipate that to be a problem.  And before they do, and/or
>> if you are dealing with a system where the kernel has been upgraded,
>> but not libc, you have your choice of either sticking with the
>> binary_sysctl approach, or calling getrandom directly using the
>> syscall method; and in that case, whether we use getrandom() or
>> provide an exact getentropy() replacement system call isn't that much
>> difference, since you'd have to have Linux-specific workaround code
>> anyway....
>>
>>                                 - Ted

Re: [PATCH, RFC] random: introduce getrandom(2) system call

[Bob Beck]

Or perhaps to put that another way, since you don't do minherit -
maybe a FORK_ZERO for madvise? or a similar way
to do that?


On Thu, Jul 17, 2014 at 11:46 AM, Bob Beck <beck@openbsd.org> wrote:
> And thanks btw.
>
> I don't suppose you guys know who we should talk to about possibly
> getting MAP_INHERIT_ZERO minherit() support?
>
> On Thu, Jul 17, 2014 at 11:45 AM, Bob Beck <beck@openbsd.org> wrote:
>> we have diffs pending that will do the syscall method until we start
>> to see it in libc :)
>>
>> So basically we're going to put that in right away :)
>>
>> On Thu, Jul 17, 2014 at 11:34 AM, Theodore Ts'o <tytso@mit.edu> wrote:
>>> On Thu, Jul 17, 2014 at 11:05:01AM -0600, Bob Beck wrote:
>>>> Hi Ted, yeah I understand the reasoning, it would be good if there was
>>>> a way to influence the various libc people to
>>>> ensure they manage to provide a getentropy().
>>>
>>> I don't anticipate that to be a problem.  And before they do, and/or
>>> if you are dealing with a system where the kernel has been upgraded,
>>> but not libc, you have your choice of either sticking with the
>>> binary_sysctl approach, or calling getrandom directly using the
>>> syscall method; and in that case, whether we use getrandom() or
>>> provide an exact getentropy() replacement system call isn't that much
>>> difference, since you'd have to have Linux-specific workaround code
>>> anyway....
>>>
>>>                                 - Ted

Re: [PATCH, RFC] random: introduce getrandom(2) system call

[Theodore Ts'o]

On Thu, Jul 17, 2014 at 11:45:56AM -0600, Bob Beck wrote:
> we have diffs pending that will do the syscall method until we start
> to see it in libc :)

One warning --- please don't check in the syscall number until it
actually hits mainline.  Kees has another patch outstanding for
seccomp(2) that is using the same syscall numbers I have in my patch.
Please treat the numbers in my patch as placeholders until it has been
accepted and we see what numbers we actually get assigned.

Thanks,

						- Ted

Re: [PATCH, RFC] random: introduce getrandom(2) system call

[Bob Beck]

Hey Ted, one more nit. Yes, I have a bicycle too..

I see here we add a flag to make it block - whereas it seems most
other system calls that can block the flag is
added to make it not block (I.E. O_NONBLOCK, etc. etc.)  Would it make
more sense to invert this so it was more
like the typical convention in other system calls?


On Thu, Jul 17, 2014 at 11:34 AM, Theodore Ts'o <tytso@mit.edu> wrote:
> On Thu, Jul 17, 2014 at 11:05:01AM -0600, Bob Beck wrote:
>> Hi Ted, yeah I understand the reasoning, it would be good if there was
>> a way to influence the various libc people to
>> ensure they manage to provide a getentropy().
>
> I don't anticipate that to be a problem.  And before they do, and/or
> if you are dealing with a system where the kernel has been upgraded,
> but not libc, you have your choice of either sticking with the
> binary_sysctl approach, or calling getrandom directly using the
> syscall method; and in that case, whether we use getrandom() or
> provide an exact getentropy() replacement system call isn't that much
> difference, since you'd have to have Linux-specific workaround code
> anyway....
>
>                                 - Ted

Re: [PATCH, RFC] random: introduce getrandom(2) system call

[Dwayne Litzenberger]

On Thu, Jul 17, 2014 at 01:01:16PM -0400, Theodore Ts'o wrote:
>The getrandom(2) system call is a superset of getentropy(2).  When we
>add the support for this into glibc, it won't be terribly difficult
>nor annoying to drop the following in alongside the standard support
>needed for any new system call:
>
>int getentropy(void *buf, size_t buflen)
>{
>	int	ret;
>
>	ret = getentropy(buf, buflen, 0);
>	return (ret > 0) ? 0 : ret;
>}
>
>The reason for the additional flags is that I'm trying to solve more
>problems than just getentropy()'s raison d'etre.  The discussion of
>this is in the commit description; let me know if there bits that I
>could make clearer.

This could still return predictable bytes during early boot, though, 
right?

-- 
Dwayne C. Litzenberger <dlitz@dlitz.net>
 OpenPGP: 19E1 1FE8 B3CF F273 ED17  4A24 928C EC13 39C2 5CF7

Re: [PATCH, RFC] random: introduce getrandom(2) system call

[Theodore Ts'o]

On Sun, Jul 20, 2014 at 05:25:40PM -0700, Dwayne Litzenberger wrote:
> 
> This could still return predictable bytes during early boot, though, right?

Read the suggetsed man page; especially, the version of the man page
in the commit description -v4 version of the patch.

getrandom(2) is a new interface, so I can afford do things differently
from redaing from /dev/urandom.  Specifically, it will block until it
is fully initialized, and in GRND_NONBLOCK mode, it will return
EAGAIN.

There have been some people kvetching and whining that this is less
convenient for seeding srand(3), but for non-crypto uses, using
getpid() and time() to seed random(3) or rand(3) is just fine, and if
they really want, they can open /dev/urandom.

> >	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.
> 
> Aside from poor performance for the offending application, will anything
> actually break if an application ignores this warning and makes heavy
> use of getrandom(2)?  

It will be slow, and then the graduate student will whine and complain
and send a bug report.  It will cause urandom to pull more heavily on
entropy, and if that means that you are using some kind of hardware
random generator on a laptop, such as tpm-rng, you will burn more
battery, but no, it will not break.  This is why the man page says
SHOULD not, and not MUST not.   :-)


> As the developer of a userspace crypto library, I can't always prevent
> downstream developers from doing silly things, and many developers
> simply don't understand different "kinds" of random numbers, so I prefer
> to tell them to just use the kernel CSPRNG by default, and to ask for
> help once they run into performance problems.  It's not ideal, but it's
> safer than the alternative.[1]

Yes, but the point is that Monte Carlo simulations don't need any kind
crypto guarantees.

> Hm.  Is it correct that, in blocking mode, the call is guaranteed either
> to return -EINVAL immediately, or to block until the buffer is
> *completely* populated with buflen bytes?  If so, I think a few small
> changes could make this a really nice interface to work with:
> 
> * Use blocking mode by default.

Read the -v4 version of the patch.  Blocking is now the default.

> * Add a new flag called GRND_PARTIAL (replacing GRND_BLOCK), which
>  indicates that the caller is prepared to handle a partial/incomplete
>  result.

This is not needed if you are using the preferred use of flags == 0,
and are extracting a sane amount of entropy.  For values of buflen <
256 bytes, once urandom pool is initialized, getrandom(buf, buflen, 0)
will not block and will always return the amount of entropy that you
asked for.

But if the user asks for INT_MAX bytes, getrandom(2) must be
interruptible, or else you will end up burning CPU time for a long,
LONG, LONG time.  The choice was to either pick some arbitrarily
limit, such as 256, and then return EIO, which is what OpenBSD did.  I
decided to simply allow getrandom(2) to be interruptible if buflen >
256.

Similarly, if you use GRND_RANDOM, you are also implicitly agreeing
for what you are calling GRND_PARTIAL semantics, because otherwise,
you could end up blocking for a very long time, with no way to
interrupt a buggy program.

I'd much rather keep things simple, and not add too many extra flags,
especially when certain combination of flags result in a really
unfriendly / insane result.

> * If GRND_PARTIAL is *not* set, just return 0 on success.  (This avoids
>  all signed-unsigned confusion when buflen > INT_MAX.)

We simply cap any requests for buflen > INT_MAX, and in practice, it
would take so long to generate the requested number of bytes that the
user would want to interrupt the process anyway.

I considered adding a printk to shame any application writer that
tried to ask for more than 256 bytes, but ultimately decided that was
a bit over the top.  But in any case, any request anywhere near
INT_MAX bytes is really not anything I'm concerned about.  If we do
start seeing evidence for that, I might reconsider and add some kind
of "Warning!  The author of [current->comm] is asking for insane
amounts of entropy" printk.

> With those changes, it would be trivial for a userspace library to
> implement a reliable RNG interface as recommended in [2] or [3]:

The userspace library should ideally be integrted into glibc, so it is
fork- and thread- aware, and it should use a proper CRNG, much like
OpenBSD's arcrandom(3).  There's been a proposal submitted to the
Austin Group for future standardization in Posix, and I'm all in favor
of something like that.

> P.S.  If I had my way, I would also drop GRND_RANDOM.  Most software
> won't use it, there's no legacy installed base, and no developer who
> still wants that behavior can legitimately claim to care about RNG
> availability guarantees, IMHO.

GnuPG uses /dev/random when generating long term public keys.  So
there are some use cases where I think GRND_RANDOM makes sense.  I
agree they are not the normal ones, but they do exist.

> [1] On more than one occasion, I've seen developers use Python's
>    standard "random" module to generate IVs.  I mean, why not?  IVs are
>    public, right?

Why are they writing code that needs raw IV's in the first place?
Sounds like a misdesigned crypto library for me, or they were using
low-level crypto when they should have been using a more higher-level
abstraction.

The fundamental problem is that it's impossible to make an interface
be moron-proof because morons are so ingenious.  :-) If they are so
misinformed that they are using random(3) to generate IV's, they are
extremely likely to be making other, much more fundamentally wrong
mistakes.  The only real solution is to not let them anywhere near
that level of crypto programming, which means you need to have high
level libraries which are so appealing and easy to use that they
aren't tempted to go low level on you...

Originally I thought that the answer was to have a userspace library
interface where you would ask the user to tell you whether they needed
the interface for Monte Carlo simulations, IV's, padding, session
keys, long-term keys, etc.  However, the counter argument was that the
morons that would be helped by this explicit statement of need would
likely be making other crypto-implementation mistakes anyway, so it's
not worth it.

The one reamining reason why specifying the usage of the random values
might be useful is if you are in a world where various NIST standards
or some Military- or credit-card-processing imposed standards might
require you to say, using a certified hardware RNG for long-term keys,
but allowed the use of a crypto-based DRBG for session keys (for
example), that's something that could be configured or controlled in a
single location.

Cheers,

						- Ted

Re: [PATCH, RFC] random: introduce getrandom(2) system call

[Greg KH]

On Thu, Jul 17, 2014 at 05:18:15AM -0400, Theodore Ts'o wrote:

Minor nit:

> @@ -469,6 +471,8 @@ static struct entropy_store nonblocking_pool = {
>  					push_to_pool),
>  };
>  
> +DECLARE_COMPLETION(urandom_initialized);
> +

static DECLARE_COMPLETION(urandom_initialized);

instead?

thanks,

greg k-h

Re: [PATCH, RFC] random: introduce getrandom(2) system call

[Greg KH]

On Thu, Jul 17, 2014 at 05:18:15AM -0400, Theodore Ts'o wrote:
> +SYSCALL_DEFINE3(getrandom, char __user *, buf, size_t, count,
> +		unsigned int, flags)
> +{
> +	int r;
> +
> +	if (count > 256)
> +		return -EINVAL;
> +
> +	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);
> +}

You should fail if any other bits are set that you don't understand in
the flags value, to make it easier for newer kernels with more flags to
fail properly on old kernel releases.

thanks,

greg k-h

Re: [PATCH, RFC] random: introduce getrandom(2) system call

[Zach Brown]

> SYNOPSIS
> 	#include <linux/random.h>
> 
> 	int getrandom(void *buf, size_t buflen, unsigned int flags);

I certainly like the idea of getting entropy without having to worry
about fds.

> 	If the GRND_RANDOM flags bit is not set, then the /dev/raundom

(raundom typo)

> RETURN VALUE
>        On success, the number of bytes that was returned is returned.

The description talks about filling the buffer, maybe say 'the number of
bytes filled is returned'?  

> +DECLARE_COMPLETION(urandom_initialized);

static?

> +SYSCALL_DEFINE3(getrandom, char __user *, buf, size_t, count,
> +		unsigned int, flags)
> +{
> +	int r;
> +

Michael Kerrisk wants you to return -EINVAL on unknown flags :)

http://lwn.net/Articles/588444/

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

I'd vote for not having the limit.  It seems easy enough to iterate over
the buffer.  We'd need to clamp the count to ssize_t, though.

> +	if (flags & GRND_RANDOM) {
> +		return _random_read(!(flags & GRND_BLOCK), buf, count);
> +	}

Do we want it to block by default and have the flag be _NONBLOCK?  Feels
more.. familiar.

> +	if (flags & GRND_BLOCK) {
> +		r = wait_for_completion_interruptible(&urandom_initialized);
> +		if (r)
> +			return r;

I can *never* remember the rules for -ERESTARTSYS.  The syscall callers
take care of this?

> +	return urandom_read(NULL, buf, count, NULL);

I wonder if we want to refactor the entry points a bit more instead of
directly calling the device read functions.  get_random_bytes() and
urandom_read() both have their own uninitialied use warning message and
tracing.  Does the syscall want its own little extraction function as
well?

- z

Re: [PATCH, RFC] random: introduce getrandom(2) system call

[Theodore Ts'o]

On Thu, Jul 17, 2014 at 12:48:12PM -0700, Zach Brown wrote:
> 
> > +	return urandom_read(NULL, buf, count, NULL);
> 
> I wonder if we want to refactor the entry points a bit more instead of
> directly calling the device read functions.  get_random_bytes() and
> urandom_read() both have their own uninitialied use warning message and
> tracing.  Does the syscall want its own little extraction function as
> well?

I'm not sure what warning you are worried about?  urandom_read() never
uses file or ppos, so passing in NULL works just fine as near as I can
tell.

I could refactor the entropy point, but it probably wouldn't add any
extra bloat, since the compiler would hopefully compile it away, but
adding the extra static function would seem to make things less
readable at least in my opinion.

					- Ted

Re: [PATCH, RFC] random: introduce getrandom(2) system call

[Zach Brown]

On Thu, Jul 17, 2014 at 04:54:17PM -0400, Theodore Ts'o wrote:
> On Thu, Jul 17, 2014 at 12:48:12PM -0700, Zach Brown wrote:
> > 
> > > +	return urandom_read(NULL, buf, count, NULL);
> > 
> > I wonder if we want to refactor the entry points a bit more instead of
> > directly calling the device read functions.
> 
> I could refactor the entropy point, but it probably wouldn't add any
> extra bloat, since the compiler would hopefully compile it away, but
> adding the extra static function would seem to make things less
> readable at least in my opinion.

Fair enough, I don't have a strong preference either way.  It was just
something I noticed when leafing through the (unfamiliar) code.

- z

Re: [PATCH, RFC] random: introduce getrandom(2) system call

[Andy Lutomirski]

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@mit.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 */
> 

Re: [PATCH, RFC] random: introduce getrandom(2) system call

[Theodore Ts'o]

On Thu, Jul 17, 2014 at 01:27:06PM -0700, Andy Lutomirski wrote:
> > +	return urandom_read(NULL, buf, count, NULL);
> 
> This can return -ERESTARTSYS.  Does it need any logic to restart correctly?

Nope; because we only return -ERESTARTSYS when we haven't generated
any randomness yet.  The way /dev/urandom and /dev/random devices work
is that if we get interrupted, we return a short read.  We do *not*
resume generation of random bytes from where we got interrupted from
the signal handler.

This is consistent with the definition in the signal(7) man page:

       If a blocked call to one of the following interfaces is
       interrupted by a signal handler, then the call will be
       automatically restarted after the signal handler returns if the
       SA_RESTART flag was used; otherwise the call will fail with the
       error EINTR:

           * read(2), readv(2), write(2), writev(2), and ioctl(2)
             calls on "slow" devices.  A "slow" device is one where
             the I/O call may block for an indefinite time, for
             example, a terminal, pipe, or socket.  (A disk is not a
             slow device according to this definition.)  If an I/O
             call on a slow device has already transferred some data
             by the time it is interrupted by a signal handler, then
             the call will return a success status (normally, the
             number of bytes transferred).

And in answer to Zach's question along these lines, ERESTARTSYS gets
restarted or transformed into EINTR by the system call layer, so long
as you only set ERESTARTSYS when signal_pending(current) is true.  If
you accidentally set the return value to ERESTARTSYS when a signal is
not pending, this error code can escape out to user space, which is
considered a bug.  But we're using this correctly in
drivers/char/random.c.

						- Ted

Re: [PATCH, RFC] random: introduce getrandom(2) system call

[Rolf Eike Beer]

[-- Attachment #1: Type: text/plain, Size: 468 bytes --]

Theodore Ts'o wrote:

> 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.

The "for" looks misplaced, shouldn't this be "for Monte Carlo simulations or 
other ..."?

Greetings,

Eike

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH, RFC] random: introduce getrandom(2) system call

[Andi Kleen]

Theodore Ts'o <tytso@mit.edu> writes:
>  
>  #undef __NR_syscalls
> -#define __NR_syscalls 277
> +#define __NR_syscalls 278

You need to add the syscall to kernel/sys_ni.c too, otherwise it will be
impossible to build without random device.

-Andi

-- 
ak@linux.intel.com -- Speaking for myself only

Re: [PATCH, RFC] random: introduce getrandom(2) system call

[Theodore Ts'o]

On Sun, Jul 20, 2014 at 08:50:06AM -0700, Andi Kleen wrote:
> Theodore Ts'o <tytso@mit.edu> writes:
> >  
> >  #undef __NR_syscalls
> > -#define __NR_syscalls 277
> > +#define __NR_syscalls 278
> 
> You need to add the syscall to kernel/sys_ni.c too, otherwise it will be
> impossible to build without random device.

The random device is not optional; it is alaways guaranteed to be
present.  From drivers/char/Makefile:

obj-y			+= mem.o random.o

Cheers,

					- Ted

Re: [PATCH, RFC] random: introduce getrandom(2) system call

[Andreas Schwab]

Theodore Ts'o <tytso@mit.edu> writes:

> ERRORS
> 	EINVAL		The buflen value was invalid.

Also on unknown flags?  Without that it would be impossible to probe for
implemented flags.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [PATCH, RFC] random: introduce getrandom(2) system call

[Theodore Ts'o]

On Sun, Jul 20, 2014 at 07:27:42PM +0200, Andreas Schwab wrote:
> Theodore Ts'o <tytso@mit.edu> writes:
> 
> > ERRORS
> > 	EINVAL		The buflen value was invalid.
> 
> Also on unknown flags?  Without that it would be impossible to probe for
> implemented flags.

We removed the cap on the buflen size (although if someone gives
something insanely large, it can get capped), so EINVAL will only
happen for unknown flags.  I'll fix the suggested man page.

	       	   	   	   	    - Ted

P.S. The reason why OpenBSD had a very strong opinion about returning
EIO for buflen greater than 256 bytes was they really wanted to pound
it into application writers than if they were trying to fetch more
than 256 bytes, they were probably Doing Something Wrong, and they
decided EIO was less subtle that EINVAL....

Re: [PATCH, RFC] random: introduce getrandom(2) system call

[Dwayne Litzenberger]

[-- Attachment #1: Type: text/plain, Size: 4640 bytes --]

On Thu, Jul 17, 2014 at 05:18:15AM -0400, Theodore Ts'o wrote:
>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.

Aside from poor performance for the offending application, will anything
actually break if an application ignores this warning and makes heavy
use of getrandom(2)?  It would be helpful if the documentation made this
clearer, rather than just saying, "don't do that".

As the developer of a userspace crypto library, I can't always prevent
downstream developers from doing silly things, and many developers
simply don't understand different "kinds" of random numbers, so I prefer
to tell them to just use the kernel CSPRNG by default, and to ask for
help once they run into performance problems.  It's not ideal, but it's
safer than the alternative.[1]

>	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

Hm.  Is it correct that, in blocking mode, the call is guaranteed either
to return -EINVAL immediately, or to block until the buffer is
*completely* populated with buflen bytes?  If so, I think a few small
changes could make this a really nice interface to work with:

* Use blocking mode by default.

* Add a new flag called GRND_PARTIAL (replacing GRND_BLOCK), which
  indicates that the caller is prepared to handle a partial/incomplete
  result.

* On success with GRND_PARTIAL, return the number of bytes that were
  written into buf.  (Or -EAGAIN, as is currently done.)

* If GRND_PARTIAL is *not* set, just return 0 on success.  (This avoids
  all signed-unsigned confusion when buflen > INT_MAX.)

With those changes, it would be trivial for a userspace library to
implement a reliable RNG interface as recommended in [2] or [3]:

/*
 * memzap(3)
 *
 * Fills the buffer pointed to by buf with exactly buflen random bytes
 * suitable for cryptographic purposes.  Nothing is returned.
 *
 * This function is thread-safe, and is safe to call from inside a
 * signal handler.
 *
 * It blocks if the kernel random number generator is not yet fully
 * initialized (e.g. early in the boot process), and it may trigger
 * abort(3) if invoked on an old kernel version that does not support
 * the getrandom(2) system call.
 */
void memzap(void *buf, size_t buflen)
{
        int     ret;

        ret = getrandom(buf, buflen, 0);
        if (ret != 0) {
                perror("getrandom(2) failed");
                abort();
        }
}

Best,
- Dwayne

P.S.  If I had my way, I would also drop GRND_RANDOM.  Most software
won't use it, there's no legacy installed base, and no developer who
still wants that behavior can legitimately claim to care about RNG
availability guarantees, IMHO.  Anyone who really wants the old
/dev/random behavior can always just continue to use the existing
character device.  I don't really care enough to put up a fight about
it, though, as long as it doesn't affect the quality of the
non-GRND_RANDOM interface.


[1] On more than one occasion, I've seen developers use Python's
    standard "random" module to generate IVs.  I mean, why not?  IVs are
    public, right?
[2] http://cr.yp.to/highspeed/coolnacl-20120725.pdf
[3] https://groups.google.com/forum/#!msg/randomness-generation/4opmDHA6_3w/__TyKhbnNWsJ

-- 
Dwayne C. Litzenberger <dlitz@dlitz.net>
 OpenPGP: 19E1 1FE8 B3CF F273 ED17  4A24 928C EC13 39C2 5CF7

[-- Attachment #2: Type: application/pgp-signature, Size: 222 bytes --]

Re: [PATCH, RFC] random: introduce getrandom(2) system call

[Manuel Schölling]

Hi,

I am wondering if we could improve the design of the system call a bit
to prevent programming errors.
Right now, EINVAL is returned in case of invalid flags (or in the older
version of getrandom() also if buflen is too large), EFAULT if buf is an
invalid address and EAGAIN if there is not enough entropy.

However, of course no programmer is save against programming errors.
Everybody *should* check the return value of syscalls, but sometimes it
is forgotten, and theoretically you must be stoned to death for that.

Still, we should think about how we could prevent these errors. Here is
a list of possible modifications of getrandom() and pros and cons:

1. memset(buf, 0x0, buflen) in case of an error
pros:
 - it is more obvious to the userspace programmer that the content of
buffer does *not* contain random bytes
cons:
 - in case even the zero-ed buf is not noticed by the programmer, she/he
might end up using a 100% predictable string of "random bytes". In
contrast if zero-ing the buf is ommitted, you would at least end up
using some (not-cryptographically) random bytes from somewhere in RAM.

I am aware that this memset() call should theoretically be superfluous
but it would only be executed in very rare cases where the programmer
misuses getrandom().


2. int getrandom(void **buf, size_t buflen, unsigned int flags)
                      ^^
If flags, are fine, return a pointer to a buffer of random bytes,
otherwise return a pointer to NULL.

pros:
 - it would ensure that an error in a getrandom() call cannot be
ignored.
cons:
 - not sure if a syscall should allocate memory in the name of a
userspace program
 - not a very unix-like syscall signature
 - anytime getrandom() is called, it will allocate a new buffer which
might end up in decreased performance (however, getrandom() should not
be called multiple times)


3. send a signal to the userland process that (by default) leads to an
abnormal termination of the process
Essentially an error in getrandom() could be seen as critical as a
division by 0.

pros:
 - the userspace programmer is forced to handle this error (otherwise
the signal would terminate the program)
cons:
 - adds more complexity to the userspace program that might lead to new
programming errors


These are three possibilities. Maybe one of you is more creative and can
come up with a much better idea. At the moment, I like option 2 the
best, because it forces the programmer to deal with these errors, but
probably one of you has a good point why this is not a good idea.
Handling the NULL pointer would be much easier than using signals
(option 3). However, it lead to a syscall signature that is different
from, let's say read(), because the syscall itself would allocate its
buffer.

Again, I am aware that you must always check return values, but
programming errors happen. E.g. everybody knows that you cannot trust
data that you received via network, yet heartbleed happened.
Here we have the chance to eradicate a critical programming error by
improving the syscall design and I think we should spend some time
thinking about that.


Best,

Manuel

Re: [PATCH, RFC] random: introduce getrandom(2) system call

[Mark Kettenis]

On Thu, Jul 17, 2014, Theodore Ts'o wrote:
> 
> The getrandom(2) system call is a superset of getentropy(2).  When we
> add the support for this into glibc, it won't be terribly difficult
> nor annoying to drop the following in alongside the standard support
> needed for any new system call:
> 
> int getentropy(void *buf, size_t buflen)
> {
> 	int	ret;
> 
> 	ret = getentropy(buf, buflen, 0);
> 	return (ret > 0) ? 0 : ret;
> }

I'm sure you meant to use getrandom() there ;)

Since for LibreSSL we'd want a getentropy() that cannot fail the
getrandom() call should use GRND_BLOCK flag.  Actually it makes sense
(to me) to make blocking the default behaviour and have a
BRND_NONBLOCK flag.  Much in the same way as you need to specify
O_NONBLOCK if you want non-blocking behaviour for files.

Re: [PATCH, RFC] random: introduce getrandom(2) system call

[Andy Lutomirski]

On 07/17/2014 11:48 AM, Mark Kettenis wrote:
> On Thu, Jul 17, 2014, Theodore Ts'o wrote:
>>
>> The getrandom(2) system call is a superset of getentropy(2).  When we
>> add the support for this into glibc, it won't be terribly difficult
>> nor annoying to drop the following in alongside the standard support
>> needed for any new system call:
>>
>> int getentropy(void *buf, size_t buflen)
>> {
>> 	int	ret;
>>
>> 	ret = getentropy(buf, buflen, 0);
>> 	return (ret > 0) ? 0 : ret;
>> }
> 
> I'm sure you meant to use getrandom() there ;)
> 
> Since for LibreSSL we'd want a getentropy() that cannot fail the
> getrandom() call should use GRND_BLOCK flag.  Actually it makes sense
> (to me) to make blocking the default behaviour and have a
> BRND_NONBLOCK flag.  Much in the same way as you need to specify
> O_NONBLOCK if you want non-blocking behaviour for files.
> 

Can we please have a mode in which getrandom(2) can neither block nor
fail?  If that gets added, then this can replace things like AT_RANDOM.

There are non-crypto things out there that will want this.  There are
also probably VM systems (especially ones that have something like my
KVM_GET_RNG_SEED patches applied, or many VMs on Haswell, for that
matter) that will have perfectly fine cryptographically secure urandom
output immediately after bootup but that won't consider themselves
"initialized" for a while.  At least these will be perfectly fine from
the POV of those who trust their hypervisor and Intel :)

--Andy

Re: [PATCH, RFC] random: introduce getrandom(2) system call

[Theodore Ts'o]

On Thu, Jul 17, 2014 at 01:35:15PM -0700, Andy Lutomirski wrote:
> 
> Can we please have a mode in which getrandom(2) can neither block nor
> fail?  If that gets added, then this can replace things like AT_RANDOM.

AT_RANDOM has been around for a long time; it's not something we can
remove. 

> There are non-crypto things out there that will want this.  There are
> also probably VM systems (especially ones that have something like my
> KVM_GET_RNG_SEED patches applied, or many VMs on Haswell, for that
> matter) that will have perfectly fine cryptographically secure urandom
> output immediately after bootup but that won't consider themselves
> "initialized" for a while.  At least these will be perfectly fine from
> the POV of those who trust their hypervisor and Intel :)

If you trust Intel, then you can either use RDRAND directly, or you
can use rngd.  There is also plans to set up an hw_random RDRAND that
can be configured to automatically fill the entropy pool using the new
khwrngd, which could be configured using the appropriate boot options
for those who want to blindly trus their hypervisor and/or Intel.

However, I don't think that should be the default.  And on x86 systems
at least, this is largely a moot point, since the /dev/urandom pool
gets initialized *very* quickly after the system boots.  It's only on
the !@#! ARM systems that don't have a cycle counter, or apparently no
reliable way to make sure the cycle counter is present, which seems to
be the place where we have problems.  But I'd much rather gradually
apply more pressure to the ARM folks so they finally fix their CPU
architecture, and this is one step down that path without forcibly
breaking existing userspace.

						- Ted

Re: [PATCH, RFC] random: introduce getrandom(2) system call

[Andy Lutomirski]

On Thu, Jul 17, 2014 at 2:28 PM, Theodore Ts'o <tytso@mit.edu> wrote:
> On Thu, Jul 17, 2014 at 01:35:15PM -0700, Andy Lutomirski wrote:
>>
>> Can we please have a mode in which getrandom(2) can neither block nor
>> fail?  If that gets added, then this can replace things like AT_RANDOM.
>
> AT_RANDOM has been around for a long time; it's not something we can
> remove.

I'm not suggesting removing AT_RANDOM.  To the contrary, I'm
suggesting that libraries that need to seed some kind of
non-cryptographic, non-security-related RNG could use getrandom(2)
with appropriate flags rather than screwing around with getpid,
clock_gettime, etc.

For example:

unsigned int seed;
getrandom(&seed, sizeof(seed), GRND_BEST_EFFORT); /* Never fails on
new enough kernels */
srand(seed);

No error checking, no weird cases, and if the RNG isn't well seeded,
then I tried my best.

--Andy

Re: [PATCH, RFC] random: introduce getrandom(2) system call

[David Lang]

On Thu, 17 Jul 2014, Andy Lutomirski wrote:

> Can we please have a mode in which getrandom(2) can neither block nor
> fail?  If that gets added, then this can replace things like AT_RANDOM.

If a call to get random data isn't allowed to either block or fail, what is it 
supposed to return if there isn't random data available?

David Lang

Re: [PATCH, RFC] random: introduce getrandom(2) system call

[George Spelvin]

One basic question... why limit this to /dev/random?

If we're trying to avoid fd exhaustion attacks, wouldn't an "atomically
read a file into a buffer" system call (that could be used on
/dev/urandom, or /etc/hostname, or /proc/foo, or...) be more useful?

E.g.

ssize_t readat(int dirfd, char const *path, struct stat *st,
	char *buf, size_t len, int flags);

It's basically equivalent to openat(), optional fstat() (if st is non-NULL),
read(), close(), but it doesn't allocate an fd number.

Is it necessary to have a system call just for entropy?

If you want a "urandom that blocks until seeded", you can always create
another device node for the purpose.

> The main argument I can see for putting in a limit is to encourage the
> "proper" use of the interface.  In practice, anything larger than 128
> probably means the interface is getting misused, either due to a bug
> or some other kind of oversight.

Agreed.  Even 1024 bits is excessive.  32 bytes is the "real" maximum
that people should be asking for with current primitives, so an interface
limitation to 64 is quite defensible.  (But 128 isn't *wildly* excessive.)

If you do stick with a random-specific call, specifying the entropy
in bits (with some specified convention for the last fractional byte)
is anothet interesting idea.  Perhaps too prone to bugs, though.
(People thinking it's bytes and producing low-entropy keys.)

Re: [PATCH, RFC] random: introduce getrandom(2) system call

[George Spelvin]

> In the end people would just recall getentropy in a loop and fetch 256
> bytes each time. I don't think the artificial limit does make any sense.
> I agree that this allows a potential misuse of the interface, but
> doesn't a warning in dmesg suffice?

It makes their code not work, so they can are forced to think about
fixing it before adding the obvious workaround.

> It also makes it easier to port applications from open("/dev/*random"),
> read(...) to getentropy() by reusing the same limits.

But such an application *is broken*.  Making it easier to port is
an anti-goal.  The goal is to make it enough of a hassle that
people will *fix* their code.

There's a *reason* that the /dev/random man page explicitly tells
people not to trust software that reads more than 32 bytes at a time
from /dev/random:

> While some safety margin above that minimum is reasonable, as a guard
> against flaws in the CPRNG algorithm, no cryptographic primitive avail-
> able today can hope to promise more than 256 bits of security, so if
> any program reads more than 256 bits (32 bytes) from the kernel random
> pool per invocation, or per reasonable reseed interval (not less than
> one minute), that should be taken as a sign that its cryptography is
> *not* skillfully implemented.

("not skilfuly implemented" was the phrase chosen after some discussion to
convey "either a quick hack or something you dhouldn't trust.")

To expand on what I said in my mail to Ted, 256 is too high.
I'd go with OpenBSD's 128 bytes or even drop it to 64.

Re: [PATCH, RFC] random: introduce getrandom(2) system call

[Hannes Frederic Sowa]

On So, 2014-07-20 at 13:03 -0400, George Spelvin wrote:
> > In the end people would just recall getentropy in a loop and fetch 256
> > bytes each time. I don't think the artificial limit does make any sense.
> > I agree that this allows a potential misuse of the interface, but
> > doesn't a warning in dmesg suffice?
> 
> It makes their code not work, so they can are forced to think about
> fixing it before adding the obvious workaround.
> 
> > It also makes it easier to port applications from open("/dev/*random"),
> > read(...) to getentropy() by reusing the same limits.
> 
> But such an application *is broken*.  Making it easier to port is
> an anti-goal.  The goal is to make it enough of a hassle that
> people will *fix* their code.
> 
> There's a *reason* that the /dev/random man page explicitly tells
> people not to trust software that reads more than 32 bytes at a time
> from /dev/random:
> 
> > While some safety margin above that minimum is reasonable, as a guard
> > against flaws in the CPRNG algorithm, no cryptographic primitive avail-
> > able today can hope to promise more than 256 bits of security, so if
> > any program reads more than 256 bits (32 bytes) from the kernel random
> > pool per invocation, or per reasonable reseed interval (not less than
> > one minute), that should be taken as a sign that its cryptography is
> > *not* skillfully implemented.
> 
> ("not skilfuly implemented" was the phrase chosen after some discussion to
> convey "either a quick hack or something you dhouldn't trust.")
> 
> To expand on what I said in my mail to Ted, 256 is too high.
> I'd go with OpenBSD's 128 bytes or even drop it to 64.

I don't like partial reads/writes and think that a lot of people get
them wrong, because they often only check for negative return values.

I thought about the following check (as replacement for the old check):

/* we will always generate a partial buffer fill */
if (flags & GRND_RANDOM && count > 512)
	return -EINVAL;

We could also be more conservative and return -EINVAL in case a stray
write happened if one tried to extract less than 512 by checking the
return values of random_read(), but somehow this sounds dangerous to me.

In case of urandom extraction, I wouldn't actually limit the number of
bytes. A lot of applications I have seen already extract more than 128
out of urandom (not for seeding a prng but just to mess around with some
memory). I don't see a reason why getrandom shouldn't be used for that.
It just adds one more thing to look out for if using getrandom() in
urandom mode, especially during porting an application over to this new
interface.

Bye,
Hannes

Re: [PATCH, RFC] random: introduce getrandom(2) system call

[George Spelvin]

> I don't like partial reads/writes and think that a lot of people get
> them wrong, because they often only check for negative return values.

The v1 patch, which did it right IMHO, didn't do partial reads in the
case we're talking about:

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

> In case of urandom extraction, I wouldn't actually limit the number of
> bytes. A lot of applications I have seen already extract more than 128
> out of urandom (not for seeding a prng but just to mess around with some
> memory). I don't see a reason why getrandom shouldn't be used for that.
> It just adds one more thing to look out for if using getrandom() in
> urandom mode, especially during porting an application over to this new
> interface.

Again, I disagree.  If it's "just messing around" code, use /dev/urandom.
It's more portable and you don't care about the fd exhaustion attacks.

If it's actual code to be used in anger, fix it to not abuse /dev/urandom.

You're right that a quick hack might be "broken on purpose", but without
exception, *all* code that I have seen which reads 64 or more bytes from
/dev/*random is broken, and highlighting the brokenness is a highly
desirable thing.

The sole and exclusive reason for this syscall to exist at all is to
solve a security problem.  Supporting broken security code does no favors
to anyone.

Re: [PATCH, RFC] random: introduce getrandom(2) system call

[Hannes Frederic Sowa]

On Mo, 2014-07-21 at 07:21 -0400, George Spelvin wrote:
> > I don't like partial reads/writes and think that a lot of people get
> > them wrong, because they often only check for negative return values.
> 
> The v1 patch, which did it right IMHO, didn't do partial reads in the
> case we're talking about:
> 
> +	if (count > 256)
> +		return -EINVAL;

I checked and unfortunately it does not; 256 bytes is way too large to
guarantee non-existence of partial reads. On a typical older system
without rdrand/rdseed you would have to limit the amount of bytes to
extract to about 32. That's way too low. That said, the 512 bytes check
only in case of extracting bytes from blocking pool would serve no
purpose.

> > In case of urandom extraction, I wouldn't actually limit the number of
> > bytes. A lot of applications I have seen already extract more than 128
> > out of urandom (not for seeding a prng but just to mess around with some
> > memory). I don't see a reason why getrandom shouldn't be used for that.
> > It just adds one more thing to look out for if using getrandom() in
> > urandom mode, especially during porting an application over to this new
> > interface.
> 
> Again, I disagree.  If it's "just messing around" code, use /dev/urandom.
> It's more portable and you don't care about the fd exhaustion attacks.
> 
> If it's actual code to be used in anger, fix it to not abuse /dev/urandom.
> 
> You're right that a quick hack might be "broken on purpose", but without
> exception, *all* code that I have seen which reads 64 or more bytes from
> /dev/*random is broken, and highlighting the brokenness is a highly
> desirable thing.
> 
> The sole and exclusive reason for this syscall to exist at all is to
> solve a security problem.  Supporting broken security code does no favors
> to anyone.

Then let's agree to disagree. :)

I think it is dangerous if application will get ported to this new
interface without checking size limitations and will only notice it
after the applications will be rolled out (if at all).

Bye,
Hannes

Re: [PATCH, RFC] random: introduce getrandom(2) system call

[Hannes Frederic Sowa]

On Mo, 2014-07-21 at 17:27 +0200, Hannes Frederic Sowa wrote:
> On Mo, 2014-07-21 at 07:21 -0400, George Spelvin wrote:
> > > I don't like partial reads/writes and think that a lot of people get
> > > them wrong, because they often only check for negative return values.
> > 
> > The v1 patch, which did it right IMHO, didn't do partial reads in the
> > case we're talking about:
> > 
> > +	if (count > 256)
> > +		return -EINVAL;
> 
> I checked and unfortunately it does not; 256 bytes is way too large to
> guarantee non-existence of partial reads. On a typical older system
> without rdrand/rdseed you would have to limit the amount of bytes to
> extract to about 32. That's way too low. That said, the 512 bytes check
> only in case of extracting bytes from blocking pool would serve no
> purpose.

Ted, would it make sense to specifiy a 512 byte upper bound limit for
random entropy extraction (I am not yet convinced to do that for
urandom) and in case the syscall should block we make sure that we
extract the amount of bytes the user requested?

Having a quick look maybe something like this? Only compile tested and
maybe can still be simplified. It guarantees we don't do a partial write
to user space for sub 512 bytes requests.

diff --git a/drivers/char/random.c b/drivers/char/random.c
index 2721543..c0db6f5 100644
--- a/drivers/char/random.c
+++ b/drivers/char/random.c
@@ -1345,8 +1345,14 @@ static int arch_random_refill(void)
 	return n;
 }
 
+enum blocking_mode {
+	NONBLOCKING,
+	SYSCALL_BLOCK,
+	DEV_RANDOM_BLOCK
+};
+
 static ssize_t
-_random_read(int nonblock, char __user *buf, size_t nbytes)
+_random_read(enum blocking_mode mode, char __user *buf, size_t nbytes)
 {
 	ssize_t n;
 
@@ -1361,7 +1367,7 @@ _random_read(int nonblock, char __user *buf, size_t nbytes)
 		trace_random_read(n*8, (nbytes-n)*8,
 				  ENTROPY_BITS(&blocking_pool),
 				  ENTROPY_BITS(&input_pool));
-		if (n > 0)
+		if (mode != SYSCALL_BLOCK && n > 0)
 			return n;
 
 		/* Pool is (near) empty.  Maybe wait and retry. */
@@ -1370,7 +1376,7 @@ _random_read(int nonblock, char __user *buf, size_t nbytes)
 		if (arch_random_refill())
 			continue;
 
-		if (nonblock)
+		if (mode == NONBLOCKING)
 			return -EAGAIN;
 
 		wait_event_interruptible(random_read_wait,
@@ -1384,7 +1390,8 @@ _random_read(int nonblock, char __user *buf, size_t nbytes)
 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);
+	return _random_read(file->f_flags & O_NONBLOCK ? NONBLOCKING :
+			    DEV_RANDOM_BLOCK, buf, nbytes);
 }
 
 static ssize_t
@@ -1534,8 +1541,6 @@ const struct file_operations urandom_fops = {
 SYSCALL_DEFINE3(getrandom, char __user *, buf, size_t, count,
 		unsigned int, flags)
 {
-	int r;
-
 	if (flags & ~(GRND_NONBLOCK|GRND_RANDOM))
 		return -EINVAL;
 
@@ -1543,7 +1548,8 @@ SYSCALL_DEFINE3(getrandom, char __user *, buf, size_t, count,
 		count = INT_MAX;
 
 	if (flags & GRND_RANDOM)
-		return _random_read(flags & GRND_NONBLOCK, buf, count);
+		return _random_read(flags & GRND_NONBLOCK ? NONBLOCKING :
+				    SYSCALL_BLOCK, buf, count);
 
 	if (unlikely(nonblocking_pool.initialized == 0)) {
 		if (flags & GRND_NONBLOCK)

Bye,
Hannes

Re: [PATCH, RFC] random: introduce getrandom(2) system call

[Theodore Ts'o]

On Tue, Jul 22, 2014 at 03:02:20AM +0200, Hannes Frederic Sowa wrote:
> 
> Ted, would it make sense to specifiy a 512 byte upper bound limit for
> random entropy extraction (I am not yet convinced to do that for
> urandom) and in case the syscall should block we make sure that we
> extract the amount of bytes the user requested?

On some systems, it's still possible that with a large /dev/random
extraction, you could end up blocking for hours.  So either the
getrandom(2) syscall needs to be uninterruptible, which has one set of
problems (and not just the user typing ^C, but also things like being
able to process alarms, which is highly problematic indeed), or you
need to allow it to be interruptible by a signal, in which case
userspace needs to check the error return for things like EINTR
anyway.  And if you have to check the error return, you might as well
check the number of bytes returned.

Yes, one could in theory set up a new variant of "uninterruptible"
signals that only exited if the signal caused the process to exit, and
otherwise, forced a system call restart even if SA_INTERRUPTIBLE was
not set in sigalarim, but that's add *way* more complexity than this
deserves.

Basically, I view /dev/random as an advanced call, and someone who
uses it should know what they are doing.  It's not the default for a
reason.

						- Ted

Re: [PATCH, RFC] random: introduce getrandom(2) system call

[Hannes Frederic Sowa]

Hello,

On Di, 2014-07-22 at 00:44 -0400, Theodore Ts'o wrote:
> On Tue, Jul 22, 2014 at 03:02:20AM +0200, Hannes Frederic Sowa wrote:
> > 
> > Ted, would it make sense to specifiy a 512 byte upper bound limit for
> > random entropy extraction (I am not yet convinced to do that for
> > urandom) and in case the syscall should block we make sure that we
> > extract the amount of bytes the user requested?
> 
> On some systems, it's still possible that with a large /dev/random
> extraction, you could end up blocking for hours.  So either the
> getrandom(2) syscall needs to be uninterruptible, which has one set of
> problems (and not just the user typing ^C, but also things like being
> able to process alarms, which is highly problematic indeed), or you
> need to allow it to be interruptible by a signal, in which case
> userspace needs to check the error return for things like EINTR
> anyway.  And if you have to check the error return, you might as well
> check the number of bytes returned.

I think a lot of checks are of the type "if (getrandom() < 0)", so this
actually was the kind of programming errors I wanted to guard against. 
Also, on some systems it is very likely that we return a short write to
user space, so it prevents this very common situation. Even if one would
like to extract 64 bytes randomness, one would often need two calls to
getrandom() on my virtualized systems. Would be great to know that in
blocking mode, either I get a -1 or the buffer is always filled and I
won't need to do pointer arithmetic to fill the rest of the buffer.

I still think it is a good idea even without caring about the signal
interruptions.

> Yes, one could in theory set up a new variant of "uninterruptible"
> signals that only exited if the signal caused the process to exit, and
> otherwise, forced a system call restart even if SA_INTERRUPTIBLE was
> not set in sigalarim, but that's add *way* more complexity than this
> deserves.

I don't want to do that. It is totally ok if we return -1 and set errno
to EINTR in case of pending signals.

> Basically, I view /dev/random as an advanced call, and someone who
> uses it should know what they are doing.  It's not the default for a
> reason.

I agree, but I still think this would make the interface a bit more
friendly to use.

Thanks,
Hannes

Re: [PATCH, RFC] random: introduce getrandom(2) system call

[Theodore Ts'o]

On Tue, Jul 22, 2014 at 11:49:52AM +0200, Hannes Frederic Sowa wrote:
> 
> I think a lot of checks are of the type "if (getrandom() < 0)", so this
> actually was the kind of programming errors I wanted to guard against. 
> Also, on some systems it is very likely that we return a short write to
> user space, so it prevents this very common situation. Even if one would
> like to extract 64 bytes randomness, one would often need two calls to
> getrandom() on my virtualized systems. Would be great to know that in
> blocking mode, either I get a -1 or the buffer is always filled and I
> won't need to do pointer arithmetic to fill the rest of the buffer.

But why would you need to use GRND_RANDOM in your scenario, and accept
your application potentially getting stalled and stuck in amber for
perhaps hours?  If you are going to accept your application stalling
like that, you can do the pointer arithmatic.  It's really not hard,
and someone who can't do that, again, shouldn't be allowd anywhere
near crypto code in the first place (and if they are, they'll probably
be making lots of other, equally fatal if not more so, newbie
mistakes).

							- Ted

Re: [PATCH, RFC] random: introduce getrandom(2) system call

[Hannes Frederic Sowa]

Hi,

On Wed, Jul 23, 2014, at 00:59, Theodore Ts'o wrote:
> But why would you need to use GRND_RANDOM in your scenario, and accept
> your application potentially getting stalled and stuck in amber for
> perhaps hours?  If you are going to accept your application stalling
> like that, you can do the pointer arithmatic.  It's really not hard,
> and someone who can't do that, again, shouldn't be allowd anywhere
> near crypto code in the first place (and if they are, they'll probably
> be making lots of other, equally fatal if not more so, newbie
> mistakes).

I favored the idea of having a non-failing non-partial-read getrandom
syscall. But I am with you if it often causes long stalls that we should
stick to the old semantics.

Thanks,
Hannes

Re: [PATCH, RFC] random: introduce getrandom(2) system call

[George Spelvin]

I keep wishing for a more general solution.  For example, some way to
have a "spare" extra fd that could be accessed with a special O_NOFAIL
flag.

That would allow any number of library functions to not fail, such as
logging from nasty corner cases.

But you'd have to provide one per thread, and block non-fatal signals
while it was open, so you don't get reentrancy problems.  Ick.


This overly-specialized system call (and worse yet, a blocking
system call that you can't put into a poll() loop) just feels ugly
to me.  Is it *absolutely* necessary?

For example, how about simply making getentropy() a library function that
aborts if it can't open /dev/urandom?  If you're suffering fd exhaustion,
you're being DoSed already.

Re: [PATCH, RFC] random: introduce getrandom(2) system call

[Hannes Frederic Sowa]

On Wed, Jul 23, 2014, at 13:52, George Spelvin wrote:
> I keep wishing for a more general solution.  For example, some way to
> have a "spare" extra fd that could be accessed with a special O_NOFAIL
> flag.
> 
> That would allow any number of library functions to not fail, such as
> logging from nasty corner cases.
> 
> But you'd have to provide one per thread, and block non-fatal signals
> while it was open, so you don't get reentrancy problems.  Ick.
> 
> 
> This overly-specialized system call (and worse yet, a blocking
> system call that you can't put into a poll() loop) just feels ugly
> to me.  Is it *absolutely* necessary?

One point that often came up besides fd exhaustion is missing
/dev/u?random device nodes in chroot environments.

I also thought about a more general interface, like e.g. an
opennod(dev_t device, int flags) call but all those ideas ended up being
very complex changes besides having design issues. getrandom is simple
and solves a real problem.

The only problem I see, that we allow access to /dev/random without
checking any permission bits like we did on opening /dev/random before
and we cannot restrict applications to deplete the whole entropy pool.

> For example, how about simply making getentropy() a library function that
> aborts if it can't open /dev/urandom?  If you're suffering fd exhaustion,
> you're being DoSed already.

Maybe applications want to mitigate fd exhaustion.

Bye,
Hannes

Re: [PATCH, RFC] random: introduce getrandom(2) system call

[Pavel Machek]

On Wed 2014-07-23 14:10:16, Hannes Frederic Sowa wrote:
> 
> 
> On Wed, Jul 23, 2014, at 13:52, George Spelvin wrote:
> > I keep wishing for a more general solution.  For example, some way to
> > have a "spare" extra fd that could be accessed with a special O_NOFAIL
> > flag.
> > 
> > That would allow any number of library functions to not fail, such as
> > logging from nasty corner cases.
> > 
> > But you'd have to provide one per thread, and block non-fatal signals
> > while it was open, so you don't get reentrancy problems.  Ick.
> > 
> > 
> > This overly-specialized system call (and worse yet, a blocking
> > system call that you can't put into a poll() loop) just feels ugly
> > to me.  Is it *absolutely* necessary?
> 
> One point that often came up besides fd exhaustion is missing
> /dev/u?random device nodes in chroot environments.

>From the maillist discussion, it seems you sometimes _want_
/dev/random not to be present.

For example you want to trace exactly the same path through malware
every time.

> > For example, how about simply making getentropy() a library function that
> > aborts if it can't open /dev/urandom?  If you're suffering fd exhaustion,
> > you're being DoSed already.
> 
> Maybe applications want to mitigate fd exhaustion.

Dunno. Will we add special read_passwd() syscall that reads just
/etc/passwd, to allow uid<->name resolution without available FDs?

I like the library function suggestion, this should not need a new
syscall.

And btw -- compatibility with getentropy() is _not_ going to be easy,
if they have different blocking / partial read / signals policy.

									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCH, RFC] random: introduce getrandom(2) system call

[Theodore Ts'o]

On Sun, Jul 20, 2014 at 12:26:22PM -0400, George Spelvin wrote:
> One basic question... why limit this to /dev/random?
> 
> If we're trying to avoid fd exhaustion attacks, wouldn't an "atomically
> read a file into a buffer" system call (that could be used on
> /dev/urandom, or /etc/hostname, or /proc/foo, or...) be more useful?
> 
> E.g.
> 
> ssize_t readat(int dirfd, char const *path, struct stat *st,
> 	char *buf, size_t len, int flags);
> 
> It's basically equivalent to openat(), optional fstat() (if st is non-NULL),
> read(), close(), but it doesn't allocate an fd number.
> 
> Is it necessary to have a system call just for entropy?
> 
> If you want a "urandom that blocks until seeded", you can always create
> another device node for the purpose.

I'd really rather not go down this path.  Your readat(2) proposal is
interesting, but it adds a whole lot of complications.  For example,
just simply booting a new kernel doesn't guarantee that a new device
node for "blocks until seeded" will exist.  So that means a lot of
applications will just either continue to use /dev/urandom, or have to
put in fallback code to first try the new device name, and then fall
back to /dev/urandom.  (And of course, they have to deal with what to
do if /dev/urandom doesn't exist --- which presumably would be
raise(SIGKILL), but we're now talking about a number of lines of codes
that application writers would have to get right.)

Readat(2) would also have to get linked into auditing, and LSM, and
honestly, it's a lot more work that I'm not all that interested in
doing and trying to get right.

> If you do stick with a random-specific call, specifying the entropy
> in bits (with some specified convention for the last fractional byte)
> is anothet interesting idea.  Perhaps too prone to bugs, though.
> (People thinking it's bytes and producing low-entropy keys.)

Definitely not worth the complexity.

							- Ted