linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Jason A. Donenfeld" <Jason@zx2c4.com>
To: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: LKML <linux-kernel@vger.kernel.org>,
	Netdev <netdev@vger.kernel.org>,
	David Miller <davem@davemloft.net>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Andrew Lutomirski <luto@kernel.org>,
	Samuel Neves <sneves@dei.uc.pt>,
	Jean-Philippe Aumasson <jeanphilippe.aumasson@gmail.com>,
	Linux Crypto Mailing List <linux-crypto@vger.kernel.org>
Subject: Re: [PATCH net-next v3 02/17] zinc: introduce minimal cryptography library
Date: Thu, 13 Sep 2018 16:15:19 +0200	[thread overview]
Message-ID: <CAHmME9oYAEwFQAriVRm5zZKCo0Sh1=t5YheNZ+MtKQLQPoMWeg@mail.gmail.com> (raw)
In-Reply-To: <CAKv+Gu9RGo9rtGsmwMLZ_=-WSQ_h7har4agXP-2XOKupq-KYtA@mail.gmail.com>

Hi Ard,

On Thu, Sep 13, 2018 at 12:56 AM Ard Biesheuvel
<ard.biesheuvel@linaro.org> wrote:
> In this series, you are dumping a huge volume of unannotated,
> generated asm into the kernel which has been modified [by you] to
> [among other things?] adhere to the kernel API (without documenting
> what the changes are exactly). How does that live up to the promise of
> better, peer reviewed code?

The code still benefits from the review that's gone into OpenSSL. It's
not modified in ways that would affect the cryptographic operations
being done. It's modified to be suitable for kernel space.

> Then there is the performance claim. We know for instance that the
> OpenSSL ARM NEON code for ChaCha20 is faster on cores that happen to
> possess a micro-architectural property that ALU instructions are
> essentially free when they are interleaved with SIMD instructions. But
> we also know that a) Cortex-A7, which is a relevant target, is not one
> of those cores, and b) that chip designers are not likely to optimize
> for that particular usage pattern so relying on it in generic code is
> unwise in general.

That's interesting. I'll bring this up with AndyP. FWIW, if you think
you have a real and compelling claim here, I'd be much more likely to
accept a different ChaCha20 implementation than I would be to accept a
different Poly1305 implementation. (It's a *lot* harder to screw up
ChaCha20 than it is to screw up Poly1305.)

> I am also concerned about your claim that all software algorithms will
> be moved into this crypto library.

I'll defer to Andy's response here, which I think is a correct one:
https://lkml.org/lkml/2018/9/13/27

The short answer is that Zinc is going to be adding the ciphers that
people want to use for normal reasons from normal code. For example,
after this merges, we'll next be working on moving the remaining
non-optimized C code out of lib/ that's called by places (such as
SHA2).

> You are not specific about whose
> responsibility it will be that this is going to happen in a timely
> fashion.

I thought I laid out the roadmap for this in the commit message. In
case I wasn't clear: my plan is to tackle lib/ after merging, and I
plan to do so in a timely manner. It's a pretty common tactic to keep
layering on tasks, "what about X?", "what about Y?", "I won't agree
unless Z!" -- when in reality kernel development and refactorings are
done incrementally. I've been around on this list contributing code
for long enough that you should have a decent amount of confidence
that I'm not just going to disappear working on this or something
insane like that. And neither are the two academic cryptographers CC'd
on this thread. So, as Andy said, we're going to be porting to Zinc
the primitives that are useful for the various applications of Zinc.
This means yes, we'll have SHA2 in there.

> chaining modes
> What are the APIs
> going to look like for block ciphers, taking chaining modes into
> account?

As mentioned in the commit message and numerous times, we're not
trying to make a win32-like crypto API here or to remake the existing
Linux crypto API. Rather we're providing libraries of specific
functions that are useful for various circumstances. For example, if
AES-GCM is desired at some point, then we'll have a similar API for
that as we do for ChaPoly -- one that takes buffers and one that takes
sg. Likewise, hash functions use the familiar init/update/final.
"Generic" chaining modes aren't really part of the equation or design
goals.

Again, I realize you've spent a long time working on the existing
crypto API, and so your questions and concerns are in the line of,
"how are we going to make Zinc look like the existing crypto API in
functionality?" But that's not what we're up to here. We have a
different and complementary design goal. I understand why you're
squirming, but please recognize we're working on different things.

> I'm sure it is rather simple to port the crypto API implementation of
> ChaCha20 to use your library. I am more concerned about how your
> library is going to expand to cover all other software algorithms that
> we currently use in the kernel.

The subset of algorithms we add will be developed with the same
methodology as the present ones. There is nothing making this
particularly difficult or even more difficult for other primitives
than it was for ChaCha20. It's especially easy, in fact, since we're
following similar design methodologies as the vast majority of other
cryptography libraries that have been developed. Namely, we're
creating simple things called "functions".

