[libvirt] [PATCH v2 1/2] nwfilter: Disallow binding creation in session mode

John Ferlan posted 2 patches 7 years, 5 months ago
[libvirt] [PATCH v2 1/2] nwfilter: Disallow binding creation in session mode
Posted by John Ferlan 7 years, 5 months ago
Similar to nwfilterDefineXML, let's be sure the a filter binding
creation is not attempted in session mode and generate the proper
error message.

Failure to open nwfilter in session mode (nwfilterConnectOpen)
fails already, but that doesn't stop the free thinker from using
a different connection in order to attempt to attempt to create
the binding. Although even doing that would result in a failure:

$ virsh nwfilter-binding-create QEMUGuest1-binding.xml
error: Failed to create network filter from QEMUGuest1-binding.xml
error: internal error: Could not get access to ACL tech driver 'ebiptables'

$

Signed-off-by: John Ferlan <jferlan@redhat.com>
---
 src/nwfilter/nwfilter_driver.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/src/nwfilter/nwfilter_driver.c b/src/nwfilter/nwfilter_driver.c
index ac3a964388..1ee5162b9a 100644
--- a/src/nwfilter/nwfilter_driver.c
+++ b/src/nwfilter/nwfilter_driver.c
@@ -745,6 +745,12 @@ nwfilterBindingCreateXML(virConnectPtr conn,
 
     virCheckFlags(0, NULL);
 
+    if (!driver->privileged) {
+        virReportError(VIR_ERR_OPERATION_INVALID, "%s",
+                       _("Can't define NWFilter bindings in session mode"));
+        return NULL;
+    }
+
     def = virNWFilterBindingDefParseString(xml);
     if (!def)
         return NULL;
-- 
2.17.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 1/2] nwfilter: Disallow binding creation in session mode
Posted by Daniel P. Berrangé 7 years, 5 months ago
On Thu, Aug 30, 2018 at 11:06:07AM -0400, John Ferlan wrote:
> Similar to nwfilterDefineXML, let's be sure the a filter binding
> creation is not attempted in session mode and generate the proper
> error message.
> 
> Failure to open nwfilter in session mode (nwfilterConnectOpen)
> fails already, but that doesn't stop the free thinker from using
> a different connection in order to attempt to attempt to create
> the binding. Although even doing that would result in a failure:
> 
> $ virsh nwfilter-binding-create QEMUGuest1-binding.xml
> error: Failed to create network filter from QEMUGuest1-binding.xml
> error: internal error: Could not get access to ACL tech driver 'ebiptables'
> 
> $
> 
> Signed-off-by: John Ferlan <jferlan@redhat.com>
> ---
>  src/nwfilter/nwfilter_driver.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/src/nwfilter/nwfilter_driver.c b/src/nwfilter/nwfilter_driver.c
> index ac3a964388..1ee5162b9a 100644
> --- a/src/nwfilter/nwfilter_driver.c
> +++ b/src/nwfilter/nwfilter_driver.c
> @@ -745,6 +745,12 @@ nwfilterBindingCreateXML(virConnectPtr conn,
>  
>      virCheckFlags(0, NULL);
>  
> +    if (!driver->privileged) {
> +        virReportError(VIR_ERR_OPERATION_INVALID, "%s",
> +                       _("Can't define NWFilter bindings in session mode"));
> +        return NULL;
> +    }
> +
>      def = virNWFilterBindingDefParseString(xml);
>      if (!def)
>          return NULL;

How do we ever get to this point in a session daemon ?

The nwfilterConnectOpen() method should have failed due to 'driver'
being NULL, so the virConnectPtr doesn't exist and so no driver
callback points to nwfilterBindingCreateXML.


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 1/2] nwfilter: Disallow binding creation in session mode
Posted by John Ferlan 7 years, 5 months ago

