[patch] cgd

Taylor R Campbell <[email protected]> Sat, 27 Nov 2010 02:40:11 +0000
Newsgroups gmane.os.netbsd.devel.crypto
Message-ID <[email protected]>
(I am not subscribed to this list, so please cc me in replies.)

I tried to use cgd yesterday, and was pretty by the man pages and the
use of cgdconfig; I also stumbled across some bugs in cgdconfig.  I am
not a cryptographer, and thus am unqualified to give cryptographic
advice, but I thought it might be worthwhile for the man pages to be a
little clearer on the disk format -- enough to write programs that
read and write it -- and to mention some cryptographic limitations of
cgd.

Attached is a patch to HEAD that fixes some errors in the
implementation and documentation.  However, I eventually gave up on
trying to use cgdconfig[*], so I wrote my own trivial utilities (at
<http://mumble.net/~campbell/tmp/cgdutil-20101127.tgz> for the time
being, for anyone curious) to serve its function, when combined with
external tools such as scrypt <http://www.tarsnap.com/scrypt/>.

Comments?  I would submit a PR for the patch, but, as I said, I am not
a cryptographer, so I am interested more in review on the patch (and
corrections to my misconceptions about cgd, if any) than just in
applying the patch.

[*] Aside from having a usage model that confused me, cgdconfig lacks
    any cryptographic integrity checks -- a passphrased parameters
    file is not actually bound to a particular key, so, for instance,
    if you `cgdconfig -G' up a new parameters file, mistype a
    passphrase, and delete the old one, cgdconfig won't (can't!)
    notice, and your cgd will be lost and gone forever.  In contrast,
    an scrypt file is cryptographically bound to its contents, so
    scrypt can say whether you typed the correct passphrase or not.
cgd-20101127.patch (application/octet-stream, 15 KB)
* cgdconfig.8

Clarify some imprecise language.
State the default action.
Clarify the `-G' action.
Say that `-k' may be specified more than once.
Describe what `-s' does, not a part of what it does.
Explain that `-k shell_cmd' doesn't work.
Fix syntax of `key' statement: should be base64, not string.

* cgdconfig.c

