| Message ID | 11608519.pS4L9VjM2n@tachyon.chronox.de (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Herbert Xu |
| Headers | show |
On Sun, Nov 16, 2014 at 03:23:50AM +0100, Stephan Mueller wrote: > AEAD requires the following data in addition to normal symmetric > ciphers: > > * Associated authentication data of arbitrary length > > * Authentication tag for decryption > > * Length of authentication tag for encryption > > The authentication tag data is communicated as part of the actual > ciphertext as mandated by the kernel crypto API. Therefore we only need > to provide a user space interface for the associated authentication data > as well as for the authentication tag length. > > This patch adds both as a setsockopt interface that is identical to the > AF_ALG interface for setting an IV and for selecting the cipher > operation type (encrypt or decrypt). > > Signed-off-by: Stephan Mueller <smueller@chronox.de> I don't like the fact that we're putting arbitrary limits on the AD, as well as the fact that the way you're doing it the AD has to be copied. How about simply saying that the first X bytes of the input shall be the AD? Cheers,
Am Dienstag, 18. November 2014, 22:06:31 schrieb Herbert Xu: Hi Herbert, > On Sun, Nov 16, 2014 at 03:23:50AM +0100, Stephan Mueller wrote: > > AEAD requires the following data in addition to normal symmetric > > > > ciphers: > > * Associated authentication data of arbitrary length > > > > * Authentication tag for decryption > > > > * Length of authentication tag for encryption > > > > The authentication tag data is communicated as part of the actual > > ciphertext as mandated by the kernel crypto API. Therefore we only need > > to provide a user space interface for the associated authentication data > > as well as for the authentication tag length. > > > > This patch adds both as a setsockopt interface that is identical to the > > AF_ALG interface for setting an IV and for selecting the cipher > > operation type (encrypt or decrypt). > > > > Signed-off-by: Stephan Mueller <smueller@chronox.de> > > I don't like the fact that we're putting arbitrary limits on > the AD, as well as the fact that the way you're doing it the > AD has to be copied. > > How about simply saying that the first X bytes of the input > shall be the AD? That is a very good idea. To cover that approach, the kernel needs to be informed about the length of the authentication data size to separate the ciphertext/plaintext from the authentication data. To cover that, I would recommend to simply send a u32 value to the kernel for the AD size instead of the AD. The kernel then can adjust the pointers as necessary. I will update the patch in the next days to cover that scenario. Thanks
Am Dienstag, 18. November 2014, 22:06:31 schrieb Herbert Xu: Hi Herbert, > On Sun, Nov 16, 2014 at 03:23:50AM +0100, Stephan Mueller wrote: > > AEAD requires the following data in addition to normal symmetric > > > > ciphers: > > * Associated authentication data of arbitrary length > > > > * Authentication tag for decryption > > > > * Length of authentication tag for encryption > > > > The authentication tag data is communicated as part of the actual > > ciphertext as mandated by the kernel crypto API. Therefore we only need > > to provide a user space interface for the associated authentication data > > as well as for the authentication tag length. > > > > This patch adds both as a setsockopt interface that is identical to the > > AF_ALG interface for setting an IV and for selecting the cipher > > operation type (encrypt or decrypt). > > > > Signed-off-by: Stephan Mueller <smueller@chronox.de> > > I don't like the fact that we're putting arbitrary limits on > the AD, as well as the fact that the way you're doing it the > AD has to be copied. > > How about simply saying that the first X bytes of the input > shall be the AD? When looking deeper into skcipher_sendmsg, I see that the input data is copied into the kernel using memcpy_fromiovec. The memory is allocated before the memcpy call by skcipher_alloc_sgl. The AD is input data and therefore needs to be copied into the kernel just like plaintext. Of course it is possible to require user space to concatenate the AD with the plaintext (or ciphertext in case of decryption). However, I see the following drawbacks when we do that: - either user space has to do a very good memory allocation or it has to copy the data into the buffer before the plaintext. Also, if the plaintext buffer is not allocated in the right way, even the plaintext has to be copied to the newly created buffer that concatenates the AD with the plaintext. So, unless user space is very careful, at least some memcpy must be done in user space to accommodate the requirement that AD and plaintext is concatenated. - The kernel function of skcipher_sendmsg would now become very complicated, because a similar logic currently applied to plaintext needs to be also implemented to copy and track the initial AD into the kernel. However I see your point as the sendmsg approach to handle AD currently implements two memcpy() calls: one is the copy_from_user of the sendmsg system call framework to copy in msg and then the memcpy in skcipher_sendmsg. To avoid the cluttering of the skcipher_sendmsg function (which already is complex), may I propose that a setsockopt call is used just like the SET_IV call? When using the setsockopt call, I see the following advantages: - only one memcpy (the copy_from_user) is needed to move the data into kernel land -- this would be then en-par with the skcipher_sendmsg approach. - the implementation of the setsockopt approach would be very clean and small as just one copy_from_user call is to be made followed by a aead_request_set_assoc call. - user space memory handling is very clean as user space does not need a very specific memory setup to deliver AD. So, if the memory layout is not as specific as needed for the AEAD call, with the setsockopt approach, we do not need additional memcpy calls in user space. In any case, we need to set some upper boundary for the maximum size of the AD. As I do not know of any standardized upper limit for the AD size, I would consider the standard CAVP testing requirements. These tests have the maximum AD size of 1<<16. When using the setsockopt call approach we can allocate the in-kernel AD memory at the setsockopt call time. IMHO it would be save now to limit the maximum AD size to 1<<16 at this point. > > Cheers,
On Wed, Nov 19, 2014 at 05:20:42AM +0100, Stephan Mueller wrote: > > When looking deeper into skcipher_sendmsg, I see that the input data is copied > into the kernel using memcpy_fromiovec. The memory is allocated before the > memcpy call by skcipher_alloc_sgl. Zero-copy is done through sendpage. Cheers,
Am Mittwoch, 19. November 2014, 12:27:04 schrieb Herbert Xu: Hi Herbert, > On Wed, Nov 19, 2014 at 05:20:42AM +0100, Stephan Mueller wrote: > > When looking deeper into skcipher_sendmsg, I see that the input data is > > copied into the kernel using memcpy_fromiovec. The memory is allocated > > before the memcpy call by skcipher_alloc_sgl. > > Zero-copy is done through sendpage. I am slightly at a loss here -- if you could give me a hint on how you think it can be implemented, I would be grateful. Let us assume the AD || plaintext buffer is known to the kernel, either through sendpage or sendmsg. The entire buffer is split into chunks of scatterlists with ctx->tsgl. After processing one scatterlist from ctx->tsgl, that scatterlist is released via skcipher_pull_sgl. Now, for AD, we need to consider: - AD can span multiple ctx->tsgl chunks - these AD scatterlist chunks cannot be released after a normal encryption operation. The associated data must be available for multiple operations. So, while plaintext data is still flowing in, we need to keep operating with the same AD. Thus I am wondering how the rather static nature of the AD can fit with the dynamic nature of the plaintext given the current implementation on how plaintext is handled in the kernel. To me, AD in league with an IV considering its rather static nature. Having said that, the IV is also not transported via the plaintext interface, but via a setsockopt. Shouldn't the AD be handled the same way? > > Cheers,
On Wed, Nov 19, 2014 at 07:30:52AM +0100, Stephan Mueller wrote: > > - these AD scatterlist chunks cannot be released after a normal encryption > operation. The associated data must be available for multiple operations. So, > while plaintext data is still flowing in, we need to keep operating with the > same AD. We don't start an AEAD operation until the entire input has been received. Unlike ciphers you cannot process AEAD requests as you go. So there is no need to special-case AD chunks since you will have everything at your disposal before you can feed the request to the crypto API. > Thus I am wondering how the rather static nature of the AD can fit with the > dynamic nature of the plaintext given the current implementation on how > plaintext is handled in the kernel. > > To me, AD in league with an IV considering its rather static nature. Having > said that, the IV is also not transported via the plaintext interface, but via > a setsockopt. Shouldn't the AD be handled the same way? AD is not like an IV at all. An IV is a fixed-size (and small) input while AD can be of any length. Think about how this is used in real life. For IPsec AD is the part of the packet that we don't encrypt. So there is nothing fundamentally different between AD and the plain-text that we do encrypt except that you don't encrypt it :) Cheers,
diff --git a/crypto/af_alg.c b/crypto/af_alg.c index 6a3ad80..635140b 100644 --- a/crypto/af_alg.c +++ b/crypto/af_alg.c @@ -421,6 +421,23 @@ int af_alg_cmsg_send(struct msghdr *msg, struct af_alg_control *con) con->op = *(u32 *)CMSG_DATA(cmsg); break; + + case ALG_SET_AEAD_AUTHSIZE: + if (cmsg->cmsg_len < CMSG_LEN(sizeof(u32))) + return -EINVAL; + con->aead_authsize = *(u32 *)CMSG_DATA(cmsg); + break; + + case ALG_SET_AEAD_ASSOC: + if (cmsg->cmsg_len < CMSG_LEN(sizeof(*con->aead_assoc))) + return -EINVAL; + con->aead_assoc = (void *)CMSG_DATA(cmsg); + if (cmsg->cmsg_len < + CMSG_LEN(con->aead_assoc->aead_assoclen + + sizeof(*con->aead_assoc))) + return -EINVAL; + break; + default: return -EINVAL; } diff --git a/include/crypto/if_alg.h b/include/crypto/if_alg.h index d61c111..c741483 100644 --- a/include/crypto/if_alg.h +++ b/include/crypto/if_alg.h @@ -41,7 +41,9 @@ struct af_alg_completion { struct af_alg_control { struct af_alg_iv *iv; + struct af_alg_aead_assoc *aead_assoc; int op; + unsigned int aead_authsize; }; struct af_alg_type { diff --git a/include/uapi/linux/if_alg.h b/include/uapi/linux/if_alg.h index 0f9acce..64e7008 100644 --- a/include/uapi/linux/if_alg.h +++ b/include/uapi/linux/if_alg.h @@ -28,10 +28,17 @@ struct af_alg_iv { __u8 iv[0]; }; +struct af_alg_aead_assoc { + __u32 aead_assoclen; + __u8 aead_assoc[0]; +}; + /* Socket options */ #define ALG_SET_KEY 1 #define ALG_SET_IV 2 #define ALG_SET_OP 3 +#define ALG_SET_AEAD_ASSOC 4 +#define ALG_SET_AEAD_AUTHSIZE 5 /* Operations */ #define ALG_OP_DECRYPT 0
AEAD requires the following data in addition to normal symmetric ciphers: * Associated authentication data of arbitrary length * Authentication tag for decryption * Length of authentication tag for encryption The authentication tag data is communicated as part of the actual ciphertext as mandated by the kernel crypto API. Therefore we only need to provide a user space interface for the associated authentication data as well as for the authentication tag length. This patch adds both as a setsockopt interface that is identical to the AF_ALG interface for setting an IV and for selecting the cipher operation type (encrypt or decrypt). Signed-off-by: Stephan Mueller <smueller@chronox.de> --- crypto/af_alg.c | 17 +++++++++++++++++ include/crypto/if_alg.h | 2 ++ include/uapi/linux/if_alg.h | 7 +++++++ 3 files changed, 26 insertions(+)