Re: [libvirt] [PATCH 13/18] tpm: Parse the capabilities supported by swtpm and swtpm_setup

On Tue, Jul 9, 2019 at 9:24 PM Stefan Berger <stefanb(a)linux.vnet.ibm.com&gt; wrote: > Run 'swtpm socket --print-capabilities' and > 'swtpm_setup --print-capabilities' to get the JSON object of the > features the programs are supporting and parse them into a bitmap. > > Signed-off-by: Stefan Berger <stefanb(a)linux.ibm.com&gt; > --- > src/conf/Makefile.inc.am | 6 ++ > src/conf/virtpm_conf.c | 36 ++++++++++++ > src/conf/virtpm_conf.h | 36 ++++++++++++ > src/libvirt_private.syms | 5 ++ > src/tpm/Makefile.inc.am | 5 +- > src/tpm/virtpm.c | 123 ++++++++++++++++++++++++++++++++++++++- > 6 files changed, 209 insertions(+), 2 deletions(-) > create mode 100644 src/conf/virtpm_conf.c > create mode 100644 src/conf/virtpm_conf.h > > diff --git a/src/conf/Makefile.inc.am b/src/conf/Makefile.inc.am > index 08c7c9da7f..e42425fcc5 100644 > --- a/src/conf/Makefile.inc.am > +++ b/src/conf/Makefile.inc.am > @@ -153,6 +153,11 @@ DEVICE_CONF_SOURCES = \ > conf/device_conf.h \ > $(NULL) > > +TPM_CONF_SOURCES = \ > + conf/virtpm_conf.c \ > + conf/virtpm_conf.h \ > + $(NULL) > + > CONF_SOURCES = \ > $(NETDEV_CONF_SOURCES) \ > $(DOMAIN_CONF_SOURCES) \ > @@ -171,6 +176,7 @@ CONF_SOURCES = \ > $(CPU_CONF_SOURCES) \ > $(CHRDEV_CONF_SOURCES) \ > $(DEVICE_CONF_SOURCES) \ > + $(TPM_CONF_SOURCES) \ > $(NULL) > > noinst_LTLIBRARIES += libvirt_conf.la > diff --git a/src/conf/virtpm_conf.c b/src/conf/virtpm_conf.c > new file mode 100644 > index 0000000000..12e69e67b3 > --- /dev/null > +++ b/src/conf/virtpm_conf.c > @@ -0,0 +1,36 @@ > +/* > + * virtpm_conf.c: vTPM XML processing > + * > + * Copyright (C) 2019 IBM Corporation > + * > + * This library is free software; you can redistribute it and/or > + * modify it under the terms of the GNU Lesser General Public > + * License as published by the Free Software Foundation; either > + * version 2.1 of the License, or (at your option) any later version. > + * > + * This library is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + * Lesser General Public License for more details. > + * > + * You should have received a copy of the GNU Lesser General Public > + * License along with this library. If not, see > + * <http://www.gnu.org/licenses/>;. > + */ > + > +#include <config.h> > + > +#include "virenum.h" > +#include "virtpm_conf.h" > + > +#define VIR_FROM_THIS VIR_FROM_NONE > + > +VIR_ENUM_IMPL(virTPMSwtpmFeature, > + VIR_TPM_SWTPM_FEATURE_LAST, > + "cmdarg-pwd-fd", > +); > + > +VIR_ENUM_IMPL(virTPMSwtpmSetupFeature, > + VIR_TPM_SWTPM_SETUP_FEATURE_LAST, > + "cmdarg-pwdfile-fd", > +); > diff --git a/src/conf/virtpm_conf.h b/src/conf/virtpm_conf.h > new file mode 100644 > index 0000000000..73c6c67271 > --- /dev/null > +++ b/src/conf/virtpm_conf.h > @@ -0,0 +1,36 @@ > +/* > + * virtpm_conf.h: vTPM XML processing > + * > + * Copyright (C) 2019 IBM Corporation > + * > + * This library is free software; you can redistribute it and/or > + * modify it under the terms of the GNU Lesser General Public > + * License as published by the Free Software Foundation; either > + * version 2.1 of the License, or (at your option) any later version. > + * > + * This library is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + * Lesser General Public License for more details. > + * > + * You should have received a copy of the GNU Lesser General Public > + * License along with this library. If not, see > + * <http://www.gnu.org/licenses/>;. > + */ > + > +#pragma once > + > +typedef enum { > + VIR_TPM_SWTPM_FEATURE_CMDARG_PWD_FD, > + > + VIR_TPM_SWTPM_FEATURE_LAST > +} virTPMSwtpmFeature; > + > +typedef enum { > + VIR_TPM_SWTPM_SETUP_FEATURE_CMDARG_PWDFILE_FD, > + > + VIR_TPM_SWTPM_SETUP_FEATURE_LAST > +} virTPMSwtpmSetupFeature; > + > +VIR_ENUM_DECL(virTPMSwtpmFeature); > +VIR_ENUM_DECL(virTPMSwtpmSetupFeature); > diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms > index e33d7d9f14..d2045895a1 100644 > --- a/src/libvirt_private.syms > +++ b/src/libvirt_private.syms > @@ -1236,6 +1236,11 @@ virStoragePoolObjVolumeGetNames; > virStoragePoolObjVolumeListExport; > > > +# conf/virtpm_conf.h > +virTPMSwtpmFeatureTypeFromString; > +virTPMSwtpmSetupFeatureTypeFromString; > + > + > # cpu/cpu.h > cpuDecode; > cpuEncode; > diff --git a/src/tpm/Makefile.inc.am b/src/tpm/Makefile.inc.am > index 1f5131bf34..d8a15c406c 100644 > --- a/src/tpm/Makefile.inc.am > +++ b/src/tpm/Makefile.inc.am > @@ -12,6 +12,9 @@ EXTRA_DIST += \ > > noinst_LTLIBRARIES += libvirt_tpm.la > libvirt_la_BUILT_LIBADD += libvirt_tpm.la > -libvirt_tpm_la_CFLAGS = $(AM_CFLAGS) > +libvirt_tpm_la_CFLAGS = \ > + -I$(srcdir)/conf \ > + $(AM_CFLAGS) \ > + $(NULL) > libvirt_tpm_la_LDFLAGS = $(AM_LDFLAGS) > libvirt_tpm_la_SOURCES = $(TPM_UTIL_SOURCES) > diff --git a/src/tpm/virtpm.c b/src/tpm/virtpm.c > index e4735f9c4d..42dd2b1bb2 100644 > --- a/src/tpm/virtpm.c > +++ b/src/tpm/virtpm.c > @@ -27,6 +27,10 @@ > #include "viralloc.h" > #include "virfile.h" > #include "virtpm.h" > +#include "vircommand.h" > +#include "virbitmap.h" > +#include "virjson.h" > +#include "virtpm_conf.h" > > #define VIR_FROM_THIS VIR_FROM_NONE > > @@ -74,17 +78,22 @@ virTPMCreateCancelPath(const char *devpath) > } > > /* > - * executables for the swtpm; to be found on the host > + * executables for the swtpm; to be found on the host along with > + * capabilties bitmap > */ > static char *swtpm_path; > static struct stat swtpm_stat; > +static virBitmapPtr swtpm_caps; > > static char *swtpm_setup; > static struct stat swtpm_setup_stat; > +static virBitmapPtr swtpm_setup_caps; > > static char *swtpm_ioctl; > static struct stat swtpm_ioctl_stat; > > +typedef int (*TypeFromStringFn)(const char *); > + > const char * > virTPMGetSwtpm(void) > { > @@ -109,6 +118,106 @@ virTPMGetSwtpmIoctl(void) > return swtpm_ioctl; > } > > +/* virTPMExecGetCaps > + * > + * Execute the prepared command and parse the returned JSON object > + * to get the capabilities supported by the executable. > + * A JSON object like this is expected: > + * > + * { > + * "type": "swtpm", > + * "features": [ > + * "cmdarg-seccomp", > + * "cmdarg-key-fd", > + * "cmdarg-pwd-fd" > + * ] > + * } > + */ > +static virBitmapPtr > +virTPMExecGetCaps(virCommandPtr cmd, > + TypeFromStringFn typeFromStringFn) > +{ > + int exitstatus; > + virBitmapPtr bitmap; > + char *outbuf = NULL; > + virJSONValuePtr json = NULL; > + virJSONValuePtr featureList; > + virJSONValuePtr item; > + size_t idx; > + const char *str; > + int typ; > + > + if (!(bitmap = virBitmapNewEmpty())) > + return NULL; > + > + virCommandSetOutputBuffer(cmd, &outbuf); > + /* We allow the command to fail since older versions of it may > + * not support --print-capabilities > + */ > + if (virCommandRun(cmd, &exitstatus) < 0 || exitstatus != 0) > + goto cleanup; We should probably fail if the command failed to run, and skip if exitstatus != 0. > + > + json = virJSONValueFromString(outbuf); > + if (!json) > + goto error_bad_json; > + > + featureList = virJSONValueObjectGetArray(json, "features"); > + if (!featureList) > + goto error_bad_json; > + > + if (!virJSONValueIsArray(featureList)) > + goto error_bad_json; > + > + for (idx = 0; idx < virJSONValueArraySize(featureList); idx++) { > + item = virJSONValueArrayGet(featureList, idx); > + if (!item) > + continue; > + > + str = virJSONValueGetString(item); > + if (!str) > + goto error_bad_json; > + typ = typeFromStringFn(str); > + if (typ < 0) > + continue; > + > + if (virBitmapSetBitExpand(bitmap, typ) < 0) > + goto cleanup; > + } > + > + cleanup: > + VIR_FREE(outbuf); > + virJSONValueFree(json); Please use VIR_AUTOFREE and VIR_AUTOPTR

> + > + return bitmap; > + > + error_bad_json: > + virReportError(VIR_ERR_INTERNAL_ERROR, > + _("Unexpected JSON format: %s"), outbuf); > + goto cleanup; > +} > + > +static virBitmapPtr > +virTPMGetCaps(TypeFromStringFn typeFromStringFn, > + const char *exec, const char *param1) > +{ > + virCommandPtr cmd; > + virBitmapPtr bitmap; > + > + cmd = virCommandNew(exec); > + if (!cmd) > + return NULL; > + if (param1) > + virCommandAddArg(cmd, param1); Hmm, I would expect --print-capabilites to be top-level argument, not a sub-command argument.