On 4/12/23 09:05, Niels Möller wrote:
Simo Sorce writes:
Ah you do not need to pass any property for the default provider so you can pass "" or even NULL.
Thanks, I now have the RSA code updated (on branch update-openssl-bench, if anyone wants to see the details). Initialization is now
ctx->pkey_ctx = EVP_PKEY_CTX_new_from_name (NULL, "RSA", ""); if (!ctx->pkey_ctx) die ("OpenSSL EVP_PKEY_CTX_new_from_name ("RSA") failed.\n");
FWIW, In Squid with OpenSSLv3 we use this:
EVP_PKEY_CTX_new_id(EVP_PKEY_RSA, NULL)
if (EVP_PKEY_keygen_init (ctx->pkey_ctx) <= 0) die ("OpenSSL EVP_PKEY_keygen_init failed.\n"); if (EVP_PKEY_CTX_set_rsa_keygen_bits(ctx->pkey_ctx, size) <= 0) die ("OpenSSL EVP_PKEY_CTX_set_rsa_keygen_bits failed.\n"); BIGNUM *e = BN_new(); BN_set_word(e, 65537); EVP_PKEY_CTX_set1_rsa_keygen_pubexp (ctx->pkey_ctx, e); EVP_PKEY_keygen (ctx->pkey_ctx, &ctx->key);
However, when I run this under valgrind (to check the corresponding cleanup code doesn't leak memory), I get an error:
==3016684== Conditional jump or move depends on uninitialised value(s) ==3016684== at 0x4B0B824: EVP_PKEY_generate (in /usr/lib/x86_64-linux-gnu/libcrypto.so.3) ==3016684== by 0x10F30A: bench_openssl_rsa_init (hogweed-benchmark.c:721) ==3016684== by 0x10D7AE: bench_alg (hogweed-benchmark.c:153) ==3016684== by 0x10D7AE: main (hogweed-benchmark.c:972) ==3016684==
I wonder if that my code missing some initialization, or if that's an openssl problem?
How was the "ctx" variable created and initialized?
The new EVP_PKEY logic has a lot of "ctx_is_legacy" checks based on the ctx itself. So that matters now where it did not before.
It's also unclear to me when the e bignum above can be deallocated, does EVP_PKEY_CTX_set1_rsa_keygen_pubexp imply a full copy into the context?
Quick reading of the source code indicates that yes the context used BN_dup() one way or another.
Next is updating the ecdsa benchmarks, since, e.g., EC_KEY_new_by_curve_name, generates deprecation warnings.
Regards, /Niels
HTH Amos
On Tue, 2023-12-05 at 13:17 +1300, Amos Jeffries wrote:
On 4/12/23 09:05, Niels Möller wrote:
Simo Sorce writes:
Ah you do not need to pass any property for the default provider so you can pass "" or even NULL.
Thanks, I now have the RSA code updated (on branch update-openssl-bench, if anyone wants to see the details). Initialization is now
ctx->pkey_ctx = EVP_PKEY_CTX_new_from_name (NULL, "RSA", ""); if (!ctx->pkey_ctx) die ("OpenSSL EVP_PKEY_CTX_new_from_name ("RSA") failed.\n");
FWIW, In Squid with OpenSSLv3 we use this:
EVP_PKEY_CTX_new_id(EVP_PKEY_RSA, NULL)
EVP_PKEY_CTX_new_from_name is the more proper way in OpenSSL 3.0
if (EVP_PKEY_keygen_init (ctx->pkey_ctx) <= 0) die ("OpenSSL EVP_PKEY_keygen_init failed.\n"); if (EVP_PKEY_CTX_set_rsa_keygen_bits(ctx->pkey_ctx, size) <= 0) die ("OpenSSL EVP_PKEY_CTX_set_rsa_keygen_bits failed.\n"); BIGNUM *e = BN_new(); BN_set_word(e, 65537); EVP_PKEY_CTX_set1_rsa_keygen_pubexp (ctx->pkey_ctx, e); EVP_PKEY_keygen (ctx->pkey_ctx, &ctx->key);
However, when I run this under valgrind (to check the corresponding cleanup code doesn't leak memory), I get an error:
==3016684== Conditional jump or move depends on uninitialised value(s) ==3016684== at 0x4B0B824: EVP_PKEY_generate (in /usr/lib/x86_64-linux-gnu/libcrypto.so.3) ==3016684== by 0x10F30A: bench_openssl_rsa_init (hogweed-benchmark.c:721) ==3016684== by 0x10D7AE: bench_alg (hogweed-benchmark.c:153) ==3016684== by 0x10D7AE: main (hogweed-benchmark.c:972) ==3016684==
If you can get debug symbols on your machine so we can see where (file/line number) in openssl this leaks we can easily find out what is the source.
I wonder if that my code missing some initialization, or if that's an openssl problem?
How was the "ctx" variable created and initialized?
The new EVP_PKEY logic has a lot of "ctx_is_legacy" checks based on the ctx itself. So that matters now where it did not before.
It's also unclear to me when the e bignum above can be deallocated, does EVP_PKEY_CTX_set1_rsa_keygen_pubexp imply a full copy into the context?
Form the manpage: EVP_PKEY_CTX_set1_rsa_keygen_pubexp() sets the public exponent value for RSA key generation to the value stored in pubexp. Currently it should be an odd integer. In accordance with the OpenSSL naming convention, the pubexp pointer must be freed independently of the EVP_PKEY_CTX (ie, it is internally copied). If not specified 65537 is used.
Quick reading of the source code indicates that yes the context used BN_dup() one way or another.
Next is updating the ecdsa benchmarks, since, e.g., EC_KEY_new_by_curve_name, generates deprecation warnings.
Regards, /Niels
HTH Amos _______________________________________________ nettle-bugs mailing list -- nettle-bugs@lists.lysator.liu.se To unsubscribe send an email to nettle-bugs-leave@lists.lysator.liu.se
Simo Sorce simo@redhat.com writes:
On Tue, 2023-12-05 at 13:17 +1300, Amos Jeffries wrote:
FWIW, In Squid with OpenSSLv3 we use this:
EVP_PKEY_CTX_new_id(EVP_PKEY_RSA, NULL)
EVP_PKEY_CTX_new_from_name is the more proper way in OpenSSL 3.0
My current version uses EVP_RSA_gen and EVP_EC_gen, then I only need EVP_PKEY, no EVP_PKEY_CTX. As far as I can tell from the docs, those functions are recommended and not deprecated.
With that change, the valgrind warning disappeared as well.
I've also had to make some changes to nettle-benchmark, it seems blowfish, cast128 and des are no longer supported for the default provider, one would need to somehow enable the "legacy" provider, and crashed (assert failure in the glue code) with recent openssl. It seemed easier to just delete those benchmarks; comparative benchmarking of those algorithms doesn't seem that interesting.
Tangent: Not sure why openssl has demoted blowfish to "legacy", if it's just the shorter 64-bit blocksize that is considered a problem? (According to https://www.schneier.com/academic/blowfish/: "Blowfish was created in 1993. While there is still no practical attack against the cipher, it only has a 64-bit block length and was optimized for 32-bit CPUs. If you are thinking of using this algorithm, I recommend that you use Twofish instead").
Regards, /Niels
nettle-bugs@lists.lysator.liu.se
-
Amos Jeffries -
Niels Möller -
Simo Sorce