Re: [PATCH] Fix ability to use ECDSA host keys

[Alan Dunn <amdunn@xxxxxxxxx>]

Hi Aris,

On Sat, Feb 15, 2014 at 1:52 PM, Aris Adamantiadis <aris@xxxxxxxxxxxx> wrote:
> Hi Alan,
>
> Thanks for your patch. I did not review your patch but there's already
> something that needs change. You use EVP_* functions in pki.c which is
> supposed to be crypto backend independant. I think your code will not
> compile on libgcrypt builds.

I was merely imitating code elsewhere in pki.c (which does use the
function evp which eventually calls down to EVP_*).  All that code is
contained within "#ifdef HAVE_ECC" blocks that I believe only be set
when a libgcrypt build is not used if I'm interpreting
ConfigureChecks.cmake correctly.  If I set -DWITH_GCRYPT:BOOL=ON when
invoking cmake then the code does appear to compile, but if that's not
the set of options where you were afraid of a build break with then
let me know.  I can try to write a version that will also work for
libgcrypt builds, though I don't know the state of libgcrypt ECC with
libssh; there are several blocks that use the ifdef I described.
Working with libcrypto and always compiling is at least a step forward
over no host ECDSA keys :)

> On your last question if we should add the option or keep the HOSTKEYS
> options: I think you're right, and why not keep both ? In OpenSSH
> semantic, the Hostkey option can be used several time to add keys to the
> list while our implementation use the latest. We can easily fix that
> behaviour.

I am fine with having both an SSH_BIND_OPTIONS_ECDSAKEY and an option
to add a host key regardless of type.

The question for me is about the specifics of the option to add a host
key regardless of type.  I think one source of confusion for me is
that if you look in the documentation for ssh_bind_options_set in
include/libssh/server.h, it claims that SSH_BIND_OPTIONS_HOSTKEY is an
option to specify a private host key path.  However, if you look at
the documentation for the same function in src/options.c, it claims
that SSH_BIND_OPTIONS_HOSTKEY sets the server public key type.  The
latter is what the parameter actually seems to do in the code.

So do we want SSH_BIND_OPTIONS_HOSTKEY to actually specify a host key
(making it the option we describe above), or to continue to specify
which key types are usable?  Specifying which key types are usable
seems like a weird option to have; by specifying key paths and perhaps
using the explicitly key-typed versions SSH_BIND_OPTIONS_<x>KEY I
think a libssh user is specifying which key types they want to be
usable.  However, changing the behavior of SSH_BIND_OPTIONS_HOSTKEY
could break existing code, so I would be fine with creating one more
option name for the option to add a host key regardless of type.

Thanks,
- Alan

> thanks,
>
> Aris
> Le 15/02/14 20:17, Alan Dunn a écrit :
>> Hi folks,
>>
>> After our previous discussion on the inability to enable ECDSA keys, I
>> found some bugs in how they are actually used in libssh (even if one
>> were able to enable them).  With these changes, and some version of
>> changes to allow ECDSA host keys to be enabled (I used my prior patch
>> for testing and added an option to samplesshd), I was able to
>> successfully get examples sample and samplesshd to communicate, as
>> well as an OpenSSH client and samplesshd.
>>
>> There were two issues:
>> - ecdsa_nid was not copied to duplicated ECDSA private keys
>> - SHA-2 hashing was not used for sessionid generation for ECDSA keys
>> (instead SHA-1 was being used)
>>
>> Thanks,
>> - Alan
>
>