On Mon, 2019-02-04 at 19:10 -0500, Jon Simons wrote:
> Included here is an update to the pkd tests to reproduce a bug in
> RFC8332 RSA extension selection, as well as a fix which makes the
> test pass.
>
> When libssh server is provided "rsa-sha2-256,rsa-sha2-512" by the
> client for host key algorithms, it will unconditionally reply using
> the rsa-sha2-512 variant. But, the server should respect the
> client's preference in this case and use rsa-sha2-256.
>
> Also available here:
>
> *
> https://github.com/simonsj/libssh/tree/simonsj/patch/fix-rfc8332-bug-2-4-2019
> *
> https://gitlab.com/simonsj1/libssh-mirror/tree/simonsj/patch/fix-rfc8332-bug-2-4-2019
>
> Jon Simons (2):
> tests/pkd: repro rsa-sha2-{256,512} negotiation bug
> kex: honor client preference for rsa-sha2-{256,512} host key
> algorithms
>
> src/kex.c | 24 ++++++++++++++++++++++++
> tests/pkd/pkd_client.h | 15 +++++++++------
> tests/pkd/pkd_hello.c | 8 ++++++++
> 3 files changed, 41 insertions(+), 6 deletions(-)
Hello,
this generally looks good to me. Thank you for having a look into this
and fixing this corner case.
I am only a bit concerned about constructions like
+ } else if (0 == strcmp(rsa_sig_ext, "rsa-sha2-512")) {
I did not find explicit note in the coding guidelines forbidding this,
but I did not find the expression nowhere in the libssh code so I would
rather see it written the other way round:
+ } else if (strcmp(rsa_sig_ext, "rsa-sha2-512") == 0) {
but ideally completely outside of the if-condition as hinted by the
coding style [1].
[1]
https://gitlab.com/libssh/libssh-mirror/blob/master/README.CodingStyle#L344
Thanks,
--
Jakub Jelen
Software Engineer
Security Technologies
Red Hat, Inc.
