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 ToscanoAttachment:
signature.asc
Description: This is a digitally signed message part.
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list