-
Notifications
You must be signed in to change notification settings - Fork 154
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
Added new OpenSSL IO Handler for OpenSSL 1.1.1 #299
Conversation
Just an FYI, when |
My last commit ensures that with the |
Modified source for OpenSSL 1.1.1 Compile and tested with: Delphi 7 + Win XP sp2 |
Very impressive work. It is working by me on XE6, Delphi 10.2, 10.3, 10.4. To avoid update Indy and avoid changes in Indy IdGlobal.pas and Idctypes.pas, I've added new IdOpenSSL_IdCTypesEx.pas with missing consts and types. To make it working in Linux (tested on Raspbian) in Lazarus, I had to edit / replace SafeLoadLibrary with HackLoad in IdOpenSSLLoader.pas
and add constants to IdOpenSSLConsts.pas CLibCryptoLinuxRaw = 'libcrypto'; |
Why would you want to avoid that, if that is where they belong? What exactly did you add that needed to be put into its own unit? |
Normally I would do it as You do, but I can't upgrade / change Indy source in all versions of Delphi I have. Because of that, I made a separate IdOpenSSL_IdCTypesEx.pas in which the missing types and procedures are added, and therefore I don't need to change and recompile Embardadero\Studio\xx.0\source\Indy10. I have also added IdOpenSSL_IdCTypesEx.pas to "uses" in units, where needed. IdOpenSSL_IdCTypesEx.zip Updated 18.8.2020 |
Any ETA for the official merge to master? |
I'm sorry, I have no experience with Git at all, but if You're interested, I can provide You changed source (zipped). |
I needed to make some changes to the code for FPC (v3.3.1):
I should update this PR? |
And on Linux:
Also, there's a number of getProcAddress that fail on the standard lib |
Failed to load list below. These are pretty long lists that will generate lots of confusion. Should we $IFDEF these out? Windows + Linux (ubuntu 20): Windows only: linux only: |
@mezen This doesn't appear to work as a server for TIdHTTPServer - any connection generates 'error:140940F4:SSL routines:ssl3_read_bytes:unexpected message'. The server set up code is
It doesn't matter what options I set. I think that that SSL negotiation is working ok - if I set to verify the client certificate that part works ok |
OpenSSL documentation: "Note, however, that these implementations are not available on all platforms." Think it's not a major problem as all of them are not needed by Indy code thus the programmer should check if it was loaded before using it in own code. |
Agree that it's not a major problem but the problem is that routine practice in the old code is to check FailedToLoad and raise an exception if it's not empty. That fact that this list is not empty normally will generate a stream of questions in all the forums |
@mezen dsa_st and rsa_st are defined in both IdOpenSSLHeaders_ossl_typ and IdOpenSSLHeaders_evp and the definitions of EVP_PKEY_get0_RSA etc appear wrong - should be prsa not prsa_st etc |
It appears that the definitions of EVP_PKEY_set1_RSA and EVP_PKEY_set1_DSA are wrong: from openSSL doco: int EVP_PKEY_set1_RSA(EVP_PKEY *pkey, RSA *key); proposed pascal code: EVP_PKEY_set1_RSA: function(pkey: PEVP_PKEY): TIdC_INT cdecl = nil; |
Also this code should be in the loader (see attachment) (uses IdGlobal, IdCTypes, IdFIPS, IdOpenSSLHeaders_ossl_typ, IdOpenSSLHeaders_rsa, IdOpenSSLHeaders_dsa, IdOpenSSLHeaders_bn, IdOpenSSLHeaders_bio, IdOpenSSLHeaders_hmac, IdOpenSSLHeaders_pem, IdOpenSSLHeaders_err, IdOpenSSLHeaders_x509, IdOpenSSLHeaders_evp, IdOpenSSLHeaders_crypto, IdOpenSSLVersion) |
And the types are wrong here: EVP_DigestSignInit: function(ctx: PEVP_MD_CTX; pctx: PPEVP_PKEY_CTX; const &type: PEVP_MD; e: PENGINE; pkey: PEVP_PKEY): TIdC_INT cdecl = nil; |
Ok, now some feedback came in, so one after the other: @JedrzejczykRobert I have implemented most of your changes @xjikka also adopted @grahamegrieve here it gets a bit longer
|
your header:
where is the RSA Key? You can't use this function without this parameter, and I don't understand your comment. This is the correct header that actually makes signing possible:
the openSSL doco:
your header:
Correct header:
It's the type of the first parameter |
And @mezen you did not add the fips loading routines? |
@grahamegrieve AFAIK, FIPS support is not available in OpenSSL 1.1.x, that will come in OpenSSL 3.0. https://www.openssl.org/docs/fips.html |
Well, at least some of it is working for me now. I use and test: IsHMACAvail := OpenSSLIsHMACAvail; |
I've been testing the proposed patch out under Linux/Lazarus/fpc. It seems to work, with the following comments:
Lib/Protocols/OpenSSL Lib/Protocols/OpenSSL/dynamic directories, and adding to the include path /Lib/FCL These are missing from indylaz.lpk hence have to be added manually to the project.
Otherwise, all works well and impressed by the work. |
That is a legacy folder, Indy stopped using that folder many years ago. You don't need to refer to it.
Can you report that to https://github.com/mezen/Indy/issues, or maybe even fix it and push it to https://github.com/mezen/Indy/pulls ? |
The problem I had here was to get a command line program to compile. That is, I was not using Lazarus, and anyway indylaz.lpk has not been updated to include the OpenSSL directories. I thus had to guess which unit and include directories to use. All of the files in the OpenSSL directory include the directive {$i IdCompilerDefines.inc}. However, this include file is not within the OpenSSL directory hierarchy. A quick "find" found four occurrences: /Lib/FCL/IdCompilerDefines.inc I guessed the first. Indylaz.lpk includes the line which means that ./Lib/Core/IdCompilerDefines.inc ends being used in all cases. Hence, you are saying that the include path should be Is that correct? |
Five, actually.
And also
That is fine, as all 5 copies have the same content.
Actually, no. For FPC/Lazarus, generally one copies all of Indy's source files into a single folder before then compiling Indy. |
I have raised an issue. I also want suggest some fixes to make the patch more Lazarus/FPC friendly and have forked this repo to MWASofware/Indy. Branch NewOpenSSL_PR_FPC has been added with the proposed compile time fix. I hope to add some build files later. |
Any progress on this so far? |
I cannot speak for anyone else and have no idea whether @mezen has done any further work, but in absence of anyone else appearing to step forward I have been working on my own solution. It all looks very encouraging and I hope to release the first version this week, Only final readme's and packaging are needed before I press the button. While @mezen's work was very useful, I had two criticisms of it, which is why I decided to go down my own path. 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. I have taken @mezen's new openssl loader, and intermediate code OpenSSL header files, added a couple more header files and written a new code processor (a sed script) to process the intermediate code into usable unit files. With a few mods to IdSSLOpenSSL, I now have a working set of code supporting:
I have 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. |
I follow this discussion since a very long time. Thanks to all who had worked on OSSL3 support! But it is all senseless if it will not be merged into the main branch. Most users wil not use an "hacked" version in any productive environment. Or with the words from our chief developer: Only the GetIt version is on the build server. |
That is really great! Are you planning to merge it with the master Indy repository? |
My main development environment is Lazarus/fpc under Linux and I have worked on an extract of the Indy source tree better presented for Lazarus. I now need to merge the files back into the main source tree and test. I have also split the Lazarus package into separate design and runtime packages to better support console mode applications. However, apart from testing the source code with Delphi (both test apps seem to be work OK) I have not and probably will not do any work on updating the Delphi packages. Someone else, with a Delphi licence will need to do this. |
Link to almost latest statement of @mezen
many people has his own solutions, some "hack" solutions too... it's not the best way but we have no choice cause openssl eol
we're all (i guess anyone here) is waiting for it... |
The main problem with using the existing Indy OpenSSL components is that they are tied to OpenSSL 1.0.2. There were major API changes/breakages in OpenSSL 1.1, making it hard to use the existing code for future versions. If you found a way around that, then good on you! I would personally prefer that the existing components be updated to handle newer OpenSSL versions without requiring users to re-write their existing code. Not to put down @mezen's work, because he did a lot of great work, but its such a massive PR that I just never got to review it all, which is why it never got merged.
Would that require me/Indy users to maintain/use that script whenever Indy is recompiled? Or when OpenSSL is updated? Indy's already difficult enough to update/reinstall, I'm just wondering what extra work this will add?
That sounds cool. My only real concern to to make sure
That I'm a little concerned about. I know using CryptoAPI/SChannel is a hot item for Windows users (issue #49), but CryptoAPI is a completely separate API from OpenSSL that it really doesn't belong in the OpenSSL components, it should be kept separate. Unless you are referring to OpenSSL's own use of CryptoAPI on Windows?
Keep in mind that Indy also supports Android and iOS in Delphi, as well as quite a few other platforms in FPC. So any new OpenSSL work will have to be usable in a wide array of platforms that OpenSSL supports. |
Thats why we use Indy instead of Wininet components. We wont bound us to this proprietary piece and we do not trust it.
So please make sure that OpenSSL and Wininet use is never mingled in Indy. We want to have the control to decide which backend is used in our products. |
Currently I am not working on this PR anymore.
and for the reboot of a new PR
Unfortunately, OpenSSL 1.1.1 has since gone EOL without me being able to complete my work beforehand. Time, the most precious commodity and always in short supply. My biggest obstacle is actually the header translations. The original translations using OpenSSL 1.0.2 (and before) were very selective and only what was necessary was translated. My latest approach is no longer a manual translation, but a technical one. However, all popular automatic translation tools failed due to the complexity of the OpenSSL headers, so I started to write my own tool. Knowing that it will never be able to translate everything, but if it could do 95% of the work for me, it would be a great help. And indeed, this is my current status: the tool is not yet finished.
OpenSSL 3.2 has builtin support for the Windows system certificate store, so no big import of dependancies are needed.
(source Openssl 3.2 release notes) @MWASoftware To save time, I have even put the dynamic loading of functions on the side in the current approach. |
@mezen I'd like to volunteer my assistance in this effort, if you are interested and it would be helpful. I have no particular experience with OpenSSL at this low level but it sounds like what is needed is work on the header translations. If I can help I would like to do so, please let me know. |
I have now published the proposed update to https://github.com/MWASoftware/Indy.proposedUpdate |
The only time the script needs to be run is when the OpenSSL header files are updated. Otherwise, no one needs to touch the script. |
I don't see any OpenSSL dependencies in IdFIPS.pas. But then again, I am not a .dot net user. IdSSLOpenSSLHeaders_static.pas has been superseded by embedding the static headers as a conditional compilation part of each header file. Let me know if there is a problem that I had not spotted. |
There is an open ticket (#9) for using
This has nothing to do with .NET. |
Looks like I missed that one. Will investigate. |
I can see what needs doing. Easy enough to add back in the initialisation of the functions in IdFIPS. Will update my repo. |
OpenSSL version 3.3 alpha 1 was released today. I have downloaded a copy and compiled it on 64-bit Linux and did a quick test of my test client and server. All seems OK. The programs gave the expected result. I am thus reasonably confident that my proposed update will support OpenSSL 3.3. |
I have updated the proposed update to include FIPS support using OpenSSL. There is a new unit IdSSLOpenSSLFIPS contained mostly the original code from the OpenSSLHeaders unit, but modified to work with OpenSSL 3 as well as earlier versions. See the README.OpenSSL.FIPS for more information. |
I have not had any feedback since I made the update available. It would be useful to know if my proposed update is of any use or whether I should just keep it to myself. Comments both good and bad would be appreciated. |
I have added a simple solution to the proposed update for delayed loading - but hopefully sufficient. In this update, if Indy is compiled with the OPENSSL_USE_DELAYED_LOADING defined symbol then the static loading of a shared library link model is implied and each OpenSSL library external declaration includes the "delayed" directive. This should result in each such function only being loaded immediately before it is first used. |
I don't have a problem with delay-loading functions at runtime. There are several units in Indy which do that. However, Indy 10 still supports Delphi versions all the way back to Delphi 5. And it supports FreePascal. Also, there are several areas of Indy where it can report which imports actually failed to load at runtime, and doing that with That's the main reasons why Although, I have been thinking about changing |
That's why I made "delayed" a conditional compilation and put in a trap so that it gives a meaningful error message if you try to compile with FPC. Probably should add a similar one for old versions of Delphi.
Looks like you need a framework for handling the callback.
You seem to be saying that Indy should implement its own version of delay loading - certainly more generic. Doable, especially when using a code generator for the header files but whether you can extend this throughout Indy is more problematic. The need for OpenSSL 3 is much more pressing that a minor optimisation of library load.
That would be how you would do it with a code generator. The code generator creates a stub for each function which is called the first time user code calls a given function. The stub then loads the actual function address, replaces the function variable with the newly loaded function address and calls it. |
@mezen and @MWASoftware (and anyone else who is interested in contributing to this effort) - there is a new joint initiative with Embarcadero and some other 3rd party devs to get Indy updated with OpenSSL. Please contact me privately if you are interested in joining in. https://www.indyproject.org/2024/08/05/ongoing-work-in-indy-for-openssl-updates/ |
Complete new io handler which supports OpenSSL 1.1.1. The existing io handler for OpenSSL 1.0.2 is untouched and should still work.
Only IdCTypes.pas and IdGlobal.pas are modified (the last files in the diff): I added some more types.
The Binaries from http://wiki.overbyte.eu/wiki/index.php/ICS_Download#Download_OpenSSL_Binaries_.28required_for_SSL-enabled_components.29 are used, but at the beginning I tested with slWebPro.
The complete io handler is written with Delphi Berlin 10.1.2, I tried to support older Delphi versions, but due lack of Installations I havent tested it.
Also only Win32 and Win64 are tested, no MacOS, no mobile.
Test are done in small test project and a big (three tier) real world application with tcp server/client, smtp/imap/pop client, ftp client and http client.
This should fix IndySockets/IndyTLS-OpenSSL#20, IndySockets/IndyTLS-OpenSSL#18 (ok, no event, but virtual method for overriding), IndySockets/IndyTLS-OpenSSL#17 (not on purpose for this issue, but for myself - found out later that issue exists^^) and of course IndySockets/IndyTLS-OpenSSL#10.