Patchwork block/curl: Add support for Secure Shell (ssh/sftp) block device

login
register
mail settings
Submitter Richard W.M. Jones
Date March 22, 2013, 1:04 p.m.
Message ID <20130322130455.GI1504@rhmail.home.annexia.org>
Download mbox | patch
Permalink /patch/230004/
State New
Headers show

Comments

Richard W.M. Jones - March 22, 2013, 1:04 p.m.
I got it working with Curl, patch attached.

However there are multiple issues (these are mainly notes for myself):

(1) libcurl cannot read the size of the file.  I had to hard-code
this.  This is probably just a shortcoming of libcurl (libssh2/sftp
itself can read the size of files).  Will try to work on a patch for
upstream.

(2) Fedora's curl (which is heavily patched) is broken in some way and
deadlocks itself.  Upstream curl from git works better.  I haven't yet
identified which patch/commit is responsible.

(3) ssh-agent authentication doesn't work.  It appears that either
ssh-agent itself doesn't like multiple connections from a single
process (qemu), or libcurl/libssh2 is having a problem with making
multiple connections out to ssh-agent.  If I disable ssh-agent auth,
it works.  Still investigating this.

(4) You must specify a user@ in the URL, else libcurl tries to
authenticate as user "".  I will see if I can send a fix for this
upstream.

(5) Although it gets much of the way through a boot of a guest, it
eventually segfaults.  Still investigating this.

(6) There are several more issues marked by XXX's in the code.

Rich.
Stefan Hajnoczi - March 22, 2013, 1:41 p.m.
On Fri, Mar 22, 2013 at 01:04:55PM +0000, Richard W.M. Jones wrote:
> 
> I got it working with Curl, patch attached.
> 
> However there are multiple issues (these are mainly notes for myself):
> 
> (1) libcurl cannot read the size of the file.  I had to hard-code
> this.  This is probably just a shortcoming of libcurl (libssh2/sftp
> itself can read the size of files).  Will try to work on a patch for
> upstream.
> 
> (2) Fedora's curl (which is heavily patched) is broken in some way and
> deadlocks itself.  Upstream curl from git works better.  I haven't yet
> identified which patch/commit is responsible.
> 
> (3) ssh-agent authentication doesn't work.  It appears that either
> ssh-agent itself doesn't like multiple connections from a single
> process (qemu), or libcurl/libssh2 is having a problem with making
> multiple connections out to ssh-agent.  If I disable ssh-agent auth,
> it works.  Still investigating this.
> 
> (4) You must specify a user@ in the URL, else libcurl tries to
> authenticate as user "".  I will see if I can send a fix for this
> upstream.
> 
> (5) Although it gets much of the way through a boot of a guest, it
> eventually segfaults.  Still investigating this.
> 
> (6) There are several more issues marked by XXX's in the code.

Thank you for improving libcurl!  You're making it better for everybody.

A lot of people go back to NIH when they hit limitations in existing
software.

Stefan
Richard W.M. Jones - March 25, 2013, 12:32 p.m.
On Fri, Mar 22, 2013 at 01:04:55PM +0000, Richard W.M. Jones wrote:
> I got it working with Curl, patch attached.
> 
> However there are multiple issues (these are mainly notes for myself):
> 
> (1) libcurl cannot read the size of the file.  I had to hard-code
> this.  This is probably just a shortcoming of libcurl (libssh2/sftp
> itself can read the size of files).  Will try to work on a patch for
> upstream.

After my holiday and in the cold of day I've had a long look at the
SFTP implementation in libcurl.

  https://github.com/bagder/curl/blob/master/lib/ssh.c

It's implemented as a huge state machine and simply implementing (1)
above is problematic (I believe we would have to reopen the connection
after our call to curl_easy_perform).

The larger issue is that the qemu curl block driver doesn't support
writes.  Now these could in theory be added.  Indeed curl does support
"random access" writes, although AFAICT you have to open a new
connection each time you want to seek backwards, and you can't read
and write over the same connection (so you'd need >= 1 connections for
reading and another >= 1 connections for writing).

I think I will continue with the pure libssh2-based block driver,
making it support all the missing features discussed earlier:

  http://www.mail-archive.com/qemu-devel@nongnu.org/msg161997.html

plus of course non-blocking AIO.

I think this way we'll end up with a much more robust, reliable and
easier to debug ssh implementation in qemu.

