Re: [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  wrote:

> Quoting Jamie Strandboge (ja...@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  > > > 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:
> > > 
> > >   
> > >path='/var/run/vrouter/uvh_vif_tapa8396c51-2a'
> > > mode='client'/>
> > >   
> > >> > function='0x0'/>
> > > 
> > >
> > > 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@redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
>
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] Socket files in virt-aa-helper

[Serge Hallyn]

Quoting Jamie Strandboge (ja...@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  > > 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:
> > 
> >   
> >> mode='client'/>
> >   
> >> function='0x0'/>
> > 
> > 
> > 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@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] Socket files in virt-aa-helper

[Michał Dubiel]

Thanks Jamie for the explanation.

Regards,
Michal

On 16 June 2015 at 17:15, Jamie Strandboge  wrote:

> 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  > > 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:
> > 
> >   
> >path='/var/run/vrouter/uvh_vif_tapa8396c51-2a'
> > mode='client'/>
> >   
> >> function='0x0'/>
> > 
> >
> > 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.
>
> --
> Jamie Strandboge http://www.ubuntu.com/
>
>
> --
> libvir-list mailing list
> libvir-list@redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
>
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] Socket files in virt-aa-helper

[Jamie Strandboge]

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  > 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:
> 
>   
>mode='client'/>
>   
>function='0x0'/>
> 
> 
> 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.

-- 
Jamie Strandboge http://www.ubuntu.com/



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] Socket files in virt-aa-helper

[Michał Dubiel]

Hi all,

May I kindly ask someone for some advice on this topic?

Regards,
Michal

On 21 May 2015 at 20:23, Michał Dubiel  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:
> 
>   
>mode='client'/>
>   
>function='0x0'/>
> 
>
> 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.
>
> Regards,
> Michal
>
>
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] Socket files in virt-aa-helper

[Michał Dubiel]

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:

  
  
  
  


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.

Regards,
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list