Remove dependancies on IdSSLOpenSSLHeaders and IdSSLOpenSSL units

[rlebeau]

Several units depend directly on the IdSSLOpenSSLHeaders and IdSSLOpenSSL units, namely:

IdAuthenticationNTLM.pas IdDsnRegister.pas IdNTLM.pas IdNTLMv2.pas IdRegister.pas

This means that when new SSLIOHandlers are used, such as the one being worked on for https://github.com/IndySockets/Indy/pull/299, the old TIdSSLIOHandlerSocketOpenSSL is also being used/registered, which causes issues for TIdHTTP for instance (default SSLIOHandler creation, use of TIdAuthenticationNTLM, etc).

In IdCompilerDefines.inc there is a USE_OPENSSL conditional, but it is currently being ignored by some of the above units (the relevant IFDEFs are commented out).

So, this makes it very difficult to upgrade Indy to support OpenSSL 1.1.x/3.0.0 when 1.0.2 is still being depended on in places.

Need to expose new function pointers to abstract away all access to the older IdSSLOpenSSL... units. Maybe add them to the IdFIPS.pas unit, with the existing hash function pointers? Or maybe in another unit would make more sense, perhaps IdSSL.pas?

[CodehunterWorks]

Hello Remy, since I had chased and found the SSL3_STATE.write_mac_secret field issue I know how much complex the TLS implementation is. Therefore my toughts may be sound a little bit easier said than done but I suggest to not integrate TLS 1.1.x and 3.0.0 into the old 1.0.x stuff. It might be better to encapsulate all of them into each individual "universes", selectable by an compiler switch like {$IFDEF USE_TLS_102}, {$IFDEF USE_TLS_111} and {$IFDEF USE_TLS_300}. With a view to 3.0.0 i mean: Sometimes a total demolition and new construction is easier and cleaner than an addition, in particular for the future evolvement of OpenSSL.

[rlebeau]

I never said the new OpenSSL 1.1.x/3.0 stuff was being incorporated into the old 1.0.x stuff. Indeed, it is being kept separate, the new 1.1.x/3.0 stuff is being implemented in a new set of units and SSLIOHandler classes. This ticket is meant for removing some hard-coded dependencies on the older units.

I'll consider the IFDEF approach, but I don't think it will work out, at least initially. AFAIK, the new SSLIOHandler classes are not a drop-in replacement for the old classes, so there will be a transition period where people will have to re-write existing code, so the 2 sets of code will have to co-exist for awhile. Eventually, the old classes will have to be deprecated and then removed. But all of this is moot at the moment since the newer units aren't even finalized and incorporated into the main code yet.

[CodehunterWorks]

Ah ok, then I'd misunderstand you :) Generally I prefer separate Units. And, BTW: I suggest to change the name for the new Handler from TIdSSLIOHandlerSocketOpenSSL to TIdTLSIOHandlerSocket. Simply because we do not have another external crypto but OpenSSL and the "SSL" part is a little bit outdated.

[rlebeau]

TIdSSLIOHandlerSocketOpenSSL is the old OpenSSL 1.0.2 handler. The new OpenSSL 1.1.x/3.0 handler is named TIdOpenSSLIOHandlerClient. Have you even looked at the new code yet?

[MWASoftware]

I noticed the problem with IdAuthenticationNTLM when I was preparing my proposed update (https://github.com/MWASoftware/Indy.proposedUpdate). To solve these dependency issues, I would propose:

1. Moving IdSSL into the "core" package
2. Creating a new OpenSSL package comprising all the IdSSLOpenSSL* units, IdNTLM, idNTMLv2 and IdAuthenticationNTLM.
3. Modifying IdAllAuthentications to remove registration of NTLM.
4. Registration of NTML to be performed when OpenSSL package is included project.

The downside is that existing code will need to be modified to use the new package, The upside is that the dependency tree is cleaner. Note: NTMLv2 uses RC4 and which is deprecated in OpenSSL 3.

[rlebeau]

1. Moving IdSSL into the "core" package

Why? Nothing in the Core package uses SSL/TLS, encryption, or hashing APIs. All of that is in the Protocols package.

2. Creating a new OpenSSL package

I had thought of that before, just hadn't implemented it yet. And it might make sense to have other SSLIOHandler's (ie SChannel, etc) be implemented in their own packages, too.

... comprising all the IdSSLOpenSSL* units, IdNTLM, idNTMLv2 and IdAuthenticationNTLM.

IdNTLM/v2 and IdAuthenticationNTLM would not need to be moved to a new package if their use of encryption/hashing APIs were being properly abstracted.

3. Modifying IdAllAuthentications to remove registration of NTLM.
4. Registration of NTML to be performed when OpenSSL package is included project.

Not necessary if IdAuthenticationNTLM is not moved out of the Protocols package to begin with.

[MWASoftware]

1. Moving IdSSL into the "core" package

Why? Nothing in the Core package uses SSL/TLS, encryption, or hashing APIs. All of that is in the Protocols package.

I proposed this because IdSSL seems to be the only unit in the protocols package that is used by the OpenSSL units. Moving it to "core" would mean that an OpenSSL package would be dependent on "core" and not "protocols" - unless I have missed something of course.

2. Creating a new OpenSSL package

I had thought of that before, just hadn't implemented it yet. And it might make sense to have other SSLIOHandler's (ie SChannel, etc) be implemented in their own packages, too.

... comprising all the IdSSLOpenSSL* units, IdNTLM, idNTMLv2 and IdAuthenticationNTLM.

IdNTLM/v2 and IdAuthenticationNTLM would not need to be moved to a new package if their use of encryption/hashing APIs were being properly abstracted.

Good idea. I would use pascal "interfaces" as the abstraction. BTW, checking through IdNTLMv2: the unit loads the RC2 functions but then I can't find any use of them.

3. Modifying IdAllAuthentications to remove registration of NTLM.
4. Registration of NTML to be performed when OpenSSL package is included project.

Not necessary if IdAuthenticationNTLM is not moved out of the Protocols package to begin with.

Providing an interface to DES in IdSSL would be the best way of leaving it in Protocols IMHO and better than my original proposal.

[rlebeau]

FYI, a new GitHub repo has now been created to separate the OpenSSL logic, most likely into a new package, so it can be updated independently of the main Indy library.

https://github.com/IndySockets/IndyTLS-OpenSSL