LKML Archive on lore.kernel.org
 help / color / Atom feed
From: Eric Biggers <ebiggers@kernel.org>
To: "Jason A. Donenfeld" <Jason@zx2c4.com>
Cc: Herbert Xu <herbert@gondor.apana.org.au>,
	Linux Crypto Mailing List <linux-crypto@vger.kernel.org>,
	LKML <linux-kernel@vger.kernel.org>,
	Netdev <netdev@vger.kernel.org>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	David Miller <davem@davemloft.net>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Ard Biesheuvel <ard.biesheuvel@linaro.org>,
	Samuel Neves <samuel.c.p.neves@gmail.com>
Subject: Re: [PATCH net-next v9 00/19] WireGuard: Secure Network Tunnel
Date: Sun, 31 Mar 2019 11:42:45 -0700
Message-ID: <20190331184244.GA723@sol.localdomain> (raw)
In-Reply-To: <CAHmME9o+HqyrXX403bSXJX4Xs0dsZ9Qo=owiu7QGzBbaLQWZag@mail.gmail.com>

On Sun, Mar 31, 2019 at 08:18:13PM +0200, Jason A. Donenfeld wrote:
> On Sat, Mar 30, 2019 at 6:53 AM Eric Biggers <ebiggers@kernel.org> wrote:
> > poly1305-simd is among the failing algorithms because it loses carry bits when
> > handling long "all 0xff bytes" inputs.  poly1305-avx2-x86_64.S is definitely
> > broken, and poly1305-sse2-x86_64.S *might* be too.  I am working on a patch...
> 
> Yea.... yikes. I'm kind of souring on this plan of having to deal with
> that code in Zinc, versus the extensively studied, fuzzed, and
> scrutinized code from Andy. Subtle carry bugs like that are kind of a
> testament to my overall plan of preferring formally verified or
> heavily used implementations to bespoke ones. This stuff is hard to
> get right.
> 
> Jason

I agree that Andy's Poly1305 code is better and we should probably switch to it,
but to be fair it's also longer and more complex, and I think you overestimate
the extent to which it's actually been "studied, fuzzed, and scrutinized".  Andy
previously made essentially the same mistake in three of his Poly1305
implementations as well, which he had to fix:
https://mta.openssl.org/pipermail/openssl-commits/2016-April/006639.html

Also OpenSSL's PowerPC implementation of AES-CTR that was incorporated into the
kernel was incorrect, as was recently fixed by

	commit dcf7b48212c0fab7df69e84fab22d6cb7c8c0fb9
	Author: Daniel Axtens <dja@axtens.net>
	Date:   Fri Mar 15 13:09:01 2019 +1100

	    crypto: vmx - fix copy-paste error in CTR mode
	    
	    The original assembly imported from OpenSSL has two copy-paste
	    errors in handling CTR mode. When dealing with a 2 or 3 block tail,
	    the code branches to the CBC decryption exit path, rather than to
	    the CTR exit path.
	
and by https://github.com/openssl/openssl/pull/8510.  This took almost 5 years.

I also still think the extent that you keep emphasizing the phrase "formally
verified" when discussing Zinc is somewhat misleading, because the only
implementation that is actually formally verified is Curve25519 in C.  No other
algorithms or implementations are formally verified.  E.g. none of the Poly1305
implementations under discussion are formally verified.  That needs to be made
very clear, and as one consequence of that we really need good tests.

