For benchmarking purposes provide wrappers around OpenSSL AES GCM implementation. Note, digest callback will work only for encryption due to OpenSSL internals.
Signed-off-by: Dmitry Eremin-Solenikov dbaryshkov@gmail.com --- examples/nettle-openssl.c | 109 +++++++++++++++++++++++++++++++++++++++++++++- nettle-internal.h | 3 ++ 2 files changed, 110 insertions(+), 2 deletions(-)
diff --git a/examples/nettle-openssl.c b/examples/nettle-openssl.c index b549ba54..a0b20d3c 100644 --- a/examples/nettle-openssl.c +++ b/examples/nettle-openssl.c @@ -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); } static void @@ -89,7 +89,7 @@ openssl_evp_set_decrypt_key(void *p, const uint8_t *key, { struct openssl_cipher_ctx *ctx = p; ctx->evp = EVP_CIPHER_CTX_new(); - assert(EVP_DecryptInit_ex(ctx->evp, cipher, NULL, key, NULL) == 1); + assert(EVP_CipherInit_ex(ctx->evp, cipher, NULL, key, NULL, 0) == 1); EVP_CIPHER_CTX_set_padding(ctx->evp, 0); }
@@ -110,6 +110,47 @@ openssl_evp_decrypt(const void *p, size_t length, assert(EVP_DecryptUpdate(ctx->evp, dst, &len, src, length) == 1); }
+static void +openssl_evp_set_nonce(void *p, const uint8_t *nonce) +{ + const struct openssl_cipher_ctx *ctx = p; + assert(EVP_CipherInit_ex(ctx->evp, NULL, NULL, NULL, nonce, -1) == 1); +} + +static void +openssl_evp_update(void *p, size_t length, const uint8_t *src) +{ + const struct openssl_cipher_ctx *ctx = p; + int len; + assert(EVP_EncryptUpdate(ctx->evp, NULL, &len, src, length) == 1); +} + +/* This will work for encryption only! */ +static void +openssl_evp_gcm_digest(void *p, size_t length, uint8_t *dst) +{ + const struct openssl_cipher_ctx *ctx = p; + assert(EVP_CIPHER_CTX_ctrl(ctx->evp, EVP_CTRL_GCM_GET_TAG, length, dst) == 1); +} + +static void +openssl_evp_aead_encrypt(void *p, size_t length, + uint8_t *dst, const uint8_t *src) +{ + const struct openssl_cipher_ctx *ctx = p; + int len; + assert(EVP_EncryptUpdate(ctx->evp, dst, &len, src, length) == 1); +} + +static void +openssl_evp_aead_decrypt(void *p, size_t length, + uint8_t *dst, const uint8_t *src) +{ + const struct openssl_cipher_ctx *ctx = p; + int len; + assert(EVP_DecryptUpdate(ctx->evp, dst, &len, src, length) == 1); +} + /* AES */ static nettle_set_key_func openssl_aes128_set_encrypt_key; static nettle_set_key_func openssl_aes128_set_decrypt_key; @@ -175,6 +216,70 @@ nettle_openssl_aes256 = { openssl_evp_encrypt, openssl_evp_decrypt };
+/* AES-GCM */ +static void +openssl_gcm_aes128_set_encrypt_key(void *ctx, const uint8_t *key) +{ + openssl_evp_set_encrypt_key(ctx, key, EVP_aes_128_gcm()); +} +static void +openssl_gcm_aes128_set_decrypt_key(void *ctx, const uint8_t *key) +{ + openssl_evp_set_decrypt_key(ctx, key, EVP_aes_128_gcm()); +} + +static void +openssl_gcm_aes192_set_encrypt_key(void *ctx, const uint8_t *key) +{ + openssl_evp_set_encrypt_key(ctx, key, EVP_aes_192_gcm()); +} +static void +openssl_gcm_aes192_set_decrypt_key(void *ctx, const uint8_t *key) +{ + openssl_evp_set_decrypt_key(ctx, key, EVP_aes_192_gcm()); +} + +static void +openssl_gcm_aes256_set_encrypt_key(void *ctx, const uint8_t *key) +{ + openssl_evp_set_encrypt_key(ctx, key, EVP_aes_256_gcm()); +} +static void +openssl_gcm_aes256_set_decrypt_key(void *ctx, const uint8_t *key) +{ + openssl_evp_set_decrypt_key(ctx, key, EVP_aes_256_gcm()); +} + +const struct nettle_aead +nettle_openssl_gcm_aes128 = { + "openssl gcm_aes128", sizeof(struct openssl_cipher_ctx), + 16, 16, 12, 16, + openssl_gcm_aes128_set_encrypt_key, openssl_gcm_aes128_set_decrypt_key, + openssl_evp_set_nonce, openssl_evp_update, + openssl_evp_aead_encrypt, openssl_evp_aead_decrypt, + openssl_evp_gcm_digest +}; + +const struct nettle_aead +nettle_openssl_gcm_aes192 = { + "openssl gcm_aes192", sizeof(struct openssl_cipher_ctx), + 16, 24, 12, 16, + openssl_gcm_aes192_set_encrypt_key, openssl_gcm_aes192_set_decrypt_key, + openssl_evp_set_nonce, openssl_evp_update, + openssl_evp_aead_encrypt, openssl_evp_aead_decrypt, + openssl_evp_gcm_digest +}; + +const struct nettle_aead +nettle_openssl_gcm_aes256 = { + "openssl gcm_aes256", sizeof(struct openssl_cipher_ctx), + 16, 32, 12, 16, + openssl_gcm_aes256_set_encrypt_key, openssl_gcm_aes256_set_decrypt_key, + openssl_evp_set_nonce, openssl_evp_update, + openssl_evp_aead_encrypt, openssl_evp_aead_decrypt, + openssl_evp_gcm_digest +}; + /* Arcfour */ static void openssl_arcfour128_set_encrypt_key(void *ctx, const uint8_t *key) diff --git a/nettle-internal.h b/nettle-internal.h index 0b0d25c9..38c8d2a8 100644 --- a/nettle-internal.h +++ b/nettle-internal.h @@ -76,6 +76,9 @@ extern const struct nettle_aead nettle_arcfour128; extern const struct nettle_aead nettle_chacha; extern const struct nettle_aead nettle_salsa20; extern const struct nettle_aead nettle_salsa20r12; +extern const struct nettle_aead nettle_openssl_gcm_aes128; +extern const struct nettle_aead nettle_openssl_gcm_aes192; +extern const struct nettle_aead nettle_openssl_gcm_aes256;
/* Glue to openssl, for comparative benchmarking. Code in * examples/nettle-openssl.c. */
Signed-off-by: Dmitry Eremin-Solenikov dbaryshkov@gmail.com --- examples/nettle-benchmark.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/examples/nettle-benchmark.c b/examples/nettle-benchmark.c index 11f62709..53c4e5f9 100644 --- a/examples/nettle-benchmark.c +++ b/examples/nettle-benchmark.c @@ -766,6 +766,9 @@ main(int argc, char **argv) &nettle_gcm_aes128, &nettle_gcm_aes192, &nettle_gcm_aes256, + OPENSSL(&nettle_openssl_gcm_aes128) + OPENSSL(&nettle_openssl_gcm_aes192) + OPENSSL(&nettle_openssl_gcm_aes256) &nettle_gcm_camellia128, &nettle_gcm_camellia256, &nettle_eax_aes128,
Hello,
2017-12-12 3:54 GMT+03:00 Dmitry Eremin-Solenikov dbaryshkov@gmail.com:
For benchmarking purposes provide wrappers around OpenSSL AES GCM implementation. Note, digest callback will work only for encryption due to OpenSSL internals.
Signed-off-by: Dmitry Eremin-Solenikov dbaryshkov@gmail.com
examples/nettle-openssl.c | 109 +++++++++++++++++++++++++++++++++++++++++++++- nettle-internal.h | 3 ++ 2 files changed, 110 insertions(+), 2 deletions(-)
What about these two patches? They allow comparing nettle vs OpenSSL speed for AES-GCM.
Dmitry Eremin-Solenikov dbaryshkov@gmail.com writes:
What about these two patches? They allow comparing nettle vs OpenSSL speed for AES-GCM.
They look ok, but I haven't yet had time to merge them.
Regards, /Niels
Dmitry Eremin-Solenikov dbaryshkov@gmail.com 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.
Regards, /Niels
nisse@lysator.liu.se (Niels Möller) writes:
Dmitry Eremin-Solenikov dbaryshkov@gmail.com writes:
For benchmarking purposes provide wrappers around OpenSSL AES GCM implementation. Note, digest callback will work only for encryption due to OpenSSL internals.
And regarding the numbers, I think it is gcm_hash which is the bottleneck for gcm. Nettle's x86_64 assembly does the gf operations with one table lookup + shifting per byte. One could do something faster and more clever with pclmul https://software.intel.com/en-us/articles/intel-carry-less-multiplication-in....
I remember that in the "embedded nettle" project a few years ago, I looked at ARM neon instructions for carry-less multiplication. And it seems to be quite complicated, since it offered only carryless mul only on 8-bit values (with nice SIMD parallelism).
On my machine, Nettle's gcm_aes128 encrypt is roughly 8 cycles per byte, compared to 1.25 for aes128 in plain ecb mode. Openssl numbers 0.75 and 0.65, respectively.
Regards, /Niels
On Sat, Feb 17, 2018 at 4:35 AM, Niels Möller nisse@lysator.liu.se wrote:
...
@@ -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'.
No asserts, period. They should not get through an audit.
What happens when an assert fires? According to Posix an abort() happens (http://pubs.opengroup.org/onlinepubs/009695399/functions/assert.html).
What happens when the abort happens? The sensitive information inside the app's security boundary leaves the security boundary. It is egressed from the app and written to a core file.
It gets worse. If services are engaged like Windows Error Reporting, Appport, Apple Crash Reporter, Crashlytics , and friends, then the sensitive information leaves the computer or device, too. It just crossed another security boundary and a third party owns it.
Everything that used to be private, like password lists and private keys, are now public information. The NSA, GCHQ and law enforcement thanks you.
The library that really perplexes me is libsodium. It is used in software like Bitcoin and Monero. Every time an assert fires it is like opening the wallet up for thieves to take what they want.
Please ensure you always build high integrity software with -DNDEBUG to remove the asserts.
Jeff
On 17/02/18 22:59, Jeffrey Walton wrote:
On Sat, Feb 17, 2018 at 4:35 AM, Niels Möller wrote:
...
@@ -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'.
No asserts, period. They should not get through an audit.
This is the QA code for testing. Asserting that it works correctly and aborting to fail the test ASAP is the whole purpose of these.
The -DNDEBUG has to be accounted for even in test code because additional compiler optimization may introduce issues for production builds that do not show up when debug info clutters the binary.
No production systems should ever be running the test code on sensitive data. Test data maybe, but not real PII.
AYJ
On Sat, Feb 17, 2018 at 6:30 AM, Amos Jeffries squid3@treenet.co.nz wrote:
On 17/02/18 22:59, Jeffrey Walton wrote:
On Sat, Feb 17, 2018 at 4:35 AM, Niels Möller wrote:
...
...
It's not right to use assert on expressions with side-effects. Since will break builds with ./configure CFLAGS='-DNDEBUG'.
No asserts, period. They should not get through an audit.
This is the QA code for testing. Asserting that it works correctly and aborting to fail the test ASAP is the whole purpose of these.
My bad, I should have said:
No asserts in production, period
I use them prodigiously during development. Everything is validated with an assert. I usually don't have to debug because the asserts debug for me. My code can't fart or sneeze without me knowing about it.
But production is a different story.... The time for debugging is over...
Jeff
Jeffrey Walton noloader@gmail.com writes:
But production is a different story.... The time for debugging is over...
I would generally assume that there are a few bugs left in also the production code. And I think that in most cases, the bug manifesting itself as an assertion failure is a lot better then the alternatives.
I know there are different opinions. We should support -DNDEBUG builds, but it's not going to be the default in Nettle.
Regards, /Niels
On Sat, Feb 17, 2018 at 7:36 AM, Niels Möller nisse@lysator.liu.se wrote:
Jeffrey Walton noloader@gmail.com writes:
But production is a different story.... The time for debugging is over...
I would generally assume that there are a few bugs left in also the production code. And I think that in most cases, the bug manifesting itself as an assertion failure is a lot better then the alternatives.
I know there are different opinions. We should support -DNDEBUG builds, but it's not going to be the default in Nettle.
Yes, please do support NDEBUG. I build all my software with it because I don't want to lose the sensitive information.
It is easy enough to audit. Everywhere there is an assert() to assert a condition, then there should be an if() statement with the same test that returns failure. It is OK to avoid processing if you don't like the condition.
Postel's law is dangerous nowadays. The threat landscape has changed. Look for any reason you can to fail processing. If you can't find a reason, then begrudgingly process the data.
I found using the anit-Postel law made my software incredibly stable. It was nearly impossible to make it fail or crash.
Jeff
Jeffrey Walton noloader@gmail.com writes:
It is easy enough to audit. Everywhere there is an assert() to assert a condition, then there should be an if() statement with the same test that returns failure.
Sorry for getting a bit off-topic, but I would like to advice against that practice.
In particular, it makes the error handling for the "return failure" case awkward to test properly. At least, in my experience it is desirable to be able to run the same testsuite on either release or debug builds. But any tests intended to exercise that error handling will crash with an assertion failure when testing a debug build. I don't like that at all.
Getting back to Nettle, I strive to design the interfaces so that there are no ways to fail, given valid inputs. No return value, and no error handling required from callers. And where practical, documented input requirements are backed up with asserts.
E.g, consider the _set_key method for some cipher. Due to an application bug, this function might be called with an empty key, with a null key pointer or zero key length. Nettle will then crash with either an assertion failure or fatal signal from a null pointer dereference.
We could avoid crashing, by silently ignoring the error and doing nothing. That seems dangerous to me, the application might end up sending sensitive data encrypted with an uninitialized cipher, and where the value of uninitialized subkeys is likely easy to guess by the attacker.
We could return a error code, but for that to be a real improvement, *all* applications must clutter up their code to check errors from _set_key, which are not expected to ever happen, and with error handling which is inconvenient to test properly. If the applications with the empty key bug fails to check the return value, or checks it but with bugs in the error handling code, it will end up in the same case of silently using a cipher with uninitialized easy-to-guess subkeys.
On the other hand, the _set_key function for ciphers with weak keys does have a return value in Nettle. I think this makes sense because
1. It's Nettle's job to know which keys are weak, not the application's.
2. Checking for weak keys is somewhat optional. It might useful in some cases, but not terribly useful if the application selects the key using some random process where the probability of weak keys are negligible. And for the applications where it does matter, the error handling should be reasonably easy to test.
Postel's law is dangerous nowadays. The threat landscape has changed. Look for any reason you can to fail processing. If you can't find a reason, then begrudgingly process the data.
I'm afraid I can't make much sense of this remark. You probably read the robustness principle (RFC 1112) differently than I do.
The way I read it, one should be prepared to receive all possible input from the remote end, make sure that everything allowed by the spec is handled correctly, and with proper error handling for anything invalid.
While when sending data, one should stay in the main stream. Avoid any obscure and rarely used protocol features and corner cases, even when they are technically correct according to the spec.
Regards, /Niels
That's a nice point. Operating systems evolved to be able to obtain crash at a level which is not reflected to low level functions like abort and assert. Can that be addressed in the nettle or individual lib level or at the POSIX level with the introduction of secure-assert or so? Removing asserts on production will cause more harm than good.
On February 17, 2018 9:59:28 AM UTC, Jeffrey Walton noloader@gmail.com wrote:
On Sat, Feb 17, 2018 at 4:35 AM, Niels Möller nisse@lysator.liu.se wrote:
...
@@ -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'.
No asserts, period. They should not get through an audit.
What happens when an assert fires? According to Posix an abort() happens (http://pubs.opengroup.org/onlinepubs/009695399/functions/assert.html).
What happens when the abort happens? The sensitive information inside the app's security boundary leaves the security boundary. It is egressed from the app and written to a core file.
It gets worse. If services are engaged like Windows Error Reporting, Appport, Apple Crash Reporter, Crashlytics , and friends, then the sensitive information leaves the computer or device, too. It just crossed another security boundary and a third party owns it.
Everything that used to be private, like password lists and private keys, are now public information. The NSA, GCHQ and law enforcement thanks you.
The library that really perplexes me is libsodium. It is used in software like Bitcoin and Monero. Every time an assert fires it is like opening the wallet up for thieves to take what they want.
Please ensure you always build high integrity software with -DNDEBUG to remove the asserts.
Jeff _______________________________________________ nettle-bugs mailing list nettle-bugs@lists.lysator.liu.se http://lists.lysator.liu.se/mailman/listinfo/nettle-bugs
Jeffrey Walton noloader@gmail.com writes:
No asserts, period. They should not get through an audit.
I see that you have a strong opinion on the subject. I'll state my opinion, but I don't want to get into a heated debate.
When the condition in an assert is fails, that's evidence of a software bug. In my experience, it's usually prefable to crash immediately and in a controlled manner, to reduce risk of silent data corruption, exploitable buffer overruns, and the like. I guess there are a few applications where it might be better for the program to continua running and hope for the best, but those are exceptions.
What happens when the abort happens?
That depends on various per-process and system-level settings.
Thaere are lots of possibly software bugs that can lead to a crash of the process, not all involving any asserts. I think it's common practice in security critical applications to disable core dumps using the standard ulimit facility. I can't see asserts as a problem at all in this context.
Regards, /Niels
2018-02-17 12:35 GMT+03:00 Niels Möller nisse@lysator.liu.se:
Dmitry Eremin-Solenikov dbaryshkov@gmail.com 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.
Thank you!
@@ -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.
I've sent a followup patch with an additional variable. It is easier to maintain.
Regarding the numbers, here is what I see on my laptop (i3-4005U):
libnettle: fat library initialization. libnettle: cpu features: vendor:intel,aesni libnettle: using aes instructions. libnettle: not using sha_ni instructions. libnettle: intel SSE2 will be used for memxor. sha1_compress: 463.20 cycles salsa20_core: 403.70 cycles sha3_permute: 1838.10 cycles (76.59 / round)
benchmark call overhead: 0.003764 us 6.02 cycles
Algorithm mode Mbyte/s cycles/byte cycles/block poly1305-aes update 951.68 1.60 1641.84
aes128 ECB encrypt 2328.39 0.66 10.49 aes128 ECB decrypt 1850.82 0.82 13.19 aes128 CBC encrypt 267.16 5.71 91.38 aes128 CBC decrypt 2015.34 0.76 12.11 aes128 (in-place) 1598.20 0.95 15.28 aes128 CTR 1683.51 0.91 14.50 aes128 (in-place) 1343.33 1.14 18.17
aes192 ECB encrypt 1695.07 0.90 14.40 aes192 ECB decrypt 1737.14 0.88 14.05 aes192 CBC encrypt 231.50 6.59 105.46 aes192 CBC decrypt 1548.88 0.99 15.76 aes192 (in-place) 1290.06 1.18 18.92 aes192 CTR 1349.77 1.13 18.09 aes192 (in-place) 1135.87 1.34 21.49
aes256 ECB encrypt 1462.52 1.04 16.69 aes256 ECB decrypt 1371.13 1.11 17.81 aes256 CBC encrypt 204.46 7.46 119.41 aes256 CBC decrypt 1296.66 1.18 18.83 aes256 (in-place) 1056.29 1.44 23.11 aes256 CTR 1123.06 1.36 21.74 aes256 (in-place) 981.12 1.56 24.88
openssl aes128 ECB encrypt 2308.04 0.66 10.58 openssl aes128 ECB decrypt 2294.12 0.67 10.64
openssl aes192 ECB encrypt 1929.11 0.79 12.66 openssl aes192 ECB decrypt 1926.87 0.79 12.67
openssl aes256 ECB encrypt 1653.41 0.92 14.77 openssl aes256 ECB decrypt 1666.08 0.92 14.65
gcm_aes128 encrypt 200.26 7.62 121.91 gcm_aes128 decrypt 200.26 7.62 121.91 gcm_aes128 update 240.93 6.33 101.33
gcm_aes192 encrypt 193.38 7.89 126.25 gcm_aes192 decrypt 194.50 7.85 125.52 gcm_aes192 update 241.02 6.33 101.29
gcm_aes256 encrypt 189.66 8.05 128.73 gcm_aes256 decrypt 189.74 8.04 128.67 gcm_aes256 update 240.14 6.35 101.67
openssl gcm_aes128 encrypt 1492.12 1.02 16.36 openssl gcm_aes128 decrypt 1496.77 1.02 16.31 openssl gcm_aes128 update 3683.07 0.41 6.63
openssl gcm_aes192 encrypt 1308.33 1.17 18.66 openssl gcm_aes192 decrypt 1315.36 1.16 18.56 openssl gcm_aes192 update 3662.13 0.42 6.67
openssl gcm_aes256 encrypt 1170.64 1.30 20.86 openssl gcm_aes256 decrypt 1173.83 1.30 20.80 openssl gcm_aes256 update 3489.30 0.44 7.00
On February 17, 2018 9:35:34 AM UTC, nisse@lysator.liu.se wrote:
Dmitry Eremin-Solenikov dbaryshkov@gmail.com 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.
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
nettle-bugs@lists.lysator.liu.se
-
Amos Jeffries -
Dmitry Eremin-Solenikov -
Jeffrey Walton -
Nikos Mavrogiannopoulos -
nisse＠lysator.liu.se