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 belowdiff --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.aesActually, 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 anyonev2 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