Re: [PATCH] eCryptfs: Clean up crypto initialization

[Tyler Hicks <tyhicks@canonical.com>]

[-- Attachment #1: Type: text/plain, Size: 3400 bytes --]

[fixed mhalcrow's email address]

Hi Dan - thanks for the alert. I think the code is fine in this situation.

On 2016-01-25 17:23:11, Dan Carpenter wrote:
> Hello Michael Halcrow,
> 
> The patch e5d9cbde6ce0: "[PATCH] eCryptfs: Clean up crypto
> initialization" from Oct 30, 2006, leads to the following static
> checker warning:
> 
> 	fs/ecryptfs/crypto.c:1625 ecryptfs_process_key_cipher()
> 	error: get_random_bytes() 'dummy_key' too small (64 vs 4294967295)
> 
> fs/ecryptfs/crypto.c
>   1593  static int
>   1594  ecryptfs_process_key_cipher(struct crypto_blkcipher **key_tfm,
>   1595                              char *cipher_name, size_t *key_size)
>   1596  {
>   1597          char dummy_key[ECRYPTFS_MAX_KEY_BYTES];
>   1598          char *full_alg_name = NULL;
>   1599          int rc;
>   1600  
>   1601          *key_tfm = NULL;
>   1602          if (*key_size > ECRYPTFS_MAX_KEY_BYTES) {
>   1603                  rc = -EINVAL;
>   1604                  printk(KERN_ERR "Requested key size is [%zd] bytes; maximum "
>   1605                        "allowable is [%d]\n", *key_size, ECRYPTFS_MAX_KEY_BYTES);
>   1606                  goto out;
>   1607          }
>   1608          rc = ecryptfs_crypto_api_algify_cipher_name(&full_alg_name, cipher_name,
>   1609                                                      "ecb");
>   1610          if (rc)
>   1611                  goto out;
>   1612          *key_tfm = crypto_alloc_blkcipher(full_alg_name, 0, CRYPTO_ALG_ASYNC);
>   1613          if (IS_ERR(*key_tfm)) {
>   1614                  rc = PTR_ERR(*key_tfm);
>   1615                  printk(KERN_ERR "Unable to allocate crypto cipher with name "
>   1616                         "[%s]; rc = [%d]\n", full_alg_name, rc);
>   1617                  goto out;
>   1618          }
>   1619          crypto_blkcipher_set_flags(*key_tfm, CRYPTO_TFM_REQ_WEAK_KEY);
>   1620          if (*key_size == 0) {
>   1621                  struct blkcipher_alg *alg = crypto_blkcipher_alg(*key_tfm);
>   1622  
>   1623                  *key_size = alg->max_keysize;
> 
> My concern here is that arc4 has a max_keysize of ARC4_MAX_KEY_SIZE (256).
> 
>   1624          }
>   1625          get_random_bytes(dummy_key, *key_size);
> 
> Potentially leading to memory corruption here.  This is static analysis
> work so I may be wrong.

There are two things stopping this from being an issue. The first is the
check on key_size at line 1602. The second is that eCryptfs only
supports a small set of ciphers that have max key sizes well within
ECRYPTFS_MAX_KEY_BYTES:

fs/ecryptfs/crypto.c
   962  /* Add support for additional ciphers by adding elements here. The
   963   * cipher_code is whatever OpenPGP applicatoins use to identify the
   964   * ciphers. List in order of probability. */
   965  static struct ecryptfs_cipher_code_str_map_elem
   966  ecryptfs_cipher_code_str_map[] = {
   967          {"aes",RFC2440_CIPHER_AES_128 },
   968          {"blowfish", RFC2440_CIPHER_BLOWFISH},
   969          {"des3_ede", RFC2440_CIPHER_DES3_EDE},
   970          {"cast5", RFC2440_CIPHER_CAST_5},
   971          {"twofish", RFC2440_CIPHER_TWOFISH},
   972          {"cast6", RFC2440_CIPHER_CAST_6},
   973          {"aes", RFC2440_CIPHER_AES_192},
   974          {"aes", RFC2440_CIPHER_AES_256}
   975  };

Tyler

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]