Re: [PATCH 1/3] options: Repurpose SSH_BIND_OPTIONS_HOSTKEY to load host keys

[Andreas Schneider <asn@xxxxxxxxxxxxxx>]

On Friday 21 March 2014 21:28:39 Alan Dunn wrote:
> SSH_BIND_OPTIONS_HOSTKEY will now load host keys of any supported type
> rather than set the algorithms that the server permits (which seems
> like an unhelpful option anyway; it seems you can always control this
> by just loading the right keys).
> 
> This option has slightly different semantics than the
> SSH_BIND_OPTIONS_<x>KEY options because it requires the key file to
> exist immediately rather than on ssh_bind_listen or
> ssh_bind_accept_fd.  The semantics of this option makes more sense to
> me.
> 
> We also eliminate ssh_bind_options_set_algo, since it is no longer
> used.

Hi Alan,

thanks for your contribution. See comment inline.

> 
> Signed-off-by: Alan Dunn <amdunn@xxxxxxxxx>
> ---
>  src/options.c |   72
> ++++++++++++++++++++++++++++++++++++++++----------------- 1 file changed,
> 51 insertions(+), 21 deletions(-)
> 
> diff --git a/src/options.c b/src/options.c
> index 1fda0c3..e272e4b 100644
> --- a/src/options.c
> +++ b/src/options.c
> @@ -1284,25 +1284,6 @@ int ssh_options_apply(ssh_session session) {
>   * @addtogroup libssh_server
>   * @{
>   */
> -static int ssh_bind_options_set_algo(ssh_bind sshbind, int algo,
> -    const char *list) {
> -  if (!verify_existing_algo(algo, list)) {
> -    ssh_set_error(sshbind, SSH_REQUEST_DENIED,
> -        "Setting method: no algorithm for method \"%s\" (%s)\n",
> -        ssh_kex_get_description(algo), list);
> -    return -1;
> -  }
> -
> -  SAFE_FREE(sshbind->wanted_methods[algo]);
> -  sshbind->wanted_methods[algo] = strdup(list);
> -  if (sshbind->wanted_methods[algo] == NULL) {
> -    ssh_set_error_oom(sshbind);
> -    return -1;
> -  }
> -
> -  return 0;
> -}
> -
>  static int ssh_bind_set_key(ssh_bind sshbind, char **key_loc,
>                              const void *value) {
>      if (value == NULL) {
> @@ -1392,8 +1373,57 @@ int ssh_bind_options_set(ssh_bind sshbind, enum
> ssh_bind_options_e type, ssh_set_error_invalid(sshbind);
>          return -1;
>        } else {
> -        if (ssh_bind_options_set_algo(sshbind, SSH_HOSTKEYS, value) < 0)
> -          return -1;
> +          int key_type;
> +          ssh_key key;
> +          ssh_key *bind_key_loc = NULL;
> +          char **bind_key_path_loc;
> +
> +          rc = ssh_pki_import_privkey_file(value, NULL, NULL, NULL, &key);
> +          if (rc != SSH_OK) {
> +              return -1;
> +          }
> +          key_type = ssh_key_type(key);
> +          switch (key_type) {
> +          case SSH_KEYTYPE_DSS:
> +              bind_key_loc = &sshbind->dsa;
> +              bind_key_path_loc = &sshbind->dsakey;
> +              break;
> +          case SSH_KEYTYPE_ECDSA:
> +#ifdef HAVE_ECC
> +              bind_key_loc = &sshbind->ecdsa;
> +              bind_key_path_loc = &sshbind->ecdsakey;
> +#else
> +              ssh_set_error(sshbind,
> +                            SSH_FATAL,
> +                            "ECDSA key used and libssh compiled "
> +                            "without ECDSA support");
> +#endif
> +              break;
> +          case SSH_KEYTYPE_RSA:
> +          case SSH_KEYTYPE_RSA1:
> +              bind_key_loc = &sshbind->rsa;
> +              bind_key_path_loc = &sshbind->rsakey;
> +              break;
> +          default:
> +              ssh_set_error(sshbind,
> +                            SSH_FATAL,
> +                            "Unsupported key type %d", key_type);
> +          }
> +
> +          if (!bind_key_loc) {

Please use:

	     if (bind_key_loc == NULL) {


> +              ssh_key_free(key);
> +              return -1;
> +          }
> +
> +          /* Set the location of the key on disk even though we don't
> +             need it in case some other function wants it */
> +          rc = ssh_bind_set_key(sshbind, bind_key_path_loc, value);
> +          if (rc < 0) {
> +              ssh_key_free(key);
> +              return -1;
> +          }
> +          ssh_key_free(*bind_key_loc);
> +          *bind_key_loc = key;
>        }
>        break;
>      case SSH_BIND_OPTIONS_BINDADDR:

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