Re: [PATCH] qemu: Label master key file

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.

- Cole

Attachment: signature.asc
Description: Digital signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]