Proposed Update to support OpenSSL 3.x and earlier versions #529
Conversation
… problem
on Linux where -L option meant that the OpenSSL lib directory given
on the command line was ignored.
…were introduced in 3.2 rather than 3.0
…r static linking.
rlebeau
added
Type: Enhancement
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
| begin | ||
| inherited; | ||
| FFailed := TStringList.Create(); | ||
| FLibraryLoaded := TIdThreadSafeBoolean.Create; |
There was a problem hiding this comment.
You need to initialize FFailedToLoad to False in the constructor. Otherwise it might be accidentally initialized as True and TOpenSSLLoader.Load will fail (and not even try) to load the DLLs.
There was a problem hiding this comment.
@baka0815 The memory for a class object is initialized with zero bytes before the constructor is called. Thus, by default all numeric members are 0, booleans are False, strings are empty, etc.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
Ok, looks like a testing mistake on my end.
The TOpenSSLLoader is only created once and after the loading failed once it won't try to load again. So you need to restart the program if the OpenSSL 3.x DLLs were not present when you first tried to load them
However I'm a fan of explicit initializations, but it then worked for me without it. So this thread can be closed.
There was a problem hiding this comment.
I am not really that happy with the whole strategy for handling load failures. The existing strategy of refusing to load the SSL libraries as soon as any function fails to load follows on from the legacy version. However, it does cause a lot of problems given that different builds select different compile time options - especially with legacy functions such as MD2. I also inherited the one shot load approach.
I would really like to change the whole approach to one where only when certain core functions (such as reading version information) will cause a failure to load. Otherwise, a run time exception is called when and if a function that failed to load is invoked. That way missing functions that are not critical for the calling app do not cause a problem and an explicit error message can focus attention on the problematic function.
This is a relatively simply change to bring in as the update is to the header file generator script. Any views?
There was a problem hiding this comment.
There are other areas of Indy (ie, WinSock, ZLib) that dynamically load using per-function stubs, raising load failures only at the invocation site. I didn't write the OpenSSL support code, but I think at the time it was probably not a good idea for OpenSSL to use such stubs because of the sheer number of functions being loaded, that's probably why a one-time load strategy was used instead. I'm not against switching to stubs, but I can imagine that would severely bloat the loading unit.
There was a problem hiding this comment.
I have created a new branch to my proposed update. This is called "NewLibLoadStrategy". This changes the function loading strategy such if a function cannot be loaded, the call is set to a call to an exception handler unless an appropriate backwards or forwards compatibility function exists. When the call is set to an exception handler, the function name is added to the FailedToLoad list but no exception is generated. it is up to the user to check this list if they need to test if a required (optional) function is present. If they do not then when it is called a run time exception is generated given the name of the missing function.
I have made this a separate branch so that they two load strategies can be compared for userability/bloat, etc.
|
I'm creating my server with the following options: However the connection via Firefox is still established as TLS v1.3 (TLS_AES_1128_GCM_SHA256, 128-Bit-Key, TLS 1.3). Also cURL can connect via |
|
Loading the legacy OpenSSL 1.0.2 DLLs fails for me with the following function not being present: Is this expected or should this be added to |
| {$IFDEF DEBUG} | ||
| if not Result then | ||
| begin | ||
| writeln; |
There was a problem hiding this comment.
This gives me E/A error 105 if I'm running my program as a GUI application.
You might want to use OutputDebugMessage ({$IFDEF Windows}) or check for {$IFDEF CONSOLE}.
There was a problem hiding this comment.
Point taken - all my testing was on console mode apps or Linux...
In the README.OpenSSL.txt I have noted a typo when I said that "SSLOptions.Mode" is ignored. I meant "SLLOptions.Method" is ignored. "SSLOptions.Versions" is now the only means of control over which version is supported. The README does state that this determines in the minimum version only. This was intentional. On thinking about it, perhaps it should also determined the maximum supported. I will update. |
… maximum versions
of the TLS Protocol negotiated for OpenSSLVersions 1.1.0 and later
Both the README and the code have been updated (and tested), and pushed to github. As the update log says "SSLOptions.SSLVersions can now be used to select both the minimum and maximum versions of the TLS Protocol negotiated for OpenSSLVersions 1.1.0 and later. OpenSSL will always try to negotiate the highest version acceptable to both ends of the session." |
|
Thanks for the change @MWASoftware and the work you're putting into this! I will take a look at the changes later on.
Any comments regarding this? |
See NewLibLoadStrategy branch and response to earlier comment. |
This pull request includes work performed by @mezen in developing a version of Indy supporting OpenSSL 1.1.1. This version is intended to overcome two criticisms of @mezen's update: The first is that the proposal for 1.1.1 did not use the existing Indy OpenSSL classes. The second is that Openssl 3.x was not yet supported.
This pull request has taken @mezen's new openssl loader, and intermediate code OpenSSL header files, added Pascal versions of additional OpenSSL header files (e.g. for DES support) and a new code processor (a sed script) to process the intermediate code into usable unit files. With a few mods to IdSSLOpenSSL, the result supports OpenSSL 1.0.2, 1.1.1 and 3.x and probably works with earlier versions - but not tested.
The IdSSLOpenSSL components are unchanged except for the addition of two useful properties:
There are also some low level changes to handle the change to opaque data structures (e.g. SSL_CTX) in 1.1.1 and 3.x.
Note that the IdSSLOpenSSLHeaders.pas and IdSSLOpenSSLHeaders_static.pas units have been removed and have been replaced by the header files in Lib/Protocols/opensslHdrs. The new header files are intended to have a one-to-one correspondence with the OpenSSL header (.h) files.
Three link models are supported.
This version has been tested on both Windows and Linux with both Lazarus/FPC
(3.2.2) and Delphi (Berlin 32 bit only). There are two test programs used for
validation:
Hopefully the result will be found useful.
Please read README.OpenSSL for further information on the update.
NOTES FOR USING THE TEST PROGRAMS
The directory Test/OpenSSL contains the two test programs together with a
README giving more details on their use. They are intended to run "standalone".
That is without having to integrate Indy into the IDE. There are variants for
both Delphi and Lazarus/fpc.
Note: when testing under Windows, if there are no default OpenSSL dlls in the
search path then you will either need to copy libssl and libcrypto into the
project folder or add a command line parameter providing the location of the
OpenSSL dlls.
NOTES FOR LAZARUS/FPC
Please try out the test programs before following the instructions below.
Before trying out the update with your own programs, you should run the script:
These scripts are intended to extract the files relevant to Lazarus/FPC to
another directory and to place the Lazarus package files in the destination
directory itself. By default the scripts extract the files to:
$HOME/Indy (Linux)
"%homedrive%%homepath%"\Indy (Windows)
To override the default, provide the target directory as the only command line
argument for the script. There is no need for the target directory to exist
before running the script.
After running the script, you should read the file README.lazarus-fpc for
further guidance.
Note that you must ensure that any existing Lazarus project that you test this
update with, includes the indyprotocols run time package in the requirements
list.
NOTES FOR DELPHI USERS
Integration with the Delphi IDE is tbd and needs someone to take this on. That is there is a need to update the Delphi package definitions, but otherwise, the update should work with Delphi.