Received-SPF: pass (google.com: best guess record for domain of linux-crypto-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Tue, 25 Jun 2019 10:12:36 -0700
From:   Eric Biggers <ebiggers@kernel.org>
To:     Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc:     linux-crypto@vger.kernel.org, herbert@gondor.apana.org.au,
        omosnace@redhat.com, geert@linux-m68k.org,
        Milan Broz <gmazyland@gmail.com>
Subject: Re: [PATCH] crypto: morus - remove generic and x86 implementations
Message-ID: <20190625171234.GB81914@gmail.com>
References: <20190625145254.28510-1-ard.biesheuvel@linaro.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20190625145254.28510-1-ard.biesheuvel@linaro.org>
User-Agent: Mutt/1.10.1 (2018-07-13)
Sender: linux-crypto-owner@vger.kernel.org
Precedence: bulk

[+Cc Milan]

On Tue, Jun 25, 2019 at 04:52:54PM +0200, Ard Biesheuvel wrote:
> MORUS was not selected as a winner in the CAESAR competition, which
> is not surprising since it is considered to be cryptographically
> broken. (Note that this is not an implementation defect, but a flaw
> in the underlying algorithm). Since it is unlikely to be in use
> currently, let's remove it before we're stuck with it.
> 
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  arch/m68k/configs/amiga_defconfig     |    2 -
>  arch/m68k/configs/apollo_defconfig    |    2 -
>  arch/m68k/configs/atari_defconfig     |    2 -
>  arch/m68k/configs/bvme6000_defconfig  |    2 -
>  arch/m68k/configs/hp300_defconfig     |    2 -
>  arch/m68k/configs/mac_defconfig       |    2 -
>  arch/m68k/configs/multi_defconfig     |    2 -
>  arch/m68k/configs/mvme147_defconfig   |    2 -
>  arch/m68k/configs/mvme16x_defconfig   |    2 -
>  arch/m68k/configs/q40_defconfig       |    2 -
>  arch/m68k/configs/sun3_defconfig      |    2 -
>  arch/m68k/configs/sun3x_defconfig     |    2 -
>  arch/x86/crypto/Makefile              |   13 -
>  arch/x86/crypto/morus1280-avx2-asm.S  |  622 ---------
>  arch/x86/crypto/morus1280-avx2-glue.c |   66 -
>  arch/x86/crypto/morus1280-sse2-asm.S  |  896 -------------
>  arch/x86/crypto/morus1280-sse2-glue.c |   65 -
>  arch/x86/crypto/morus1280_glue.c      |  209 ---
>  arch/x86/crypto/morus640-sse2-asm.S   |  615 ---------
>  arch/x86/crypto/morus640-sse2-glue.c  |   65 -
>  arch/x86/crypto/morus640_glue.c       |  204 ---
>  crypto/Kconfig                        |   56 -
>  crypto/Makefile                       |    2 -
>  crypto/morus1280.c                    |  542 --------
>  crypto/morus640.c                     |  533 --------
>  crypto/testmgr.c                      |   12 -
>  crypto/testmgr.h                      | 1707 -------------------------
>  include/crypto/morus1280_glue.h       |   97 --
>  include/crypto/morus640_glue.h        |   97 --
>  include/crypto/morus_common.h         |   18 -
>  30 files changed, 5843 deletions(-)
>  delete mode 100644 arch/x86/crypto/morus1280-avx2-asm.S
>  delete mode 100644 arch/x86/crypto/morus1280-avx2-glue.c
>  delete mode 100644 arch/x86/crypto/morus1280-sse2-asm.S
>  delete mode 100644 arch/x86/crypto/morus1280-sse2-glue.c
>  delete mode 100644 arch/x86/crypto/morus1280_glue.c
>  delete mode 100644 arch/x86/crypto/morus640-sse2-asm.S
>  delete mode 100644 arch/x86/crypto/morus640-sse2-glue.c
>  delete mode 100644 arch/x86/crypto/morus640_glue.c
>  delete mode 100644 crypto/morus1280.c
>  delete mode 100644 crypto/morus640.c
>  delete mode 100644 include/crypto/morus1280_glue.h
>  delete mode 100644 include/crypto/morus640_glue.h
>  delete mode 100644 include/crypto/morus_common.h

Maybe include a link to the cryptanalysis paper
https://eprint.iacr.org/2019/172.pdf in the commit message, so people seeing
this commit can better understand the reasoning?

Otherwise this patch itself looks fine to me, though I'm a little concerned
we'll break someone actually using MORUS.  An alternate approach would be to
leave just the C implementation, and make it print a deprecation warning for a
year or two before actually removing it.  But I'm not sure that's needed, and it
might be counterproductive as it would allow more people to start using it.

From a Google search I don't see any documentation floating around specifically
telling people to use MORUS with cryptsetup, other than an email on the dm-crypt
mailing list (https://www.spinics.net/lists/dm-crypt/msg07763.html) which
mentioned it alongside other options.  So hopefully there are at most a couple
odd adventurous users, who won't mind migrating their data to a new LUKS volume.

- Eric