Linux-Wireless Archive on lore.kernel.org
 help / color / Atom feed
* Re: [RFC PATCH 4/7] crypto: remove ARC4 support from the skcipher API
       [not found] ` <20200702101947.682-5-ardb@kernel.org>
@ 2020-07-02 17:50   ` Eric Biggers
  2020-07-02 18:21     ` Ard Biesheuvel
  0 siblings, 1 reply; 6+ messages in thread
From: Eric Biggers @ 2020-07-02 17:50 UTC (permalink / raw)
  To: Ard Biesheuvel, linux-wireless, Marcel Holtmann, Denis Kenzior
  Cc: linux-kernel, Herbert Xu, David S. Miller, Greg Kroah-Hartman,
	Trond Myklebust, Anna Schumaker, J. Bruce Fields, Chuck Lever,
	linux-crypto, netdev, devel, linux-nfs

[+linux-wireless, Marcel Holtmann, and Denis Kenzior]

On Thu, Jul 02, 2020 at 12:19:44PM +0200, Ard Biesheuvel wrote:
> Remove the generic ecb(arc4) skcipher, which is slightly cumbersome from
> a maintenance perspective, since it does not quite behave like other
> skciphers do in terms of key vs IV lifetime. Since we are leaving the
> library interface in place, which is used by the various WEP and TKIP
> implementations we have in the tree, we can safely drop this code now
> it no longer has any users.
> 
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>

Last year there was a discussion where it was mentioned that iwd uses
"ecb(arc4)" via AF_ALG.  So can we really remove it yet?
See https://lkml.kernel.org/r/97BB95F6-4A4C-4984-9EAB-6069E19B4A4F@holtmann.org
Note that the code isn't in "iwd" itself but rather in "libell" which iwd
depends on: https://git.kernel.org/pub/scm/libs/ell/ell.git/

Apparently it also uses md4 and ecb(des) too.

Marcel and Denis, what's your deprecation plan for these obsolete and insecure
algorithms?

- Eric

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

* Re: [RFC PATCH 4/7] crypto: remove ARC4 support from the skcipher API
  2020-07-02 17:50   ` [RFC PATCH 4/7] crypto: remove ARC4 support from the skcipher API Eric Biggers
@ 2020-07-02 18:21     ` Ard Biesheuvel
  2020-07-02 23:04       ` Ard Biesheuvel
  0 siblings, 1 reply; 6+ messages in thread
From: Ard Biesheuvel @ 2020-07-02 18:21 UTC (permalink / raw)
  To: Eric Biggers
  Cc: linux-wireless, Marcel Holtmann, Denis Kenzior,
	Linux Kernel Mailing List, Herbert Xu, David S. Miller,
	Greg Kroah-Hartman, Trond Myklebust, Anna Schumaker,
	J. Bruce Fields, Chuck Lever, Linux Crypto Mailing List, netdev,
	devel, linux-nfs

On Thu, 2 Jul 2020 at 19:50, Eric Biggers <ebiggers@kernel.org> wrote:
>
> [+linux-wireless, Marcel Holtmann, and Denis Kenzior]
>
> On Thu, Jul 02, 2020 at 12:19:44PM +0200, Ard Biesheuvel wrote:
> > Remove the generic ecb(arc4) skcipher, which is slightly cumbersome from
> > a maintenance perspective, since it does not quite behave like other
> > skciphers do in terms of key vs IV lifetime. Since we are leaving the
> > library interface in place, which is used by the various WEP and TKIP
> > implementations we have in the tree, we can safely drop this code now
> > it no longer has any users.
> >
> > Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
>
> Last year there was a discussion where it was mentioned that iwd uses
> "ecb(arc4)" via AF_ALG.  So can we really remove it yet?
> See https://lkml.kernel.org/r/97BB95F6-4A4C-4984-9EAB-6069E19B4A4F@holtmann.org
> Note that the code isn't in "iwd" itself but rather in "libell" which iwd
> depends on: https://git.kernel.org/pub/scm/libs/ell/ell.git/
>
> Apparently it also uses md4 and ecb(des) too.
>

