OpenSSL library loading fixes. Patch file is attached.  Comments are welcome.

Summary:
   1)  For Windows and OS2, remove DLLSSLNameX and DLLUtilNameX
variables.  Instead use matched arrays of DLL filenames (SSL_DLL_Names
and Crypto_DLL_Names)

   2)  Due to refactoring, remove LoadLibHack or LoadLib functions.

   3)  Remove LoadUtilLibrary function, UnloadSSLLib procedure, and
UnloadUtilLib procedure since these were not being called internally
and are unreachable externally.

   4)  Add new function TryLoadLibPair which attempts to load a
matched pair of DLLs (SSL and Crypto).  If either fails to load,
UnloadLibraries is called to clean up.

   5)  Refactored LoadLibraries to use arrays of names for Windows and
OS2, and to use the new TryLoadLibPair helper function for all
platforms.

On Fri, Jun 26, 2020 at 3:36 PM Michael Van Canneyt
<mich...@freepascal.org> wrote:
>
>
>
> On Fri, 26 Jun 2020, Wayne Sherman wrote:
>
> > On Thu, Jun 25, 2020 at 11:43 PM Michael Van Canneyt
> > <mich...@freepascal.org> wrote:
> >>>
> >>> 3) I recommend the newest libraries be tried first, working backwards
> >>> towards the oldest.
> >>
> >> This is already so ? The version numbers are added from new to old ?
> >
> > The libraries are currently loading in this order:
> >     First attempt:  ssleay32.dll / libeay32.dll (older names, but not 
> > oldest)
> >     Second attempt (util):  libcrypto-1_1-x64.dll (newest name)
> >     Third attempt (ssl):  libssl32.dll (oldest name)
> >     Fourth attempt (ssl):  libssl-1_1-x64.dll (newest name)
>
> Oh, you mean the DLL names.
> I thought you meant the version numbers. Yes, this can be changed too.
>
> >>
> >> This needs to be checked.
> >
> > I ran a test.  If a system has a stray file it can cause mismatched
> > DLLs to load (which could result in strange bugs)
> > Please see attached image "OpenSSL_dll_loading_screenshot.png"
>
> We'll adapt the mechanism.
>
> >> I have changed that to 4.0
> >
> > From:  https://www.openssl.org/policies/releasestrat.html
> >    openssl versions 1.1.0, 1.0.2, 1.0.1, 1.0.0 and 0.9.8 are no
> > longer supported
>
> Maybe, but for backwards compatibility they must be supported in FPC.
>
> >
> > So can Windows openssl v1.1 library loading be included in the next
> > maintenance release (v3.2.1?) instead of waiting for v4.0?
>
> No worries, 4.0 means we can consider it for the next fix release.
> That will be 3.2.2 (even numbers only for releases, odd indicates
> development sources)
>
> >
> > Would you like me to have a go at a revised patch?
>
> Sure. If you create a patch, upload it to the bugtracker and assign it to me
> (or mail here so I can have a look)
>
> Michael.
> _______________________________________________
> fpc-devel maillist  -  fpc-devel@lists.freepascal.org
> https://lists.freepascal.org/cgi-bin/mailman/listinfo/fpc-devel
Index: packages/openssl/src/openssl.pas
===================================================================
--- packages/openssl/src/openssl.pas	(revision 45716)
+++ packages/openssl/src/openssl.pas	(working copy)
@@ -84,30 +84,24 @@
 {$ENDIF OS2}
   DynLibs, cTypes, SysUtils;
 
