On 08/03/2012 08:03 AM, Peter Krempa wrote:
This patch adds helper functions to libssh2 that enable us to use libssh2 in conjunction with libvirt-native virNetSockets instead of using a spawned "ssh" client process.
This implemetation supports tunneled plaintext, keyboard-interactive,
s/implemtation/implementation/
private key, ssh agent based and null authentication. Libvirt's Auth callback is used for interaction with the user. (Keyboard interactive authentication, adding of host keys, private key passphrases). This enables seamless integration into the application using libvirt, as no helpers as "ssh-askpass" are needed.
Reading and writing of OpenSSH style "known_hosts" files is supported.
Communication is done using SSH exec channel, where the user may specify arbitrary command to be executed on the remote side and reads and writes to/from stdin/out are sent through the ssh channel. Usage of stderr is not supported.
As a bonus, this should (untested) add SSH support to libvirt clients running on Windows.
if test "$with_phyp" = "yes"; then AC_DEFINE_UNQUOTED([WITH_PHYP], 1, [whether IBM HMC / IVM driver is enabled]) fi +if test "$with_libssh2_transport" = "yes"; then + AC_DEFINE_UNQUOTED([HAVE_LIBSSH2], 1, [wether libssh2 transport is enabled])
s/wether/whether/
+++ b/src/Makefile.am @@ -1508,6 +1508,13 @@ libvirt_net_rpc_la_SOURCES = \ rpc/virnettlscontext.h rpc/virnettlscontext.c \ rpc/virkeepaliveprotocol.h rpc/virkeepaliveprotocol.c \ rpc/virkeepalive.h rpc/virkeepalive.c +if HAVE_LIBSSH2 +libvirt_net_rpc_la_SOURCES += \ + rpc/virnetlibsshcontext.h rpc/virnetlibsshcontext.c +else +EXTRA_DIST += \ + rpc/virnetlibsshcontext.h rpc/virnetlibsshcontext.c +endif
Doing it this way requires a separate .syms file. How hard would it be to provide stub symbols, so that we always compile and link against the file, but the file is a no-op when HAVE_LIBSSH2 is not available?
+++ b/src/rpc/virnetlibsshcontext.c
+/* keyboard interactive authentication callback */ +static void +virNetLibSSHKbIntCb(const char *name ATTRIBUTE_UNUSED, + int name_len ATTRIBUTE_UNUSED, + const char *instruction ATTRIBUTE_UNUSED, + int instruction_len ATTRIBUTE_UNUSED, + int num_prompts, + const LIBSSH2_USERAUTH_KBDINT_PROMPT *prompts, + LIBSSH2_USERAUTH_KBDINT_RESPONSE *responses, + void **opaque) +{
prompts is marked 'const', but...
+ /* fill data structures for auth callback */ + for (i = 0; i < num_prompts; i++) { + /* remove colon and trailing spaces from prompts, as default behavior + * of libvirt's auth callback is to add them */ + if ((tmp = strrchr(prompts[i].text, ':'))) + *tmp = '\0';
...you are modifying it in-place by using strrchr to cast away const. Is that an issue where you could cause a SEGV, and should be strdup'ing the prompt before modifying it?
+/* 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, not 0 otherwise + */ +static int +virNetLibSSHCheckHostKey(virNetLibSSHSessionPtr sess)
Is the return always negative on error?
+ /* format the host key into a nice userfriendly string. + * Sadly, there's no constant to describe the hash length, so + * we have to use a *MAGIC* constant. */ + for (i = 0; i < 16; i++) { + virBufferAsprintf(&buff, "%02X", (unsigned char) keyhash[i]);
Is the cast there to avoid unintentional sign-extension because keyhash[i] might be signed? If so, you could avoid the cast by telling asprintf that you are passing 1 byte in the first place with the 'hh' modifier.
+ if (i != 15) + virBufferAddChar(&buff, ':'); + }
Slightly more efficient to use: for (i = 0; i < 16; i++) virBufferAsprintf(&buff, "%02hhX:", keyhash[i]); virBufferTrim(&buff, ":", 1);
+ + if (STRNEQ_NULLABLE(askKey.result, _("yes"))) { + virReportError(VIR_ERR_LIBSSH_ERROR, + _("SSH host key for '%s' (%s) was not accepted"),
When parsing user input, don't you want a case-insensitive comparison? Also, what about users that want to type 'y' instead of 'yes'? And is comparing to a translated string wise? Without consulting LC_MESSAGES for the proper regex for the user's chosen locale, I'd almost feel safer with a check hard-coded to English 'y' and 'n' rather than to the arbitrary language _("yes"). I'm not quite sure how I would test all of the code, but the bulk of it looked sane by just glancing over it. Having not specifically coded with libssh2, I can't say if you were using the library API correctly without spending a lot longer on the review; but if it is possible to easily test the results, that would go a long way to convince me that the code itself is doing the right thing. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org