Ah yes, I remember now :-(

> Marcel and Denis, what's your deprecation plan for these obsolete and insecure
> algorithms?
>

Given Denis's statement:

  It sounds to me like it was broken and should be fixed.  So our vote /
  preference is to have ARC4 fixed to follow the proper semantics.  We
  can deal with the kernel behavioral change on our end easily enough;
  the required workarounds are the worse evil.

I would think that an ABI break is not the end of the world for them,
and given how trivial it is to implement RC4 in C, the workaround
should be to simply implement RC4 in user space, and not even bother
trying to use AF_ALG to get at ecb(arc4)

(same applies to md4 and ecb(des) btw)

There will always be a long tail of use cases, and at some point, we
just have to draw the line and remove obsolete and insecure cruft,
especially when it impedes progress on other fronts.



Full implementation of arc4 aka ecb(arc4) below.

void arc4_crypt(struct arc4_ctx *ctx, u8 *out, const u8 *in, unsigned int len)
{
  u32 *const S = ctx->S;
  u32 x, y, a, b;
  u32 ty, ta, tb;

  if (len == 0)
    return;

  x = ctx->x;
  y = ctx->y;

  a = S[x];
  y = (y + a) & 0xff;
  b = S[y];

  do {
    S[y] = a;
    a = (a + b) & 0xff;
    S[x] = b;
    x = (x + 1) & 0xff;
    ta = S[x];
    ty = (y + ta) & 0xff;
    tb = S[ty];
    *out++ = *in++ ^ S[a];
    if (--len == 0)
      break;
    y = ty;
    a = ta;
    b = tb;
  } while (true);

  ctx->x = x;
  ctx->y = y;
}

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

* Re: [RFC PATCH 4/7] crypto: remove ARC4 support from the skcipher API
  2020-07-02 18:21     ` Ard Biesheuvel
@ 2020-07-02 23:04       ` Ard Biesheuvel
  2020-07-18  8:18         ` Ard Biesheuvel
  0 siblings, 1 reply; 6+ messages in thread
From: Ard Biesheuvel @ 2020-07-02 23:04 UTC (permalink / raw)
  To: Eric Biggers
  Cc: linux-wireless, Marcel Holtmann, Denis Kenzior,
	Linux Kernel Mailing List, Herbert Xu, David S. Miller,
	Greg Kroah-Hartman, Trond Myklebust, Anna Schumaker,
	J. Bruce Fields, Chuck Lever, Linux Crypto Mailing List, netdev,
	devel, linux-nfs

On Thu, 2 Jul 2020 at 20:21, Ard Biesheuvel <ardb@kernel.org> wrote:
>
> On Thu, 2 Jul 2020 at 19:50, Eric Biggers <ebiggers@kernel.org> wrote:
> >
> > [+linux-wireless, Marcel Holtmann, and Denis Kenzior]
> >
> > On Thu, Jul 02, 2020 at 12:19:44PM +0200, Ard Biesheuvel wrote:
> > > Remove the generic ecb(arc4) skcipher, which is slightly cumbersome from
> > > a maintenance perspective, since it does not quite behave like other
> > > skciphers do in terms of key vs IV lifetime. Since we are leaving the
> > > library interface in place, which is used by the various WEP and TKIP
> > > implementations we have in the tree, we can safely drop this code now
> > > it no longer has any users.
> > >
> > > Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> >
> > Last year there was a discussion where it was mentioned that iwd uses
> > "ecb(arc4)" via AF_ALG.  So can we really remove it yet?
> > See https://lkml.kernel.org/r/97BB95F6-4A4C-4984-9EAB-6069E19B4A4F@holtmann.org
> > Note that the code isn't in "iwd" itself but rather in "libell" which iwd
> > depends on: https://git.kernel.org/pub/scm/libs/ell/ell.git/
> >
> > Apparently it also uses md4 and ecb(des) too.
> >
>
> Ah yes, I remember now :-(
>
> > Marcel and Denis, what's your deprecation plan for these obsolete and insecure
> > algorithms?
> >
>
> Given Denis's statement:
>
>   It sounds to me like it was broken and should be fixed.  So our vote /
>   preference is to have ARC4 fixed to follow the proper semantics.  We
>   can deal with the kernel behavioral change on our end easily enough;
>   the required workarounds are the worse evil.
>
> I would think that an ABI break is not the end of the world for them,
> and given how trivial it is to implement RC4 in C, the workaround
> should be to simply implement RC4 in user space, and not even bother
> trying to use AF_ALG to get at ecb(arc4)
>
> (same applies to md4 and ecb(des) btw)
>
> There will always be a long tail of use cases, and at some point, we
> just have to draw the line and remove obsolete and insecure cruft,
> especially when it impedes progress on other fronts.
>

I have ported iwd to Nettle's LGPL 2.1 implementation of ARC4, and the
diffstat is

 src/crypto.c      | 80 ++++++++++++--------
 src/main.c        |  8 --
 unit/test-eapol.c |  3 +-
 3 files changed, 51 insertions(+), 40 deletions(-)

https://git.kernel.org/pub/scm/linux/kernel/git/ardb/iwd.git/log/?h=arc4-cleanup

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

* Re: [RFC PATCH 4/7] crypto: remove ARC4 support from the skcipher API
  2020-07-02 23:04       ` Ard Biesheuvel
@ 2020-07-18  8:18         ` Ard Biesheuvel
  2020-07-25  7:06           ` Ard Biesheuvel
  0 siblings, 1 reply; 6+ messages in thread
From: Ard Biesheuvel @ 2020-07-18  8:18 UTC (permalink / raw)
  To: Eric Biggers
  Cc: linux-wireless, Marcel Holtmann, Denis Kenzior,
	Linux Kernel Mailing List, Herbert Xu, David S. Miller,
	Greg Kroah-Hartman, Trond Myklebust, Anna Schumaker,
	J. Bruce Fields, Chuck Lever, Linux Crypto Mailing List,
	open list:BPF JIT for MIPS (32-BIT AND 64-BIT),
	devel, linux-nfs

On Fri, 3 Jul 2020 at 02:04, Ard Biesheuvel <ardb@kernel.org> wrote:
>
> On Thu, 2 Jul 2020 at 20:21, Ard Biesheuvel <ardb@kernel.org> wrote:
> >
> > On Thu, 2 Jul 2020 at 19:50, Eric Biggers <ebiggers@kernel.org> wrote:
> > >
> > > [+linux-wireless, Marcel Holtmann, and Denis Kenzior]
> > >
> > > On Thu, Jul 02, 2020 at 12:19:44PM +0200, Ard Biesheuvel wrote:
> > > > Remove the generic ecb(arc4) skcipher, which is slightly cumbersome from
> > > > a maintenance perspective, since it does not quite behave like other
> > > > skciphers do in terms of key vs IV lifetime. Since we are leaving the
> > > > library interface in place, which is used by the various WEP and TKIP
> > > > implementations we have in the tree, we can safely drop this code now
> > > > it no longer has any users.
> > > >
> > > > Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> > >
> > > Last year there was a discussion where it was mentioned that iwd uses
> > > "ecb(arc4)" via AF_ALG.  So can we really remove it yet?
> > > See https://lkml.kernel.org/r/97BB95F6-4A4C-4984-9EAB-6069E19B4A4F@holtmann.org
> > > Note that the code isn't in "iwd" itself but rather in "libell" which iwd
> > > depends on: https://git.kernel.org/pub/scm/libs/ell/ell.git/
> > >
> > > Apparently it also uses md4 and ecb(des) too.
> > >
> >
> > Ah yes, I remember now :-(
> >
> > > Marcel and Denis, what's your deprecation plan for these obsolete and insecure
> > > algorithms?
> > >
> >
> > Given Denis's statement:
> >
> >   It sounds to me like it was broken and should be fixed.  So our vote /
> >   preference is to have ARC4 fixed to follow the proper semantics.  We
> >   can deal with the kernel behavioral change on our end easily enough;
> >   the required workarounds are the worse evil.
> >
> > I would think that an ABI break is not the end of the world for them,
> > and given how trivial it is to implement RC4 in C, the workaround
> > should be to simply implement RC4 in user space, and not even bother
> > trying to use AF_ALG to get at ecb(arc4)
> >
> > (same applies to md4 and ecb(des) btw)
> >
> > There will always be a long tail of use cases, and at some point, we
> > just have to draw the line and remove obsolete and insecure cruft,
> > especially when it impedes progress on other fronts.
> >
>
> I have ported iwd to Nettle's LGPL 2.1 implementation of ARC4, and the
> diffstat is
>
>  src/crypto.c      | 80 ++++++++++++--------
>  src/main.c        |  8 --
>  unit/test-eapol.c |  3 +-
>  3 files changed, 51 insertions(+), 40 deletions(-)
>
> https://git.kernel.org/pub/scm/linux/kernel/git/ardb/iwd.git/log/?h=arc4-cleanup

Marcel, Denis,

Do you have any objections to the ecb(arc4) skcipher being dropped
from the kernel, given the fallback i proposed above (which is a much
better way of doing rc4 in user space anyway)?

For libell, I would suggest dropping rc4 entirely, once iwd stops
relying on it, as using rc4 for tls is obsolete as well.

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

* Re: [RFC PATCH 4/7] crypto: remove ARC4 support from the skcipher API
  2020-07-18  8:18         ` Ard Biesheuvel
@ 2020-07-25  7:06           ` Ard Biesheuvel
  2020-08-04 13:59             ` Ard Biesheuvel
  0 siblings, 1 reply; 6+ messages in thread
From: Ard Biesheuvel @ 2020-07-25  7:06 UTC (permalink / raw)
  To: Eric Biggers
  Cc: linux-wireless, Marcel Holtmann, Denis Kenzior,
	Linux Kernel Mailing List, Herbert Xu, David S. Miller,
	Greg Kroah-Hartman, Trond Myklebust, Anna Schumaker,
	J. Bruce Fields, Chuck Lever, Linux Crypto Mailing List,
	open list:BPF JIT for MIPS (32-BIT AND 64-BIT),
	devel, linux-nfs

On Sat, 18 Jul 2020 at 11:18, Ard Biesheuvel <ardb@kernel.org> wrote:
>
> On Fri, 3 Jul 2020 at 02:04, Ard Biesheuvel <ardb@kernel.org> wrote:
> >
> > On Thu, 2 Jul 2020 at 20:21, Ard Biesheuvel <ardb@kernel.org> wrote:
> > >
> > > On Thu, 2 Jul 2020 at 19:50, Eric Biggers <ebiggers@kernel.org> wrote:
> > > >
> > > > [+linux-wireless, Marcel Holtmann, and Denis Kenzior]
> > > >
> > > > On Thu, Jul 02, 2020 at 12:19:44PM +0200, Ard Biesheuvel wrote:
> > > > > Remove the generic ecb(arc4) skcipher, which is slightly cumbersome from
> > > > > a maintenance perspective, since it does not quite behave like other
> > > > > skciphers do in terms of key vs IV lifetime. Since we are leaving the
> > > > > library interface in place, which is used by the various WEP and TKIP
> > > > > implementations we have in the tree, we can safely drop this code now
> > > > > it no longer has any users.
> > > > >
> > > > > Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> > > >
> > > > Last year there was a discussion where it was mentioned that iwd uses
> > > > "ecb(arc4)" via AF_ALG.  So can we really remove it yet?
> > > > See https://lkml.kernel.org/r/97BB95F6-4A4C-4984-9EAB-6069E19B4A4F@holtmann.org
> > > > Note that the code isn't in "iwd" itself but rather in "libell" which iwd
> > > > depends on: https://git.kernel.org/pub/scm/libs/ell/ell.git/
> > > >
> > > > Apparently it also uses md4 and ecb(des) too.
> > > >
> > >
> > > Ah yes, I remember now :-(
> > >
> > > > Marcel and Denis, what's your deprecation plan for these obsolete and insecure
> > > > algorithms?
> > > >
> > >
> > > Given Denis's statement:
> > >
> > >   It sounds to me like it was broken and should be fixed.  So our vote /
> > >   preference is to have ARC4 fixed to follow the proper semantics.  We
> > >   can deal with the kernel behavioral change on our end easily enough;
> > >   the required workarounds are the worse evil.
> > >
> > > I would think that an ABI break is not the end of the world for them,
> > > and given how trivial it is to implement RC4 in C, the workaround
> > > should be to simply implement RC4 in user space, and not even bother
> > > trying to use AF_ALG to get at ecb(arc4)
> > >
> > > (same applies to md4 and ecb(des) btw)
> > >
> > > There will always be a long tail of use cases, and at some point, we
> > > just have to draw the line and remove obsolete and insecure cruft,
> > > especially when it impedes progress on other fronts.
> > >
> >
> > I have ported iwd to Nettle's LGPL 2.1 implementation of ARC4, and the
> > diffstat is
> >
> >  src/crypto.c      | 80 ++++++++++++--------
> >  src/main.c        |  8 --
> >  unit/test-eapol.c |  3 +-
> >  3 files changed, 51 insertions(+), 40 deletions(-)
> >
> > https://git.kernel.org/pub/scm/linux/kernel/git/ardb/iwd.git/log/?h=arc4-cleanup
>
> Marcel, Denis,
>
> Do you have any objections to the ecb(arc4) skcipher being dropped
> from the kernel, given the fallback i proposed above (which is a much
> better way of doing rc4 in user space anyway)?
>
> For libell, I would suggest dropping rc4 entirely, once iwd stops
> relying on it, as using rc4 for tls is obsolete as well.

Ping?

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

* Re: [RFC PATCH 4/7] crypto: remove ARC4 support from the skcipher API
  2020-07-25  7:06           ` Ard Biesheuvel
@ 2020-08-04 13:59             ` Ard Biesheuvel
  0 siblings, 0 replies; 6+ messages in thread
From: Ard Biesheuvel @ 2020-08-04 13:59 UTC (permalink / raw)
  To: Eric Biggers, Denis Kenzior, Marcel Holtmann
  Cc: linux-wireless, Linux Kernel Mailing List, Herbert Xu,
	David S. Miller, Greg Kroah-Hartman, Trond Myklebust,
	Anna Schumaker, J. Bruce Fields, Chuck Lever,
	Linux Crypto Mailing List,
	open list:BPF JIT for MIPS (32-BIT AND 64-BIT),
	devel, linux-nfs

On Sat, 25 Jul 2020 at 10:06, Ard Biesheuvel <ardb@kernel.org> wrote:
>
> On Sat, 18 Jul 2020 at 11:18, Ard Biesheuvel <ardb@kernel.org> wrote:
> >
> > On Fri, 3 Jul 2020 at 02:04, Ard Biesheuvel <ardb@kernel.org> wrote:
> > >
> > > On Thu, 2 Jul 2020 at 20:21, Ard Biesheuvel <ardb@kernel.org> wrote:
> > > >
> > > > On Thu, 2 Jul 2020 at 19:50, Eric Biggers <ebiggers@kernel.org> wrote:
> > > > >
> > > > > [+linux-wireless, Marcel Holtmann, and Denis Kenzior]
> > > > >
> > > > > On Thu, Jul 02, 2020 at 12:19:44PM +0200, Ard Biesheuvel wrote:
> > > > > > Remove the generic ecb(arc4) skcipher, which is slightly cumbersome from
> > > > > > a maintenance perspective, since it does not quite behave like other
> > > > > > skciphers do in terms of key vs IV lifetime. Since we are leaving the
> > > > > > library interface in place, which is used by the various WEP and TKIP
> > > > > > implementations we have in the tree, we can safely drop this code now
> > > > > > it no longer has any users.
> > > > > >
> > > > > > Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> > > > >
> > > > > Last year there was a discussion where it was mentioned that iwd uses
> > > > > "ecb(arc4)" via AF_ALG.  So can we really remove it yet?
> > > > > See https://lkml.kernel.org/r/97BB95F6-4A4C-4984-9EAB-6069E19B4A4F@holtmann.org
> > > > > Note that the code isn't in "iwd" itself but rather in "libell" which iwd
> > > > > depends on: https://git.kernel.org/pub/scm/libs/ell/ell.git/
> > > > >
> > > > > Apparently it also uses md4 and ecb(des) too.
> > > > >
> > > >
> > > > Ah yes, I remember now :-(
> > > >
> > > > > Marcel and Denis, what's your deprecation plan for these obsolete and insecure
> > > > > algorithms?
> > > > >
> > > >
> > > > Given Denis's statement:
> > > >
> > > >   It sounds to me like it was broken and should be fixed.  So our vote /
> > > >   preference is to have ARC4 fixed to follow the proper semantics.  We
> > > >   can deal with the kernel behavioral change on our end easily enough;
> > > >   the required workarounds are the worse evil.
> > > >
> > > > I would think that an ABI break is not the end of the world for them,
> > > > and given how trivial it is to implement RC4 in C, the workaround
> > > > should be to simply implement RC4 in user space, and not even bother
> > > > trying to use AF_ALG to get at ecb(arc4)
> > > >
> > > > (same applies to md4 and ecb(des) btw)
> > > >
> > > > There will always be a long tail of use cases, and at some point, we
> > > > just have to draw the line and remove obsolete and insecure cruft,
> > > > especially when it impedes progress on other fronts.
> > > >
> > >
> > > I have ported iwd to Nettle's LGPL 2.1 implementation of ARC4, and the
> > > diffstat is
> > >
> > >  src/crypto.c      | 80 ++++++++++++--------
> > >  src/main.c        |  8 --
> > >  unit/test-eapol.c |  3 +-
> > >  3 files changed, 51 insertions(+), 40 deletions(-)
> > >
> > > https://git.kernel.org/pub/scm/linux/kernel/git/ardb/iwd.git/log/?h=arc4-cleanup
> >
> > Marcel, Denis,
> >
> > Do you have any objections to the ecb(arc4) skcipher being dropped
> > from the kernel, given the fallback i proposed above (which is a much
> > better way of doing rc4 in user space anyway)?
> >
> > For libell, I would suggest dropping rc4 entirely, once iwd stops
> > relying on it, as using rc4 for tls is obsolete as well.
>
> Ping?

Denis was kind enough to take the changes to iwd and libell that
remove all dependencies on the ecb(arc4) skcipher exposed by the
kernel, so we can at least deprecate it in the short term, and
hopefully remove it entirely at a later stage.

Perhaps we should introduce a Kconfig symbol that needs to be set to
enable deprecated algorithms? That way, we can work with the distros
to phase out the old junk that is piling up, but in a way that doesn't
break people's systems.

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

end of thread, back to index

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <20200702101947.682-1-ardb@kernel.org>
     [not found] ` <20200702101947.682-5-ardb@kernel.org>
2020-07-02 17:50   ` [RFC PATCH 4/7] crypto: remove ARC4 support from the skcipher API Eric Biggers
2020-07-02 18:21     ` Ard Biesheuvel
2020-07-02 23:04       ` Ard Biesheuvel
2020-07-18  8:18         ` Ard Biesheuvel
2020-07-25  7:06           ` Ard Biesheuvel
2020-08-04 13:59             ` Ard Biesheuvel

Linux-Wireless Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-wireless/0 linux-wireless/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-wireless linux-wireless/ https://lore.kernel.org/linux-wireless \
		linux-wireless@vger.kernel.org
	public-inbox-index linux-wireless

Example config snippet for mirrors

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


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