Encryption problems in DC++ 0.851 when connecting to a LUADCH hub

Bug #1484807 reported by Janne
22
This bug affects 3 people
Affects Status Importance Assigned to Milestone
DC++
Fix Released
Undecided
Unassigned

Bug Description

we are running Luadch 2.14 in the hubs and when we updated to the latest we got problems whit 0.851 clients. It works whit 0.843.
Whit the 0.851 we get tls error..
I have talkt to the Dev from Luadch and they say that this is something wrong whit 0.851

Kungen

Revision history for this message
poy (poy) wrote :

hey,

can you paste the error in detail?
then the best would be to run DC++ with the dev plugin and paste here the whole network log up to that error. :)
(it's at <http://sourceforge.net/projects/dcplusplus/files/Plugins/DevPlugin/>.)

since the luadch devs have already debugged this issue, could you ask them what their analysis is? could help whoever tackles this bug since they have already done the debugging work.

thanks

Revision history for this message
eMTee (realprogger) wrote :

I have a contact to the LUADCH camp through someone and they willing to give access to anyone from the team to their dev hub to discuss the compatibility problem with the latest version of DC++.

Also I am not exactly sure we should keep this report private as my guessing is that the issue is about unsupported (recently removed) ciphers from DC++ that might required for secure connection to a current LUADCH hub.

Even if the reason is different I don't think a fatal connection failure problem would pose any severe security risk and opening the bug up to the public would also make LUADCH people able to join.

Revision history for this message
cologic (cologic) wrote :

I agree there's no reason for this bug to be private.

Revision history for this message
poy (poy) wrote :

this thread is no longer private.

could luadch devs write their thoughts here? it would make much more sense to have that discussion recorded on a bug tracker that lost in a DC chat.

information type: Private Security → Public
eMTee (realprogger)
summary: - encrypt problems in 0.851
+ Encryption problems in DC++ 0.851 when connecting to a LUADCH hub
Revision history for this message
pulsar (pulsar-mail) wrote :

Hi@all,
I'am a Luadch dev and our hubsoft is using a cert with an elliptic curve 256bit prime key.
The problem is: There is no support for ciphers with elliptic curves in current versions of dc++ (v0.851).
It would be great if dc++ supports these cipher suites:

TLSv1:

   ECDHE-ECDSA-AES128-SHA

TLSv1.2:

   ECDHE-ECDSA-AES128-GCM-SHA256
   ECDHE-ECDSA-AES128-SHA256

PS: I know dc++ not support AES256 ciphers thats the reason why i only added AES128 ones.

greets pulsar

Revision history for this message
cologic (cologic) wrote :

Copying my response from the other bug:

I fully support elliptic curve cryptography, but your statement that "There is no support for ciphers with elliptic curves in current versions of dc++ (v0.851)." is simply inaccurate.

For the record, as of DC++ 0.851, it supports the following ciphersuites, copy/pasted directly from CryptoManager.cpp: "ECDHE-RSA-AES128-GCM-SHA256:ECDHE-RSA-AES128-SHA256:ECDHE-RSA-AES128-SHA:DHE-RSA-AES128-SHA:AES128-SHA". ECDHE-RSA-AES128-GCM-SHA256, ECDHE-RSA-AES128-SHA256, and ECDHE-RSA-AES128-SHA do use elliptic curves.

Regarding ECDSA specifically, vs the also-elliptic-curve ECDHE, https://security.stackexchange.com/questions/5096/rsa-vs-dsa-for-ssh-authentication-keys/41509 and, for example:
"Also, DSA and ECDSA have a nasty property: they require a parameter usually called k to be completely random, secret, and unique. In practice that means that if you connect to your server from a machine with a poor random number generator and e.g. the the same k happens to be used twice, an observer of the traffic can figure out your private key. (source: Wikipedia on DSA and ECDSA, also this)."

As https://tools.ietf.org/html/rfc6979 elaborates:
   One characteristic of DSA and ECDSA is that they need to produce, for
   each signature generation, a fresh random value (hereafter designated
   as k). For effective security, k must be chosen randomly and
   uniformly from a set of modular integers, using a cryptographically
   secure process. Even slight biases in that process may be turned
   into attacks on the signature schemes.

ECDHE-RSA-* don't have this problem, while ECDHE-ECDSA-* at least historically have.

It's possible that https://git.openssl.org/gitweb/?p=openssl.git;a=commitdiff;h=190c615d4398cc6c8b61eb7881d7409314529a75 adequately protects against this threat, though it does not implement RFC 6979 per se. I'll investigate whether ECDSA's glass jaw has been adequately ameliorated.

Revision history for this message
cologic (cologic) wrote :

Alright, I'm satisfied that OpenSSL as used in DC++ sufficiently addresses this issue such that I'll add ECDHE-ECDSA-AES128-GCM-SHA256 and ECDHE-ECDSA-AES128-SHA.

However, what additional value does ECDHE-ECDSA-AES128-SHA256 add? It's CBC, rather than AEAD [1], and thus vulnerable to http://www.isg.rhul.ac.uk/tls/Lucky13.html unlike ECDHE-ECDSA-AES128-GCM-SHA256 without, as far as I can tell, being more compatible than it, as both are TLS v1.2 ciphersuites.

[1] "The following lists give the SSL or TLS cipher suites names from the relevant specification and their OpenSSL equivalents. ... TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256 ECDHE-ECDSA-AES128-SHA256", https://www.openssl.org/docs/manmaster/apps/ciphers.html

Revision history for this message
pulsar (pulsar-mail) wrote :

Thanks for your quick response.

The first TLSv1.2 cipher i wrote is negligible and not important, it's enough if you add this:
   ECDHE-ECDSA-AES128-GCM-SHA256
   (TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256)

greets

Revision history for this message
pulsar (pulsar-mail) wrote :

I mean, of course the second TLSv1.2 cipher is negligible and not important, my fault ;)

