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@xxxxxxxxxx>
>>>> ---
>>>> 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
Thanks,
Cole
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list