On 08/30/2018 12:27 PM, Daniel P. Berrangé wrote:
> On Thu, Aug 30, 2018 at 11:06:07AM -0400, John Ferlan wrote:
>> Similar to nwfilterDefineXML, let's be sure the a filter binding
>> creation is not attempted in session mode and generate the proper
>> error message.
>>
>> Failure to open nwfilter in session mode (nwfilterConnectOpen)
>> fails already, but that doesn't stop the free thinker from using
>> a different connection in order to attempt to attempt to create
>> the binding. Although even doing that would result in a failure:
>>
>> $ virsh nwfilter-binding-create QEMUGuest1-binding.xml
>> error: Failed to create network filter from QEMUGuest1-binding.xml
>> error: internal error: Could not get access to ACL tech driver 'ebiptables'
>>
>> $
>>
>> Signed-off-by: John Ferlan <jferlan@redhat.com>
>> ---
>>  src/nwfilter/nwfilter_driver.c | 6 ++++++
>>  1 file changed, 6 insertions(+)
>>
>> diff --git a/src/nwfilter/nwfilter_driver.c b/src/nwfilter/nwfilter_driver.c
>> index ac3a964388..1ee5162b9a 100644
>> --- a/src/nwfilter/nwfilter_driver.c
>> +++ b/src/nwfilter/nwfilter_driver.c
>> @@ -745,6 +745,12 @@ nwfilterBindingCreateXML(virConnectPtr conn,
>>  
>>      virCheckFlags(0, NULL);
>>  
>> +    if (!driver->privileged) {
>> +        virReportError(VIR_ERR_OPERATION_INVALID, "%s",
>> +                       _("Can't define NWFilter bindings in session mode"));
>> +        return NULL;
>> +    }
>> +
>>      def = virNWFilterBindingDefParseString(xml);
>>      if (!def)
>>          return NULL;
> 
> How do we ever get to this point in a session daemon ?

Like I noted in the commit message an enterprising user...

With no guest running:

$ virsh -c qemu:///session nwfilter-binding-create QEMUGuest1-binding.xml
Network filter binding on tap0 created from QEMUGuest1-binding.xml

$

With a guest running, one would get the error:

$ virsh -c qemu:///session nwfilter-binding-create QEMUGuest1-binding.xml
error: Failed to create network filter from QEMUGuest1-binding.xml
error: internal error: Could not get access to ACL tech driver 'ebiptables'

$

It fails now, so I suppose it doesn't matter other than the tap0 which
when the enterprising consumer does:

$ virsh -c qemu:///session nwfilter-binding-create QEMUGuest1-binding.xml
Network filter binding on tap0 created from QEMUGuest1-binding.xml

$ virsh start QEMUGuest1
Domain QEMUGuest1 started

$ virsh nwfilter-binding-list
 Port Dev              Filter
------------------------------------------------------------------
 tap0                  clean-traffic

$ virsh dumpxml QEMUGuest1
<domain type='qemu' id='3'>
  <name>QEMUGuest1</name>
...
    <interface type='bridge'>
      <mac address='52:54:00:f7:d6:f9'/>
      <source network='default' bridge='virbr0'/>
      <target dev='tap0'/>
      <model type='virtio'/>
      <alias name='net0'/>
      <address type='pci' domain='0x0000' bus='0x00' slot='0x02'
function='0x0'/>
    </interface>
...

$
$ cat QEMUGuest1-binding.xml
<filterbinding>
  <owner>
    <name>QEMUGuest1</name>
    <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
  </owner>
  <portdev name='tap0'/>
  <mac address='52:54:00:f7:d6:f9'/>
  <filterref filter='clean-traffic'>
    <parameter name='CTRL_IP_LEARNING' value='dhcp'/>
  </filterref>
</filterbinding>


My AVC'S started firing like crazy...


John

> 
> The nwfilterConnectOpen() method should have failed due to 'driver'
> being NULL, so the virConnectPtr doesn't exist and so no driver
> callback points to nwfilterBindingCreateXML.
> 
> 
> Regards,
> Daniel
> 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 1/2] nwfilter: Disallow binding creation in session mode
Posted by Daniel P. Berrangé 7 years, 5 months ago
On Thu, Aug 30, 2018 at 12:50:09PM -0400, John Ferlan wrote:
> 
> 
> On 08/30/2018 12:27 PM, Daniel P. Berrangé wrote:
> > On Thu, Aug 30, 2018 at 11:06:07AM -0400, John Ferlan wrote:
> >> Similar to nwfilterDefineXML, let's be sure the a filter binding
> >> creation is not attempted in session mode and generate the proper
> >> error message.
> >>
> >> Failure to open nwfilter in session mode (nwfilterConnectOpen)
> >> fails already, but that doesn't stop the free thinker from using
> >> a different connection in order to attempt to attempt to create
> >> the binding. Although even doing that would result in a failure:
> >>
> >> $ virsh nwfilter-binding-create QEMUGuest1-binding.xml
> >> error: Failed to create network filter from QEMUGuest1-binding.xml
> >> error: internal error: Could not get access to ACL tech driver 'ebiptables'
> >>
> >> $
> >>
> >> Signed-off-by: John Ferlan <jferlan@redhat.com>
> >> ---
> >>  src/nwfilter/nwfilter_driver.c | 6 ++++++
> >>  1 file changed, 6 insertions(+)
> >>
> >> diff --git a/src/nwfilter/nwfilter_driver.c b/src/nwfilter/nwfilter_driver.c
> >> index ac3a964388..1ee5162b9a 100644
> >> --- a/src/nwfilter/nwfilter_driver.c
> >> +++ b/src/nwfilter/nwfilter_driver.c
> >> @@ -745,6 +745,12 @@ nwfilterBindingCreateXML(virConnectPtr conn,
> >>  
> >>      virCheckFlags(0, NULL);
> >>  
> >> +    if (!driver->privileged) {
> >> +        virReportError(VIR_ERR_OPERATION_INVALID, "%s",
> >> +                       _("Can't define NWFilter bindings in session mode"));
> >> +        return NULL;
> >> +    }
> >> +
> >>      def = virNWFilterBindingDefParseString(xml);
> >>      if (!def)
> >>          return NULL;
> > 
> > How do we ever get to this point in a session daemon ?
> 
> Like I noted in the commit message an enterprising user...
> 
> With no guest running:
> 
> $ virsh -c qemu:///session nwfilter-binding-create QEMUGuest1-binding.xml
> Network filter binding on tap0 created from QEMUGuest1-binding.xml

Oh, i see it is because when using qemu://session, we never
actually call the nwfilterConnectOpen method - it is opened
implicitly. So

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

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