Skip to content

Commit e1e6060

Browse files
AlphixNoltari
authored andcommitted
odhcpd: improve odhcpd_urandom()
First, note that not a single caller checks the return value - which is quite reasonable. What are they supposed to do with a failure? Second, none of the callers do anything that's *really* security-sensitive, the closest we have is the force reconf nonce, and that is blorted out over the network, so it's really a best-effort kind of thing. Third, odhcpd_urandom() currently doesn't check if it e.g. got interrupted by a signal. So, simplify and modernize this a bit by using getrandom(), which allows us to skip one fd, and which avoids syscalls by using the vDSO approach instead. Also, check for things like signal interrupts (don't really happen on calls for entropy < 256 bytes, but still). And make a reasonable effort, but not much more. Signed-off-by: David Härdeman <david@hardeman.nu> Link: #285 Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
1 parent c2eb4b5 commit e1e6060

File tree

2 files changed

+27
-9
lines changed

2 files changed

+27
-9
lines changed

src/odhcpd.c

Lines changed: 26 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -39,12 +39,12 @@
3939
#include <sys/types.h>
4040
#include <sys/wait.h>
4141
#include <sys/syscall.h>
42+
#include <sys/random.h>
4243

4344
#include <libubox/uloop.h>
4445
#include "odhcpd.h"
4546

4647
static int ioctl_sock = -1;
47-
static int urandom_fd = -1;
4848

4949
void __iflog(int lvl, const char *fmt, ...)
5050
{
@@ -148,13 +148,9 @@ int main(int argc, char **argv)
148148
}
149149

150150
ioctl_sock = socket(AF_INET, SOCK_DGRAM | SOCK_CLOEXEC, 0);
151-
152151
if (ioctl_sock < 0)
153152
return 4;
154153

155-
if ((urandom_fd = open("/dev/urandom", O_RDONLY | O_CLOEXEC)) < 0)
156-
return 4;
157-
158154
signal(SIGUSR1, SIG_IGN);
159155
signal(SIGINT, sighandler);
160156
signal(SIGTERM, sighandler);
@@ -542,11 +538,33 @@ void odhcpd_process(struct odhcpd_event *event)
542538
odhcpd_receive_packets(&event->uloop, 0);
543539
}
544540

545-
int odhcpd_urandom(void *data, size_t len)
541+
void odhcpd_urandom(void *data, size_t len)
546542
{
547-
return read(urandom_fd, data, len);
548-
}
543+
static bool warned_once = false;
549544

545+
while (true) {
546+
ssize_t r;
547+
548+
if (len == 0)
549+
return;
550+
551+
r = getrandom(data, len, GRND_INSECURE);
552+
if (r < 0) {
553+
if (errno == EINTR)
554+
continue;
555+
556+
if (!warned_once) {
557+
error("getrandom(): %m");
558+
warned_once = true;
559+
}
560+
561+
return;
562+
}
563+
564+
len -= r;
565+
data = (uint8_t *)data + r;
566+
}
567+
}
550568

551569
time_t odhcpd_time(void)
552570
{

src/odhcpd.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -514,7 +514,7 @@ int odhcpd_get_interface_config(const char *ifname, const char *what);
514514
int odhcpd_get_mac(const struct interface *iface, uint8_t mac[6]);
515515
int odhcpd_get_flags(const struct interface *iface);
516516
struct interface* odhcpd_get_interface_by_index(int ifindex);
517-
int odhcpd_urandom(void *data, size_t len);
517+
void odhcpd_urandom(void *data, size_t len);
518518

519519
void odhcpd_run(void);
520520
time_t odhcpd_time(void);

0 commit comments

Comments
 (0)