diff mbox series

crypto: mark unused ciphers as obsolete

Message ID 20200911141103.14832-1-ardb@kernel.org (mailing list archive)
State Accepted
Delegated to: Herbert Xu
Headers show
Series crypto: mark unused ciphers as obsolete | expand

Commit Message

Ard Biesheuvel Sept. 11, 2020, 2:11 p.m. UTC
We have a few interesting pieces in our cipher museum, which are never
used internally, and were only ever provided as generic C implementations.

Unfortunately, we cannot simply remove this code, as we cannot be sure
that it is not being used via the AF_ALG socket API, however unlikely.

So let's mark the Anubis, Khazad, SEED and TEA algorithms as obsolete,
which means they can only be enabled in the build if the socket API is
enabled in the first place.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
Hopefully, I will be able to convince the distro kernel maintainers to
disable CRYPTO_USER_API_ENABLE_OBSOLETE in their v5.10+ builds once the
iwd changes for arc4 make it downstream (Debian already has an updated
version in its unstable distro). With the joint coverage of their QA,
we should be able to confirm that these algos are never used, and
actually remove them altogether.

 crypto/Kconfig | 4 ++++
 1 file changed, 4 insertions(+)

Comments

Van Leeuwen, Pascal Sept. 11, 2020, 4:23 p.m. UTC | #1
> -----Original Message-----
> From: linux-crypto-owner@vger.kernel.org <linux-crypto-owner@vger.kernel.org> On Behalf Of Ard Biesheuvel
> Sent: Friday, September 11, 2020 4:11 PM
> To: linux-crypto@vger.kernel.org
> Cc: herbert@gondor.apana.org.au; ebiggers@kernel.org; Ard Biesheuvel <ardb@kernel.org>
> Subject: [PATCH] crypto: mark unused ciphers as obsolete
>
> <<< External Email >>>
> We have a few interesting pieces in our cipher museum, which are never
> used internally, and were only ever provided as generic C implementations.
>
> Unfortunately, we cannot simply remove this code, as we cannot be sure
> that it is not being used via the AF_ALG socket API, however unlikely.
> So let's mark the Anubis, Khazad, SEED and TEA algorithms as obsolete,
>
Wouldn't the IKE deamon be able to utilize these algorithms through the XFRM API?
I'm by no means an expert on the subject, but it looks like the cipher template is
provided there directly via XFRM, so it does not need to live in the kernel source.
And I know for a fact that SEED is being used for IPsec (and TLS) in Korea.

The point being, there are more users to consider beyond "internal" (meaning hard
coded in the kernel source in this context?) and AF_ALG.

I'm not aware of any real use cases for Anubis, Khazad and TEA though.

> which means they can only be enabled in the build if the socket API is
> enabled in the first place.
>
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> ---
> Hopefully, I will be able to convince the distro kernel maintainers to
> disable CRYPTO_USER_API_ENABLE_OBSOLETE in their v5.10+ builds once the
> iwd changes for arc4 make it downstream (Debian already has an updated
> version in its unstable distro). With the joint coverage of their QA,
> we should be able to confirm that these algos are never used, and
> actually remove them altogether.
>
>  crypto/Kconfig | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/crypto/Kconfig b/crypto/Kconfig
> index e85d8a059489..fac10143d23f 100644
> --- a/crypto/Kconfig
> +++ b/crypto/Kconfig
> @@ -1185,6 +1185,7 @@ config CRYPTO_AES_PPC_SPE
>
>  config CRYPTO_ANUBIS
>  tristate "Anubis cipher algorithm"
> +depends on CRYPTO_USER_API_ENABLE_OBSOLETE
>  select CRYPTO_ALGAPI
>  help
>    Anubis cipher algorithm.
> @@ -1424,6 +1425,7 @@ config CRYPTO_FCRYPT
>
>  config CRYPTO_KHAZAD
>  tristate "Khazad cipher algorithm"
> +depends on CRYPTO_USER_API_ENABLE_OBSOLETE
>  select CRYPTO_ALGAPI
>  help
>    Khazad cipher algorithm.
> @@ -1487,6 +1489,7 @@ config CRYPTO_CHACHA_MIPS
>
>  config CRYPTO_SEED
>  tristate "SEED cipher algorithm"
> +depends on CRYPTO_USER_API_ENABLE_OBSOLETE
>  select CRYPTO_ALGAPI
>  help
>    SEED cipher algorithm (RFC4269).
> @@ -1613,6 +1616,7 @@ config CRYPTO_SM4
>
>  config CRYPTO_TEA
>  tristate "TEA, XTEA and XETA cipher algorithms"
> +depends on CRYPTO_USER_API_ENABLE_OBSOLETE
>  select CRYPTO_ALGAPI
>  help
>    TEA cipher algorithm.
> --
> 2.17.1

