[libvirt] [PATCH] qemu: Label master key file
Martin Kletzander
mkletzan at redhat.com
Mon Apr 18 06:34:13 UTC 2016
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.
>Thanks,
>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/458d2e03/attachment-0001.sig>
More information about the libvir-list
mailing list