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

[Martin Kletzander]

On Mon, Apr 18, 2016 at 08:55:02AM -0400, Cole Robinson wrote:
>On 04/18/2016 02:34 AM, Martin Kletzander wrote:
>> On Fri, Apr 15, 2016 at 12:16:18PM -0400, Cole Robinson wrote:
>>> On 04/14/2016 03:04 AM, Martin Kletzander wrote:
>>>> 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.
>>>
>>> Laine was hitting issues with this too, so I pushed this patch. The API rename
>>> isn't blocking anyone
>>>
>>
>> v2 was alrady on the list, but it can be done the other way around.
>> I'll send the rename rebased now so it's easier to review.
>>
>
>Oh sorry, I must have missed it? Though looking through the archives I don't
>see anything, only the original patch and the v3. Maybe it didn't hit the list...
>

Then I must've missed it somehow.  Anyway, at least it's done now :)

>- 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/20160418/94d9d9f5/attachment-0001.sig>