[libvirt] [PATCH v3 3/6] libssh_transport: add new libssh-based transport
Peter Krempa
pkrempa at redhat.com
Tue Nov 1 12:20:28 UTC 2016
On Wed, Oct 19, 2016 at 14:40:36 +0200, Pino Toscano wrote:
> Implement a new libssh transport, which uses libssh to communicate with
> remote hosts, and add all the build system stuff (search of libssh,
> private symbols, etc) to built it.
>
> This new transport supports all the common ssh authentication methods,
> making use of libvirt's auth callbacks for interaction with the user.
> ---
> config-post.h | 2 +
> configure.ac | 3 +
> m4/virt-libssh.m4 | 26 +
> po/POTFILES.in | 1 +
> src/Makefile.am | 21 +-
> src/libvirt_libssh.syms | 22 +
> src/rpc/virnetlibsshsession.c | 1458 +++++++++++++++++++++++++++++++++++++++++
> src/rpc/virnetlibsshsession.h | 80 +++
> 8 files changed, 1611 insertions(+), 2 deletions(-)
> create mode 100644 m4/virt-libssh.m4
> create mode 100644 src/libvirt_libssh.syms
> create mode 100644 src/rpc/virnetlibsshsession.c
> create mode 100644 src/rpc/virnetlibsshsession.h
> diff --git a/src/rpc/virnetlibsshsession.c b/src/rpc/virnetlibsshsession.c
> new file mode 100644
> index 0000000..13dd52c
> --- /dev/null
> +++ b/src/rpc/virnetlibsshsession.c
> @@ -0,0 +1,1458 @@
[...]
> +static void
> +virNetLibsshSessionAuthMethodsFree(virNetLibsshSessionPtr sess)
> +{
> + size_t i;
> +
> + for (i = 0; i < sess->nauths; i++) {
> + VIR_FREE(sess->auths[i]->password);
This should be turned into VIR_DISPOSE_STR.
> + VIR_FREE(sess->auths[i]->filename);
> + VIR_FREE(sess->auths[i]);
> + }
> +
> + VIR_FREE(sess->auths);
> + sess->nauths = 0;
> +}
[...]
> +
> +/* string representation of public key of remote server */
> +static char *
> +virSshServerKeyAsString(virNetLibsshSessionPtr sess)
> +{
> + int ret;
> + ssh_key key;
> + unsigned char *keyhash;
> + size_t keyhashlen;
> + char *str;
> +
> + if (ssh_get_publickey(sess->session, &key) != SSH_OK) {
> + virReportError(VIR_ERR_LIBSSH, "%s",
> + _("failed to get the key of the current "
> + "session"));
> + return NULL;
> + }
> +
> + /* calculate remote key hash, using MD5 algorithm that is
> + * usual in OpenSSH. The returned value must be freed */
> + ret = ssh_get_publickey_hash(key, SSH_PUBLICKEY_HASH_MD5,
> + &keyhash, &keyhashlen);
Openssh currently is using SHA256 with base64 encoding. I think we
should at least for libssh switch to this newer approach.
> + ssh_key_free(key);
> + if (ret < 0) {
> + virReportError(VIR_ERR_LIBSSH, "%s",
> + _("failed to calculate ssh host key hash"));
> + return NULL;
> + }
> + /* format the host key into a nice userfriendly string. */
> + str = ssh_get_hexa(keyhash, keyhashlen);
> + ssh_clean_pubkey_hash(&keyhash);
> +
> + return str;
> +}
[...]
> +/* check session host keys
> + *
> + * this function checks the known host database and verifies the key
> + * errors are raised in this func
> + *
> + * return value: 0 on success, -1 on error
> + */
> +static int
> +virNetLibsshCheckHostKey(virNetLibsshSessionPtr sess)
> +{
> + int state;
> + char *keyhashstr;
> + const char *errmsg;
> +
> + if (sess->hostKeyVerify == VIR_NET_LIBSSH_HOSTKEY_VERIFY_IGNORE)
> + return 0;
> +
> + state = ssh_is_server_known(sess->session);
> +
> + switch (state) {
> + case SSH_SERVER_KNOWN_OK:
> + /* host key matches */
> + return 0;
> +
> + case SSH_SERVER_FOUND_OTHER:
> + case SSH_SERVER_KNOWN_CHANGED:
> + keyhashstr = virSshServerKeyAsString(sess);
> + if (!keyhashstr)
> + return -1;
> +
> + /* host key verification failed */
> + virReportError(VIR_ERR_AUTH_FAILED,
> + _("!!! SSH HOST KEY VERIFICATION FAILED !!!: "
> + "Identity of host '%s:%d' differs from stored identity. "
> + "Please verify the new host key '%s' to avoid possible "
> + "man in the middle attack. The key is stored in '%s'."),
> + sess->hostname, sess->port,
> + keyhashstr, sess->knownHostsFile);
> +
> + ssh_string_free_char(keyhashstr);
> + return -1;
> +
> + case SSH_SERVER_FILE_NOT_FOUND:
> + case SSH_SERVER_NOT_KNOWN:
> + /* key was not found, query to add it to database */
> + if (sess->hostKeyVerify == VIR_NET_LIBSSH_HOSTKEY_VERIFY_NORMAL) {
> + virConnectCredential askKey;
> + int cred_type;
> + char *tmp;
> +
> + /* ask to add the key */
> + if (!sess->cred || !sess->cred->cb) {
> + virReportError(VIR_ERR_LIBSSH, "%s",
> + _("No user interaction callback provided: "
> + "Can't verify the session host key"));
> + return -1;
> + }
> +
> + cred_type = virCredTypeForPrompt(sess->cred, 1 /* echo */);
> + if (cred_type == -1) {
> + virReportError(VIR_ERR_LIBSSH, "%s",
> + _("no suitable callback for host key "
> + "verification"));
> + return -1;
> + }
> +
> + /* prepare data for the callback */
> + memset(&askKey, 0, sizeof(virConnectCredential));
> + askKey.type = cred_type;
> +
> + keyhashstr = virSshServerKeyAsString(sess);
> + if (!keyhashstr)
> + return -1;
> +
> + if (virAsprintf((char **)&askKey.prompt,
If you asprintf into 'tmp' first you won't need to typecast here ...
> + _("Accept SSH host key with hash '%s' for "
> + "host '%s:%d' (%s/%s)?"),
> + keyhashstr,
> + sess->hostname, sess->port,
> + "y", "n") < 0) {
> + ssh_string_free_char(keyhashstr);
> + return -1;
> + }
> +
> + if (sess->cred->cb(&askKey, 1, sess->cred->cbdata)) {
> + virReportError(VIR_ERR_LIBSSH, "%s",
> + _("failed to retrieve decision to accept "
> + "host key"));
> + tmp = (char *)askKey.prompt;
> + VIR_FREE(tmp);
> + ssh_string_free_char(keyhashstr);
> + return -1;
> + }
> +
> + tmp = (char *)askKey.prompt;
... and here.
> + VIR_FREE(tmp);
> +
> + if (!askKey.result ||
> + STRCASENEQ(askKey.result, "y")) {
> + virReportError(VIR_ERR_LIBSSH,
> + _("SSH host key for '%s' (%s) was not accepted"),
> + sess->hostname, keyhashstr);
> + ssh_string_free_char(keyhashstr);
> + VIR_FREE(askKey.result);
> + return -1;
> + }
> + ssh_string_free_char(keyhashstr);
> + VIR_FREE(askKey.result);
> + }
> +
> + /* write the host key file */
> + if (ssh_write_knownhost(sess->session) < 0) {
> + errmsg = ssh_get_error(sess->session);
> + virReportError(VIR_ERR_LIBSSH,
> + _("failed to write known_host file '%s': %s"),
> + sess->knownHostsFile,
> + errmsg);
> + return -1;
> + }
> + /* key was accepted and added */
> + return 0;
> +
> + case SSH_SERVER_ERROR:
> + errmsg = ssh_get_error(sess->session);
> + virReportError(VIR_ERR_LIBSSH,
> + _("failed to validate SSH host key: %s"),
> + errmsg);
> + return -1;
> +
> + default: /* should never happen (tm) */
> + virReportError(VIR_ERR_LIBSSH, "%s",
> + _("Unknown state of the remote server SSH key"));
> + return -1;
> + }
> +
> + return -1;
> +}
[...]
> +/* perform private key authentication
> + *
> + * returns SSH_AUTH_* values
> + */
> +static int
> +virNetLibsshAuthenticatePrivkey(virNetLibsshSessionPtr sess,
> + virNetLibsshAuthMethodPtr priv)
> +{
> + int err;
> + int ret;
> + char *tmp = NULL;
> + ssh_key public_key = NULL;
> + ssh_key private_key = NULL;
> +
> + VIR_DEBUG("sess=%p", sess);
> +
> + /* try open the key with the password set, first; since it can
> + * fail with SSH_ERROR also without the callback being called,
> + * reset the error so it is possible to check whether the callback
> + * failed or libssh did.
> + */
> + virResetLastError();
> + ret = ssh_pki_import_privkey_file(priv->filename, priv->password,
> + virNetLibsshAuthenticatePrivkeyCb,
> + sess, &private_key);
> + if (ret == SSH_EOF) {
> + virReportError(VIR_ERR_AUTH_FAILED,
> + _("error while reading private key '%s'"),
> + priv->filename);
> + err = SSH_AUTH_ERROR;
> + goto error;
> + } else if (ret == SSH_ERROR) {
> + virErrorPtr vir_err = virGetLastError();
> +
> + if (!vir_err || !vir_err->code) {
> + virReportError(VIR_ERR_AUTH_FAILED,
> + _("error while opening private key '%s', wrong "
> + "passphrase?"),
> + priv->filename);
> + }
> + err = SSH_AUTH_ERROR;
> + goto error;
> + }
> +
> + if (virAsprintf(&tmp, "%s.pub", priv->filename) < 0) {
> + err = SSH_AUTH_ERROR;
> + goto error;
> + }
> +
> + /* try to open the public part of the private key */
> + ret = ssh_pki_import_pubkey_file(tmp, &public_key);
> + if (ret == SSH_ERROR) {
> + virReportError(VIR_ERR_AUTH_FAILED,
> + _("error while reading public key '%s'"),
> + tmp);
> + err = SSH_AUTH_ERROR;
> + goto error;
> + } else if (ret == SSH_EOF) {
> + /* create the public key from the private key */
> + ret = ssh_pki_export_privkey_to_pubkey(private_key, &public_key);
> + if (ret == SSH_ERROR) {
> + virReportError(VIR_ERR_AUTH_FAILED,
> + _("cannot export the public key from the "
> + "private key '%s'"),
> + tmp);
> + err = SSH_AUTH_ERROR;
> + goto error;
> + }
> + }
> +
> + VIR_FREE(tmp);
> +
> + ret = ssh_userauth_try_publickey(sess->session, NULL, public_key);
> + if (ret != SSH_AUTH_SUCCESS) {
> + err = SSH_AUTH_DENIED;
> + goto error;
> + }
So this code needs to be called before asking the user for the
passphrase in cases where you are able to find the public part even
without decrypting the private key.
This is necessary, because otherwise if you don't have the SSH key
installed on the server you are connecting to, you still need to enter
the passphrase.
By reordering the code it should no longer be necessary if the key is
not installed.
> +
> + ret = ssh_userauth_publickey(sess->session, NULL, private_key);
> + if (ret != SSH_AUTH_SUCCESS) {
> + err = SSH_AUTH_DENIED;
> + goto error;
> + }
> +
> + ssh_key_free(private_key);
> + ssh_key_free(public_key);
> +
> + return SSH_AUTH_SUCCESS;
> +
> + error:
> + if (private_key)
> + ssh_key_free(private_key);
> + if (public_key)
> + ssh_key_free(public_key);
> + VIR_FREE(tmp);
> + return err;
> +}
[...]
> +/* perform keyboard interactive authentication
> + *
> + * returns SSH_AUTH_* values
> + */
> +static int
> +virNetLibsshAuthenticateKeyboardInteractive(virNetLibsshSessionPtr sess,
> + virNetLibsshAuthMethodPtr priv)
> +{
> + int ret;
> + const char *errmsg;
> + int try = 0;
> +
> + /* request user's key password */
> + if (!sess->cred || !sess->cred->cb) {
> + virReportError(VIR_ERR_LIBSSH, "%s",
> + _("No user interaction callback provided: "
> + "Can't get input from keyboard interactive "
> + "authentication"));
> + return SSH_AUTH_ERROR;
> + }
> +
> + again:
Please use a while loop for this purpose.
> + ret = ssh_userauth_kbdint(sess->session, NULL, NULL);
> + while (ret == SSH_AUTH_INFO) {
> + const char *name, *instruction;
> + int nprompts, iprompt;
> + virBuffer buff = VIR_BUFFER_INITIALIZER;
> +
> + name = ssh_userauth_kbdint_getname(sess->session);
> + instruction = ssh_userauth_kbdint_getinstruction(sess->session);
> + nprompts = ssh_userauth_kbdint_getnprompts(sess->session);
> +
> + /* compose the main buffer with name and instruction, if present */
> + if (name && name[0])
> + virBufferAddStr(&buff, name);
> + if (instruction && instruction[0]) {
> + if (virBufferUse(&buff) > 0)
> + virBufferAddChar(&buff, '\n');
> + virBufferAddStr(&buff, instruction);
> + }
> +
> + if (virBufferCheckError(&buff) < 0)
> + return -1;
> +
> + for (iprompt = 0; iprompt < nprompts; ++iprompt) {
> + virConnectCredential retr_passphrase;
> + const char *promptStr;
> + char echo;
> + virBuffer prompt_buff = VIR_BUFFER_INITIALIZER;
> + char *prompt = NULL;
> + int cred_type;
> +
> + /* get the prompt, stripping the trailing newlines and spaces */
> + promptStr = ssh_userauth_kbdint_getprompt(sess->session, iprompt,
> + &echo);
> +
> + cred_type = virCredTypeForPrompt(sess->cred, echo);
> + if (cred_type == -1) {
> + virReportError(VIR_ERR_LIBSSH, "%s",
> + _("no suitable callback for input of keyboard "
> + "response"));
> + goto prompt_error;
> + }
> +
> + /* compose the instruction buffer with this prompt */
> + if (virBufferUse(&buff) > 0) {
> + virBufferAddBuffer(&prompt_buff, &buff);
> + virBufferAddChar(&prompt_buff, '\n');
> + }
> + virBufferAdd(&prompt_buff, promptStr,
> + virLengthForPromptString(promptStr));
> +
> + if (virBufferCheckError(&prompt_buff) < 0)
> + goto prompt_error;
> +
> + prompt = virBufferContentAndReset(&prompt_buff);
> +
> + memset(&retr_passphrase, 0, sizeof(virConnectCredential));
> + retr_passphrase.type = cred_type;
> + retr_passphrase.prompt = prompt;
> +
> + if (retr_passphrase.type == -1) {
> + virReportError(VIR_ERR_LIBSSH, "%s",
> + _("no suitable callback for input of key "
> + "passphrase"));
> + goto prompt_error;
> + }
> +
> + if (sess->cred->cb(&retr_passphrase, 1, sess->cred->cbdata)) {
> + virReportError(VIR_ERR_LIBSSH, "%s",
> + _("failed to retrieve keyboard interactive "
> + "result: callback has failed"));
> + goto prompt_error;
> + }
> +
> + VIR_FREE(prompt);
> +
> + ret = ssh_userauth_kbdint_setanswer(sess->session, iprompt,
> + retr_passphrase.result);
> + VIR_DISPOSE_STRING(retr_passphrase.result);
> + if (ret < 0) {
> + errmsg = ssh_get_error(sess->session);
> + virReportError(VIR_ERR_AUTH_FAILED,
> + _("authentication failed: %s"), errmsg);
> + goto prompt_error;
> + }
Maybe it would be better to extract this into a function since this
looks rather complex.
> +
> + continue;
> +
> + prompt_error:
> + VIR_FREE(prompt);
> + virBufferFreeAndReset(&prompt_buff);
> + virBufferFreeAndReset(&buff);
> + return SSH_AUTH_ERROR;
> + }
> +
> + virBufferFreeAndReset(&buff);
> +
> + ret = ssh_userauth_kbdint(sess->session, NULL, NULL);
> + ++try;
> + if (ret == SSH_AUTH_DENIED && (priv->tries < 0 || try < priv->tries))
> + goto again;
> + }
> +
> + if (ret == SSH_AUTH_ERROR) {
> + /* error path */
> + errmsg = ssh_get_error(sess->session);
> + virReportError(VIR_ERR_AUTH_FAILED,
> + _("authentication failed: %s"), errmsg);
> + }
> +
> + return ret;
> +}
[...]
> +
> +/* validate if all required parameters are configured */
> +static int
> +virNetLibsshValidateConfig(virNetLibsshSessionPtr sess)
> +{
> + size_t i;
> + bool has_auths = false;
> +
> + for (i = 0; i < ARRAY_CARDINALITY(sess->auths); ++i) {
ARRAY_CARDINALITY can be used only on static arrays. This will always
iterate only the first element since it's used on a dynamically alocated
object.
> + if (sess->auths[i]) {
> + has_auths = true;
> + break;
> + }
> + }
> + if (!has_auths) {
> + virReportError(VIR_ERR_LIBSSH, "%s",
> + _("No authentication methods and credentials "
> + "provided"));
> + return -1;
> + }
> +
> + if (!sess->channelCommand) {
> + virReportError(VIR_ERR_LIBSSH, "%s",
> + _("No channel command provided"));
> + return -1;
> + }
> +
> + if (sess->hostKeyVerify != VIR_NET_LIBSSH_HOSTKEY_VERIFY_IGNORE) {
> + if (!sess->hostname) {
> + virReportError(VIR_ERR_LIBSSH, "%s",
> + _("Hostname is needed for host key verification"));
> + return -1;
> + }
> + }
> +
> + /* everything ok */
> + return 0;
> +}
> +
> +/* ### PUBLIC API ### */
[...]
> +void
> +virNetLibsshSessionAuthReset(virNetLibsshSessionPtr sess)
This function is not used anywhere in this series.
> +{
> + virObjectLock(sess);
> + virNetLibsshSessionAuthMethodsFree(sess);
> + virObjectUnlock(sess);
> +}
> +
[...]
> +int
> +virNetLibsshSessionSetChannelCommand(virNetLibsshSessionPtr sess,
> + const char *command)
This header is misaligned.
> +{
> + int ret = 0;
> + virObjectLock(sess);
> +
> + VIR_FREE(sess->channelCommand);
> +
> + if (VIR_STRDUP(sess->channelCommand, command) < 0)
> + ret = -1;
> +
> + virObjectUnlock(sess);
> + return ret;
> +}
> +
[...]
The rest looks good or at least similar to the libssh2 impl. The
semantics with getting the passprhase for a sshkey need to be fixed
though.
Peter
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20161101/fc359c21/attachment-0001.sig>
More information about the libvir-list
mailing list