[libvirt] Socket files in virt-aa-helper

[Michał Dubiel]

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 at ubuntu.com> wrote:

> Quoting Jamie Strandboge (jamie at canonical.com):
> > 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 at semihalf.com
> > > <mailto:md at semihalf.com>> 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 at redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20150619/95766c97/attachment-0001.htm>