3:46 p.m.
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(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org