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

Re: [libvirt] [PATCH] BZ 657918 Default iptables setup in libvirt breaks mDNS



On 12/11/2012 03:54 PM, Brian J. Murrell wrote:
> Hi,
> 
> Per the request on https://bugzilla.redhat.com/show_bug.cgi?id=657918
> please find attached a patch that should address the issue.

Thanks!

> 
> I'm not subscribed to this list though (I know, it's pretty rude, but
> my e-mail traffic is already too heavy to add another list to it), so
> if you could either CC me on any follow-up or just move followups to
> the BZ ticket where the patch also appears, that would be great.

Don't worry about it.  List policy is to reply-to-all precisely so that
you do not have to subscribe while still following a thread you initiate
(and mail-followup-to exists for those who do subscribe but don't like
duplicates).

I'll comment on the minor coding issues, but hopefully someone more
familiar with actual filtering can chime in on the logical correctness.

> 
> Cheers,
> b.
> 
> --- src/network/bridge_driver.c.orig	2012-10-27 16:56:23.000000000 -0400
> +++ src/network/bridge_driver.c	2012-12-11 15:49:13.937133883 -0500
> @@ -1301,9 +1301,10 @@
>       *
>       * We need to end up with 3 rules in the table in this order

Comment should now mention 4 rules.

>       *
> -     *  1. protocol=tcp with sport mapping restriction
> -     *  2. protocol=udp with sport mapping restriction
> -     *  3. generic any protocol
> +     *  1. multicast is exempted
> +     *  2. protocol=tcp with sport mapping restriction
> +     *  3. protocol=udp with sport mapping restriction
> +     *  4. generic any protocol
>       *
>       * The sport mappings are required, because default IPtables
>       * MASQUERADE maintain port numbers unchanged where possible.
> @@ -1361,8 +1362,21 @@
>          goto masqerr5;
>      }
>  
> +    /* exempt multicast traffic */
> +    if (iptablesAddForwardMasqueradeExempt(driver->iptables) < 0) {
> +        virReportError(VIR_ERR_SYSTEM_ERROR,
> +                           _("failed to add iptables rule to exempt multicast traffic from masquerading"));

Indentation is a bit off, and you need a "%s" argument to keep the
syntax-checker happy about a message with no other % operand.

> +        goto masqerr6;
> +    }
> +
>      return 0;
>  
> + masqerr6:
> +    iptablesRemoveForwardMasquerade(driver->iptables,
> +                                    &ipdef->address,
> +                                    prefix,
> +                                    forwardIf,
> +                                    "tcp");
>   masqerr5:
>      iptablesRemoveForwardMasquerade(driver->iptables,
>                                      &ipdef->address,
> --- src/util/iptables.c.orig	2012-10-27 16:56:23.000000000 -0400
> +++ src/util/iptables.c	2012-12-11 15:53:28.715044866 -0500
> @@ -858,6 +858,26 @@
>  }
>  
>  /**
> + * iptablesAddForwardMasqueradeExempt:
> + * @ctx: pointer to the IP table context
> + *
> + * Add rules to the IP table context to exempt masquerading
> + * for multicast networks
> + *
> + * Returns 0 in case of success or an error code otherwise
> + */
> +int
> +iptablesAddForwardMasqueradeExempt(iptablesContext *ctx)
> +{
> +    return iptablesAddRemoveRule(ctx->nat_postrouting,
> +                                 AF_INET,
> +                                 ADD,
> +                                 "--destination", "224.0.0.0/4",
> +                                 "--jump", "RETURN",
> +                                 NULL);
> +}

Do we need an IPv6 counterpart?  (Or am I just showing my ignorance of
what IPv6 does as a counterpart to IPv4 multicast?)

> +
> +/**
>   * iptablesAddForwardMasquerade:
>   * @ctx: pointer to the IP table context
>   * @network: the source network name
> --- src/util/iptables.h.orig	2012-10-27 16:56:23.000000000 -0400
> +++ src/util/iptables.h	2012-12-11 15:57:03.284144679 -0500
> @@ -101,6 +101,7 @@
>                                                    int family,
>                                                    const char *iface);
>  
> +int              iptablesAddForwardMasqueradeExempt (iptablesContext *ctx);
>  int              iptablesAddForwardMasquerade    (iptablesContext *ctx,
>                                                    virSocketAddr *netaddr,
>                                                    unsigned int prefix,
> --- src/libvirt_private.syms.orig	2012-12-11 15:46:11.141932324 -0500
> +++ src/libvirt_private.syms	2012-12-11 15:58:11.715865516 -0500
> @@ -681,6 +681,7 @@
>  iptablesAddForwardAllowOut;
>  iptablesAddForwardAllowRelatedIn;
>  iptablesAddForwardMasquerade;
> +iptablesAddForwardMasqueradeExempt;
>  iptablesAddForwardRejectIn;
>  iptablesAddForwardRejectOut;
>  iptablesAddOutputFixUdpChecksum;

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
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]