Re: [PATCH] diffie-hellman-group-exchange-sha256

[Andreas Schneider <asn@xxxxxxxxxxxxxx>]

On Wednesday 25 November 2015 19:02:51 Yanis Kurganov wrote:
> Here is a fresh patch on 0.7.2
> Successfully tested in our company.
> Maybe this will help!

Aris, didn't you work on that?

> 
> 2015-09-16 17:14 GMT+03:00 Yanis Kurganov <yanis.kurganov@xxxxxxxxx>:
> > Hi, Aris!
> > I got it.
> > OK, use my code as you wish!
> > I'm waiting GEX in a future releases of libssh.
> > And finally remove my own repository)))
> > 
> > 2015-07-27 16:47 GMT+03:00 Aris Adamantiadis <aris@xxxxxxxxxxxx>:
> >> Le 23/01/15 15:40, Yanis Kurganov a écrit :
> >> > It's a final version with modern SSH_MSG_KEY_DH_GEX_REQUEST.
> >> > Some clients (for example, Tera Term) use only this message.
> >> > 
> >> > 2015-01-23 13:52 GMT+03:00 Yanis Kurganov <yanis.kurganov@xxxxxxxxx
> >> > 
> >> > <mailto:yanis.kurganov@xxxxxxxxx>>:
> >> >     Andreas, sorry, I missed something for server (for group1-14
> >> >     algos).
> >> >     Patch Diffie-Hellman Group Exchange - attempt 2
> >> 
> >> Hi Yanis,
> >> 
> >> Sorry it took me so much time, but I'm finally on holidays and have more
> >> time to work :)
> >> I have carefully reviewed your patch. Unfortunately, I cannot accept it
> >> as-is. Here is my feedback:
> >> - The patch is too big. Some new functionalities are introduced while
> >> some core code is refactorized. It makes it hard to figure out what
> >> really changed, and also harder to debug or git bisect.
> >> - Some changes are too intrusive and should probably have been discussed
> >> before. For example, the pre-initialization of group1 and group14
> >> parameters that moved into a runtime operation.
> >> - I really don't like the way the new packet handlers are implemented.
> >> It's not your fault, the single function pointers array only works
> >> correctly when the overlapping packets numbers have similar functions.
> >> It's not the case with GEX anymore and it forced you to use a dirty hack
> >> for the multiplexing. I'm implementing right now a way to have
> >> independent callbacks for each key exchange method, so this problem goes
> >> away.
> >> - The current implementation will blindly accept any group that is
> >> provided by the server. I want to check on what OpenSSH does but I'm
> >> certain we should at least check the parameter size and maybe some basic
> >> characteristics (is it a strong prime, is the generator/prime congruency
> >> relation compliant with the RFC).
> >> - I wasn't really expecting the server-side to serve group1 or group14
> >> instead of groups in /etc/ssh/moduli. The whole point of
> >> dh-group-exchange is to use different groups to discourage the use of
> >> fixed groups like group1 & group14 that are more likely to be cracked
> >> and have efficient attacks around. Serving these two groups will solve a
> >> technical problem (client xxx won't connect) but at the cost of
> >> introducing new security problems.
> >> 
> >> Your code however correctly and neatly implements the packet parsing and
> >> packet sending, together with the corner case of SSH_MSG_GEX_OLD_INIT.
> >> My proposition now is that I'll work on a better way of decoupling the
> >> different key exchange methods, and implement GEX by using as much of
> >> your code as I can. I'll make sure you get your name as the commiter so
> >> you're properly credited for you work.
> >> 
> >> Best regards,
> >> 
> >> Aris

-- 
Andreas Schneider                   GPG-ID: CC014E3D
www.cryptomilk.org                asn@xxxxxxxxxxxxxx