Regards,
Pascal van Leeuwen
Silicon IP Architect Multi-Protocol Engines, Rambus Security
Rambus ROTW Holding BV
+31-73 6581953

Note: The Inside Secure/Verimatrix Silicon IP team was recently acquired by Rambus.
Please be so kind to update your e-mail address book with my new e-mail address.


** This message and any attachments are for the sole use of the intended recipient(s). It may contain information that is confidential and privileged. If you are not the intended recipient of this message, you are prohibited from printing, copying, forwarding or saving it. Please delete the message and attachments and notify the sender immediately. **

Rambus Inc.<http://www.rambus.com>
Ard Biesheuvel Sept. 11, 2020, 4:30 p.m. UTC | #2
(cc Milan and dm-devel)

On Fri, 11 Sep 2020 at 19:24, Van Leeuwen, Pascal
<pvanleeuwen@rambus.com> wrote:
>
> > -----Original Message-----
> > From: linux-crypto-owner@vger.kernel.org <linux-crypto-owner@vger.kernel.org> On Behalf Of Ard Biesheuvel
> > Sent: Friday, September 11, 2020 4:11 PM
> > To: linux-crypto@vger.kernel.org
> > Cc: herbert@gondor.apana.org.au; ebiggers@kernel.org; Ard Biesheuvel <ardb@kernel.org>
> > Subject: [PATCH] crypto: mark unused ciphers as obsolete
> >
> > <<< External Email >>>
> > We have a few interesting pieces in our cipher museum, which are never
> > used internally, and were only ever provided as generic C implementations.
> >
> > Unfortunately, we cannot simply remove this code, as we cannot be sure
> > that it is not being used via the AF_ALG socket API, however unlikely.
> > So let's mark the Anubis, Khazad, SEED and TEA algorithms as obsolete,
> >
> Wouldn't the IKE deamon be able to utilize these algorithms through the XFRM API?
> I'm by no means an expert on the subject, but it looks like the cipher template is
> provided there directly via XFRM, so it does not need to live in the kernel source.
> And I know for a fact that SEED is being used for IPsec (and TLS) in Korea.
>

I have been staring at net/xfrm/xfrm_algo.c, and as far as I can tell,
algorithms have to be mentioned there in order to be usable. None of
the ciphers that this patch touches are listed there or anywhere else
in the kernel.

> The point being, there are more users to consider beyond "internal" (meaning hard
> coded in the kernel source in this context?) and AF_ALG.
>

That is a good point, actually, since dm-crypt could be affected here
as well, hence the CCs.

Milan (or others): are you aware of any of these ciphers being used
for dm-crypt?


> I'm not aware of any real use cases for Anubis, Khazad and TEA though.
>

OK, thanks for confirming. Removing those would be a good start.

