On Sat, Aug 11, 2012 at 11:20:59PM +0200, Peter Krempa wrote:
diff --git a/include/libvirt/virterror.h
b/include/libvirt/virterror.h
index 913fc5d..9c751b6 100644
--- a/include/libvirt/virterror.h
+++ b/include/libvirt/virterror.h
@@ -110,6 +110,7 @@ typedef enum {
VIR_FROM_AUTH = 46, /* Error from auth handling */
VIR_FROM_DBUS = 47, /* Error from DBus */
VIR_FROM_PARALLELS = 48, /* Error from Parallels */
+ VIR_FROM_LIBSSH = 49, /* Error from libssh2 connection transport */
# ifdef VIR_ENUM_SENTINELS
VIR_ERR_DOMAIN_LAST
@@ -279,6 +280,7 @@ typedef enum {
VIR_ERR_BLOCK_COPY_ACTIVE = 83, /* action prevented by block copy job */
VIR_ERR_OPERATION_UNSUPPORTED = 84, /* The requested operation is not
supported */
+ VIR_ERR_LIBSSH2_ERROR = 85, /* error in libssh2 transport driver */
Using '_ERROR' as a suffix is redundant here - just call it VIR_ERR_LIBSSH2
diff --git a/src/rpc/virnetlibssh2session.c
b/src/rpc/virnetlibssh2session.c
+struct _virNetLibSSH2AuthMethod {
+ virNetLibSSH2AuthMethods method;
+ const char *username;
+ const char *password;
+ const char *filename;
The strings here are all dynamically allocated and freed,
so these shouldn't be marked 'const'.
+struct _virNetLibSSH2Session {
+ virObject object;
+ virNetLibSSH2SessionState state;
+ virMutex lock;
+
+ /* libssh2 internal stuff */
+ LIBSSH2_SESSION *session;
+ LIBSSH2_CHANNEL *channel;
+ LIBSSH2_KNOWNHOSTS *knownHosts;
+ LIBSSH2_AGENT *agent;
+
+ /* for host key checking */
+ virNetLibSSH2HostkeyVerify hostKeyVerify;
+ const char *knownHostsFile;
+ const char *hostname;
Same as above - remove the const
+ int port;
+
+ /* authentication stuff */
+ virConnectAuthPtr cred;
+ virNetLibSSH2AuthCallbackError authCbErr;
+ size_t nauths;
+ virNetLibSSH2AuthMethodPtr *auths;
+
+ /* channel stuff */
+ const char *channelCommand;
Remove the const
+ int channelCommandReturnValue;
+
+ /* read cache */
+ char rbuf[VIR_NET_LIBSSH2_BUFFER_SIZE];
+ size_t bufUsed;
+ size_t bufStart;
+};
+
+static virClassPtr virNetLibSSH2SessionClass;
+static int
+virNetLibSSH2SessionOnceInit(void)
+{
+ if (!(virNetLibSSH2SessionClass = virClassNew("virNetLibSSH2Session",
+ sizeof(virNetLibSSH2Session),
+ virNetLibSSH2SessionDispose)))
+ return -1;
+
+ return 0;
+}
+static int
+virNetLibSSH2AuthenticatePrivkey(virNetLibSSH2SessionPtr sess,
+ virNetLibSSH2AuthMethodPtr priv)
+{
+ virConnectCredential retr_passphrase;
+ int i;
+ char *errmsg;
+ int ret;
+
+ /* try open the key with no password */
+ if ((ret = libssh2_userauth_publickey_fromfile(sess->session,
+ priv->username,
+ NULL,
+ priv->filename,
+ priv->password)) == 0)
+ return 0; /* success */
+
+ if (priv->password ||
+ ret == LIBSSH2_ERROR_PUBLICKEY_UNRECOGNIZED ||
+ ret == LIBSSH2_ERROR_AUTHENTICATION_FAILED) {
+ libssh2_session_last_error(sess->session, &errmsg, NULL, 0);
+ virReportError(VIR_ERR_AUTH_FAILED,
+ _("authentication with private key '%s' "
+ "has failed: %s"),
+ priv->filename, errmsg);
+ return 1; /* auth failed */
+ }
+
+ /* request user's key password */
+ if (!sess->cred || !sess->cred->cb) {
+ virReportError(VIR_ERR_LIBSSH2_ERROR, "%s",
+ _("No user interaction callback provided: "
+ "Can't retrieve private key passphrase"));
+ return -1;
+ }
+
+ memset(&retr_passphrase, 0, sizeof(virConnectCredential));
+ retr_passphrase.type = -1;
+
+ for (i = 0; i < sess->cred->ncredtype; i++) {
+ if (sess->cred->credtype[i] == VIR_CRED_PASSPHRASE ||
+ sess->cred->credtype[i] == VIR_CRED_NOECHOPROMPT) {
+ retr_passphrase.type = sess->cred->credtype[i];
+ break;
+ }
+ }
+
+ if (retr_passphrase.type < 0) {
+ virReportError(VIR_ERR_LIBSSH2_ERROR, "%s",
+ _("no suitable method to retrieve key passphrase"));
+ return -1;
+ }
+
+ if (virAsprintf((char **)&retr_passphrase.prompt,
+ _("Passphrase for key '%s'"),
+ priv->filename) < 0) {
+ virReportOOMError();
+ return -1;
+ }
+
+ if (sess->cred->cb(&retr_passphrase, 1, sess->cred->cbdata)) {
+ virReportError(VIR_ERR_LIBSSH2_ERROR, "%s",
+ _("failed to retrieve private key passphrase: "
+ "callback has failed"));
+ VIR_FREE(retr_passphrase.prompt);
+ return -1;
+ }
+
+ VIR_FREE(retr_passphrase.prompt);
+
+ ret = libssh2_userauth_publickey_fromfile(sess->session,
+ priv->username,
+ NULL,
+ priv->filename,
+ retr_passphrase.result);
+
+ VIR_FREE(retr_passphrase.result);
+
+ if (ret < 0) {
+ libssh2_session_last_error(sess->session, &errmsg, NULL, 0);
+ virReportError(VIR_ERR_AUTH_FAILED,
+ _("authentication with private key '%s' "
+ "has failed: %s"),
+ priv->filename, errmsg);
+
+ if (ret == LIBSSH2_ERROR_PUBLICKEY_UNRECOGNIZED ||
+ ret == LIBSSH2_ERROR_AUTHENTICATION_FAILED)
+ return 1;
+ else
+ return -1;
+ }
+
+ return 0;
+}
+
+/* perform tunelled password authentication
+ *
+ * Returns: 0 on success
+ * 1 on authentication failure
+ * -1 on error
+ */
+static int
+virNetLibSSH2AuthenticatePassword(virNetLibSSH2SessionPtr sess,
+ virNetLibSSH2AuthMethodPtr priv)
+{
+ char *errmsg;
+ int ret;
+
+ /* tunelled password authentication */
+ if ((ret = libssh2_userauth_password(sess->session,
+ priv->username,
+ priv->password)) < 0) {
+ libssh2_session_last_error(sess->session, &errmsg, NULL, 0);
+ virReportError(VIR_ERR_AUTH_FAILED,
+ _("tunelled password authentication failed: %s"),
+ errmsg);
+
+ if (ret == LIBSSH2_ERROR_AUTHENTICATION_FAILED)
+ return 1;
+ else
+ return -1;
+ }
+ /* auth success */
+ return 0;
+}
+
+/* perform keyboard interactive authentication
+ *
+ * Returns: 0 on success
+ * 1 on authentication failure
+ * -1 on error
+ */
+static int
+virNetLibSSH2AuthenticateKeyboardInteractive(virNetLibSSH2SessionPtr sess,
+ virNetLibSSH2AuthMethodPtr priv)
+{
+ char *errmsg;
+ int ret;
+
+ if (!sess->cred || !sess->cred->cb) {
+ virReportError(VIR_ERR_LIBSSH2_ERROR, "%s",
+ _("Can't perform keyboard-interactive authentication:
"
+ "Authentication callback not provided "));
+ return -1;
+ }
+
+ /* try the auth infinite number of times, the server should break the
+ * connection if maximum number of bad auth tries is exceeded */
+ while (priv->tries < 0 || priv->tries-- > 0) {
If this is supposed to be infinit as the comment suggests, then
the normal paradigm is
while (1) {
+ ret =
libssh2_userauth_keyboard_interactive(sess->session,
+ priv->username,
+ virNetLibSSH2KbIntCb);
+
+ /* check for errors while calling the callback */
+ switch (sess->authCbErr) {
+ case VIR_NET_LIBSSH2_AUTHCB_NO_METHOD:
+ virReportError(VIR_ERR_LIBSSH2_ERROR, "%s",
+ _("no suitable method to retrieve "
+ "authentication cretentials"));
+ return -1;
+ case VIR_NET_LIBSSH2_AUTHCB_OOM:
+ virReportOOMError();
+ return -1;
+ case VIR_NET_LIBSSH2_AUTHCB_RETR_ERR:
+ virReportError(VIR_ERR_LIBSSH2_ERROR, "%s",
+ _("failed to retrieve credentials"));
+ return -1;
+ case VIR_NET_LIBSSH2_AUTHCB_OK:
+ /* everything went fine, let's continue */
+ break;
+ }
+
+ if (ret == 0)
+ /* authentication succeeded */
+ return 0;
+
+ if (ret == LIBSSH2_ERROR_AUTHENTICATION_FAILED)
+ continue; /* try again */
+
+ if (ret < 0) {
+ libssh2_session_last_error(sess->session, &errmsg, NULL, 0);
+ virReportError(VIR_ERR_AUTH_FAILED,
+ _("keyboard interactive authentication failed:
%s"),
+ errmsg);
+ return -1;
+ }
+ }
+ libssh2_session_last_error(sess->session, &errmsg, NULL, 0);
+ virReportError(VIR_ERR_AUTH_FAILED,
+ _("keyboard interactive authentication failed: %s"),
+ errmsg);
+ return 1;
+}
+int
+virNetLibSSH2SessionAuthAddKeyboardAuth(virNetLibSSH2SessionPtr sess,
+ const char *username,
+ int tries)
+{
+ virNetLibSSH2AuthMethodPtr auth;
+ char *user = NULL;
+
+ if (!username) {
+ virReportError(VIR_ERR_LIBSSH2_ERROR, "%s",
+ _("Username must be provided "
+ "for ssh agent authentication"));
+ return -1;
+ }
+
+ virMutexLock(&sess->lock);
+
+ if (!(user = strdup(username)))
+ goto no_memory;
+
+
+ if (!(auth = virNetLibSSH2SessionAuthMethodNew(sess))) {
+ virReportOOMError();
+ virMutexUnlock(&sess->lock);
+ return -1;
'goto no_memory' otherwise you leak 'user'
+ }
+
+ auth->username = user;
+ auth->tries = tries;
+ auth->method = VIR_NET_LIBSSH2_AUTH_KEYBOARD_INTERACTIVE;
+
+ virMutexUnlock(&sess->lock);
+ return 0;
+
+no_memory:
+ VIR_FREE(user);
+ virReportOOMError();
+ virMutexUnlock(&sess->lock);
+ return -1;
+
+}
+
+int
+virNetLibSSH2SessionSetHostKeyVerification(virNetLibSSH2SessionPtr sess,
+ const char *hostname,
+ int port,
+ const char *hostsfile,
+ bool readonly,
+ virNetLibSSH2HostkeyVerify opt)
+{
+ char *errmsg;
+
+ virMutexLock(&sess->lock);
+
+ sess->port = port;
+ sess->hostKeyVerify = opt;
+
+ VIR_FREE(sess->hostname);
+
+ if (hostname && !(sess->hostname = strdup(hostname)))
+ goto no_memory;
+
+ /* load the known hosts file */
+ if (hostsfile) {
+ if (libssh2_knownhost_readfile(sess->knownHosts,
+ hostsfile,
+ LIBSSH2_KNOWNHOST_FILE_OPENSSH) < 0) {
+ libssh2_session_last_error(sess->session, &errmsg, NULL, 0);
+ virReportError(VIR_ERR_LIBSSH2_ERROR,
+ _("unable to load knownhosts file '%s':
%s"),
+ hostsfile, errmsg);
+ }
A return -1 or goto error missing here ?
+
+ /* set filename only if writing to the known hosts file is requested */
+
+ if (!readonly) {
+ VIR_FREE(sess->knownHostsFile);
+ if ((sess->knownHostsFile = strdup(hostsfile)) == NULL)
+ goto no_memory;
+ }
+ }
+
+ virMutexUnlock(&sess->lock);
+ return 0;
+
+no_memory:
+ virMutexUnlock(&sess->lock);
+ virReportOOMError();
+ return -1;
+}
+
+/* do a read from a ssh channel, used instead of normal read on
socket */
+ssize_t
+virNetLibSSH2ChannelRead(virNetLibSSH2SessionPtr sess,
+ char *buf,
+ size_t len)
+{
+ ssize_t ret = -1;
+ ssize_t read_n = 0;
+
+ virMutexLock(&sess->lock);
+
+ if (sess->state != VIR_NET_LIBSSH2_STATE_HANDSHAKE_COMPLETE) {
+ if (sess->state == VIR_NET_LIBSSH2_STATE_ERROR_REMOTE)
+ virReportError(VIR_ERR_LIBSSH2_ERROR,
+ _("Remote program terminated "
+ "with non-zero code: %d"),
+ sess->channelCommandReturnValue);
+ else
+ virReportError(VIR_ERR_LIBSSH2_ERROR, "%s",
+ _("Tried to write socket in error state"));
+
+ virMutexUnlock(&sess->lock);
+ return -1;
+ }
+
+ if (sess->bufUsed > 0) {
+ /* copy the rest (or complete) internal buffer to the output buffer */
+ memcpy(buf,
+ sess->rbuf + sess->bufStart,
+ len > sess->bufUsed?sess->bufUsed:len);
Can you add whitespace around the '?' and ':'
+
+ if (len >= sess->bufUsed) {
+ read_n = sess->bufUsed;
+
+ sess->bufStart = 0;
+ sess->bufUsed = 0;
+ } else {
+ read_n = len;
+ sess->bufUsed -= len;
+ sess->bufStart += len;
+
+ goto success;
+ }
+ }
+
+ /* continue reading into the buffer supplied */
+ if (read_n < len) {
+ ret = libssh2_channel_read(sess->channel,
+ buf + read_n,
+ len - read_n);
+
+ if (ret == LIBSSH2_ERROR_EAGAIN)
+ goto success;
+
+ if (ret < 0)
+ goto error;
+
+ read_n += ret;
+ }
+
+ /* try to read something into the internal buffer */
+ if (sess->bufUsed == 0) {
+ ret = libssh2_channel_read(sess->channel,
+ sess->rbuf,
+ VIR_NET_LIBSSH2_BUFFER_SIZE);
+
+ if (ret == LIBSSH2_ERROR_EAGAIN)
+ goto success;
+
+ if (ret < 0)
+ goto error;
+
+ sess->bufUsed = ret;
+ sess->bufStart = 0;
+ }
+
+ if (read_n == 0) {
+ /* get rid of data in stderr stream */
+ ret = libssh2_channel_read_stderr(sess->channel,
+ sess->rbuf,
+ VIR_NET_LIBSSH2_BUFFER_SIZE - 1);
+ if (ret > 0) {
+ sess->rbuf[ret] = '\0';
+ VIR_DEBUG("flushing stderr, data='%s'", sess->rbuf);
+ }
+ }
+
+ if (libssh2_channel_eof(sess->channel)) {
+ if (libssh2_channel_get_exit_status(sess->channel)) {
+ virReportError(VIR_ERR_LIBSSH2_ERROR,
+ _("Remote command terminated with non-zero code:
%d"),
+ libssh2_channel_get_exit_status(sess->channel));
+ sess->channelCommandReturnValue =
libssh2_channel_get_exit_status(sess->channel);
+ sess->state = VIR_NET_LIBSSH2_STATE_ERROR_REMOTE;
+ virMutexUnlock(&sess->lock);
+ return -1;
+ }
+
+ sess->state = VIR_NET_LIBSSH2_STATE_CLOSED;
+ virMutexUnlock(&sess->lock);
+ return -1;
+ }
+
+success:
+ virMutexUnlock(&sess->lock);
+ return read_n;
+
+error:
+ sess->state = VIR_NET_LIBSSH2_STATE_ERROR;
+ virMutexUnlock(&sess->lock);
+ return ret;
+}
+
diff --git a/src/rpc/virnetlibssh2session.h
b/src/rpc/virnetlibssh2session.h
new file mode 100644
index 0000000..88d4307
--- /dev/null
+++ b/src/rpc/virnetlibssh2session.h
+# include "internal.h"
+
+typedef struct _virNetLibSSH2Session virNetLibSSH2Session;
+typedef virNetLibSSH2Session *virNetLibSSH2SessionPtr;
+
+virNetLibSSH2SessionPtr virNetLibSSH2SessionNew(void);
+void virNetLibSSH2SessionFree(virNetLibSSH2SessionPtr sess);
+
+typedef enum {
+ VIR_NET_LIBSSH2_HOSTKEY_VERIFY_NORMAL,
+ VIR_NET_LIBSSH2_HOSTKEY_VERIFY_AUTO_ADD,
+ VIR_NET_LIBSSH2_HOSTKEY_VERIFY_IGNORE
+} virNetLibSSH2HostkeyVerify;
+
+int virNetLibSSH2SessionSetChannelCommand(virNetLibSSH2SessionPtr sess,
+ const char *command);
+
+void virNetLibSSH2SessionAuthReset(virNetLibSSH2SessionPtr sess);
+
+int virNetLibSSH2SessionAuthSetCallback(virNetLibSSH2SessionPtr sess,
+ virConnectAuthPtr auth);
+
+int virNetLibSSH2SessionAuthAddPasswordAuth(virNetLibSSH2SessionPtr sess,
+ const char *username,
+ const char *password);
+
+int virNetLibSSH2SessionAuthAddAgentAuth(virNetLibSSH2SessionPtr sess,
+ const char *username);
+
+int virNetLibSSH2SessionAuthAddPrivKeyAuth(virNetLibSSH2SessionPtr sess,
+ const char *username,
+ const char *keyfile,
+ const char *password);
+
+int virNetLibSSH2SessionAuthAddKeyboardAuth(virNetLibSSH2SessionPtr sess,
+ const char *username,
+ int tries);
+
+int virNetLibSSH2SessionSetHostKeyVerification(virNetLibSSH2SessionPtr sess,
+ const char *hostname,
+ int port,
+ const char *hostsfile,
+ bool readonly,
+ virNetLibSSH2HostkeyVerify opt);
+
+int virNetLibSSH2SessionConnect(virNetLibSSH2SessionPtr sess,
+ int sock);
+
+ssize_t virNetLibSSH2ChannelRead(virNetLibSSH2SessionPtr sess,
+ char *buf,
+ size_t len);
+
+ssize_t virNetLibSSH2ChannelWrite(virNetLibSSH2SessionPtr sess,
+ const char *buf,
+ size_t len);
+
+bool virNetLibSSH2SessionHasCachedData(virNetLibSSH2SessionPtr sess);
+
+#endif /* ___VIR_NET_LIBSSH2_SESSION_H_ */
I think I'd be inclined to change the filename and API names throughout
this patch to just use 'ssh' instead of 'libssh2'. eg
virNetSSHSessionHasCachedData
and virnetsshsession.{c,h}
For comparison see the SASL or TLS code which uses a naming that is
independant of the implementation (Cyrus-SASL / GNU-TLS).
ACK if you rename the APIs and fix the couple of minor issues mentioned
inline
Daniel
--
|:
http://berrange.com -o-
http://www.flickr.com/photos/dberrange/ :|
|:
http://libvirt.org -o-
http://virt-manager.org :|
|:
http://autobuild.org -o-
http://search.cpan.org/~danberr/ :|
|:
http://entangle-photo.org -o-
http://live.gnome.org/gtk-vnc :|