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

[Peter Lieven <pl@kamp.de>]

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 <pl@kamp.de>
>> ---
>>   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.

Peter