Skip to contentSkip to conversation

crypto: arm/chacha-neon - add missing counter increment

PATCH In review Ard Biesheuvel 3 participants 6 messages Dec 13, 2020 – Jan 2, 2021

Subsystems (2)
Arm (32-bit) Crypto API
lore
Share & tools
lore

Ard Biesheuvel

author
#0 lore raw

PATCH v2 1/1 crypto: arm/chacha-neon - add missing counter increment

Commit 86cd97ec4b943af3 ("crypto: arm/chacha-neon - optimize for non-block size multiples") refactored the chacha block handling in the glue code in a way that may result in the counter increment to be omitted when calling chacha_block_xor_neon() to process a full block. This violates the skcipher API, which requires that the output IV is suitable for handling more input as long as the preceding input has been presented in round multiples of the block size. Also, the same code is exposed via the chacha library interface whose callers may actually rely on this increment to occur even for final blocks that are smaller than the chacha block size.

So increment the counter after calling chacha_block_xor_neon().

Fixes 86cd97ec4b943af3 ("crypto: arm/chacha-neon - optimize for non-block size multiples")
Reported-by Eric Biggers <[email protected]>
Signed-off-by Ard Biesheuvel <[email protected]>

---
v2: - use ++ instead of += 1
- make note in the commit log of the fact that the library API needs the increment to occur in all cases, not only for final blocks whose size is exactly the block size

 arch/arm/crypto/chacha-glue.c | 1 +
 1 file changed, 1 insertion(+)
arch/arm/crypto/chacha-glue.c +1 -0
index 7b5cf8430c6d..cdde8fd01f8f 100644
@@ -60,6 +60,7 @@ static void chacha_doneon(u32 *state, u8 *dst, const u8 *src,
 		chacha_block_xor_neon(state, d, s, nrounds);
 		if (d != dst)
 			memcpy(dst, buf, bytes);
+		state[12]++;
 	}
 }
Signature · 1 line
2.17.1
#1 lore raw
Quoted 3 lines · Ard Biesheuvel

This violates the skcipher API, which requires that the output IV is suitable for handling more input as long as the preceding input has been presented in round multiples of the block size.

This part doesn't seem to be true, since the chacha implementations don't implement the "output IV" thing. It's only cbc and ctr that do (or at least those are the only algorithms it's tested for).

- Eric

maintainer
#2 lore raw
Quoted 4 lines · Eric Biggers

This part doesn't seem to be true, since the chacha implementations don't implement the "output IV" thing. It's only cbc and ctr that do (or at least those are the only algorithms it's tested for).

If this algorithm can be used through algif_skcipher then it will be making use of the output IV.

Cheers,

Signature · 3 lines
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
#3 lore raw
Quoted 9 lines · Herbert Xu
Quoted 4 lines · Eric Biggers

This part doesn't seem to be true, since the chacha implementations don't implement the "output IV" thing. It's only cbc and ctr that do (or at least those are the only algorithms it's tested for).

If this algorithm can be used through algif_skcipher then it will be making use of the output IV.

That doesn't make sense, given that most algorithms don't implement it...

- Eric

maintainer
#4 lore raw
Quoted 2 lines · Eric Biggers

That doesn't make sense, given that most algorithms don't implement it...

It doesn't matter for chacha, but it's still true for the Crypto API in general.

Yes I know that lots of algorithms get it wrong, but it is on my list of things to fix.

Cheers,

Signature · 3 lines
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
maintainer
#5 lore raw
In reply to Ard Biesheuvel
Quoted 15 lines · Ard Biesheuvel

Commit 86cd97ec4b943af3 ("crypto: arm/chacha-neon - optimize for non-block size multiples") refactored the chacha block handling in the glue code in a way that may result in the counter increment to be omitted when calling chacha_block_xor_neon() to process a full block. This violates the skcipher API, which requires that the output IV is suitable for handling more input as long as the preceding input has been presented in round multiples of the block size. Also, the same code is exposed via the chacha library interface whose callers may actually rely on this increment to occur even for final blocks that are smaller than the chacha block size.

So increment the counter after calling chacha_block_xor_neon().

Fixes 86cd97ec4b943af3 ("crypto: arm/chacha-neon - optimize for non-block size multiples")
Reported-by Eric Biggers <[email protected]>
Signed-off-by Ard Biesheuvel <[email protected]>
Quoted 8 lines

---
v2: - use ++ instead of += 1
- make note in the commit log of the fact that the library API needs the increment to occur in all cases, not only for final blocks whose size is exactly the block size

 arch/arm/crypto/chacha-glue.c | 1 +
 1 file changed, 1 insertion(+)

Patch applied. Thanks.

Signature · 3 lines
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt