Add new 'launch-security' command, the command can be used to get or set
the launch security information when booting encrypted VMs.
Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
---
tools/virsh-domain.c | 84 ++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 84 insertions(+)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 2b775fc..4dca191 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -13877,6 +13877,84 @@ cmdDomFSInfo(vshControl *ctl, const vshCmd *cmd)
return ret >= 0;
}
+/*
+ * "launch-security" command
+ */
+static const vshCmdInfo info_launch_security[] = {
+ {.name = "help",
+ .data = N_("Get or set launch-security information")
+ },
+ {.name = "desc",
+ .data = N_("Get or set the current launch-security information for a guest"
+ " domain.\n"
+ " To get the launch-security information use following command: \n\n"
+ " virsh # launch-security <domain>")
+ },
+ {.name = NULL}
+};
+
+static const vshCmdOptDef opts_launch_security[] = {
+ VIRSH_COMMON_OPT_DOMAIN_FULL(0),
+ {.name = "get",
+ .type = VSH_OT_STRING,
+ .help = N_("Show the launch-security info")
+ },
+ VIRSH_COMMON_OPT_DOMAIN_CONFIG,
+ VIRSH_COMMON_OPT_DOMAIN_LIVE,
+ VIRSH_COMMON_OPT_DOMAIN_CURRENT,
+ {.name = NULL}
+};
+
+static void
+virshPrintLaunchSecurityInfo(vshControl *ctl, virTypedParameterPtr params,
+ int nparams)
+{
+ size_t i;
+
+ for (i = 0; i < nparams; i++) {
+ if (params[i].type == VIR_TYPED_PARAM_STRING)
+ vshPrintExtra(ctl, "%-15s: %s\n", params[i].field, params[i].value.s);
+ }
+}
+
+static bool
+cmdLaunchSecurity(vshControl *ctl, const vshCmd *cmd)
+{
+ virDomainPtr dom;
+ int nparams = 0;
+ virTypedParameterPtr params = NULL;
+ bool ret = false;
+ unsigned int flags = VIR_DOMAIN_AFFECT_CURRENT;
+ bool current = vshCommandOptBool(cmd, "current");
+ bool config = vshCommandOptBool(cmd, "config");
+ bool live = vshCommandOptBool(cmd, "live");
+
+ VSH_EXCLUSIVE_OPTIONS_VAR(current, live);
+ VSH_EXCLUSIVE_OPTIONS_VAR(current, config);
+
+ if (config)
+ flags |= VIR_DOMAIN_AFFECT_CONFIG;
+ if (live)
+ flags |= VIR_DOMAIN_AFFECT_LIVE;
+
+ if (!(dom = virshCommandOptDomain(ctl, cmd, NULL)))
+ return false;
+
+ if (virDomainGetLaunchSecurityInfo(dom, ¶ms, &nparams, flags) != 0) {
+ vshError(ctl, "%s", _("Unable to get launch security info"));
+ goto cleanup;
+ }
+
+ virshPrintLaunchSecurityInfo(ctl, params, nparams);
+
+ ret = true;
+ cleanup:
+ virTypedParamsFree(params, nparams);
+ virshDomainFree(dom);
+ return ret;
+}
+
+
const vshCmdDef domManagementCmds[] = {
{.name = "attach-device",
.handler = cmdAttachDevice,
@@ -14492,5 +14570,11 @@ const vshCmdDef domManagementCmds[] = {
.info = info_domblkthreshold,
.flags = 0
},
+ {.name = "launch-security",
+ .handler = cmdLaunchSecurity,
+ .opts = opts_launch_security,
+ .info = info_launch_security,
+ .flags = 0
+ },
{.name = NULL}
};
--
2.7.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 04/02/2018 10:18 AM, Brijesh Singh wrote:
> Add new 'launch-security' command, the command can be used to get or set
> the launch security information when booting encrypted VMs.
>
> Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
> ---
> tools/virsh-domain.c | 84 ++++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 84 insertions(+)
>
Need to modify tools/virsh.pod too in order to supply the man page
information.
> diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
> index 2b775fc..4dca191 100644
> --- a/tools/virsh-domain.c
> +++ b/tools/virsh-domain.c
> @@ -13877,6 +13877,84 @@ cmdDomFSInfo(vshControl *ctl, const vshCmd *cmd)
> return ret >= 0;
> }
>
> +/*
> + * "launch-security" command
> + */
> +static const vshCmdInfo info_launch_security[] = {
> + {.name = "help",
> + .data = N_("Get or set launch-security information")
> + },
> + {.name = "desc",
> + .data = N_("Get or set the current launch-security information for a guest"
> + " domain.\n"
> + " To get the launch-security information use following command: \n\n"
> + " virsh # launch-security <domain>")
> + },
Rather lengthy... don't think the last 2 lines are necessary. Is there
another command doing the same?
Probably should fill in the ".help" entry too like other commands.
> + {.name = NULL}
> +};
> +
> +static const vshCmdOptDef opts_launch_security[] = {
> + VIRSH_COMMON_OPT_DOMAIN_FULL(0),
> + {.name = "get",
> + .type = VSH_OT_STRING,
> + .help = N_("Show the launch-security info")
> + },
> + VIRSH_COMMON_OPT_DOMAIN_CONFIG,
> + VIRSH_COMMON_OPT_DOMAIN_LIVE,
> + VIRSH_COMMON_OPT_DOMAIN_CURRENT,
> + {.name = NULL}
> +};
> +
2 lines...
> +static void
> +virshPrintLaunchSecurityInfo(vshControl *ctl, virTypedParameterPtr params,
> + int nparams)
> +{
> + size_t i;
> +
Should there perhaps be a header here for the columns?
> + for (i = 0; i < nparams; i++) {
> + if (params[i].type == VIR_TYPED_PARAM_STRING)
> + vshPrintExtra(ctl, "%-15s: %s\n", params[i].field, params[i].value.s);
> + }
> +}
> +
2 lines...
John
> +static bool
> +cmdLaunchSecurity(vshControl *ctl, const vshCmd *cmd)
> +{
> + virDomainPtr dom;
> + int nparams = 0;
> + virTypedParameterPtr params = NULL;
> + bool ret = false;
> + unsigned int flags = VIR_DOMAIN_AFFECT_CURRENT;
> + bool current = vshCommandOptBool(cmd, "current");
> + bool config = vshCommandOptBool(cmd, "config");
> + bool live = vshCommandOptBool(cmd, "live");
> +
> + VSH_EXCLUSIVE_OPTIONS_VAR(current, live);
> + VSH_EXCLUSIVE_OPTIONS_VAR(current, config);
> +
> + if (config)
> + flags |= VIR_DOMAIN_AFFECT_CONFIG;
> + if (live)
> + flags |= VIR_DOMAIN_AFFECT_LIVE;
> +
> + if (!(dom = virshCommandOptDomain(ctl, cmd, NULL)))
> + return false;
> +
> + if (virDomainGetLaunchSecurityInfo(dom, ¶ms, &nparams, flags) != 0) {
> + vshError(ctl, "%s", _("Unable to get launch security info"));
> + goto cleanup;
> + }
> +
> + virshPrintLaunchSecurityInfo(ctl, params, nparams);
> +
> + ret = true;
> + cleanup:
> + virTypedParamsFree(params, nparams);
> + virshDomainFree(dom);
> + return ret;
> +}
> +
> +
> const vshCmdDef domManagementCmds[] = {
> {.name = "attach-device",
> .handler = cmdAttachDevice,
> @@ -14492,5 +14570,11 @@ const vshCmdDef domManagementCmds[] = {
> .info = info_domblkthreshold,
> .flags = 0
> },
> + {.name = "launch-security",
> + .handler = cmdLaunchSecurity,
> + .opts = opts_launch_security,
> + .info = info_launch_security,
> + .flags = 0
> + },
> {.name = NULL}
> };
>
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 4/2/18 6:31 PM, John Ferlan wrote:
>
> On 04/02/2018 10:18 AM, Brijesh Singh wrote:
>> Add new 'launch-security' command, the command can be used to get or set
>> the launch security information when booting encrypted VMs.
>>
>> Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
>> ---
>> tools/virsh-domain.c | 84 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 84 insertions(+)
>>
> Need to modify tools/virsh.pod too in order to supply the man page
> information.
Ah, I missed that. Will add help in next rev.
>
>> diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
>> index 2b775fc..4dca191 100644
>> --- a/tools/virsh-domain.c
>> +++ b/tools/virsh-domain.c
>> @@ -13877,6 +13877,84 @@ cmdDomFSInfo(vshControl *ctl, const vshCmd *cmd)
>> return ret >= 0;
>> }
>>
>> +/*
>> + * "launch-security" command
>> + */
>> +static const vshCmdInfo info_launch_security[] = {
>> + {.name = "help",
>> + .data = N_("Get or set launch-security information")
>> + },
>> + {.name = "desc",
>> + .data = N_("Get or set the current launch-security information for a guest"
>> + " domain.\n"
>> + " To get the launch-security information use following command: \n\n"
>> + " virsh # launch-security <domain>")
>> + },
> Rather lengthy... don't think the last 2 lines are necessary. Is there
> another command doing the same?
Yes, I tried to follow other commands.
> Probably should fill in the ".help" entry too like other commands.
OK.
>> + {.name = NULL}
>> +};
>> +
>> +static const vshCmdOptDef opts_launch_security[] = {
>> + VIRSH_COMMON_OPT_DOMAIN_FULL(0),
>> + {.name = "get",
>> + .type = VSH_OT_STRING,
>> + .help = N_("Show the launch-security info")
>> + },
>> + VIRSH_COMMON_OPT_DOMAIN_CONFIG,
>> + VIRSH_COMMON_OPT_DOMAIN_LIVE,
>> + VIRSH_COMMON_OPT_DOMAIN_CURRENT,
>> + {.name = NULL}
>> +};
>> +
> 2 lines...
>
>> +static void
>> +virshPrintLaunchSecurityInfo(vshControl *ctl, virTypedParameterPtr params,
>> + int nparams)
>> +{
>> + size_t i;
>> +
> Should there perhaps be a header here for the columns?
For now we have only one column and I am not too sure if we need to add
a header file for it, if it grows then we can revisit.
>
>> + for (i = 0; i < nparams; i++) {
>> + if (params[i].type == VIR_TYPED_PARAM_STRING)
>> + vshPrintExtra(ctl, "%-15s: %s\n", params[i].field, params[i].value.s);
>> + }
>> +}
>> +
> 2 lines...
>
> John
>
>> +static bool
>> +cmdLaunchSecurity(vshControl *ctl, const vshCmd *cmd)
>> +{
>> + virDomainPtr dom;
>> + int nparams = 0;
>> + virTypedParameterPtr params = NULL;
>> + bool ret = false;
>> + unsigned int flags = VIR_DOMAIN_AFFECT_CURRENT;
>> + bool current = vshCommandOptBool(cmd, "current");
>> + bool config = vshCommandOptBool(cmd, "config");
>> + bool live = vshCommandOptBool(cmd, "live");
>> +
>> + VSH_EXCLUSIVE_OPTIONS_VAR(current, live);
>> + VSH_EXCLUSIVE_OPTIONS_VAR(current, config);
>> +
>> + if (config)
>> + flags |= VIR_DOMAIN_AFFECT_CONFIG;
>> + if (live)
>> + flags |= VIR_DOMAIN_AFFECT_LIVE;
>> +
>> + if (!(dom = virshCommandOptDomain(ctl, cmd, NULL)))
>> + return false;
>> +
>> + if (virDomainGetLaunchSecurityInfo(dom, ¶ms, &nparams, flags) != 0) {
>> + vshError(ctl, "%s", _("Unable to get launch security info"));
>> + goto cleanup;
>> + }
>> +
>> + virshPrintLaunchSecurityInfo(ctl, params, nparams);
>> +
>> + ret = true;
>> + cleanup:
>> + virTypedParamsFree(params, nparams);
>> + virshDomainFree(dom);
>> + return ret;
>> +}
>> +
>> +
>> const vshCmdDef domManagementCmds[] = {
>> {.name = "attach-device",
>> .handler = cmdAttachDevice,
>> @@ -14492,5 +14570,11 @@ const vshCmdDef domManagementCmds[] = {
>> .info = info_domblkthreshold,
>> .flags = 0
>> },
>> + {.name = "launch-security",
>> + .handler = cmdLaunchSecurity,
>> + .opts = opts_launch_security,
>> + .info = info_launch_security,
>> + .flags = 0
>> + },
>> {.name = NULL}
>> };
>>
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Mon, Apr 02, 2018 at 09:18:55AM -0500, Brijesh Singh wrote:
> Add new 'launch-security' command, the command can be used to get or set
> the launch security information when booting encrypted VMs.
>
> Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
> ---
> tools/virsh-domain.c | 84 ++++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 84 insertions(+)
>
> diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
> index 2b775fc..4dca191 100644
> --- a/tools/virsh-domain.c
> +++ b/tools/virsh-domain.c
> @@ -13877,6 +13877,84 @@ cmdDomFSInfo(vshControl *ctl, const vshCmd *cmd)
> return ret >= 0;
> }
>
> +/*
> + * "launch-security" command
> + */
> +static const vshCmdInfo info_launch_security[] = {
> + {.name = "help",
> + .data = N_("Get or set launch-security information")
> + },
> + {.name = "desc",
> + .data = N_("Get or set the current launch-security information for a guest"
> + " domain.\n"
> + " To get the launch-security information use following command: \n\n"
> + " virsh # launch-security <domain>")
As John has pointed out, you might want to shorten ^these 2 lines, however, I
think it makes sense to make it obvious that running without any
arguments/options this behaves like a getter, otherwise it's going to behave
like a setter, right? (it's a common practice in libvirt, so nothing against
conceptually).
> + },
> + {.name = NULL}
> +};
> +
> +static const vshCmdOptDef opts_launch_security[] = {
> + VIRSH_COMMON_OPT_DOMAIN_FULL(0),
> + {.name = "get",
> + .type = VSH_OT_STRING,
> + .help = N_("Show the launch-security info")
> + },
> + VIRSH_COMMON_OPT_DOMAIN_CONFIG,
> + VIRSH_COMMON_OPT_DOMAIN_LIVE,
> + VIRSH_COMMON_OPT_DOMAIN_CURRENT,
> + {.name = NULL}
> +};
Sorry if I missed the obvious, but what exactly is the --get <string> supposed
to do?
Erik
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
> > + },
> > + {.name = NULL}
> > +};
> > +
> > +static const vshCmdOptDef opts_launch_security[] = {
> > + VIRSH_COMMON_OPT_DOMAIN_FULL(0),
> > + {.name = "get",
> > + .type = VSH_OT_STRING,
> > + .help = N_("Show the launch-security info")
> > + },
> > + VIRSH_COMMON_OPT_DOMAIN_CONFIG,
> > + VIRSH_COMMON_OPT_DOMAIN_LIVE,
> > + VIRSH_COMMON_OPT_DOMAIN_CURRENT,
> > + {.name = NULL}
> > +};
>
> Sorry if I missed the obvious, but what exactly is the --get <string> supposed
> to do?
Giving it another thought, why do we need the virsh part anyway, this is only
relevant for the config, aka shut off domain, so virsh edit and virsh dumpxml
will do the job of a getter and a setter just fine, the main purpose of the
virsh commands is to tune the 'live' configuration of a domain.
Erik
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Tue, Apr 03, 2018 at 04:55:42PM +0200, Erik Skultety wrote:
> > > + },
> > > + {.name = NULL}
> > > +};
> > > +
> > > +static const vshCmdOptDef opts_launch_security[] = {
> > > + VIRSH_COMMON_OPT_DOMAIN_FULL(0),
> > > + {.name = "get",
> > > + .type = VSH_OT_STRING,
> > > + .help = N_("Show the launch-security info")
> > > + },
> > > + VIRSH_COMMON_OPT_DOMAIN_CONFIG,
> > > + VIRSH_COMMON_OPT_DOMAIN_LIVE,
> > > + VIRSH_COMMON_OPT_DOMAIN_CURRENT,
> > > + {.name = NULL}
> > > +};
> >
> > Sorry if I missed the obvious, but what exactly is the --get <string> supposed
> > to do?
>
> Giving it another thought, why do we need the virsh part anyway, this is only
> relevant for the config, aka shut off domain, so virsh edit and virsh dumpxml
> will do the job of a getter and a setter just fine, the main purpose of the
> virsh commands is to tune the 'live' configuration of a domain.
Please disregard this, I got confused by the name and created an association
with the launch-security data formatted into the domain XML which we retrieved
from the hypervisor, where in fact, this command is necessary to obtain the SEV
measurement.
Erik
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 4/3/18 9:32 AM, Erik Skultety wrote:
> On Mon, Apr 02, 2018 at 09:18:55AM -0500, Brijesh Singh wrote:
>> Add new 'launch-security' command, the command can be used to get or set
>> the launch security information when booting encrypted VMs.
>>
>> Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
>> ---
>> tools/virsh-domain.c | 84 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 84 insertions(+)
>>
>> diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
>> index 2b775fc..4dca191 100644
>> --- a/tools/virsh-domain.c
>> +++ b/tools/virsh-domain.c
>> @@ -13877,6 +13877,84 @@ cmdDomFSInfo(vshControl *ctl, const vshCmd *cmd)
>> return ret >= 0;
>> }
>>
>> +/*
>> + * "launch-security" command
>> + */
>> +static const vshCmdInfo info_launch_security[] = {
>> + {.name = "help",
>> + .data = N_("Get or set launch-security information")
>> + },
>> + {.name = "desc",
>> + .data = N_("Get or set the current launch-security information for a guest"
>> + " domain.\n"
>> + " To get the launch-security information use following command: \n\n"
>> + " virsh # launch-security <domain>")
> As John has pointed out, you might want to shorten ^these 2 lines, however, I
> think it makes sense to make it obvious that running without any
> arguments/options this behaves like a getter, otherwise it's going to behave
> like a setter, right? (it's a common practice in libvirt, so nothing against
> conceptually).
>
Yes, without any command line it should be getter otherwise settter.
Currently, we don't have anything in setter yet.
>> + },
>> + {.name = NULL}
>> +};
>> +
>> +static const vshCmdOptDef opts_launch_security[] = {
>> + VIRSH_COMMON_OPT_DOMAIN_FULL(0),
>> + {.name = "get",
>> + .type = VSH_OT_STRING,
>> + .help = N_("Show the launch-security info")
>> + },
>> + VIRSH_COMMON_OPT_DOMAIN_CONFIG,
>> + VIRSH_COMMON_OPT_DOMAIN_LIVE,
>> + VIRSH_COMMON_OPT_DOMAIN_CURRENT,
>> + {.name = NULL}
>> +};
> Sorry if I missed the obvious, but what exactly is the --get <string> supposed
> to do?
The command will return a measurement of encrypted image. The
measurement value can be used by guest owner to validate the image
before it launches the VM. In a typical scenario we may have something
like this:
# virsh create guest.xml --paused
# virsh launch-security --domain guest
// validate the measurement obtained through above command
if measurement is wrong then;
destory the guest
else
// optionally inject secret in guest using virsh launch-security set <....>
resume the guest
> Erik
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Wed, Apr 04, 2018 at 07:36:37AM -0500, Brijesh Singh wrote:
>
>
> On 4/3/18 9:32 AM, Erik Skultety wrote:
> > On Mon, Apr 02, 2018 at 09:18:55AM -0500, Brijesh Singh wrote:
> >> Add new 'launch-security' command, the command can be used to get or set
> >> the launch security information when booting encrypted VMs.
> >>
> >> Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
> >> ---
> >> tools/virsh-domain.c | 84 ++++++++++++++++++++++++++++++++++++++++++++++++++++
> >> 1 file changed, 84 insertions(+)
> >>
> >> diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
> >> index 2b775fc..4dca191 100644
> >> --- a/tools/virsh-domain.c
> >> +++ b/tools/virsh-domain.c
> >> @@ -13877,6 +13877,84 @@ cmdDomFSInfo(vshControl *ctl, const vshCmd *cmd)
> >> return ret >= 0;
> >> }
> >>
> >> +/*
> >> + * "launch-security" command
> >> + */
> >> +static const vshCmdInfo info_launch_security[] = {
> >> + {.name = "help",
> >> + .data = N_("Get or set launch-security information")
> >> + },
> >> + {.name = "desc",
> >> + .data = N_("Get or set the current launch-security information for a guest"
> >> + " domain.\n"
> >> + " To get the launch-security information use following command: \n\n"
> >> + " virsh # launch-security <domain>")
> > As John has pointed out, you might want to shorten ^these 2 lines, however, I
> > think it makes sense to make it obvious that running without any
> > arguments/options this behaves like a getter, otherwise it's going to behave
> > like a setter, right? (it's a common practice in libvirt, so nothing against
> > conceptually).
> >
>
> Yes, without any command line it should be getter otherwise settter.
> Currently, we don't have anything in setter yet.
>
> >> + },
> >> + {.name = NULL}
> >> +};
> >> +
> >> +static const vshCmdOptDef opts_launch_security[] = {
> >> + VIRSH_COMMON_OPT_DOMAIN_FULL(0),
> >> + {.name = "get",
> >> + .type = VSH_OT_STRING,
> >> + .help = N_("Show the launch-security info")
> >> + },
> >> + VIRSH_COMMON_OPT_DOMAIN_CONFIG,
> >> + VIRSH_COMMON_OPT_DOMAIN_LIVE,
> >> + VIRSH_COMMON_OPT_DOMAIN_CURRENT,
> >> + {.name = NULL}
> >> +};
> > Sorry if I missed the obvious, but what exactly is the --get <string> supposed
> > to do?
>
> The command will return a measurement of encrypted image. The
> measurement value can be used by guest owner to validate the image
> before it launches the VM. In a typical scenario we may have something
> like this:
>
> # virsh create guest.xml --paused
> # virsh launch-security --domain guest
> // validate the measurement obtained through above command
> if measurement is wrong then;
> destory the guest
> else
> // optionally inject secret in guest using virsh launch-security set <....>
> resume the guest
Yeah, but since you agreed that the command behaves like a getter without any
parameters, I don't see a reason for the --get <string> parameter, that's why I
asked about it. Also, the name of the command should be changed since this way,
it's a bit deceptive as it hints that it queries/modifies the launch-security
data which you got from querying qemu capabilities within the domain XML.
I'm also going to respond to the other email to this patch I sent yesterday,
because it's irrelevant given the fact that I got confused by the name (even
though I saw what API it called) and created a false association.
Other than what's already been noted, the logic in the virsh command itself
looks solid, not much to it since the setter part will come later I presume.
Erik
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2025 Red Hat, Inc.