linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
From: "Jason A. Donenfeld" <Jason@zx2c4.com>
To: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Peter Zijlstra <peterz@infradead.org>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Will Deacon <will.deacon@arm.com>,
	Samuel Neves <sneves@dei.uc.pt>,
	Paul Mackerras <paulus@samba.org>,
	Herbert Xu <herbert@gondor.apana.org.au>,
	Richard Weinberger <richard@nod.at>,
	Eric Biggers <ebiggers@kernel.org>,
	Ingo Molnar <mingo@redhat.com>, Kees Cook <keescook@chromium.org>,
	Arnd Bergmann <arnd@arndb.de>, Andy Lutomirski <luto@kernel.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	linux-arm-kernel@lists.infradead.org,
	"Martin K. Petersen" <martin.petersen@oracle.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	linux-kernel@vger.kernel.org, linux-crypto@vger.kernel.org,
	Andrew Morton <akpm@linux-foundation.org>,
	linuxppc-dev@lists.ozlabs.org,
	"David S. Miller" <davem@davemloft.net>
Subject: Re: [RFC PATCH 0/9] patchable function pointers for pluggable crypto routines
Date: Fri, 5 Oct 2018 15:37:06 +0200	[thread overview]
Message-ID: <20181005133705.GA4588@zx2c4.com> (raw)
In-Reply-To: <20181005081333.15018-1-ard.biesheuvel@linaro.org>

Hi Ard,