-var
-  {$IFDEF WINDOWS}
-  DLLSSLName: string = 'ssleay32.dll';
-  DLLSSLName2: string = 'libssl32.dll';
-  DLLSSLName3: string = {$IFDEF WIN64}'libssl-1_1-x64.dll'{$ELSE}'libssl-1_1.dll'{$ENDIF};
-  DLLUtilName: string = 'libeay32.dll';
-  DLLUtilName2: string = {$IFDEF WIN64}'libcrypto-1_1-x64.dll'{$ELSE}'libcrypto-1_1.dll'{$ENDIF};
-  {$ELSE}
-   {$IFDEF OS2}
-    {$IFDEF OS2GCC}
-  DLLSSLName: string = 'kssl10.dll';
-  DLLUtilName: string = 'kcrypt10.dll';
-  DLLSSLName2: string = 'kssl.dll';
-  DLLUtilName2: string = 'kcrypto.dll';
-    {$ELSE OS2GCC}
-  DLLSSLName: string = 'emssl10.dll';
-  DLLUtilName: string = 'emcrpt10.dll';
-  DLLSSLName2: string = 'ssl.dll';
-  DLLUtilName2: string = 'crypto.dll';
-    {$ENDIF OS2GCC}
-   {$ELSE OS2}
-  DLLSSLName: string = 'libssl';
-  DLLUtilName: string = 'libcrypto';
-
+const
+// SSL and Crypto DLL arrays must have the same length and contain
+// matched pairs of DLL filenames. Place newer versions at the beginning.
+{$IFDEF WIN64}
+  SSL_DLL_Names:    array[1..3] of string = ('libssl-1_1-x64.dll',    'ssleay32.dll', 'libssl32.dll');
+  Crypto_DLL_Names: array[1..3] of string = ('libcrypto-1_1-x64.dll', 'libeay32.dll', 'libeay32.dll');
+{$ELSEIF DEFINED(WINDOWS)}
+  SSL_DLL_Names:    array[1..3] of string = ('libssl-1_1.dll',    'ssleay32.dll', 'libssl32.dll');
+  Crypto_DLL_Names: array[1..3] of string = ('libcrypto-1_1.dll', 'libeay32.dll', 'libeay32.dll');
+{$ELSEIF DEFINED(OS2GCC)}
+  SSL_DLL_Names:    array[1..2] of string = ('kssl10.dll',   'kssl.dll');
+  Crypto_DLL_Names: array[1..2] of string = ('kcrypt10.dll', 'kcrypto.dll');
+{$ELSEIF DEFINED(OS2)}
+  SSL_DLL_Names:    array[1..2] of string = ('emssl10.dll',  'ssl.dll');
+  Crypto_DLL_Names: array[1..2] of string = ('emcrpt10.dll', 'crypto.dll');
+{$ELSE}
+  BaseSSLName: string = 'libssl';
+  BaseCryptoName: string = 'libcrypto';
   { ADD NEW ONES WHEN THEY APPEAR!
     Always make .so/dylib first, then versions, in descending order!
     Add "." .before the version, first is always just "" }
@@ -115,13 +109,12 @@
                                         '.1.0.2', '.1.0.1','.1.0.0','.0.9.8',
                                         '.0.9.7', '.0.9.6', '.0.9.5', '.0.9.4',
                                         '.0.9.3', '.0.9.2', '.0.9.1');
-   {$ENDIF OS2}
-  {$ENDIF WINDOWS}
+{$ENDIF}
 
 const
   // EVP.h Constants
 
-  EVP_MAX_MD_SIZE               = 64; //* longest known is SHA512 */
+  EVP_MAX_MD_SIZE       = 64; //* longest known is SHA512 */
   EVP_MAX_KEY_LENGTH    = 32;
   EVP_MAX_IV_LENGTH     = 16;
   EVP_MAX_BLOCK_LENGTH  = 32;
@@ -4755,42 +4748,6 @@
     _OPENSSLaddallalgorithms;
 end;
 
-{$IFNDEF WINDOWS}
- {$IFNDEF OS2}
-{ Try to load all library versions until you find or run out }
-function LoadLibHack(const Value: String): HModule;
-var
-  i: cInt;
-begin
-  Result := NilHandle;
-
-  for i := Low(DLLVersions) to High(DLLVersions) do begin
-    {$IFDEF DARWIN}
-    Result := LoadLibrary(Value + DLLVersions[i] + '.dylib');
-    {$ELSE}
-    Result := LoadLibrary(Value + '.so' + DLLVersions[i]);
-    {$ENDIF}
-
-    if Result <> NilHandle then
-      Break;
-  end;
-end;
- {$ENDIF OS2}
-{$ENDIF WINDOWS}
-
-function LoadLib(const Value: String): HModule;
-begin
-  {$IFDEF WINDOWS}
-  Result := LoadLibrary(Value);
-  {$ELSE WINDOWS}
-   {$IFDEF OS2}
-  Result := LoadLibrary(Value);
-   {$ELSE OS2}
-  Result := LoadLibHack(Value);
-   {$ENDIF OS2}
-  {$ENDIF WINDOWS}
-end;
-
 Function CheckOK(ProcName : string ) : string;
 
 
@@ -5201,19 +5158,6 @@
   _BN_free:=GetProcAddr(SSLUtilHandle,'BN_free');
 end;
 
