[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] basic client certificate support for libssh


On Friday 21 August 2015 13:09:40 Axel Eppe wrote:
> Hi,

Hi Axel,

> Any chance this might cut it for a release? Do you see anything contentious
> that should be changed?

I will release this with libssh 0.8. For libssh 0.8 I want to get the 
connector code in and cwrap support. I will work on this beginning of 
September. So release will be mid or end of September.

> I'd be interested in adding support for more key types (and maybe server
> side too), but it'd help to know the change proposed are OK.

The code looks pretty good, but I find it hard to review and comment on such 
big patches. Could you please split them up in smaller patches?

The first patch(es) should introduce functions to pki*.c. Then the tests for 
the new pki functions. Afterward start using them in the libssh code ...

Some comments:

Do not mix tabs and spaces (see changes to agent.c) and always braces on if-
clause see README.CodingStyle.

Yes, we do not have this all over the place for historic reasons, but the code 
should look like pki.c ...

In ssh_userauth_publickey():

Why is the privkey->type_c incorrect? it should already hold the correct type 
...

ssh_pki_copy_cert_to_privkey() uses 2 spaces instead of 4 :)


pki_publickey_to_blob():

You should use buffer_add_ssh_string()


pki_import_cert_buffer():

Use buffer_get_ssh_string()


Best regards,


	-- andreas

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


References:
Re: [PATCH] basic client certificate support for libsshAndreas Schneider <asn@xxxxxxxxxxxxxx>
Re: [PATCH] basic client certificate support for libsshAxel Eppe <aeppe@xxxxxxxxxx>
Archive administrator: postmaster@lists.cynapses.org