On Mon, Aug 09, 2010 at 08:00:55PM -0400, Miloslav Trmac wrote: > ----- "Herbert Xu" <herbert@xxxxxxxxxxxxxxxxxxxx> wrote: > > > On Thu, Aug 05, 2010 at 10:17:53PM +0200, Miloslav Trmač wrote: > > > Hello, > > > following is a patchset providing an user-space interface to the kernel crypto > > > API. It is based on the older, BSD-compatible, implementation, but the > > > user-space interface is different. > > > > Thanks for the patches. > > > > Unfortunately it fails to satisfy the requirement of supporting > > all our existing kernel crypto interfaces, such as AEAD, as well > > as being flexible enough in adding new interfaces such as xor. > Thanks for the review. > > I think I can add AEAD (and compression/RNG, if requested) easily enough, I'll send an updated patch set. > > (Talking specifically about xor, xor does not seem to be cryptographic operation that should be exposed as such to userspace - and AFAICS it is not integrated in the crypto API algorithm mechanism in the kernel either.) > > Is the proposed interface acceptable in the general approach (enums for algorithms/operations, unions for parameters, session init/update/finalize)? With respect to flexibility, do you have specific suggestions or areas of concern? I know we spoke about this previously, but since you're asking publically, I'll make my argument here so that we can have the debate in public as well. I'm ok wih the general enumeration of operations in user space (I honestly can't see how you would do it otherwise). What worries me though is the use ioctl for these operations and the flexibility that it denies you in future updates. Specifically, my concerns are twofold: 1) struct format. By passing down a structure as your doing through an ioctl call, theres no way to extend/modify that structure easily for future use. For instance the integration of aead might I think requires a blocking factor to be sepcified, and entry for which you have no available field in your crypt_ops structure. If you extend the structure in a later release of this code, you create a potential incompatibility with user space because you are no longer guaranteed that the size of the crypt_op structure is the same, and you need to be prepared for a range of sizes to get passed down, at which point it becomes difficult to differentiate between older code thats properly formatted, and newer code thats legitimately broken. You might could add a version field to mitigate that, but it gets ugly pretty quickly. 2) Use of pointers. Thats just a pain. You have the compat ioctl stuff in place to handle the 32/64 bit conversions, but it would be really nice if you could avoid having to maintain that extra code path. Thats why I had suggested the use of a netlink protocol to manage this kind of interface. There are other issue to manage there, but they're really not that big a deal, comparatively speaking, and that interface allows for the easy specification of arbitrary length data in a fashion that doesn't cause user space breakage when additional algorithms/apis are added. Thats just my opinion, I'm sure others have differing views. I'd be interested to hear what others think. Regards Neil > -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html