[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH] [DOCS] nwfilter: documentation



On 05/22/2010 09:31 AM, Stefan Berger wrote:
> This patch adds documentation of the nwfilter subsystem of libvirt to
> the existing (web) docs. I am attaching a PDF in case you don't want to
> read the plain html sources.

Sometimes, that really is a much easier way to review :)  If nothing
else, it proves your patch was well-formed enough to make it through the
conversion to a pdf file without hassle.

> +
> +    <h2><a name="goals">Goals and background</a></h2>
> +
> +    <p>
> +      The goal of the network filtering XML is to enable administrators
> +      of virtualized system to configure and enforce network traffic

s/of virtualized/of a virtualized/

> +
> +    <h2><a name="nwfconcpts">Concepts</a></h2>

s/nwfconcpts/nwfconcepts/; does anything else link here yet?

> +    <p>
> +      The network traffic filtering subsystem enables configuration
> +      of network traffic filtering rules on individual network
> +      interfaces that are configured for certain types of
> +      network configurations. Supported network types are
> +    </p>
> +      <ul>
> +       <li><code>network</code></li>
> +       <li><code>ethernet</code> -- must be used in bridging mode</li>
> +       <li><code>bridge</code></li>
> +       <li><code>direct</code> -- only protocols mac, arp, ip and ipv6
> +            can be filtered</li>

Unrelated to this patch - does it make sense to add support for a filter
that rejects all traffic that does not contain one of these supported
protocols?

...
> +    <p>
> +    Network filters are written in XML and may either contain references
> +    to other filters, contain rules for traffic filtering or can

s/filtering or can/filtering, or/

> +    hold a combination of both. The above referenced filter
> +    <code>clean-traffic </code> is a filter that for example only

s/for example //

> +    contains references to
> +    other filters and no actual filtering rules. Since references to
> +    other filters can be used, a <i>tree</i> of filters can be built.
> +

...
> +    <h3><a name="nwfconcptsvars">Usage of variables in filters</a></h3>

s/nwfconcptsvars/nwfconceptvars/; does anything else link here yet?

> +
> +    <h2><a name="nwfelems">Element and attribute overview</a></h2>
> +
> +    <p>
> +      The root element required for all network filters is
> +      named <code>filter</code> with two possible attributes. The
> +      <code>name</code> attribute provides a unique name of the
> +      given filter. The <code>chain</code> attribute is optional but
> +      allows certain filters to be better organized for more efficient
> +      processing by the firewall subsystem of the underlying host.
> +      Currently the system only supports the chains <code>root,
> +      ipv4, ipv6, arp and rarp</code>.
> +    </p>
> +
> +    <h3><a name="nwfelemsRefs">References to other filers</a></h3>

s/filers/filters/

...
> +    <h2><a name="nwfcli">Command line tools</a></h2>
> +    <p>
> +      The libvirt command line tool <code>virsh</code> has been extended
> +      with life-cycle support for network filters. All commands related
> +      to the network filtering subsystem start with the prefix
> +      <code>nwfilter</code>. The following commands are available:
> +    <p>
> +    <ul>
> +     <li>nwfilter-list : list UUIDs and names of all network filters</li>
> +     <li>nwfilter-define : define a new network filter or update an existing one</li>
> +     <li>nwfilter-undefine : delete a network filter given its name; it must not be currently in use</li>
> +     <li>nwfilter-dumpxml : display a network filter given its name</li>
> +     <li>nwfilter-edit : edit a network filter given its name</li>
> +    </ul>
> +
> +    <h2><a name="nwfexamples">Example network filters</a></h2>
> +    <p>
> +     The following is a list of example network filters that are
> +     automatically installed with libvirt.     </p>

Unusual whitespace.

> +      <table class="top_table">
> +       <tr>
> +         <th> Name </th>
> +         <th> Description </th>
> +       </tr>
> +       <tr>
> +         <td> no-arp-spoofing </td>
> +         <td> Prevent a VM from spoofing ARP traffic; this filter
> +              only allows ARP request and reply messages and enforces
> +              that those packets contain the MAC and IP addresses
> +              of the VM.</td>

The section heading of "example network filters" is a bit misleading - I
was expecting to see the actual filter bodies here to form the example.
 Either we should document the filter bodies, or we should retitle the