-Function LoadUtilLibrary : Boolean;
-
-begin
-  Result:=(SSLUtilHandle<>0);
-  if not Result then
-    begin
-    SSLUtilHandle := LoadLib(DLLUtilName);
-    Result:=(SSLUtilHandle<>0);
-    end;
-end;
-
-
-
 Procedure ClearSSLEntryPoints;
 
 begin
@@ -5373,26 +5317,6 @@
   _BN_free:=nil;
 end;
 
-Procedure UnloadSSLLib;
-
-begin
-  if (SSLLibHandle<>0) then
-    begin
-    FreeLibrary(SSLLibHandle);
-    SSLLibHandle:=0;
-    end;
-end;
-
-Procedure UnloadUtilLib;
-
-begin
-  if (SSLUtilHandle<>0) then
-     begin
-     FreeLibrary(SSLUtilHandle);
-     SSLUtilHandle := 0;
-     end;
-end;
-
 Procedure ClearUtilEntryPoints;
 
 begin
@@ -5621,32 +5545,46 @@
   end;
 end;
 
+function TryLoadLibPair(const SSL_DLL_Name, Crypto_DLL_Name: string):boolean;
+begin
+  Assert((SSLUtilHandle = 0) and (SSLLibHandle = 0),
+    'LoadTryLoadLibPair: Handle is not zero');
+
+  SSLUtilHandle := LoadLibrary(Crypto_DLL_Name);
+  if (SSLUtilHandle <> 0) then
+    SSLLibHandle := LoadLibrary(SSL_DLL_Name);
+
+  Result := (SSLUtilHandle <> 0) and (SSLLibHandle <> 0);
+  if not Result then UnloadLibraries;
+end;
+
 Function LoadLibraries : Boolean;
-
+var
+ Idx: Integer;
 begin
   Result:=False;
-{$IFDEF DARWIN}  
+
+{$IF DEFINED(WINDOWS) OR DEFINED(OS2)}
+  Assert(Low(SSL_DLL_Names) = Low(Crypto_DLL_Names));
+  Assert(High(SSL_DLL_Names) = High(Crypto_DLL_Names));
+  for Idx := Low(SSL_DLL_Names) to High(SSL_DLL_Names) do begin
+    Result := TryLoadLibPair(SSL_DLL_Names[Idx], Crypto_DLL_Names[Idx]);
+    if Result then EXIT;
+  end;
+{$ELSE}
+  {$IFDEF DARWIN}
   // Mac OS no longer allows you to load the unversioned one. Bug ID 36484.
-  DLLVERSIONS[1]:=DLLVERSIONS[2];
+  for Idx := Low(DLLVersions)+1 to High(DLLVersions) do begin
+    Result := TryLoadLibPair(BaseSSLName + DLLVersions[Idx] + '.dylib',
+      BaseCryptoName + DLLVersions[Idx] + '.dylib');
+  {$ELSE}
+  for Idx := Low(DLLVersions) to High(DLLVersions) do begin
+    Result := TryLoadLibPair(BaseSSLName + '.so' + DLLVersions[Idx],
+      BaseCryptoName + '.so' + DLLVersions[Idx]);
+  {$ENDIF}
+    if Result then EXIT;
+  end;
 {$ENDIF}
-  SSLUtilHandle := LoadLib(DLLUtilName);
-  SSLLibHandle := LoadLib(DLLSSLName);
-  {$IFDEF MSWINDOWS}
-  if (SSLUtilHandle = 0) then
-    SSLUtilHandle := LoadLib(DLLUtilName2);
-  if (SSLLibHandle = 0) then
-    SSLLibHandle := LoadLib(DLLSSLName2);
-  if (SSLLibHandle = 0) then
-    SSLLibHandle := LoadLib(DLLSSLName3);
-  {$ELSE MSWINDOWS}
-   {$IFDEF OS2}
-  if (SSLUtilHandle = 0) then
-    SSLUtilHandle := LoadLib(DLLUtilName2);
-  if (SSLLibHandle = 0) then
-    SSLLibHandle := LoadLib(DLLSSLName2);
-   {$ENDIF OS2}
-  {$ENDIF MSWINDOWS}
-  Result:=(SSLLibHandle<>0) and (SSLUtilHandle<>0);
 end;
 
 function InitSSLInterface: Boolean;
_______________________________________________
fpc-devel maillist  -  fpc-devel@lists.freepascal.org
https://lists.freepascal.org/cgi-bin/mailman/listinfo/fpc-devel

Reply via email to