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

Re: Support for SHA2 HMAC algorithms

On Thursday 10 April 2014 14:35:55 Dirkjan Bussink wrote:
> Hi all,

Hi Dirkjan,

> I’ve written a patch that adds support to libssh for SHA2 HMAC algorithms.
> Newer versions of OpenSSH are disabling SHA1 as the default, so it would be
> good to be prepared here for this future.

That's awesome!

> http://www.openssh.com/cgi-bin/man.cgi?query=sshd_config
> The default is:
> umac-64-etm@xxxxxxxxxxx,umac-128-etm@xxxxxxxxxxx,
> hmac-sha2-256-etm@xxxxxxxxxxx,hmac-sha2-512-etm@xxxxxxxxxxx,
> umac-64@xxxxxxxxxxx,umac-128@xxxxxxxxxxx,
> hmac-sha2-256,hmac-sha2-512
> If this patch needs any changes to be accepted, please let me know.

I have to leave soon so I just did a quick scan. Here are some comments:

crypto->digest_len is size_t

so hmac_digest_len() should return size_t

generate_one_key() should use 'size_t requested'. The name is not the best, 
maybe requested_size.

I don't have to fully go trough the while loop of generate_one_key() but I 
think we might need to check for an overflow here.

In generate_session_keys() for all see if-clause I would like to see two 
lines. It is easier to debug.

int rc;

rc = generate_one_key();
if (rc < 0) {

I know our CodingStyle is horrible but try to apply what you see in pki.c.

in ssh_packet_socket_callback()

if(session->current_crypto) {

please use session->current_crypto == NULL and current_macsize should be 

In packet_send2() -> if (ssh_buffer_add_data() < 0) should be two lines, see 

These are only cosmetic things. I will do a in-depth review in the next days.

It would be great if you could submit another patch with testcases!

I think you can copy tests/client/torture_algorithms.c and modify it.

Thanks again for your contribution!

Best regards,

	-- andreas

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

Re: Support for SHA2 HMAC algorithmsDirkjan Bussink <d.bussink@xxxxxxxxx>
Support for SHA2 HMAC algorithmsDirkjan Bussink <d.bussink@xxxxxxxxx>
Archive administrator: postmaster@lists.cynapses.org