> > which means they can only be enabled in the build if the socket API is
> > enabled in the first place.
> >
> > Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> > ---
> > Hopefully, I will be able to convince the distro kernel maintainers to
> > disable CRYPTO_USER_API_ENABLE_OBSOLETE in their v5.10+ builds once the
> > iwd changes for arc4 make it downstream (Debian already has an updated
> > version in its unstable distro). With the joint coverage of their QA,
> > we should be able to confirm that these algos are never used, and
> > actually remove them altogether.
> >
> >  crypto/Kconfig | 4 ++++
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/crypto/Kconfig b/crypto/Kconfig
> > index e85d8a059489..fac10143d23f 100644
> > --- a/crypto/Kconfig
> > +++ b/crypto/Kconfig
> > @@ -1185,6 +1185,7 @@ config CRYPTO_AES_PPC_SPE
> >
> >  config CRYPTO_ANUBIS
> >  tristate "Anubis cipher algorithm"
> > +depends on CRYPTO_USER_API_ENABLE_OBSOLETE
> >  select CRYPTO_ALGAPI
> >  help
> >    Anubis cipher algorithm.
> > @@ -1424,6 +1425,7 @@ config CRYPTO_FCRYPT
> >
> >  config CRYPTO_KHAZAD
> >  tristate "Khazad cipher algorithm"
> > +depends on CRYPTO_USER_API_ENABLE_OBSOLETE
> >  select CRYPTO_ALGAPI
> >  help
> >    Khazad cipher algorithm.
> > @@ -1487,6 +1489,7 @@ config CRYPTO_CHACHA_MIPS
> >
> >  config CRYPTO_SEED
> >  tristate "SEED cipher algorithm"
> > +depends on CRYPTO_USER_API_ENABLE_OBSOLETE
> >  select CRYPTO_ALGAPI
> >  help
> >    SEED cipher algorithm (RFC4269).
> > @@ -1613,6 +1616,7 @@ config CRYPTO_SM4
> >
> >  config CRYPTO_TEA
> >  tristate "TEA, XTEA and XETA cipher algorithms"
> > +depends on CRYPTO_USER_API_ENABLE_OBSOLETE
> >  select CRYPTO_ALGAPI
> >  help
> >    TEA cipher algorithm.
> > --
> > 2.17.1
>
> Regards,
> Pascal van Leeuwen
> Silicon IP Architect Multi-Protocol Engines, Rambus Security
> Rambus ROTW Holding BV
> +31-73 6581953
>
> Note: The Inside Secure/Verimatrix Silicon IP team was recently acquired by Rambus.
> Please be so kind to update your e-mail address book with my new e-mail address.
>
>
> ** This message and any attachments are for the sole use of the intended recipient(s). It may contain information that is confidential and privileged. If you are not the intended recipient of this message, you are prohibited from printing, copying, forwarding or saving it. Please delete the message and attachments and notify the sender immediately. **
>
> Rambus Inc.<http://www.rambus.com>
Van Leeuwen, Pascal Sept. 11, 2020, 4:46 p.m. UTC | #3
> -----Original Message-----
> From: Ard Biesheuvel <ardb@kernel.org>
> Sent: Friday, September 11, 2020 6:30 PM
> To: Van Leeuwen, Pascal <pvanleeuwen@rambus.com>; dm-devel@redhat.com; Milan Broz <gmazyland@gmail.com>
> Cc: linux-crypto@vger.kernel.org; herbert@gondor.apana.org.au; ebiggers@kernel.org
> Subject: Re: [PATCH] crypto: mark unused ciphers as obsolete
>
> <<< External Email >>>
> (cc Milan and dm-devel)
>
> On Fri, 11 Sep 2020 at 19:24, Van Leeuwen, Pascal
> <pvanleeuwen@rambus.com> wrote:
> >
> > > -----Original Message-----
> > > From: linux-crypto-owner@vger.kernel.org <linux-crypto-owner@vger.kernel.org> On Behalf Of Ard Biesheuvel
> > > Sent: Friday, September 11, 2020 4:11 PM
> > > To: linux-crypto@vger.kernel.org
> > > Cc: herbert@gondor.apana.org.au; ebiggers@kernel.org; Ard Biesheuvel <ardb@kernel.org>
> > > Subject: [PATCH] crypto: mark unused ciphers as obsolete
> > >
> > > <<< External Email >>>
> > > We have a few interesting pieces in our cipher museum, which are never
> > > used internally, and were only ever provided as generic C implementations.
> > >
> > > Unfortunately, we cannot simply remove this code, as we cannot be sure
> > > that it is not being used via the AF_ALG socket API, however unlikely.
> > > So let's mark the Anubis, Khazad, SEED and TEA algorithms as obsolete,
> > >
> > Wouldn't the IKE deamon be able to utilize these algorithms through the XFRM API?
> > I'm by no means an expert on the subject, but it looks like the cipher template is
> > provided there directly via XFRM, so it does not need to live in the kernel source.
> > And I know for a fact that SEED is being used for IPsec (and TLS) in Korea.
> >
>
> I have been staring at net/xfrm/xfrm_algo.c, and as far as I can tell,
> algorithms have to be mentioned there in order to be usable. None of
> the ciphers that this patch touches are listed there or anywhere else
> in the kernel.
>
Hmmm ... good point. Wasn't aware XFRM was actively allowing only a subset.
Actually found this commented out code in my Pluto source (kernel_netlink.c):

/*
 * Not yet implemented in Linux kernel xfrm_algo.c
{ SADB_X_EALG_SEEDCBC, "cbc(seed)" },
 */

Go figure.