> Of course. But please respond to all the concerns,
> You have not
> responded to that concern yet.

Sorry, it's certainly not my intention. I've been on vacation with my
family for the last several weeks, and only returned home
sleep-deprived last night after 4 days of plane delays. I've now
rested and will resume working on this full-time and I'll try my best
to address concerns, and also go back through emails to find things I
might have missed. (First, though, I'm going to deal with getting back
the three suitcases the airline lost in transit...)

> > Anyway, it sounds like this whole thing may have ruffled your feathers
> > a bit. Will you be at Linux Plumbers Conference in November? I'm
> > planning on attending, and perhaps we could find some time there to
> > sit down and talk one on one a bit.
>
> That would be good, yes. I will be there.

Looking forward to talking to you there, and hopefully we can put to
rest any lingering concerns.

Jason

  parent reply	other threads:[~2018-09-13 14:15 UTC|newest]

Thread overview: 103+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-09-11  1:08 [PATCH net-next v3 00/17] WireGuard: Secure Network Tunnel Jason A. Donenfeld
2018-09-11  1:08 ` [PATCH net-next v3 01/17] asm: simd context helper API Jason A. Donenfeld
2018-09-12  6:14   ` Kevin Easton
2018-09-12 18:10     ` Jason A. Donenfeld
2018-09-13  5:03       ` Kevin Easton
2018-09-13 13:52         ` Jason A. Donenfeld
2018-09-13 13:53           ` Jason A. Donenfeld
2018-09-15 19:58           ` Andy Lutomirski
2018-09-15 20:01             ` Jason A. Donenfeld
2018-09-17 13:14         ` Jason A. Donenfeld
2018-09-11  1:08 ` [PATCH net-next v3 02/17] zinc: introduce minimal cryptography library Jason A. Donenfeld
2018-09-11 10:08   ` Ard Biesheuvel
2018-09-11 14:56     ` Greg Kroah-Hartman
2018-09-11 21:47       ` Eric Biggers
2018-09-11 22:02         ` Jason A. Donenfeld
2018-09-11 23:30           ` Andrew Lunn
2018-09-11 23:57             ` David Miller
2018-09-12  0:02               ` Jason A. Donenfeld
2018-09-17  4:09               ` Andy Lutomirski
2018-09-17  4:45                 ` David Miller
2018-09-17 14:55                   ` Andy Lutomirski
2018-09-17 14:59                     ` Jason A. Donenfeld
2018-09-17  5:07                 ` Jason A. Donenfeld
2018-09-17 14:53                   ` Andy Lutomirski
2018-09-17 14:59                     ` Jason A. Donenfeld
2018-09-17 16:24                     ` Jason A. Donenfeld
2018-09-18 16:06                     ` Ard Biesheuvel
2018-09-18 16:45                       ` Jason A. Donenfeld
2018-09-17  5:26                 ` Ard Biesheuvel
2018-09-17 14:51                   ` Andy Lutomirski
2018-09-17 15:28                     ` Jason A. Donenfeld
2018-09-17 16:06                       ` Andy Lutomirski
2018-09-17 16:17                         ` Jason A. Donenfeld
2018-09-17 15:31                     ` Jason A. Donenfeld
2018-09-17 16:07                       ` Andy Lutomirski
2018-09-17 16:16                         ` Jason A. Donenfeld
2018-09-17 16:18                           ` Andy Lutomirski
2018-09-18  0:56                             ` Jason A. Donenfeld
2018-09-17 15:52                   ` Jason A. Donenfeld
2018-09-18  4:21                     ` Herbert Xu
2018-09-18  4:26                       ` Jason A. Donenfeld
2018-09-18 18:53                     ` Ard Biesheuvel
2018-09-18 20:36                       ` Jason A. Donenfeld
2018-09-19 16:55                         ` Ard Biesheuvel
2018-09-11 22:16         ` Andy Lutomirski
2018-09-11 22:18           ` Jason A. Donenfeld
2018-09-11 23:01             ` Andy Lutomirski
2018-09-12  0:01               ` Jason A. Donenfeld
2018-09-12  4:29                 ` Jason A. Donenfeld
2018-09-11 21:22     ` Jason A. Donenfeld
2018-09-12 22:56       ` Ard Biesheuvel
2018-09-12 23:45         ` Andy Lutomirski
2018-09-13  5:41           ` Ard Biesheuvel
2018-09-13 14:32             ` Jason A. Donenfeld
2018-09-13 15:42               ` Ard Biesheuvel
2018-09-13 15:58                 ` Jason A. Donenfeld
2018-09-14  6:15                   ` Ard Biesheuvel
2018-09-14  9:53                     ` Jason A. Donenfeld
2018-09-13  6:39           ` Milan Broz
2018-09-13 14:34             ` Jason A. Donenfeld
2018-09-13 15:26             ` Andy Lutomirski
2018-09-13 14:18           ` Jason A. Donenfeld
2018-09-13 15:07             ` Ard Biesheuvel
2018-09-13 14:15         ` Jason A. Donenfeld [this message]
2018-09-13 15:04           ` Ard Biesheuvel
2018-09-13 15:45             ` Jason A. Donenfeld
2018-09-11 22:08     ` Eric Biggers
2018-09-12 18:16       ` Jason A. Donenfeld
2018-09-12 18:19         ` Ard Biesheuvel
2018-09-12 18:34           ` Eric Biggers
2018-09-14  6:21             ` Ard Biesheuvel
2018-09-11  1:08 ` [PATCH net-next v3 03/17] zinc: ChaCha20 generic C implementation Jason A. Donenfeld
2018-09-11  1:08 ` [PATCH net-next v3 04/17] zinc: ChaCha20 ARM and ARM64 implementations Jason A. Donenfeld
2018-09-11  1:08 ` [PATCH net-next v3 05/17] zinc: ChaCha20 x86_64 implementation Jason A. Donenfeld
2018-09-11  8:22   ` Thomas Gleixner
2018-09-11  9:00     ` Samuel Neves
2018-09-11  9:09       ` Thomas Gleixner
2018-09-11 21:12         ` Jason A. Donenfeld
2018-09-11 21:27           ` Thomas Gleixner
2018-09-11 21:28             ` Jason A. Donenfeld
2018-09-11 21:48           ` Eric Biggers
2018-09-11 22:04             ` Jason A. Donenfeld
2018-09-11  1:08 ` [PATCH net-next v3 06/17] zinc: ChaCha20 MIPS32r2 implementation Jason A. Donenfeld
2018-09-11  1:08 ` [PATCH net-next v3 07/17] zinc: Poly1305 generic C implementation and selftest Jason A. Donenfeld
2018-09-11  1:17   ` Jason A. Donenfeld
2018-09-11  1:08 ` [PATCH net-next v3 07/17] zinc: Poly1305 generic C implementations " Jason A. Donenfeld
2018-09-11  1:08 ` [PATCH net-next v3 08/17] zinc: Poly1305 ARM and ARM64 implementations Jason A. Donenfeld
2018-09-11  1:08 ` [PATCH net-next v3 09/17] zinc: Poly1305 x86_64 implementation Jason A. Donenfeld
2018-09-11  1:08 ` [PATCH net-next v3 10/17] zinc: Poly1305 MIPS32r2 and MIPS64 implementations Jason A. Donenfeld
2018-09-11  1:08 ` [PATCH net-next v3 11/17] zinc: ChaCha20Poly1305 construction and selftest Jason A. Donenfeld
2018-09-11  1:08 ` [PATCH net-next v3 12/17] zinc: BLAKE2s generic C implementation " Jason A. Donenfeld
2018-09-11  1:08 ` [PATCH net-next v3 13/17] zinc: BLAKE2s x86_64 implementation Jason A. Donenfeld
2018-09-11  1:08 ` [PATCH net-next v3 14/17] zinc: Curve25519 generic C implementations and selftest Jason A. Donenfeld
2018-09-11  1:08 ` [PATCH net-next v3 15/17] zinc: Curve25519 ARM implementation Jason A. Donenfeld
2018-09-11  1:08 ` [PATCH net-next v3 16/17] zinc: Curve25519 x86_64 implementation Jason A. Donenfeld
2018-09-11  1:08 ` [PATCH net-next v3 17/17] net: WireGuard secure network tunnel Jason A. Donenfeld
2018-09-11 12:59   ` kbuild test robot
2018-09-11 20:53     ` Jason A. Donenfeld
2018-09-11 13:17   ` kbuild test robot
2018-09-11 21:05     ` Jason A. Donenfeld
2018-09-11 13:30   ` Andrew Lunn
2018-09-11 21:08     ` Jason A. Donenfeld
2018-09-11 21:55       ` Andrew Lunn

Reply instructions:

You may reply publicly 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='CAHmME9oYAEwFQAriVRm5zZKCo0Sh1=t5YheNZ+MtKQLQPoMWeg@mail.gmail.com' \
    --to=jason@zx2c4.com \
    --cc=ard.biesheuvel@linaro.org \
    --cc=davem@davemloft.net \
    --cc=gregkh@linuxfoundation.org \
    --cc=jeanphilippe.aumasson@gmail.com \
    --cc=linux-crypto@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=luto@kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=sneves@dei.uc.pt \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).