ejabberd fails at ssl

[emerose]

Binary package hint: ejabberd

Hi,

From what I can tell, it looks like there are two significant security problems in the patch that added SSL support to ejabberd's ldap module (added in 2.0.0-7; see debian bug #477918) --

1) The validity of peer SSL certificates is not verified: the call to ssl:connect in the patch explicitly sets the value for "verify" to 0:

ssl:connect(Host, State#eldap.port, [{verify,0}|Opts]).

In the Erlang SSL library[1] this setting translates to "do not verify peer." An attacker who is able to perform a man-in-the-middle attack would thus be able to spoof the LDAP server and intercept any passwords sent over the wire. For sites using LDAP with simple authentication, this would expose the credentials of all users who authenticate to the ejabberd server.

(Please note that, simply enabling peer certificate validation will probably break a number of ejabberd installations, as there is currently no way (that I have found) of specifying a custom CA certificate...)

2) The same bit of code uses a very low-entropy source of data to seed the SSL library:

{_,_,X} = erlang:now(),
ssl:seed("bkrlnateqqo" ++ integer_to_list(X)),

This seeds the SSL library with a constant string and the current number of microseconds, which is a very weak source of entropy. I am uncertain of the practical effect of this seed, but the Erlang SSL documentation[1] contains a number of notes like this:

It is strongly advised to seed the random generator after the ssl application has been started, and before any connections are established. Although the port program interfacing to the OpenSSL libraries does a "random" seeding of its own in order to make everything work properly, that seeding is by no means random for the world since it has a constant value which is known to everyone reading the source code of the seeding.

On Debian/Ubuntu systems, it seems wiser to read a few bytes from /dev/urandom and use that as the seed...

Please take a few minutes to review these issues, and let mw know what you think.

Thanks!
-sq

[1] http://www.erlang.org/doc/man/ssl.html

[Rhonda D'Vine (rhonda)]

 Since version 2.0.5-1 (in karmic) the ldaps patch got dropped because it didn't apply anymore. Upstream is working on integrating ldaps support into mainline so the extra patch won't be needed anymore - but it might take a bit until a release is made that ships this.

 So for the time being this bugreport can either be considered invalid for karmic, or as a wishlist to have it re-added.

[Tim 'Shaggy' Bielawa (tbielawa)]

Regarding LDAPS support:

http://www.ejabberd.im/ejabberd-2.1.0 Posted by Jérôme Sautret on November 13, 2009:

"ejabberd 2.1.0 - HTTP-Bind, LDAPS, PubSub 1.12, STUN, Ping, Roster Versioning"

Amongst other things they have completed their LDAPS support.

[Neustradamus (neustradamus)]

There is a patch here : https://support.process-one.net/browse/EJAB-1229

Can you try and tell me the result ?

[Konstantin Khomoutov (flatworm)]

The modified fix for EJAB-1229 appears to be committed to the 2.1.x branch, so we can expect it do be available in 2.1.4: http://github.com/processone/ejabberd/commit/f58d03c12e1160f40a7c38b61b0b6a47a1bc6a1b

[Konstantin Khomoutov (flatworm)]

The patches mentioned in comments #3 and #4 do only implement a way to enable LDAPS's server certificate checking; the second problem mentioned in the original bug report was fixed in one of the following commits: https://git.process-one.net/ejabberd/mainline/commit/6ae8b9c4d6b66296829483487a9d41e5eff8f2c5

[Neustradamus (neustradamus)]

Now this bug is correct, we are ok ? This ticket can be updated ?

[Konstantin Khomoutov (flatworm)]

I'm not sure what do you mean, Neustradamus.
The bug can be told to be "correct" in the sense it is not invalid, but the patches being discussed here will only hit the next stable release (2.1.4) which is not ready yet.
If I will push these patches to the next version of the 2.1.3 Debian package, I'll report here so this bug could be closed; otherwise the bug should possibly wait for 2.1.4 to appear in Ubuntu.
If Launchpad allows, you could "tag" this bug as "fixed upstream".