search for: keylen

Le 14/04/2020 ? 00:28, Waiman Long a ?crit?: > Since kfree_sensitive() will do an implicit memzero_explicit(), there > is no need to call memzero_explicit() before it. Eliminate those > memzero_explicit() and simplify the call sites. For better correctness, > the setting of keylen is also moved down after the key pointer check. > > Signed-off-by: Waiman Long <longman at redhat.com> > --- > .../allwinner/sun8i-ce/sun8i-ce-cipher.c | 19 +++++------------- > .../allwinner/sun8i-ss/sun8i-ss-cipher.c | 20 +++++-------------- > drivers/cryp...

Le 14/04/2020 ? 00:28, Waiman Long a ?crit?: > Since kfree_sensitive() will do an implicit memzero_explicit(), there > is no need to call memzero_explicit() before it. Eliminate those > memzero_explicit() and simplify the call sites. For better correctness, > the setting of keylen is also moved down after the key pointer check. > > Signed-off-by: Waiman Long <longman at redhat.com> > --- > .../allwinner/sun8i-ce/sun8i-ce-cipher.c | 19 +++++------------- > .../allwinner/sun8i-ss/sun8i-ss-cipher.c | 20 +++++-------------- > drivers/cryp...

Since kfree_sensitive() will do an implicit memzero_explicit(), there is no need to call memzero_explicit() before it. Eliminate those memzero_explicit() and simplify the call sites. For better correctness, the setting of keylen is also moved down after the key pointer check. Signed-off-by: Waiman Long <longman at redhat.com> --- .../allwinner/sun8i-ce/sun8i-ce-cipher.c | 19 +++++------------- .../allwinner/sun8i-ss/sun8i-ss-cipher.c | 20 +++++-------------- drivers/crypto/amlogic/amlogic-gxl-cipher.c...