Revision history for this message
cologic (cologic) wrote :

Added in http://sourceforge.net/p/dcplusplus/code/ci/e09fb9a0729e4240498c19c507f2aa9c059093b2/

While in theory this should work, I'll wait for empirical verification before changing this bug's status.

Revision history for this message
pulsar (pulsar-mail) wrote :
Revision history for this message
pulsar (pulsar-mail) wrote :

Tested the dc++ testbuild i got from eMTee, works with Luadch in TLSv1.2 mode.
Could not test TLSv1 with this testbuild...

Revision history for this message
cologic (cologic) wrote :

Why run TLS v1.0? There's no overlap between DC++ versions which don't support TLS v1.2 and DC++ versions which are otherwise incompatible with Luadch as I understand it.

Revision history for this message
pulsar (pulsar-mail) wrote :

There are many Luadch hubs where still uses TLSv1 atm. and so i hav tested TLSv1 too. The client should support TLSv1 too. You have wrote: "I'll add ECDHE-ECDSA-AES128-GCM-SHA256 and ECDHE-ECDSA-AES128-SHA" but i only can find the first one in the new cryptomanager file.

Revision history for this message
cologic (cologic) wrote :

Fair enough, I suppose, and I did write that. It seems like a problem that never had to to exist before, though. To the extent it's a legacy problem, it's one for which the simultaneous conditions (DC++ 0.851-type ciphersuite supports + newish Luadch ciphersuit supports) couldn't have happened until recently. I don't understand why any Luadch hubs that recent would exclusively support TLSv1, rather than both TLSv1 and TLSv1.2. But whatever.

Does http://sourceforge.net/p/dcplusplus/code/ci/285f7c38fc3fff087d772a1d4c76ee5761494fea/ fix the TLSv1 thing?

Revision history for this message
pulsar (pulsar-mail) wrote :

Can you add a new fixed testbuild please?

Revision history for this message
eMTee (realprogger) wrote :

New build uploaded to builds.dcbase.org.

cologic (cologic)
Changed in dcplusplus:
status: New → Fix Committed
Revision history for this message
pulsar (pulsar-mail) wrote :

Now everything is fine, thank you guys!

greets pulsar

Revision history for this message
poy (poy) wrote :

Fixed in DC++ 0.860.

Changed in dcplusplus:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Duplicates of this bug

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.