Linux-Crypto Archive on lore.kernel.org
 help / color / Atom feed
* [RFC/RFT PATCH 0/2] crypto: add CTS output IVs for arm64 and testmgr
@ 2020-05-19 19:02 Ard Biesheuvel
  2020-05-19 19:02 ` [RFC/RFT PATCH 1/2] crypto: arm64/aes - align output IV with generic CBC-CTS driver Ard Biesheuvel
                   ` (4 more replies)
  0 siblings, 5 replies; 25+ messages in thread
From: Ard Biesheuvel @ 2020-05-19 19:02 UTC (permalink / raw)
  To: linux-crypto; +Cc: linux-arm-kernel, ebiggers, Ard Biesheuvel, Stephan Mueller

Stephan reports that the arm64 implementation of cts(cbc(aes)) deviates
from the generic implementation in what it returns as the output IV. So
fix this, and add some test vectors to catch other non-compliant
implementations.

Stephan, could you provide a reference for the NIST validation tool and
how it flags this behaviour as non-compliant? Thanks.

Cc: Stephan Mueller <smueller@chronox.de>

Ard Biesheuvel (2):
  crypto: arm64/aes - align output IV with generic CBC-CTS driver
  crypto: testmgr - add output IVs for AES-CBC with ciphertext stealing

 arch/arm64/crypto/aes-modes.S |  2 ++
 crypto/testmgr.h              | 12 ++++++++++++
 2 files changed, 14 insertions(+)

-- 
2.20.1


^ permalink raw reply	[flat|nested] 25+ messages in thread

* [RFC/RFT PATCH 1/2] crypto: arm64/aes - align output IV with generic CBC-CTS driver
  2020-05-19 19:02 [RFC/RFT PATCH 0/2] crypto: add CTS output IVs for arm64 and testmgr Ard Biesheuvel
@ 2020-05-19 19:02 ` Ard Biesheuvel
  2020-05-19 19:02 ` [RFC/RFT PATCH 2/2] crypto: testmgr - add output IVs for AES-CBC with ciphertext stealing Ard Biesheuvel
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 25+ messages in thread
From: Ard Biesheuvel @ 2020-05-19 19:02 UTC (permalink / raw)
  To: linux-crypto; +Cc: linux-arm-kernel, ebiggers, Ard Biesheuvel, Stephan Mueller

The generic CTS chaining mode wraps the CBC mode driver in a way that
results in the IV buffer referenced by the skcipher request to be
updated with the last block of ciphertext. The arm64 implementation
deviates from this, given that CTS itself does not specify the concept
of an output IV, or how it should be generated, and so it was assumed
that the output IV does not matter.

However, Stephan reports that code exists that relies on this behavior,
and that there is even a NIST validation tool that flags it as
non-compliant [citation needed. Stephan?]

So let's align with the generic implementation here, and return the
penultimate block of ciphertext as the output IV.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 arch/arm64/crypto/aes-modes.S | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm64/crypto/aes-modes.S b/arch/arm64/crypto/aes-modes.S
index cf618d8f6cec..80832464df50 100644
--- a/arch/arm64/crypto/aes-modes.S
+++ b/arch/arm64/crypto/aes-modes.S
@@ -275,6 +275,7 @@ AES_FUNC_START(aes_cbc_cts_encrypt)
 	add		x4, x0, x4
 	st1		{v0.16b}, [x4]			/* overlapping stores */
 	st1		{v1.16b}, [x0]
+	st1		{v1.16b}, [x5]
 	ret
 AES_FUNC_END(aes_cbc_cts_encrypt)
 
@@ -291,6 +292,7 @@ AES_FUNC_START(aes_cbc_cts_decrypt)
 	ld1		{v1.16b}, [x1]
 
 	ld1		{v5.16b}, [x5]			/* get iv */
+	st1		{v0.16b}, [x5]
 	dec_prepare	w3, x2, x6
 
 	decrypt_block	v0, w3, x2, x6, w7
-- 
2.20.1


^ permalink raw reply	[flat|nested] 25+ messages in thread

* [RFC/RFT PATCH 2/2] crypto: testmgr - add output IVs for AES-CBC with ciphertext stealing
  2020-05-19 19:02 [RFC/RFT PATCH 0/2] crypto: add CTS output IVs for arm64 and testmgr Ard Biesheuvel
  2020-05-19 19:02 ` [RFC/RFT PATCH 1/2] crypto: arm64/aes - align output IV with generic CBC-CTS driver Ard Biesheuvel
@ 2020-05-19 19:02 ` Ard Biesheuvel
  2020-05-19 19:04 ` [RFC/RFT PATCH 0/2] crypto: add CTS output IVs for arm64 and testmgr Ard Biesheuvel
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 25+ messages in thread
From: Ard Biesheuvel @ 2020-05-19 19:02 UTC (permalink / raw)
  To: linux-crypto; +Cc: linux-arm-kernel, ebiggers, Ard Biesheuvel, Stephan Mueller

Add some test vectors to get coverage for the IV that is output by CTS
implementations.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 crypto/testmgr.h | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/crypto/testmgr.h b/crypto/testmgr.h
index d29983908c38..d45fa1ad91ee 100644
--- a/crypto/testmgr.h
+++ b/crypto/testmgr.h
@@ -31041,6 +31041,8 @@ static const struct cipher_testvec cts_mode_tv_template[] = {
 		.ctext	= "\xc6\x35\x35\x68\xf2\xbf\x8c\xb4"
 			  "\xd8\xa5\x80\x36\x2d\xa7\xff\x7f"
 			  "\x97",
+		.iv_out	= "\xc6\x35\x35\x68\xf2\xbf\x8c\xb4"
+			  "\xd8\xa5\x80\x36\x2d\xa7\xff\x7f",
 	}, {
 		.klen	= 16,
 		.key    = "\x63\x68\x69\x63\x6b\x65\x6e\x20"
@@ -31054,6 +31056,8 @@ static const struct cipher_testvec cts_mode_tv_template[] = {
 			  "\xd4\x45\xd4\xc8\xef\xf7\xed\x22"
 			  "\x97\x68\x72\x68\xd6\xec\xcc\xc0"
 			  "\xc0\x7b\x25\xe2\x5e\xcf\xe5",
+		.iv_out	= "\xfc\x00\x78\x3e\x0e\xfd\xb2\xc1"
+			  "\xd4\x45\xd4\xc8\xef\xf7\xed\x22",
 	}, {
 		.klen	= 16,
 		.key    = "\x63\x68\x69\x63\x6b\x65\x6e\x20"
@@ -31067,6 +31071,8 @@ static const struct cipher_testvec cts_mode_tv_template[] = {
 			  "\xbe\x7f\xcb\xcc\x98\xeb\xf5\xa8"
 			  "\x97\x68\x72\x68\xd6\xec\xcc\xc0"
 			  "\xc0\x7b\x25\xe2\x5e\xcf\xe5\x84",
+		.iv_out	= "\x39\x31\x25\x23\xa7\x86\x62\xd5"
+			  "\xbe\x7f\xcb\xcc\x98\xeb\xf5\xa8",
 	}, {
 		.klen	= 16,
 		.key    = "\x63\x68\x69\x63\x6b\x65\x6e\x20"
@@ -31084,6 +31090,8 @@ static const struct cipher_testvec cts_mode_tv_template[] = {
 			  "\x1b\x55\x49\xd2\xf8\x38\x02\x9e"
 			  "\x39\x31\x25\x23\xa7\x86\x62\xd5"
 			  "\xbe\x7f\xcb\xcc\x98\xeb\xf5",
+		.iv_out	= "\xb3\xff\xfd\x94\x0c\x16\xa1\x8c"
+			  "\x1b\x55\x49\xd2\xf8\x38\x02\x9e",
 	}, {
 		.klen	= 16,
 		.key    = "\x63\x68\x69\x63\x6b\x65\x6e\x20"
@@ -31101,6 +31109,8 @@ static const struct cipher_testvec cts_mode_tv_template[] = {
 			  "\x3b\xc1\x03\xe1\xa1\x94\xbb\xd8"
 			  "\x39\x31\x25\x23\xa7\x86\x62\xd5"
 			  "\xbe\x7f\xcb\xcc\x98\xeb\xf5\xa8",
+		.iv_out	= "\x9d\xad\x8b\xbb\x96\xc4\xcd\xc0"
+			  "\x3b\xc1\x03\xe1\xa1\x94\xbb\xd8",
 	}, {
 		.klen	= 16,
 		.key    = "\x63\x68\x69\x63\x6b\x65\x6e\x20"
@@ -31122,6 +31132,8 @@ static const struct cipher_testvec cts_mode_tv_template[] = {
 			  "\x26\x73\x0d\xbc\x2f\x7b\xc8\x40"
 			  "\x9d\xad\x8b\xbb\x96\xc4\xcd\xc0"
 			  "\x3b\xc1\x03\xe1\xa1\x94\xbb\xd8",
+		.iv_out	= "\x48\x07\xef\xe8\x36\xee\x89\xa5"
+			  "\x26\x73\x0d\xbc\x2f\x7b\xc8\x40",
 	}
 };
 
-- 
2.20.1


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [RFC/RFT PATCH 0/2] crypto: add CTS output IVs for arm64 and testmgr
  2020-05-19 19:02 [RFC/RFT PATCH 0/2] crypto: add CTS output IVs for arm64 and testmgr Ard Biesheuvel
  2020-05-19 19:02 ` [RFC/RFT PATCH 1/2] crypto: arm64/aes - align output IV with generic CBC-CTS driver Ard Biesheuvel
  2020-05-19 19:02 ` [RFC/RFT PATCH 2/2] crypto: testmgr - add output IVs for AES-CBC with ciphertext stealing Ard Biesheuvel
@ 2020-05-19 19:04 ` Ard Biesheuvel
  2020-05-20  6:03 ` Stephan Mueller
  2020-05-28  7:33 ` Herbert Xu
  4 siblings, 0 replies; 25+ messages in thread
