Hi Antoine, On Sat, Jan 30, 2016 at 9:45 PM, Antoine Beaupré <[email protected]> wrote: > > It looks, however, that there's a bit missing in the patch... Upstream > seems to have made *two* patches to solve the issue. It looks like you > backported this: > > https://github.com/bjc/prosody/commit/8708def4f55e61acdd5b2c762d420ab40da0d015
This is a patch which works for 0.9.10 and earlier (moreover, it won't work directly with 0.7.0 in squeeze because the code is actually in another file - core/s2smanager.lua and not plugins/mod_dialback.lua) > > but there's also: > > https://github.com/bjc/prosody/commit/c9ce85a5d7575f9c55ce85b45db812f3e8392b07 This is the same patch but suitable for the master branch. Since 0.7.0 don't have an option named "dialback_secret" in module dialback, one doesn't have to backport it. > > It looks like there's some initialisation of the dialback_secret > variable missing... Upstream master currently has: > > local dialback_secret = > sha256_hash(module:get_option_string("dialback_secret", uuid_gen()), true); > > ... which i think is missing from the resulting patch. It shouldn't go to the 0.7.0. > > I am also unclear as to the source of the second patch, regarding the > RNG seeding. It sure looks like we do not seed it anymore: > > +-function seed(x) > +- urandom:write(x); > +- urandom:flush(); > ++function seed() > + end > > That looks wrong, no? Is that a patch upstream? I see that 0.9.1 uses > the lua "random" module instead of the above: util.random is a new module which appears in 0.9.10. It can be seen here: http://hg.prosody.im/trunk/file/43328166dcf1/util/random.lua You can see that it's using /dev/urandom just the way the patch does. And the patch is a non-intrusive adaptation of this new module to the old code. The patch fixes a regression caused by a previous one (0008-CVE-2016-1232-weak-PRNG-for-dialback-on-S2S.patch) which someone has already ported to 0.7.0 currently in squeeze-lts. The problem is that if /dev/urandom is readonly then prosody stops working. So, the solution is to drop seeding at all because /dev/urandom is already sufficiently random. > > https://github.com/bjc/prosody/blob/master/util/uuid.lua > > Yet your patch says the source is "upstream"... could you clarify where > it comes from or the rationale for this fix? See above. Cheers! -- Sergei Golovan
