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/5c78bb737c553f2064271f1a7c4768b88a09b665,
>> 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
_______________________________________________
nettle-bugs mailing list
nettle-bugs@lists.lysator.liu.se
http://lists.lysator.liu.se/mailman/listinfo/nettle-bugs
