On Tue, Sep 11, 2018 at 04:56:24PM +0200, Greg Kroah-Hartman wrote: > On Tue, Sep 11, 2018 at 12:08:56PM +0200, Ard Biesheuvel wrote: > > > As Zinc is simply library code, its config options are un-menued, with > > > the exception of CONFIG_ZINC_DEBUG, which enables various selftests and > > > BUG_ONs. > > > > > > > In spite of the wall of text, you fail to point out exactly why the > > existing AEAD API in unsuitable, and why fixing it is not an option. > > > > As I pointed out in a previous version, I don't think we need a > > separate crypto API/library in the kernel, and I don't think you have > > convinced anyone else yet either. > > Um, then why do people keep sprinkling new crypto/hash code all around > the kernel tree? It's because what we have as a crypto api is complex > and is hard to use for many in-kernel users. > > Something like this new interface (zinc) is a much "saner" api for > writing kernel code that has to interact with crypto/hash primitives. > > I see no reason why the existing crypto code can be redone to use the > zinc crypto primitives over time, making there only be one main location > for the crypto algorithms. But to do it the other way around is pretty > much impossible given the complexities in the existing api that has been > created over time. > > Not to say that the existing api is not a viable one, but ugh, really? > You have to admit it is a pain to try to use in any "normal" type of > "here's a bytestream, go give me a hash" type of method, right? > > Also there is the added benefit that the crypto primitives here have > been audited by a number of people (so Jason stated), and they are > written in a way that the crypto community can more easily interact and > contribute to. Which is _way_ better than what we have today. > > So this gets my "stamp of approval" for whatever it is worth :) > I think you mean you see no reason why it *cannot* be converted? The conversions definitely *should* be done, just like how some of the existing crypto API algorithms like SHA-256 already wrap implementations in lib/. In my view, lib/zinc/ isn't fundamentally different from what we already have for some algorithms. So it's misguided to design/present it as some novel thing, which unfortunately this patchset still does to a large extent. (The actual new thing is how architecture-specific implementations are handled.) Of course, the real problem is that even after multiple revisions of this patchset, there's still no actual conversions of the existing crypto API algorithms over to use the new implementations. "Zinc" is still completely separate from the existing crypto API. So, it's not yet clear that the conversions will actually work out without problems that would require changes in "Zinc". I don't think it makes sense to merge all this stuff without doing the conversions, or at the very least demonstrating how they will be done. In particular, in its current form "Zinc" is useless for anyone that needs the existing crypto API. For example, for HPolyC, (https://lkml.org/lkml/2018/8/6/857), I need to make improvements to ChaCha and Poly1305 in the existing crypto API, e.g. to add support for XChaCha and NEON-accelerated Poly1305. Having completely separate ChaCha and Poly1305 implementations in Zinc doesn't help at all. If anything, it makes things harder because people will have to review/maintain both sets of implementations; and when trying to make the improvements I need, I'll find myself in the middle of a holy war between two competing camps who each have their own opinion about The Right Way To Do Crypto, and their own crypto implementations and APIs in the kernel. - Eric