From: Ard Biesheuvel @ 2020-05-19 19:04 UTC (permalink / raw)
  To: Linux Crypto Mailing List, Gilad Ben-Yossef
  Cc: Linux ARM, Eric Biggers, Stephan Mueller

(add Gilad for cc-ree)

On Tue, 19 May 2020 at 21:02, Ard Biesheuvel <ardb@kernel.org> wrote:
>
> Stephan reports that the arm64 implementation of cts(cbc(aes)) deviates
> from the generic implementation in what it returns as the output IV. So
> fix this, and add some test vectors to catch other non-compliant
> implementations.
>
> Stephan, could you provide a reference for the NIST validation tool and
> how it flags this behaviour as non-compliant? Thanks.
>
> Cc: Stephan Mueller <smueller@chronox.de>
>
> Ard Biesheuvel (2):
>   crypto: arm64/aes - align output IV with generic CBC-CTS driver
>   crypto: testmgr - add output IVs for AES-CBC with ciphertext stealing
>
>  arch/arm64/crypto/aes-modes.S |  2 ++
>  crypto/testmgr.h              | 12 ++++++++++++
>  2 files changed, 14 insertions(+)
>
> --
> 2.20.1
>

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [RFC/RFT PATCH 0/2] crypto: add CTS output IVs for arm64 and testmgr
  2020-05-19 19:02 [RFC/RFT PATCH 0/2] crypto: add CTS output IVs for arm64 and testmgr Ard Biesheuvel
                   ` (2 preceding siblings ...)
  2020-05-19 19:04 ` [RFC/RFT PATCH 0/2] crypto: add CTS output IVs for arm64 and testmgr Ard Biesheuvel
@ 2020-05-20  6:03 ` Stephan Mueller
  2020-05-20  6:40   ` Ard Biesheuvel
  2020-05-28  7:33 ` Herbert Xu
  4 siblings, 1 reply; 25+ messages in thread
From: Stephan Mueller @ 2020-05-20  6:03 UTC (permalink / raw)
  To: Ard Biesheuvel; +Cc: linux-crypto, linux-arm-kernel, ebiggers

Am Dienstag, 19. Mai 2020, 21:02:09 CEST schrieb Ard Biesheuvel:

Hi Ard,

> Stephan reports that the arm64 implementation of cts(cbc(aes)) deviates
> from the generic implementation in what it returns as the output IV. So
> fix this, and add some test vectors to catch other non-compliant
> implementations.
> 
> Stephan, could you provide a reference for the NIST validation tool and
> how it flags this behaviour as non-compliant? Thanks.

The test definition that identified the inconsistent behavior is specified 
with [1]. Note, this testing is intended to become an RFC standard.

To facilitate that testing, NIST offers an internet service, the ACVP server, 
that allows obtaining test vectors and uploading responses. You see the large 
number of concluded testing with [2]. A particular completion of the CTS 
testing I finished yesterday is given in [3]. That particular testing was also 
performed on an ARM system with CE where the issue was detected.

I am performing the testing with [4] that has an extension to test the kernel 
crypto API.

[1] https://github.com/usnistgov/ACVP/blob/master/artifacts/draft-celi-acvp-block-ciph-00.txt#L366

[2] https://csrc.nist.gov/projects/cryptographic-algorithm-validation-program/
validation-search?searchMode=validation&family=1&productType=-1&ipp=25

[3] https://csrc.nist.gov/projects/cryptographic-algorithm-validation-program/
details?validation=32608

[4] https://github.com/smuellerDD/acvpparser
> 
> Cc: Stephan Mueller <smueller@chronox.de>
> 
> Ard Biesheuvel (2):
>   crypto: arm64/aes - align output IV with generic CBC-CTS driver
>   crypto: testmgr - add output IVs for AES-CBC with ciphertext stealing
> 
>  arch/arm64/crypto/aes-modes.S |  2 ++
>  crypto/testmgr.h              | 12 ++++++++++++
>  2 files changed, 14 insertions(+)


Ciao
Stephan



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [RFC/RFT PATCH 0/2] crypto: add CTS output IVs for arm64 and testmgr
  2020-05-20  6:03 ` Stephan Mueller
@ 2020-05-20  6:40   ` Ard Biesheuvel
  2020-05-20  6:47     ` Stephan Mueller
  0 siblings, 1 reply; 25+ messages in thread
From: Ard Biesheuvel @ 2020-05-20  6:40 UTC (permalink / raw)
  To: Stephan Mueller; +Cc: Linux Crypto Mailing List, Linux ARM, Eric Biggers

On Wed, 20 May 2020 at 08:03, Stephan Mueller <smueller@chronox.de> wrote:
>
> Am Dienstag, 19. Mai 2020, 21:02:09 CEST schrieb Ard Biesheuvel:
>
> Hi Ard,
>
> > Stephan reports that the arm64 implementation of cts(cbc(aes)) deviates
> > from the generic implementation in what it returns as the output IV. So
> > fix this, and add some test vectors to catch other non-compliant
> > implementations.
> >
> > Stephan, could you provide a reference for the NIST validation tool and
> > how it flags this behaviour as non-compliant? Thanks.
>
> The test definition that identified the inconsistent behavior is specified
> with [1]. Note, this testing is intended to become an RFC standard.
>

Are you referring to the line

CT[j] = AES_CBC_CS_ENCRYPT(Key[i], PT[j])

where the CTS transform is invoked without an IV altogether? That
simply seems like a bug to me. In an abstract specification like this,
it would be insane for pseudocode functions to be stateful objects,
and there is nothing in the pseudocode that explicitly captures the
'output IV' of that function call.


> To facilitate that testing, NIST offers an internet service, the ACVP server,
> that allows obtaining test vectors and uploading responses. You see the large
> number of concluded testing with [2]. A particular completion of the CTS
> testing I finished yesterday is given in [3]. That particular testing was also
> performed on an ARM system with CE where the issue was detected.
>
> I am performing the testing with [4] that has an extension to test the kernel
> crypto API.
>

OK. So given that that neither the CTS spec nor this document makes
any mention of an output IV or what its value should be, my suggestion
would be to capture the IV directly from the ciphertext, rather than
relying on some unspecified behavior to give you the right data. Note
that we have other implementations of cts(cbc(aes)) in the kernel as
well (h/w accelerated ones) and if there is no specification that
defines this behavior, you really shouldn't be relying on it.


That 'specification' invokes AES_CBC_CS_ENCRYPT() twice using a
different prototype, without any mention whatsoever what the implied
value of IV[] is if it is missing. This is especially problematic
given that it seems to cover all of CS1/2/3, and the relation between
next IV and ciphertext is not even the same between those for inputs
that are a multiple of the blocksize.


