jens-maus / amissl

:closed_lock_with_key: AmiSSL is the AmigaOS/MorphOS/AROS port of OpenSSL. It wraps the full functionality of OpenSSL into a full-fledged Amiga shared library that makes it possible for Amiga applications to use the full OpenSSL API through a standard Amiga shared library interface (e.g. web browsers wanting to support HTTPS, etc.)...
Apache License 2.0
88 stars 15 forks source link

AmiSSL version defines mismatches #64

Open patrikaxelsson opened 2 years ago

patrikaxelsson commented 2 years ago

For AmiSSL 4.0-4.2, AMISSL_V11x is defined as 0x06.

For AmiSSL 4.3+, AMISSL_V11x is changed to 0x07 and is claimed to be AmiSSL 4.0 and 0x08 is claimed to be AmiSSL 4.1, which reasonably is not correct given the 4.0-4.2 includes states they both are 0x06.

Additionally, to successfully use AmiSSL 4.0-4.2, InitAmiSSLMaster() must be called with version 0x06, else it fails.

See: https://github.com/jens-maus/amissl/blob/4.0/include/libraries/amisslmaster.h#L10 https://github.com/jens-maus/amissl/blob/4.1/include/libraries/amisslmaster.h#L10 https://github.com/jens-maus/amissl/blob/4.2/include/libraries/amisslmaster.h#L10 https://github.com/jens-maus/amissl/blob/4.3/include/libraries/amisslmaster.h#L11 https://github.com/jens-maus/amissl/blob/5.3/include/libraries/amisslmaster.h#L35

So when specifying AMISSL_V11x, I would according to a current amisslmaster.h expect to be able to be able to use AmiSSL 4.0+, but I will instead need AmiSSL 4.3+.

Futaura commented 2 years ago

I'm not sure what you mean by "to successfully use AmiSSL 4.0-4.2, InitAmiSSLMaster() must be called with version 0x06, else it fails". Which version of amisslmaster.library is installed when it fails?

Actually, if you specify 0x06, that doesn't mean you get 4.0 - any newer compatible version will be opened instead. That means it tries to open 4.12 first, then 4.11 and so on down to 4.0.

The reason I moved away from a constant AMISSL_V11x is because, for example, if a program compiled with the 4.2 SDK and used newly added functions in 4.2 (from OpenSSL 1.1.0e), that program would crash on any system with 4.0 (OpenSSL 1.1.0c) or 4.1 (OpenSSL 1.1.0d) installed and not 4.2. It became a bigger issue with 4.3 (OpenSSL 1.1.1a) because that is when TLS 1.3 was first supported and programs built with 4.3 were even more likely to crash on systems with only 4.0-4.2 installed.

The only solution was to move back to the AmiSSL v3 concept and ensure every AmiSSL release gets a unique version number, when the OpenSSL version was updated. That way amisslmaster.library can always correctly choose the correct compatible version. Backwards compatibility was not affected by these changes.

