Source-Changes-HG archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

[src/trunk]: src Tweak logic to decide whether a medium is safe for an rndseed.



details:   https://anonhg.NetBSD.org/src/rev/2bc99acbde60
branches:  trunk
changeset: 1009890:2bc99acbde60
user:      riastradh <riastradh%NetBSD.org@localhost>
date:      Wed May 06 18:49:26 2020 +0000

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

diffstat:

 etc/rc.d/random_seed |   52 ++++++--------
 sbin/rndctl/rndctl.8 |   12 +++-
 sbin/rndctl/rndctl.c |  186 +++++++++++++++++++++++++++++++++-----------------
 3 files changed, 156 insertions(+), 94 deletions(-)

diffs (truncated from 464 to 300 lines):

diff -r 186e64270175 -r 2bc99acbde60 etc/rc.d/random_seed
--- a/etc/rc.d/random_seed      Wed May 06 18:38:20 2020 +0000
+++ b/etc/rc.d/random_seed      Wed May 06 18:49:26 2020 +0000
@@ -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 @@
        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 @@
        # 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 @@
        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
diff -r 186e64270175 -r 2bc99acbde60 sbin/rndctl/rndctl.8
--- a/sbin/rndctl/rndctl.8      Wed May 06 18:38:20 2020 +0000
+++ b/sbin/rndctl/rndctl.8      Wed May 06 18:49:26 2020 +0000
@@ -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 @@
 .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
diff -r 186e64270175 -r 2bc99acbde60 sbin/rndctl/rndctl.c
--- a/sbin/rndctl/rndctl.c      Wed May 06 18:38:20 2020 +0000
+++ b/sbin/rndctl/rndctl.c      Wed May 06 18:49:26 2020 +0000
@@ -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 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 @@
            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 @@
        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 @@
        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 @@
         * 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 @@
        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',
         *



Home | Main Index | Thread Index | Old Index