> [1] https://github.com/usnistgov/ACVP/blob/master/artifacts/draft-celi-acvp-block-ciph-00.txt#L366
>
> [2] https://csrc.nist.gov/projects/cryptographic-algorithm-validation-program/
> validation-search?searchMode=validation&family=1&productType=-1&ipp=25
>
> [3] https://csrc.nist.gov/projects/cryptographic-algorithm-validation-program/
> details?validation=32608
>
> [4] https://github.com/smuellerDD/acvpparser
> >
> > Cc: Stephan Mueller <smueller@chronox.de>
> >
> > Ard Biesheuvel (2):
> >   crypto: arm64/aes - align output IV with generic CBC-CTS driver
> >   crypto: testmgr - add output IVs for AES-CBC with ciphertext stealing
> >
> >  arch/arm64/crypto/aes-modes.S |  2 ++
> >  crypto/testmgr.h              | 12 ++++++++++++
> >  2 files changed, 14 insertions(+)
>
>
> Ciao
> Stephan
>
>

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [RFC/RFT PATCH 0/2] crypto: add CTS output IVs for arm64 and testmgr
  2020-05-20  6:40   ` Ard Biesheuvel
@ 2020-05-20  6:47     ` Stephan Mueller
  2020-05-20  6:54       ` Ard Biesheuvel
  0 siblings, 1 reply; 25+ messages in thread
From: Stephan Mueller @ 2020-05-20  6:47 UTC (permalink / raw)
  To: Ard Biesheuvel; +Cc: Linux Crypto Mailing List, Linux ARM, Eric Biggers

Am Mittwoch, 20. Mai 2020, 08:40:57 CEST schrieb Ard Biesheuvel:

Hi Ard,

> On Wed, 20 May 2020 at 08:03, Stephan Mueller <smueller@chronox.de> wrote:
> > Am Dienstag, 19. Mai 2020, 21:02:09 CEST schrieb Ard Biesheuvel:
> > 
> > Hi Ard,
> > 
> > > Stephan reports that the arm64 implementation of cts(cbc(aes)) deviates
> > > from the generic implementation in what it returns as the output IV. So
> > > fix this, and add some test vectors to catch other non-compliant
> > > implementations.
> > > 
> > > Stephan, could you provide a reference for the NIST validation tool and
> > > how it flags this behaviour as non-compliant? Thanks.
> > 
> > The test definition that identified the inconsistent behavior is specified
> > with [1]. Note, this testing is intended to become an RFC standard.
> 
> Are you referring to the line
> 
> CT[j] = AES_CBC_CS_ENCRYPT(Key[i], PT[j])
> 
> where the CTS transform is invoked without an IV altogether?

Precisely.

> That
> simply seems like a bug to me. In an abstract specification like this,
> it would be insane for pseudocode functions to be stateful objects,
> and there is nothing in the pseudocode that explicitly captures the
> 'output IV' of that function call.

I think the description may be updated by simply refer to IV[j-1]. Then you 
would not have a stateful operation, but you rest on the IV of the previous 
operation.

The state of all block chaining modes we currently have is defined with the 
IV. That is the reason why I mentioned it can be implemented stateless when I 
am able to get the IV output from the previous operation.

> 
> > To facilitate that testing, NIST offers an internet service, the ACVP
> > server, that allows obtaining test vectors and uploading responses. You
> > see the large number of concluded testing with [2]. A particular
> > completion of the CTS testing I finished yesterday is given in [3]. That
> > particular testing was also performed on an ARM system with CE where the
> > issue was detected.
> > 
> > I am performing the testing with [4] that has an extension to test the
> > kernel crypto API.
> 
> OK. So given that that neither the CTS spec nor this document makes
> any mention of an output IV or what its value should be, my suggestion
> would be to capture the IV directly from the ciphertext, rather than
> relying on some unspecified behavior to give you the right data. Note
> that we have other implementations of cts(cbc(aes)) in the kernel as
> well (h/w accelerated ones) and if there is no specification that
> defines this behavior, you really shouldn't be relying on it.

Agreed, but all I need is the IV from the previous round without relying on 
any state.
> 
> 
> That 'specification' invokes AES_CBC_CS_ENCRYPT() twice using a
> different prototype, without any mention whatsoever what the implied
> value of IV[] is if it is missing. This is especially problematic
> given that it seems to cover all of CS1/2/3, and the relation between
> next IV and ciphertext is not even the same between those for inputs
> that are a multiple of the blocksize.

I will relay that comment back to the authors for update.
> 
> > [1]
> > https://github.com/usnistgov/ACVP/blob/master/artifacts/draft-celi-acvp-b
> > lock-ciph-00.txt#L366
> > 
> > [2]
> > https://csrc.nist.gov/projects/cryptographic-algorithm-validation-program
> > / validation-search?searchMode=validation&family=1&productType=-1&ipp=25
> > 
> > [3]
> > https://csrc.nist.gov/projects/cryptographic-algorithm-validation-program
> > / details?validation=32608
> > 
> > [4] https://github.com/smuellerDD/acvpparser
> > 
> > > Cc: Stephan Mueller <smueller@chronox.de>
> > > 
> > > Ard Biesheuvel (2):
> > >   crypto: arm64/aes - align output IV with generic CBC-CTS driver
> > >   crypto: testmgr - add output IVs for AES-CBC with ciphertext stealing
> > >  
> > >  arch/arm64/crypto/aes-modes.S |  2 ++
> > >  crypto/testmgr.h              | 12 ++++++++++++
> > >  2 files changed, 14 insertions(+)
> > 
> > Ciao
> > Stephan


Ciao
Stephan



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [RFC/RFT PATCH 0/2] crypto: add CTS output IVs for arm64 and testmgr
  2020-05-20  6:47     ` Stephan Mueller
@ 2020-05-20  6:54       ` Ard Biesheuvel
  2020-05-20  7:01         ` Stephan Mueller
  0 siblings, 1 reply; 25+ messages in thread
From: Ard Biesheuvel @ 2020-05-20  6:54 UTC (permalink / raw)
  To: Stephan Mueller; +Cc: Linux Crypto Mailing List, Linux ARM, Eric Biggers

On Wed, 20 May 2020 at 08:47, Stephan Mueller <smueller@chronox.de> wrote:
>
> Am Mittwoch, 20. Mai 2020, 08:40:57 CEST schrieb Ard Biesheuvel:
>
> Hi Ard,
>
> > On Wed, 20 May 2020 at 08:03, Stephan Mueller <smueller@chronox.de> wrote:
> > > Am Dienstag, 19. Mai 2020, 21:02:09 CEST schrieb Ard Biesheuvel:
> > >
> > > Hi Ard,
> > >
> > > > Stephan reports that the arm64 implementation of cts(cbc(aes)) deviates
> > > > from the generic implementation in what it returns as the output IV. So
> > > > fix this, and add some test vectors to catch other non-compliant
> > > > implementations.
> > > >
> > > > Stephan, could you provide a reference for the NIST validation tool and
> > > > how it flags this behaviour as non-compliant? Thanks.
> > >
> > > The test definition that identified the inconsistent behavior is specified
> > > with [1]. Note, this testing is intended to become an RFC standard.
> >
> > Are you referring to the line
> >
> > CT[j] = AES_CBC_CS_ENCRYPT(Key[i], PT[j])
> >
> > where the CTS transform is invoked without an IV altogether?
>
> Precisely.
>
> > That
> > simply seems like a bug to me. In an abstract specification like this,
> > it would be insane for pseudocode functions to be stateful objects,
> > and there is nothing in the pseudocode that explicitly captures the
> > 'output IV' of that function call.
>
> I think the description may be updated by simply refer to IV[j-1]. Then you
> would not have a stateful operation, but you rest on the IV of the previous
> operation.
>

But that value is not the value you are using now, right? I suspect
that the line

IV[i+1] = MSB(CT[j], IV.length)

needs to be duplicated in the inner loop for j, although that would
require different versions for CS1/2/3


> The state of all block chaining modes we currently have is defined with the
> IV. That is the reason why I mentioned it can be implemented stateless when I
> am able to get the IV output from the previous operation.
>

