2017-04-21 11:03:15

by Milan Broz

[permalink] [raw]
Subject: [PATCH] crypto: Allow ecb(cipher_null) in FIPS mode

The cipher_null is not a real cipher, FIPS mode should not restrict its use.

It is used for several tests (for example in cryptsetup testsuite) and also
temporarily for reencryption of not yet encrypted device in cryptsetup-reencrypt tool.

Problem is easily reproducible with
cryptsetup benchmark -c null

Signed-off-by: Milan Broz <[email protected]>
---
crypto/testmgr.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/crypto/testmgr.c b/crypto/testmgr.c
index f9c378af3907..5075e4d982ee 100644
--- a/crypto/testmgr.c
+++ b/crypto/testmgr.c
@@ -2875,6 +2875,7 @@ static const struct alg_test_desc alg_test_descs[] = {
}, {
.alg = "ecb(cipher_null)",
.test = alg_test_null,
+ .fips_allowed = 1,
}, {
.alg = "ecb(des)",
.test = alg_test_skcipher,
--
2.11.0


2017-04-21 12:18:48

by Herbert Xu

[permalink] [raw]
Subject: Re: [PATCH] crypto: Allow ecb(cipher_null) in FIPS mode

Milan Broz <[email protected]> wrote:
> The cipher_null is not a real cipher, FIPS mode should not restrict its use.
>
> It is used for several tests (for example in cryptsetup testsuite) and also
> temporarily for reencryption of not yet encrypted device in cryptsetup-reencrypt tool.
>
> Problem is easily reproducible with
> cryptsetup benchmark -c null
>
> Signed-off-by: Milan Broz <[email protected]>

Stephan?
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

2017-04-21 18:33:18

by Stephan Müller

[permalink] [raw]
Subject: Re: [PATCH] crypto: Allow ecb(cipher_null) in FIPS mode

Am Freitag, 21. April 2017, 14:18:20 CEST schrieb Herbert Xu:

Hi Herbert,

> Milan Broz <[email protected]> wrote:
> > The cipher_null is not a real cipher, FIPS mode should not restrict its
> > use.
> >
> > It is used for several tests (for example in cryptsetup testsuite) and
> > also
> > temporarily for reencryption of not yet encrypted device in
> > cryptsetup-reencrypt tool.
> >
> > Problem is easily reproducible with
> >
> > cryptsetup benchmark -c null
> >
> > Signed-off-by: Milan Broz <[email protected]>
>
> Stephan?

Acked-by: Stephan M?ller <[email protected]>

Ciao
Stephan

2017-04-21 18:56:16

by Stephan Müller

[permalink] [raw]
Subject: Re: [PATCH] crypto: Allow ecb(cipher_null) in FIPS mode

Am Freitag, 21. April 2017, 17:25:41 CEST schrieb Stephan M?ller:

Hi,

>
> Acked-by: Stephan M?ller <[email protected]>

Just for the records: for FIPS 140-2 rules, cipher_null is to be interpreted
as a memcpy on SGLs. Thus it is no cipher even though it sounds like one.

cipher_null is also needed for seqiv which is required for rfc4106(gcm(aes)),
which is an approved cipher. Also, it is needed for authenc() which uses it
for copying the AAD from src to dst.

That said, cipher_null must not be used for "encryption" operation but rather
for handling data that is not subjected to FIPS 140-2 rules.

Ciao
Stephan

2017-04-22 07:54:10

by Sandy Harris

[permalink] [raw]
Subject: Re: [PATCH] crypto: Allow ecb(cipher_null) in FIPS mode

On Sat, Apr 22, 2017 at 2:56 AM, Stephan Müller <[email protected]> wrote:

> Am Freitag, 21. April 2017, 17:25:41 CEST schrieb Stephan Müller:

> Just for the records: for FIPS 140-2 rules, cipher_null is to be interpreted
> as a memcpy on SGLs. Thus it is no cipher even though it sounds like one.
>
> cipher_null is also needed for seqiv which is required for rfc4106(gcm(aes)),
> which is an approved cipher. Also, it is needed for authenc() which uses it
> for copying the AAD from src to dst.
>
> That said, cipher_null must not be used for "encryption" operation but rather
> for handling data that is not subjected to FIPS 140-2 rules.

In the FreeS/WAN project, back around the turn of the century,
we refused to implement several things required by the RFCs
because we thought they were insecure: null cipher, single
DES & 768-bit DH Group 1.

At that time, not having DES did cause some problems in
interoperating with other IPsec implementations, but I
doubt it would today. Neither of the other dropped items
caused any problems at all.

Today I'd say drop all of those plus the 1024-bit Group 2,
and then look at whether others should go as well. As of
2001 or so, the 1536-bit Group 5 was very widely used,
so dropping it might well be problematic, but I am not
certain if it is either secure or widely used now.

2017-04-22 07:57:06

by Sandy Harris

[permalink] [raw]
Subject: Re: [PATCH] crypto: Allow ecb(cipher_null) in FIPS mode

On Sat, Apr 22, 2017 at 3:54 PM, Sandy Harris <[email protected]> wrote:

> In the FreeS/WAN project, back around the turn of the century,
> we refused to implement several things required by the RFCs

Link to documentation:
http://www.freeswan.org/freeswan_trees/freeswan-2.06/doc/compat.html#dropped

2017-04-23 19:06:34

by Stephan Müller

[permalink] [raw]
Subject: Re: [PATCH] crypto: Allow ecb(cipher_null) in FIPS mode

Am Samstag, 22. April 2017, 09:54:08 CEST schrieb Sandy Harris:

Hi Sandy,

> In the FreeS/WAN project, back around the turn of the century,
> we refused to implement several things required by the RFCs
> because we thought they were insecure: null cipher, single
> DES & 768-bit DH Group 1.
>
> At that time, not having DES did cause some problems in
> interoperating with other IPsec implementations, but I
> doubt it would today. Neither of the other dropped items
> caused any problems at all.
>
> Today I'd say drop all of those plus the 1024-bit Group 2,
> and then look at whether others should go as well. As of
> 2001 or so, the 1536-bit Group 5 was very widely used,
> so dropping it might well be problematic, but I am not
> certain if it is either secure or widely used now.

This approach is fully appropriate and I strongly support that. However, the
kernel crypto API has a need of a memcpy over SGLs, because the entire crypto
API operates on SGLs. There are valid use cases. The one I am currently
working on are AEAD ciphers where we want to copy the AAD from the src SGL to
the dst SGL. Instead of walking through the SGLs in my code and invoke the
memcpy, I simply use the null cipher.

What I would like to say is that there are valid use cases of the null cipher
which do not impair security constraints. For those use cases, the null cipher
should and must be allowed.

For all other use cases, including the "encryption operation" in IPSEC or dm-
crypt, it should never be used.

Ciao
Stephan

2017-04-24 10:40:03

by Herbert Xu

[permalink] [raw]
Subject: Re: [PATCH] crypto: Allow ecb(cipher_null) in FIPS mode

Milan Broz <[email protected]> wrote:
> The cipher_null is not a real cipher, FIPS mode should not restrict its use.
>
> It is used for several tests (for example in cryptsetup testsuite) and also
> temporarily for reencryption of not yet encrypted device in cryptsetup-reencrypt tool.
>
> Problem is easily reproducible with
> cryptsetup benchmark -c null
>
> Signed-off-by: Milan Broz <[email protected]>

Patch applied. Thanks.
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt