[Qemu-devel] [PATCH] Add -f FMT / --format FMT arg to qemu-nbd

[Qemu-devel] [PATCH] Add -f FMT / --format FMT arg to qemu-nbd

[Anthony Liguori]

From: "Daniel P. Berrange" <berrange@redhat.com>

Currently the qemu-nbd program will auto-detect the format of
any disk it is given. This behaviour is known to be insecure.
For example, if qemu-nbd initially exposes a 'raw' file to an
unprivileged app, and that app runs

   'qemu-img create -f qcow2 -o backing_file=/etc/shadow /dev/nbd0'

then the next time the app is started, the qemu-nbd will now
detect it as a 'qcow2' file and expose /etc/shadow to the
unprivileged app.

The only way to avoid this is to explicitly tell qemu-nbd what
disk format to use on the command line, completely disabling
auto-detection. This patch adds a '-f' / '--format' arg for
this purpose, mirroring what is already available via qemu-img
and qemu commands.

  qemu-nbd --format raw -p 9000 evil.img

will now always use raw, regardless of what format 'evil.img'
looks like it contains

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
[Use errx, not err. - Paolo]
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
 qemu-nbd.c    | 20 ++++++++++++++++++--
 qemu-nbd.texi |  2 ++
 2 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/qemu-nbd.c b/qemu-nbd.c
