2019-07-26 20:26:44

by Horia Geanta

[permalink] [raw]
Subject: Re: [dm-devel] xts fuzz testing and lack of ciphertext stealing support

On 7/26/2019 1:31 PM, Pascal Van Leeuwen wrote:
> Ok, find below a patch file that adds your vectors from the specification
> plus my set of additional vectors covering all CTS alignments combined
> with the block sizes you desired. Please note though that these vectors
> are from our in-house home-grown model so no warranties.
I've checked the test vectors against caam (HW + driver).

Test vectors from IEEE 1619-2007 (i.e. up to and including "XTS-AES 18")
are fine.

caam complains when /* Additional vectors to increase CTS coverage */
section starts:
alg: skcipher: xts-aes-caam encryption test failed (wrong result) on test vector 9, cfg="in-place"

(Unfortunately it seems that testmgr lost the capability of dumping
the incorrect output.)

IMO we can't rely on test vectors if they are not taken
straight out of a spec, or cross-checked with other implementations.

Horia

> They do run fine on the inside-secure driver + HW though, and I hereby
> donate them to the public domain i.e. feel free to use them as you see fit.
> (in case Outlook 365 messed up the patch below, it's also available from
> my public Git: https://github.com/pvanleeuwen/linux-cryptodev.git,
> branch is_driver_patch2)
>
> --
>
> This patch adds testvectors for AES-XTS that cover data inputs that are
> not a multiple of 16 bytes and therefore require cipher text stealing
> (CTS) to be applied. Vectors were added to cover all possible alignments
> combined with various interesting (i.e. for vector implementations working
> on 3,4,5 or 8 AES blocks in parallel) lengths.
>
> Signed-off-by: Pascal van Leeuwen <[email protected]>
> ---
> crypto/testmgr.h | 368 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 368 insertions(+)
>
> diff --git a/crypto/testmgr.h b/crypto/testmgr.h
> index 105f2ce..1046e47 100644
> --- a/crypto/testmgr.h
> +++ b/crypto/testmgr.h
> @@ -15594,6 +15594,374 @@ struct len_range_sel {
> "\xc4\xf3\x6f\xfd\xa9\xfc\xea\x70"
> "\xb9\xc6\xe6\x93\xe1\x48\xc1\x51",
> .len = 512,
> + }, { /* XTS-AES 15 */
> + .key = "\xff\xfe\xfd\xfc\xfb\xfa\xf9\xf8"
> + "\xf7\xf6\xf5\xf4\xf3\xf2\xf1\xf0"
> + "\xbf\xbe\xbd\xbc\xbb\xba\xb9\xb8"
> + "\xb7\xb6\xb5\xb4\xb3\xb2\xb1\xb0",
> + .klen = 32,
> + .iv = "\x9a\x78\x56\x34\x12\x00\x00\x00"
> + "\x00\x00\x00\x00\x00\x00\x00\x00",
> + .ptext = "\x00\x01\x02\x03\x04\x05\x06\x07"
> + "\x08\x09\x0a\x0b\x0c\x0d\x0e\x0f"
> + "\x10",
> + .ctext = "\x6c\x16\x25\xdb\x46\x71\x52\x2d"
> + "\x3d\x75\x99\x60\x1d\xe7\xca\x09"
> + "\xed",
> + .len = 17,
[...]
> + }, { /* XTS-AES 18 */
> + .key = "\xff\xfe\xfd\xfc\xfb\xfa\xf9\xf8"
> + "\xf7\xf6\xf5\xf4\xf3\xf2\xf1\xf0"
> + "\xbf\xbe\xbd\xbc\xbb\xba\xb9\xb8"
> + "\xb7\xb6\xb5\xb4\xb3\xb2\xb1\xb0",
> + .klen = 32,
> + .iv = "\x9a\x78\x56\x34\x12\x00\x00\x00"
> + "\x00\x00\x00\x00\x00\x00\x00\x00",
> + .ptext = "\x00\x01\x02\x03\x04\x05\x06\x07"
> + "\x08\x09\x0a\x0b\x0c\x0d\x0e\x0f"
> + "\x10\x11\x12\x13",
> + .ctext = "\x9d\x84\xc8\x13\xf7\x19\xaa\x2c"
> + "\x7b\xe3\xf6\x61\x71\xc7\xc5\xc2"
> + "\xed\xbf\x9d\xac",
> + .len = 20,
> + /* Additional vectors to increase CTS coverage */
> + }, { /* 1 block + 21 bytes */
> + .key = "\xa1\x34\x0e\x49\x38\xfd\x8b\xf6"
> + "\x45\x60\x67\x07\x0f\x50\xa8\x2b"
> + "\xa8\xf1\xfe\x7e\xf4\xf0\x47\xcd"
> + "\xfd\x91\x78\xf9\x14\x8b\x7d\x27"
> + "\x0e\xdc\xca\xe6\xf4\xfc\xd7\x4f"
> + "\x19\x8c\xd0\xe6\x9e\x2f\xf8\x75"
> + "\xb5\xe2\x48\x00\x4f\x07\xd9\xa1"
> + "\x42\xbc\x9d\xfc\x17\x98\x00\x48",
> + .klen = 64,
> + .iv = "\xcb\x35\x47\x5a\x7a\x06\x28\xb9"
> + "\x80\xf5\xa7\xe6\x8a\x23\x42\xf8",
> + .ptext = "\x04\x52\xc8\x7f\xb0\x5a\x12\xc5"
> + "\x96\x47\x6b\xf4\xbc\x2e\xdb\x74"
> + "\xd2\x20\x24\x32\xe5\x84\xb6\x25"
> + "\x4c\x2f\x96\xc7\x55\x9c\x90\x6f"
> + "\x0e\x96\x94\x68\xf4",
> + .ctext = "\x6a\x2d\x57\xb8\x72\x49\x10\x6b"
> + "\x5b\x5a\xc9\x92\xab\x59\x79\x36"
> + "\x7a\x01\x95\xf7\xdd\xcb\x3f\xbf"
> + "\xb2\xe3\x7e\x35\xe3\x11\x04\x68"
> + "\x28\xc3\x70\x6a\xe1",
> + .len = 37,
> + }, { /* 3 blocks + 22 bytes */
[...]


2019-07-26 21:45:09

by Pascal Van Leeuwen

[permalink] [raw]
Subject: RE: [dm-devel] xts fuzz testing and lack of ciphertext stealing support

> -----Original Message-----
> From: Horia Geanta <[email protected]>
> Sent: Friday, July 26, 2019 9:59 PM
> To: Pascal Van Leeuwen <[email protected]>; Ard Biesheuvel <[email protected]>
> Cc: Milan Broz <[email protected]>; Herbert Xu <[email protected]>; [email protected]; linux-
> [email protected]
> Subject: Re: [dm-devel] xts fuzz testing and lack of ciphertext stealing support
>
> On 7/26/2019 1:31 PM, Pascal Van Leeuwen wrote:
> > Ok, find below a patch file that adds your vectors from the specification
> > plus my set of additional vectors covering all CTS alignments combined
> > with the block sizes you desired. Please note though that these vectors
> > are from our in-house home-grown model so no warranties.
> I've checked the test vectors against caam (HW + driver).
>
> Test vectors from IEEE 1619-2007 (i.e. up to and including "XTS-AES 18")
> are fine.
>
> caam complains when /* Additional vectors to increase CTS coverage */
> section starts:
> alg: skcipher: xts-aes-caam encryption test failed (wrong result) on test vector 9, cfg="in-place"
>
> (Unfortunately it seems that testmgr lost the capability of dumping
> the incorrect output.)
>
> IMO we can't rely on test vectors if they are not taken
> straight out of a spec, or cross-checked with other implementations.
>

First off, I fully agree with your statement, which is why I did not post this as a straight
patch. The problem is that specification vectors usually (or actuaclly, always) don't cover
all the relevant corner cases needed for verification. And "reference" implementations
by academics are usually shady at best as well.

In this particular case, the reference vectors only cover 5 out of 16 possible alignment
cases and the current situation proves that this is not sufficient. As we have 2 imple-
mentations (or actually more, if you count the models used for vector generation)
that are considered to be correct that disagree on results.

Which is very interesting, because which one is correct? I know that our model and
hardware implementation were independently developed (by 2 different engineers)
from the IEEE spec and match on results. And our hardware has been used "out in
the field" for many years already in disk controllers from major silicon vendors.
But that's still not a guarantee .... So how do we resolve this? Majority vote? ;-)

> Horia
>

Regards,
Pascal van Leeuwen
Silicon IP Architect, Multi-Protocol Engines @ Verimatrix
http://www.insidesecure.com


2019-07-27 05:39:53

by Ard Biesheuvel

[permalink] [raw]
Subject: Re: [dm-devel] xts fuzz testing and lack of ciphertext stealing support

On Sat, 27 Jul 2019 at 00:43, Pascal Van Leeuwen
<[email protected]> wrote:
>
> > -----Original Message-----
> > From: Horia Geanta <[email protected]>
> > Sent: Friday, July 26, 2019 9:59 PM
> > To: Pascal Van Leeuwen <[email protected]>; Ard Biesheuvel <[email protected]>
> > Cc: Milan Broz <[email protected]>; Herbert Xu <[email protected]>; [email protected]; linux-
> > [email protected]
> > Subject: Re: [dm-devel] xts fuzz testing and lack of ciphertext stealing support
> >
> > On 7/26/2019 1:31 PM, Pascal Van Leeuwen wrote:
> > > Ok, find below a patch file that adds your vectors from the specification
> > > plus my set of additional vectors covering all CTS alignments combined
> > > with the block sizes you desired. Please note though that these vectors
> > > are from our in-house home-grown model so no warranties.
> > I've checked the test vectors against caam (HW + driver).
> >
> > Test vectors from IEEE 1619-2007 (i.e. up to and including "XTS-AES 18")
> > are fine.
> >
> > caam complains when /* Additional vectors to increase CTS coverage */
> > section starts:
> > alg: skcipher: xts-aes-caam encryption test failed (wrong result) on test vector 9, cfg="in-place"
> >
> > (Unfortunately it seems that testmgr lost the capability of dumping
> > the incorrect output.)
> >
> > IMO we can't rely on test vectors if they are not taken
> > straight out of a spec, or cross-checked with other implementations.
> >
>
> First off, I fully agree with your statement, which is why I did not post this as a straight
> patch. The problem is that specification vectors usually (or actuaclly, always) don't cover
> all the relevant corner cases needed for verification. And "reference" implementations
> by academics are usually shady at best as well.
>
> In this particular case, the reference vectors only cover 5 out of 16 possible alignment
> cases and the current situation proves that this is not sufficient. As we have 2 imple-
> mentations (or actually more, if you count the models used for vector generation)
> that are considered to be correct that disagree on results.
>
> Which is very interesting, because which one is correct? I know that our model and
> hardware implementation were independently developed (by 2 different engineers)
> from the IEEE spec and match on results. And our hardware has been used "out in
> the field" for many years already in disk controllers from major silicon vendors.
> But that's still not a guarantee .... So how do we resolve this? Majority vote? ;-)
>

Thanks for the additional test vectors. They work fine with my SIMD
implementations for ARM [0], so this looks like it might be a CAAM
problem, not a problem with the test vectors.

I will try to find some time today to run them through OpenSSL to double check.


[0] https://git.kernel.org/pub/scm/linux/kernel/git/ardb/linux.git/commit/?h=xts-cts

2019-07-27 12:58:29

by Pascal Van Leeuwen

[permalink] [raw]
Subject: RE: [dm-devel] xts fuzz testing and lack of ciphertext stealing support


> -----Original Message-----
> From: Ard Biesheuvel <[email protected]>
> Sent: Saturday, July 27, 2019 7:39 AM
> To: Pascal Van Leeuwen <[email protected]>
> Cc: Horia Geanta <[email protected]>; Milan Broz <[email protected]>; Herbert Xu <[email protected]>; dm-
> [email protected]; [email protected]
> Subject: Re: [dm-devel] xts fuzz testing and lack of ciphertext stealing support
>
> On Sat, 27 Jul 2019 at 00:43, Pascal Van Leeuwen
> <[email protected]> wrote:
> >
> > > -----Original Message-----
> > > From: Horia Geanta <[email protected]>
> > > Sent: Friday, July 26, 2019 9:59 PM
> > > To: Pascal Van Leeuwen <[email protected]>; Ard Biesheuvel <[email protected]>
> > > Cc: Milan Broz <[email protected]>; Herbert Xu <[email protected]>; [email protected]; linux-
> > > [email protected]
> > > Subject: Re: [dm-devel] xts fuzz testing and lack of ciphertext stealing support
> > >
> > > On 7/26/2019 1:31 PM, Pascal Van Leeuwen wrote:
> > > > Ok, find below a patch file that adds your vectors from the specification
> > > > plus my set of additional vectors covering all CTS alignments combined
> > > > with the block sizes you desired. Please note though that these vectors
> > > > are from our in-house home-grown model so no warranties.
> > > I've checked the test vectors against caam (HW + driver).
> > >
> > > Test vectors from IEEE 1619-2007 (i.e. up to and including "XTS-AES 18")
> > > are fine.
> > >
> > > caam complains when /* Additional vectors to increase CTS coverage */
> > > section starts:
> > > alg: skcipher: xts-aes-caam encryption test failed (wrong result) on test vector 9, cfg="in-place"
> > >
> > > (Unfortunately it seems that testmgr lost the capability of dumping
> > > the incorrect output.)
> > >
> > > IMO we can't rely on test vectors if they are not taken
> > > straight out of a spec, or cross-checked with other implementations.
> > >
> >
> > First off, I fully agree with your statement, which is why I did not post this as a straight
> > patch. The problem is that specification vectors usually (or actuaclly, always) don't cover
> > all the relevant corner cases needed for verification. And "reference" implementations
> > by academics are usually shady at best as well.
> >
> > In this particular case, the reference vectors only cover 5 out of 16 possible alignment
> > cases and the current situation proves that this is not sufficient. As we have 2 imple-
> > mentations (or actually more, if you count the models used for vector generation)
> > that are considered to be correct that disagree on results.
> >
> > Which is very interesting, because which one is correct? I know that our model and
> > hardware implementation were independently developed (by 2 different engineers)
> > from the IEEE spec and match on results. And our hardware has been used "out in
> > the field" for many years already in disk controllers from major silicon vendors.
> > But that's still not a guarantee .... So how do we resolve this? Majority vote? ;-)
> >
>
> Thanks for the additional test vectors. They work fine with my SIMD
> implementations for ARM [0], so this looks like it might be a CAAM
> problem, not a problem with the test vectors.
>
Thanks for the heads up! As the engineer actually responsible for our hardware
implementation, I can now sleep at night again :-)

