View Issue Details
|ID||Project||Category||View Status||Date Submitted||Last Update|
|0004738||unreal||module api||public||2016-08-22 17:47||2016-09-29 19:19|
|Fixed in Version||4.0.7|
|Summary||0004738: DH parameters not loaded|
|Description||OpenSSL dh.pem is not loaded when configured.|
The relative path is not recognized.
Also, it's missing in the online documentation.
|Steps To Reproduce||Configure set::ssl::dh "ssl/dh.pem";|
|Additional Information||s_conf.c :|
- else if (!strcmp(cepp->ce_varname, "dh"))
- ircstrdup(tempiConf.x_dh_pem, cepp->ce_vardata);
+ else if (!strcmp(cepp->ce_varname, "dh"))
+ convert_to_absolute_path(&cepp->ce_vardata, CONFDIR);
+ ircstrdup(tempiConf.x_dh_pem, cepp->ce_vardata);
|Tags||No tags attached.|
|3rd party modules|
If the additional information you submitted is supposed to be a diff/commit please consider doing a Pull request of your changes on UnrealIRCd's Github page. https://github.com/unrealircd/unrealircd/pulls In the comment of the pull you may reference this bug report.
||Ok I had no fork, but I did one :)|
First of all, thank you for reporting the bug. And even better that you did a pull request :)
I would not recommend using static DH. You should use ECDHE instead, which is also DH but "ephemeral", in other words: temporary, a key is generated for every connection. ECDHE is supported by UnrealIRCd 4 out of the box (by default). One important thing is that ECDHE supports Forward Secrecy (PFS) and static DH does not.
So, your bug report is valid, and the relative-path fix too, but... taking it one step further:
Perhaps we should remove DH support? Because people may think it's more secure, while in fact it isn't.
Or print a warning on launch/rehash.
Ok it makes sense. Yes, I thought DH params could increasing ECDHE security,
but I see now the difference between static and elliptic key.
The warning is a good idea. It looks like a static key of 2048+ is not yet discouraged.
But evolution is so fast, and I've always had my connections negociated to ECDHE on Unreal 4, so I guess not many people would contest removing the DH params.
It seems I was slightly confused. There are not just 2 DH variations but more: static DH, DHE, ADH, AECDH, ECDHE, ..
Anyway, we tend to favor ECDHE at UnrealIRCd because it is lightweight (it only adds little CPU %) and the most modern one.
If you have a dh.pem file you will enable DHE support, something I previously didn't mention. And DHE provides forward secrecy.
That being said, DHE was affected by vulnerabilities like Logjam (which doesn't affect you as you said you used a 2048+ key).
Also I saw this when reading up about it, Chromium intending to remove DHE support:
"Intent to deprecate: DHE-based cipher suites"
Also fun fact: "DHE today is only negotiated for 2% of connections by Chrome"
So I don't think we should advertise too much about support for it, but it wasn't as bad as I told you earlier :D
I'll commit your fix, though.
Thanks for the report & patch, fixed now.
|2016-08-22 17:47||capitaine||New Issue|
|2016-08-24 03:25||GTAXL||Note Added: 0019387|
|2016-08-25 12:52||capitaine||Note Added: 0019390|
|2016-08-29 09:11||syzop||Note Added: 0019396|
|2016-08-29 09:12||syzop||Assigned To||=> syzop|
|2016-08-29 09:12||syzop||Status||new => feedback|
|2016-08-29 12:19||capitaine||Note Added: 0019398|
|2016-09-26 18:08||syzop||Note Added: 0019419|
|2016-09-26 18:22||syzop||Note Edited: 0019419||View Revisions|
|2016-09-29 19:19||syzop||Note Added: 0019429|
|2016-09-29 19:19||syzop||Status||feedback => resolved|
|2016-09-29 19:19||syzop||Fixed in Version||=> 4.0.7|
|2016-09-29 19:19||syzop||Resolution||open => fixed|
|2017-01-06 15:48||syzop||Category||module => module api|