Posts: 19
Threads: 4
Joined: Sep 2020
Reputation:
0
Location: Bulgaria
09-03-2020, 09:55 AM
(This post was last modified: 09-03-2020, 04:58 PM by rlebeau.)
I have a problem with OpenSSL libraries (libeay32.dll and ssleay32.dll), version 1.02.u (but the same issue with older releases), Windows 64, both Embarcadero Delphi 10.3 Rio and Lazarus 2.1 (FPC 3.3.1) - from the trunk.
The problem is in my multi-threading application, which uses TIdHTTP.Get() for consuming data from a REST api (using SSL/TLS) and picture download function:
Code: begin
Result := TMemoryStream.Create;
Http := TIdHttp.Create(nil);
IOHandler := TIdSSLIOHandlerSocketOpenSSL.Create(nil);
IOHandler.SSLOptions.SSLVersions := [sslvSSLv2, sslvSSLv23, sslvSSLv3, sslvTLSv1,sslvTLSv1_1,sslvTLSv1_2];
Request := TIdHTTPRequest.Create( Http );
Request.UserAgent := UserAgent;
HTTP.Request := Request;
HTTP.IOHandler := IOHandler;
try
try
ii := 0;
repeat
Result.Position := 0;
Http.Get(AURL, Result);
Result.Position := 0;
Result.read( Buffer, 20 );
Inc(ii);
until CheckValidPicture( Buffer )or (ii>2);
if (ii>2) then
Result.Clear;
Result.Position := 0;
except
// Just in case - second try
try
ii := 0;
repeat
Result.Position := 0;
Http.Get(AURL, Result);
Result.Position := 0;
Result.read( Buffer, 20 );
Inc(ii);
until CheckValidPicture( Buffer ) or (ii>2);;
if (ii>2) then
Result.Clear;
Result.Position := 0;
except
if (ii>2) then
Result.Clear;
end;
end;
finally
Http.DisposeOf;
Request.DisposeOf;
IOHandler.DisposeOf;
end;
When I am executing the code for a first time, everything is OK.
But, on second code execution(in other thread), I received the following exception core:
Error creating SSL context:error:140A90F1:lib(20):func(169):reason(241)
in HTTP.Get method.
So after some debug in Indy sources, I found that the problem comes in execution of method "SSL_CTX_new" in unit IdSSLOpenSSL, line 3156:
Code: // create new SSL context
fContext := SSL_CTX_new(SSLMethod);
if fContext = nil then
EIdOSSLCreatingContextError.RaiseException(RSSSLCreatingContextError);
end;
//set SSL Versions we will use
After some google research, I found this:
https://curl.haxx.se/mail/lib-2018-07/0057.html
and modify Indy code like described:
Code: // create new SSL context
fContext := SSL_CTX_new(SSLMethod);
if fContext = nil then begin
OpenSSL_add_all_digests;
fContext := SSL_CTX_new(SSLMethod);
if fContext = nil then
EIdOSSLCreatingContextError.RaiseException(RSSSLCreatingContextError);
end;
//set SSL Versions we will use
and now everything works fine.
Is this OK?
and if it is, how to modify Indy source code ( is there some bug tracker or whatever, to create issue)?
Posts: 649
Threads: 2
Joined: Mar 2018
Reputation:
35
Location: California, USA
09-03-2020, 04:55 PM
(This post was last modified: 09-03-2020, 05:01 PM by rlebeau.)
(09-03-2020, 09:55 AM)ZGabrovski@gmail.com Wrote: Code: IOHandler.SSLOptions.SSLVersions := [sslvSSLv2, sslvSSLv23, sslvSSLv3, sslvTLSv1,sslvTLSv1_1,sslvTLSv1_2];
You should NOT be specifying sslvSSLv2, sslvSSLv23, or sslvSSLv3 at all. SSLv2 and SSLv3 are deprecated and unsecure, nobody uses them anymore. And SSLv23 is a wildcard you should not be using directly, Indy uses it internally. So, just stick with the TLS versions only.
(09-03-2020, 09:55 AM)ZGabrovski@gmail.com Wrote: Code: Request := TIdHTTPRequest.Create( Http );
There is no need to create a TIdHTTPRequest object manually. TIdHTTP will handle that for you. Simply use the TIdHTTP.Request property as-is when setting sub-properties, eg:
Code: Http := TIdHttp.Create(nil);
Http.Request.UserAgent := UserAgent;
(09-03-2020, 09:55 AM)ZGabrovski@gmail.com Wrote: When I am executing the code for a first time, everything is OK.
But, on second code execution(in other thread), I received the following exception core:
Why are you using TIdHTTP across thread boundaries?
(09-03-2020, 09:55 AM)ZGabrovski@gmail.com Wrote: After some google research, I found this:
https://curl.haxx.se/mail/lib-2018-07/0057.html
and modify Indy code like described:
Code: // create new SSL context
fContext := SSL_CTX_new(SSLMethod);
if fContext = nil then begin
OpenSSL_add_all_digests;
fContext := SSL_CTX_new(SSLMethod);
if fContext = nil then
EIdOSSLCreatingContextError.RaiseException(RSSSLCreatingContextError);
end;
//set SSL Versions we will use
and now everything works fine.
Is this OK?
Indy already calls OpenSSL_add_all_digests() when it loads the OpenSSL DLLs into memory (see the LoadOpenSSLLibrary() function in IdSSLOpenSSL.pas). There should be no need to call OpenSSL_add_all_digests() again.
Also, the message you linked to was never answered by the libcurl author, and looking at the libcurl source code, the proposed change was never merged in. The message author stated what the root problem was:
Quote:I found that EVP_get_digestbyname() return NULL because list digest is NULL, function EVP_cleanup() was called before.
Which would imply a bug elsewhere in whatever code is deciding to call EVP_cleanup() prematurely.
Indy calls EVP_cleanup() only when it is unloading the OpenSSL DLLs from memory (see the Unload() function in IdSSLOpenSSLHeaders.pas).
So no, I'm not inclined to add this SSL_CTX_new() "fix" to Indy, when the root cause is clearly elsewhere and should be fixed in that code instead. If this is indeed a bug in Indy itself at all, and not something else in your app that is also using OpenSSL and cleaning it up prematurely.
(09-03-2020, 09:55 AM)ZGabrovski@gmail.com Wrote: if it is, how to modify Indy source code
You would have to submit a Pull Request on GitHub.
(09-03-2020, 09:55 AM)ZGabrovski@gmail.com Wrote: is there some bug tracker or whatever, to create issue?
Yes: https://github.com/IndySockets/Indy/issues
Posts: 19
Threads: 4
Joined: Sep 2020
Reputation:
0
Location: Bulgaria
09-04-2020, 06:02 AM
(This post was last modified: 02-06-2023, 01:53 AM by rlebeau.)
First of all, many thanks for your reply!
"Why are you using TIdHTTP across thread boundaries?"
I have Delphi Datasnap server application which is using a plugin-dll written on FPC (Lazarus). Both of them are using Indy and open SSL to download pictures from web server and consume data form REST Api. May be it is something related with Windows Dll handling (when host app and plugin DLL used one and the same openSSL dll's Windows provide the same dll handlers). For sure When I call method for picture download for second time it raises the exception described above and it SSL_CTX_new(SSLMethod); returns "nill".
When I am not using plugin library the problem does not exists.
May be the better solution will be (if it is possible of course) to pass a openssl dll handlers from the host app to a plugin library and to tell to Indy in plugin dll to use it instead of load /unload open all. This (may be) will fix the case?
I had the similar case with firebird fbclient.dll, in the same aspect. When my host app with Firedac and fb driver calls a plugin library (also written in fpc which uses fbconnection and TSQLQuery) , the plugin dll unloads a fbclient.dll and this causes hosapp to raise internal exception.
I will try to modify and test how this will works.
Posts: 19
Threads: 4
Joined: Sep 2020
Reputation:
0
Location: Bulgaria
09-04-2020, 09:26 AM
(This post was last modified: 09-04-2020, 09:43 AM by ZGabrovski@gmail.com.)
I can confirm now, that for sure the problem comes from openSSL dll library sharing between the host exe app and dll plugin library.
Main problem comes from Windows dll handlig - if you use one and the same dll library in the host app and in the plugin dll libray, Windows does not load fresh copy of dll library, it uses the same. allready loaded into host app, described here: https://stackoverflow.com/questions/1253...iple-times
In this case, this cause call of "LoadOpenSSLLibrary" twice, for one and the same OpenSLL library instance.
It mismatching call of "CRYPTO_set_locking_callback", and finally - it will call "CRYPTO_set_locking_callback(nil);" into dll plugin ibrary, which cause to fail an Openssl in main .exe application.
So - I did a following:
- I create a global DLL handle objects in IdSSLOpenSSLHeaders.pas unit:
Code: hGlobalIdSSL : TIdLibHandle = IdNilHandle;
hGlobalIdCrypto : TIdLibHandle = IdNilHandle;
and global critical section
Code: csGlobalIDSSL : TIdCriticalSection;
and thread-safe methods for get/set values:
Code: function GlobalIdSSL : TIdLibHandle;
function GlobalIdCrypto : TIdLibHandle;
procedure SetGlobalIdSSL ( fGlobalIdSSL : TIdLibHandle );
procedure SetGlobalIdCrypto ( fGlobalIdCrypto : TIdLibHandle );
and move then
Code: hIdSSL : TIdLibHandle = IdNilHandle;
hIdCrypto : TIdLibHandle = IdNilHandle;
from "Implementaion" to "Interface" section;
and I modify a "load" method:
Code: if hIdCrypto = IdNilHandle then begin
//
// Zdravko Gabrovski - Global OpenSSL handler implementation
//
if GlobalIdCrypto <> IdNilHandle then
hIdCrypto := GlobalIdCrypto
else begin
hIdCrypto := LoadSSLCryptoLibrary;
end;
if hIdCrypto = IdNilHandle then begin
FFailedLoadList.Add(IndyFormat(RSOSSFailedToLoad, [GIdOpenSSLPath + SSLCLIB_DLL_name {$IFDEF UNIX}+ LIBEXT{$ENDIF}]));
Exit;
end;
end;
if hIdSSL = IdNilHandle then begin
//
// Zdravko Gabrovski - Global OpenSSL handler implementation
//
if GlobalIdSSL <> IdNilHandle then
hIdSSL := GlobalIdSSL
else
hIdSSL := LoadSSLLibrary;
if hIdSSL = IdNilHandle then begin
FFailedLoadList.Add(IndyFormat(RSOSSFailedToLoad, [GIdOpenSSLPath + SSL_DLL_name {$IFDEF UNIX}+ LIBEXT{$ENDIF}]));
Exit;
end;
end;
- modification of the both functions LoadOpenSSLLibrary and UnLoadOpenSSLLibrary in the IdSSLOpenSSL.pas - now they will check if the new " GlobalIdSSL " and "GlobalIdCrypto " object are assigned, they take the hadle from the variables instad of to load them from a Windows load library, and in that case - Do not call
CRYPTO_set_locking_callback method and all other funcions for init.
Now everything works fine. A pass a handles from the host .exe to the plugin dll library:
In my Host .exe code (delhi 10.3 rio):
Code: Params.Values['hIdSSL']:= hIdSSL.ToString; // from IdSSLOpenSSLHeaders
Params.Values['hIdCrypto']:= hIdCrypto.ToString; // from IdSSLOpenSSLHeaders
In my plugin dll library (Lazarus 2.1 fpc 3.3.1)
Code: lhIdSSL := StrToUInt64Def( Tl.Values['hIdSSL'], 0 ); // Handler comes from host app
lhIdCrypto := StrToUInt64Def( Tl.Values['hIdCrypto'], 0 );
if ( lhIdSSL <> IdNilHandle ) and ( lhIdCrypto <> IdNilHandle ) then begin
SetGlobalIdSSL( lhIdSSL );
SetGlobalIdCrypto( lhIdCrypto );
LoadOpenSSLLibrary;
end;
That's all.
I try to attach changes here, but I can not. It will tell me "File type is not allowed".
I try to create a branch and open a new pull request, but it is not possible, as described here: https://stackoverflow.com/questions/3094...-in-github
My Github account is ZGabrovski.
Posts: 649
Threads: 2
Joined: Mar 2018
Reputation:
35
Location: California, USA
09-04-2020, 07:14 PM
(This post was last modified: 09-04-2020, 10:08 PM by rlebeau.)
(09-04-2020, 09:26 AM)ZGabrovski@gmail.com Wrote: I can confirm now, that for sure the problem comes from openSSL dll library sharing between the host exe app and dll plugin library.
Using Indy statically in multiple modules, especially modules that are unloaded and reloaded dynamically, is a known issue for OpenSSL, which is not designed to be unloaded and reloaded dynamically (and in fact, in OpenSSL 1.1.x, it was updated to avoid being unloaded from memory at all). The separate modules need to share a single copy of Indy/OpenSSL, such as by moving your Indy code to a separate runtime package that they can share.
(09-04-2020, 09:26 AM)ZGabrovski@gmail.com Wrote: So - I did a following:
Interesting approach. I'll consider something like that for a future release. Probably not for Indy 10, but maybe for Indy 11.
(09-04-2020, 09:26 AM)ZGabrovski@gmail.com Wrote: I try to create a branch and open a new pull request, but it is not possible, as described here: https://stackoverflow.com/questions/3094...-in-github
You can't create a branch in Indy's repo. You will have to pull Indy to your own repo first, modify it as desired, and then submit a Pull Request from your repo to Indy's repo.
Posts: 19
Threads: 4
Joined: Sep 2020
Reputation:
0
Location: Bulgaria
Thanks I will do this.
Why not in Indy 10- I can confirm now it works very well.
Posts: 649
Threads: 2
Joined: Mar 2018
Reputation:
35
Location: California, USA
(09-04-2020, 08:01 PM)ZGabrovski@gmail.com Wrote: Why not in Indy 10
What you are proposing is an interface change to the IdSSLOpenSSLHeaders unit. I try not to introduce interface changes if I can help it (though sometimes I do need to make changes).
More so, because Embarcadero bundles Indy with RAD Studio. They just pulled the latest Indy a few months ago for inclusion in RAD Studio 10.4, so they won't be able to pull a new Indy until RAD Studio 10.5 if there are interface changes in it. But, maybe when RAD Studio 10.5 comes around, and we are still on Indy 10, I might sneak something like this into Indy, if I'm inclined to add it at that time.
Posts: 19
Threads: 4
Joined: Sep 2020
Reputation:
0
Location: Bulgaria
Here you are - there is a Pull request created.
Regards,
Zdravko
|