Re: [kernel-hardening] Re: Remaining crypto API regressions with CONFIG_VMAP_STACK
cc: Viro because I'm talking about iov_iter. On Sat, Dec 10, 2016 at 6:45 AM, Jason A. Donenfeld wrote: > Hi Herbert, > > On Sat, Dec 10, 2016 at 6:37 AM, Herbert Xu > wrote: >> As for AEAD we never had a sync interface to begin with and I >> don't think I'm going to add one. > > That's too bad to hear. I hope you'll reconsider. Modern cryptographic > design is heading more and more in the direction of using AEADs for > interesting things, and having a sync interface would be a lot easier > for implementing these protocols. In the same way many protocols need > a hash of some data, now protocols often want some particular data > encrypted with an AEAD using a particular key and nonce and AD. One > protocol that comes to mind is Noise [1]. > I think that sync vs async has gotten conflated with vectored-vs-nonvectored and the results are unfortunate. There are a lot of users in the tree that are trying to do crypto on very small pieces of data and want to have that data consist of the concatenation of two small buffers and/or want to use primitives that don't have "sync" interfaces. These users are stuck using async interfaces even though using async implementations makes no sense for them. I'd love to see the API restructured a bit to decouple all of these considerations. One approach might be to teach iov_iter about scatterlists. Then, for each primitive, there could be two entry points: 1. A simplified and lower-overhead entry. You pass it an iov_iter (and, depending on what the operation is, an output iov_iter), it does the crypto synchronously, and returns. Operating in-place might be permitted for some primitives. 2. A full-featured async entry. You pass it iov_iters and it requires that the iov_iters be backed by scatterlists in order to operate asynchronously. I see no reason that the decisions to use virtual vs physical addressing or to do vectored vs non-vectored IO should be tied up with asynchronicity. --Andy -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [kernel-hardening] Re: Remaining crypto API regressions with CONFIG_VMAP_STACK
Hi Herbert, On Sat, Dec 10, 2016 at 6:37 AM, Herbert Xu wrote: > As for AEAD we never had a sync interface to begin with and I > don't think I'm going to add one. That's too bad to hear. I hope you'll reconsider. Modern cryptographic design is heading more and more in the direction of using AEADs for interesting things, and having a sync interface would be a lot easier for implementing these protocols. In the same way many protocols need a hash of some data, now protocols often want some particular data encrypted with an AEAD using a particular key and nonce and AD. One protocol that comes to mind is Noise [1]. I know that in my own [currently external to the tree] kernel code, I just forego the use of the crypto API all together, and one of the primary reasons for that is lack of a sync interface for AEADs. When I eventually send this upstream, presumably everyone will want me to use the crypto API, and having a sync AEAD interface would be personally helpful for that. I guess I could always write the sync interface myself, but I imagine you'd prefer having the design control etc. Jason [1] http://noiseprotocol.org/ -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [kernel-hardening] Re: Remaining crypto API regressions with CONFIG_VMAP_STACK
Why did you drop me from the CC list when you were replying to my email? Eric Biggers wrote: > On Sat, Dec 10, 2016 at 01:32:08PM +0800, Herbert Xu wrote: > >> Are you sure? Any instance of *_ON_STACK must only be used with >> sync algorithms and most drivers under drivers/crypto declare >> themselves as async. > > Why exactly is that? Obviously, it wouldn't work if you returned from the > stack > frame before the request completed, but does anything stop someone from using > an > *_ON_STACK() request and then waiting for the request to complete before > returning from the stack frame? The *_ON_STACK variants (except SHASH of course) were simply hacks to help legacy crypto API users to cope with the new async interface. In general we should avoid using the sync interface when possible. It's a bad idea for the obvious reason that most of our async algorithms want to DMA and that doesn't work very well when you're using memory from the stack. Cheers, -- Email: Herbert Xu Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [kernel-hardening] Re: Remaining crypto API regressions with CONFIG_VMAP_STACK
On Sat, Dec 10, 2016 at 01:37:12PM +0800, Herbert Xu wrote: > On Fri, Dec 09, 2016 at 09:25:38PM -0800, Andy Lutomirski wrote: > > > > Herbert, how hard would it be to teach the crypto code to use a more > > sensible data structure than scatterlist and to use coccinelle fix > > this stuff for real? > > First of all we already have a sync non-SG hash interface, it's > called shash. > > If we had enough sync-only users of skcipher then I'll consider > adding an interface for it. However, at this point in time it > appears to more sense to convert such users over to the async > interface rather than the other way around. > > As for AEAD we never had a sync interface to begin with and I > don't think I'm going to add one. > Isn't the question of "should the API use physical or virtual addresses" independent of the question of "should the API support asynchronous requests"? You can already choose, via the flags and mask arguments when allocating a crypto transform, whether you want it to be synchronous or asynchronous or whether you don't care. I don't see what that says about whether the API should take in physical memory (e.g. scatterlists or struct pages) or virtual memory (e.g. iov_iters or just regular pointers). And while it's true that asynchronous algorithms are often provided by hardware drivers that operate on physical memory, it's not always the case. For example some of the AES-NI algorithms are asynchronous only because they use the SSE registers which can't always available to kernel code, so the request may need to be processed by another thread. Eric -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [kernel-hardening] Re: Remaining crypto API regressions with CONFIG_VMAP_STACK
On Sat, Dec 10, 2016 at 01:32:08PM +0800, Herbert Xu wrote: > On Fri, Dec 09, 2016 at 09:25:38PM -0800, Andy Lutomirski wrote: > > > > > The following crypto drivers initialize a scatterlist to point into an > > > ablkcipher_request, which may have been allocated on the stack with > > > SKCIPHER_REQUEST_ON_STACK(): > > > > > > drivers/crypto/ccp/ccp-crypto-aes-xts.c:162 > > > drivers/crypto/ccp/ccp-crypto-aes.c:94 > > > > These are real, and I wish I'd known about them sooner. > > Are you sure? Any instance of *_ON_STACK must only be used with > sync algorithms and most drivers under drivers/crypto declare > themselves as async. > Why exactly is that? Obviously, it wouldn't work if you returned from the stack frame before the request completed, but does anything stop someone from using an *_ON_STACK() request and then waiting for the request to complete before returning from the stack frame? Eric -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html