Pierangelo Masarati wrote:
Howard Chu wrote:
Pierangelo Masarati wrote:
Howard Chu wrote:
So, back to the question of libldap's TLS support... The alternate code in HEAD allows multiple TLS implementations to be used at once. The idea here was that a single libldap binary would be able to coexist with multiple apps even if they explicitly used different TLS libraries themselves. In practice I'm not sure that's so important, since probably few apps are aware enough to even make that choice. The other downside of this approach is that the meaning of SSL and SSL_CTX handles in libldap changed (they had to be wrapped so we could insert a tag identifying which implementation went with which handle).
One possibility here is to preserve the old TLS options for OpenSSL only, and make them fail on the other implementations, and introduce new TLS options just for manipulating the generic handles.
Another choice here is to keep the modular layout but still only support one implementation at a time, chosen at compile time. Then we don't need the identifying tag in each session and context handle.
I favor the second option.
One implementation chosen at compile time?
Yes. As you said, it is very unlikely that one needs to use more than one. I do not object to having more than one compiled in, or even more than one usable, but is it really worth the effort?
I suppose since the code was already written, there wasn't much more effort to consider. But without dynamic loading it's probably moot, since the people who care about using non-OpenSSL would never link more than one into their build anyway. So I've stripped all that out and gone with the single approach...
I still see a few warts with the visible implementation; the few functions we added that need an SSL handle (get_strength, get_DN, etc) should not use an SSL handle. We should have never have exposed the underlying SSL/SSL_CTX handles in our own APIs. get_strength and get_dn could just as easily have passed the Sockbuf pointer around instead, and let the library internals find the SSL handles themselves.
Of course, these are all ldap_pvt_*, so we may still decide to change them in this respect.