> > The point being, there are more users to consider beyond "internal" (meaning hard
> > coded in the kernel source in this context?) and AF_ALG.
> >
>
> That is a good point, actually, since dm-crypt could be affected here
> as well, hence the CCs.
>
> Milan (or others): are you aware of any of these ciphers being used
> for dm-crypt?
>
>
> > I'm not aware of any real use cases for Anubis, Khazad and TEA though.
> >
>
> OK, thanks for confirming. Removing those would be a good start.
>
> > > which means they can only be enabled in the build if the socket API is
> > > enabled in the first place.
> > >
> > > Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> > > ---
> > > Hopefully, I will be able to convince the distro kernel maintainers to
> > > disable CRYPTO_USER_API_ENABLE_OBSOLETE in their v5.10+ builds once the
> > > iwd changes for arc4 make it downstream (Debian already has an updated
> > > version in its unstable distro). With the joint coverage of their QA,
> > > we should be able to confirm that these algos are never used, and
> > > actually remove them altogether.
> > >
> > >  crypto/Kconfig | 4 ++++
> > >  1 file changed, 4 insertions(+)
> > >
> > > diff --git a/crypto/Kconfig b/crypto/Kconfig
> > > index e85d8a059489..fac10143d23f 100644
> > > --- a/crypto/Kconfig
> > > +++ b/crypto/Kconfig
> > > @@ -1185,6 +1185,7 @@ config CRYPTO_AES_PPC_SPE
> > >
> > >  config CRYPTO_ANUBIS
> > >  tristate "Anubis cipher algorithm"
> > > +depends on CRYPTO_USER_API_ENABLE_OBSOLETE
> > >  select CRYPTO_ALGAPI
> > >  help
> > >    Anubis cipher algorithm.
> > > @@ -1424,6 +1425,7 @@ config CRYPTO_FCRYPT
> > >
> > >  config CRYPTO_KHAZAD
> > >  tristate "Khazad cipher algorithm"
> > > +depends on CRYPTO_USER_API_ENABLE_OBSOLETE
> > >  select CRYPTO_ALGAPI
> > >  help
> > >    Khazad cipher algorithm.
> > > @@ -1487,6 +1489,7 @@ config CRYPTO_CHACHA_MIPS
> > >
> > >  config CRYPTO_SEED
> > >  tristate "SEED cipher algorithm"
> > > +depends on CRYPTO_USER_API_ENABLE_OBSOLETE
> > >  select CRYPTO_ALGAPI
> > >  help
> > >    SEED cipher algorithm (RFC4269).
> > > @@ -1613,6 +1616,7 @@ config CRYPTO_SM4
> > >
> > >  config CRYPTO_TEA
> > >  tristate "TEA, XTEA and XETA cipher algorithms"
> > > +depends on CRYPTO_USER_API_ENABLE_OBSOLETE
> > >  select CRYPTO_ALGAPI
> > >  help
> > >    TEA cipher algorithm.
> > > --
> > > 2.17.1
> >
> > Regards,
> > Pascal van Leeuwen
> > Silicon IP Architect Multi-Protocol Engines, Rambus Security
> > Rambus ROTW Holding BV
> > +31-73 6581953
> >
> > Note: The Inside Secure/Verimatrix Silicon IP team was recently acquired by Rambus.
> > Please be so kind to update your e-mail address book with my new e-mail address.
> >
> >
> > ** This message and any attachments are for the sole use of the intended recipient(s). It may contain information that is
> confidential and privileged. If you are not the intended recipient of this message, you are prohibited from printing, copying,
> forwarding or saving it. Please delete the message and attachments and notify the sender immediately. **
> >
> > Rambus Inc.<http://www.rambus.com>

Regards,
Pascal van Leeuwen
Silicon IP Architect Multi-Protocol Engines, Rambus Security
Rambus ROTW Holding BV
+31-73 6581953

Note: The Inside Secure/Verimatrix Silicon IP team was recently acquired by Rambus.
Please be so kind to update your e-mail address book with my new e-mail address.


** This message and any attachments are for the sole use of the intended recipient(s). It may contain information that is confidential and privileged. If you are not the intended recipient of this message, you are prohibited from printing, copying, forwarding or saving it. Please delete the message and attachments and notify the sender immediately. **

