RFC: Adding Mac support to secur32/schannel.c

[Ken Thomases]

Hi Juan,

On Jan 28, 2011, at 10:36 AM, Juan Lang wrote:

>> I'm planning to add an alternative implementation of schannel (SSL/TLS) support for the Mac.  The current implementation is based on GnuTLS.  That library is not typically found on Mac OS X.  Although packagers can build it and ship it and its dependencies with Wine for Mac OS X, I think it's better (especially for security-related functionality) to use the system-provided library.
> 
> What's the issue with building GnuTLS?  Is it that GnuTLS doesn't
> support the Mac Keychain?  Is it that it's an external dependency?

As Henri said, it's that it's a set of external dependencies (not just one; GnuTLS has its own dependencies) and that they are security-related.  To the greatest extent practical, security-related libraries should come from one's distro or OS vendor.

> If the latter, we already pull in quite a bit that isn't found on the
> Mac, so the incremental change isn't large.

Well, I'd prefer to reduce those other external dependencies, too.  But that's a fight for another day. ;)


> One minor nit:  s/adaptor/adapter/

Well, my dictionary shows both as correct, although "adapter" is apparently preferred in American English.  Thanks.


>> I plan to introduce a new internal interface (the schan_imp_* stuff in my patch) and incrementally refactor the code to hide uses of GnuTLS behind that interface.  Then, I'm thinking of breaking the GnuTLS implementation out into a separate module, schannel_gnutls.c.  Then, I'd add a second implementation module, schannel_mac.c, based on the Mac Secure Transport API (as shown in the patch).  Each of the two modules would be made "empty" by preprocessor conditionals, as appropriate.
>> 
>> I'd appreciate a review of this general plan.
> 
> I do see two problems with the general plan.  One isn't specific to
> your plan:  schannel as it is is buggy.  We don't know where the bugs
> are, and they've languished for a long time.  Your proposed plan
> doesn't help.

Well, it doesn't help make schannel less buggy, but it doesn't aim to.  However, it does help Macs without GnuTLS (the default) go from a completely non-functional schannel to a merely buggy schannel.

> The second is the additional bug triaging complexity.
> 
> I'd much prefer to see a decent set of test cases for schannel.  I
> suppose one could argue that that's orthogonal to this plan, but see
> my second complaint:  without test cases, an additional implementation
> just makes debugging schannel bugs harder.

Well, if schannel were under active development or were soon to be, then I'd hold off on adding a second implementation.  However, that doesn't seem to be the case.

As to making debugging harder, I think the impact is pretty minor.  The internal schan_imp_* interface is pretty close to GnuTLS.  There's not much of the schannel logic -- the stuff that's potentially buggy -- that ends up behind that interface.  It mostly consists of making the GnuTLS call, translating its results to Win32 error codes, and logging errors with gnutls_perror().  Frankly, in several respects, I think that moving some of the GnuTLS-using code into smaller, more narrowly focused functions improved the code quite aside from enabling a second implementation.

Admittedly, as schannel is developed, the internal interface may need to grow.  And having a second implementation does increase the inertia for that.  But that's different from debugging.  And, again, schannel is currently pretty static, so this is a hypothetical concern for an unknown time in the future.

To my mind it seems that, if having a Mac-specific implementation is ever going to be accepted, accepting it now is better than waiting indefinitely until the GnuTLS-based implementation is perfected and then revamping the by-then-even-more-GnuTLS-centric design to accommodate a second implementation.

Thanks for your feedback.

Cheers,
Ken