Really, it is only safe to use AMISSL_CURRENT_VERSION - I often think about removing AMISSL_V11x, and similar, completely. It was originally left in for backwards compatibility. It should maybe be removed from AmiSSL v5 because it would be wrong to specify AMISSL_V11x when using the AmiSSL v5 SDK (you don't want to compile an application for OpenSSL 3.0 and end up opening OpenSSL 1.1.x - it will probably crash!).

patrikaxelsson commented 2 years ago

I'm not sure what you mean by "to successfully use AmiSSL 4.0-4.2, InitAmiSSLMaster() must be called with version 0x06, else it fails". Which version of amisslmaster.library is installed when it fails?

I was using a separate install of AmiSSL 4.0, 4,1 or 4.2. So no combo-install in that case and those versions of amisslmaster.library. I am guessing that a newer (4.3+) amisslmaster.library would give you the corresponding amissl.library for AmiSSL 4.0 if you call InitAmiSSLMaster() with version 0x07?

Anyway, what I am getting at is that if you only have AmiSSL 4.0 with its old amisslmaster.library you can't call InitAmiSSLMaster() with version 0x07, but need to use 0x06.

Maybe this is an irrelevant issue, don't know why someone would want to be using these old AmiSSL versions. For some background: the functionality I need (basic http/https/gemini client), every function was already present in OpenSSL 1.1.0d of AmiSSL 4.0, so it actually works the whole span from 4.0-5.3 without recompiling... if I call InitAmiSSLMaster with version 0x06 of course to get the 4.0-4.2 installations working. Walking a thin line perhaps :D.

Futaura commented 2 years ago

Ok, I understand your issue now - it is a pretty complex subject. If you want to maintain a minimum AmiSSL version requirement in any application, which is fine, it is always recommended to use the SDK from that version (same as AMISSL_CURRENT_VERSION). So, that means using the AmiSSL 4.0 SDK for you. As you say, this means your application will then be able to use 4.0-4.12 depending on what the user has installed (it will never use 5.1+ though). That said, I've been guilty of specifying AMISSL_v111d to InitAmiSSLMaster() when using a newer SDK, in IBrowse. It is kind of safe to do that, albeit not recommended, if you can be sure nothing in the newer SDK will cause the application to crash with an older AmiSSL version. A prime example is when the OpenSSL team replace an older function with a macro redirecting to a new function - this would cause an application to crash if you passed an older version than AMISSL_CURRENT_VERSION, even though you made no changes to your own code.

I perhaps should have left AMISSL_V11x set to 0x06, but I changed that to AMISSL_V111c and then AMISSL_V111d to force those as minimum versions. For AmiSSL v5, I think it is going to be best to phase AMISSL_V11x out completely (remove it or define it as AMISSL_CURRENT_VERSION instead) as it is completely unsafe to specify a v4 version to InitAmiSSLMaster() using the v5 SDK.

I understand Jens' original intention to use AmiSSL_V11x to load any available 4.x version, due to the OpenSSL ABI remaining backwards compatible in OpenSSL 1.1.x. It would have made things easier to update and a little cleaner. However, as indicated in my previous reply, it gave amisslmaster.library no power to pick and choose specific 4.x versions. This was a critical oversight as it was required to handle what happens when applications use newer OpenSSL functions added to newer 4.x versions. For example, if you were to use an newer OpenSSL tool with only older AmiSSL libraries installed, which would overwise crash. It is similar to AmigaOS - if you want to use newer functions available in a V47 library, you must ask for V47 as a minimum, not V40 (if using AmiSSL 4.0-4.2 approach, even if you asked for V47, the OS would allow V40 to be opened instead if V47 was not present - a certain crash if using functions added to V47). Changing the version numbering in 4.3 was the only real option.

The flipside of the applications maintaining an old minimum version of AmiSSL is that if the user doesn't bother to update AmiSSL they miss out on OpenSSL bugfixes and other AmiSSL improvements. For example, 4.3 was a big step for 68060 users and it also added TLS 1.3 support - it would probably be a good idea to force 4.3 as a minimum (well, 4.4 now due to mistakes I unfortunately made with 4.3!). When applications are updated, if they are always built with the latest AmiSSL SDK, not only does it mean that application gets to use the latest AmiSSL, all other existing applications built using older versions will automatically get upgraded to the latest AmiSSL too (that applies to 4.0-4.12 separately to 5-1-5.3 of course).

I could probably write a whole essay on more pros and cons, but ultimately it is best to always use AMISSL_CURRENT_VERSION and if you want to keep an old minimum version, always use the matching SDK version. If you want an application to use 4.0-4.12, there is no need to update the SDK to anything newer than 4.0. If you want to use 5.1, you'll need to recompile using the 5.1 SDK.

patrikaxelsson commented 2 years ago

Thanks for going into details, very interesting. I would indeed say that I am walking a thin line then with no functions being turned into macros or changed otherwise. So I started using the 4.12 SDK in my project and still is, also when using 5.x 😁. To use 5.x, I do like this:

  1. OpenLibrary("amisslmaster.library", 4).
  2. If amisslmaster.library is greater than v4 then call InitAmiSSLMaster() with version 0x15 (5.1), else 0x06 (4.0).

Apart from that, my code is the same as when I only used 4.12 and this works, but I guess then only because I am lucky that the OpenSSL functions I am using have not changed:

Futaura commented 2 years ago

Aha, that's quite clever - hope you're using SSL_free() too :wink:. Just as well I didn't really mess with the <= 4.12 library jump table for 5.1, in terms of not moving functions about, and left the deprecated OpenSSL stuff in. Deprecated function entry points in the jump table will still work and use the newer function. This was with the full intention of originally allowing applications built with 4.x to auto upgrade to 5.x, as usual - IBrowse worked fine like that during testing. I had to abandon that idea mainly due to some changes in the public OpenSSL structures and a few changes in existing API functions (int32 changing to int64, for example). I probably could have patched around everything, but it would become a nightmare to maintain, plus I could never be 100% sure that I had patched everything (due to the heavy macro usage in the OpenSSL includes that define certain functions and structures). There is also the fact that InitAmiSSLMaster() is deprecated too in v5 (actually, the new OpenAmiSSLTags() uses it internally :smile:). Probably you will be safe to continue like this, at least until OpenSSL 3.1.