But it is simply the same as the penultimate block of ciphertext. So
you can simply capture it after encrypt, or before decrypt. There is
really no need to rely on the CTS transformation to pass it back to
you via the buffer that is only specified to provide an input to the
CTS transform.


> >
> > > To facilitate that testing, NIST offers an internet service, the ACVP
> > > server, that allows obtaining test vectors and uploading responses. You
> > > see the large number of concluded testing with [2]. A particular
> > > completion of the CTS testing I finished yesterday is given in [3]. That
> > > particular testing was also performed on an ARM system with CE where the
> > > issue was detected.
> > >
> > > I am performing the testing with [4] that has an extension to test the
> > > kernel crypto API.
> >
> > OK. So given that that neither the CTS spec nor this document makes
> > any mention of an output IV or what its value should be, my suggestion
> > would be to capture the IV directly from the ciphertext, rather than
> > relying on some unspecified behavior to give you the right data. Note
> > that we have other implementations of cts(cbc(aes)) in the kernel as
> > well (h/w accelerated ones) and if there is no specification that
> > defines this behavior, you really shouldn't be relying on it.
>
> Agreed, but all I need is the IV from the previous round without relying on
> any state.

So just grab it from the ciphertext of the previous round.

> >
> >
> > That 'specification' invokes AES_CBC_CS_ENCRYPT() twice using a
> > different prototype, without any mention whatsoever what the implied
> > value of IV[] is if it is missing. This is especially problematic
> > given that it seems to cover all of CS1/2/3, and the relation between
> > next IV and ciphertext is not even the same between those for inputs
> > that are a multiple of the blocksize.
>
> I will relay that comment back to the authors for update.

Thanks.


> >
> > > [1]
> > > https://github.com/usnistgov/ACVP/blob/master/artifacts/draft-celi-acvp-b
> > > lock-ciph-00.txt#L366
> > >
> > > [2]
> > > https://csrc.nist.gov/projects/cryptographic-algorithm-validation-program
> > > / validation-search?searchMode=validation&family=1&productType=-1&ipp=25
> > >
> > > [3]
> > > https://csrc.nist.gov/projects/cryptographic-algorithm-validation-program
> > > / details?validation=32608
> > >
> > > [4] https://github.com/smuellerDD/acvpparser
> > >
> > > > Cc: Stephan Mueller <smueller@chronox.de>
> > > >
> > > > Ard Biesheuvel (2):
> > > >   crypto: arm64/aes - align output IV with generic CBC-CTS driver
> > > >   crypto: testmgr - add output IVs for AES-CBC with ciphertext stealing
> > > >
> > > >  arch/arm64/crypto/aes-modes.S |  2 ++
> > > >  crypto/testmgr.h              | 12 ++++++++++++
> > > >  2 files changed, 14 insertions(+)
> > >
> > > Ciao
> > > Stephan
>
>
> Ciao
> Stephan
>
>

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [RFC/RFT PATCH 0/2] crypto: add CTS output IVs for arm64 and testmgr
  2020-05-20  6:54       ` Ard Biesheuvel
@ 2020-05-20  7:01         ` Stephan Mueller
  2020-05-20  7:09           ` Ard Biesheuvel
  0 siblings, 1 reply; 25+ messages in thread
From: Stephan Mueller @ 2020-05-20  7:01 UTC (permalink / raw)
  To: Ard Biesheuvel; +Cc: Linux Crypto Mailing List, Linux ARM, Eric Biggers

Am Mittwoch, 20. Mai 2020, 08:54:10 CEST schrieb Ard Biesheuvel:

Hi Ard,

> On Wed, 20 May 2020 at 08:47, Stephan Mueller <smueller@chronox.de> wrote:
> > Am Mittwoch, 20. Mai 2020, 08:40:57 CEST schrieb Ard Biesheuvel:
> > 
> > Hi Ard,
> > 
> > > On Wed, 20 May 2020 at 08:03, Stephan Mueller <smueller@chronox.de> 
wrote:
> > > > Am Dienstag, 19. Mai 2020, 21:02:09 CEST schrieb Ard Biesheuvel:
> > > > 
> > > > Hi Ard,
> > > > 
> > > > > Stephan reports that the arm64 implementation of cts(cbc(aes))
> > > > > deviates
> > > > > from the generic implementation in what it returns as the output IV.
> > > > > So
> > > > > fix this, and add some test vectors to catch other non-compliant
> > > > > implementations.
> > > > > 
> > > > > Stephan, could you provide a reference for the NIST validation tool
> > > > > and
> > > > > how it flags this behaviour as non-compliant? Thanks.
> > > > 
> > > > The test definition that identified the inconsistent behavior is
> > > > specified
> > > > with [1]. Note, this testing is intended to become an RFC standard.
> > > 
> > > Are you referring to the line
> > > 
> > > CT[j] = AES_CBC_CS_ENCRYPT(Key[i], PT[j])
> > > 
> > > where the CTS transform is invoked without an IV altogether?
> > 
> > Precisely.
> > 
> > > That
> > > simply seems like a bug to me. In an abstract specification like this,
> > > it would be insane for pseudocode functions to be stateful objects,
> > > and there is nothing in the pseudocode that explicitly captures the
> > > 'output IV' of that function call.
> > 
> > I think the description may be updated by simply refer to IV[j-1]. Then
> > you
> > would not have a stateful operation, but you rest on the IV of the
> > previous
> > operation.
> 
> But that value is not the value you are using now, right? I suspect
> that the line
> 
> IV[i+1] = MSB(CT[j], IV.length)

Yes, such a line would be needed.
> 
> needs to be duplicated in the inner loop for j, although that would
> require different versions for CS1/2/3

Correct in the sense to specify exactly what the IV after a cipher operation 
actually is.
> 
> > The state of all block chaining modes we currently have is defined with
> > the
> > IV. That is the reason why I mentioned it can be implemented stateless
> > when I am able to get the IV output from the previous operation.
> 
> But it is simply the same as the penultimate block of ciphertext. So
> you can simply capture it after encrypt, or before decrypt. There is
> really no need to rely on the CTS transformation to pass it back to
> you via the buffer that is only specified to provide an input to the
> CTS transform.

Let me recheck that as I am not fully sure on that one. But if it can be 
handled that way, it would make life easier.
> 
> > > > To facilitate that testing, NIST offers an internet service, the ACVP
> > > > server, that allows obtaining test vectors and uploading responses.
> > > > You
> > > > see the large number of concluded testing with [2]. A particular
> > > > completion of the CTS testing I finished yesterday is given in [3].
> > > > That
> > > > particular testing was also performed on an ARM system with CE where
> > > > the
> > > > issue was detected.
> > > > 
> > > > I am performing the testing with [4] that has an extension to test the
> > > > kernel crypto API.
> > > 
> > > OK. So given that that neither the CTS spec nor this document makes
> > > any mention of an output IV or what its value should be, my suggestion
> > > would be to capture the IV directly from the ciphertext, rather than
> > > relying on some unspecified behavior to give you the right data. Note
> > > that we have other implementations of cts(cbc(aes)) in the kernel as
> > > well (h/w accelerated ones) and if there is no specification that
> > > defines this behavior, you really shouldn't be relying on it.
> > 
> > Agreed, but all I need is the IV from the previous round without relying
> > on
> > any state.
> 
> So just grab it from the ciphertext of the previous round.
> 
> > > That 'specification' invokes AES_CBC_CS_ENCRYPT() twice using a
> > > different prototype, without any mention whatsoever what the implied
> > > value of IV[] is if it is missing. This is especially problematic
> > > given that it seems to cover all of CS1/2/3, and the relation between
> > > next IV and ciphertext is not even the same between those for inputs
> > > that are a multiple of the blocksize.
> > 
> > I will relay that comment back to the authors for update.
> 
> Thanks.

Thank you for your ideas!
> 
> > > > [1]
> > > > https://github.com/usnistgov/ACVP/blob/master/artifacts/draft-celi-acv
> > > > p-b
> > > > lock-ciph-00.txt#L366
> > > > 
> > > > [2]
> > > > https://csrc.nist.gov/projects/cryptographic-algorithm-validation-prog
> > > > ram
> > > > /
> > > > validation-search?searchMode=validation&family=1&productType=-1&ipp=2
> > > > 5
> > > > 
> > > > [3]
> > > > https://csrc.nist.gov/projects/cryptographic-algorithm-validation-prog
> > > > ram
> > > > / details?validation=32608
> > > > 
> > > > [4] https://github.com/smuellerDD/acvpparser
> > > > 
> > > > > Cc: Stephan Mueller <smueller@chronox.de>
> > > > > 
> > > > > Ard Biesheuvel (2):
> > > > >   crypto: arm64/aes - align output IV with generic CBC-CTS driver
> > > > >   crypto: testmgr - add output IVs for AES-CBC with ciphertext
> > > > >   stealing
> > > > >  
> > > > >  arch/arm64/crypto/aes-modes.S |  2 ++
> > > > >  crypto/testmgr.h              | 12 ++++++++++++
> > > > >  2 files changed, 14 insertions(+)
> > > > 
> > > > Ciao
> > > > Stephan
> > 
> > Ciao
> > Stephan


Ciao
Stephan



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [RFC/RFT PATCH 0/2] crypto: add CTS output IVs for arm64 and testmgr
  2020-05-20  7:01         ` Stephan Mueller
