[libvirt] [PATCH] qemu: Label master key file

[Martin Kletzander]

On Wed, Apr 13, 2016 at 07:58:06PM -0400, Cole Robinson wrote:
>On 04/13/2016 11:56 AM, Cole Robinson wrote:
>> On 04/13/2016 11:17 AM, Martin Kletzander wrote:
>>> When creating the master key, we used mode 0600 (which we should) but
>>> because we were creating it as root, the file is not readable by any
>>> qemu running as non-root.  Fortunately, it's just a matter of labelling
>>> the file.  We are generating the file path few times already, so let's
>>> label it in the same function that has access to the path already.
>>>
>>> Signed-off-by: Martin Kletzander <mkletzan at redhat.com>
>>> ---
>>>  src/qemu/qemu_domain.c  | 15 ++++++++++++---
>>>  src/qemu/qemu_domain.h  |  3 ++-
>>>  src/qemu/qemu_process.c |  2 +-
>>>  3 files changed, 15 insertions(+), 5 deletions(-)
>>>
>>
>> ACK, makes sense and fixes things for me. One comment below
>>
>>> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
>>> index 5d54fffcfb98..83e765ef6868 100644
>>> --- a/src/qemu/qemu_domain.c
>>> +++ b/src/qemu/qemu_domain.c
>>> @@ -504,11 +504,13 @@ qemuDomainGetMasterKeyFilePath(const char *libDir)
>>>   * Returns 0 on success, -1 on failure with error message indicating failure
>>>   */
>>>  static int
>>> -qemuDomainWriteMasterKeyFile(qemuDomainObjPrivatePtr priv)
>>> +qemuDomainWriteMasterKeyFile(virQEMUDriverPtr driver,
>>> +                             virDomainObjPtr vm)
>>>  {
>>>      char *path;
>>>      int fd = -1;
>>>      int ret = -1;
>>> +    qemuDomainObjPrivatePtr priv = vm->privateData;
>>>
>>>      if (!(path = qemuDomainGetMasterKeyFilePath(priv->libDir)))
>>>          return -1;
>>> @@ -525,6 +527,10 @@ qemuDomainWriteMasterKeyFile(qemuDomainObjPrivatePtr priv)
>>>          goto cleanup;
>>>      }
>>>
>>> +    if (virSecurityManagerDomainSetDirLabel(driver->securityManager,
>>> +                                            vm->def, path) < 0)
>>> +        goto cleanup;
>>> +
>>>      ret = 0;
>>>
>>
>> I looked briefly at fixing this but know if there was a function to ask the
>> security driver 'just set a on this arbitrary path'. I saw DirLabel but was
>> thrown off by the 'Dir' name. Maybe change it to something more generic?
>>

Yes, it's just a naming; I had to add it for similar purpose when
labelling directories, but it "Just Works" for arbitrary path.  I'll
rename that.

>
>Also adding some CC, I'm guessing virt-aa-helper.c needs to be extended to to
>allow access to $libDir/master-key.aes
>

Actually, it shouldn't.  It's in the per-domain directory and
everything under that should be available.

Anyway, it's a pity that we're not very likely to have a test case for
that.  But couldn't the paths in virt-aa-helper be created from a
security driver?  It knows all the paths, doesn't it?

I'll send a v2 with the rename.

>- Cole
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20160414/ba3d9a3d/attachment-0001.sig>