Comment 12 for bug 991342

1-2) The verify_callback for the most part has been "empirically tested" over the past year in ApexDC, and before inclusion there it was tested against a set of such test cases (heck it was mostly created based on those test cases). The only notable change related to verify_callback that this does not apply is the logging stuff that I added fairly recently. Same applies to 3), howerver, the default cipher changes discussed on the hub is a different story when it comes to interop with Java 6 in particular but that should be a separate patch on its own.

Generally when it comes to interoperability the bottom line is, anything newer than openssl 1.x.x based or equivalent should be fine, the old 0.9 versions of openssl are a mystery at this point (although, openssl default build is rather conservative, so there should be no problems unless openssl has explicitly stated otherwise). And this particular patch isn't touching openssl itself anyways (which needs to be upgraded to 1.0.1f by the way).

a) This is mostly because of the use of the SSL_ex_app_data functions, and more of a precaution than anything else. Other clients have used this kind of setup far longer, and all testing was done on a setup with it. (see: https://bugs.launchpad.net/dcplusplus/+bug/378829)
b) This is a fairly common practice, look at f.ex. Apache, two years ago when I intially started the work on cryptomanager in ApexDC Firefox would fail without variable DH sizes (ApexDC does do some web serving as well, to give context).
c) You need to free memory even if the process is exiting, this is openssl, which is C and designed to work as external module, so it does lot of memory copying of objects passed to it
d) Changes to cert generation have been guided by the common practices (ie. don't use sha1 anymore, it has been on its way out for years from use with certificates now). Also without those changes openssl defaults to a rather conservative (read: bad) ciphers for reasons that are arcane by nature (ie. it shouldn't according to keyp author but quite obviously it does).
e) The commented out code is pretty cut and dry, isTrusted() returns based on cert verification status, and if said verification fails the connection never gets this far when ALLOW_UNTRUSTED_CLIENTS is false (this is doubly true for pre-patch code, due to use of SSL_VERIFY_NONE being used, in which case the server (the one who requested CTM) has no peer cert to verify to begin with. ConnectionManager::checkKeyprint() deals with the ALLOW_UNTRUSTED_CLIENTS option for servers anyways, and clients do it during handshake (see UserConnection::connect())

For the TODO's those only reflect the fact that in practice, hub without pinned KeyPrint is not trusted, so should the KeyPrints of its clients be blindly trusted (there is no chain of trust from hub to its peers and, possibly even not from whoerver distributes the hubs KeyPrint to users). Ideally, a hub has the capability to check the KP fields of connecting clients and not let them in if there is a mismatch.