Not that I was too worried - the design has been in active use for nearly 12 years
already without any known issues, but still ...

> I will try to find some time today to run them through OpenSSL to double check.
>
>
> [0] https://git.kernel.org/pub/scm/linux/kernel/git/ardb/linux.git/commit/?h=xts-cts

Regards,
Pascal van Leeuwen
Silicon IP Architect, Multi-Protocol Engines @ Verimatrix
http://www.insidesecure.com

2019-07-27 16:08:21

by Milan Broz

[permalink] [raw]
Subject: Re: [dm-devel] xts fuzz testing and lack of ciphertext stealing support

On 27/07/2019 07:39, Ard Biesheuvel wrote:
> Thanks for the additional test vectors. They work fine with my SIMD
> implementations for ARM [0], so this looks like it might be a CAAM
> problem, not a problem with the test vectors.
>
> I will try to find some time today to run them through OpenSSL to double check.

I shamelessly copied your test vectors to my vector test for cryptsetup backend.

Both OpenSSL and gcrypt XTS implementation passed all tests here!

If interested - this is copy of backend we have in cryptsetup, vectors added in crypto-vectors.c
(there are some hard defines in Makefile, cryptsetup uses autoconf instead).
OpenSSL: https://github.com/mbroz/cryptsetup_backend_test
gcrypt branch: https://github.com/mbroz/cryptsetup_backend_test/tree/gcrypt