On Fri, Oct 5, 2018 at 10:13 AM Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> At the moment, the Zinc library [1] is being proposed as a solution for that,
> and while it does address the usability problems, it does a lot more than
> that, and what we end up with is a lot less flexible than what we have now.
>
> Currently, arch specific implementations (based on SIMD or optimized
> assembler) live in arch/*/crypto, where each sub-community is in charge
> of its own specialized versions of the various primitives (although still
> under the purview of the crypto maintainer). Any proposal to change this
> model should be judged on its own merit, and not blindly accepted as the
> fallout of cleaning up some library code.
>
> Also, Zinc removes the possibility to plug different versions of a routine,
> and instead, keeps all versions in the same module. Currently, the kernel's
> module support permits user land to take charge of the policies that decide
> which version to use in which context (by loading or blacklisting modules).

I think this explanation misunderstands many of the design goals of Zinc and
also points to why going your direction instead is a bad idea that will cause
even more problems down the road.

Zinc does several important things:

- Introduces direct C function calls throughout, as a central way of being
  implemented and as a central way of being used by consumers of the
  API. This has various obvious benefits for the consumers of the API,
  but it also has big benefits for the developers of the library as
  well. Namely, it keeps the relationship between different parts
  extremely clear and direct. It's an explicit choice for simplicity.
  And by being the simpler and more direct solution, it also gives gcc
  an important opportunity to optimize and inline.

- Reorganizes crypto routines so that they're grouped together by
  primitive. This again leads to a much simpler design and layout,
  making it more obvious what's happening, and making things generally
  cleaner. It is not only useful and clearer for developers, but it
  also makes contributors and auditors more easily aware of what
  implementations are available.

- Has higher standards for code and implementations that are introduced.
  Zinc prefers code that has been formally verified, that has been in
  widespread usage and has received many eyeballs and fuzzing hours,
  that has been fuzzed extensively, that is simple in design, that comes
  from well-known high-quality authors -- in roughly but not precisely
  that order of preference. That's a bit different from the current
  practices of the existing crypto API.

- Has a simpler mechanism that is just as effective for choosing
  available implementations. This is, again, more obvious and direct
  than the present crypto API's module approach, leads to smaller code
  size, and has the potential of being just as flexible with the
  inevitable desire for nobs, adjustable from userspace, from
  kernelspace, or from elsewhere.

- Is designed to promote collaboration with the larger cryptography
  community and with academia, which will yield better implementations
  and for assurance.

- Can easily be extracted to userspace libraries (perhaps a future
  libzinc could be easily based on it), which makes testing and fuzzing
  using tools like libfuzzer and afl more accessible.

- Has faster implementations than the current crypto API.

- Has, again, a very strong focus on being simple and minimal, as
  opposed to bloated and complicated, so that it's actually possible to
  understand and audit the library.

Therefore, I think this patch goes in exactly the wrong direction. I
mean, if you want to introduce dynamic patching as a means for making
the crypto API's dynamic dispatch stuff not as slow in a post-spectre
world, sure, go for it; that may very well be a good idea. But
presenting it as an alternative to Zinc very widely misses the point and
serves to prolong a series of bad design choices, which are now able to
be rectified by putting energy into Zinc instead.

Jason

  parent reply	other threads:[~2018-10-05 23:44 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-10-05  8:13 [RFC PATCH 0/9] patchable function pointers for pluggable crypto routines Ard Biesheuvel
2018-10-05  8:13 ` [RFC PATCH 1/9] kernel: add support for patchable function pointers Ard Biesheuvel
2018-10-05 13:57   ` Peter Zijlstra
2018-10-05 14:03     ` Ard Biesheuvel
2018-10-05 14:14   ` Peter Zijlstra
2018-10-05 14:57     ` Ard Biesheuvel
2018-10-05 15:08     ` Andy Lutomirski
2018-10-05 15:24       ` Ard Biesheuvel
2018-10-05 16:58         ` Andy Lutomirski
2018-10-05 17:11           ` Ard Biesheuvel
2018-10-05 17:20             ` Andy Lutomirski
2018-10-05 17:23               ` Ard Biesheuvel
2018-10-05 17:28                 ` Andy Lutomirski
2018-10-05 17:37                   ` Jason A. Donenfeld
2018-10-05 17:44                     ` Andy Lutomirski
2018-10-05 18:28                       ` Jason A. Donenfeld
2018-10-05 19:13                         ` Ard Biesheuvel
2018-10-05  8:13 ` [RFC PATCH 2/9] arm64: kernel: add arch " Ard Biesheuvel
2018-10-05  8:13 ` [RFC PATCH 3/9] crypto: crc-t10dif - make crc_t10dif a static inline Ard Biesheuvel
2018-10-05  8:13 ` [RFC PATCH 4/9] crypto: crc-t10dif - use patchable function pointer for core update routine Ard Biesheuvel
2018-10-05  8:13 ` [RFC PATCH 5/9] crypto: crc-t10dif/arm64 - move PMULL based code into core library Ard Biesheuvel
2018-10-05  8:13 ` [RFC PATCH 6/9] crypto: crc-t10dif/arm " Ard Biesheuvel
2018-10-05  8:13 ` [RFC PATCH 7/9] crypto: crct10dif/generic - switch crypto API driver to " Ard Biesheuvel
2018-10-05  8:13 ` [RFC PATCH 8/9] crypto: crc-t10dif/powerpc - move PMULL based code into " Ard Biesheuvel
2018-10-05  8:13 ` [RFC PATCH 9/9] crypto: crc-t10dif/x86 " Ard Biesheuvel
2018-10-05 13:37 ` Jason A. Donenfeld [this message]
2018-10-05 17:15   ` [RFC PATCH 0/9] patchable function pointers for pluggable crypto routines Ard Biesheuvel
2018-10-05 17:26     ` Andy Lutomirski
2018-10-05 17:28       ` Ard Biesheuvel
2018-10-05 18:00         ` Andy Lutomirski

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=20181005133705.GA4588@zx2c4.com \
    --to=jason@zx2c4.com \
    --cc=akpm@linux-foundation.org \
    --cc=ard.biesheuvel@linaro.org \
    --cc=arnd@arndb.de \
    --cc=catalin.marinas@arm.com \
    --cc=davem@davemloft.net \
    --cc=ebiggers@kernel.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=herbert@gondor.apana.org.au \
    --cc=keescook@chromium.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-crypto@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=luto@kernel.org \
    --cc=martin.petersen@oracle.com \
    --cc=mingo@redhat.com \
    --cc=paulus@samba.org \
    --cc=peterz@infradead.org \
    --cc=richard@nod.at \
    --cc=sneves@dei.uc.pt \
    --cc=tglx@linutronix.de \
    --cc=will.deacon@arm.com \
    /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).