From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:36796) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RYEpG-00085C-RS for qemu-devel@nongnu.org; Wed, 07 Dec 2011 05:34:15 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1RYEpC-0003EF-Pc for qemu-devel@nongnu.org; Wed, 07 Dec 2011 05:34:10 -0500 Received: from mx1.redhat.com ([209.132.183.28]:62883) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RYEpC-0003Dr-HC for qemu-devel@nongnu.org; Wed, 07 Dec 2011 05:34:06 -0500 Message-ID: <4EDF4119.5090300@redhat.com> Date: Wed, 07 Dec 2011 12:34:01 +0200 From: Dor Laor MIME-Version: 1.0 References: <1323230623-8709-1-git-send-email-mdroth@linux.vnet.ibm.com> In-Reply-To: <1323230623-8709-1-git-send-email-mdroth@linux.vnet.ibm.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 1/2] guest agent: add RPC blacklist command-line option Reply-To: dlaor@redhat.com List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Michael Roth Cc: aliguori@us.ibm.com, qemu-devel@nongnu.org On 12/07/2011 06:03 AM, Michael Roth wrote: > This adds a command-line option, -b/--blacklist, that accepts a > comma-seperated list of RPCs to disable, or prints a list of > available RPCs if passed "?". > > In consequence this also adds general blacklisting and RPC listing > facilities to the new QMP dispatch/registry facilities, should the > QMP monitor ever have a need for such a thing. Beyond run time disablement, how easy it is to compile out some of the general commands such as exec/file-handling? Security certifications like common criteria usually ask to compile out anything that might tamper security. > > Ideally, to avoid support/compatability issues in the future, > blacklisting guest agent functionality will be the exceptional > case, but we add the functionality here to handle guest administrators > with specific requirements. > > Signed-off-by: Michael Roth > --- > qapi/qmp-core.h | 3 +++ > qapi/qmp-dispatch.c | 4 ++++ > qapi/qmp-registry.c | 43 ++++++++++++++++++++++++++++++++++++++----- > qemu-ga.c | 37 ++++++++++++++++++++++++++++++++++--- > qerror.c | 4 ++++ > qerror.h | 3 +++ > 6 files changed, 86 insertions(+), 8 deletions(-) > > diff --git a/qapi/qmp-core.h b/qapi/qmp-core.h > index f1c26e4..3cf1781 100644 > --- a/qapi/qmp-core.h > +++ b/qapi/qmp-core.h > @@ -31,11 +31,14 @@ typedef struct QmpCommand > QmpCommandType type; > QmpCommandFunc *fn; > QTAILQ_ENTRY(QmpCommand) node; > + bool enabled; > } QmpCommand; > > void qmp_register_command(const char *name, QmpCommandFunc *fn); > QmpCommand *qmp_find_command(const char *name); > QObject *qmp_dispatch(QObject *request); > +void qmp_disable_command(const char *name); > +char **qmp_get_command_list(void); > > #endif > > diff --git a/qapi/qmp-dispatch.c b/qapi/qmp-dispatch.c > index 5584693..43f640a 100644 > --- a/qapi/qmp-dispatch.c > +++ b/qapi/qmp-dispatch.c > @@ -79,6 +79,10 @@ static QObject *do_qmp_dispatch(QObject *request, Error **errp) > error_set(errp, QERR_COMMAND_NOT_FOUND, command); > return NULL; > } > + if (!cmd->enabled) { > + error_set(errp, QERR_COMMAND_DISABLED, command); > + return NULL; > + } > > if (!qdict_haskey(dict, "arguments")) { > args = qdict_new(); > diff --git a/qapi/qmp-registry.c b/qapi/qmp-registry.c > index 5ff99cf..abafa34 100644 > --- a/qapi/qmp-registry.c > +++ b/qapi/qmp-registry.c > @@ -14,7 +14,7 @@ > > #include "qapi/qmp-core.h" > > -static QTAILQ_HEAD(, QmpCommand) qmp_commands = > +static QTAILQ_HEAD(QmpCommandList, QmpCommand) qmp_commands = > QTAILQ_HEAD_INITIALIZER(qmp_commands); > > void qmp_register_command(const char *name, QmpCommandFunc *fn) > @@ -24,17 +24,50 @@ void qmp_register_command(const char *name, QmpCommandFunc *fn) > cmd->name = name; > cmd->type = QCT_NORMAL; > cmd->fn = fn; > + cmd->enabled = true; > QTAILQ_INSERT_TAIL(&qmp_commands, cmd, node); > } > > QmpCommand *qmp_find_command(const char *name) > { > - QmpCommand *i; > + QmpCommand *cmd; > > - QTAILQ_FOREACH(i,&qmp_commands, node) { > - if (strcmp(i->name, name) == 0) { > - return i; > + QTAILQ_FOREACH(cmd,&qmp_commands, node) { > + if (strcmp(cmd->name, name) == 0) { > + return cmd; > } > } > return NULL; > } > + > +void qmp_disable_command(const char *name) > +{ > + QmpCommand *cmd; > + > + QTAILQ_FOREACH(cmd,&qmp_commands, node) { > + if (strcmp(cmd->name, name) == 0) { > + cmd->enabled = false; > + return; > + } > + } > +} > + > +char **qmp_get_command_list(void) > +{ > + QmpCommand *cmd; > + int count = 1; > + char **list_head, **list; > + > + QTAILQ_FOREACH(cmd,&qmp_commands, node) { > + count++; > + } > + > + list_head = list = g_malloc0(count * sizeof(char *)); > + > + QTAILQ_FOREACH(cmd,&qmp_commands, node) { > + *list = strdup(cmd->name); > + list++; > + } > + > + return list_head; > +} > diff --git a/qemu-ga.c b/qemu-ga.c > index 4932013..200bb15 100644 > --- a/qemu-ga.c > +++ b/qemu-ga.c > @@ -27,6 +27,7 @@ > #include "signal.h" > #include "qerror.h" > #include "error_int.h" > +#include "qapi/qmp-core.h" > > #define QGA_VIRTIO_PATH_DEFAULT "/dev/virtio-ports/org.qemu.guest_agent.0" > #define QGA_PIDFILE_DEFAULT "/var/run/qemu-ga.pid" > @@ -91,6 +92,8 @@ static void usage(const char *cmd) > " -v, --verbose log extra debugging information\n" > " -V, --version print version information and exit\n" > " -d, --daemonize become a daemon\n" > +" -b, --blacklist comma-seperated list of RPCs to disable (no spaces, \"?\"" > +" to list available RPCs)\n" > " -h, --help display this help and exit\n" > "\n" > "Report bugs to\n" > @@ -548,7 +551,7 @@ static void init_guest_agent(GAState *s) > > int main(int argc, char **argv) > { > - const char *sopt = "hVvdm:p:l:f:"; > + const char *sopt = "hVvdm:p:l:f:b:"; > const char *method = NULL, *path = NULL, *pidfile = QGA_PIDFILE_DEFAULT; > const struct option lopt[] = { > { "help", 0, NULL, 'h' }, > @@ -559,13 +562,16 @@ int main(int argc, char **argv) > { "method", 0, NULL, 'm' }, > { "path", 0, NULL, 'p' }, > { "daemonize", 0, NULL, 'd' }, > + { "blacklist", 0, NULL, 'b' }, > { NULL, 0, NULL, 0 } > }; > - int opt_ind = 0, ch, daemonize = 0; > + int opt_ind = 0, ch, daemonize = 0, i, j, len; > GLogLevelFlags log_level = G_LOG_LEVEL_ERROR | G_LOG_LEVEL_CRITICAL; > FILE *log_file = stderr; > GAState *s; > > + module_call_init(MODULE_INIT_QAPI); > + > while ((ch = getopt_long(argc, argv, sopt, lopt,&opt_ind)) != -1) { > switch (ch) { > case 'm': > @@ -595,6 +601,32 @@ int main(int argc, char **argv) > case 'd': > daemonize = 1; > break; > + case 'b': { > + char **list_head, **list; > + if (*optarg == '?') { > + list_head = list = qmp_get_command_list(); > + while (*list != NULL) { > + printf("%s\n", *list); > + g_free(*list); > + list++; > + } > + g_free(list_head); > + return 0; > + } > + for (j = 0, i = 0, len = strlen(optarg); i< len; i++) { > + if (optarg[i] == ',') { > + optarg[i] = 0; > + qmp_disable_command(&optarg[j]); > + g_debug("disabling command: %s",&optarg[j]); > + j = i + 1; > + } > + } > + if (j< i) { > + qmp_disable_command(&optarg[j]); > + g_debug("disabling command: %s",&optarg[j]); > + } > + break; > + } > case 'h': > usage(argv[0]); > return 0; > @@ -624,7 +656,6 @@ int main(int argc, char **argv) > ga_command_state_init_all(s->command_state); > ga_state = s; > > - module_call_init(MODULE_INIT_QAPI); > init_guest_agent(ga_state); > register_signal_handlers(); > > diff --git a/qerror.c b/qerror.c > index 656efc2..a998d2f 100644 > --- a/qerror.c > +++ b/qerror.c > @@ -65,6 +65,10 @@ static const QErrorStringTable qerror_table[] = { > .desc = "The command %(name) has not been found", > }, > { > + .error_fmt = QERR_COMMAND_DISABLED, > + .desc = "The command %(name) has been disabled for this instance", > + }, > + { > .error_fmt = QERR_DEVICE_ENCRYPTED, > .desc = "Device '%(device)' is encrypted", > }, > diff --git a/qerror.h b/qerror.h > index 161d654..0d3d4c5 100644 > --- a/qerror.h > +++ b/qerror.h > @@ -66,6 +66,9 @@ QError *qobject_to_qerror(const QObject *obj); > #define QERR_COMMAND_NOT_FOUND \ > "{ 'class': 'CommandNotFound', 'data': { 'name': %s } }" > > +#define QERR_COMMAND_DISABLED \ > + "{ 'class': 'CommandDisabled', 'data': { 'name': %s } }" > + > #define QERR_DEVICE_ENCRYPTED \ > "{ 'class': 'DeviceEncrypted', 'data': { 'device': %s } }" >