kern/132351: rijndael CBC mode encryption incorrect

[Patrick Lamaizière]

The following reply was made to PR kern/132351; it has been noted by GNATS.

From: Patrick =?ISO-8859-15?Q?Lamaizi=E8re?= <patfbsd at davenulle.org>
To: bug-followup at FreeBSD.org
Cc: Rajesh Patel <RajeshMPatel at yahoo.com>
Subject: Re: kern/132351: rijndael CBC mode encryption incorrect
Date: Sun, 8 Mar 2009 12:23:01 +0100

 Le Fri, 6 Mar 2009 02:16:42 GMT,
 Rajesh Patel <RajeshMPatel at yahoo.com>:
 
 > >Environment:
 > Windows XP professional - 32 bit
 
 ?
 
 > >Description:
 > The function has bug in CBC mode encryption
 > int rijndael_blockEncrypt(cipherInstance *cipher, keyInstance *key,
 > 		BYTE *input, int inputLen, BYTE *outBuffer) {
 
 > 
 > Original code
 > 
 > 		for (i = numBlocks - 1; i > 0; i--) {
 > #if 1 /*STRICT_ALIGN*/
 > 			AF_BCOPY(outBuffer, block, 16);
 > ========>
 > 			((word32*)block)[0] ^= ((word32*)iv)[0];
 > 			((word32*)block)[1] ^= ((word32*)iv)[1];
 > 			((word32*)block)[2] ^= ((word32*)iv)[2];
 > 			((word32*)block)[3] ^= ((word32*)iv)[3];
 > #else
 > 			((word32*)block)[0] = ((word32*)outBuffer)[0]
 > ^ ((word32*)input)[0]; ((word32*)block)[1] = ((word32*)outBuffer)[1]
 > ^ ((word32*)input)[1]; ((word32*)block)[2] = ((word32*)outBuffer)[2]
 > ^ ((word32*)input)[2]; ((word32*)block)[3] = ((word32*)outBuffer)[3]
 > ^ ((word32*)input)[3]; #endif
 > 			outBuffer += 16;
 > 			rijndaelEncrypt(block, outBuffer,
 > key->keySched, key->ROUNDS); input += 16;
 > 		}
 > 
 > This keeps using the same iv. As a result, the initial block is
 > encrypted multiple times. input should be copied over iv inside the
 > for loop.
 
 You are right, but this code is not a part of the FreeBSD operating
 system.
 
 > >Fix:
 > 
 > Code with Fix
 > 
 > 		for (i = numBlocks - 1; i > 0; i--) {
 > #if 1 /*STRICT_ALIGN*/
 > 			AF_BCOPY(outBuffer, block, 16);
 > /*needs this =======>*/	AF_BCOPY(input, iv, 16); /* Added by
 > Rajesh */ 
 
 The implementation of rijndael_blockEncrypt()
 [sys/cryto/rijndael/rijndael-api-fst.c] in FreeBSD already contains
 this :
 
                 for (i = numBlocks - 1; i > 0; i--) {
 #if 1 /*STRICT_ALIGN*/
                         memcpy(block, outBuffer, 16);
                         memcpy(iv, input, 16);
 
 ----------
 
 I think we should clause this PR. Why: not the good operating system!
 
 Thanks.