section to "pre-existing network filters".  I could go either way, but
obviously re-titling the section is the only one of the two options that
is small enough to not affect my ACK (if you want to go with including
the filter bodies, submit it as a second patch).

...
> +    The role of the <code>chain</code> attribute in the network filter
> +    XML is that internally a new user-defined ebtables table is created
> +    that then for example receives all <code>arp</code> traffic coming
> +    from or going to a virtual machine, if the chain <code>arp</code>
> +    has been specified. Further, a rule is generated in an interface's
> +    <code>root</code> chain that directs all ipv4 traffic into the
> +    user-defined chain. Therefore, all ARP traffic rules should then be
> +    placed into filters specifying this chain. This type of branching
> +    into user-define tables is only supported with filtering on the ebtables

s/define/defined/

> +    layer.
> +
> +    <h3><a name="nwfwriteexample">Example custom filter</a></h3>
> +    <p>
> +     As an example we want to now build a filter that fulfills the following
> +     list of requirements:
> +    </p>
> +    <ul>
> +     <li>prevents a VM's interface from MAC, IP and ARP spoofing</li>
> +     <li>opens only TCP ports 22 and 80 of a VM's interface</li>
> +     <li>allows the VM to send ping traffic from an interface
> +        but no let the VM be pinged on the interface</li>
> +    </ul>
> +    <p>
> +     The requirement to prevent spoofing is fulfilled by the existing
> +     <code>clean-traffic</code> network filter, thus we will reference this
> +     filter from our custom filter.
> +     <br>
> +     To enable traffic for TCP ports 22 and 80 we will add 2 rules to
> +     enable this type of traffic. To allow the VM to send ping traffic
> +     we will add a rule for ICMP traffic. For simplicity reasons
> +     we allow general ICMP traffic to be initated from the VM, not
> +     just ICMP echo request and response messages. To then
> +     disallow all other traffic to reach or be initated by the
> +     VM we will then need to add a rule that drops all other traffic.
> +     Assuming our VM is called <i>test</i> and
> +     the interface we want to associate our filter with is called <i>eth0</i>,
> +     we name our filter <i>test-eth0</i>.
> +     The result of these considerations is the following network filter XML:
> +    </p>
> +<pre>
> +&lt;filter name='test-eth0'&gt;
> +  &lt;!-- reference the clean traffic filter preventing

s/preventing/to prevent/

> +       MAC, IP and ARP spoofing. By not providing
> +       and IP address parameter libvirt will detect the

s/parameter/parameter,/

> +       IP address the VM is using. --&gt;
> +  &lt;filterref filter='clean-traffic'/&gt;
> +
> +  &lt;!-- enable TCP ports 22 (ssh) and 80 (http) to be reachable --&gt;
> +  &lt;rule action='accept' direction='in'&gt;
> +    &lt;tcp dstportstart='22'/&gt;
> +  &lt;/rule&gt;

Didn't see mention any earlier that an omitted dstportend defaults to
dstportstart, but it wasn't too hard to figure out from this example.

> +
> +    <h3><a name="nwflimitsmigr">VM Migration</a></h3>
> +     <p>
> +      VM migration is only supported if the whole filter tree
> +      that is referenced by a virtual machine's top level filter
> +      is also available on the target host. The network filter
> +      <i>clean-traffic</i>
> +      for example should be available on all libvirt installations
> +      of version 0.8.1 or later and thus enable migration of VMs that
> +      for example reference this filter. All other
> +      custom filters must be migrated using higher layer software. It is
> +      outside the scope of libvirt to ensure that referenced filters
> +      on the source system are equivalent to those on the target system
> +      and vice versa.
> +      <br><br>
> +      Migration must occurr between libvirt insallations of version

s/occurr/occur/; s/insallations/installations/

> +      0.8.1 or later in order not to loose the network traffic filters

s/loose/lose/

> +      associated with an interface.
> +     </p>
> +
> +  </body>
> +</html>
> 

Big patch, but much appreciated.  You have my ACK, once the nits are
addressed; if we come up with follow-up patches, it will be easier to
see them as incremental diffs than to wait for a v2.

-- 
Eric Blake   eblake redhat com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]