diff --git a/net/bridge.go b/net/bridge.go index 0c87bcbe..f46c08c9 100644 --- a/net/bridge.go +++ b/net/bridge.go @@ -411,7 +411,7 @@ func configureIPTables(config *BridgeConfig) error { } if config.DockerBridgeName != "" { if config.WeaveBridgeName != config.DockerBridgeName { - // This check is not ideal, as the rule should be at the very top + // This is not ideal, as it does not check whether the rule is at the top // of the chain. found, err := ipt.Exists("filter", "FORWARD", "-i", config.DockerBridgeName, "-o", config.WeaveBridgeName, "-j", "DROP") if err != nil { @@ -450,48 +450,46 @@ func configureIPTables(config *BridgeConfig) error { } } + // The order among weave filter/FORWARD rules is important! + fwdRules := make([][]string, 0) + if config.NPC { // Steer traffic via the NPC _ = ipt.NewChain("filter", "WEAVE-NPC") // ignore error because it might already exist - // If WEAVE-NPC chain doesn't exist then next line will fail - if err = ipt.AppendUnique("filter", "FORWARD", "-o", config.WeaveBridgeName, "-j", "WEAVE-NPC"); err != nil { - return err - } - if err = ipt.AppendUnique("filter", "FORWARD", "-o", config.WeaveBridgeName, "-m", "state", "--state", "NEW", "-j", "NFLOG", "--nflog-group", "86"); err != nil { - return err - } - if err = ipt.AppendUnique("filter", "FORWARD", "-o", config.WeaveBridgeName, "-j", "DROP"); err != nil { - return err - } + // If WEAVE-NPC chain doesn't exist then creating a rule in the chain will fail + fwdRules = append(fwdRules, + [][]string{ + {"-o", config.WeaveBridgeName, "-j", "WEAVE-NPC"}, + {"-o", config.WeaveBridgeName, "-m", "state", "--state", "NEW", "-j", "NFLOG", "--nflog-group", "86"}, + {"-o", config.WeaveBridgeName, "-j", "DROP"}, + }...) } else { // Work around the situation where there are no rules allowing traffic // across our bridge. E.g. ufw - if err = ipt.AppendUnique("filter", "FORWARD", "-i", config.WeaveBridgeName, "-o", config.WeaveBridgeName, "-j", "ACCEPT"); err != nil { - return err - } + fwdRules = append(fwdRules, []string{"-i", config.WeaveBridgeName, "-o", config.WeaveBridgeName, "-j", "ACCEPT"}) } if !config.NPC { // Create a chain for allowing ingress traffic when the bridge is exposed _ = ipt.NewChain("filter", "WEAVE-EXPOSE") - if err = ipt.AppendUnique("filter", "FORWARD", "-o", config.WeaveBridgeName, "-j", "WEAVE-EXPOSE"); err != nil { - return err - } + fwdRules = append(fwdRules, []string{"-o", config.WeaveBridgeName, "-j", "WEAVE-EXPOSE"}) } // Forward from weave to the rest of the world - if err = ipt.AppendUnique("filter", "FORWARD", "-i", config.WeaveBridgeName, "!", "-o", config.WeaveBridgeName, "-j", "ACCEPT"); err != nil { - return err - } + fwdRules = append(fwdRules, []string{"-i", config.WeaveBridgeName, "!", "-o", config.WeaveBridgeName, "-j", "ACCEPT"}) // and allow replies back - if err = ipt.AppendUnique("filter", "FORWARD", "-o", config.WeaveBridgeName, "-m", "conntrack", "--ctstate", "RELATED,ESTABLISHED", "-j", "ACCEPT"); err != nil { + fwdRules = append(fwdRules, []string{"-o", config.WeaveBridgeName, "-m", "conntrack", "--ctstate", "RELATED,ESTABLISHED", "-j", "ACCEPT"}) + + if err := ensureRules("filter", "FORWARD", fwdRules, ipt); err != nil { return err } // create a chain for masquerading - if err = ipt.ClearChain("nat", "WEAVE"); err != nil { - return errors.Wrap(err, "clearing WEAVE chain") - } + // + // NB: we do not clear the chain to preserve existing rules + // inserted by "weave expose". + _ = ipt.NewChain("nat", "WEAVE") + return ipt.AppendUnique("nat", "POSTROUTING", "-j", "WEAVE") } @@ -502,3 +500,40 @@ func linkSetUpByName(linkName string) error { } return netlink.LinkSetUp(link) } + +// ensureRules ensures the presence of given iptables rules. +// +// If any rule from the list is missing, the function deletes all given +// rules and re-appends them to ensure the order of the rules. +func ensureRules(table, chain string, rulespecs [][]string, ipt *iptables.IPTables) error { + allFound := true + + for _, rs := range rulespecs { + found, err := ipt.Exists(table, chain, rs...) + if err != nil { + return errors.Wrapf(err, "ipt.Exists(%s, %s, %s)", table, chain, rs) + } + if !found { + allFound = false + break + } + } + + // All rules exist, do nothing. + if allFound { + return nil + } + + for _, rs := range rulespecs { + // If any is missing, then delete all, as we need to preserve the order of + // given rules. Ignore errors, as rule might not exist. + if !allFound { + ipt.Delete(table, chain, rs...) + } + if err := ipt.Append(table, chain, rs...); err != nil { + return errors.Wrapf(err, "ipt.Append(%s, %s, %s)", table, chain, rs) + } + } + + return nil +}