Nuke trailing whitespace.
Fix verbose message that printed an uninitialized buffer.
Make `-G' refuse urandomkey, not just randomkey.
Elide needless storedkey entries in `-G'.
Suppress spurious warning in `-G'.

* pkcs5_pbkdf2.c

Nuke trailing whitespace.


Index: share/man/man4/cgd.4
===================================================================
RCS file: /cvsroot/src/share/man/man4/cgd.4,v
retrieving revision 1.16
diff -p -u -r1.16 cgd.4
--- share/man/man4/cgd.4	15 Apr 2010 09:56:56 -0000	1.16
+++ share/man/man4/cgd.4	27 Nov 2010 02:15:13 -0000
@@ -27,7 +27,7 @@
 .\" ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
 .\" POSSIBILITY OF SUCH DAMAGE.
 .\"
-.Dd April 15, 2010
+.Dd November 26, 2010
 .Dt CGD 4
 .Os
 .Sh NAME
@@ -38,8 +38,19 @@
 .Sh DESCRIPTION
 The
 .Nm
-driver provides the capability of encrypting blocks on their way
-to and from a disk or partition.
+driver provides confidentiality to a disk block device.
+A
+.Nm
+is configured with an underlying disk and a set of encryption
+parameters.
+When a disk block is written to a
+.Nm ,
+it is encrypted, and the ciphertext is written to the corresponding
+location on the underlying disk.
+When a disk block is read from a
+.Nm ,
+the disk block at the corresponding location is read from the
+underlying disk and decrypted, and the plaintext is returned.
 .Pp
 In order to compile support for the
 .Nm
@@ -54,7 +65,9 @@ The count argument defines how many
 .Nm
 devices may be configured at a time.
 .Ss Encryption Algorithms
-Currently the following cryptographic algorithms are supported:
+The
+.Nm
+driver supports the following ciphers for encrypting disk blocks:
 .Bl -tag -width indentxxxxxxx
 .It Ic aes-cbc
 .Tn AES
@@ -62,8 +75,8 @@ in
 .Tn CBC
 mode.
 .Tn AES
-uses a 128 bit blocksize and can accept keys of length 128, 192, or 256.
-The default key length is 128.
+uses cipher blocks of 128 bits and keys of 128, 192, or 256 bits.
+The default key length is 128 bits.
 .It Ic 3des-cbc
 Triple
 .Tn DES
@@ -72,37 +85,44 @@ in
 mode.
 Triple
 .Tn DES
-uses a 64 bit blocksize and is performed in
+uses cipher blocks of 64 bits and is performed in
 .Tn EDE3
-mode with a 168 bit key.
-The key passed to the kernel is 192 bits but the parity bits are ignored.
+mode with a 168-bit key.
+The key passed to the kernel is 192 bits, but the parity bits are
+ignored.
 .It Ic blowfish-cbc
 Blowfish in
 .Tn CBC
 mode.
-Blowfish uses a 64 bit blocksize and can accept keys between 40 and
-448 bits in multiples of 8.
+Blowfish uses cipher blocks of 64 bits and keys between 40 and 448 bits
+in multiples of 8.
 It is strongly encouraged that keys be at least 128 bits long.
 There are no performance advantages of using shorter keys.
 The default key length is 128 bits.
 .El
 .Ss IV Methods
-Currently, the following
-.Tn IV
-Methods are supported:
+The
+.Nm
+driver supports the following methods of generating CBC initialization
+vectors for each disk block:
 .Bl -tag -width encblkno1
 .It Ic encblkno1
-This method  encrypts the block number of the physical disk block once with
-the cipher and key provided and uses the result as the
-.Tn IV
-for
-.Tn CBC
-mode.
-This method should ensure that each block has a different
-.Tn IV
-and that the
+The
 .Tn IV
-is reasonably unpredictable.
+for each disk block is computed from the disk block number by
+encrypting it with the cipher and key that the
+.Nm
+is configured with.
+Specifically, the bytes of the disk block number are laid out in
+little-endian order, followed by enough zeros to fill a cipher block,
+and the resulting cipher block is encrypted to obtain the
+.Tn IV .
+Thus, the
+.Tn IV
+is hard to predict without the key, and different disk blocks are
+encrypted with different
+.Tn IVs ,
+in order to resist watermarking attacks.
 This is the default method used by
 .Xr cgdconfig 8
 when configuring a new
@@ -113,20 +133,10 @@ This is the original
 method used by
 .Nm
 and provided for backward compatibility.
-It repeatedly encrypts the block number of the physical disk block
-eight times and uses the result as the
-.Tn IV
-for
-.Tn CBC
-mode.
-This method should ensure that each block has a different
-.Tn IV
-and that the
-.Tn IV
-is reasonably unpredictable.
+It is like encblkno1, except that it iterates the encryption eight
+times rather than only once.
 The eightfold encryption was not intended and causes a notable
-performance loss with little (if any) increase in security over a
-single encryption.
+performance loss with no increase in security.
 .El
 .Ss IOCTLS
 A
@@ -161,6 +171,46 @@ to configure a
 .Nm ,
 then you have irrevocably lost all of the data on the disk.
 Please ensure that you are using an appropriate backup strategy.
+.Pp
+The
+.Nm
+driver provides no authenticity or integrity protection.
+When rewriting a disk block, the new data are encrypted with the same
+key and
+.Tn IV
+as the old data; if an attacker reverts the ciphertext of a disk block
+to an old version, the plaintext will be reverted to the old version,
+and the
+.Nm
+will not detect this.
+In CBC mode, if an attacker flips a bit in a ciphertext block, the
+corresponding bit in the corresponding plaintext block will be flipped,
+and the preceding plaintext block will be randomized; the
+.Nm
+will not detect this, either.
+.Pp
+Because disk blocks are encrypted independently, if an attacker
+observes two snapshots of the disk at different times, the attacker can
+see which disk blocks have changed.  Moreover, because
+.Nm
+reuses the
+.Tn IV
+in CBC mode to encrypt different versions of a disk block at different
+times, if an attacker observes two versions of the disk block, the
+attacker can see what prefix of the disk block did not change: common
+prefixes in the plaintext are reflected as common prefixes in the
+ciphertext.
+.Pp
+In CBC mode, if two ciphertext blocks are identical, then the xor of
+their corresponding plaintext blocks is the same as the xor of the
+respective preceding two ciphertext blocks, leaking information about
+the contents of the disk.
+By the birthday paradox, if a cipher block has b bits, the expected
+number of cipher blocks before a collision is 2^(b/2).
+For a cipher with 128-bit cipher blocks, such as AES, this many cipher
+blocks fills a quarter of a zettabyte.
+However, for a cipher with 64-bit cipher blocks such as 3DES or
+Blowfish, this many cipher blocks fills only thirty-two gigabytes.
 .Sh FILES
 .Bl -tag -width indentxxxxxxxxxxx
 .It /dev/{,r}cgd*
Index: sbin/cgdconfig/cgdconfig.8
===================================================================
RCS file: /cvsroot/src/sbin/cgdconfig/cgdconfig.8,v
retrieving revision 1.30
diff -p -u -r1.30 cgdconfig.8
--- sbin/cgdconfig/cgdconfig.8	30 Mar 2010 14:26:55 -0000	1.30
+++ sbin/cgdconfig/cgdconfig.8	27 Nov 2010 02:15:13 -0000
@@ -27,7 +27,7 @@
 .\" ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
 .\" POSSIBILITY OF SUCH DAMAGE.
 .\"
-.Dd October 19, 2009
+.Dd November 26, 2010
 .Dt CGDCONFIG 8
 .Os
 .Sh NAME
@@ -81,6 +81,27 @@ and to maintain the configuration files 
 For more information about cryptographic disk devices see
 .Xr cgd 4 .
 .Pp
+The default action is to configure
+.Ar cgd
+from the block device
+.Ar dev
+using the specified
+.Ar paramsfile
+or the file
+in
+.Pa /etc/cgd/
+with the basename of
+.Ar dev .
+The
+.Fl C ,
+.Fl U ,
+.Fl G ,
+.Fl g ,
+.Fl s ,
+and
+.Fl u
+options choose different actions.
+.Pp
 The options are as follows:
 .Bl -tag -width configfilexxxx
 .It Fl C
@@ -90,24 +111,29 @@ Specify the configuration file explicitl
 configuration file
 .Pa /etc/cgd/cgd.conf .
 .It Fl G
-Generate a new paramsfile (to stdout) using the values from
+Generate a parameters file (to stdout) that will generate the same key
+as
+.Ar paramsfile .
+This will prompt for for any passphrase key generation method in
+.Ar paramsfile ,
+and for any passphrase key generation method for the output parameters
+file.
 .Ar paramsfile
-which will generate the same key.
-This may need to prompt for multiple passphrases.
+may not use the randomkey or urandomkey key generation methods.
 .It Fl g
-Generate a paramsfile (to stdout).
+Generate a parameters file (to stdout).
 .It Fl i Ar ivmeth
 Specify the IV method (default: encblkno1).
 .It Fl k Ar kgmeth
-Specify the key generation method (default: pkcs5_pbkdf2/sha1).
+Specify a key generation method (default: pkcs5_pbkdf2/sha1).
+Multiple key generation methods may be specified.
 .It Fl n
 Do not actually configure or unconfigure a cryptographic disk
 device, but instead report the steps that would be taken.
 .It Fl o Ar outfile
-When generating a
-.Ar paramsfile ,
-store it in
-.Ar outfile .
+When generating a parameters file, store it in
+.Ar outfile
+rather than writing it to stdout.
 .It Fl p
 Read all passphrases from stdin rather than
 .Pa /dev/tty .
@@ -118,16 +144,23 @@ If this flag is specified then verificat
 in question to be unconfigured rather than prompting for the passphrase
 again.
 .It Fl s
-Read the key from stdin.
+Configure
+.Ar cgd
+from the block device
+.Ar dev
+using a key read from stdin for the cipher algorithm
+.Ar alg ,
+rather than reading a parameters file.
 .It Fl U
 Unconfigure all the devices listed in the cgd configuration file.
 .It Fl u
-Unconfigure a cgd.
+Unconfigure
+.Ar cgd .
 .It Fl V Ar vmeth
 Specify the verification method (default: none).
 .It Fl v
 Be verbose.
-May be specified multiple times.
+May be specified up to three times.
 .El
 .Pp
 For more information about the cryptographic algorithms and IV methods
@@ -182,6 +215,10 @@ number generator.
 This method executes a shell command via
 .Xr popen 3
 and reads the key from stdout.
+This method cannot be specified with the
+.Fl k
+option, which has no way to specify the shell command to run; instead,
+the shell command must be written by hand into a parameters file.
 .El
 .Ss Verification Method
 The verification method is how
@@ -236,9 +273,9 @@ for an example of
 .Ss Parameters File
 The Parameters File contains the required information to generate the
 key and configure a device.
-These files are typically generated by the
+These files can be generated by the
 .Fl g
-flag and not edited by hand.
+flag or written by hand.
 When a device is configured the default parameters file is constructed
 by taking the basename of the target disk and prepending
 .Pa /etc/cgd/
@@ -296,7 +333,7 @@ contains statements that are specific to
 .Pp
 The keygen statement's statement block may contain the following statements:
 .Bl -tag -width indentxx
-.It key Ar string
+.It key Ar base64
 The key.
 Only used for the storedkey key generation method.
 .It cmd Ar string
@@ -312,7 +349,7 @@ Only used for pkcs5_pbkdf2/sha1 and pkcs
 .Sh FILES
 .Bl -tag -width indentxxxxxxxxxxxxxxxxxx -compact
 .It Pa /etc/cgd/
-configuration directory, used to store paramsfiles.
+configuration directory, used to store parameters files.
 .It Pa /etc/cgd/cgd.conf
 cgd configuration file.
 .El
Index: sbin/cgdconfig/cgdconfig.c
===================================================================
RCS file: /cvsroot/src/sbin/cgdconfig/cgdconfig.c,v
retrieving revision 1.28
diff -p -u -r1.28 cgdconfig.c
--- sbin/cgdconfig/cgdconfig.c	8 Sep 2009 21:36:35 -0000	1.28
+++ sbin/cgdconfig/cgdconfig.c	27 Nov 2010 02:15:13 -0000
@@ -393,7 +393,7 @@ maybe_getpass(char *prompt)
 }
 
 /*ARGSUSED*/
-/* 
+/*
  * XXX take, and pass through, a compat flag that indicates whether we
  * provide backwards compatibility with a previous bug.  The previous
  * behaviour is indicated by the keygen method pkcs5_pbkdf2, and a
@@ -503,7 +503,7 @@ configure(int argc, char **argv, struct 
 	int		 ret;
 	char		 cgdname[PATH_MAX];
 
-	if (argc == 2) {	
+	if (argc == 2) {
 		char *pfile;
 
 		if (asprintf(&pfile, "%s/%s",
@@ -645,7 +645,7 @@ opendisk_werror(const char *cgd, char *b
 {
 	int	fd;
 
-	VPRINTF(3, ("opendisk_werror(%s, %s, %zu) called.\n", cgd, buf, buflen));
+	VPRINTF(3, ("opendisk_werror(\"%s\", %p, %zu) called.\n", cgd, buf, buflen));
 
 	/* sanity */
 	if (!cgd || !buf)
@@ -795,7 +795,7 @@ verify_reenter(struct params *p)
 
 	ret = 0;
 	for (kg = p->keygen; kg && !ret; kg = kg->next) {
-		if ((kg->kg_method != KEYGEN_PKCS5_PBKDF2_SHA1) && 
+		if ((kg->kg_method != KEYGEN_PKCS5_PBKDF2_SHA1) &&
 		    (kg->kg_method != KEYGEN_PKCS5_PBKDF2_OLD ))
 			continue;
 
@@ -872,11 +872,13 @@ generate_convert(struct params *p, int a
 
 	/* for sanity, we ensure that none of the keygens are randomkey */
 	for (kg=p->keygen; kg; kg=kg->next)
-		if (kg->kg_method == KEYGEN_RANDOMKEY)
-			goto bail;
+		if ((kg->kg_method == KEYGEN_RANDOMKEY) ||
+		    (kg->kg_method == KEYGEN_URANDOMKEY))
+			goto bail_random;
 	for (kg=oldp->keygen; kg; kg=kg->next)
-		if (kg->kg_method == KEYGEN_RANDOMKEY)
-			goto bail;
+		if ((kg->kg_method == KEYGEN_RANDOMKEY) ||
+		    (kg->kg_method == KEYGEN_URANDOMKEY))
+			goto bail_random;
 
 	if (!params_verify(oldp)) {
 		warnx("invalid old parameters file \"%s\"", *argv);
@@ -904,23 +906,36 @@ generate_convert(struct params *p, int a
 		if (!p->keygen)
 			return -1;
 	}
-	(void)params_filldefaults(p);
-	(void)keygen_filldefaults(p->keygen, p->keylen);
-	p->key = getkey("new file", p->keygen, p->keylen);
 
-	kg = keygen_generate(KEYGEN_STOREDKEY);
-	kg->kg_key = bits_xor(p->key, oldp->key);
-	keygen_addlist(&p->keygen, kg);
+	(void)params_filldefaults(p);
+	if (p->keygen && !p->keygen->next &&
+	    p->keygen->kg_method == KEYGEN_STOREDKEY)
+		p->keygen->kg_key = bits_dup(oldp->key);
+	else
+		(void)keygen_filldefaults(p->keygen, p->keylen);
 
 	if (!params_verify(p)) {
 		warnx("can't generate new parameters file");
 		return -1;
 	}
 
+	p->key = getkey("new file", p->keygen, p->keylen);
+
+	/* put in a stored key only if necessary to preserve the key */
+	if (!bits_match(p->key, oldp->key)) {
+		kg = keygen_generate(KEYGEN_STOREDKEY);
+		kg->kg_key = bits_xor(p->key, oldp->key);
+		keygen_addlist(&p->keygen, kg);
+	}
+
 	return params_cput(p, outfile);
 bail:
 	params_free(oldp);
 	return -1;
+
+bail_random:
+	warnx("can't preserve randomly generated key");
+	goto bail;
 }
 
 static int
Index: sbin/cgdconfig/pkcs5_pbkdf2.c
===================================================================
RCS file: /cvsroot/src/sbin/cgdconfig/pkcs5_pbkdf2.c,v
retrieving revision 1.14
diff -p -u -r1.14 pkcs5_pbkdf2.c
--- sbin/cgdconfig/pkcs5_pbkdf2.c	28 Apr 2008 20:23:08 -0000	1.14
+++ sbin/cgdconfig/pkcs5_pbkdf2.c	27 Nov 2010 02:15:13 -0000
@@ -93,7 +93,7 @@ prf_iterate(u_int8_t *r, const u_int8_t 
 		if (first_time) {
 			(void)memcpy(r, tmp, PRF_BLOCKLEN);
 			first_time = 0;
-		} else 
+		} else
 			memxor(r, tmp, PRF_BLOCKLEN);
 		(void)memcpy(data, tmp, PRF_BLOCKLEN);
 		datalen = PRF_BLOCKLEN;
@@ -128,7 +128,7 @@ pkcs5_pbkdf2(u_int8_t **r, size_t dkLen,
 
 	/* Step 3 */
 	for (i = 0; i < l; i++)
-		prf_iterate(*r + (PRF_BLOCKLEN * i), P, Plen, S, Slen, c, 
+		prf_iterate(*r + (PRF_BLOCKLEN * i), P, Plen, S, Slen, c,
 			(compat?i:i+1));
 
 	/* Step 4 and 5