Re: [Qemu-devel] [Qemu-block] [PATCH] block/nfs: add support for setting debug level

[Stefan Hajnoczi]

On Thu, Jun 25, 2015 at 03:26:46PM +0200, Peter Lieven wrote:
> Am 25.06.2015 um 15:18 schrieb Stefan Hajnoczi:
> >On Tue, Jun 23, 2015 at 10:12:15AM +0200, Peter Lieven wrote:
> >>upcoming libnfs versions will support logging debug messages. Add
> >>support for it in qemu through an URL parameter.
> >>
> >>Signed-off-by: Peter Lieven <address@hidden>
> >>---
> >>  block/nfs.c | 4 ++++
> >>  1 file changed, 4 insertions(+)
> >>
> >>diff --git a/block/nfs.c b/block/nfs.c
> >>index ca9e24e..f7388a3 100644
> >>--- a/block/nfs.c
> >>+++ b/block/nfs.c
> >>@@ -329,6 +329,10 @@ static int64_t nfs_client_open(NFSClient *client, 
> >>const char *filename,
> >>          } else if (!strcmp(qp->p[i].name, "readahead")) {
> >>              nfs_set_readahead(client->context, val);
> >>  #endif
> >>+#ifdef LIBNFS_FEATURE_DEBUG
> >>+        } else if (!strcmp(qp->p[i].name, "debug")) {
> >>+            nfs_set_debug(client->context, val);
> >>+#endif
> >>          } else {
> >>              error_setg(errp, "Unknown NFS parameter name: %s",
> >>                         qp->p[i].name);
> >Untrusted users may be able to set these options since they are encoded
> >in the URI.  I'm imagining a hosting or cloud scenario like OpenStack.
> >
> >A verbose debug level spams stderr and could consume a lot of disk
> >space.
> >
> >(The uid and gid options are probably okay since the NFS server cannot
> >trust the uid/gid coming from QEMU anyway.)
> >
> >I think we can merge this patch for QEMU 2.4 but I'd like to have a
> >discussion about the security risk of encoding libnfs options in the
> >URI.
> >
> >CCed Eric Blake in case libvirt is affected.
> >
> >Has anyone thought about this and what are the rules?
> 
> Good point. In general I think there should be some kind of sanitization of 
> the parameters
> before they are passed on to Qemu. In our use case the user cannot pass any 
> kind of URIs himself,
> but this might be different in other backends. The readahead value is as 
> dangerous as well
> if not sanitized.
> 
> I had a discussion with Ronnie in the past if we should encode parameters in 
> the URI or via environment
> like it is done in libiscsi. If I remember correctly we came up with the URI 
> parameters for better usability,
> but hadn't attack scenarios in mind.
> 
> I am also open to only allow uncritical parameters in the URI and pass others 
> via a new -nfs cmdline option.
> Or limit max readahead and max debug level settable via URI.

I'd feel safer if the option was in runtime_opts instead.  The the
management tool has to pass them explicitly and the end user cannot
influence them via the URI.

If an option is needed both at open and create time, then it must also
be parsed from nfs_file_create() opts.

Stefan

pgpOKIdkFCQRF.pgp
Description: PGP signature