Once kernel AF_ALG supports it, I can easily test it the same way.

Thanks,
Milan

2019-08-04 08:37:06

by Ard Biesheuvel

[permalink] [raw]
Subject: Re: [dm-devel] xts fuzz testing and lack of ciphertext stealing support

On Sat, 27 Jul 2019 at 19:04, Milan Broz <[email protected]> wrote:
>
> On 27/07/2019 07:39, Ard Biesheuvel wrote:
> > Thanks for the additional test vectors. They work fine with my SIMD
> > implementations for ARM [0], so this looks like it might be a CAAM
> > problem, not a problem with the test vectors.
> >
> > I will try to find some time today to run them through OpenSSL to double check.
>
> I shamelessly copied your test vectors to my vector test for cryptsetup backend.
>
> Both OpenSSL and gcrypt XTS implementation passed all tests here!
>
> If interested - this is copy of backend we have in cryptsetup, vectors added in crypto-vectors.c
> (there are some hard defines in Makefile, cryptsetup uses autoconf instead).
> OpenSSL: https://github.com/mbroz/cryptsetup_backend_test
> gcrypt branch: https://github.com/mbroz/cryptsetup_backend_test/tree/gcrypt
>
> Once kernel AF_ALG supports it, I can easily test it the same way.
>

Thanks for confirming. So we can be reasonably confident that the test
vectors contributed by Pascal are sound.

I'll try to send out my ARM/arm64 changes shortly. However, I won't
have any access to hardware until end of next month, so they are
tested on QEMU only, which means I won't be able to provide any
performance numbers.