Re: [Cryptodev-linux-devel] [PATCH] fix dst_len for TLS mode with aead ciphers
On Tue, Jul 1, 2014 at 10:48 AM, cristian.sto...@freescale.com wrote: > Hi Phil, >> This means we write more data into the userspace-supplied buffer than >> requested without noticing it. Although this might be correct in regards >> of the cipher mode's requirements, we could corrupt userspace memory by >> doing so. Correctly calculating the needed bufferspace for the requested >> operation is something I consider to be the caller's job, cryptodev >> should rather deny the operation instead of silently fixing it. Or what >> do you think? > There are several places where dst_len is calculated from src_len by adding > tag length and/or padding. > I'm not sure why this should work; my conclusion is that userland code is > expected to allocate enough space for the larger destination. The problem is > more evident in same-place destination where additional space must exist or > the encryption would fail. > Normally I would not decide what the destination length should be. Instead I > would tell cryptodev both src_len and dst_len and work with the values I get. > At least that frees cryptodev from making assumptions about existing buffers. I didn't add the dst_len as it was straightforward for the caller to calculate. In fact that API isn't very usable if the caller doesn't know the length in advance, so I guess as long as it is documented in cryptodev.h how to calculate that length in various ciphers (or have a macro), it is ok. Alternatively we can add an ioctl that has the length explicit, and drop the CIOCAUTHCRYPT. The latter would require changing the number of CRYPTO_AES_GCM, or any users of gnutls with that ioctl would break on an upgrade of cryptodev. regards, Nikos ___ Cryptodev-linux-devel mailing list Cryptodev-linux-devel@gna.org https://mail.gna.org/listinfo/cryptodev-linux-devel
Re: [Cryptodev-linux-devel] [PATCH] fix dst_len for TLS mode with aead ciphers
Hi Phil, > This means we write more data into the userspace-supplied buffer than > requested without noticing it. Although this might be correct in regards > of the cipher mode's requirements, we could corrupt userspace memory by > doing so. Correctly calculating the needed bufferspace for the requested > operation is something I consider to be the caller's job, cryptodev > should rather deny the operation instead of silently fixing it. Or what > do you think? There are several places where dst_len is calculated from src_len by adding tag length and/or padding. I'm not sure why this should work; my conclusion is that userland code is expected to allocate enough space for the larger destination. The problem is more evident in same-place destination where additional space must exist or the encryption would fail. Normally I would not decide what the destination length should be. Instead I would tell cryptodev both src_len and dst_len and work with the values I get. At least that frees cryptodev from making assumptions about existing buffers. If we agree to this point, I can send another patch and work from there. The only problem is that it breaks the API by introducing a dst_len field to crypt_auth_op. See https://mail.gna.org/public/cryptodev-linux-devel/2013-12/msg00022.html for the idea. >From my knowledge, only gnutls is public with using CIOCAUTHCRYPT and Nikos >can tell us what he thinks about adding dst_len. Cristian S. ___ Cryptodev-linux-devel mailing list Cryptodev-linux-devel@gna.org https://mail.gna.org/listinfo/cryptodev-linux-devel
Re: [Cryptodev-linux-devel] [PATCH] fix dst_len for TLS mode with aead ciphers
Hi, On Mon, Jun 23, 2014 at 06:57:25PM +0300, Cristian Stoica wrote: > - destination length is rounded to multiple of blockcipher length > before encryption This means we write more data into the userspace-supplied buffer than requested without noticing it. Although this might be correct in regards of the cipher mode's requirements, we could corrupt userspace memory by doing so. Correctly calculating the needed bufferspace for the requested operation is something I consider to be the caller's job, cryptodev should rather deny the operation instead of silently fixing it. Or what do you think? Best wishes, Phil ___ Cryptodev-linux-devel mailing list Cryptodev-linux-devel@gna.org https://mail.gna.org/listinfo/cryptodev-linux-devel
