| Message ID | 20190518213035.21699-1-chunkeey@gmail.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Herbert Xu |
| Headers | show |
| Series | [RFC] crypto: skcipher - perform len fits into blocksize check at the top | expand |
Hi Christian, On Sat, May 18, 2019 at 11:30:35PM +0200, Christian Lamparter wrote: > This patch adds early check to test the given data length > is aligned with the supported ciphers blocksize, or abort > with -EINVAL in case the supplied chunk size does not fit > without padding into the blocksize for block ciphers. > > Signed-off-by: Christian Lamparter <chunkeey@gmail.com> > > --- > > This will also work instead of the > "crypto: crypto4xx - blockciphers should only accept complete blocks" > It will fix all potential driver issues in other drivers as well as > break the drivers that don't have the correct blocksize set. it will > also make the extra checks scattered around in the drivers make > redundand as well as the extra tests that do send incomplete blocks > to the hardware drivers. > --- > include/crypto/skcipher.h | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/include/crypto/skcipher.h b/include/crypto/skcipher.h > index e555294ed77f..971294602a41 100644 > --- a/include/crypto/skcipher.h > +++ b/include/crypto/skcipher.h > @@ -494,6 +494,8 @@ static inline int crypto_skcipher_encrypt(struct skcipher_request *req) > crypto_stats_get(alg); > if (crypto_skcipher_get_flags(tfm) & CRYPTO_TFM_NEED_KEY) > ret = -ENOKEY; > + else if (!IS_ALIGNED(cryptlen, crypto_skcipher_blocksize(tfm))) > + ret = -EINVAL; > else > ret = tfm->encrypt(req); > crypto_stats_skcipher_encrypt(cryptlen, ret, alg); > @@ -521,6 +523,8 @@ static inline int crypto_skcipher_decrypt(struct skcipher_request *req) > crypto_stats_get(alg); > if (crypto_skcipher_get_flags(tfm) & CRYPTO_TFM_NEED_KEY) > ret = -ENOKEY; > + else if (!IS_ALIGNED(cryptlen, crypto_skcipher_blocksize(tfm))) > + ret = -EINVAL; > else > ret = tfm->decrypt(req); > crypto_stats_skcipher_decrypt(cryptlen, ret, alg); > -- Unfortunately this doesn't work because there are some skcipher algorithms ("cts" and "adiantum") that set cra_blocksize to their minimum message length (16) rather than their length alignment requirement (1). I.e., these algorithms support all messages with length >= 16 bytes. So before we can add this check, we need to get this straightened out. I actually don't think "block size" should be a property of all crypto algorithms at all, e.g. for skciphers we should instead have "min_msgsize" and "msgsize_alignment". The notion of "block size" is not meaningful for length-preserving encryption algorithms in general, let alone all crypto algorithms. But unfortunately "block size" is pretty deeply embedded into the crypto API, so it doesn't look easy to fix this... Maybe for skciphers we could get away with it always meaning the msgsize_alignment, though. - Eric
diff --git a/include/crypto/skcipher.h b/include/crypto/skcipher.h index e555294ed77f..971294602a41 100644 --- a/include/crypto/skcipher.h +++ b/include/crypto/skcipher.h @@ -494,6 +494,8 @@ static inline int crypto_skcipher_encrypt(struct skcipher_request *req) crypto_stats_get(alg); if (crypto_skcipher_get_flags(tfm) & CRYPTO_TFM_NEED_KEY) ret = -ENOKEY; + else if (!IS_ALIGNED(cryptlen, crypto_skcipher_blocksize(tfm))) + ret = -EINVAL; else ret = tfm->encrypt(req); crypto_stats_skcipher_encrypt(cryptlen, ret, alg); @@ -521,6 +523,8 @@ static inline int crypto_skcipher_decrypt(struct skcipher_request *req) crypto_stats_get(alg); if (crypto_skcipher_get_flags(tfm) & CRYPTO_TFM_NEED_KEY) ret = -ENOKEY; + else if (!IS_ALIGNED(cryptlen, crypto_skcipher_blocksize(tfm))) + ret = -EINVAL; else ret = tfm->decrypt(req); crypto_stats_skcipher_decrypt(cryptlen, ret, alg);
This patch adds early check to test the given data length is aligned with the supported ciphers blocksize, or abort with -EINVAL in case the supplied chunk size does not fit without padding into the blocksize for block ciphers. Signed-off-by: Christian Lamparter <chunkeey@gmail.com> --- This will also work instead of the "crypto: crypto4xx - blockciphers should only accept complete blocks" It will fix all potential driver issues in other drivers as well as break the drivers that don't have the correct blocksize set. it will also make the extra checks scattered around in the drivers make redundand as well as the extra tests that do send incomplete blocks to the hardware drivers. --- include/crypto/skcipher.h | 4 ++++ 1 file changed, 4 insertions(+)