Skip to contentSkip to conversation

potential underfow in crypto/lrw.c setkey() setkey

DISCUSSION Dan Carpenter 2 participants 2 messages May 9, 2019 – May 10, 2019 Crypto API

lore
Share & tools
lore
#0 lore raw

crypto/lrw.c

    72  static int setkey(struct crypto_skcipher *parent, const u8 *key,
    73                    unsigned int keylen)
    74  {
    75          struct priv *ctx = crypto_skcipher_ctx(parent);
    76          struct crypto_skcipher *child = ctx->child;
    77          int err, bsize = LRW_BLOCK_SIZE;
    78          const u8 *tweak = key + keylen - bsize;
                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Smatch thinks that keylen is user controlled from zero to some upper bound. How do we know it's >= LRW_BLOCK_SIZE (16)?

I find the crypto code sort of hard to follow... There are a bunch of setkey pointers and they're sometimes called recursively. Is there some trick or hints?

    79          be128 tmp = { 0 };
    80          int i;
    81  
    82          crypto_skcipher_clear_flags(child, CRYPTO_TFM_REQ_MASK);
    83          crypto_skcipher_set_flags(child, crypto_skcipher_get_flags(parent) &
    84                                           CRYPTO_TFM_REQ_MASK);
    85          err = crypto_skcipher_setkey(child, key, keylen - bsize);
    86          crypto_skcipher_set_flags(parent, crypto_skcipher_get_flags(child) &
    87                                            CRYPTO_TFM_RES_MASK);
    88          if (err)
    89                  return err;
    90  
    91          if (ctx->table)
    92                  gf128mul_free_64k(ctx->table);
    93  
    94          /* initialize multiplication table for Key2 */
    95          ctx->table = gf128mul_init_64k_bbe((be128 *)tweak);
    96          if (!ctx->table)
    97                  return -ENOMEM;
    98  
    99          /* initialize optimization table */
   100          for (i = 0; i < 128; i++) {
   101                  setbit128_bbe(&tmp, i);
   102                  ctx->mulinc[i] = tmp;
   103                  gf128mul_64k_bbe(&ctx->mulinc[i], ctx->table);

104 }
105
106 return 0;
107 }

regards,
dan carpenter

#1 lore raw
Quoted 11 lines · Dan Carpenter

crypto/lrw.c

    72  static int setkey(struct crypto_skcipher *parent, const u8 *key,
    73                    unsigned int keylen)
    74  {
    75          struct priv *ctx = crypto_skcipher_ctx(parent);
    76          struct crypto_skcipher *child = ctx->child;
    77          int err, bsize = LRW_BLOCK_SIZE;
    78          const u8 *tweak = key + keylen - bsize;
                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Smatch thinks that keylen is user controlled from zero to some upper bound. How do we know it's >= LRW_BLOCK_SIZE (16)?

See create() in crypto/lrw.c, which is the function that creates an LRW instance. It sets .min_keysize and .max_keysize:

        inst->alg.min_keysize = crypto_skcipher_alg_min_keysize(alg) +
                                LRW_BLOCK_SIZE;
        inst->alg.max_keysize = crypto_skcipher_alg_max_keysize(alg) +
                                LRW_BLOCK_SIZE;

Then when sometime calls crypto_skcipher_setkey(), it calls skcipher_setkey() in crypto/skcipher.c which verifies the key length is in within the bounds the algorithm declares:

        if (keylen < cipher->min_keysize || keylen > cipher->max_keysize) {
                crypto_skcipher_set_flags(tfm, CRYPTO_TFM_RES_BAD_KEY_LEN);
                return -EINVAL;
        }
Quoted 5 lines

I find the crypto code sort of hard to follow... There are a bunch of setkey pointers and they're sometimes called recursively. Is there some trick or hints?

Quoted 11 lines
    79          be128 tmp = { 0 };
    80          int i;
    81  
    82          crypto_skcipher_clear_flags(child, CRYPTO_TFM_REQ_MASK);
    83          crypto_skcipher_set_flags(child, crypto_skcipher_get_flags(parent) &
    84                                           CRYPTO_TFM_REQ_MASK);
    85          err = crypto_skcipher_setkey(child, key, keylen - bsize);
    86          crypto_skcipher_set_flags(parent, crypto_skcipher_get_flags(child) &
    87                                            CRYPTO_TFM_RES_MASK);
    88          if (err)
    89                  return err;

LRW is a template for a block cipher mode of operation which is implemented on top of ECB. So, LRW's setkey() method calls setkey() on the underlying ECB transform. Similarly, ECB then may call setkey() of the underlying block cipher such as AES, or alternatively it may be implemented directly.

- Eric