Skip to content

Commit

Permalink
crypto: skcipher - prevent using skciphers without setting key
Browse files Browse the repository at this point in the history 
Similar to what was done for the hash API, update the skcipher API to
track whether each transform has been keyed, and reject
encryption/decryption if a key is needed but one hasn't been set.

This isn't as important as the equivalent fix for the hash API because
symmetric ciphers almost always require a key (the "null cipher" is the
only exception), so are unlikely to be used without one.  Still,
tracking the key will prevent accidental unkeyed use.  algif_skcipher
also had to track the key anyway, so the new flag replaces that and
simplifies the algif_skcipher implementation.

Signed-off-by: Eric Biggers <ebiggers@google.com>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
  • Loading branch information
@ebiggers @herbertx
ebiggers authored and herbertx committed Jan 12, 2018
1 parent 4e1d14b commit f8d33fa
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 55 deletions.
59 changes: 13 additions & 46 deletions crypto/algif_skcipher.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,20 +38,14 @@
#include <linux/net.h>
#include <net/sock.h>

struct skcipher_tfm {
struct crypto_skcipher *skcipher;
bool has_key;
};

static int skcipher_sendmsg(struct socket *sock, struct msghdr *msg,
size_t size)
{
struct sock *sk = sock->sk;
struct alg_sock *ask = alg_sk(sk);
struct sock *psk = ask->parent;
struct alg_sock *pask = alg_sk(psk);
struct skcipher_tfm *skc = pask->private;
struct crypto_skcipher *tfm = skc->skcipher;
struct crypto_skcipher *tfm = pask->private;
unsigned ivsize = crypto_skcipher_ivsize(tfm);

return af_alg_sendmsg(sock, msg, size, ivsize);
Expand All @@ -65,8 +59,7 @@ static int _skcipher_recvmsg(struct socket *sock, struct msghdr *msg,
struct sock *psk = ask->parent;
struct alg_sock *pask = alg_sk(psk);
struct af_alg_ctx *ctx = ask->private;
struct skcipher_tfm *skc = pask->private;
struct crypto_skcipher *tfm = skc->skcipher;
struct crypto_skcipher *tfm = pask->private;
unsigned int bs = crypto_skcipher_blocksize(tfm);
struct af_alg_async_req *areq;
int err = 0;
Expand Down Expand Up @@ -221,7 +214,7 @@ static int skcipher_check_key(struct socket *sock)
int err = 0;
struct sock *psk;
struct alg_sock *pask;
struct skcipher_tfm *tfm;
struct crypto_skcipher *tfm;
struct sock *sk = sock->sk;
struct alg_sock *ask = alg_sk(sk);

Expand All @@ -235,7 +228,7 @@ static int skcipher_check_key(struct socket *sock)

err = -ENOKEY;
lock_sock_nested(psk, SINGLE_DEPTH_NESTING);
if (!tfm->has_key)
if (crypto_skcipher_get_flags(tfm) & CRYPTO_TFM_NEED_KEY)
goto unlock;

if (!pask->refcnt++)
Expand Down Expand Up @@ -314,41 +307,17 @@ static struct proto_ops algif_skcipher_ops_nokey = {

static void *skcipher_bind(const char *name, u32 type, u32 mask)
{
struct skcipher_tfm *tfm;
struct crypto_skcipher *skcipher;

tfm = kzalloc(sizeof(*tfm), GFP_KERNEL);
if (!tfm)
return ERR_PTR(-ENOMEM);

skcipher = crypto_alloc_skcipher(name, type, mask);
if (IS_ERR(skcipher)) {
kfree(tfm);
return ERR_CAST(skcipher);
}

tfm->skcipher = skcipher;

return tfm;
return crypto_alloc_skcipher(name, type, mask);
}

static void skcipher_release(void *private)
{
struct skcipher_tfm *tfm = private;

crypto_free_skcipher(tfm->skcipher);
kfree(tfm);
crypto_free_skcipher(private);
}

static int skcipher_setkey(void *private, const u8 *key, unsigned int keylen)
{
struct skcipher_tfm *tfm = private;
int err;

err = crypto_skcipher_setkey(tfm->skcipher, key, keylen);
tfm->has_key = !err;

return err;
return crypto_skcipher_setkey(private, key, keylen);
}

static void skcipher_sock_destruct(struct sock *sk)
Expand All @@ -357,8 +326,7 @@ static void skcipher_sock_destruct(struct sock *sk)
struct af_alg_ctx *ctx = ask->private;
struct sock *psk = ask->parent;
struct alg_sock *pask = alg_sk(psk);
struct skcipher_tfm *skc = pask->private;
struct crypto_skcipher *tfm = skc->skcipher;
struct crypto_skcipher *tfm = pask->private;

af_alg_pull_tsgl(sk, ctx->used, NULL, 0);
sock_kzfree_s(sk, ctx->iv, crypto_skcipher_ivsize(tfm));
Expand All @@ -370,22 +338,21 @@ static int skcipher_accept_parent_nokey(void *private, struct sock *sk)
{
struct af_alg_ctx *ctx;
struct alg_sock *ask = alg_sk(sk);
struct skcipher_tfm *tfm = private;
struct crypto_skcipher *skcipher = tfm->skcipher;
struct crypto_skcipher *tfm = private;
unsigned int len = sizeof(*ctx);

ctx = sock_kmalloc(sk, len, GFP_KERNEL);
if (!ctx)
return -ENOMEM;

ctx->iv = sock_kmalloc(sk, crypto_skcipher_ivsize(skcipher),
ctx->iv = sock_kmalloc(sk, crypto_skcipher_ivsize(tfm),
GFP_KERNEL);
if (!ctx->iv) {
sock_kfree_s(sk, ctx, len);
return -ENOMEM;
}

memset(ctx->iv, 0, crypto_skcipher_ivsize(skcipher));
memset(ctx->iv, 0, crypto_skcipher_ivsize(tfm));

INIT_LIST_HEAD(&ctx->tsgl_list);
ctx->len = len;
Expand All @@ -405,9 +372,9 @@ static int skcipher_accept_parent_nokey(void *private, struct sock *sk)

static int skcipher_accept_parent(void *private, struct sock *sk)
{
struct skcipher_tfm *tfm = private;
struct crypto_skcipher *tfm = private;

if (!tfm->has_key && crypto_skcipher_has_setkey(tfm->skcipher))
if (crypto_skcipher_get_flags(tfm) & CRYPTO_TFM_NEED_KEY)
return -ENOKEY;

return skcipher_accept_parent_nokey(private, sk);
Expand Down
30 changes: 26 additions & 4 deletions crypto/skcipher.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -598,8 +598,11 @@ static int skcipher_setkey_blkcipher(struct crypto_skcipher *tfm,
err = crypto_blkcipher_setkey(blkcipher, key, keylen);
crypto_skcipher_set_flags(tfm, crypto_blkcipher_get_flags(blkcipher) &
CRYPTO_TFM_RES_MASK);
if (err)
return err;

return err;
crypto_skcipher_clear_flags(tfm, CRYPTO_TFM_NEED_KEY);
return 0;
}

static int skcipher_crypt_blkcipher(struct skcipher_request *req,
Expand Down Expand Up @@ -674,6 +677,9 @@ static int crypto_init_skcipher_ops_blkcipher(struct crypto_tfm *tfm)
skcipher->ivsize = crypto_blkcipher_ivsize(blkcipher);
skcipher->keysize = calg->cra_blkcipher.max_keysize;

if (skcipher->keysize)
crypto_skcipher_set_flags(skcipher, CRYPTO_TFM_NEED_KEY);

return 0;
}

Expand All @@ -692,8 +698,11 @@ static int skcipher_setkey_ablkcipher(struct crypto_skcipher *tfm,
crypto_skcipher_set_flags(tfm,
crypto_ablkcipher_get_flags(ablkcipher) &
CRYPTO_TFM_RES_MASK);
if (err)
return err;

return err;
crypto_skcipher_clear_flags(tfm, CRYPTO_TFM_NEED_KEY);
return 0;
}

static int skcipher_crypt_ablkcipher(struct skcipher_request *req,
Expand Down Expand Up @@ -767,6 +776,9 @@ static int crypto_init_skcipher_ops_ablkcipher(struct crypto_tfm *tfm)
sizeof(struct ablkcipher_request);
skcipher->keysize = calg->cra_ablkcipher.max_keysize;

if (skcipher->keysize)
crypto_skcipher_set_flags(skcipher, CRYPTO_TFM_NEED_KEY);

return 0;
}

Expand Down Expand Up @@ -796,16 +808,23 @@ static int skcipher_setkey(struct crypto_skcipher *tfm, const u8 *key,
{
struct skcipher_alg *cipher = crypto_skcipher_alg(tfm);
unsigned long alignmask = crypto_skcipher_alignmask(tfm);
int err;

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

if ((unsigned long)key & alignmask)
return skcipher_setkey_unaligned(tfm, key, keylen);
err = skcipher_setkey_unaligned(tfm, key, keylen);
else
err = cipher->setkey(tfm, key, keylen);

if (err)
return err;

return cipher->setkey(tfm, key, keylen);
crypto_skcipher_clear_flags(tfm, CRYPTO_TFM_NEED_KEY);
return 0;
}

static void crypto_skcipher_exit_tfm(struct crypto_tfm *tfm)
Expand Down Expand Up @@ -834,6 +853,9 @@ static int crypto_skcipher_init_tfm(struct crypto_tfm *tfm)
skcipher->ivsize = alg->ivsize;
skcipher->keysize = alg->max_keysize;

if (skcipher->keysize)
crypto_skcipher_set_flags(skcipher, CRYPTO_TFM_NEED_KEY);

if (alg->exit)
skcipher->base.exit = crypto_skcipher_exit_tfm;

Expand Down
11 changes: 6 additions & 5 deletions include/crypto/skcipher.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -401,11 +401,6 @@ static inline int crypto_skcipher_setkey(struct crypto_skcipher *tfm,
return tfm->setkey(tfm, key, keylen);
}

static inline bool crypto_skcipher_has_setkey(struct crypto_skcipher *tfm)
{
return tfm->keysize;
}

static inline unsigned int crypto_skcipher_default_keysize(
struct crypto_skcipher *tfm)
{
Expand Down Expand Up @@ -442,6 +437,9 @@ static inline int crypto_skcipher_encrypt(struct skcipher_request *req)
{
struct crypto_skcipher *tfm = crypto_skcipher_reqtfm(req);

if (crypto_skcipher_get_flags(tfm) & CRYPTO_TFM_NEED_KEY)
return -ENOKEY;

return tfm->encrypt(req);
}

Expand All @@ -460,6 +458,9 @@ static inline int crypto_skcipher_decrypt(struct skcipher_request *req)
{
struct crypto_skcipher *tfm = crypto_skcipher_reqtfm(req);

if (crypto_skcipher_get_flags(tfm) & CRYPTO_TFM_NEED_KEY)
return -ENOKEY;

return tfm->decrypt(req);
}

Expand Down

0 comments on commit f8d33fa

Please sign in to comment.