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;
MIME-Version: 1.0
References: <20190625145254.28510-1-ard.biesheuvel@linaro.org> <20190625171234.GB81914@gmail.com>
In-Reply-To: <20190625171234.GB81914@gmail.com>
From:   Ard Biesheuvel <ard.biesheuvel@linaro.org>
Date:   Tue, 25 Jun 2019 20:37:23 +0200
Message-ID: <CAKv+Gu8P4AUNbf636d=h=RDFV+CPEZCoPi9EZ+OtKEd5cBky5g@mail.gmail.com>
Subject: Re: [PATCH] crypto: morus - remove generic and x86 implementations
To:     Eric Biggers <ebiggers@kernel.org>
Cc:     "open list:HARDWARE RANDOM NUMBER GENERATOR CORE" 
        <linux-crypto@vger.kernel.org>,
        Herbert Xu <herbert@gondor.apana.org.au>,
        Ondrej Mosnacek <omosnace@redhat.com>,
        Geert Uytterhoeven <geert@linux-m68k.org>,
        Milan Broz <gmazyland@gmail.com>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-crypto-owner@vger.kernel.org
Precedence: bulk

On Tue, 25 Jun 2019 at 19:12, Eric Biggers <ebiggers@kernel.org> wrote:
>
> [+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?
>

Sure.

> 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.
>

Indeed. 'Breaking userspace' is permitted if nobody actually notices,
and given how broken MORUS is, anyone who truly cares about security
wouldn't have chosen it to begin with. And if it does turn out to be a
real issue, we can always put the C version back where it was.

> 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.
>