Conversation
Previously BN_MONT_CTX objects were not freed. The default RSA method uses static function rsa_ossl_finish() to free them. This fix gets the reference to the function and call it from our finish function.
|
@kazuho Sure. |
omasanori
left a comment
omasanori
left a comment
There was a problem hiding this comment.
In principle, I am happy with this change. However, I doubt if it affects newer OpenSSL only. Please consider my comments below.
| #ifdef NEVERBLEED_OPAQUE_RSA_METHOD | ||
| rsa_default_method = RSA_PKCS1_OpenSSL(); | ||
| rsa_method = RSA_meth_dup(rsa_default_method); | ||
| rsa_finish = RSA_meth_get_finish(rsa_method); |
There was a problem hiding this comment.
I may miss some context, but I don't know why this and other changes are only for newer OpenSSL since RSA_METHOD has int (*finish) (RSA *rsa); even before OpenSSL 1.1.0. If there is no specific reasons, I would suggest putting rsa_finish = rsa_default_method->finish; in #else -- #endif below for older versions.
There was a problem hiding this comment.
Note that, while getting the finish function pointer from the default method and doing from a clone of it is the same, the former might be better for consistency if you approve my comment.
| } | ||
|
|
||
| #ifdef NEVERBLEED_OPAQUE_RSA_METHOD | ||
| static int (*rsa_finish)(RSA *rsa); |
There was a problem hiding this comment.
If you make the change in neverbleed_init also for !NEVERBLEED_OPAQUE_RSA_METHOD, this declaration should be moved out of #ifdef.
| get_privsep_data(rsa, &exdata, &thdata); | ||
|
|
||
| #ifdef NEVERBLEED_OPAQUE_RSA_METHOD | ||
| rsa_finish(rsa); |
3 participants
As reported in #26.
Differences from tatsuhiro-t@003de52 are:
exdatahas been omitted, as that is checked withinget_privsep_datarsa_finishhas been surrounded byNEVERBLEED_OPAQUE_RSA_METHOD@omasanori Would you mind reviewing this PR?