Rambus Inc.<http://www.rambus.com>
Milan Broz Sept. 12, 2020, 10:05 a.m. UTC | #4
On 11/09/2020 18:30, Ard Biesheuvel wrote:
> (cc Milan and dm-devel)
> 
> On Fri, 11 Sep 2020 at 19:24, Van Leeuwen, Pascal
> <pvanleeuwen@rambus.com> wrote:
>>
>>> -----Original Message-----
>>> From: linux-crypto-owner@vger.kernel.org <linux-crypto-owner@vger.kernel.org> On Behalf Of Ard Biesheuvel
>>> Sent: Friday, September 11, 2020 4:11 PM
>>> To: linux-crypto@vger.kernel.org
>>> Cc: herbert@gondor.apana.org.au; ebiggers@kernel.org; Ard Biesheuvel <ardb@kernel.org>
>>> Subject: [PATCH] crypto: mark unused ciphers as obsolete
>>>
>>> <<< External Email >>>
>>> We have a few interesting pieces in our cipher museum, which are never
>>> used internally, and were only ever provided as generic C implementations.
>>>
>>> Unfortunately, we cannot simply remove this code, as we cannot be sure
>>> that it is not being used via the AF_ALG socket API, however unlikely.
>>> So let's mark the Anubis, Khazad, SEED and TEA algorithms as obsolete,
>>>
>> Wouldn't the IKE deamon be able to utilize these algorithms through the XFRM API?
>> I'm by no means an expert on the subject, but it looks like the cipher template is
>> provided there directly via XFRM, so it does not need to live in the kernel source.
>> And I know for a fact that SEED is being used for IPsec (and TLS) in Korea.
>>
> 
> I have been staring at net/xfrm/xfrm_algo.c, and as far as I can tell,
> algorithms have to be mentioned there in order to be usable. None of
> the ciphers that this patch touches are listed there or anywhere else
> in the kernel.
> 
>> The point being, there are more users to consider beyond "internal" (meaning hard
>> coded in the kernel source in this context?) and AF_ALG.
>>
> 
> That is a good point, actually, since dm-crypt could be affected here
> as well, hence the CCs.
> 
> Milan (or others): are you aware of any of these ciphers being used
> for dm-crypt?

Cryptsetup/dm-crypt can use them (talking about Seed, Khazad, Anubis, TEA), but I think
there is no real use of these.
(IOW these are used only if someone deliberately uses them - manually specifying on format.)

For dm-crypt. there should be no big harm if these are marked obsolete.

Milan
Herbert Xu Sept. 18, 2020, 7:30 a.m. UTC | #5
On Fri, Sep 11, 2020 at 05:11:03PM +0300, Ard Biesheuvel wrote:
> We have a few interesting pieces in our cipher museum, which are never
> used internally, and were only ever provided as generic C implementations.
> 
> Unfortunately, we cannot simply remove this code, as we cannot be sure
> that it is not being used via the AF_ALG socket API, however unlikely.
> 
> So let's mark the Anubis, Khazad, SEED and TEA algorithms as obsolete,
> which means they can only be enabled in the build if the socket API is
> enabled in the first place.
> 
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> ---
> Hopefully, I will be able to convince the distro kernel maintainers to
> disable CRYPTO_USER_API_ENABLE_OBSOLETE in their v5.10+ builds once the
> iwd changes for arc4 make it downstream (Debian already has an updated
> version in its unstable distro). With the joint coverage of their QA,
> we should be able to confirm that these algos are never used, and
> actually remove them altogether.
> 
>  crypto/Kconfig | 4 ++++
>  1 file changed, 4 insertions(+)

Patch applied.  Thanks.
diff mbox series

Patch

diff --git a/crypto/Kconfig b/crypto/Kconfig
index e85d8a059489..fac10143d23f 100644
--- a/crypto/Kconfig
+++ b/crypto/Kconfig
@@ -1185,6 +1185,7 @@  config CRYPTO_AES_PPC_SPE
 
 config CRYPTO_ANUBIS
 	tristate "Anubis cipher algorithm"
+	depends on CRYPTO_USER_API_ENABLE_OBSOLETE
 	select CRYPTO_ALGAPI
 	help
 	  Anubis cipher algorithm.
@@ -1424,6 +1425,7 @@  config CRYPTO_FCRYPT
 
 config CRYPTO_KHAZAD
 	tristate "Khazad cipher algorithm"
+	depends on CRYPTO_USER_API_ENABLE_OBSOLETE
 	select CRYPTO_ALGAPI
 	help
 	  Khazad cipher algorithm.
@@ -1487,6 +1489,7 @@  config CRYPTO_CHACHA_MIPS
 
 config CRYPTO_SEED
 	tristate "SEED cipher algorithm"
+	depends on CRYPTO_USER_API_ENABLE_OBSOLETE
 	select CRYPTO_ALGAPI
 	help
 	  SEED cipher algorithm (RFC4269).
@@ -1613,6 +1616,7 @@  config CRYPTO_SM4
 
 config CRYPTO_TEA
 	tristate "TEA, XTEA and XETA cipher algorithms"
+	depends on CRYPTO_USER_API_ENABLE_OBSOLETE
 	select CRYPTO_ALGAPI
 	help
 	  TEA cipher algorithm.