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

Re: [libvirt] [PATCH v3 2/4] util/viriptables: add/remove rules that short-circuit masquerading



On 09/23/2013 08:03 PM, Laszlo Ersek wrote:
> The functions
> - iptablesAddDontMasquerade(),
> - iptablesRemoveDontMasquerade()
> 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 redhat com>
> ---
>  v2->v3:
>  - Rename iptables(Add|Remove)ForwardDontMasquerade to
>           iptables(Add|Remove)DontMasquerade [Laine].
>  - Pass (address, prefix) pairs as both source and destination parameters
>    to these functions.
>  - Introduce virPfxSocketAddr structure for simpler handling of said
>    (address, prefix) pairs.
>
>  src/util/viriptables.h   | 11 +++++++
>  src/util/viriptables.c   | 82 ++++++++++++++++++++++++++++++++++++++++++++++++
>  src/libvirt_private.syms |  2 ++
>  3 files changed, 95 insertions(+)
>
> diff --git a/src/util/viriptables.h b/src/util/viriptables.h
> index 447f4a8..fcff5f8 100644
> --- a/src/util/viriptables.h
> +++ b/src/util/viriptables.h
> @@ -26,6 +26,11 @@
>  
>  # include "virsocketaddr.h"
>  
> +typedef struct {
> +    virSocketAddr addr;
> +    unsigned int prefix;
> +} virPfxSocketAddr;
> +

I'm conflicted about whether or not it's really beneficial to have this
struct (is the setup really worth it just to save one arg?), but I
suppose it doesn't hurt anything, so I'll go along with it if nobody
else objects. It *does* make these functions' APIs inconsistent with the
other iptables functions that take a separate addr and prefix, though -
if we keep this then we should probably update the rest of the API to be
consistent (not in this patch though).


>  int              iptablesAddTcpInput             (int family,
>                                                    const char *iface,
>                                                    int port);
> @@ -94,6 +99,12 @@ int              iptablesRemoveForwardMasquerade (virSocketAddr *netaddr,
>                                                    virSocketAddrRangePtr addr,
>                                                    virPortRangePtr port,
>                                                    const char *protocol);
> +int              iptablesAddDontMasquerade       (const virPfxSocketAddr *src,
> +                                                  const char *physdev,
> +                                                  const virPfxSocketAddr *dst);
> +int              iptablesRemoveDontMasquerade    (const virPfxSocketAddr *src,
> +                                                  const char *physdev,
> +                                                  const virPfxSocketAddr *dst);
>  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 52e2bde..90fe900 100644
> --- a/src/util/viriptables.c
> +++ b/src/util/viriptables.c
> @@ -832,6 +832,88 @@ iptablesRemoveForwardMasquerade(virSocketAddr *netaddr,
>  }
>  
>  
> +/* Don't masquerade traffic coming from the network associated with the bridge
> + * if said traffic targets @destaddr/@destprefix.
> + */
> +static int
> +iptablesDontMasquerade(const virPfxSocketAddr *src,
> +                       const char *physdev,
> +                       const virPfxSocketAddr *dst,
> +                       int action)
> +{
> +    int ret = -1;
> +    char *srcStr = NULL;
> +    char *dstStr = NULL;
> +    virCommandPtr cmd = NULL;
> +
> +    if (!(srcStr = iptablesFormatNetwork(&src->addr, src->prefix)))
> +        return -1;
> +
> +    if (!(dstStr = iptablesFormatNetwork(&dst->addr, dst->prefix)))
> +        goto cleanup;
> +
> +    if (!VIR_SOCKET_ADDR_IS_FAMILY(&src->addr, 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."),
> +                       srcStr);
> +        goto cleanup;
> +    }

Don't you need to do the same check for dst?

> +
> +    cmd = iptablesCommandNew("nat", "POSTROUTING", AF_INET, action);
> +
> +    if (physdev && physdev[0])
> +        virCommandAddArgList(cmd, "--out-interface", physdev, NULL);
> +
> +    virCommandAddArgList(cmd, "--source", srcStr, "--destination", dstStr,
> +                         "--jump", "RETURN", NULL);
> +    ret = virCommandRun(cmd, NULL);
> +cleanup:
> +    virCommandFree(cmd);
> +    VIR_FREE(dstStr);
> +    VIR_FREE(srcStr);
> +    return ret;
> +}
> +
> +/**
> + * iptablesAddDontMasquerade:
> + * @src: the source network address and prefix
> + * @physdev: the physical output device or NULL
> + * @destaddr: the destination network address and prefix
> + *
> + * Add rules to the "nat" IP table to avoid masquerading from @src/srcprefix to
> + * @dst/dstprefix on @physdev.
> + *
> + * Returns 0 in case of success and -1 on failure.
> + */
> +int
> +iptablesAddDontMasquerade(const virPfxSocketAddr *src,
> +                          const char *physdev,
> +                          const virPfxSocketAddr *dst)
> +{
> +    return iptablesDontMasquerade(src, physdev, dst, ADD);
> +}
> +
> +/**
> + * iptablesRemoveDontMasquerade:
> + * @src: the source network address and prefix
> + * @physdev: the physical output device or NULL
> + * @destaddr: the destination network address and prefix
> + *
> + * Remove rules from the "nat" IP table that prevent masquerading from
> + * @src/srcprefix to @dst/dstprefix on @physdev.
> + *
> + * Returns 0 in case of success and -1 on failure.
> + */
> +int
> +iptablesRemoveDontMasquerade(const virPfxSocketAddr *src,
> +                             const char *physdev,
> +                             const virPfxSocketAddr *dst)
> +{
> +    return iptablesDontMasquerade(src, physdev, dst, REMOVE);
> +}
> +
> +
>  static int
>  iptablesOutputFixUdpChecksum(const char *iface,
>                               int port,
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index 50e2f48..fb212ce 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -1446,6 +1446,7 @@ virInitctlSetRunLevel;
>  
>  
>  # util/viriptables.h
> +iptablesAddDontMasquerade;
>  iptablesAddForwardAllowCross;
>  iptablesAddForwardAllowIn;
>  iptablesAddForwardAllowOut;
> @@ -1456,6 +1457,7 @@ iptablesAddForwardRejectOut;
>  iptablesAddOutputFixUdpChecksum;
>  iptablesAddTcpInput;
>  iptablesAddUdpInput;
> +iptablesRemoveDontMasquerade;
>  iptablesRemoveForwardAllowCross;
>  iptablesRemoveForwardAllowIn;
>  iptablesRemoveForwardAllowOut;


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