Re: [PATCH] crypto: Fix next IV issue for CTS template

On 17 Feb 2017, at 09:17, Dennis Chen [off-list ref] wrote:

Hello Ard,
Morning!

quoted

On Fri, Feb 17, 2017 at 07:12:46AM +0000, Ard Biesheuvel wrote:
Hello Libo,

quoted

On 17 February 2017 at 03:47,  [off-list ref] wrote:
From: Libo Wang <Libo Wang@arm.com>

CTS template assumes underlying CBC algorithm will carry out next IV for
further process.But some implementations of CBC algorithm in kernel break
this assumption, for example, some hardware crypto drivers ignore next IV
for performance consider, inthis case, tcry cts(cbc(aes)) test case will
fail. This patch is trying to fix it by getting next IV information ready
before last two blocks processed.

Signed-off-by: Libo Wang <redacted>
Signed-off-by: Dennis Chen <redacted>

Which algorithms in particular break this assumption? I recently fixed
some ARM accelerated software drivers for this reason. If there are
others, we should fix those rather than try to fix it in the CTS
driver.

There're some ARM HW accelerated drivers which are not upstream yet(IP license)
breaks this assumption. The current CTS template requires the underlying CBC 
algorithm always fill the next IV, but in some cases like CBC standalone mode, 
it doesn't need to fill the next IV, so apparently it will induce performance
degrade from the point of drivers, that's why we fix it in the CTS template,except
that it will also mitigate the potential defect of other HW-based CBC drivers. 

You should fix this in the driver, not in the cts code.

quoted

quoted

---
crypto/cts.c | 29 +++++++++++++++++++++++++----
1 file changed, 25 insertions(+), 4 deletions(-)

diff --git a/crypto/cts.c b/crypto/cts.c
index a1335d6..712164b 100644
--- a/crypto/cts.c
+++ b/crypto/cts.c

@@ -154,6 +154,7 @@ static int crypto_cts_encrypt(struct skcipher_request *req)
       unsigned int nbytes = req->cryptlen;
       int cbc_blocks = (nbytes + bsize - 1) / bsize - 1;
       unsigned int offset;
+       int ret = 0;

       skcipher_request_set_tfm(subreq, ctx->child);

@@ -174,8 +175,17 @@ static int crypto_cts_encrypt(struct skcipher_request *req)
       skcipher_request_set_crypt(subreq, req->src, req->dst,
                                  offset, req->iv);

-       return crypto_skcipher_encrypt(subreq) ?:
-              cts_cbc_encrypt(req);
+       /* process CBC blocks */
+       ret = crypto_skcipher_encrypt(subreq);
+       /* process last two blocks */
+       if (!ret) {

What happens if an async driver returns -EINPROGRESS here?

For this case, the CTS will return -EINPROGRESS directly.

I see. But how do you handle the missing out IV when the async call completes?

quoted

quoted

+               /* Get IVn-1 back */
+               scatterwalk_map_and_copy(req->iv, req->dst, (offset - bsize), bsize, 0);
+               /* Continue last two blocks */
+               return cts_cbc_encrypt(req);
+       }
+
+       return ret;
}

static int cts_cbc_decrypt(struct skcipher_request *req)

@@ -248,6 +258,8 @@ static int crypto_cts_decrypt(struct skcipher_request *req)
       int cbc_blocks = (nbytes + bsize - 1) / bsize - 1;
       unsigned int offset;
       u8 *space;
+       int ret = 0;
+       u8 iv_next[bsize];

       skcipher_request_set_tfm(subreq, ctx->child);

@@ -277,8 +289,17 @@ static int crypto_cts_decrypt(struct skcipher_request *req)
       skcipher_request_set_crypt(subreq, req->src, req->dst,
                                  offset, req->iv);

-       return crypto_skcipher_decrypt(subreq) ?:
-              cts_cbc_decrypt(req);
+       /* process last two blocks */
+       scatterwalk_map_and_copy(iv_next, req->src, (offset - bsize), bsize, 0);
+       ret = crypto_skcipher_decrypt(subreq);
+       if (!ret) {
+               /* Set Next IV */
+               subreq->iv = iv_next;
+               /* process last two blocks */
+               return cts_cbc_decrypt(req);
+       }
+
+       return ret;

}

static int crypto_cts_init_tfm(struct crypto_skcipher *tfm)
--
1.8.3.1