@ 2020-05-20  7:09           ` Ard Biesheuvel
  2020-05-21 13:01             ` Gilad Ben-Yossef
  0 siblings, 1 reply; 25+ messages in thread
From: Ard Biesheuvel @ 2020-05-20  7:09 UTC (permalink / raw)
  To: Stephan Mueller; +Cc: Linux Crypto Mailing List, Linux ARM, Eric Biggers

On Wed, 20 May 2020 at 09:01, Stephan Mueller <smueller@chronox.de> wrote:
>
> Am Mittwoch, 20. Mai 2020, 08:54:10 CEST schrieb Ard Biesheuvel:
>
> Hi Ard,
>
> > On Wed, 20 May 2020 at 08:47, Stephan Mueller <smueller@chronox.de> wrote:
...
> > > The state of all block chaining modes we currently have is defined with
> > > the
> > > IV. That is the reason why I mentioned it can be implemented stateless
> > > when I am able to get the IV output from the previous operation.
> >
> > But it is simply the same as the penultimate block of ciphertext. So
> > you can simply capture it after encrypt, or before decrypt. There is
> > really no need to rely on the CTS transformation to pass it back to
> > you via the buffer that is only specified to provide an input to the
> > CTS transform.
>
> Let me recheck that as I am not fully sure on that one. But if it can be
> handled that way, it would make life easier.

Please refer to patch 2. The .iv_out test vectors were all simply
copied from the appropriate offset into the associated .ctext member.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [RFC/RFT PATCH 0/2] crypto: add CTS output IVs for arm64 and testmgr
  2020-05-20  7:09           ` Ard Biesheuvel
@ 2020-05-21 13:01             ` Gilad Ben-Yossef
  2020-05-21 13:23               ` Ard Biesheuvel
  0 siblings, 1 reply; 25+ messages in thread
From: Gilad Ben-Yossef @ 2020-05-21 13:01 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: Stephan Mueller, Linux Crypto Mailing List, Linux ARM, Eric Biggers

Hi Ard,

Thank you for looping me in.

On Wed, May 20, 2020 at 10:09 AM Ard Biesheuvel <ardb@kernel.org> wrote:
>
> On Wed, 20 May 2020 at 09:01, Stephan Mueller <smueller@chronox.de> wrote:
> >
> > Am Mittwoch, 20. Mai 2020, 08:54:10 CEST schrieb Ard Biesheuvel:
> >
> > Hi Ard,
> >
> > > On Wed, 20 May 2020 at 08:47, Stephan Mueller <smueller@chronox.de> wrote:
> ...
> > > > The state of all block chaining modes we currently have is defined with
> > > > the
> > > > IV. That is the reason why I mentioned it can be implemented stateless
> > > > when I am able to get the IV output from the previous operation.
> > >
> > > But it is simply the same as the penultimate block of ciphertext. So
> > > you can simply capture it after encrypt, or before decrypt. There is
> > > really no need to rely on the CTS transformation to pass it back to
> > > you via the buffer that is only specified to provide an input to the
> > > CTS transform.
> >
> > Let me recheck that as I am not fully sure on that one. But if it can be
> > handled that way, it would make life easier.
>
> Please refer to patch 2. The .iv_out test vectors were all simply
> copied from the appropriate offset into the associated .ctext member.

Not surprisingly since to the best of my understanding this behaviour
is not strictly specified, ccree currently fails the IV output check
with the 2nd version of the patch.

If I understand you correctly, the expected output IV is simply the
next to last block of the ciphertext?

Thanks,
Gilad



-- 
Gilad Ben-Yossef
Chief Coffee Drinker

values of β will give rise to dom!

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [RFC/RFT PATCH 0/2] crypto: add CTS output IVs for arm64 and testmgr
  2020-05-21 13:01             ` Gilad Ben-Yossef
@ 2020-05-21 13:23               ` Ard Biesheuvel
  2020-05-23 18:52                 ` Stephan Müller
  0 siblings, 1 reply; 25+ messages in thread
From: Ard Biesheuvel @ 2020-05-21 13:23 UTC (permalink / raw)
  To: Gilad Ben-Yossef
  Cc: Stephan Mueller, Linux Crypto Mailing List, Linux ARM, Eric Biggers

On Thu, 21 May 2020 at 15:01, Gilad Ben-Yossef <gilad@benyossef.com> wrote:
>
> Hi Ard,
>
> Thank you for looping me in.
>
> On Wed, May 20, 2020 at 10:09 AM Ard Biesheuvel <ardb@kernel.org> wrote:
> >
> > On Wed, 20 May 2020 at 09:01, Stephan Mueller <smueller@chronox.de> wrote:
> > >
> > > Am Mittwoch, 20. Mai 2020, 08:54:10 CEST schrieb Ard Biesheuvel:
> > >
> > > Hi Ard,
> > >
> > > > On Wed, 20 May 2020 at 08:47, Stephan Mueller <smueller@chronox.de> wrote:
> > ...
> > > > > The state of all block chaining modes we currently have is defined with
> > > > > the
> > > > > IV. That is the reason why I mentioned it can be implemented stateless
> > > > > when I am able to get the IV output from the previous operation.
> > > >
> > > > But it is simply the same as the penultimate block of ciphertext. So
> > > > you can simply capture it after encrypt, or before decrypt. There is
> > > > really no need to rely on the CTS transformation to pass it back to
> > > > you via the buffer that is only specified to provide an input to the
> > > > CTS transform.
> > >
> > > Let me recheck that as I am not fully sure on that one. But if it can be
> > > handled that way, it would make life easier.
> >
> > Please refer to patch 2. The .iv_out test vectors were all simply
> > copied from the appropriate offset into the associated .ctext member.
>
> Not surprisingly since to the best of my understanding this behaviour
> is not strictly specified, ccree currently fails the IV output check
> with the 2nd version of the patch.
>

That is what I suspected, hence the cc:

> If I understand you correctly, the expected output IV is simply the
> next to last block of the ciphertext?

