Skip to content

Commit

Permalink
Merge pull request moby#46493 from rhansen/bridge-cleanups
Browse files Browse the repository at this point in the history 
bridge driver: various code quality improvements
  • Loading branch information
@corhere
corhere authored Oct 16, 2023
2 parents 6040283 + 96f85de commit af22957
Show file tree
Hide file tree
Showing 6 changed files with 113 additions and 114 deletions.
6 changes: 3 additions & 3 deletions libnetwork/drivers/bridge/bridge_linux.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ type networkConfiguration struct {
Mtu int
DefaultBindingIP net.IP
DefaultBridge bool
HostIP net.IP
HostIPv4 net.IP
ContainerIfacePrefix string
// Internal fields set after ipam data parsing
AddressIPv4 *net.IPNet
Expand Down Expand Up @@ -266,8 +266,8 @@ func (c *networkConfiguration) fromLabels(labels map[string]string) error {
}
case netlabel.ContainerIfacePrefix:
c.ContainerIfacePrefix = value
case netlabel.HostIP:
if c.HostIP = net.ParseIP(value); c.HostIP == nil {
case netlabel.HostIPv4:
if c.HostIPv4 = net.ParseIP(value); c.HostIPv4 == nil {
return parseErr(label, value, "nil ip")
}
}
Expand Down
10 changes: 5 additions & 5 deletions libnetwork/drivers/bridge/bridge_linux_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -298,7 +298,7 @@ func TestCreateFullOptionsLabels(t *testing.T) {
}

bndIPs := "127.0.0.1"
testHostIP := "1.2.3.4"
testHostIPv4 := "1.2.3.4"
nwV6s := "2001:db8:2600:2700:2800::/80"
gwV6s := "2001:db8:2600:2700:2800::25/80"
nwV6, _ := types.ParseCIDR(nwV6s)
Expand All @@ -310,7 +310,7 @@ func TestCreateFullOptionsLabels(t *testing.T) {
EnableICC: "true",
EnableIPMasquerade: "true",
DefaultBindingIP: bndIPs,
netlabel.HostIP: testHostIP,
netlabel.HostIPv4: testHostIPv4,
}

netOption := make(map[string]interface{})
Expand Down Expand Up @@ -358,9 +358,9 @@ func TestCreateFullOptionsLabels(t *testing.T) {
t.Fatalf("Unexpected: %v", nw.config.DefaultBindingIP)
}

hostIP := net.ParseIP(testHostIP)
if !hostIP.Equal(nw.config.HostIP) {
t.Fatalf("Unexpected: %v", nw.config.HostIP)
hostIP := net.ParseIP(testHostIPv4)
if !hostIP.Equal(nw.config.HostIPv4) {
t.Fatalf("Unexpected: %v", nw.config.HostIPv4)
}

if !types.CompareIPNet(nw.config.AddressIPv6, nwV6) {
Expand Down
6 changes: 4 additions & 2 deletions libnetwork/drivers/bridge/bridge_store.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,8 @@ func (ncfg *networkConfiguration) MarshalJSON() ([]byte, error) {
nMap["Internal"] = ncfg.Internal
nMap["DefaultBridge"] = ncfg.DefaultBridge
nMap["DefaultBindingIP"] = ncfg.DefaultBindingIP.String()
nMap["HostIP"] = ncfg.HostIP.String()
// This key is "HostIP" instead of "HostIPv4" to preserve compatibility with the on-disk format.
nMap["HostIP"] = ncfg.HostIPv4.String()
nMap["DefaultGatewayIPv4"] = ncfg.DefaultGatewayIPv4.String()
nMap["DefaultGatewayIPv6"] = ncfg.DefaultGatewayIPv6.String()
nMap["ContainerIfacePrefix"] = ncfg.ContainerIfacePrefix
Expand Down Expand Up @@ -189,8 +190,9 @@ func (ncfg *networkConfiguration) UnmarshalJSON(b []byte) error {
ncfg.ContainerIfacePrefix = v.(string)
}

// This key is "HostIP" instead of "HostIPv4" to preserve compatibility with the on-disk format.
if v, ok := nMap["HostIP"]; ok {
ncfg.HostIP = net.ParseIP(v.(string))
ncfg.HostIPv4 = net.ParseIP(v.(string))
}

ncfg.DefaultBridge = nMap["DefaultBridge"].(bool)
Expand Down
176 changes: 87 additions & 89 deletions libnetwork/drivers/bridge/setup_ip_tables_linux.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -157,15 +157,11 @@ func (n *bridgeNetwork) setupIPTables(ipVersion iptables.IPVersion, maskedAddr *
return setupInternalNetworkRules(config.BridgeName, maskedAddr, config.EnableICC, false)
})
} else {
hostIP := config.HostIP
if ipVersion != iptables.IPv4 {
hostIP = nil
}
if err = setupIPTablesInternal(hostIP, config.BridgeName, maskedAddr, config.EnableICC, config.EnableIPMasquerade, hairpinMode, true); err != nil {
if err = setupIPTablesInternal(ipVersion, config, maskedAddr, hairpinMode, true); err != nil {
return fmt.Errorf("Failed to Setup IP tables: %s", err.Error())
}
n.registerIptCleanFunc(func() error {
return setupIPTablesInternal(hostIP, config.BridgeName, maskedAddr, config.EnableICC, config.EnableIPMasquerade, hairpinMode, false)
return setupIPTablesInternal(ipVersion, config, maskedAddr, hairpinMode, false)
})
natChain, filterChain, _, _, err := n.getDriverChains(ipVersion)
if err != nil {
Expand Down Expand Up @@ -200,140 +196,138 @@ func (n *bridgeNetwork) setupIPTables(ipVersion iptables.IPVersion, maskedAddr *
}

type iptRule struct {
table iptables.Table
chain string
preArgs []string
args []string
ipv iptables.IPVersion
table iptables.Table
chain string
args []string
}

// Exists returns true if the rule exists in the kernel.
func (r iptRule) Exists() bool {
return iptables.GetIptable(r.ipv).Exists(r.table, r.chain, r.args...)
}

func (r iptRule) cmdArgs(op iptables.Action) []string {
return append([]string{"-t", string(r.table), string(op), r.chain}, r.args...)
}

func (r iptRule) exec(op iptables.Action) error {
return iptables.GetIptable(r.ipv).RawCombinedOutput(r.cmdArgs(op)...)
}

// Append appends the rule to the end of the chain. If the rule already exists anywhere in the
// chain, this is a no-op.
func (r iptRule) Append() error {
if r.Exists() {
return nil
}
return r.exec(iptables.Append)
}

func setupIPTablesInternal(hostIP net.IP, bridgeIface string, addr *net.IPNet, icc, ipmasq, hairpin, enable bool) error {
// Insert inserts the rule at the head of the chain. If the rule already exists anywhere in the
// chain, this is a no-op.
func (r iptRule) Insert() error {
if r.Exists() {
return nil
}
return r.exec(iptables.Insert)
}

// Delete deletes the rule from the kernel. If the rule does not exist, this is a no-op.
func (r iptRule) Delete() error {
if !r.Exists() {
return nil
}
return r.exec(iptables.Delete)
}

func setupIPTablesInternal(ipVer iptables.IPVersion, config *networkConfiguration, addr *net.IPNet, hairpin, enable bool) error {
var (
address = addr.String()
skipDNAT = iptRule{table: iptables.Nat, chain: DockerChain, preArgs: []string{"-t", "nat"}, args: []string{"-i", bridgeIface, "-j", "RETURN"}}
outRule = iptRule{table: iptables.Filter, chain: "FORWARD", args: []string{"-i", bridgeIface, "!", "-o", bridgeIface, "-j", "ACCEPT"}}
skipDNAT = iptRule{ipv: ipVer, table: iptables.Nat, chain: DockerChain, args: []string{"-i", config.BridgeName, "-j", "RETURN"}}
outRule = iptRule{ipv: ipVer, table: iptables.Filter, chain: "FORWARD", args: []string{"-i", config.BridgeName, "!", "-o", config.BridgeName, "-j", "ACCEPT"}}
natArgs []string
hpNatArgs []string
)
// if hostIP is set use this address as the src-ip during SNAT
if hostIP != nil {
hostAddr := hostIP.String()
natArgs = []string{"-s", address, "!", "-o", bridgeIface, "-j", "SNAT", "--to-source", hostAddr}
hpNatArgs = []string{"-m", "addrtype", "--src-type", "LOCAL", "-o", bridgeIface, "-j", "SNAT", "--to-source", hostAddr}
// If config.HostIPv4 is set, the user wants IPv4 SNAT with the given address.
if config.HostIPv4 != nil && ipVer == iptables.IPv4 {
hostAddr := config.HostIPv4.String()
natArgs = []string{"-s", address, "!", "-o", config.BridgeName, "-j", "SNAT", "--to-source", hostAddr}
hpNatArgs = []string{"-m", "addrtype", "--src-type", "LOCAL", "-o", config.BridgeName, "-j", "SNAT", "--to-source", hostAddr}
// Else use MASQUERADE which picks the src-ip based on NH from the route table
} else {
natArgs = []string{"-s", address, "!", "-o", bridgeIface, "-j", "MASQUERADE"}
hpNatArgs = []string{"-m", "addrtype", "--src-type", "LOCAL", "-o", bridgeIface, "-j", "MASQUERADE"}
natArgs = []string{"-s", address, "!", "-o", config.BridgeName, "-j", "MASQUERADE"}
hpNatArgs = []string{"-m", "addrtype", "--src-type", "LOCAL", "-o", config.BridgeName, "-j", "MASQUERADE"}
}

natRule := iptRule{table: iptables.Nat, chain: "POSTROUTING", preArgs: []string{"-t", "nat"}, args: natArgs}
hpNatRule := iptRule{table: iptables.Nat, chain: "POSTROUTING", preArgs: []string{"-t", "nat"}, args: hpNatArgs}

ipVer := iptables.IPv4
if addr.IP.To4() == nil {
ipVer = iptables.IPv6
}
natRule := iptRule{ipv: ipVer, table: iptables.Nat, chain: "POSTROUTING", args: natArgs}
hpNatRule := iptRule{ipv: ipVer, table: iptables.Nat, chain: "POSTROUTING", args: hpNatArgs}

// Set NAT.
if ipmasq {
if err := programChainRule(ipVer, natRule, "NAT", enable); err != nil {
if config.EnableIPMasquerade {
if err := programChainRule(natRule, "NAT", enable); err != nil {
return err
}
}

if ipmasq && !hairpin {
if err := programChainRule(ipVer, skipDNAT, "SKIP DNAT", enable); err != nil {
if config.EnableIPMasquerade && !hairpin {
if err := programChainRule(skipDNAT, "SKIP DNAT", enable); err != nil {
return err
}
}

// In hairpin mode, masquerade traffic from localhost. If hairpin is disabled or if we're tearing down
// that bridge, make sure the iptables rule isn't lying around.
if err := programChainRule(ipVer, hpNatRule, "MASQ LOCAL HOST", enable && hairpin); err != nil {
if err := programChainRule(hpNatRule, "MASQ LOCAL HOST", enable && hairpin); err != nil {
return err
}

// Set Inter Container Communication.
if err := setIcc(ipVer, bridgeIface, icc, enable); err != nil {
if err := setIcc(ipVer, config.BridgeName, config.EnableICC, enable); err != nil {
return err
}

// Set Accept on all non-intercontainer outgoing packets.
return programChainRule(ipVer, outRule, "ACCEPT NON_ICC OUTGOING", enable)
return programChainRule(outRule, "ACCEPT NON_ICC OUTGOING", enable)
}

func programChainRule(version iptables.IPVersion, rule iptRule, ruleDescr string, insert bool) error {
iptable := iptables.GetIptable(version)

var (
prefix []string
operation string
condition bool
doesExist = iptable.Exists(rule.table, rule.chain, rule.args...)
)

func programChainRule(rule iptRule, ruleDescr string, insert bool) error {
operation := "disable"
fn := rule.Delete
if insert {
condition = !doesExist
prefix = []string{"-I", rule.chain}
operation = "enable"
} else {
condition = doesExist
prefix = []string{"-D", rule.chain}
operation = "disable"
}
if rule.preArgs != nil {
prefix = append(rule.preArgs, prefix...)
fn = rule.Insert
}

if condition {
if err := iptable.RawCombinedOutput(append(prefix, rule.args...)...); err != nil {
return fmt.Errorf("Unable to %s %s rule: %s", operation, ruleDescr, err.Error())
}
if err := fn(); err != nil {
return fmt.Errorf("Unable to %s %s rule: %s", operation, ruleDescr, err.Error())
}

return nil
}

func setIcc(version iptables.IPVersion, bridgeIface string, iccEnable, insert bool) error {
iptable := iptables.GetIptable(version)
var (
table = iptables.Filter
chain = "FORWARD"
args = []string{"-i", bridgeIface, "-o", bridgeIface, "-j"}
acceptArgs = append(args, "ACCEPT")
dropArgs = append(args, "DROP")
)

args := []string{"-i", bridgeIface, "-o", bridgeIface, "-j"}
acceptRule := iptRule{ipv: version, table: iptables.Filter, chain: "FORWARD", args: append(args, "ACCEPT")}
dropRule := iptRule{ipv: version, table: iptables.Filter, chain: "FORWARD", args: append(args, "DROP")}
if insert {
if !iccEnable {
iptable.Raw(append([]string{"-D", chain}, acceptArgs...)...)

if !iptable.Exists(table, chain, dropArgs...) {
if err := iptable.RawCombinedOutput(append([]string{"-A", chain}, dropArgs...)...); err != nil {
return fmt.Errorf("Unable to prevent intercontainer communication: %s", err.Error())
}
acceptRule.Delete()
if err := dropRule.Append(); err != nil {
return fmt.Errorf("Unable to prevent intercontainer communication: %s", err.Error())
}
} else {
iptable.Raw(append([]string{"-D", chain}, dropArgs...)...)

if !iptable.Exists(table, chain, acceptArgs...) {
if err := iptable.RawCombinedOutput(append([]string{"-I", chain}, acceptArgs...)...); err != nil {
return fmt.Errorf("Unable to allow intercontainer communication: %s", err.Error())
}
dropRule.Delete()
if err := acceptRule.Insert(); err != nil {
return fmt.Errorf("Unable to allow intercontainer communication: %s", err.Error())
}
}
} else {
// Remove any ICC rule.
if !iccEnable {
if iptable.Exists(table, chain, dropArgs...) {
iptable.Raw(append([]string{"-D", chain}, dropArgs...)...)
}
dropRule.Delete()
} else {
if iptable.Exists(table, chain, acceptArgs...) {
iptable.Raw(append([]string{"-D", chain}, acceptArgs...)...)
}
acceptRule.Delete()
}
}

return nil
}

Expand Down Expand Up @@ -404,33 +398,37 @@ func setupInternalNetworkRules(bridgeIface string, addr *net.IPNet, icc, insert
if addr.IP.To4() != nil {
version = iptables.IPv4
inDropRule = iptRule{
ipv: version,
table: iptables.Filter,
chain: IsolationChain1,
args: []string{"-i", bridgeIface, "!", "-d", addr.String(), "-j", "DROP"},
}
outDropRule = iptRule{
ipv: version,
table: iptables.Filter,
chain: IsolationChain1,
args: []string{"-o", bridgeIface, "!", "-s", addr.String(), "-j", "DROP"},
}
} else {
version = iptables.IPv6
inDropRule = iptRule{
ipv: version,
table: iptables.Filter,
chain: IsolationChain1,
args: []string{"-i", bridgeIface, "!", "-o", bridgeIface, "!", "-d", addr.String(), "-j", "DROP"},
}
outDropRule = iptRule{
ipv: version,
table: iptables.Filter,
chain: IsolationChain1,
args: []string{"!", "-i", bridgeIface, "-o", bridgeIface, "!", "-s", addr.String(), "-j", "DROP"},
}
}

if err := programChainRule(version, inDropRule, "DROP INCOMING", insert); err != nil {
if err := programChainRule(inDropRule, "DROP INCOMING", insert); err != nil {
return err
}
if err := programChainRule(version, outDropRule, "DROP OUTGOING", insert); err != nil {
if err := programChainRule(outDropRule, "DROP OUTGOING", insert); err != nil {
return err
}

Expand Down
Loading

0 comments on commit af22957

Please sign in to comment.