Re: [PATCH] qemu: Label master key file

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

 



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@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


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

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]