Rich.
Stefan Hajnoczi - March 25, 2013, 1:12 p.m.
On Mon, Mar 25, 2013 at 1:32 PM, Richard W.M. Jones <rjones@redhat.com> wrote:
> On Fri, Mar 22, 2013 at 01:04:55PM +0000, Richard W.M. Jones wrote:
>> I got it working with Curl, patch attached.
>>
>> However there are multiple issues (these are mainly notes for myself):
>>
>> (1) libcurl cannot read the size of the file.  I had to hard-code
>> this.  This is probably just a shortcoming of libcurl (libssh2/sftp
>> itself can read the size of files).  Will try to work on a patch for
>> upstream.
>
> After my holiday and in the cold of day I've had a long look at the
> SFTP implementation in libcurl.
>
>   https://github.com/bagder/curl/blob/master/lib/ssh.c
>
> It's implemented as a huge state machine and simply implementing (1)
> above is problematic (I believe we would have to reopen the connection
> after our call to curl_easy_perform).
>
> The larger issue is that the qemu curl block driver doesn't support
> writes.  Now these could in theory be added.  Indeed curl does support
> "random access" writes, although AFAICT you have to open a new
> connection each time you want to seek backwards, and you can't read
> and write over the same connection (so you'd need >= 1 connections for
> reading and another >= 1 connections for writing).
>
> I think I will continue with the pure libssh2-based block driver,
> making it support all the missing features discussed earlier:
>
>   http://www.mail-archive.com/qemu-devel@nongnu.org/msg161997.html
>
> plus of course non-blocking AIO.
>
> I think this way we'll end up with a much more robust, reliable and
> easier to debug ssh implementation in qemu.

Fair enough.

Stefan

Patch

diff --git a/block/curl.c b/block/curl.c
index 98947da..b236f7f 100644
--- a/block/curl.c
+++ b/block/curl.c
@@ -36,7 +36,7 @@ 
 
 #define PROTOCOLS (CURLPROTO_HTTP | CURLPROTO_HTTPS | \
                    CURLPROTO_FTP | CURLPROTO_FTPS | \
-                   CURLPROTO_TFTP)
+                   CURLPROTO_TFTP | CURLPROTO_SFTP)
 
 #define CURL_NUM_STATES 8
 #define CURL_NUM_ACB    8
@@ -305,6 +305,17 @@  static CURLState *curl_init_state(BDRVCURLState *s)
     curl_easy_setopt(state->curl, CURLOPT_NOSIGNAL, 1);
     curl_easy_setopt(state->curl, CURLOPT_ERRORBUFFER, state->errmsg);
     curl_easy_setopt(state->curl, CURLOPT_FAILONERROR, 1);
+#if defined(CURLSSH_AUTH_ANY)
+    curl_easy_setopt(state->curl, CURLOPT_SSH_AUTH_TYPES, CURLSSH_AUTH_ANY);
+#endif
+
+    /* XXX */
+    curl_easy_setopt (state->curl, CURLOPT_SSH_PRIVATE_KEYFILE,
+                      "/home/rjones/.ssh/id_rsa"); /* XXX */
+    curl_easy_setopt (state->curl, CURLOPT_SSH_PUBLIC_KEYFILE,
+                      "/home/rjones/.ssh/id_rsa.pub"); /* XXX */
+    curl_easy_setopt (state->curl, CURLOPT_KEYPASSWD,
+                      "XXX PUT YOUR PASSPHRASE HERE XXX"); /* XXX */
 
     /* Restrict supported protocols to avoid security issues in the more
      * obscure protocols.  For example, do not allow POP3/SMTP/IMAP see
@@ -406,8 +417,12 @@  static int curl_open(BlockDriverState *bs, const char *filename, int flags)
     curl_easy_setopt(state->curl, CURLOPT_NOBODY, 0);
     if (d)
         s->len = (size_t)d;
+#if 0
     else if(!s->len)
         goto out;
+#else
+    s->len = 8589934592; /* XXX */
+#endif
     DPRINTF("CURL: Size = %zd\n", s->len);
 
     curl_clean_state(state);
@@ -626,6 +641,18 @@  static BlockDriver bdrv_tftp = {
     .bdrv_aio_readv  = curl_aio_readv,
 };
 
+static BlockDriver bdrv_sftp = {
+    .format_name     = "sftp",
+    .protocol_name   = "sftp",
+
+    .instance_size   = sizeof(BDRVCURLState),
+    .bdrv_file_open  = curl_open,
+    .bdrv_close      = curl_close,
+    .bdrv_getlength  = curl_getlength,
+
+    .bdrv_aio_readv  = curl_aio_readv,
+};
+
 static void curl_block_init(void)
 {
     bdrv_register(&bdrv_http);
@@ -633,6 +660,7 @@  static void curl_block_init(void)
     bdrv_register(&bdrv_ftp);
     bdrv_register(&bdrv_ftps);
     bdrv_register(&bdrv_tftp);
+    bdrv_register(&bdrv_sftp);
 }
 
 block_init(curl_block_init);