- Eric

  reply index

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-03-22  7:11 Jason A. Donenfeld
2019-03-22  7:11 ` [PATCH net-next v9 01/19] asm: simd context helper API Jason A. Donenfeld
2019-03-22 11:21   ` Thomas Gleixner
2019-03-22  7:11 ` [PATCH net-next v9 02/19] zinc: introduce minimal cryptography library Jason A. Donenfeld
2019-03-22  7:11 ` [PATCH net-next v9 03/19] zinc: ChaCha20 generic C implementation and selftest Jason A. Donenfeld
2019-03-22  7:11 ` [PATCH net-next v9 04/19] zinc: ChaCha20 x86_64 implementation Jason A. Donenfeld
2019-03-22  7:11 ` [PATCH net-next v9 05/19] zinc: ChaCha20 ARM and ARM64 implementations Jason A. Donenfeld
2019-03-22 23:17   ` Stefan Agner
2019-03-22  7:11 ` [PATCH net-next v9 06/19] zinc: ChaCha20 MIPS32r2 implementation Jason A. Donenfeld
2019-03-22  7:11 ` [PATCH net-next v9 07/19] zinc: Poly1305 generic C implementations and selftest Jason A. Donenfeld
2019-03-22  7:11 ` [PATCH net-next v9 08/19] zinc: Poly1305 x86_64 implementation Jason A. Donenfeld
2019-03-22  7:11 ` [PATCH net-next v9 09/19] zinc: Poly1305 ARM and ARM64 implementations Jason A. Donenfeld
2019-03-22  7:11 ` [PATCH net-next v9 10/19] zinc: Poly1305 MIPS64 and MIPS32r2 implementations Jason A. Donenfeld
2019-03-22  7:11 ` [PATCH net-next v9 11/19] zinc: ChaCha20Poly1305 construction and selftest Jason A. Donenfeld
2019-03-22  7:11 ` [PATCH net-next v9 12/19] zinc: BLAKE2s generic C implementation " Jason A. Donenfeld
2019-03-26 17:38   ` Eric Biggers
2019-03-22  7:11 ` [PATCH net-next v9 13/19] zinc: BLAKE2s x86_64 implementation Jason A. Donenfeld
2019-03-22  7:11 ` [PATCH net-next v9 14/19] zinc: Curve25519 generic C implementations and selftest Jason A. Donenfeld
2019-03-22  7:11 ` [PATCH net-next v9 15/19] zinc: Curve25519 x86_64 implementation Jason A. Donenfeld
2019-03-22  7:11 ` [PATCH net-next v9 16/19] zinc: import Bernstein and Schwabe's Curve25519 ARM implementation Jason A. Donenfeld
2019-03-22  7:11 ` [PATCH net-next v9 17/19] zinc: " Jason A. Donenfeld
2019-03-22  7:11 ` [PATCH net-next v9 18/19] security/keys: rewrite big_key crypto to use Zinc Jason A. Donenfeld
2019-03-22  7:11 ` [PATCH net-next v9 19/19] net: WireGuard secure network tunnel Jason A. Donenfeld
2019-03-25  0:02   ` David Miller
2019-03-25 10:13     ` Jason A. Donenfeld
2019-03-25 11:51 ` [PATCH net-next v9 00/19] WireGuard: Secure Network Tunnel Herbert Xu
2019-03-25 11:57   ` Jason A. Donenfeld
2019-03-25 12:03     ` Herbert Xu
2019-03-30  5:53     ` Eric Biggers
2019-03-31 18:18       ` Jason A. Donenfeld
2019-03-31 18:42         ` Eric Biggers [this message]
2019-03-25 16:04   ` David Miller

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190331184244.GA723@sol.localdomain \
    --to=ebiggers@kernel.org \
    --cc=Jason@zx2c4.com \
    --cc=ard.biesheuvel@linaro.org \
    --cc=davem@davemloft.net \
    --cc=gregkh@linuxfoundation.org \
    --cc=herbert@gondor.apana.org.au \
    --cc=linux-crypto@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=samuel.c.p.neves@gmail.com \
    --cc=torvalds@linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

LKML Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/lkml/0 lkml/git/0.git
	git clone --mirror https://lore.kernel.org/lkml/1 lkml/git/1.git
	git clone --mirror https://lore.kernel.org/lkml/2 lkml/git/2.git
	git clone --mirror https://lore.kernel.org/lkml/3 lkml/git/3.git
	git clone --mirror https://lore.kernel.org/lkml/4 lkml/git/4.git
	git clone --mirror https://lore.kernel.org/lkml/5 lkml/git/5.git
	git clone --mirror https://lore.kernel.org/lkml/6 lkml/git/6.git
	git clone --mirror https://lore.kernel.org/lkml/7 lkml/git/7.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 lkml lkml/ https://lore.kernel.org/lkml \
		linux-kernel@vger.kernel.org linux-kernel@archiver.kernel.org
	public-inbox-index lkml


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


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