Module Name: src Committed By: riastradh Date: Wed May 6 18:49:26 UTC 2020
Modified Files: src/etc/rc.d: random_seed src/sbin/rndctl: rndctl.8 rndctl.c Log Message: Tweak logic to decide whether a medium is safe for an rndseed. - Teach rndctl to load the seed, but treat it as zero entropy, if the medium is read-only or if the update fails. - Teach rndctl to accept `-i' flag instructing it to ignore the entropy estimate in the seed. - Teach /etc/rc.d/random_seed to: (a) assume nonlocal file systems are unsafe, and use -i, but (b) assume / is safe, even if it is nonlocal. If the medium is nonwritable, leave it to rndctl to detect that. (Could use statvfs and check for ST_LOCAL in rndctl, I guess, but I already implemented it this way.) Treating nonlocal / as safe is a compromise: it's up to the operator to secure the network for (e.g.) nfs mounts, but that's true whether we're talking entropy or not -- if the adversary has access to the network that you've mounted / from, they can do a lot more damage anyway; this reduces warning fatigue for diskless systems, e.g. test racks. To generate a diff of this commit: cvs rdiff -u -r1.9 -r1.10 src/etc/rc.d/random_seed cvs rdiff -u -r1.23 -r1.24 src/sbin/rndctl/rndctl.8 cvs rdiff -u -r1.33 -r1.34 src/sbin/rndctl/rndctl.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files.
Modified files: Index: src/etc/rc.d/random_seed diff -u src/etc/rc.d/random_seed:1.9 src/etc/rc.d/random_seed:1.10 --- src/etc/rc.d/random_seed:1.9 Fri May 1 15:52:38 2020 +++ src/etc/rc.d/random_seed Wed May 6 18:49:26 2020 @@ -1,6 +1,6 @@ #!/bin/sh # -# $NetBSD: random_seed,v 1.9 2020/05/01 15:52:38 riastradh Exp $ +# $NetBSD: random_seed,v 1.10 2020/05/06 18:49:26 riastradh Exp $ # # PROVIDE: random_seed @@ -29,43 +29,40 @@ message() echo "${name}: ${random_file}: $@" 1>&2 } -getfstype() { - df -G "$1" | while read line; do - set -- $line - if [ "$2" = "fstype" ]; then - echo "$1" - return - fi - done -} - fs_safe() { - # - # Enforce that the file's on a local file system. - # Include only the types we can actually write. - # - fstype="$(getfstype "$1")" - case "${fstype}" in - ffs|lfs|ext2fs|msdos|v7fs|zfs) - return 0 + # Consider the root file system safe always. + df -P "$1" | (while read dev total used avail cap mountpoint; do + case $mountpoint in + 'Mounted on') continue;; + /) exit 0;; + *) exit 1;; + esac + done) && return 0 + + # Otherwise, consider local file systems safe and non-local + # file systems unsafe. + case $(df -l "$1") in + *Warning:*) + return 1 ;; *) - message "Bad file system type ${fstype}" - return 1 + return 0 ;; esac } random_load() { + local flags= + if [ ! -f "${random_file}" ]; then message "Not present" return fi if ! fs_safe "$(dirname "${random_file}")"; then - return 1 + flags=-i fi set -- $(ls -ldn "${random_file}") @@ -75,15 +72,15 @@ random_load() # The file must be owned by root, if [ "$st_uid" != "0" ]; then message "Bad owner ${st_uid}" - return 1 + flags=-i fi # and root read/write only. if [ "$st_mode" != "-rw-------" ]; then message "Bad mode ${st_mode}" - return 1 + flags=-i fi - if rndctl -L "${random_file}"; then + if rndctl $flags -L "${random_file}"; then echo "Loaded entropy from ${random_file}." fi } @@ -93,11 +90,6 @@ random_save() oum="$(umask)" umask 077 - if ! fs_safe "$(dirname "${random_file}")"; then - umask "${oum}" - return 1 - fi - if rndctl -S "${random_file}"; then echo "Saved entropy to ${random_file}." fi Index: src/sbin/rndctl/rndctl.8 diff -u src/sbin/rndctl/rndctl.8:1.23 src/sbin/rndctl/rndctl.8:1.24 --- src/sbin/rndctl/rndctl.8:1.23 Fri Dec 6 14:43:18 2019 +++ src/sbin/rndctl/rndctl.8 Wed May 6 18:49:26 2020 @@ -1,4 +1,4 @@ -.\" $NetBSD: rndctl.8,v 1.23 2019/12/06 14:43:18 riastradh Exp $ +.\" $NetBSD: rndctl.8,v 1.24 2020/05/06 18:49:26 riastradh Exp $ .\" .\" Copyright (c) 1997 Michael Graff .\" All rights reserved. @@ -76,6 +76,16 @@ but no entropy is assumed to be present. .It Fl e Enable entropy estimation using the collected timing information for the given device name or device type. +.It Fl i +With the +.Fl L +option to load a seed from a file, ignore any estimate in the file of +the entropy of the seed. +This still loads the data into the kernel, but won't unblock +.Pa /dev/random +even if the file claims to have adequate entropy. +This is useful if the file is on a medium, such as an NFS share, that +the operator does not know to be secret. .It Fl L Load saved entropy from file .Ar save-file Index: src/sbin/rndctl/rndctl.c diff -u src/sbin/rndctl/rndctl.c:1.33 src/sbin/rndctl/rndctl.c:1.34 --- src/sbin/rndctl/rndctl.c:1.33 Thu Apr 30 03:27:15 2020 +++ src/sbin/rndctl/rndctl.c Wed May 6 18:49:26 2020 @@ -1,4 +1,4 @@ -/* $NetBSD: rndctl.c,v 1.33 2020/04/30 03:27:15 riastradh Exp $ */ +/* $NetBSD: rndctl.c,v 1.34 2020/05/06 18:49:26 riastradh Exp $ */ /*- * Copyright (c) 1997 Michael Graff. @@ -31,7 +31,7 @@ #include <sys/cdefs.h> #ifndef lint -__RCSID("$NetBSD: rndctl.c,v 1.33 2020/04/30 03:27:15 riastradh Exp $"); +__RCSID("$NetBSD: rndctl.c,v 1.34 2020/05/06 18:49:26 riastradh Exp $"); #endif #include <sys/param.h> @@ -78,6 +78,7 @@ static char * strflags(u_int32_t); static void do_list(int, u_int32_t, char *); static void do_stats(void); +static int iflag; static int vflag; static void @@ -88,7 +89,8 @@ usage(void) getprogname()); fprintf(stderr, " %s [-lsv] [-d devname | -t devtype]\n", getprogname()); - fprintf(stderr, " %s -[L|S] save-file\n", getprogname()); + fprintf(stderr, " %s [-i] -L save-file\n", getprogname()); + fprintf(stderr, " %s -S save-file\n", getprogname()); exit(1); } @@ -126,8 +128,8 @@ find_name(u_int32_t type) return ("???"); } -static void -do_save(const char *filename, const void *extra, size_t nextra, +static int +update_seed(const char *filename, const void *extra, size_t nextra, uint32_t extraentropy) { char tmp[PATH_MAX]; @@ -143,23 +145,30 @@ do_save(const char *filename, const void memset(&rs, 0, sizeof rs); /* Format the temporary file name. */ - if (snprintf(tmp, sizeof tmp, "%s.tmp", filename) >= PATH_MAX) - errx(1, "path too long"); + if (snprintf(tmp, sizeof tmp, "%s.tmp", filename) >= PATH_MAX) { + warnx("path too long"); + return -1; + } /* Open /dev/urandom. */ - if ((fd = open(_PATH_URANDOM, O_RDONLY)) == -1) - err(1, "device open"); + if ((fd = open(_PATH_URANDOM, O_RDONLY)) == -1) { + warn("device open"); + return -1; + } /* Find how much entropy is in the pool. */ - if (ioctl(fd, RNDGETENTCNT, &systementropy) == -1) - err(1, "ioctl(RNDGETENTCNT)"); + if (ioctl(fd, RNDGETENTCNT, &systementropy) == -1) { + warn("ioctl(RNDGETENTCNT)"); + systementropy = 0; + } /* Read some data from /dev/urandom. */ if ((size_t)(nread = read(fd, buf, sizeof buf)) != sizeof buf) { if (nread == -1) - err(1, "read"); + warn("read"); else - errx(1, "truncated read"); + warnx("truncated read"); + return -1; } /* Close /dev/urandom; we're done with it. */ @@ -212,30 +221,47 @@ do_save(const char *filename, const void * begin with in which case we're hosed either way, or we've * just revealed some output which is not a problem. */ - if ((fd = open(tmp, O_CREAT|O_TRUNC|O_WRONLY, 0600)) == -1) - err(1, "open seed file to save"); + if ((fd = open(tmp, O_CREAT|O_TRUNC|O_WRONLY, 0600)) == -1) { + warn("open seed file to save"); + return -1; + } if ((size_t)(nwrit = write(fd, &rs, sizeof rs)) != sizeof rs) { int error = errno; if (unlink(tmp) == -1) warn("unlink"); if (nwrit == -1) - errc(1, error, "write"); + warnc(error, "write"); else - errx(1, "truncated write"); + warnx("truncated write"); + return -1; } explicit_memset(&rs, 0, sizeof rs); /* paranoia */ if (fsync_range(fd, FDATASYNC|FDISKSYNC, 0, 0) == -1) { int error = errno; if (unlink(tmp) == -1) warn("unlink"); - errc(1, error, "fsync_range"); + warnc(error, "fsync_range"); + return -1; } if (close(fd) == -1) warn("close"); /* Rename it over the original file to commit. */ - if (rename(tmp, filename) == -1) - err(1, "rename"); + if (rename(tmp, filename) == -1) { + warn("rename"); + return -1; + } + + /* Success! */ + return 0; +} + +static void +do_save(const char *filename) +{ + + if (update_seed(filename, NULL, 0, 0) == -1) + exit(1); } static void @@ -248,42 +274,52 @@ do_load(const char *filename) ssize_t nread, nwrit; SHA1_CTX s; uint8_t digest[SHA1_DIGEST_LENGTH]; + int ro = 0, fail = 0; + int error; /* * The order of operations is important here: * * 1. Load the old seed. - * 2. Feed the old seed into the kernel. - * 3. Generate and write a new seed. - * 4. Erase the old seed. + * 2. Generate and write a new seed. + * => If writing the new seed fails, assume the medium was + * read-only and we might be reading the same thing on + * multiple boots, so treat the entropy estimate as zero. + * 3. Feed the old seed into the kernel. + * 4. Erase the old seed if we can. * - * This follows the procedure in + * We used to follow the similar procedure in * * Niels Ferguson, Bruce Schneier, and Tadayoshi Kohno, * _Cryptography Engineering_, Wiley, 2010, Sec. 9.6.2 - * `Update Seed File'. + * `Update Seed File', * - * There is a race condition: If another process generates a - * key from /dev/urandom after step (2) but before step (3), - * and if the machine crashes before step (3), an adversary who - * can read the disk after the crash can probably guess the - * complete state of the entropy pool and thereby predict the - * key. + * which is different mainly in that it does step (3) before + * step (2). We don't need to feed the old seed into the + * kernel first, because we hash the old seed together with + * /dev/urandom output. * - * There's not much we can do here without some kind of - * systemwide lock on /dev/urandom and without introducing an - * opportunity for a crash to wipe out the entropy altogether. - * To avoid this race, you should ensure that any key - * generation happens _after_ `rndctl -L' has completed. + * Writing the new seed to disk before feeding the old seed to + * the kernel lets us notice if the disk is read-only, and if + * so, zero the estimate of entropy in case we're getting the + * same seed each time we boot. */ /* Format the temporary file name. */ if (snprintf(tmp, sizeof tmp, "%s.tmp", filename) >= PATH_MAX) errx(1, "path too long"); - /* 1. Load the old seed. */ - if ((fd_seed = open(filename, O_RDWR)) == -1) - err(1, "open seed file to load"); + /* + * 1. Load the old seed. + */ + if ((fd_seed = open(filename, O_RDWR)) == -1) { + error = errno; + if ((error != EPERM && error != EROFS) || + (fd_seed = open(filename, O_RDONLY)) == -1) + err(1, "open seed file to load"); + warnc(error, "seed file is nonwritable"); + ro = 1; + } if ((size_t)(nread = read(fd_seed, &rs, sizeof rs)) != sizeof rs) { if (nread == -1) err(1, "read seed"); @@ -318,46 +354,61 @@ do_load(const char *filename) rs.entropy = 0; } - /* Format the ioctl request. */ + /* + * If the user asked or if the medium can't be updated, zero + * the entropy estimate. + */ + if (iflag || ro) + rs.entropy = 0; + + /* + * 2. Generate and write a new seed. If anything goes wrong, + * assume the medium is read-only and treat it as zero entropy. + */ + if (update_seed(filename, rs.data, sizeof(rs.data), rs.entropy) == -1) + rs.entropy = 0; + + /* Decide whether to fail at the end if there's no entropy. */ + fail = (rs.entropy == 0); + + /* + * 3. Feed the old seed into the kernel. + */ rd.len = MIN(sizeof(rd.data), sizeof(rs.data)); rd.entropy = rs.entropy; memcpy(rd.data, rs.data, rd.len); explicit_memset(&rs, 0, sizeof rs); /* paranoia */ - - /* 2. Feed the old seed into the kernel. */ if ((fd_random = open(_PATH_URANDOM, O_WRONLY)) == -1) err(1, "open /dev/urandom"); if (ioctl(fd_random, RNDADDDATA, &rd) == -1) err(1, "RNDADDDATA"); + explicit_memset(&rd, 0, sizeof rd); /* paranoia */ if (close(fd_random) == -1) warn("close /dev/urandom"); fd_random = -1; /* paranoia */ /* - * 3. Generate and write a new seed. Note that we hash the old - * seed together with whatever /dev/urandom returns in do_save. - * Why? After RNDADDDATA, the input may not be distributed - * immediately to /dev/urandom. - */ - do_save(filename, rd.data, rd.len, rd.entropy); - explicit_memset(&rd, 0, sizeof rd); /* paranoia */ - - /* * 4. Erase the old seed. Only effective if we're on a * fixed-address file system like ffs -- doesn't help to erase * the data on lfs, but doesn't hurt either. No need to unlink - * because do_save will have already overwritten it. + * because do_save will have already renamed over it. */ - memset(&rs, 0, sizeof rs); - if ((size_t)(nwrit = pwrite(fd_seed, &rs, sizeof rs, 0)) != - sizeof rs) { - if (nwrit == -1) - err(1, "overwrite old seed"); - else - errx(1, "truncated overwrite"); + if (!ro) { + memset(&rs, 0, sizeof rs); + if ((size_t)(nwrit = pwrite(fd_seed, &rs, sizeof rs, 0)) != + sizeof rs) { + if (nwrit == -1) + err(1, "overwrite old seed"); + else + errx(1, "truncated overwrite"); + } + if (fsync_range(fd_seed, FDATASYNC|FDISKSYNC, 0, 0) == -1) + err(1, "fsync_range"); } - if (fsync_range(fd_seed, FDATASYNC|FDISKSYNC, 0, 0) == -1) - err(1, "fsync_range"); + + /* Fail noisily if there's no entropy. */ + if (fail) + errx(1, "no entropy"); } static void @@ -535,7 +586,7 @@ main(int argc, char **argv) sflag = 0; type = 0xff; - while ((ch = getopt(argc, argv, "CES:L:celt:d:sv")) != -1) { + while ((ch = getopt(argc, argv, "CES:L:celit:d:sv")) != -1) { switch (ch) { case 'C': rctl.flags |= RND_FLAG_NO_COLLECT; @@ -569,6 +620,9 @@ main(int argc, char **argv) rctl.mask |= RND_FLAG_NO_ESTIMATE; mflag++; break; + case 'i': + iflag = 1; + break; case 'l': lflag++; break; @@ -608,10 +662,16 @@ main(int argc, char **argv) usage(); /* + * -i makes sense only with -L. + */ + if (iflag && cmd != 'L') + usage(); + + /* * Save. */ if (cmd == 'S') { - do_save(filename, NULL, 0, 0); + do_save(filename); exit(0); }