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: > > [email protected],[email protected], > [email protected],[email protected], > [email protected],[email protected], > 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 size_t. In packet_send2() -> if (ssh_buffer_add_data() < 0) should be two lines, see above. 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 [email protected]