Yes. But this happens to work for the generic case because the CTS
driver itself requires the encapsulated CBC mode to return the output
IV, which is simply passed through back to the caller. CTS mode itself
does not specify any kind of output IV, so we should not rely on this
behavior.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [RFC/RFT PATCH 0/2] crypto: add CTS output IVs for arm64 and testmgr
  2020-05-21 13:23               ` Ard Biesheuvel
@ 2020-05-23 18:52                 ` Stephan Müller
  2020-05-23 22:40                   ` Ard Biesheuvel
  0 siblings, 1 reply; 25+ messages in thread
From: Stephan Müller @ 2020-05-23 18:52 UTC (permalink / raw)
  To: Gilad Ben-Yossef, Ard Biesheuvel
  Cc: Linux Crypto Mailing List, Linux ARM, Eric Biggers

Am Donnerstag, 21. Mai 2020, 15:23:41 CEST schrieb Ard Biesheuvel:

Hi Ard,

> On Thu, 21 May 2020 at 15:01, Gilad Ben-Yossef <gilad@benyossef.com> wrote:
> > Hi Ard,
> > 
> > Thank you for looping me in.
> > 
> > On Wed, May 20, 2020 at 10:09 AM Ard Biesheuvel <ardb@kernel.org> wrote:
> > > On Wed, 20 May 2020 at 09:01, Stephan Mueller <smueller@chronox.de> 
wrote:
> > > > Am Mittwoch, 20. Mai 2020, 08:54:10 CEST schrieb Ard Biesheuvel:
> > > > 
> > > > Hi Ard,
> > > > 
> > > > > On Wed, 20 May 2020 at 08:47, Stephan Mueller <smueller@chronox.de> 
wrote:
> > > ...
> > > 
> > > > > > The state of all block chaining modes we currently have is defined
> > > > > > with
> > > > > > the
> > > > > > IV. That is the reason why I mentioned it can be implemented
> > > > > > stateless
> > > > > > when I am able to get the IV output from the previous operation.
> > > > > 
> > > > > But it is simply the same as the penultimate block of ciphertext. So
> > > > > you can simply capture it after encrypt, or before decrypt. There is
> > > > > really no need to rely on the CTS transformation to pass it back to
> > > > > you via the buffer that is only specified to provide an input to the
> > > > > CTS transform.
> > > > 
> > > > Let me recheck that as I am not fully sure on that one. But if it can
> > > > be
> > > > handled that way, it would make life easier.
> > > 
> > > Please refer to patch 2. The .iv_out test vectors were all simply
> > > copied from the appropriate offset into the associated .ctext member.
> > 
> > Not surprisingly since to the best of my understanding this behaviour
> > is not strictly specified, ccree currently fails the IV output check
> > with the 2nd version of the patch.
> 
> That is what I suspected, hence the cc:
> > If I understand you correctly, the expected output IV is simply the
> > next to last block of the ciphertext?
> 
> Yes. But this happens to work for the generic case because the CTS
> driver itself requires the encapsulated CBC mode to return the output
> IV, which is simply passed through back to the caller. CTS mode itself
> does not specify any kind of output IV, so we should not rely on this
> behavior.

Note, the update to the spec based on your suggestion is already in a merge 
request:

https://github.com/usnistgov/ACVP/issues/860

Thanks for your input.

Ciao
Stephan



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [RFC/RFT PATCH 0/2] crypto: add CTS output IVs for arm64 and testmgr
  2020-05-23 18:52                 ` Stephan Müller
@ 2020-05-23 22:40                   ` Ard Biesheuvel
  0 siblings, 0 replies; 25+ messages in thread
From: Ard Biesheuvel @ 2020-05-23 22:40 UTC (permalink / raw)
  To: Stephan Müller
  Cc: Gilad Ben-Yossef, Linux Crypto Mailing List, Linux ARM, Eric Biggers

On Sat, 23 May 2020 at 20:52, Stephan Müller <smueller@chronox.de> wrote:
>
> Am Donnerstag, 21. Mai 2020, 15:23:41 CEST schrieb Ard Biesheuvel:
>
> Hi Ard,
>
> > On Thu, 21 May 2020 at 15:01, Gilad Ben-Yossef <gilad@benyossef.com> wrote:
> > > Hi Ard,
> > >
> > > Thank you for looping me in.
> > >
> > > On Wed, May 20, 2020 at 10:09 AM Ard Biesheuvel <ardb@kernel.org> wrote:
> > > > On Wed, 20 May 2020 at 09:01, Stephan Mueller <smueller@chronox.de>
> wrote:
> > > > > Am Mittwoch, 20. Mai 2020, 08:54:10 CEST schrieb Ard Biesheuvel:
> > > > >
> > > > > Hi Ard,
> > > > >
> > > > > > On Wed, 20 May 2020 at 08:47, Stephan Mueller <smueller@chronox.de>
> wrote:
> > > > ...
> > > >
> > > > > > > The state of all block chaining modes we currently have is defined
> > > > > > > with
> > > > > > > the
> > > > > > > IV. That is the reason why I mentioned it can be implemented
> > > > > > > stateless
> > > > > > > when I am able to get the IV output from the previous operation.
> > > > > >
> > > > > > But it is simply the same as the penultimate block of ciphertext. So
> > > > > > you can simply capture it after encrypt, or before decrypt. There is
> > > > > > really no need to rely on the CTS transformation to pass it back to
> > > > > > you via the buffer that is only specified to provide an input to the
> > > > > > CTS transform.
> > > > >
> > > > > Let me recheck that as I am not fully sure on that one. But if it can
> > > > > be
> > > > > handled that way, it would make life easier.
> > > >
> > > > Please refer to patch 2. The .iv_out test vectors were all simply
> > > > copied from the appropriate offset into the associated .ctext member.
> > >
> > > Not surprisingly since to the best of my understanding this behaviour
> > > is not strictly specified, ccree currently fails the IV output check
> > > with the 2nd version of the patch.
> >
> > That is what I suspected, hence the cc:
> > > If I understand you correctly, the expected output IV is simply the
> > > next to last block of the ciphertext?
> >
> > Yes. But this happens to work for the generic case because the CTS
> > driver itself requires the encapsulated CBC mode to return the output
> > IV, which is simply passed through back to the caller. CTS mode itself
> > does not specify any kind of output IV, so we should not rely on this
> > behavior.
>
> Note, the update to the spec based on your suggestion is already in a merge
> request:
>
> https://github.com/usnistgov/ACVP/issues/860
>
> Thanks for your input.
>

Thanks for the head's up. I've left a comment there, as the proposed
change is not equivalent to the unspecified current behavior.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [RFC/RFT PATCH 0/2] crypto: add CTS output IVs for arm64 and testmgr
  2020-05-19 19:02 [RFC/RFT PATCH 0/2] crypto: add CTS output IVs for arm64 and testmgr Ard Biesheuvel
                   ` (3 preceding siblings ...)
  2020-05-20  6:03 ` Stephan Mueller
@ 2020-05-28  7:33 ` Herbert Xu
  2020-05-28  8:33   ` Ard Biesheuvel
  4 siblings, 1 reply; 25+ messages in thread
From: Herbert Xu @ 2020-05-28  7:33 UTC (permalink / raw)
  To: Ard Biesheuvel; +Cc: linux-crypto, linux-arm-kernel, ebiggers, ardb, smueller

Ard Biesheuvel <ardb@kernel.org> wrote:
> Stephan reports that the arm64 implementation of cts(cbc(aes)) deviates
> from the generic implementation in what it returns as the output IV. So
> fix this, and add some test vectors to catch other non-compliant
> implementations.
> 
> Stephan, could you provide a reference for the NIST validation tool and
> how it flags this behaviour as non-compliant? Thanks.

I think our CTS and XTS are both broken with respect to af_alg.

The reason we use output IVs in general is to support chaining
which is required by algif_skcipher to break up large requests
into smaller ones.

For CTS and XTS that simply doesn't work.  So we should fix this
by changing algif_skcipher to not do chaining (and hence drop
support for large requests like algif_aead) for algorithms like
CTS/XTS.

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [RFC/RFT PATCH 0/2] crypto: add CTS output IVs for arm64 and testmgr
  2020-05-28  7:33 ` Herbert Xu
@ 2020-05-28  8:33   ` Ard Biesheuvel
  2020-05-29  8:05     ` Herbert Xu
  0 siblings, 1 reply; 25+ messages in thread
From: Ard Biesheuvel @ 2020-05-28  8:33 UTC (permalink / raw)
  To: Herbert Xu
  Cc: Linux Crypto Mailing List, Linux ARM, Eric Biggers, Stephan Mueller

On Thu, 28 May 2020 at 09:33, Herbert Xu <herbert@gondor.apana.org.au> wrote:
>
> Ard Biesheuvel <ardb@kernel.org> wrote:
> > Stephan reports that the arm64 implementation of cts(cbc(aes)) deviates
> > from the generic implementation in what it returns as the output IV. So
> > fix this, and add some test vectors to catch other non-compliant
> > implementations.
> >
> > Stephan, could you provide a reference for the NIST validation tool and
> > how it flags this behaviour as non-compliant? Thanks.
>
> I think our CTS and XTS are both broken with respect to af_alg.
>
> The reason we use output IVs in general is to support chaining
> which is required by algif_skcipher to break up large requests
> into smaller ones.
>
> For CTS and XTS that simply doesn't work.  So we should fix this
> by changing algif_skcipher to not do chaining (and hence drop
> support for large requests like algif_aead) for algorithms like
> CTS/XTS.
>

The reason we return output IVs for CBC is because our generic
implementation of CTS can wrap any CBC implementation, and relies on
this output IV rather than grabbing it from the ciphertext directly
(which may be tricky and is best left up to the CBC code)

