You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by GitBox <gi...@apache.org> on 2022/03/24 14:53:46 UTC
[GitHub] [incubator-nuttx] xiaoxiang781216 opened a new pull request #5837: libc: Implement getrandom on top of "/dev/[u]random"
xiaoxiang781216 opened a new pull request #5837:
URL: https://github.com/apache/incubator-nuttx/pull/5837
## Summary
- libc: Implement getrandom on top of "/dev/[u]random"
- libc/uuid: Call getrandom instaed arc4random_buf
## Impact
New API
## Testing
Pass CI
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] yamt commented on pull request #5837: libc: Implement getrandom on top of "/dev/[u]random"
Posted by GitBox <gi...@apache.org>.
yamt commented on pull request #5837:
URL: https://github.com/apache/incubator-nuttx/pull/5837#issuecomment-1078705345
ok. thank you for explanation.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on a change in pull request #5837: libc: Implement getrandom on top of "/dev/[u]random"
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on a change in pull request #5837:
URL: https://github.com/apache/incubator-nuttx/pull/5837#discussion_r834450589
##########
File path: libs/libc/misc/lib_getrandom.c
##########
@@ -0,0 +1,91 @@
+/****************************************************************************
+ * libs/libc/misc/lib_getrandom.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership. The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <sys/random.h>
+#include <fcntl.h>
+#include <unistd.h>
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: getrandom
+ *
+ * Description:
+ * Fill a buffer of arbitrary length with randomness. This is the
+ * preferred interface for getting random numbers. The traditional
+ * /dev/random approach is susceptible for things like the attacker
+ * exhausting file descriptors on purpose.
+ *
+ * Note that this function cannot fail, other than by asserting.
+ *
+ * Input Parameters:
+ * bytes - Buffer for returned random bytes
+ * nbytes - Number of bytes requested.
+ * flags - Bit mask that can contain zero or more of the ORed values
+ * together.
+ *
+ * Returned Value:
+ * On success, getrandom() returns the number of bytes that were copied
+ * to the buffer buf. This may be less than the number of bytes
+ * requested via buflen if either GRND_RANDOM was specified in flags and
+ * insufficient entropy was present in the random source or the system
+ * call was interrupted by a signal.
+ *
+ * On error, -1 is returned, and errno is set appropriately.
+ *
+ ****************************************************************************/
+
+ssize_t getrandom(FAR void *bytes, size_t nbytes, unsigned int flags)
+{
+ int oflags = O_RDONLY;
+ FAR const char *dev;
+ int fd;
+
+ if (flags & GRND_NONBLOCK)
+ {
+ oflags |= O_NONBLOCK;
+ }
+
+ if (flags & GRND_RANDOM)
Review comment:
`GRND_RANDOM` -- is marked as `No effect`. Did you mean `GRND_INSECURE`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] yamt commented on pull request #5837: libc: Implement getrandom on top of "/dev/[u]random"
Posted by GitBox <gi...@apache.org>.
yamt commented on pull request #5837:
URL: https://github.com/apache/incubator-nuttx/pull/5837#issuecomment-1078658264
why do you replace arc4random_buf with getrandom?
it seems awkward as one of the purpose of arc4random_buf (renamed from getrandom) was
to avoid descrptor shortage and your getrandom is based on device files.
```
commit 859e1ce63a458b206623d8e41cd93588af61b62f
Author: chao.an <an...@xiaomi.com>
Date: Sat Dec 12 14:16:07 2020 +0800
crypto/arc4random: rename getrandom to arc4random_buf
Change-Id: I5c9f0c9acf5af71f01beceaf06ebe0a2c87676bc
Signed-off-by: chao.an <an...@xiaomi.com>
```
```
commit dffb8a67e3e92500651db3eca516dbcfc275311a
Author: Jussi Kivilinna <ju...@haltian.com>
Date: Thu Mar 30 07:38:37 2017 -0600
Add entropy pool and strong random number generator
Entropy pool gathers environmental noise from device drivers, user-space, et
c., and returns good random numbers, suitable for cryptographic use. Based on en
tropy pool design from *BSDs and uses BLAKE2Xs algorithm for CSPRNG output.
Patch also adds /dev/urandom support for using entropy pool RNG and new 'getrandom' system call for getting randomness without file-descriptor usage (thus avoiding file-descriptor exhaustion attacks). The 'getrandom' interface is similar as 'getentropy' and 'getrandom' available on OpenBSD and Linux respectively.
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] yamt commented on pull request #5837: libc: Implement getrandom on top of "/dev/[u]random"
Posted by GitBox <gi...@apache.org>.
yamt commented on pull request #5837:
URL: https://github.com/apache/incubator-nuttx/pull/5837#issuecomment-1078651938
> > * is this meant for linux compat? why not getentropy, which is accepted by posix?
>
> I thought getentropy also Linux specific before. Anyway, I prefer to support both. The patch will update soon.
>
> > * it seems dangerous to ignore !GRND_INSECURE.
>
> But, GRND_INSECURE doesn't support, the current code is always secure.
by "secure", do you mean CRNG?
(i'm not familiar with nuttx /dev/urandom implementation.)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on a change in pull request #5837: libc: Implement getrandom on top of "/dev/[u]random"
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on a change in pull request #5837:
URL: https://github.com/apache/incubator-nuttx/pull/5837#discussion_r835002330
##########
File path: include/sys/random.h
##########
@@ -0,0 +1,79 @@
+/****************************************************************************
+ * include/sys/random.h
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership. The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+#ifndef __INCLUDE_SYS_RANDOM_H
+#define __INCLUDE_SYS_RANDOM_H
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <stddef.h>
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/* Flags for getrandom(2)
+ *
+ * GRND_NONBLOCK Don't block and return EAGAIN instead
+ * GRND_RANDOM Open /dev/random instead of /dev/urandom
+ * GRND_INSECURE Return non-cryptographic random bytes
+ */
+
+#define GRND_NONBLOCK (1 << 0)
+#define GRND_RANDOM (1 << 1)
+#define GRND_INSECURE (1 << 2)
Review comment:
Sorry I missed that both are pseudo generators
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #5837: libc: Implement getrandom on top of "/dev/[u]random"
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #5837:
URL: https://github.com/apache/incubator-nuttx/pull/5837#discussion_r835019529
##########
File path: libs/libc/unistd/lib_getentropy.c
##########
@@ -0,0 +1,83 @@
+/****************************************************************************
+ * libs/libc/misc/lib_getentropy.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership. The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <sys/random.h>
+#include <errno.h>
+#include <unistd.h>
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: getentropy
+ *
+ * Description:
+ * The getentropy() function writes length bytes of high-quality
+ * random data to the buffer starting at the location pointed to by
+ * buffer. The maximum permitted value for the length argument is
+ * 256.
+ *
+ * A successful call to getentropy() always provides the requested
+ * number of bytes of entropy.
+ *
+ * Input Parameters:
+ * buffer - Buffer for returned random bytes
+ * length - Number of bytes requested.
+ *
+ * Returned Value:
+ * On success, this function returns zero. On error, -1 is
+ * returned, and errno is set to indicate the error.
+ *
+ ****************************************************************************/
+
+int getentropy(FAR void *buffer, size_t length)
+{
+ FAR char *pos = buffer;
+
+ if (length > 256)
+ {
+ errno = EIO;
+ return -1;
+ }
+
+ while (length)
Review comment:
Done.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #5837: libc: Implement getrandom on top of "/dev/[u]random"
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #5837:
URL: https://github.com/apache/incubator-nuttx/pull/5837#discussion_r834479860
##########
File path: libs/libc/misc/lib_getrandom.c
##########
@@ -0,0 +1,91 @@
+/****************************************************************************
+ * libs/libc/misc/lib_getrandom.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership. The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <sys/random.h>
+#include <fcntl.h>
+#include <unistd.h>
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: getrandom
+ *
+ * Description:
+ * Fill a buffer of arbitrary length with randomness. This is the
+ * preferred interface for getting random numbers. The traditional
+ * /dev/random approach is susceptible for things like the attacker
+ * exhausting file descriptors on purpose.
+ *
+ * Note that this function cannot fail, other than by asserting.
+ *
+ * Input Parameters:
+ * bytes - Buffer for returned random bytes
+ * nbytes - Number of bytes requested.
+ * flags - Bit mask that can contain zero or more of the ORed values
+ * together.
+ *
+ * Returned Value:
+ * On success, getrandom() returns the number of bytes that were copied
+ * to the buffer buf. This may be less than the number of bytes
+ * requested via buflen if either GRND_RANDOM was specified in flags and
+ * insufficient entropy was present in the random source or the system
+ * call was interrupted by a signal.
+ *
+ * On error, -1 is returned, and errno is set appropriately.
+ *
+ ****************************************************************************/
+
+ssize_t getrandom(FAR void *bytes, size_t nbytes, unsigned int flags)
+{
+ int oflags = O_RDONLY;
+ FAR const char *dev;
+ int fd;
+
+ if (flags & GRND_NONBLOCK)
+ {
+ oflags |= O_NONBLOCK;
+ }
+
+ if (flags & GRND_RANDOM)
+ {
+ dev = "/dev/random";
+ }
+ else
+ {
+ dev = "dev/urandom";
Review comment:
Done.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 edited a comment on pull request #5837: libc: Implement getrandom on top of "/dev/[u]random"
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 edited a comment on pull request #5837:
URL: https://github.com/apache/incubator-nuttx/pull/5837#issuecomment-1078693797
BTW, from the manual:
https://www.freebsd.org/cgi/man.cgi?query=arc4random_buf&sektion=3&n=1
https://man7.org/linux/man-pages/man2/getrandom.2.html
https://man7.org/linux/man-pages/man3/getentropy.3.html
arc4random_buf is different from getrandom/getentropy. The second is designed return the true random number from hardware, but the first is a pseudo random number generator which's seed may come from getentropy, which also mention in the manual too:
The subsystem is re-seeded from the kernel random(4) subsystem using getentropy(3) on a regular basis, and also upon fork(2).
So the better design is that:
1. Decouple the random number generator interface from file handle as mention before
2. Add getrandom syscall to remove the file system interaction with item 1
3. Move random number pool from crypto to libc, so we can have arc4random_buf
4. Implement the software based /dev/[u]random by item 2 and the random event
But, this is a huge work and need the dedicated resource to finish.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #5837: libc: Implement getrandom on top of "/dev/[u]random"
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #5837:
URL: https://github.com/apache/incubator-nuttx/pull/5837#discussion_r834485253
##########
File path: include/sys/random.h
##########
@@ -0,0 +1,79 @@
+/****************************************************************************
+ * include/sys/random.h
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership. The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+#ifndef __INCLUDE_SYS_RANDOM_H
+#define __INCLUDE_SYS_RANDOM_H
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <stddef.h>
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/* Flags for getrandom(2)
+ *
+ * GRND_NONBLOCK Don't block and return EAGAIN instead
+ * GRND_RANDOM No effect
+ * GRND_INSECURE Return non-cryptographic random bytes
+ */
+
+#define GRND_NONBLOCK 0x0001
+#define GRND_RANDOM 0x0002
+#define GRND_INSECURE 0x0004
Review comment:
Done.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on a change in pull request #5837: libc: Implement getrandom on top of "/dev/[u]random"
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on a change in pull request #5837:
URL: https://github.com/apache/incubator-nuttx/pull/5837#discussion_r835118528
##########
File path: libs/libc/uuid/lib_uuid_create.c
##########
@@ -22,9 +22,40 @@
* Included Files
****************************************************************************/
+#include <sys/random.h>
+#include <errno.h>
#include <stdlib.h>
#include <uuid.h>
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+static int uuid_getrandom(FAR void *buf, size_t size, int flags)
+{
+ FAR char *tmp = buf;
+ ssize_t ret;
+
+ while (size > 0)
+ {
+ ssize_t ret = getrandom(tmp, size, flags);
Review comment:
If so, then `FAR char *pos = buffer;` and `FAR char *tmp = buf;` should be moved :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 edited a comment on pull request #5837: libc: Implement getrandom on top of "/dev/[u]random"
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 edited a comment on pull request #5837:
URL: https://github.com/apache/incubator-nuttx/pull/5837#issuecomment-1078688420
> why do you replace arc4random_buf with getrandom?
One problem is that arc4random_buf couple with the random pool and only exist when CONFIG_DEV_URANDOM_RANDOM_POOL is defined, but this is just one of three software algos:
https://github.com/apache/incubator-nuttx/blob/master/drivers/crypto/Kconfig#L30-L73
The more important problem is that if SOC has the true random number generator(DEV_URANDOM_ARCH), the software based random generator drives(drivers/crypto/dev_urandom.c) will be replaced by the hardware implementation, which mean that:
1. arc4random_buf doesn't exist anymore
2. user has to open /dev/[u]random by self
>
> it seems awkward as one of the purpose of arc4random_buf (renamed from getrandom) was to avoid descrptor shortage and your getrandom is based on device files.
It's an implementation detail, we can change it later. It's always better than the current situation(spread /dev/[u]random access in the whole code base) as I highlight before.
Why I can't remove fd in this patch directly? This is because the random number generator drivers always register /dev/[u]random directly which mean there isn't no other no-file based approach to access the hardware. So, the block issue is to refine the random number driver interface to remove the depend on file handle directly, the rest work is very easy:
1. Add syscall to expose getrandom to userspace directly
2. Add a char driver to expose /dev/[u]random
But, this PR can simplify the refinement later.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #5837: libc: Implement getrandom on top of "/dev/[u]random"
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #5837:
URL: https://github.com/apache/incubator-nuttx/pull/5837#issuecomment-1078735114
> > * is this meant for linux compat? why not getentropy, which is accepted by posix?
> > * it seems dangerous to ignore !GRND_INSECURE.
>
> BTW I failed to find `getentropy` in the latest `unistd.h` description in opengroups. I think adding of it is still a proposal only
Yes, I think so. But, it's general function implemented by many Unix like OS, so it's better to support it too.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #5837: libc: Implement getrandom on top of "/dev/[u]random"
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #5837:
URL: https://github.com/apache/incubator-nuttx/pull/5837#issuecomment-1078693797
BTW, from the manual:
https://www.freebsd.org/cgi/man.cgi?query=arc4random_buf&sektion=3&n=1
https://man7.org/linux/man-pages/man2/getrandom.2.html
https://man7.org/linux/man-pages/man3/getentropy.3.html
arc4random_buf is different from getrandom/getentropy. The second is designed return the true random number from hardware. arc4random_buf is a pseudo random number generator.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on pull request #5837: libc: Implement getrandom on top of "/dev/[u]random"
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on pull request #5837:
URL: https://github.com/apache/incubator-nuttx/pull/5837#issuecomment-1078732764
> * is this meant for linux compat? why not getentropy, which is accepted by posix?
> * it seems dangerous to ignore !GRND_INSECURE.
BTW I failed to find `getentropy` in the latest `unistd.h` description in opengroups. I think adding of it is still a proposal only
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #5837: libc: Implement getrandom on top of "/dev/[u]random"
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #5837:
URL: https://github.com/apache/incubator-nuttx/pull/5837#discussion_r834482992
##########
File path: libs/libc/uuid/lib_uuid_create.c
##########
@@ -42,25 +43,31 @@
void uuid_create(uuid_t *u, uint32_t *status)
{
-#ifdef CONFIG_CRYPTO_RANDOM_POOL
- arc4random_buf(u, sizeof(uuid_t));
-#else
unsigned long *beg = (unsigned long *)u;
unsigned long *end = (unsigned long *)(u + 1);
+ ssize_t ret;
- while (beg < end)
+ ret = getrandom(u, sizeof(uuid_t), 0);
+ if (ret < 0)
{
- *beg++ = rand();
+ ret = getrandom(u, sizeof(uuid_t), GRND_RANDOM);
}
-#endif
- u->clock_seq_hi_and_reserved &= ~(1 << 6);
- u->clock_seq_hi_and_reserved |= (1 << 7);
+ if (ret != sizeof(uuid_t))
+ {
+ while (beg < end)
+ {
+ *beg++ = rand();
+ }
+
+ u->clock_seq_hi_and_reserved &= ~(1 << 6);
+ u->clock_seq_hi_and_reserved |= (1 << 7);
- u->time_hi_and_version &= ~(1 << 12);
- u->time_hi_and_version &= ~(1 << 13);
- u->time_hi_and_version |= (1 << 14);
- u->time_hi_and_version &= ~(1 << 15);
+ u->time_hi_and_version &= ~(1 << 12);
+ u->time_hi_and_version &= ~(1 << 13);
+ u->time_hi_and_version |= (1 << 14);
+ u->time_hi_and_version &= ~(1 << 15);
Review comment:
Done.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on a change in pull request #5837: libc: Implement getrandom on top of "/dev/[u]random"
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on a change in pull request #5837:
URL: https://github.com/apache/incubator-nuttx/pull/5837#discussion_r835139779
##########
File path: libs/libc/uuid/lib_uuid_create.c
##########
@@ -22,9 +22,40 @@
* Included Files
****************************************************************************/
+#include <sys/random.h>
+#include <errno.h>
#include <stdlib.h>
#include <uuid.h>
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+static int uuid_getrandom(FAR void *buf, size_t size, int flags)
+{
+ FAR char *tmp = buf;
+ ssize_t ret;
+
+ while (size > 0)
+ {
+ ssize_t ret = getrandom(tmp, size, flags);
Review comment:
yes, missed that
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] gustavonihei commented on a change in pull request #5837: libc: Implement getrandom on top of "/dev/[u]random"
Posted by GitBox <gi...@apache.org>.
gustavonihei commented on a change in pull request #5837:
URL: https://github.com/apache/incubator-nuttx/pull/5837#discussion_r834560415
##########
File path: libs/libc/misc/lib_getrandom.c
##########
@@ -0,0 +1,91 @@
+/****************************************************************************
+ * libs/libc/misc/lib_getrandom.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership. The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <sys/random.h>
+#include <fcntl.h>
+#include <unistd.h>
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: getrandom
+ *
+ * Description:
+ * Fill a buffer of arbitrary length with randomness. This is the
+ * preferred interface for getting random numbers. The traditional
+ * /dev/random approach is susceptible for things like the attacker
+ * exhausting file descriptors on purpose.
+ *
+ * Note that this function cannot fail, other than by asserting.
+ *
+ * Input Parameters:
+ * bytes - Buffer for returned random bytes
+ * nbytes - Number of bytes requested.
+ * flags - Bit mask that can contain zero or more of the ORed values
+ * together.
+ *
+ * Returned Value:
+ * On success, getrandom() returns the number of bytes that were copied
+ * to the buffer buf. This may be less than the number of bytes
+ * requested via buflen if either GRND_RANDOM was specified in flags and
+ * insufficient entropy was present in the random source or the system
+ * call was interrupted by a signal.
Review comment:
```suggestion
* On success, getrandom() returns the number of bytes that were copied
* to the buffer bytes. This may be less than the number of bytes
* requested via nbytes if either GRND_RANDOM was specified in flags and
* insufficient entropy was present in the random source or the system
* call was interrupted by a signal.
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #5837: libc: Implement getrandom on top of "/dev/[u]random"
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #5837:
URL: https://github.com/apache/incubator-nuttx/pull/5837#discussion_r834480277
##########
File path: libs/libc/misc/lib_getrandom.c
##########
@@ -0,0 +1,91 @@
+/****************************************************************************
+ * libs/libc/misc/lib_getrandom.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership. The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <sys/random.h>
+#include <fcntl.h>
+#include <unistd.h>
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: getrandom
+ *
+ * Description:
+ * Fill a buffer of arbitrary length with randomness. This is the
+ * preferred interface for getting random numbers. The traditional
+ * /dev/random approach is susceptible for things like the attacker
+ * exhausting file descriptors on purpose.
+ *
+ * Note that this function cannot fail, other than by asserting.
+ *
+ * Input Parameters:
+ * bytes - Buffer for returned random bytes
+ * nbytes - Number of bytes requested.
+ * flags - Bit mask that can contain zero or more of the ORed values
+ * together.
+ *
+ * Returned Value:
+ * On success, getrandom() returns the number of bytes that were copied
+ * to the buffer buf. This may be less than the number of bytes
+ * requested via buflen if either GRND_RANDOM was specified in flags and
+ * insufficient entropy was present in the random source or the system
+ * call was interrupted by a signal.
+ *
+ * On error, -1 is returned, and errno is set appropriately.
+ *
+ ****************************************************************************/
+
+ssize_t getrandom(FAR void *bytes, size_t nbytes, unsigned int flags)
+{
+ int oflags = O_RDONLY;
+ FAR const char *dev;
+ int fd;
+
+ if (flags & GRND_NONBLOCK)
Review comment:
Done.
##########
File path: libs/libc/misc/lib_getrandom.c
##########
@@ -0,0 +1,91 @@
+/****************************************************************************
+ * libs/libc/misc/lib_getrandom.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership. The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <sys/random.h>
+#include <fcntl.h>
+#include <unistd.h>
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: getrandom
+ *
+ * Description:
+ * Fill a buffer of arbitrary length with randomness. This is the
+ * preferred interface for getting random numbers. The traditional
+ * /dev/random approach is susceptible for things like the attacker
+ * exhausting file descriptors on purpose.
+ *
+ * Note that this function cannot fail, other than by asserting.
+ *
+ * Input Parameters:
+ * bytes - Buffer for returned random bytes
+ * nbytes - Number of bytes requested.
+ * flags - Bit mask that can contain zero or more of the ORed values
+ * together.
+ *
+ * Returned Value:
+ * On success, getrandom() returns the number of bytes that were copied
+ * to the buffer buf. This may be less than the number of bytes
+ * requested via buflen if either GRND_RANDOM was specified in flags and
+ * insufficient entropy was present in the random source or the system
+ * call was interrupted by a signal.
+ *
+ * On error, -1 is returned, and errno is set appropriately.
+ *
+ ****************************************************************************/
+
+ssize_t getrandom(FAR void *bytes, size_t nbytes, unsigned int flags)
+{
+ int oflags = O_RDONLY;
+ FAR const char *dev;
+ int fd;
+
+ if (flags & GRND_NONBLOCK)
+ {
+ oflags |= O_NONBLOCK;
+ }
+
+ if (flags & GRND_RANDOM)
Review comment:
Done.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #5837: libc: Implement getrandom on top of "/dev/[u]random"
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #5837:
URL: https://github.com/apache/incubator-nuttx/pull/5837#issuecomment-1078607419
> * is this meant for linux compat? why not getentropy, which is accepted by posix?
I thought getentropy also Linux specific before. Anyway, I prefer to support both. The patch will update soon.
> * it seems dangerous to ignore !GRND_INSECURE.
But, GRND_INSECURE doesn't support, the current code is always secure.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #5837: libc: Implement getrandom on top of "/dev/[u]random"
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #5837:
URL: https://github.com/apache/incubator-nuttx/pull/5837#discussion_r835111644
##########
File path: libs/libc/uuid/lib_uuid_create.c
##########
@@ -22,9 +22,40 @@
* Included Files
****************************************************************************/
+#include <sys/random.h>
+#include <errno.h>
#include <stdlib.h>
#include <uuid.h>
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+static int uuid_getrandom(FAR void *buf, size_t size, int flags)
+{
+ FAR char *tmp = buf;
+ ssize_t ret;
+
+ while (size > 0)
+ {
+ ssize_t ret = getrandom(tmp, size, flags);
Review comment:
I move the line 37 instead.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #5837: libc: Implement getrandom on top of "/dev/[u]random"
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #5837:
URL: https://github.com/apache/incubator-nuttx/pull/5837#issuecomment-1078875416
> @xiaoxiang781216 do you know why CI is not started?
I don't know why, let's me push again.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #5837: libc: Implement getrandom on top of "/dev/[u]random"
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #5837:
URL: https://github.com/apache/incubator-nuttx/pull/5837#discussion_r834590574
##########
File path: libs/libc/misc/lib_getrandom.c
##########
@@ -0,0 +1,91 @@
+/****************************************************************************
+ * libs/libc/misc/lib_getrandom.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership. The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <sys/random.h>
+#include <fcntl.h>
+#include <unistd.h>
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: getrandom
+ *
+ * Description:
+ * Fill a buffer of arbitrary length with randomness. This is the
+ * preferred interface for getting random numbers. The traditional
+ * /dev/random approach is susceptible for things like the attacker
+ * exhausting file descriptors on purpose.
+ *
+ * Note that this function cannot fail, other than by asserting.
+ *
+ * Input Parameters:
+ * bytes - Buffer for returned random bytes
+ * nbytes - Number of bytes requested.
+ * flags - Bit mask that can contain zero or more of the ORed values
+ * together.
+ *
+ * Returned Value:
+ * On success, getrandom() returns the number of bytes that were copied
+ * to the buffer buf. This may be less than the number of bytes
+ * requested via buflen if either GRND_RANDOM was specified in flags and
+ * insufficient entropy was present in the random source or the system
+ * call was interrupted by a signal.
Review comment:
Done.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on a change in pull request #5837: libc: Implement getrandom on top of "/dev/[u]random"
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on a change in pull request #5837:
URL: https://github.com/apache/incubator-nuttx/pull/5837#discussion_r834959115
##########
File path: libs/libc/uuid/lib_uuid_create.c
##########
@@ -42,17 +43,24 @@
void uuid_create(uuid_t *u, uint32_t *status)
{
-#ifdef CONFIG_CRYPTO_RANDOM_POOL
- arc4random_buf(u, sizeof(uuid_t));
-#else
- unsigned long *beg = (unsigned long *)u;
- unsigned long *end = (unsigned long *)(u + 1);
+ ssize_t ret;
- while (beg < end)
+ ret = getrandom(u, sizeof(uuid_t), 0);
+ if (ret < 0)
{
- *beg++ = rand();
+ ret = getrandom(u, sizeof(uuid_t), GRND_RANDOM);
Review comment:
Do we need to check the EINT and repeat again in case if call was interrupted by a signal?
##########
File path: libs/libc/unistd/lib_getentropy.c
##########
@@ -0,0 +1,83 @@
+/****************************************************************************
+ * libs/libc/misc/lib_getentropy.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership. The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <sys/random.h>
+#include <errno.h>
+#include <unistd.h>
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: getentropy
+ *
+ * Description:
+ * The getentropy() function writes length bytes of high-quality
+ * random data to the buffer starting at the location pointed to by
+ * buffer. The maximum permitted value for the length argument is
+ * 256.
+ *
+ * A successful call to getentropy() always provides the requested
+ * number of bytes of entropy.
+ *
+ * Input Parameters:
+ * buffer - Buffer for returned random bytes
+ * length - Number of bytes requested.
+ *
+ * Returned Value:
+ * On success, this function returns zero. On error, -1 is
+ * returned, and errno is set to indicate the error.
+ *
+ ****************************************************************************/
+
+int getentropy(FAR void *buffer, size_t length)
+{
+ FAR char *pos = buffer;
+
+ if (length > 256)
+ {
+ errno = EIO;
+ return -1;
+ }
+
+ while (length)
Review comment:
```suggestion
while (length > 0)
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #5837: libc: Implement getrandom on top of "/dev/[u]random"
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #5837:
URL: https://github.com/apache/incubator-nuttx/pull/5837#issuecomment-1078688420
> why do you replace arc4random_buf with getrandom?
One problem is that arc4random_buf couple with the random pool and only exist when CONFIG_DEV_URANDOM_RANDOM_POOL is defined, but this is just one of three software algos:
https://github.com/apache/incubator-nuttx/blob/master/drivers/crypto/Kconfig#L30-L73
The more important problem is that if SOC has the true random number generator(DEV_URANDOM_ARCH), the software based random generator drives(drivers/crypto/dev_urandom.c) will be replaced by the hardware implementation, which mean that:
1. arc4random_buf doesn't exist anymore
2. user has to open /dev/[u]random by self
>
> it seems awkward as one of the purpose of arc4random_buf (renamed from getrandom) was to avoid descrptor shortage and your getrandom is based on device files.
It's an implementation detail, we can change it later. It's always better than the current situation(spread /dev/[u]random access in the whole code base) as I highlight before.
Why I can't remove fd in this patch directly? This is because the random number generator drivers always register /dev/[u]random directly which mean there isn't no other no-file based approach to access the hardware. So, the block issue is to refine the random number driver interface to remove the depend on file handle directly, the rest work is very easy:
1. Add syscall to expose getrandom to userspace directly
2. Add a char driver to expose /dev/[u]random
This PR can simplify the refinement later.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on a change in pull request #5837: libc: Implement getrandom on top of "/dev/[u]random"
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on a change in pull request #5837:
URL: https://github.com/apache/incubator-nuttx/pull/5837#discussion_r835118528
##########
File path: libs/libc/uuid/lib_uuid_create.c
##########
@@ -22,9 +22,40 @@
* Included Files
****************************************************************************/
+#include <sys/random.h>
+#include <errno.h>
#include <stdlib.h>
#include <uuid.h>
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+static int uuid_getrandom(FAR void *buf, size_t size, int flags)
+{
+ FAR char *tmp = buf;
+ ssize_t ret;
+
+ while (size > 0)
+ {
+ ssize_t ret = getrandom(tmp, size, flags);
Review comment:
If so, then `FAR char *pos = buffer;` should be moved :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #5837: libc: Implement getrandom on top of "/dev/[u]random"
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #5837:
URL: https://github.com/apache/incubator-nuttx/pull/5837#discussion_r835136744
##########
File path: libs/libc/uuid/lib_uuid_create.c
##########
@@ -22,9 +22,40 @@
* Included Files
****************************************************************************/
+#include <sys/random.h>
+#include <errno.h>
#include <stdlib.h>
#include <uuid.h>
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+static int uuid_getrandom(FAR void *buf, size_t size, int flags)
+{
+ FAR char *tmp = buf;
+ ssize_t ret;
+
+ while (size > 0)
+ {
+ ssize_t ret = getrandom(tmp, size, flags);
Review comment:
No, we can't since tmp/buf need keep the last iterator to make the next loop fill the next part of buffer.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on a change in pull request #5837: libc: Implement getrandom on top of "/dev/[u]random"
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on a change in pull request #5837:
URL: https://github.com/apache/incubator-nuttx/pull/5837#discussion_r834434602
##########
File path: libs/libc/misc/lib_getrandom.c
##########
@@ -0,0 +1,91 @@
+/****************************************************************************
+ * libs/libc/misc/lib_getrandom.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership. The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <sys/random.h>
+#include <fcntl.h>
+#include <unistd.h>
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: getrandom
+ *
+ * Description:
+ * Fill a buffer of arbitrary length with randomness. This is the
+ * preferred interface for getting random numbers. The traditional
+ * /dev/random approach is susceptible for things like the attacker
+ * exhausting file descriptors on purpose.
+ *
+ * Note that this function cannot fail, other than by asserting.
+ *
+ * Input Parameters:
+ * bytes - Buffer for returned random bytes
+ * nbytes - Number of bytes requested.
+ * flags - Bit mask that can contain zero or more of the ORed values
+ * together.
+ *
+ * Returned Value:
+ * On success, getrandom() returns the number of bytes that were copied
+ * to the buffer buf. This may be less than the number of bytes
+ * requested via buflen if either GRND_RANDOM was specified in flags and
+ * insufficient entropy was present in the random source or the system
+ * call was interrupted by a signal.
+ *
+ * On error, -1 is returned, and errno is set appropriately.
+ *
+ ****************************************************************************/
+
+ssize_t getrandom(FAR void *bytes, size_t nbytes, unsigned int flags)
+{
+ int oflags = O_RDONLY;
+ FAR const char *dev;
+ int fd;
+
+ if (flags & GRND_NONBLOCK)
Review comment:
```suggestion
if ((flags & GRND_NONBLOCK) != 0)
```
##########
File path: libs/libc/misc/lib_getrandom.c
##########
@@ -0,0 +1,91 @@
+/****************************************************************************
+ * libs/libc/misc/lib_getrandom.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership. The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <sys/random.h>
+#include <fcntl.h>
+#include <unistd.h>
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: getrandom
+ *
+ * Description:
+ * Fill a buffer of arbitrary length with randomness. This is the
+ * preferred interface for getting random numbers. The traditional
+ * /dev/random approach is susceptible for things like the attacker
+ * exhausting file descriptors on purpose.
+ *
+ * Note that this function cannot fail, other than by asserting.
+ *
+ * Input Parameters:
+ * bytes - Buffer for returned random bytes
+ * nbytes - Number of bytes requested.
+ * flags - Bit mask that can contain zero or more of the ORed values
+ * together.
+ *
+ * Returned Value:
+ * On success, getrandom() returns the number of bytes that were copied
+ * to the buffer buf. This may be less than the number of bytes
+ * requested via buflen if either GRND_RANDOM was specified in flags and
+ * insufficient entropy was present in the random source or the system
+ * call was interrupted by a signal.
+ *
+ * On error, -1 is returned, and errno is set appropriately.
+ *
+ ****************************************************************************/
+
+ssize_t getrandom(FAR void *bytes, size_t nbytes, unsigned int flags)
+{
+ int oflags = O_RDONLY;
+ FAR const char *dev;
+ int fd;
+
+ if (flags & GRND_NONBLOCK)
+ {
+ oflags |= O_NONBLOCK;
+ }
+
+ if (flags & GRND_RANDOM)
Review comment:
```suggestion
if ((flags & GRND_RANDOM) != 0)
```
##########
File path: include/sys/random.h
##########
@@ -0,0 +1,79 @@
+/****************************************************************************
+ * include/sys/random.h
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership. The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+#ifndef __INCLUDE_SYS_RANDOM_H
+#define __INCLUDE_SYS_RANDOM_H
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <stddef.h>
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/* Flags for getrandom(2)
+ *
+ * GRND_NONBLOCK Don't block and return EAGAIN instead
+ * GRND_RANDOM No effect
+ * GRND_INSECURE Return non-cryptographic random bytes
+ */
+
+#define GRND_NONBLOCK 0x0001
+#define GRND_RANDOM 0x0002
+#define GRND_INSECURE 0x0004
Review comment:
```suggestion
#define GRND_NONBLOCK (1 << 0)
#define GRND_RANDOM (1 << 1)
#define GRND_INSECURE (1 << 2)
```
##########
File path: libs/libc/uuid/lib_uuid_create.c
##########
@@ -42,25 +43,31 @@
void uuid_create(uuid_t *u, uint32_t *status)
{
-#ifdef CONFIG_CRYPTO_RANDOM_POOL
- arc4random_buf(u, sizeof(uuid_t));
-#else
unsigned long *beg = (unsigned long *)u;
unsigned long *end = (unsigned long *)(u + 1);
+ ssize_t ret;
- while (beg < end)
+ ret = getrandom(u, sizeof(uuid_t), 0);
+ if (ret < 0)
{
- *beg++ = rand();
+ ret = getrandom(u, sizeof(uuid_t), GRND_RANDOM);
}
-#endif
- u->clock_seq_hi_and_reserved &= ~(1 << 6);
- u->clock_seq_hi_and_reserved |= (1 << 7);
+ if (ret != sizeof(uuid_t))
+ {
+ while (beg < end)
+ {
+ *beg++ = rand();
+ }
+
+ u->clock_seq_hi_and_reserved &= ~(1 << 6);
+ u->clock_seq_hi_and_reserved |= (1 << 7);
- u->time_hi_and_version &= ~(1 << 12);
- u->time_hi_and_version &= ~(1 << 13);
- u->time_hi_and_version |= (1 << 14);
- u->time_hi_and_version &= ~(1 << 15);
+ u->time_hi_and_version &= ~(1 << 12);
+ u->time_hi_and_version &= ~(1 << 13);
+ u->time_hi_and_version |= (1 << 14);
+ u->time_hi_and_version &= ~(1 << 15);
Review comment:
I think this should be out of `if`.
##########
File path: libs/libc/misc/lib_getrandom.c
##########
@@ -0,0 +1,91 @@
+/****************************************************************************
+ * libs/libc/misc/lib_getrandom.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership. The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <sys/random.h>
+#include <fcntl.h>
+#include <unistd.h>
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: getrandom
+ *
+ * Description:
+ * Fill a buffer of arbitrary length with randomness. This is the
+ * preferred interface for getting random numbers. The traditional
+ * /dev/random approach is susceptible for things like the attacker
+ * exhausting file descriptors on purpose.
+ *
+ * Note that this function cannot fail, other than by asserting.
+ *
+ * Input Parameters:
+ * bytes - Buffer for returned random bytes
+ * nbytes - Number of bytes requested.
+ * flags - Bit mask that can contain zero or more of the ORed values
+ * together.
+ *
+ * Returned Value:
+ * On success, getrandom() returns the number of bytes that were copied
+ * to the buffer buf. This may be less than the number of bytes
+ * requested via buflen if either GRND_RANDOM was specified in flags and
+ * insufficient entropy was present in the random source or the system
+ * call was interrupted by a signal.
+ *
+ * On error, -1 is returned, and errno is set appropriately.
+ *
+ ****************************************************************************/
+
+ssize_t getrandom(FAR void *bytes, size_t nbytes, unsigned int flags)
+{
+ int oflags = O_RDONLY;
+ FAR const char *dev;
+ int fd;
+
+ if (flags & GRND_NONBLOCK)
+ {
+ oflags |= O_NONBLOCK;
+ }
+
+ if (flags & GRND_RANDOM)
+ {
+ dev = "/dev/random";
+ }
+ else
+ {
+ dev = "dev/urandom";
Review comment:
```suggestion
dev = "/dev/urandom";
```
##########
File path: libs/libc/uuid/lib_uuid_create.c
##########
@@ -42,25 +43,31 @@
void uuid_create(uuid_t *u, uint32_t *status)
{
-#ifdef CONFIG_CRYPTO_RANDOM_POOL
- arc4random_buf(u, sizeof(uuid_t));
-#else
unsigned long *beg = (unsigned long *)u;
unsigned long *end = (unsigned long *)(u + 1);
+ ssize_t ret;
- while (beg < end)
+ ret = getrandom(u, sizeof(uuid_t), 0);
+ if (ret < 0)
{
- *beg++ = rand();
+ ret = getrandom(u, sizeof(uuid_t), GRND_RANDOM);
}
-#endif
- u->clock_seq_hi_and_reserved &= ~(1 << 6);
- u->clock_seq_hi_and_reserved |= (1 << 7);
+ if (ret != sizeof(uuid_t))
+ {
+ while (beg < end)
Review comment:
I think we can move
```
unsigned long *beg = (unsigned long *)u;
unsigned long *end = (unsigned long *)(u + 1);
```
inside the `if`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on pull request #5837: libc: Implement getrandom on top of "/dev/[u]random"
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on pull request #5837:
URL: https://github.com/apache/incubator-nuttx/pull/5837#issuecomment-1078609547
I think the good way is to support both as `getentropy` has a limitation of no more than 256 bytes buffer length, so may be still good for uuid, but we might avoid adding a loop of 256 byte chunks in cases with large buffers
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 edited a comment on pull request #5837: libc: Implement getrandom on top of "/dev/[u]random"
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 edited a comment on pull request #5837:
URL: https://github.com/apache/incubator-nuttx/pull/5837#issuecomment-1078693797
BTW, from the manual:
https://www.freebsd.org/cgi/man.cgi?query=arc4random_buf&sektion=3&n=1
https://man7.org/linux/man-pages/man2/getrandom.2.html
https://man7.org/linux/man-pages/man3/getentropy.3.html
arc4random_buf is different from getrandom/getentropy. The second is designed return the true random number from hardware, but the first is a pseudo random number generator which's seed may come from getentropy, which also mention in the manual too:
The subsystem is re-seeded from the kernel random(4) subsystem using getentropy(3) on a regular basis, and also upon fork(2).
So the better design is that:
1. Decouple the random number generator interface from file handle as mention before
2. Move random number pool from crypto to libc, so we can have arc4random_buf
3. Implement the software based /dev/[u]random by item 2 and the random event
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #5837: libc: Implement getrandom on top of "/dev/[u]random"
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #5837:
URL: https://github.com/apache/incubator-nuttx/pull/5837#discussion_r835026236
##########
File path: libs/libc/uuid/lib_uuid_create.c
##########
@@ -42,17 +43,24 @@
void uuid_create(uuid_t *u, uint32_t *status)
{
-#ifdef CONFIG_CRYPTO_RANDOM_POOL
- arc4random_buf(u, sizeof(uuid_t));
-#else
- unsigned long *beg = (unsigned long *)u;
- unsigned long *end = (unsigned long *)(u + 1);
+ ssize_t ret;
- while (beg < end)
+ ret = getrandom(u, sizeof(uuid_t), 0);
+ if (ret < 0)
{
- *beg++ = rand();
+ ret = getrandom(u, sizeof(uuid_t), GRND_RANDOM);
Review comment:
Done.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #5837: libc: Implement getrandom on top of "/dev/[u]random"
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #5837:
URL: https://github.com/apache/incubator-nuttx/pull/5837#discussion_r834605056
##########
File path: include/sys/random.h
##########
@@ -0,0 +1,79 @@
+/****************************************************************************
+ * include/sys/random.h
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership. The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+#ifndef __INCLUDE_SYS_RANDOM_H
+#define __INCLUDE_SYS_RANDOM_H
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <stddef.h>
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/* Flags for getrandom(2)
+ *
+ * GRND_NONBLOCK Don't block and return EAGAIN instead
+ * GRND_RANDOM Open /dev/random instead of /dev/urandom
+ * GRND_INSECURE Return non-cryptographic random bytes
+ */
+
+#define GRND_NONBLOCK (1 << 0)
+#define GRND_RANDOM (1 << 1)
+#define GRND_INSECURE (1 << 2)
Review comment:
URANDOM_RANDOM_POOL is still a pseudo random number generator(one of three software algo):
https://github.com/apache/incubator-nuttx/blob/master/drivers/crypto/Kconfig#L30-L73
Only DEV_URANDOM_ARCH can generate the real random number, but in this case the random number driver under arch/ will register "/dev/urandom" instead of drivers/crypto/dev_urandom.c.
So, you can consider the implementation here is always unsecure regardless which algo is selected by user.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko merged pull request #5837: libc: Implement getrandom on top of "/dev/[u]random"
Posted by GitBox <gi...@apache.org>.
pkarashchenko merged pull request #5837:
URL: https://github.com/apache/incubator-nuttx/pull/5837
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on a change in pull request #5837: libc: Implement getrandom on top of "/dev/[u]random"
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on a change in pull request #5837:
URL: https://github.com/apache/incubator-nuttx/pull/5837#discussion_r834489067
##########
File path: libs/libc/uuid/lib_uuid_create.c
##########
@@ -42,20 +43,27 @@
void uuid_create(uuid_t *u, uint32_t *status)
{
-#ifdef CONFIG_CRYPTO_RANDOM_POOL
- arc4random_buf(u, sizeof(uuid_t));
-#else
- unsigned long *beg = (unsigned long *)u;
- unsigned long *end = (unsigned long *)(u + 1);
+ ssize_t ret;
- while (beg < end)
+ ret = getrandom(u, sizeof(uuid_t), 0);
+ if (ret < 0)
{
- *beg++ = rand();
+ ret = getrandom(u, sizeof(uuid_t), GRND_RANDOM);
}
-#endif
- u->clock_seq_hi_and_reserved &= ~(1 << 6);
- u->clock_seq_hi_and_reserved |= (1 << 7);
+ if (ret != sizeof(uuid_t))
+ {
+ unsigned long *beg = (unsigned long *)u;
+ unsigned long *end = (unsigned long *)(u + 1);
+
+ while (beg < end)
+ {
+ *beg++ = rand();
+ }
+
+ u->clock_seq_hi_and_reserved &= ~(1 << 6);
+ u->clock_seq_hi_and_reserved |= (1 << 7);
Review comment:
please move this out of `if`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] gustavonihei commented on a change in pull request #5837: libc: Implement getrandom on top of "/dev/[u]random"
Posted by GitBox <gi...@apache.org>.
gustavonihei commented on a change in pull request #5837:
URL: https://github.com/apache/incubator-nuttx/pull/5837#discussion_r834558642
##########
File path: include/sys/random.h
##########
@@ -0,0 +1,79 @@
+/****************************************************************************
+ * include/sys/random.h
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership. The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+#ifndef __INCLUDE_SYS_RANDOM_H
+#define __INCLUDE_SYS_RANDOM_H
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <stddef.h>
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/* Flags for getrandom(2)
+ *
+ * GRND_NONBLOCK Don't block and return EAGAIN instead
+ * GRND_RANDOM Open /dev/random instead of /dev/urandom
+ * GRND_INSECURE Return non-cryptographic random bytes
+ */
+
+#define GRND_NONBLOCK (1 << 0)
+#define GRND_RANDOM (1 << 1)
+#define GRND_INSECURE (1 << 2)
+
+/****************************************************************************
+ * Public Function Prototypes
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: getrandom
+ *
+ * Description:
+ * Fill a buffer of arbitrary length with randomness. This is the
+ * preferred interface for getting random numbers. The traditional
+ * /dev/random approach is susceptible for things like the attacker
+ * exhausting file descriptors on purpose.
+ *
+ * Note that this function cannot fail, other than by asserting.
+ *
+ * Input Parameters:
+ * bytes - Buffer for returned random bytes
+ * nbytes - Number of bytes requested.
+ * flags - Bit mask that can contain zero or more of the ORed values
+ * together.
+ *
+ * Returned Value:
+ * On success, getrandom() returns the number of bytes that were copied
+ * to the buffer buf. This may be less than the number of bytes
+ * requested via buflen if either GRND_RANDOM was specified in flags and
+ * insufficient entropy was present in the random source or the system
+ * call was interrupted by a signal.
Review comment:
```suggestion
* On success, getrandom() returns the number of bytes that were copied
* to the buffer bytes. This may be less than the number of bytes
* requested via nbytes if either GRND_RANDOM was specified in flags and
* insufficient entropy was present in the random source or the system
* call was interrupted by a signal.
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on a change in pull request #5837: libc: Implement getrandom on top of "/dev/[u]random"
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on a change in pull request #5837:
URL: https://github.com/apache/incubator-nuttx/pull/5837#discussion_r835071903
##########
File path: libs/libc/uuid/lib_uuid_create.c
##########
@@ -22,9 +22,40 @@
* Included Files
****************************************************************************/
+#include <sys/random.h>
+#include <errno.h>
#include <stdlib.h>
#include <uuid.h>
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+static int uuid_getrandom(FAR void *buf, size_t size, int flags)
+{
+ FAR char *tmp = buf;
+ ssize_t ret;
+
+ while (size > 0)
+ {
+ ssize_t ret = getrandom(tmp, size, flags);
Review comment:
```suggestion
ret = getrandom(tmp, size, flags);
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #5837: libc: Implement getrandom on top of "/dev/[u]random"
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #5837:
URL: https://github.com/apache/incubator-nuttx/pull/5837#discussion_r834483165
##########
File path: libs/libc/uuid/lib_uuid_create.c
##########
@@ -42,25 +43,31 @@
void uuid_create(uuid_t *u, uint32_t *status)
{
-#ifdef CONFIG_CRYPTO_RANDOM_POOL
- arc4random_buf(u, sizeof(uuid_t));
-#else
unsigned long *beg = (unsigned long *)u;
unsigned long *end = (unsigned long *)(u + 1);
+ ssize_t ret;
- while (beg < end)
+ ret = getrandom(u, sizeof(uuid_t), 0);
+ if (ret < 0)
{
- *beg++ = rand();
+ ret = getrandom(u, sizeof(uuid_t), GRND_RANDOM);
}
-#endif
- u->clock_seq_hi_and_reserved &= ~(1 << 6);
- u->clock_seq_hi_and_reserved |= (1 << 7);
+ if (ret != sizeof(uuid_t))
+ {
+ while (beg < end)
Review comment:
Done.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #5837: libc: Implement getrandom on top of "/dev/[u]random"
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #5837:
URL: https://github.com/apache/incubator-nuttx/pull/5837#discussion_r834590146
##########
File path: include/sys/random.h
##########
@@ -0,0 +1,79 @@
+/****************************************************************************
+ * include/sys/random.h
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership. The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+#ifndef __INCLUDE_SYS_RANDOM_H
+#define __INCLUDE_SYS_RANDOM_H
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <stddef.h>
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/* Flags for getrandom(2)
+ *
+ * GRND_NONBLOCK Don't block and return EAGAIN instead
+ * GRND_RANDOM Open /dev/random instead of /dev/urandom
+ * GRND_INSECURE Return non-cryptographic random bytes
+ */
+
+#define GRND_NONBLOCK (1 << 0)
+#define GRND_RANDOM (1 << 1)
+#define GRND_INSECURE (1 << 2)
+
+/****************************************************************************
+ * Public Function Prototypes
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: getrandom
+ *
+ * Description:
+ * Fill a buffer of arbitrary length with randomness. This is the
+ * preferred interface for getting random numbers. The traditional
+ * /dev/random approach is susceptible for things like the attacker
+ * exhausting file descriptors on purpose.
+ *
+ * Note that this function cannot fail, other than by asserting.
+ *
+ * Input Parameters:
+ * bytes - Buffer for returned random bytes
+ * nbytes - Number of bytes requested.
+ * flags - Bit mask that can contain zero or more of the ORed values
+ * together.
+ *
+ * Returned Value:
+ * On success, getrandom() returns the number of bytes that were copied
+ * to the buffer buf. This may be less than the number of bytes
+ * requested via buflen if either GRND_RANDOM was specified in flags and
+ * insufficient entropy was present in the random source or the system
+ * call was interrupted by a signal.
Review comment:
Done.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] yamt commented on pull request #5837: libc: Implement getrandom on top of "/dev/[u]random"
Posted by GitBox <gi...@apache.org>.
yamt commented on pull request #5837:
URL: https://github.com/apache/incubator-nuttx/pull/5837#issuecomment-1078600313
* is this meant for linux compat? why not getentropy, which is accepted by posix?
* it seems dangerous to ignore !GRND_INSECURE.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on a change in pull request #5837: libc: Implement getrandom on top of "/dev/[u]random"
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on a change in pull request #5837:
URL: https://github.com/apache/incubator-nuttx/pull/5837#discussion_r835118804
##########
File path: libs/libc/uuid/lib_uuid_create.c
##########
@@ -22,9 +22,40 @@
* Included Files
****************************************************************************/
+#include <sys/random.h>
+#include <errno.h>
#include <stdlib.h>
#include <uuid.h>
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+static int uuid_getrandom(FAR void *buf, size_t size, int flags)
+{
+ FAR char *tmp = buf;
+ ssize_t ret;
+
+ while (size > 0)
+ {
+ ssize_t ret = getrandom(tmp, size, flags);
Review comment:
we can leave as it is
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] gustavonihei commented on a change in pull request #5837: libc: Implement getrandom on top of "/dev/[u]random"
Posted by GitBox <gi...@apache.org>.
gustavonihei commented on a change in pull request #5837:
URL: https://github.com/apache/incubator-nuttx/pull/5837#discussion_r834558642
##########
File path: include/sys/random.h
##########
@@ -0,0 +1,79 @@
+/****************************************************************************
+ * include/sys/random.h
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership. The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+#ifndef __INCLUDE_SYS_RANDOM_H
+#define __INCLUDE_SYS_RANDOM_H
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <stddef.h>
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/* Flags for getrandom(2)
+ *
+ * GRND_NONBLOCK Don't block and return EAGAIN instead
+ * GRND_RANDOM Open /dev/random instead of /dev/urandom
+ * GRND_INSECURE Return non-cryptographic random bytes
+ */
+
+#define GRND_NONBLOCK (1 << 0)
+#define GRND_RANDOM (1 << 1)
+#define GRND_INSECURE (1 << 2)
+
+/****************************************************************************
+ * Public Function Prototypes
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: getrandom
+ *
+ * Description:
+ * Fill a buffer of arbitrary length with randomness. This is the
+ * preferred interface for getting random numbers. The traditional
+ * /dev/random approach is susceptible for things like the attacker
+ * exhausting file descriptors on purpose.
+ *
+ * Note that this function cannot fail, other than by asserting.
+ *
+ * Input Parameters:
+ * bytes - Buffer for returned random bytes
+ * nbytes - Number of bytes requested.
+ * flags - Bit mask that can contain zero or more of the ORed values
+ * together.
+ *
+ * Returned Value:
+ * On success, getrandom() returns the number of bytes that were copied
+ * to the buffer buf. This may be less than the number of bytes
+ * requested via buflen if either GRND_RANDOM was specified in flags and
+ * insufficient entropy was present in the random source or the system
+ * call was interrupted by a signal.
Review comment:
```suggestion
* On success, getrandom() returns the number of bytes that were copied
* to the buffer bytes. This may be less than the number of bytes
* requested via nbytes if either GRND_RANDOM was specified in flags and
* insufficient entropy was present in the random source or the system
* call was interrupted by a signal.
```
We need to either sync our documentation with the function, or sync the function parameters with the Linux API.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on a change in pull request #5837: libc: Implement getrandom on top of "/dev/[u]random"
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on a change in pull request #5837:
URL: https://github.com/apache/incubator-nuttx/pull/5837#discussion_r834502554
##########
File path: include/sys/random.h
##########
@@ -0,0 +1,79 @@
+/****************************************************************************
+ * include/sys/random.h
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership. The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+#ifndef __INCLUDE_SYS_RANDOM_H
+#define __INCLUDE_SYS_RANDOM_H
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <stddef.h>
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/* Flags for getrandom(2)
+ *
+ * GRND_NONBLOCK Don't block and return EAGAIN instead
+ * GRND_RANDOM Open /dev/random instead of /dev/urandom
+ * GRND_INSECURE Return non-cryptographic random bytes
+ */
+
+#define GRND_NONBLOCK (1 << 0)
+#define GRND_RANDOM (1 << 1)
+#define GRND_INSECURE (1 << 2)
Review comment:
I have a question of this flag.
I see next code in `/dev/urandom` driver:
```
static ssize_t devurand_read(FAR struct file *filep, FAR char *buffer,
size_t len)
{
#ifdef CONFIG_DEV_URANDOM_RANDOM_POOL
if (len > 0)
{
arc4random_buf(buffer, len);
}
#else
size_t n;
uint32_t rnd;
n = len;
```
so the question is: Maybe we can follow-up with the change and map `GRND_INSECURE` to `O_BINARY` for example and implement `devurand_read()` is the next way:
```
static ssize_t devurand_read(FAR struct file *filep, FAR char *buffer,
size_t len)
{
#ifdef CONFIG_DEV_URANDOM_RANDOM_POOL
if ((filep->f_oflags & O_BINARY) == 0)
{
if (len > 0)
{
arc4random_buf(buffer, len);
}
}
else
#endif
{
size_t n;
uint32_t rnd;
n = len;
...
}
```
what do you think?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #5837: libc: Implement getrandom on top of "/dev/[u]random"
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #5837:
URL: https://github.com/apache/incubator-nuttx/pull/5837#discussion_r834498581
##########
File path: libs/libc/uuid/lib_uuid_create.c
##########
@@ -42,20 +43,27 @@
void uuid_create(uuid_t *u, uint32_t *status)
{
-#ifdef CONFIG_CRYPTO_RANDOM_POOL
- arc4random_buf(u, sizeof(uuid_t));
-#else
- unsigned long *beg = (unsigned long *)u;
- unsigned long *end = (unsigned long *)(u + 1);
+ ssize_t ret;
- while (beg < end)
+ ret = getrandom(u, sizeof(uuid_t), 0);
+ if (ret < 0)
{
- *beg++ = rand();
+ ret = getrandom(u, sizeof(uuid_t), GRND_RANDOM);
}
-#endif
- u->clock_seq_hi_and_reserved &= ~(1 << 6);
- u->clock_seq_hi_and_reserved |= (1 << 7);
+ if (ret != sizeof(uuid_t))
+ {
+ unsigned long *beg = (unsigned long *)u;
+ unsigned long *end = (unsigned long *)(u + 1);
+
+ while (beg < end)
+ {
+ *beg++ = rand();
+ }
+
+ u->clock_seq_hi_and_reserved &= ~(1 << 6);
+ u->clock_seq_hi_and_reserved |= (1 << 7);
Review comment:
Done.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org