Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Global DLL Handlers implementation #321

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ZGabrovski
Copy link

Please, find attached global OpenSSL dll handlers implementation.
I also did some miss comment brackets which causes lazarus editor to do incorrect highlighting.

@rlebeau rlebeau added the Type: Enhancement Issue is proposing a new feature/enhancement label May 20, 2021
@rlebeau rlebeau added the Element: SSL/TLS Issues related to SSL/TLS handling, TIdSSLIOHandlerSocketBase and descendants label Jan 31, 2023
Exit;
end;
{$ENDIF}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the 2 global handles have been assigned, exiting here will bypass Indy's initialization of OpenSSL's thread-locking callbacks. Are you sure this is intended behavior?

Exit;
end;
{$ENDIF}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the 2 global handles have been assigned, exiting here will bypass Indy's cleanup of OpenSSL's thread-locking callbacks. Are you sure this is intended behavior?

hIdCrypto : TIdLibHandle = IdNilHandle;

{$ENDIF}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the point of declaring getter/setter functions if you are also making the variables themselves public as well? The variables should not be in the interface, only the implementation.

{CH fn_NCONF_load_bio = 'NCONF_load_bio';} {Do not localize}
{CH fn_NCONF_get_section = 'NCONF_get_section';} {Do not localize}
{CH fn_NCONF_get_string = 'NCONF_get_string';} {Do not localize}
{CH fn_NCONF_get_number_e = 'NCONF_get_number_e';} {Do not localize}
{CH fn_NCONF_dump_fp = 'NCONF_dump_fp'; } {Do not localize}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have incorporated this change into the master code. Please resync your PR.

