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

[Andreas Schneider <asn@xxxxxxxxxxxxxx>]

On Tuesday 20 January 2015 16:38:28 Yanis Kurganov wrote:
> 2015-01-20 14:19 GMT+03:00 Andreas Schneider <asn@xxxxxxxxxxxxxx>:
> > > Server side is ready!
> > 
> > That's great. Thanks for your contribution! I would like to see some
> > changes
> > to the code for inclusion. It following our CodingStyle, small cleanups
> > and
> > splitting it into more patches.
> > 
> > > My tests with dh-group-exchange client (PuTTY) are OK.
> > > What about tests? What format of the tests do you want?
> > 
> > Please do not call functions in if-statement or in function parameters.
> > They
> > make it harder to debug code.
> > 
> > if (buffer_get_u32(session->in_buffer, &pbits) != sizeof(uint32_t)) {
> > 
> >         ...
> > 
> > }
> > 
> > should be:
> > 
> > int rc;
> > 
> > rc = buffer_get_u32(session->in_buffer, &pbits);
> > if (rc != sizeof(uint32_t)) {
> > 
> >         ...
> > 
> > }
> > 
> > It makes it easier to debug code, also please also add braces to all
> > if/for/while/etc. clauses!
> > 
> > See also:
> > http://git.libssh.org/projects/libssh.git/tree/README.CodingStyle
> > and
> > https://blog.cryptomilk.org/2013/03/28/writing-and-reading-code/
> 
> OK, done.
> Also I made all-in-one patchset (with client and server side).
> 
> > I would also split the commit. Renames like dh_import_p ->
> > dh_import_p_string
> > should be in an own commit. They are not really related to diffie-hellman-
> > group-exchange-sha256.
> > 
> > 
> > dh_send_group_server() should use ssh_buffer_pack().
> 
> I have own libssh repository based on version 0.6.3 with other changes.
> It's hard to split commits =(((
> And I couldn't find ssh_buffer_pack. I suppose it is from master?

Yes, please checkout libssh git master and port the patches to master. They 
will be part of the libssh 0.7 release. We will not add this to 0.6.x.


ssh_buffer_pack() is only available in master.


	-- andreas


> > Thanks!
> > 
> >         -- andreas
> > 
> > --
> > Andreas Schneider                   GPG-ID: CC014E3D
> > www.cryptomilk.org                asn@xxxxxxxxxxxxxx

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