Re: Socket files in virt-aa-helper

[Michał Dubiel <md@xxxxxxxxxxxx>]

Hi Serge,

This is exactly what I had done locally when I solved the permission issues for my project, however removal of S_IFSOCK restriction is not enough to make vhost-user work correctly. You need also the changes to get_files() I wrote in the first e-mail about.

I'll send a patch for review (in separate thread) that adds these changes.

Regards,

Michal

On 18 June 2015 at 17:11, Serge Hallyn <serge.hallyn@xxxxxxxxxx> wrote:

Quoting Jamie Strandboge (jamie@xxxxxxxxxxxxx):
> On 06/16/2015 08:40 AM, Michał Dubiel wrote:
> > Hi all,
> >
> > May I kindly ask someone for some advice on this topic?
> >
> > Regards,
> > Michal
> >
> > On 21 May 2015 at 20:23, Michał Dubiel <md@xxxxxxxxxxxx
> > <mailto:md@xxxxxxxxxxxx>> wrote:
> >
> >     Hi guys,
> >
> >     I have got a question. I need to add apparmor support for vhost-user socket
> >     files used to communicate with the vhost-user server app. Those ones defined
> >     with something like:
> >     <interface type='vhostuser'>
> >           <mac address='02:ed:f3:5d:de:f3'/>
> >           <source type='unix' path='/var/run/vrouter/uvh_vif_tapa8396c51-2a'
> >     mode='client'/>
> >           <model type='virtio'/>
> >           <address type='pci' domain='0x0000' bus='0x00' slot='0x03'
> >     function='0x0'/>
> >     </interface>
> >
> >     I added something like this into get_files() function in virt-aa-helper.c:
> >         for (i = 0; i < ctl->def->nnets; i++) {
> >             if (ctl->def->nets[i] &&
> >                     ctl->def->nets[i]->type == VIR_DOMAIN_NET_TYPE_VHOSTUSER &&
> >                     ctl->def->nets[i]->data.vhostuser) {
> >                 virDomainChrSourceDefPtr vhu = ctl->def->nets[i]->data.vhostuser;
> >
> >                 if (vah_add_file_chardev(&buf, vhu->data.nix.path, "rw",
> >                            vhu->type) != 0)
> >                     goto cleanup;
> >             }
> >         }
> >
> >     However, there is a restriction for the socket file types in valid_path()
> >     function:
> >             switch (sb.st_mode & S_IFMT) {
> >                 case S_IFSOCK:
> >                     return 1;
> >                     break;
> >                 default:
> >                     break;
> >             }
> >     That prevents this from working.
> >
> >     May I ask why the socket file types are restricted? Vhost-user uses sockets
> >     so if I want to use apparmor virt-aa-helper has to be able to add the line
> >     for the socket file into /etc/apparmor.d/libvirt/libvirt-UUID.files.
> >
>
> They are restricted only because at the time virt-aa-helper.c was written there
> wasn't a valid use for them. There were more checks in this part of the code but
> over the years as more valid types were added to libvirt, they've been removed
> and now we are left with just this one. Since there is now a valid usecase for
> S_IFSOCK, it seems this can simply be removed.

So something like

diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c
index 35423b5..bb5e449 100644
--- a/src/security/virt-aa-helper.c
+++ b/src/security/virt-aa-helper.c
@@ -592,18 +592,8 @@ valid_path(const char *path, const bool readonly)

     if (!virFileExists(path)) {
         vah_warning(_("path does not exist, skipping file type checks"));
-    } else {
-        if (stat(path, &sb) == -1)
-            return -1;
-
-        switch (sb.st_mode & S_IFMT) {
-            case S_IFSOCK:
-                return 1;
-                break;
-            default:
-                break;
-        }
-    }
+    } else if (stat(path, &sb) == -1)
+        return -1;

     opaths = sizeof(override)/sizeof(*(override));

should be ok?

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

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