4:43 a.m.
On Tuesday, 1 November 2016 13:20:28 CET Peter Krempa wrote:
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.
Right. (Note the same fix should be done in the libssh2 transport too.)
> +
> +/* 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.
Unfortunately the libssh API only allows SHA1 and MD5:
enum ssh_publickey_hash_type {
SSH_PUBLICKEY_HASH_SHA1,
SSH_PUBLICKEY_HASH_MD5
};
libssh2 uses MD5, which is why I chose to do the same in this case.
I will change it to SHA1, should be slightly better.
> +void
> +virNetLibsshSessionAuthReset(virNetLibsshSessionPtr sess)
This function is not used anywhere in this series.
Neither is virNetSSHSessionAuthReset -- removed.
> +/* 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.
How "granular" should this be split?
Thanks,
--
Pino Toscano