intel / intel-ipsec-mb

Intel(R) Multi-Buffer Crypto for IPSec
BSD 3-Clause "New" or "Revised" License
289 stars 87 forks source link

Fix issue with parallel use of a gcm_ctx #8

Closed thbtcllt closed 7 years ago

thbtcllt commented 7 years ago

Current design does not support parallel encryption/decryption of the same gcm_ctx.

Issue can be emulated with one thread with the following test:

// precompute key aesni_gcm128_pre_xxx(vector->K, &gdata); // start encryption of a first message aesni_gcm128_init_xxx(&gdata, ...) // encrypt first message aesni_gcm128_enc_update_xxx(&gdata, ...) // start encryption of a second message aesni_gcm128_init_xxx(&gdata, ...) // finalize encryption of the first message aesni_gcm128_enc_finalize_xxx(&gdata, ...) // encrypt second message aesni_gcm128_enc_update_xxx(&gdata, ...) // finalize encryption of the second message aesni_gcm128_enc_finalize_xxx(&gdata, ...)

In this case encryption results for both messages are wrong as information relative to the encryption are in the gdata variable.

To allow encryption with this configuration the gcm_ctx must be split in two:

The first part is initialized by the aesni_gcm128_pre_xxx function and use later without any modification. The second part is initialized by the aesni_gcm128_init_xxx function and must be provided too the following aesni_gcm128_enc_update_xxx and aesni_gcm128_enc_finalize_xxx functions.

Way to manage this second part is under the responsability of the caller of the library.

With this fix the sequence described in the test becomes:

// precompute key aesni_gcm128_pre_xxx(vector->K, &gdata); // start encryption of a first message aesni_gcm128_init_xxx(&gdata, &int_data1, ...) // encrypt first message aesni_gcm128_enc_update_xxx(&gdata, &int_data1, ...) // start encryption of a second message aesni_gcm128_init_xxx(&gdata, &int_data2, ...) // finalize encryption of the first message aesni_gcm128_enc_finalize_xxx(&gdata, &int_data1, ...) // encrypt second message aesni_gcm128_enc_update_xxx(&gdata, &int_data2, ...) // finalize encryption of the second message aesni_gcm128_enc_finalize_xxx(&gdata, &int_data2, ...)

This change provides correct encryption/decryption results for both messages.

Performance result with perf tool provides by the latest version of the intel multibuffer library (https://github.com/01org/intel-ipsec-mb) do not show any significant differences

sgmonroy commented 7 years ago

I reckon this patch is also addressing #7.

This same issue would be observed if multiple threads were to encrypt or decrypt using the same gcm_data struct. As your patch suggests, each thread needs its own context.

tkanteck commented 7 years ago

Thanks for the patch! As sgmonroy pointed this is already subject of open issue.

I like the approach taken in this patch. I was also considering another option that wouldn't break applications using current API. Let me put more context into the issue thread https://github.com/01org/intel-ipsec-mb/issues/7

tkanteck commented 7 years ago

Thanks for the patch. This issue has been addressed in https://github.com/01org/intel-ipsec-mb/commit/46a9d9e4e116f7f183ac1458f920441a6dc28a4e