For CTS itself, as well as XTS, returning an output IV is meaningless,
given that
a) the implementations rely on the skcipher_walk infrastructure to
present all input except the last bit in chunks that are a multiple of
the block size,
b) neither specification defines how chaining of inputs should work,
regardless of whether the preceding input was a multiple of the block
size or not.

The CS3 mode that we implement for CTS swaps the final two blocks
unconditionally. So even if the input is a whole multiple of the block
size, the preceding ciphertext will turn out differently if any output
happens to follow.

For XTS, the IV is encrypted before processing the first block, so
even if you do return an output IV, the subsequent invocations of the
skcipher need to omit the encryption, which we don't implement
currently.

So if you are saying that we should never split up algif_skcipher
requests into multiple calls into the underlying skcipher, then I
agree with you. Even if the generic CTS seems to work more or less as
expected by, e.g., the NIST validation tool, this is unspecified
behavior, and definitely broken in various other places.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [RFC/RFT PATCH 0/2] crypto: add CTS output IVs for arm64 and testmgr
  2020-05-28  8:33   ` Ard Biesheuvel
@ 2020-05-29  8:05     ` Herbert Xu
  2020-05-29  8:20       ` Ard Biesheuvel
  0 siblings, 1 reply; 25+ messages in thread
From: Herbert Xu @ 2020-05-29  8:05 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: Linux Crypto Mailing List, Linux ARM, Eric Biggers, Stephan Mueller

On Thu, May 28, 2020 at 10:33:25AM +0200, Ard Biesheuvel wrote:
>
> The reason we return output IVs for CBC is because our generic
> implementation of CTS can wrap any CBC implementation, and relies on
> this output IV rather than grabbing it from the ciphertext directly
> (which may be tricky and is best left up to the CBC code)

No that's not the main reason.  The main user of chaining is
algif_skcipher.

> So if you are saying that we should never split up algif_skcipher
> requests into multiple calls into the underlying skcipher, then I
> agree with you. Even if the generic CTS seems to work more or less as
> expected by, e.g., the NIST validation tool, this is unspecified
> behavior, and definitely broken in various other places.

I was merely suggesting that requests to CTS/XTS shouldn't be
split up.  Doing it for others would be a serious regression.

However, having looked at this it would seem that the effort
in marking CTS/XTS is not that different to just adding support
to hold the last two blocks of data so that CTS/XTS can support
chaining.

I'll work on this.

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [RFC/RFT PATCH 0/2] crypto: add CTS output IVs for arm64 and testmgr
  2020-05-29  8:05     ` Herbert Xu
@ 2020-05-29  8:20       ` Ard Biesheuvel
  2020-05-29 11:51         ` Herbert Xu
  0 siblings, 1 reply; 25+ messages in thread
From: Ard Biesheuvel @ 2020-05-29  8:20 UTC (permalink / raw)
  To: Herbert Xu
  Cc: Linux Crypto Mailing List, Linux ARM, Eric Biggers, Stephan Mueller

On Fri, 29 May 2020 at 10:05, Herbert Xu <herbert@gondor.apana.org.au> wrote:
>
> On Thu, May 28, 2020 at 10:33:25AM +0200, Ard Biesheuvel wrote:
> >
> > The reason we return output IVs for CBC is because our generic
> > implementation of CTS can wrap any CBC implementation, and relies on
> > this output IV rather than grabbing it from the ciphertext directly
> > (which may be tricky and is best left up to the CBC code)
>
> No that's not the main reason.  The main user of chaining is
> algif_skcipher.
>

But many implementation do not return an output IV at all. The only
mode that requires it (for the selftests to pass) is CBC.

> > So if you are saying that we should never split up algif_skcipher
> > requests into multiple calls into the underlying skcipher, then I
> > agree with you. Even if the generic CTS seems to work more or less as
> > expected by, e.g., the NIST validation tool, this is unspecified
> > behavior, and definitely broken in various other places.
>
> I was merely suggesting that requests to CTS/XTS shouldn't be
> split up.  Doing it for others would be a serious regression.
>

Given that in these cases, doing so will give incorrect results even
if the input size is a whole multiple of the block size, I agree that
adding this restriction will not break anything that is working
consistently at the moment.

But could you elaborate on the serious regression for other cases? Do
you have anything particular in mind?

> However, having looked at this it would seem that the effort
> in marking CTS/XTS is not that different to just adding support
> to hold the last two blocks of data so that CTS/XTS can support
> chaining.
>

For XTS, we would have to carry some metadata around that tells you
whether the initial encryption of the IV has occurred or not. In the
CTS case, you need two swap the last two blocks of ciphertext at the
very end.

So does that mean some kind of init/update/final model for skcipher? I
can see how that could address these issues (init() would encrypt the
IV for XTS, and final() would do the final block handling for CTS).
Just holding two blocks of data does not seem sufficient to me to
handle these issues.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [RFC/RFT PATCH 0/2] crypto: add CTS output IVs for arm64 and testmgr
  2020-05-29  8:20       ` Ard Biesheuvel
@ 2020-05-29 11:51         ` Herbert Xu
  2020-05-29 12:00           ` Ard Biesheuvel
  0 siblings, 1 reply; 25+ messages in thread
From: Herbert Xu @ 2020-05-29 11:51 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: Linux Crypto Mailing List, Linux ARM, Eric Biggers, Stephan Mueller

On Fri, May 29, 2020 at 10:20:27AM +0200, Ard Biesheuvel wrote:
>
> But many implementation do not return an output IV at all. The only
> mode that requires it (for the selftests to pass) is CBC.

Most modes can be chained, e.g., CBC, PCBC, OFB, CFB and CTR.
As it stands algif_skcipher requres all algorithms to support
chaining.
 
> For XTS, we would have to carry some metadata around that tells you
> whether the initial encryption of the IV has occurred or not. In the

You're right, XTS in its current form cannot be chained.  So we
do need a way to mark that for algif_skcipher.

> CTS case, you need two swap the last two blocks of ciphertext at the
> very end.

CTS can be easily chained.  You just need to always keep two blocks
from being processed until you reach the end.

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [RFC/RFT PATCH 0/2] crypto: add CTS output IVs for arm64 and testmgr
  2020-05-29 11:51         ` Herbert Xu
@ 2020-05-29 12:00           ` Ard Biesheuvel
  2020-05-29 12:02             ` Herbert Xu
  0 siblings, 1 reply; 25+ messages in thread
From: Ard Biesheuvel @ 2020-05-29 12:00 UTC (permalink / raw)
  To: Herbert Xu
  Cc: Linux Crypto Mailing List, Linux ARM, Eric Biggers, Stephan Mueller

On Fri, 29 May 2020 at 13:51, Herbert Xu <herbert@gondor.apana.org.au> wrote:
>
> On Fri, May 29, 2020 at 10:20:27AM +0200, Ard Biesheuvel wrote:
> >
> > But many implementation do not return an output IV at all. The only
> > mode that requires it (for the selftests to pass) is CBC.
>
> Most modes can be chained, e.g., CBC, PCBC, OFB, CFB and CTR.
> As it stands algif_skcipher requres all algorithms to support
> chaining.
>

Even if this is the case, it requires that an skcipher implementation
stores an output IV in the buffer that skcipher request's IV field
points to. Currently, we only check whether this is the case for CBC
implementations, and so it is quite likely that lots of h/w
accelerators or arch code don't adhere to this today.


> > For XTS, we would have to carry some metadata around that tells you
> > whether the initial encryption of the IV has occurred or not. In the
>
> You're right, XTS in its current form cannot be chained.  So we
> do need a way to mark that for algif_skcipher.
>
> > CTS case, you need two swap the last two blocks of ciphertext at the
> > very end.
>
> CTS can be easily chained.  You just need to always keep two blocks
> from being processed until you reach the end.
>