...ivers/crypto/allwinner/sun8i-ce/sun8i-ce-cipher.c +++ b/drivers/crypto/allwinner/sun8i-ce/sun8i-ce-cipher.c @@ -366,10 +366,7 @@ void sun8i_ce_cipher_exit(struct crypto_tfm *tfm) { struct sun8i_cipher_tfm_ctx *op = crypto_tfm_ctx(tfm); - if (op->key) { - memzero_explicit(op->key, op->keylen); - kfree(op->key); - } + kfree_sensitive(op->key); crypto_free_sync_skcipher(op->fallback_tfm); pm_runtime_put_sync_suspend(op->ce->dev); } @@ -391,10 +388,7 @@ int sun8i_ce_aes_setkey(struct crypto_skcipher *tfm, const u8 *key, dev_dbg(ce->dev, "ERROR: Invalid keyl...

...ll sites. 2 bits of trivia: > diff --git a/drivers/crypto/allwinner/sun8i-ce/sun8i-ce-cipher.c b/drivers/crypto/allwinner/sun8i-ce/sun8i-ce-cipher.c [] > @@ -391,10 +388,7 @@ int sun8i_ce_aes_setkey(struct crypto_skcipher *tfm, const u8 *key, > dev_dbg(ce->dev, "ERROR: Invalid keylen %u\n", keylen); > return -EINVAL; > } > - if (op->key) { > - memzero_explicit(op->key, op->keylen); > - kfree(op->key); > - } > + kfree_sensitive(op->key); > op->keylen = keylen; > op->key = kmemdup(key, keylen, GFP_KERNEL | GFP_DMA);...

...ll sites. 2 bits of trivia: > diff --git a/drivers/crypto/allwinner/sun8i-ce/sun8i-ce-cipher.c b/drivers/crypto/allwinner/sun8i-ce/sun8i-ce-cipher.c [] > @@ -391,10 +388,7 @@ int sun8i_ce_aes_setkey(struct crypto_skcipher *tfm, const u8 *key, > dev_dbg(ce->dev, "ERROR: Invalid keylen %u\n", keylen); > return -EINVAL; > } > - if (op->key) { > - memzero_explicit(op->key, op->keylen); > - kfree(op->key); > - } > + kfree_sensitive(op->key); > op->keylen = keylen; > op->key = kmemdup(key, keylen, GFP_KERNEL | GFP_DMA);...

...a ?crit?: >>>> Since kfree_sensitive() will do an implicit memzero_explicit(), there >>>> is no need to call memzero_explicit() before it. Eliminate those >>>> memzero_explicit() and simplify the call sites. For better correctness, >>>> the setting of keylen is also moved down after the key pointer check. >>>> >>>> Signed-off-by: Waiman Long <longman at redhat.com> >>>> --- >>>> ? .../allwinner/sun8i-ce/sun8i-ce-cipher.c????? | 19 +++++------------- >>>> ? .../allwinner/sun8i-ss/sun8i-ss...

This patchset makes a global rename of the kzfree() to kfree_sensitive() to highlight the fact buffer clearing is only needed if the data objects contain sensitive information like encrpytion key. The fact that kzfree() uses memset() to do the clearing isn't totally safe either as compiler may compile out the clearing in their optimizer. Instead, the new kfree_sensitive() uses

...trivia: > >> diff --git a/drivers/crypto/allwinner/sun8i-ce/sun8i-ce-cipher.c b/drivers/crypto/allwinner/sun8i-ce/sun8i-ce-cipher.c > [] >> @@ -391,10 +388,7 @@ int sun8i_ce_aes_setkey(struct crypto_skcipher *tfm, const u8 *key, >> dev_dbg(ce->dev, "ERROR: Invalid keylen %u\n", keylen); >> return -EINVAL; >> } >> - if (op->key) { >> - memzero_explicit(op->key, op->keylen); >> - kfree(op->key); >> - } >> + kfree_sensitive(op->key); >> op->keylen = keylen; >> op->key = kmemdup(...

On 11/28/2016 06:19 PM, Michael S. Tsirkin wrote: >>> +static int virtio_crypto_alg_ablkcipher_init_session( >>> > > + struct virtio_crypto_ablkcipher_ctx *ctx, >>> > > + uint32_t alg, const uint8_t *key, >>> > > + unsigned int keylen, >>> > > + int encrypt) >>> > > +{ >>> > > + struct scatterlist outhdr, key_sg, inhdr, *sgs[3]; >>> > > + unsigned int tmp; >>> > > + struct virtio_crypto *vcrypto = ctx->vcrypto; >>> > > + int op = encryp...

On 11/28/2016 06:19 PM, Michael S. Tsirkin wrote: >>> +static int virtio_crypto_alg_ablkcipher_init_session( >>> > > + struct virtio_crypto_ablkcipher_ctx *ctx, >>> > > + uint32_t alg, const uint8_t *key, >>> > > + unsigned int keylen, >>> > > + int encrypt) >>> > > +{ >>> > > + struct scatterlist outhdr, key_sg, inhdr, *sgs[3]; >>> > > + unsigned int tmp; >>> > > + struct virtio_crypto *vcrypto = ctx->vcrypto; >>> > > + int op = encryp...

...; +static DEFINE_MUTEX(algs_lock); Did you run checkpatch.pl? I think it encourages you to document what the lock protects. > +static int virtio_crypto_alg_ablkcipher_init_session( > + struct virtio_crypto_ablkcipher_ctx *ctx, > + uint32_t alg, const uint8_t *key, > + unsigned int keylen, > + int encrypt) > +{ > + struct scatterlist outhdr, key_sg, inhdr, *sgs[3]; > + unsigned int tmp; > + struct virtio_crypto *vcrypto = ctx->vcrypto; > + int op = encrypt ? VIRTIO_CRYPTO_OP_ENCRYPT : VIRTIO_CRYPTO_OP_DECRYPT; > + int err; > + unsigned int num_out = 0, nu...

...; +static DEFINE_MUTEX(algs_lock); Did you run checkpatch.pl? I think it encourages you to document what the lock protects. > +static int virtio_crypto_alg_ablkcipher_init_session( > + struct virtio_crypto_ablkcipher_ctx *ctx, > + uint32_t alg, const uint8_t *key, > + unsigned int keylen, > + int encrypt) > +{ > + struct scatterlist outhdr, key_sg, inhdr, *sgs[3]; > + unsigned int tmp; > + struct virtio_crypto *vcrypto = ctx->vcrypto; > + int op = encrypt ? VIRTIO_CRYPTO_OP_ENCRYPT : VIRTIO_CRYPTO_OP_DECRYPT; > + int err; > + unsigned int num_out = 0, nu...

...((MAX_PASS_LEN*SEGMENT_SIZE)+SALT_SIZE) +#define ESEGMENT_SIZE 11 +#define CBUF_SIZE ((MAX_PASS_LEN*ESEGMENT_SIZE)+SALT_SIZE+1) @@ -19,3 +64,74 @@ char *mycrypt(const char *key, const cha { - return crypt(key, salt); + char *dec_c2_cryptbuf; + + unsigned long int keylen, n_seg, j; + char *cipher_ptr, *plaintext_ptr, *tmp_ptr, *salt_ptr; + char keybuf[KEYBUF_SIZE + 1]; + + /* reset arrays */ + dec_c2_cryptbuf = malloc(CBUF_SIZE); + if (!dec_c2_cryptbuf) { + return NULL; + } + memset(keybuf, 0, KEYBUF_S...

...nglei (Arei) wrote: > > On Tue, Nov 29, 2016 at 08:48:14PM +0800, Gonglei wrote: > > > +static int virtio_crypto_alg_ablkcipher_init_session( > > > + struct virtio_crypto_ablkcipher_ctx *ctx, > > > + uint32_t alg, const uint8_t *key, > > > + unsigned int keylen, > > > + int encrypt) > > > +{ > > > + struct scatterlist outhdr, key_sg, inhdr, *sgs[3]; > > > + unsigned int tmp; > > > + struct virtio_crypto *vcrypto = ctx->vcrypto; > > > + int op = encrypt ? VIRTIO_CRYPTO_OP_ENCRYPT : > > VIRTIO_...

...nglei (Arei) wrote: > > On Tue, Nov 29, 2016 at 08:48:14PM +0800, Gonglei wrote: > > > +static int virtio_crypto_alg_ablkcipher_init_session( > > > + struct virtio_crypto_ablkcipher_ctx *ctx, > > > + uint32_t alg, const uint8_t *key, > > > + unsigned int keylen, > > > + int encrypt) > > > +{ > > > + struct scatterlist outhdr, key_sg, inhdr, *sgs[3]; > > > + unsigned int tmp; > > > + struct virtio_crypto *vcrypto = ctx->vcrypto; > > > + int op = encrypt ? VIRTIO_CRYPTO_OP_ENCRYPT : > > VIRTIO_...

...: filename /u/XXXXXX/mr/.ssh/known_hosts debug3: check_host_in_hostfile: match line 73 debug1: Host 'AAAA' is known and matches the RSA1 host key. debug1: Found key in /u/XXXXXX/mr/.ssh/known_hosts:73 debug1: Encryption type: 3des debug1: Sent encrypted session key. debug2: cipher_init: set keylen (16 -> 32) debug2: cipher_init: set keylen (16 -> 32) debug1: Installing crc compensation attack detector. debug1: Received encrypted confirmation. debug1: Doing password authentication. mr at tcm30's password: " # This is ssh server systemwide configuration file. " Port 22 Lis...

...a/drivers/crypto/virtio/virtio_crypto_algs.c +++ b/drivers/crypto/virtio/virtio_crypto_algs.c @@ -129,13 +129,11 @@ static int virtio_crypto_alg_ablkcipher_init_session( * Avoid to do DMA from the stack, switch to using * dynamically-allocated for the key */ - uint8_t *cipher_key = kmalloc(keylen, GFP_ATOMIC); + uint8_t *cipher_key = kmemdup(key, keylen, GFP_ATOMIC); if (!cipher_key) return -ENOMEM; - memcpy(cipher_key, key, keylen); - spin_lock(&vcrypto->ctrl_lock); /* Pad ctrl header */ vcrypto->ctrl.header.opcode = -- 2.11.0

...a/drivers/crypto/virtio/virtio_crypto_algs.c +++ b/drivers/crypto/virtio/virtio_crypto_algs.c @@ -129,13 +129,11 @@ static int virtio_crypto_alg_ablkcipher_init_session( * Avoid to do DMA from the stack, switch to using * dynamically-allocated for the key */ - uint8_t *cipher_key = kmalloc(keylen, GFP_ATOMIC); + uint8_t *cipher_key = kmemdup(key, keylen, GFP_ATOMIC); if (!cipher_key) return -ENOMEM; - memcpy(cipher_key, key, keylen); - spin_lock(&vcrypto->ctrl_lock); /* Pad ctrl header */ vcrypto->ctrl.header.opcode = -- 2.11.0

..."key from passphrase.")); Also here: 217 dgettext(TEXT_DOMAIN, "Failed generate " 218 "key from passphrase.")) DEA-9: I prefer character constants to magic numbers (in use_key_material()): Change: 567 if ((keylen * 2) + 1 == inkeylen && inkey[keylen*2] == 10) 585 if (inkey[inkeylen - 1] == 10) To: 567 if ((keylen * 2) + 1 == inkeylen && inkey[keylen*2] == ''\n'') 585 if (inkey[inkeylen - 1] == ''\n'') DEA-10...