index ca722ed..9c31d45 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -306,6 +306,7 @@ static void nbd_accept(void *opaque)
 int main(int argc, char **argv)
 {
     BlockDriverState *bs;
+    BlockDriver *drv;
     off_t dev_offset = 0;
     uint32_t nbdflags = 0;
     bool disconnect = false;
@@ -313,7 +314,7 @@ int main(int argc, char **argv)
     char *device = NULL;
     int port = NBD_DEFAULT_PORT;
     off_t fd_size;
-    const char *sopt = "hVb:o:p:rsnP:c:dvk:e:t";
+    const char *sopt = "hVb:o:p:rsnP:c:dvk:e:f:t";
     struct option lopt[] = {
         { "help", 0, NULL, 'h' },
         { "version", 0, NULL, 'V' },
@@ -333,6 +334,7 @@ int main(int argc, char **argv)
 #endif
         { "discard", 1, NULL, QEMU_NBD_OPT_DISCARD },
         { "shared", 1, NULL, 'e' },
+        { "format", 1, NULL, 'f' },
         { "persistent", 0, NULL, 't' },
         { "verbose", 0, NULL, 'v' },
         { NULL, 0, NULL, 0 }
@@ -351,6 +353,7 @@ int main(int argc, char **argv)
     bool seen_aio = false;
 #endif
     pthread_t client_thread;
+    const char *fmt = NULL;
 
     /* The client thread uses SIGTERM to interrupt the server.  A signal
      * handler ensures that "qemu-nbd -v -c" exits with a nice status code.
@@ -454,6 +457,9 @@ int main(int argc, char **argv)
                 errx(EXIT_FAILURE, "Shared device number must be greater than 0\n");
             }
             break;
+        case 'f':
+            fmt = optarg;
+            break;
 	case 't':
 	    persistent = 1;
 	    break;
@@ -555,9 +561,19 @@ int main(int argc, char **argv)
     bdrv_init();
     atexit(bdrv_close_all);
 
+    if (fmt) {
+        drv = bdrv_find_format(fmt);
+        if (!drv) {
+            errx(EXIT_FAILURE, "Unknown file format '%s'", fmt);
+        }
+    } else {
+        drv = NULL;
+    }
+
     bs = bdrv_new("hda");
     srcpath = argv[optind];
-    if ((ret = bdrv_open(bs, srcpath, NULL, flags, NULL)) < 0) {
+    ret = bdrv_open(bs, srcpath, NULL, flags, drv);
+    if (ret < 0) {
         errno = -ret;
         err(EXIT_FAILURE, "Failed to bdrv_open '%s'", argv[optind]);
     }
diff --git a/qemu-nbd.texi b/qemu-nbd.texi
index 5f3f3e3..6055ec6 100644
--- a/qemu-nbd.texi
+++ b/qemu-nbd.texi
@@ -45,6 +45,8 @@ Export QEMU disk image using NBD protocol.
   disconnect the specified device
 @item -e, --shared=@var{num}
   device can be shared by @var{num} clients (default @samp{1})
+@item -f, --format=@var{fmt}
+  force block driver for format @var{fmt} instead of auto-detecting
 @item -t, --persistent
   don't exit on the last connection
 @item -v, --verbose
-- 
1.8.0

Re: [Qemu-devel] [PATCH] Add -f FMT / --format FMT arg to qemu-nbd

[Anthony Liguori]

Anthony Liguori <aliguori@us.ibm.com> writes:

> From: "Daniel P. Berrange" <berrange@redhat.com>
>
> Currently the qemu-nbd program will auto-detect the format of
> any disk it is given. This behaviour is known to be insecure.
> For example, if qemu-nbd initially exposes a 'raw' file to an
> unprivileged app, and that app runs
>
>    'qemu-img create -f qcow2 -o backing_file=/etc/shadow /dev/nbd0'

This addresses CVE-2013-1922 which was publicly announced today.  The
commit is included in the 1.4.1 release that Mike is announcing shortly.

I'd also like to thank to Daniel for finding and reporting this issue
and the Red Hat security team for coordinating the disclosure.

Regards,

Anthony Liguori

> then the next time the app is started, the qemu-nbd will now
> detect it as a 'qcow2' file and expose /etc/shadow to the
> unprivileged app.
>
> The only way to avoid this is to explicitly tell qemu-nbd what
> disk format to use on the command line, completely disabling
> auto-detection. This patch adds a '-f' / '--format' arg for
> this purpose, mirroring what is already available via qemu-img
> and qemu commands.
>
>   qemu-nbd --format raw -p 9000 evil.img
>
> will now always use raw, regardless of what format 'evil.img'
> looks like it contains
>
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> [Use errx, not err. - Paolo]
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
> ---
>  qemu-nbd.c    | 20 ++++++++++++++++++--
>  qemu-nbd.texi |  2 ++
>  2 files changed, 20 insertions(+), 2 deletions(-)
>
> diff --git a/qemu-nbd.c b/qemu-nbd.c
> index ca722ed..9c31d45 100644
> --- a/qemu-nbd.c
> +++ b/qemu-nbd.c
> @@ -306,6 +306,7 @@ static void nbd_accept(void *opaque)
>  int main(int argc, char **argv)
>  {
>      BlockDriverState *bs;
> +    BlockDriver *drv;
>      off_t dev_offset = 0;
>      uint32_t nbdflags = 0;
>      bool disconnect = false;
> @@ -313,7 +314,7 @@ int main(int argc, char **argv)
>      char *device = NULL;
>      int port = NBD_DEFAULT_PORT;
>      off_t fd_size;
> -    const char *sopt = "hVb:o:p:rsnP:c:dvk:e:t";
> +    const char *sopt = "hVb:o:p:rsnP:c:dvk:e:f:t";
>      struct option lopt[] = {
>          { "help", 0, NULL, 'h' },
>          { "version", 0, NULL, 'V' },
> @@ -333,6 +334,7 @@ int main(int argc, char **argv)
>  #endif
>          { "discard", 1, NULL, QEMU_NBD_OPT_DISCARD },
>          { "shared", 1, NULL, 'e' },
> +        { "format", 1, NULL, 'f' },
>          { "persistent", 0, NULL, 't' },
>          { "verbose", 0, NULL, 'v' },
>          { NULL, 0, NULL, 0 }
> @@ -351,6 +353,7 @@ int main(int argc, char **argv)
>      bool seen_aio = false;
>  #endif
>      pthread_t client_thread;
> +    const char *fmt = NULL;
>  
>      /* The client thread uses SIGTERM to interrupt the server.  A signal
>       * handler ensures that "qemu-nbd -v -c" exits with a nice status code.
> @@ -454,6 +457,9 @@ int main(int argc, char **argv)
>                  errx(EXIT_FAILURE, "Shared device number must be greater than 0\n");
>              }
>              break;
> +        case 'f':
> +            fmt = optarg;
> +            break;
>  	case 't':
>  	    persistent = 1;
>  	    break;
> @@ -555,9 +561,19 @@ int main(int argc, char **argv)
>      bdrv_init();
>      atexit(bdrv_close_all);
>  
> +    if (fmt) {
> +        drv = bdrv_find_format(fmt);
> +        if (!drv) {
> +            errx(EXIT_FAILURE, "Unknown file format '%s'", fmt);
> +        }
> +    } else {
> +        drv = NULL;
> +    }
> +
>      bs = bdrv_new("hda");
>      srcpath = argv[optind];
> -    if ((ret = bdrv_open(bs, srcpath, NULL, flags, NULL)) < 0) {
> +    ret = bdrv_open(bs, srcpath, NULL, flags, drv);
> +    if (ret < 0) {
>          errno = -ret;
>          err(EXIT_FAILURE, "Failed to bdrv_open '%s'", argv[optind]);
>      }
> diff --git a/qemu-nbd.texi b/qemu-nbd.texi
> index 5f3f3e3..6055ec6 100644
> --- a/qemu-nbd.texi
> +++ b/qemu-nbd.texi
> @@ -45,6 +45,8 @@ Export QEMU disk image using NBD protocol.
>    disconnect the specified device
>  @item -e, --shared=@var{num}
>    device can be shared by @var{num} clients (default @samp{1})
> +@item -f, --format=@var{fmt}
> +  force block driver for format @var{fmt} instead of auto-detecting
>  @item -t, --persistent
>    don't exit on the last connection
>  @item -v, --verbose
> -- 
> 1.8.0

Re: [Qemu-devel] [PATCH] Add -f FMT / --format FMT arg to qemu-nbd

[Anthony Liguori]

Applied.  Thanks.

Regards,

Anthony Liguori