This might be feasible for the generic CTS driver wrapping h/w
accelerated CBC. But how is this supposed to work, e.g., for the two
existing h/w implementations of cts(cbc(aes)) that currently ignore
this?

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [RFC/RFT PATCH 0/2] crypto: add CTS output IVs for arm64 and testmgr
  2020-05-29 12:00           ` Ard Biesheuvel
@ 2020-05-29 12:02             ` Herbert Xu
  2020-05-29 13:10               ` Ard Biesheuvel
  0 siblings, 1 reply; 25+ messages in thread
From: Herbert Xu @ 2020-05-29 12:02 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: Linux Crypto Mailing List, Linux ARM, Eric Biggers, Stephan Mueller

On Fri, May 29, 2020 at 02:00:14PM +0200, Ard Biesheuvel wrote:
>
> Even if this is the case, it requires that an skcipher implementation
> stores an output IV in the buffer that skcipher request's IV field
> points to. Currently, we only check whether this is the case for CBC
> implementations, and so it is quite likely that lots of h/w
> accelerators or arch code don't adhere to this today.

They are and have always been broken because algif_skcipher has
always relied on this.

> This might be feasible for the generic CTS driver wrapping h/w
> accelerated CBC. But how is this supposed to work, e.g., for the two
> existing h/w implementations of cts(cbc(aes)) that currently ignore
> this?

They'll have to disable chaining.

The way I'm doing this would allow some implementations to allow
chaining while others of the same algorithm can disable chaining
and require the whole request to be presented together.

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [RFC/RFT PATCH 0/2] crypto: add CTS output IVs for arm64 and testmgr
  2020-05-29 12:02             ` Herbert Xu
@ 2020-05-29 13:10               ` Ard Biesheuvel
  2020-05-29 13:19                 ` Herbert Xu
  0 siblings, 1 reply; 25+ messages in thread
From: Ard Biesheuvel @ 2020-05-29 13:10 UTC (permalink / raw)
  To: Herbert Xu
  Cc: Linux Crypto Mailing List, Linux ARM, Eric Biggers, Stephan Mueller

On Fri, 29 May 2020 at 14:02, Herbert Xu <herbert@gondor.apana.org.au> wrote:
>
> On Fri, May 29, 2020 at 02:00:14PM +0200, Ard Biesheuvel wrote:
> >
> > Even if this is the case, it requires that an skcipher implementation
> > stores an output IV in the buffer that skcipher request's IV field
> > points to. Currently, we only check whether this is the case for CBC
> > implementations, and so it is quite likely that lots of h/w
> > accelerators or arch code don't adhere to this today.
>
> They are and have always been broken because algif_skcipher has
> always relied on this.
>

OK, so the undocumented assumption is that algif_skcipher requests are
delineated by ALG_SET_IV commands, and that anything that gets sent to
the socket in between should be treated as a single request, right? I
think that makes sense, but do note that this deviates from Stephan's
use case, where the ciphertext stealing block swap was applied after
every call into af_alg, with the IV being inherited from one request
to the next. I think that case was invalid to begin with, I just hope
no other use cases exist where this unspecified behavior is being
relied upon.

> > This might be feasible for the generic CTS driver wrapping h/w
> > accelerated CBC. But how is this supposed to work, e.g., for the two
> > existing h/w implementations of cts(cbc(aes)) that currently ignore
> > this?
>
> They'll have to disable chaining.
>
> The way I'm doing this would allow some implementations to allow
> chaining while others of the same algorithm can disable chaining
> and require the whole request to be presented together.
>

Fair enough.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [RFC/RFT PATCH 0/2] crypto: add CTS output IVs for arm64 and testmgr
  2020-05-29 13:10               ` Ard Biesheuvel
@ 2020-05-29 13:19                 ` Herbert Xu
  2020-05-29 13:41                   ` Ard Biesheuvel
  0 siblings, 1 reply; 25+ messages in thread
From: Herbert Xu @ 2020-05-29 13:19 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: Linux Crypto Mailing List, Linux ARM, Eric Biggers, Stephan Mueller

On Fri, May 29, 2020 at 03:10:43PM +0200, Ard Biesheuvel wrote:
>
> OK, so the undocumented assumption is that algif_skcipher requests are
> delineated by ALG_SET_IV commands, and that anything that gets sent to
> the socket in between should be treated as a single request, right? I

Correct.

> think that makes sense, but do note that this deviates from Stephan's
> use case, where the ciphertext stealing block swap was applied after
> every call into af_alg, with the IV being inherited from one request
> to the next. I think that case was invalid to begin with, I just hope
> no other use cases exist where this unspecified behavior is being
> relied upon.

That does indeed sound broken.

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [RFC/RFT PATCH 0/2] crypto: add CTS output IVs for arm64 and testmgr
  2020-05-29 13:19                 ` Herbert Xu
@ 2020-05-29 13:41                   ` Ard Biesheuvel
  2020-05-29 13:42                     ` Herbert Xu
  0 siblings, 1 reply; 25+ messages in thread
From: Ard Biesheuvel @ 2020-05-29 13:41 UTC (permalink / raw)
  To: Herbert Xu
  Cc: Linux Crypto Mailing List, Linux ARM, Eric Biggers, Stephan Mueller

On Fri, 29 May 2020 at 15:19, Herbert Xu <herbert@gondor.apana.org.au> wrote:
>
> On Fri, May 29, 2020 at 03:10:43PM +0200, Ard Biesheuvel wrote:
> >
> > OK, so the undocumented assumption is that algif_skcipher requests are
> > delineated by ALG_SET_IV commands, and that anything that gets sent to
> > the socket in between should be treated as a single request, right? I
>
> Correct.
>

So what about the final request? At which point do you decide to
return the final chunk of data that you have been holding back in
order to ensure that you can perform the final processing correctly if
it is not being followed by a ALG_SET_IV command?

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [RFC/RFT PATCH 0/2] crypto: add CTS output IVs for arm64 and testmgr
  2020-05-29 13:41                   ` Ard Biesheuvel
@ 2020-05-29 13:42                     ` Herbert Xu
  0 siblings, 0 replies; 25+ messages in thread
From: Herbert Xu @ 2020-05-29 13:42 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: Linux Crypto Mailing List, Linux ARM, Eric Biggers, Stephan Mueller

On Fri, May 29, 2020 at 03:41:08PM +0200, Ard Biesheuvel wrote:
>
> So what about the final request? At which point do you decide to
> return the final chunk of data that you have been holding back in
> order to ensure that you can perform the final processing correctly if
> it is not being followed by a ALG_SET_IV command?

When the MORE flag is unset.

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply	[flat|nested] 25+ messages in thread

end of thread, back to index

Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-19 19:02 [RFC/RFT PATCH 0/2] crypto: add CTS output IVs for arm64 and testmgr Ard Biesheuvel
2020-05-19 19:02 ` [RFC/RFT PATCH 1/2] crypto: arm64/aes - align output IV with generic CBC-CTS driver Ard Biesheuvel
2020-05-19 19:02 ` [RFC/RFT PATCH 2/2] crypto: testmgr - add output IVs for AES-CBC with ciphertext stealing Ard Biesheuvel
2020-05-19 19:04 ` [RFC/RFT PATCH 0/2] crypto: add CTS output IVs for arm64 and testmgr Ard Biesheuvel
2020-05-20  6:03 ` Stephan Mueller
2020-05-20  6:40   ` Ard Biesheuvel
2020-05-20  6:47     ` Stephan Mueller
2020-05-20  6:54       ` Ard Biesheuvel
2020-05-20  7:01         ` Stephan Mueller
2020-05-20  7:09           ` Ard Biesheuvel
2020-05-21 13:01             ` Gilad Ben-Yossef
2020-05-21 13:23               ` Ard Biesheuvel
2020-05-23 18:52                 ` Stephan Müller
2020-05-23 22:40                   ` Ard Biesheuvel
2020-05-28  7:33 ` Herbert Xu
2020-05-28  8:33   ` Ard Biesheuvel
2020-05-29  8:05     ` Herbert Xu
2020-05-29  8:20       ` Ard Biesheuvel
2020-05-29 11:51         ` Herbert Xu
2020-05-29 12:00           ` Ard Biesheuvel
2020-05-29 12:02             ` Herbert Xu
2020-05-29 13:10               ` Ard Biesheuvel
2020-05-29 13:19                 ` Herbert Xu
2020-05-29 13:41                   ` Ard Biesheuvel
2020-05-29 13:42                     ` Herbert Xu

Linux-Crypto Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-crypto/0 linux-crypto/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-crypto linux-crypto/ https://lore.kernel.org/linux-crypto \
		linux-crypto@vger.kernel.org
	public-inbox-index linux-crypto

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-crypto


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git