[libvirt] [PATCH v3 4/4] util: move virtual network firwall rules into private chains

[Michal Privoznik]

On 1/24/19 3:05 PM, Daniel P. Berrangé wrote:
> The previous commit created new chains to hold the firewall rules. This
> commit changes the code that creates rules to place them in the new
> private chains instead of the builtin top level chains.
> 
> With two networks running, the rules in the filter table now look like
> 
>    -N LIBVIRT_FWI
>    -N LIBVIRT_FWO
>    -N LIBVIRT_FWX
>    -N LIBVIRT_INP
>    -N LIBVIRT_OUT
>    -A INPUT -j LIBVIRT_INP
>    -A FORWARD -j LIBVIRT_FWX
>    -A FORWARD -j LIBVIRT_FWI
>    -A FORWARD -j LIBVIRT_FWO
>    -A OUTPUT -j LIBVIRT_OUT
>    -A LIBVIRT_FWI -d 192.168.0.0/24 -o virbr0 -m conntrack --ctstate RELATED,ESTABLISHED -j ACCEPT
>    -A LIBVIRT_FWI -o virbr0 -j REJECT --reject-with icmp-port-unreachable
>    -A LIBVIRT_FWI -d 192.168.1.0/24 -o virbr1 -m conntrack --ctstate RELATED,ESTABLISHED -j ACCEPT
>    -A LIBVIRT_FWI -o virbr1 -j REJECT --reject-with icmp-port-unreachable
>    -A LIBVIRT_FWO -s 192.168.0.0/24 -i virbr0 -j ACCEPT
>    -A LIBVIRT_FWO -i virbr0 -j REJECT --reject-with icmp-port-unreachable
>    -A LIBVIRT_FWO -s 192.168.1.0/24 -i virbr1 -j ACCEPT
>    -A LIBVIRT_FWO -i virbr1 -j REJECT --reject-with icmp-port-unreachable
>    -A LIBVIRT_FWX -i virbr0 -o virbr0 -j ACCEPT
>    -A LIBVIRT_FWX -i virbr1 -o virbr1 -j ACCEPT
>    -A LIBVIRT_INP -i virbr0 -p udp -m udp --dport 53 -j ACCEPT
>    -A LIBVIRT_INP -i virbr0 -p tcp -m tcp --dport 53 -j ACCEPT
>    -A LIBVIRT_INP -i virbr0 -p udp -m udp --dport 67 -j ACCEPT
>    -A LIBVIRT_INP -i virbr0 -p tcp -m tcp --dport 67 -j ACCEPT
>    -A LIBVIRT_INP -i virbr1 -p udp -m udp --dport 53 -j ACCEPT
>    -A LIBVIRT_INP -i virbr1 -p tcp -m tcp --dport 53 -j ACCEPT
>    -A LIBVIRT_INP -i virbr1 -p udp -m udp --dport 67 -j ACCEPT
>    -A LIBVIRT_INP -i virbr1 -p tcp -m tcp --dport 67 -j ACCEPT
>    -A LIBVIRT_OUT -o virbr0 -p udp -m udp --dport 68 -j ACCEPT
>    -A LIBVIRT_OUT -o virbr1 -p udp -m udp --dport 68 -j ACCEPT
> 
> While in the nat table:
> 
>    -N LIBVIRT_PRT
>    -A POSTROUTING -j LIBVIRT_PRT
>    -A LIBVIRT_PRT -s 192.168.0.0/24 -d 224.0.0.0/24 -j RETURN
>    -A LIBVIRT_PRT -s 192.168.0.0/24 -d 255.255.255.255/32 -j RETURN
>    -A LIBVIRT_PRT -s 192.168.0.0/24 ! -d 192.168.0.0/24 -p tcp -j MASQUERADE --to-ports 1024-65535
>    -A LIBVIRT_PRT -s 192.168.0.0/24 ! -d 192.168.0.0/24 -p udp -j MASQUERADE --to-ports 1024-65535
>    -A LIBVIRT_PRT -s 192.168.0.0/24 ! -d 192.168.0.0/24 -j MASQUERADE
>    -A LIBVIRT_PRT -s 192.168.1.0/24 -d 224.0.0.0/24 -j RETURN
>    -A LIBVIRT_PRT -s 192.168.1.0/24 -d 255.255.255.255/32 -j RETURN
>    -A LIBVIRT_PRT -s 192.168.1.0/24 ! -d 192.168.1.0/24 -p tcp -j MASQUERADE --to-ports 1024-65535
>    -A LIBVIRT_PRT -s 192.168.1.0/24 ! -d 192.168.1.0/24 -p udp -j MASQUERADE --to-ports 1024-65535
>    -A LIBVIRT_PRT -s 192.168.1.0/24 ! -d 192.168.1.0/24 -j MASQUERADE
> 
> And finally the mangle table:
> 
>    -N LIBVIRT_PRT
>    -A POSTROUTING -j LIBVIRT_PRT
>    -A LIBVIRT_PRT -o virbr0 -p udp -m udp --dport 68 -j CHECKSUM --checksum-fill
>    -A LIBVIRT_PRT -o virbr1 -p udp -m udp --dport 68 -j CHECKSUM --checksum-fill
> 
> Signed-off-by: Daniel P. Berrangé <berrange at redhat.com>
> ---
>   src/libvirt_private.syms                      |   1 +
>   src/network/bridge_driver_linux.c             |  20 ++-
>   src/util/viriptables.c                        | 123 +++++++++++-------
>   src/util/viriptables.h                        |   2 +
>   .../nat-default-linux.args                    |  32 ++---
>   .../nat-ipv6-linux.args                       |  48 +++----
>   .../nat-many-ips-linux.args                   |  60 ++++-----
>   .../nat-no-dhcp-linux.args                    |  46 +++----
>   .../nat-tftp-linux.args                       |  34 ++---
>   .../route-default-linux.args                  |  22 ++--
>   10 files changed, 222 insertions(+), 166 deletions(-)
> 
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index a88856557d..77fc26376d 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -2075,6 +2075,7 @@ iptablesRemoveOutputFixUdpChecksum;
>   iptablesRemoveTcpInput;
>   iptablesRemoveUdpInput;
>   iptablesRemoveUdpOutput;
> +iptablesSetDeletePrivate;
>   iptablesSetupPrivateChains;
>   
>   
> diff --git a/src/network/bridge_driver_linux.c b/src/network/bridge_driver_linux.c
> index 61f77f2735..1e033fa21b 100644
> --- a/src/network/bridge_driver_linux.c
> +++ b/src/network/bridge_driver_linux.c
> @@ -34,17 +34,35 @@ VIR_LOG_INIT("network.bridge_driver_linux");
>   
>   #define PROC_NET_ROUTE "/proc/net/route"
>   
> -int networkPreReloadFirewallRules(bool startup ATTRIBUTE_UNUSED)
> +int networkPreReloadFirewallRules(bool startup)
>   {
>       int ret = iptablesSetupPrivateChains();
>       if (ret < 0)
>           return -1;
> +
> +    /*
> +     * If this is initial startup, and we just created the
> +     * top level private chains we either
> +     *
> +     *   - upgraded from old libvirt
> +     *   - freshly booted from clean state
> +     *
> +     * In the first case we must delete the old rules from
> +     * the built-in chains, instead of our new private chains.
> +     * In the second case it doesn't matter, since no existing
> +     * rules will be present. Thus we can safely just tell it
> +     * to always delete from the builin chain
> +     */
> +    if (startup && ret == 1) {
> +        iptablesSetDeletePrivate(false);
> +    }

No need for curly braces here.

>       return 0;
>   }
>   
>   
>   void networkPostReloadFirewallRules(bool startup ATTRIBUTE_UNUSED)
>   {
> +    iptablesSetDeletePrivate(true);
>   }
>   
>   
> diff --git a/src/util/viriptables.c b/src/util/viriptables.c
> index 770dcf04a6..04fcc84705 100644
> --- a/src/util/viriptables.c
> +++ b/src/util/viriptables.c
> @@ -48,6 +48,7 @@ enum {
>       REMOVE
>   };
>   
> +static int deletePrivate = true;

s/int/bool/

ACK

Michal