[libvirt] [PATCH v2 1/2] util/viriptables: add/remove rules that short-circuit masquerading

[Laine Stump]

On 09/23/2013 10:05 AM, Laszlo Ersek wrote:
> The functions
> - iptablesAddForwardDontMasquerade(),
> - iptablesRemoveForwardDontMasquerade
> handle exceptions in the masquerading implemented in the POSTROUTING chain
> of the "nat" table. Such exceptions should be added as chronologically
> latest, logically top-most rules.
>
> The bridge driver will call these functions beginning with the next patch:
> some special destination IP addresses always refer to the local
> subnetwork, even though they don't match any practical subnetwork's
> netmask. Packets from virbrN targeting such IP addresses are never routed
> outwards, but the current rules treat them as non-virbrN-destined packets
> and masquerade them. This causes problems for some receivers on virbrN.
>
> Signed-off-by: Laszlo Ersek <lersek at redhat.com>
> ---
>  src/util/viriptables.h   |  8 +++++
>  src/util/viriptables.c   | 88 ++++++++++++++++++++++++++++++++++++++++++++++++
>  src/libvirt_private.syms |  2 ++
>  3 files changed, 98 insertions(+)
>
> diff --git a/src/util/viriptables.h b/src/util/viriptables.h
> index 447f4a8..aee1842 100644
> --- a/src/util/viriptables.h
> +++ b/src/util/viriptables.h
> @@ -94,6 +94,14 @@ int              iptablesRemoveForwardMasquerade (virSocketAddr *netaddr,
>                                                    virSocketAddrRangePtr addr,
>                                                    virPortRangePtr port,
>                                                    const char *protocol);
> +int              iptablesAddForwardDontMasquerade(virSocketAddr *netaddr,
> +                                                  unsigned int prefix,
> +                                                  const char *physdev,
> +                                                  const char *destaddr);
> +int              iptablesRemoveForwardDontMasquerade(virSocketAddr *netaddr,
> +                                                  unsigned int prefix,
> +                                                  const char *physdev,
> +                                                  const char *destaddr);
>  int              iptablesAddOutputFixUdpChecksum (const char *iface,
>                                                    int port);
>  int              iptablesRemoveOutputFixUdpChecksum (const char *iface,
> diff --git a/src/util/viriptables.c b/src/util/viriptables.c
> index 0f57d66..349734c 100644
> --- a/src/util/viriptables.c
> +++ b/src/util/viriptables.c
> @@ -832,6 +832,94 @@ iptablesRemoveForwardMasquerade(virSocketAddr *netaddr,
>  }
>  
>  
> +/* Don't masquerade traffic coming from the network associated with the bridge
> + * if said traffic targets @destaddr.
> + */
> +static int
> +iptablesForwardDontMasquerade(virSocketAddr *netaddr,
> +                              unsigned int prefix,
> +                              const char *physdev,
> +                              const char *destaddr,
> +                              int action)


This function was based on the existing function
iptablesForwardMasquerade(), replacing addr/port/protocol with the
single destaddr. We may want to specify protocol or port in the future,
but for now we don't need it, and we can always add those args in when
we need to, so I'm okay with this.

The name of the function is a bit troublesome to me though, since it's
actually being used to setup rules for packets that *aren't* being
forwarded (and the rules aren't going into the FORWARD table). How about
naming it "iptablesDontMasquerade"? Some other name?


> +{
> +    int ret = -1;
> +    char *networkstr = NULL;
> +    virCommandPtr cmd = NULL;
> +
> +    if (!(networkstr = iptablesFormatNetwork(netaddr, prefix)))
> +        return -1;
> +
> +    if (!VIR_SOCKET_ADDR_IS_FAMILY(netaddr, AF_INET)) {
> +        /* Higher level code *should* guaranteee it's impossible to get here. */
> +        virReportError(VIR_ERR_INTERNAL_ERROR,
> +                       _("Attempted to NAT '%s'. NAT is only supported for IPv4."),
> +                       networkstr);
> +        goto cleanup;
> +    }
> +
> +    cmd = iptablesCommandNew("nat", "POSTROUTING", AF_INET, action);
> +
> +    if (physdev && physdev[0])
> +        virCommandAddArgList(cmd, "--out-interface", physdev, NULL);
> +
> +    virCommandAddArgList(cmd, "--source", networkstr,
> +                         "--destination", destaddr, "--jump", "RETURN", NULL);
> +    ret = virCommandRun(cmd, NULL);
> +cleanup:
> +    virCommandFree(cmd);
> +    VIR_FREE(networkstr);
> +    return ret;
> +}
> +
> +/**
> + * iptablesAddForwardDontMasquerade:
> + * @netaddr: the source network name
> + * @prefix: prefix (# of 1 bits) of netmask to apply to @netaddr
> + * @physdev: the physical output device or NULL
> + * @destaddr: the destination network not to masquerade for
> + *
> + * Add rules to the IP table context to avoid masquerading from
> + * @netaddr/@prefix to @destaddr on @physdev. @destaddr must be in a format
> + * directly consumable by iptables, it must not depend on user input or
> + * configuration.
> + *
> + * Returns 0 in case of success or an error code otherwise.
> + */
> +int
> +iptablesAddForwardDontMasquerade(virSocketAddr *netaddr,
> +                                 unsigned int prefix,
> +                                 const char *physdev,
> +                                 const char *destaddr)
> +{
> +    return iptablesForwardDontMasquerade(netaddr, prefix, physdev, destaddr,
> +                                         ADD);
> +}
> +
> +/**
> + * iptablesRemoveForwardDontMasquerade:
> + * @netaddr: the source network name
> + * @prefix: prefix (# of 1 bits) of netmask to apply to @netaddr
> + * @physdev: the physical output device or NULL
> + * @destaddr: the destination network not to masquerade for
> + *
> + * Remove rules from the IP table context that prevent masquerading from
> + * @netaddr/@prefix to @destaddr on @physdev. @destaddr must be in a format
> + * directly consumable by iptables, it must not depend on user input or
> + * configuration.
> + *
> + * Returns 0 in case of success or an error code otherwise.
> + */
> +int
> +iptablesRemoveForwardDontMasquerade(virSocketAddr *netaddr,
> +                                    unsigned int prefix,
> +                                    const char *physdev,
> +                                    const char *destaddr)
> +{
> +    return iptablesForwardDontMasquerade(netaddr, prefix, physdev, destaddr,
> +                                         REMOVE);
> +}
> +
> +
>  static int
>  iptablesOutputFixUdpChecksum(const char *iface,
>                               int port,
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index 50e2f48..49505c9 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -1450,6 +1450,7 @@ iptablesAddForwardAllowCross;
>  iptablesAddForwardAllowIn;
>  iptablesAddForwardAllowOut;
>  iptablesAddForwardAllowRelatedIn;
> +iptablesAddForwardDontMasquerade;
>  iptablesAddForwardMasquerade;
>  iptablesAddForwardRejectIn;
>  iptablesAddForwardRejectOut;
> @@ -1460,6 +1461,7 @@ iptablesRemoveForwardAllowCross;
>  iptablesRemoveForwardAllowIn;
>  iptablesRemoveForwardAllowOut;
>  iptablesRemoveForwardAllowRelatedIn;
> +iptablesRemoveForwardDontMasquerade;
>  iptablesRemoveForwardMasquerade;
>  iptablesRemoveForwardRejectIn;
>  iptablesRemoveForwardRejectOut;