Re: [PATCH] cit_encrypt_iv/cit_decrypt_iv for ECB mode

[Solar Designer <solar@openwall.com>]

On Mon, Aug 21, 2006 at 08:58:30AM +1000, Herbert Xu wrote:
> On Sun, Aug 20, 2006 at 06:49:08PM +0400, Solar Designer wrote:
> > 
> > Can we maybe define working but IV-ignoring functions for ECB (like I
> > did), but use memory-clearing nocrypt*() for CFB and CTR (as long as
> > these are not supported)?  Of course, all of these will return -ENOSYS.
> 
> In cryptodev-2.6, with block ciphers you can no longer select CFB/CTR
> until someone writes support for them so this is no longer an issue.
> 
> For 2.4, I don't really mind either way what nocrypt does.

OK, I've merged Willy's suggestion for the memset()s into the patch that
I had submitted previously.  The resulting patch is attached.

Alexander

diff -urp linux-2.4.33/crypto/cipher.c linux/crypto/cipher.c
--- linux-2.4.33/crypto/cipher.c	Sun Aug  8 03:26:04 2004
+++ linux/crypto/cipher.c	Tue Aug 22 09:53:35 2006
@@ -147,6 +147,15 @@ static int ecb_encrypt(struct crypto_tfm
 	             ecb_process, 1, NULL);
 }
 
+static int ecb_encrypt_iv(struct crypto_tfm *tfm,
+			  struct scatterlist *dst,
+			  struct scatterlist *src,
+			  unsigned int nbytes, u8 *iv)
+{
+	ecb_encrypt(tfm, dst, src, nbytes);
+	return -ENOSYS;
+}
+
 static int ecb_decrypt(struct crypto_tfm *tfm,
                        struct scatterlist *dst,
                        struct scatterlist *src,
@@ -157,6 +166,15 @@ static int ecb_decrypt(struct crypto_tfm
 	             ecb_process, 1, NULL);
 }
 
+static int ecb_decrypt_iv(struct crypto_tfm *tfm,
+			  struct scatterlist *dst,
+			  struct scatterlist *src,
+			  unsigned int nbytes, u8 *iv)
+{
+	ecb_decrypt(tfm, dst, src, nbytes);
+	return -ENOSYS;
+}
+
 static int cbc_encrypt(struct crypto_tfm *tfm,
                        struct scatterlist *dst,
                        struct scatterlist *src,
@@ -197,11 +215,20 @@ static int cbc_decrypt_iv(struct crypto_
 	             cbc_process, 0, iv);
 }
 
+/*
+ * nocrypt*() zeroize the destination buffer to make sure we don't leak
+ * uninitialized memory contents if the caller ignores the return value.
+ * This is bad since the data in the source buffer is unused and may be
+ * lost, but an infoleak would be even worse.  The performance cost of
+ * memset() is irrelevant since a well-behaved caller would not bump into
+ * the error repeatedly.
+ */
 static int nocrypt(struct crypto_tfm *tfm,
                    struct scatterlist *dst,
                    struct scatterlist *src,
 		   unsigned int nbytes)
 {
+	memset(dst, 0, nbytes);
 	return -ENOSYS;
 }
 
@@ -210,6 +237,7 @@ static int nocrypt_iv(struct crypto_tfm 
                       struct scatterlist *src,
                       unsigned int nbytes, u8 *iv)
 {
+	memset(dst, 0, nbytes);
 	return -ENOSYS;
 }
 
@@ -235,6 +263,11 @@ int crypto_init_cipher_ops(struct crypto
 	case CRYPTO_TFM_MODE_ECB:
 		ops->cit_encrypt = ecb_encrypt;
 		ops->cit_decrypt = ecb_decrypt;
+/* These should have been nocrypt_iv, but patch-cryptoloop-jari-2.4.22.0
+ * (and its other revisions) directly calls the *_iv() functions even in
+ * ECB mode and ignores their return value. */
+		ops->cit_encrypt_iv = ecb_encrypt_iv;
+		ops->cit_decrypt_iv = ecb_decrypt_iv;
 		break;
 		
 	case CRYPTO_TFM_MODE_CBC: