| Message ID | 20190214080355.8112-6-ebiggers@kernel.org (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Herbert Xu |
| Headers | show |
| Series | crypto: test that CBC and CTR update the IV | expand |
On Thu, 14 Feb 2019 at 09:04, Eric Biggers <ebiggers@kernel.org> wrote: > > From: Eric Biggers <ebiggers@google.com> > > Make the arm64 ctr-aes-neon and ctr-aes-ce algorithms update the IV > buffer to contain the next counter after processing a partial final > block, rather than leave it as the last counter. This makes these > algorithms pass the updated AES-CTR tests. > > Signed-off-by: Eric Biggers <ebiggers@google.com> I take it this means we return an output IV even if the algorithm could never proceed in a meaningful way, given that we throw away some keystream bits that would be needed in that case. That means this change is strictly there to make the test framework happy, even for cases that can never appear in reality. Wouldn't it be better not to set out_iv for input buffers whose size is not a multiple of the block size? > --- > arch/arm64/crypto/aes-modes.S | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/arch/arm64/crypto/aes-modes.S b/arch/arm64/crypto/aes-modes.S > index 67700045a0e0..4c7ce231963c 100644 > --- a/arch/arm64/crypto/aes-modes.S > +++ b/arch/arm64/crypto/aes-modes.S > @@ -320,8 +320,7 @@ AES_ENTRY(aes_ctr_encrypt) > > .Lctrtailblock: > st1 {v0.16b}, [x0] > - ldp x29, x30, [sp], #16 > - ret > + b .Lctrout > > .Lctrcarry: > umov x7, v4.d[0] /* load upper word of ctr */ > -- > 2.20.1 >
On Thu, Feb 14, 2019 at 09:14:13AM +0100, Ard Biesheuvel wrote: > On Thu, 14 Feb 2019 at 09:04, Eric Biggers <ebiggers@kernel.org> wrote: > > > > From: Eric Biggers <ebiggers@google.com> > > > > Make the arm64 ctr-aes-neon and ctr-aes-ce algorithms update the IV > > buffer to contain the next counter after processing a partial final > > block, rather than leave it as the last counter. This makes these > > algorithms pass the updated AES-CTR tests. > > > > Signed-off-by: Eric Biggers <ebiggers@google.com> > > I take it this means we return an output IV even if the algorithm > could never proceed in a meaningful way, given that we throw away some > keystream bits that would be needed in that case. > > That means this change is strictly there to make the test framework > happy, even for cases that can never appear in reality. > > Wouldn't it be better not to set out_iv for input buffers whose size > is not a multiple of the block size? > See the explanation in patch 4 for why the tests test for this. It's not a super strong argument but this seems like the best thing to do. - Eric
On Thu, 14 Feb 2019 at 09:28, Eric Biggers <ebiggers@kernel.org> wrote: > > On Thu, Feb 14, 2019 at 09:14:13AM +0100, Ard Biesheuvel wrote: > > On Thu, 14 Feb 2019 at 09:04, Eric Biggers <ebiggers@kernel.org> wrote: > > > > > > From: Eric Biggers <ebiggers@google.com> > > > > > > Make the arm64 ctr-aes-neon and ctr-aes-ce algorithms update the IV > > > buffer to contain the next counter after processing a partial final > > > block, rather than leave it as the last counter. This makes these > > > algorithms pass the updated AES-CTR tests. > > > > > > Signed-off-by: Eric Biggers <ebiggers@google.com> > > > > I take it this means we return an output IV even if the algorithm > > could never proceed in a meaningful way, given that we throw away some > > keystream bits that would be needed in that case. > > > > That means this change is strictly there to make the test framework > > happy, even for cases that can never appear in reality. > > > > Wouldn't it be better not to set out_iv for input buffers whose size > > is not a multiple of the block size? > > > > See the explanation in patch 4 for why the tests test for this. It's not a > super strong argument but this seems like the best thing to do. > Fair enough. Do you have a branch with this stuff that I can drop into kernelci again? Preferably one that already has the tests enabled by default, and panics on failure.
On Thu, Feb 14, 2019 at 09:33:51AM +0100, Ard Biesheuvel wrote: > On Thu, 14 Feb 2019 at 09:28, Eric Biggers <ebiggers@kernel.org> wrote: > > > > On Thu, Feb 14, 2019 at 09:14:13AM +0100, Ard Biesheuvel wrote: > > > On Thu, 14 Feb 2019 at 09:04, Eric Biggers <ebiggers@kernel.org> wrote: > > > > > > > > From: Eric Biggers <ebiggers@google.com> > > > > > > > > Make the arm64 ctr-aes-neon and ctr-aes-ce algorithms update the IV > > > > buffer to contain the next counter after processing a partial final > > > > block, rather than leave it as the last counter. This makes these > > > > algorithms pass the updated AES-CTR tests. > > > > > > > > Signed-off-by: Eric Biggers <ebiggers@google.com> > > > > > > I take it this means we return an output IV even if the algorithm > > > could never proceed in a meaningful way, given that we throw away some > > > keystream bits that would be needed in that case. > > > > > > That means this change is strictly there to make the test framework > > > happy, even for cases that can never appear in reality. > > > > > > Wouldn't it be better not to set out_iv for input buffers whose size > > > is not a multiple of the block size? > > > > > > > See the explanation in patch 4 for why the tests test for this. It's not a > > super strong argument but this seems like the best thing to do. > > > > Fair enough. > > Do you have a branch with this stuff that I can drop into kernelci > again? Preferably one that already has the tests enabled by default, > and panics on failure. I pushed it out to https://git.kernel.org/pub/scm/linux/kernel/git/ebiggers/linux.git branch "iv-out-testing", and added a hack to enable self-tests by default and panic on test failure. - Eric
diff --git a/arch/arm64/crypto/aes-modes.S b/arch/arm64/crypto/aes-modes.S index 67700045a0e0..4c7ce231963c 100644 --- a/arch/arm64/crypto/aes-modes.S +++ b/arch/arm64/crypto/aes-modes.S @@ -320,8 +320,7 @@ AES_ENTRY(aes_ctr_encrypt) .Lctrtailblock: st1 {v0.16b}, [x0] - ldp x29, x30, [sp], #16 - ret + b .Lctrout .Lctrcarry: umov x7, v4.d[0] /* load upper word of ctr */