linuxdcpp-team team mailing list archive

[Crise <991342@xxxxxxxxxxxxxxxxxx>]

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.

-- 
You received this bug notification because you are a member of
Dcplusplus-team, which is subscribed to the bug report.
https://bugs.launchpad.net/bugs/991342

Title:
  KEYP Vulnerability

Status in DC++:
  New

Bug description:
  With the current vulnerability with DC++'s current KEYP implementation
  the underlying issue seems to be this ...

  [2012-04-26 09:24] <Crise> anyways, the thing with keyp is entirely
  different problem... which is basically that it only verifies keyp on
  the peer level certificate and not on the whole chain as it should

  Crise has stated he has another source who knows the exploit but will
  not divulge in who he is.

To manage notifications about this bug go to:
https://bugs.launchpad.net/dcplusplus/+bug/991342/+subscriptions