[PATCH V3 4/4] tools: Add domsetlaunchsecstate virsh command

[Jim Fehlig]

On 12/16/21 08:02, Daniel P. Berrangé wrote:
> On Tue, Dec 14, 2021 at 09:46:06PM -0700, Jim Fehlig wrote:
>> After attesting a domain with the help of domlaunchsecinfo,
>> domsetlaunchsecstate can be used to set a secret in the guest
>> domain's memory prior to running the vcpus.
>>
>> Signed-off-by: Jim Fehlig <jfehlig at suse.com>
>> ---
>>
>> Some questions and RFC regarding this patch:
>>
>> I'm not really fond of the command and function names and would appreciate
>> suggestions :-).
> 
> I'm honestly not too fussed about the naming. THis command is really
> just about feature complete API coverage. I doubt many people will
> actually use virsh for this, instead they'll want a program that
> queries the measurement, verifies it and injects secret all in one
> go.
> 
>> Also, is reading the secret header and secret from a file sufficient? The
>> sev-tool 'package_secret' command writes the secret to a file.
> 
> Fine IMHO.
> 
>> Lastly, I'm not sure what sizes to expect for secret and secret header. I
>> may have overlooked it, but didn't find anything related to the size in the
>> docs. I've temporarily set it to VSH_MAX_XML_FILE until we know a reasonable
>> value.

I took another look at the API spec and section "6.6 LAUNCH_SECRET", subsection 
"6.6.2 Parameters" describes the parameters to the LUANCH_SECRET command and the 
layout the the secret header. IIUC, GUEST_PADDR and GUEST_LENGTH tell where to 
put the secret. GUEST_LENGTH cannot be greater than 16KB. And the secret header 
looks to be 104 bytes.

https://www.amd.com/system/files/TechDocs/55766_SEV-KM_API_Specification.pdf

> I'm not too sure either, but as a general point, IMHO, almost all
> our use of virFileReadAll has no functional or security need for
> us to supply a limit at all.
> 
> We really ought to just make it accept '-1' as a limit and treat
> that as unlimited.  Meanwhile I'd suggest just letting it be
> 1 MB which is way bigger than i expect this data will be.

Given the above, 1MB is overkill. But still go with it as a safer, more 
future-proof value?

Regards,
Jim

> 
> You can only DoS yourself with virsh, and you'll be limited by
> the RPC protocol wire limit.
> 
> 
> 
>>
>>   docs/manpages/virsh.rst |  25 ++++++++++
>>   tools/virsh-domain.c    | 107 ++++++++++++++++++++++++++++++++++++++++
>>   2 files changed, 132 insertions(+)
> 
> With a 1 MB or similar size limit
> 
> Reviewed-by: Daniel P. Berrangé <berrange at redhat.com>
> 
> 
> Regards,
> Daniel
>