On 18/02/18 00:48, Nikos Mavrogiannopoulos wrote:
On February 17, 2018 9:35:34 AM UTC, nisse wrote:
Dmitry Eremin-Solenikov writes:
For benchmarking purposes provide wrappers around OpenSSL AES GCM implementation. Note, digest callback will work only for encryption
due
to OpenSSL internals.
This patch and the next now merged to master-updates.
@@ -80,7 +80,7 @@ openssl_evp_set_encrypt_key(void *p, const uint8_t
*key,
{ struct openssl_cipher_ctx *ctx = p; ctx->evp = EVP_CIPHER_CTX_new();
- assert(EVP_EncryptInit_ex(ctx->evp, cipher, NULL, key, NULL) ==
1);
- assert(EVP_CipherInit_ex(ctx->evp, cipher, NULL, key, NULL, 1) ==
1);
EVP_CIPHER_CTX_set_padding(ctx->evp, 0); }
It's not right to use assert on expressions with side-effects. Since will break builds with ./configure CFLAGS='-DNDEBUG'.
One would need to either assign return value to a variable and assert on that, or define some alternative assert-like makro which always evaluates its argument.
Not a big problem if only in the benchmark code, but it should be avoided. It was introduced earlier, in commit https://git.lysator.liu.se/nettle/nettle/commit/5c78bb737c553f2064271f1a7c47..., but I didn't notice at the time.
An alternative fix for that could be to introduce a check in a header which will fail compilation if ndebug is used.
That alternative is worse than the actual bug. Which was context members not being initialized for -DNDEBUG builds.
Forcing all builds to avoid NDEBUG (ie asserts in production) is where the issues Jeffrey mentioned start to have relevance to other parts of the library dealing with PII.
AYJ