Re: [Qemu-devel] [PATCH] monitor: Add whitelist support for QMP commands

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 3205 bytes --]

On 1/31/19 2:26 PM, Julia Suvorova via Qemu-devel wrote:
> The whitelist option allows to run a reduced monitor with a subset of
> QMP commands. This allows the monitor to run in secure mode, which is
> convenient for sending commands via the WebSocket monitor using the
> web UI. This is planned to be done on micro:bit board.
> 
> The list of allowed commands should be written to a file, one per line.
> The command line will look like this:
>     -mon chardev_name,mode=control,whitelist=path_to_file
> 
> Signed-off-by: Julia Suvorova <jusual@mail.ru>
> ---

>  
> -void monitor_init(Chardev *chr, int flags)
> +static void process_whitelist_file(Monitor *mon, const char *whitelist_file)
> +{
> +    char cmd_name[256];
> +    FILE *fd = fopen(whitelist_file, "r");

If you use qemu_open() here (followed by fdopen if you still prefer
fscanf over read), then you can support "/dev/fdset/NNN" to
auto-magically support someone passing in the whitelist via an inherited
file descriptor, rather than having to be somewhere on disk that qemu
can directly open().

> +
> +    if (fd == NULL) {
> +        error_report("Could not open whitelist file: %s", strerror(errno));
> +        exit(1);
> +    }
> +
> +    mon->whitelist = g_hash_table_new_full(g_str_hash,
> +                                           g_str_equal,
> +                                           g_free,
> +                                           NULL);
> +
> +    g_hash_table_add(mon->whitelist, g_strdup("qmp_capabilities"));
> +    g_hash_table_add(mon->whitelist, g_strdup("query-commands"));
> +
> +    while (fscanf(fd, "%255s", cmd_name) == 1) {

%255s fits your cmd_name array declaration and stops consuming at either
255 bytes or at the first whitespace encountered, but where do you check
for overflow from a file that passes more than 255 non-whitespace bytes
without a newline?  Also, this is a bit sloppy in that it skips all
leading whitespace, rather than ensuring that the user actually passed
newline-separated command names.  Does glib provide any interfaces for
more easily reading in an array of lines from a file?


> +++ b/qemu-options.hx
> @@ -3195,12 +3195,16 @@ Like -qmp but uses pretty JSON formatting.
>  ETEXI
>  
>  DEF("mon", HAS_ARG, QEMU_OPTION_mon, \
> -    "-mon [chardev=]name[,mode=readline|control][,pretty[=on|off]]\n", QEMU_ARCH_ALL)
> +    "-mon [chardev=]name[,mode=readline|control][,pretty[=on|off]]" \
> +    "[,whitelist=file]\n", QEMU_ARCH_ALL)
>  STEXI
> -@item -mon [chardev=]name[,mode=readline|control][,pretty[=on|off]]
> +@item -mon [chardev=]name[,mode=readline|control][,pretty[=on|off]][,whitelist=@var{file}]
>  @findex -mon
>  Setup monitor on chardev @var{name}. @code{pretty} turns on JSON pretty printing
>  easing human reading and debugging.
> +The @code{whitelist} option disables all commands except those specified in
> +@var{file}. The file must contain one command name per line. This option is only
> +avaliable in 'control' mode.

s/avaliable/available/

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]