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

[Yanis Kurganov <yanis.kurganov@xxxxxxxxx>]

Got it! It takes some time...

2015-01-20 18:16 GMT+03:00 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
>
>
>