[src/trunk]: src/sys/dev Fix a longstanding bug in key-handling for the blowf...

[dan <dan%NetBSD.org@localhost>]

details:   https://anonhg.NetBSD.org/src/rev/bce6703e8c39
branches:  trunk
changeset: 559472:bce6703e8c39
user:      dan <dan%NetBSD.org@localhost>
date:      Thu Mar 18 10:42:08 2004 +0000

description:
Fix a longstanding bug in key-handling for the blowfish cipher.

This is an incompatible change, and will break all existing cgd images
encrypted with blowfish. Users will need to dump their data before
booting a kernel with this change, and recreate cgd's and restore data
afterwards.

I believe this affects a very small number of users other than myself;
indeed after several alert mails in an attempt to find them, only 2
such users have come forward. They have both agreed the requirement
for backwards compatibility does not warrant the effort nor the mess
in the code.  This code does exist, if it should later prove to be
needed, but will not be in the tree.

Further, by the nature of the issue, I have strong reasons to believe
that, even if they missed these mails, there would be few other users
of blowfish who update their systems with any regularity; any such
users would have tripped over the problem in the same way I did when
it was first found over a year ago.

The problem stems from two issues with the underlying blowfish
encryption routines used by cgd:
 - they take key length arguments counted in bytes, rather than bits
   like all the opther ciphers.
 - they silently truncate any keys longer than an internal limit,
   rather than returning an error (which would have exposed the
   previous discrepancy immediately).

As a result, the kernel reads too much data as the key from cgdconfig,
and then truncates most of it. This can easily be demonstrated/tested.
Currently, Blowfish users will find that if they mis-enter the cgd
passphrase on the first attempt, when validation fails and cgdconfig
prompts for the passphrase again, the cgd will not correctly configure
even when given a correct passphrase.

diffstat:

 sys/dev/cgd.c        |  10 ++++++----
 sys/dev/cgd_crypto.c |   8 ++++----
 2 files changed, 10 insertions(+), 8 deletions(-)

diffs (78 lines):

diff -r 3d73b6f712b1 -r bce6703e8c39 sys/dev/cgd.c
--- a/sys/dev/cgd.c     Thu Mar 18 08:52:04 2004 +0000
+++ b/sys/dev/cgd.c     Thu Mar 18 10:42:08 2004 +0000
@@ -1,4 +1,4 @@
-/* $NetBSD: cgd.c,v 1.14 2004/01/25 18:06:48 hannken Exp $ */
+/* $NetBSD: cgd.c,v 1.15 2004/03/18 10:42:08 dan Exp $ */
 
 /*-
  * Copyright (c) 2002 The NetBSD Foundation, Inc.
@@ -37,7 +37,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: cgd.c,v 1.14 2004/01/25 18:06:48 hannken Exp $");
+__KERNEL_RCSID(0, "$NetBSD: cgd.c,v 1.15 2004/03/18 10:42:08 dan Exp $");
 
 #include <sys/types.h>
 #include <sys/param.h>
@@ -484,6 +484,7 @@
        struct   cgd_ioctl *ci = data;
        struct   vnode *vp;
        int      ret;
+       int      keybytes;                      /* key length in bytes */
        char    *cp;
        char     inbuf[MAX_KEYSIZE];
 
@@ -514,12 +515,13 @@
                goto bail;
        }
 
-       if (ci->ci_keylen > MAX_KEYSIZE) {
+       keybytes = ci->ci_keylen / 8 + 1;
+       if (keybytes > MAX_KEYSIZE) {
                ret = EINVAL;
                goto bail;
        }
        memset(inbuf, 0x0, sizeof(inbuf));
-       ret = copyin(ci->ci_key, inbuf, ci->ci_keylen);
+       ret = copyin(ci->ci_key, inbuf, keybytes);
        if (ret)
                goto bail;
 
diff -r 3d73b6f712b1 -r bce6703e8c39 sys/dev/cgd_crypto.c
--- a/sys/dev/cgd_crypto.c      Thu Mar 18 08:52:04 2004 +0000
+++ b/sys/dev/cgd_crypto.c      Thu Mar 18 10:42:08 2004 +0000
@@ -1,4 +1,4 @@
-/* $NetBSD: cgd_crypto.c,v 1.2 2003/03/31 08:45:08 elric Exp $ */
+/* $NetBSD: cgd_crypto.c,v 1.3 2004/03/18 10:42:08 dan Exp $ */
 
 /*-
  * Copyright (c) 2002 The NetBSD Foundation, Inc.
@@ -44,7 +44,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: cgd_crypto.c,v 1.2 2003/03/31 08:45:08 elric Exp $");
+__KERNEL_RCSID(0, "$NetBSD: cgd_crypto.c,v 1.3 2004/03/18 10:42:08 dan Exp $");
 
 #include <sys/param.h>
 #include <sys/systm.h>
@@ -392,7 +392,7 @@
 
        if (!blocksize)
                return NULL;
-       if (keylen < 40 || keylen > 448)
+       if (keylen < 40 || keylen > 448 || (keylen % 8 != 0))
                return NULL;
        if (*blocksize == -1)
                *blocksize = 64;
@@ -401,7 +401,7 @@
        bp = malloc(sizeof(*bp), M_DEVBUF, 0);
        if (!bp)
                return NULL;
-       BF_set_key(&bp->bp_key, keylen, key);
+       BF_set_key(&bp->bp_key, keylen / 8, key);
        return (caddr_t)bp;
 }