@@ -19730,7 +19794,7 @@ function GetCryptLibHandle : TIdLibHandle;
{CH fn_CRYPTO_pop_info = 'CRYPTO_pop_info'; } {Do not localize}
{CH fn_CRYPTO_remove_all_info = 'CRYPTO_remove_all_info'; } {Do not localize}
{CH fn_OpenSSLDie = 'OpenSSLDie'; } {Do not localize}
{CH fn_OPENSSL_ia32cap_loc = 'OPENSSL_ia32cap_loc'; { {Do not localize}
{CH fn_OPENSSL_ia32cap_loc = 'OPENSSL_ia32cap_loc';} {Do not localize}
{CH fn_CRYPTO_get_new_lockid = 'CRYPTO_get_new_lockid'; } {Do not localize}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have incorporated this change into the master code. Please resync your PR.

{CH fn__ossl_old_des_string_to_2keys = '_ossl_old_des_string_to_2keys';} {Do not localize}
{CH fn__ossl_old_des_cfb64_encrypt = '_ossl_old_des_cfb64_encrypt';} {Do not localize}
{CH fn__ossl_old_des_ofb64_encrypt = '_ossl_old_des_ofb64_encrypt';} {Do not localize}
{CH fn__ossl_096_des_random_seed = '_ossl_096_des_random_seed';} {Do not localize}
{$ENDIF}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have incorporated these changes into the master code. Please resync your PR.

@@ -20764,7 +20828,7 @@ function GetCryptLibHandle : TIdLibHandle;
{CH fn_ASN1_mbstring_copy = 'ASN1_mbstring_copy'; } {Do not localize}
{CH fn_ASN1_mbstring_ncopy = 'ASN1_mbstring_ncopy'; } {Do not localize}
{CH fn_ASN1_STRING_set_by_NID = 'ASN1_STRING_set_by_NID'; } {Do not localize}
{CH fn_ASN1_STRING_TABLE_get = 'ASN1_STRING_TABLE_get'; {Do not localize}
{CH fn_ASN1_STRING_TABLE_get = 'ASN1_STRING_TABLE_get';} {Do not localize}
{CH fn_ASN1_STRING_TABLE_add = 'ASN1_STRING_TABLE_add'; } {Do not localize}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have incorporated this change into the master code. Please resync your PR.

@@ -22082,7 +22146,7 @@ function GetCryptLibHandle : TIdLibHandle;
{CH fn_SSL_use_RSAPrivateKey_ASN1 = 'SSL_use_RSAPrivateKey_ASN1'; } {Do not localize}
{$ENDIF}
{CH fn_SSL_use_PrivateKey = 'SSL_use_PrivateKey'; } {Do not localize}
{CH fn_SSL_use_PrivateKey_ASN1 = 'SSL_use_PrivateKey_ASN1'; {Do not localize}
{CH fn_SSL_use_PrivateKey_ASN1 = 'SSL_use_PrivateKey_ASN1';} {Do not localize}
{CH fn_SSL_use_certificate = 'SSL_use_certificate'; } {Do not localize}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have incorporated this change into the master code. Please resync your PR.

@@ -22437,7 +22501,7 @@ function GetCryptLibHandle : TIdLibHandle;
{CH fn_ENGINE_register_RAND = 'ENGINE_register_RAND'; } {Do not localize}
{CH fn_ENGINE_unregister_RAND = 'ENGINE_unregister_RAND'; } {Do not localize}
{CH fn_ENGINE_register_all_RAND = 'ENGINE_register_all_RAND'; } {Do not localize}
{CH fn_ENGINE_register_STORE = 'ENGINE_register_STORE'; { {Do not localize}
{CH fn_ENGINE_register_STORE = 'ENGINE_register_STORE'; } {Do not localize}
{CH fn_ENGINE_unregister_STORE = 'ENGINE_unregister_STORE'; } {Do not localize}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have incorporated this change into the master code. Please resync your PR.

@rlebeau rlebeau self-assigned this Jan 31, 2023
@rayaswdelphidev
Copy link

rayaswdelphidev commented Feb 2, 2023

Greetings, I apologize if this is not the right space, as I didn't see the follow-up to https://www.atozed.com/forums/thread-1936.html in the open issues list here on GitHub.

I would be interested in testing this fix when you believe it's ready. I recently had the same error as described in the forum thread, being unaware of issues unloading the OpenSSL DLLs from memory. In my application, I have a Delphi 2007 service running an Indy HTTPS server that dynamically loads a DLL (in 2007 as well, but never unloaded) to execute functions that also load/unload a secondary DLL (built in Delphi 10.4) that executes HTTPS REST calls to 3rd party web services.

When the 2nd DLL unloads, it causes the HTTPS service to be unable to make its own outbound HTTPS calls, with the error you are trying to fix with this pull request here, then eventually breaks down where the service can't receive HTTPS requests, either.

To get around the issue, and prevent loading/unloading of OpenSSL, we tried statically linking the 2nd DLL functions to the HTTPS service (first opportunity to load in the process), and used the statically linked functions in the 1st DLL. That appeared to work in our testing environment, preventing Indy from unloading and causing the error (we are preparing to test this in or Beta environment soon).

If the solution in this pull request is better, I would be highly interested in testing it, if possible with Delphi 2007.

But, I have some follow up questions in how to pull the correct code. When Indy was hosted in SVN, we pulled the stable tagged versions of Indy to rebuild with our exe's. Since the move to GitHub, I am unsure how to get the stable versions released, and just lace in the fixes we need with the particular version of Delphi, or do we simply always get the master copy?

Our only other alternative to get around the problem is to move our service to Delphi 10.4 (in the works, but not ready) so we don't need the 2nd DLL for REST calls.

@rlebeau
Copy link
Member

rlebeau commented Feb 6, 2023

I would be interested in testing this fix when you believe it's ready.

I don't have an ETA on that. However, I was looking at this PR the other day to evaluate how feasible it is to include in the current version, or if it should wait for a future release.

When Indy was hosted in SVN, we pulled the stable tagged versions of Indy to rebuild with our exe's. Since the move to GitHub, I am unsure how to get the stable versions released

Even in SVN, there were no "stable" releases, only tags for which revision was included in which Delphi version. However, since the switch to GitHub, Embarcadero hasn't been giving me the commit IDs they pull for each release, so I haven't been able to tag them anymore.

Eventually, I will make real versioned and tagged releases of Indy in GitHub, but that hasn't been implemented yet (see #328).

and just lace in the fixes we need with the particular version of Delphi, or do we simply always get the master copy?

Just pull the master code for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Element: SSL/TLS Issues related to SSL/TLS handling, TIdSSLIOHandlerSocketBase and descendants Type: Enhancement Issue is proposing a new feature/enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants