Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix compilation errors and test and add random-fully to the masquerade rule #108

Merged
merged 1 commit into from
May 17, 2023
Merged

Fix compilation errors and test and add random-fully to the masquerade rule #108

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions cmd/ip-masq-agent/ip-masq-agent.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,10 @@ import (

utilyaml "k8s.io/apimachinery/pkg/util/yaml"
"k8s.io/component-base/logs"
"k8s.io/component-base/version/verflag"
"k8s.io/ip-masq-agent/cmd/ip-masq-agent/testing/fakefs"
"k8s.io/ip-masq-agent/pkg/version"
utiliptables "k8s.io/kubernetes/pkg/util/iptables"
"k8s.io/component-base/version/verflag"
utilexec "k8s.io/utils/exec"

"github.com/golang/glog"
Expand Down Expand Up @@ -351,8 +351,10 @@ func (m *MasqDaemon) syncMasqRulesIPv6() error {
// Feel free to dig around in iptables and see if you can figure out exactly why; I haven't had time to fully trace how it parses and handle subcommands.
// If you want to investigate, get the source via `git clone git://git.netfilter.org/iptables.git`, `git checkout v1.4.21` (the version I've seen this issue on,
// though it may also happen on others), and start with `git grep XT_EXTENSION_MAXNAMELEN`.
const postRoutingMasqChainCommentFormat = "\"ip-masq-agent: ensure nat POSTROUTING directs all non-LOCAL destination traffic to our custom %s chain\""

func postroutingJumpComment() string {
return fmt.Sprintf("ip-masq-agent: ensure nat POSTROUTING directs all non-LOCAL destination traffic to our custom %s chain", masqChain)
return fmt.Sprintf(postRoutingMasqChainCommentFormat, masqChain)
}

func (m *MasqDaemon) ensurePostroutingJump() error {
Expand Down Expand Up @@ -382,7 +384,7 @@ func writeNonMasqRule(lines *bytes.Buffer, cidr string) {
const masqRuleComment = `-m comment --comment "ip-masq-agent: outbound traffic is subject to MASQUERADE (must be last in chain)"`

func writeMasqRule(lines *bytes.Buffer) {
writeRule(lines, utiliptables.Append, masqChain, masqRuleComment, "-j", "MASQUERADE")
writeRule(lines, utiliptables.Append, masqChain, masqRuleComment, "-j", "MASQUERADE", "--random-fully")
}

// Similar syntax to utiliptables.Interface.EnsureRule, except you don't pass a table
Expand Down
77 changes: 64 additions & 13 deletions cmd/ip-masq-agent/ip-masq-agent_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,16 +33,40 @@ import (
// turn off glog logging during tests to avoid clutter in output
func TestMain(m *testing.M) {
flag.Set("logtostderr", "false")
flag.Set("masq-chain", "IP-MASQ-AGENT")
ec := m.Run()
os.Exit(ec)
}

// returns a MasqDaemon with empty config values and a fake iptables interface
func NewFakeMasqDaemon() *MasqDaemon {
masqChain = "IP-MASQ-AGENT"
iptables := iptest.NewFake()
iptables.Dump = &iptest.IPTablesDump{
Tables: []iptest.Table{
{
Name: utiliptables.TableNAT,
MrHohn marked this conversation as resolved.
Show resolved Hide resolved
Chains: []iptest.Chain{
{Name: utiliptables.ChainPostrouting},
},
},
},
}
ip6tables := iptest.NewIPv6Fake()
ip6tables.Dump = &iptest.IPTablesDump{
Tables: []iptest.Table{
{
Name: utiliptables.TableNAT,
Chains: []iptest.Chain{
{Name: utiliptables.ChainPostrouting},
},
},
},
}
return &MasqDaemon{
config: &MasqConfig{},
iptables: iptest.NewFake(),
ip6tables: iptest.NewFake(),
iptables: iptables,
ip6tables: ip6tables,
}
}

Expand Down Expand Up @@ -242,6 +266,7 @@ func TestSyncConfig(t *testing.T) {

// tests MasqDaemon.syncMasqRules
func TestSyncMasqRules(t *testing.T) {
masqChain = utiliptables.Chain("IP-MASQ-AGENT")
var syncMasqRulesTests = []struct {
desc string // human readable description of the test
cfg *MasqConfig // Masq configuration to use
Expand All @@ -252,30 +277,39 @@ func TestSyncMasqRules(t *testing.T) {
desc: "empty config",
cfg: &MasqConfig{},
want: `*nat
:` + string(utiliptables.ChainPostrouting) + ` - [0:0]
:` + string(masqChain) + ` - [0:0]
-A ` + string(utiliptables.ChainPostrouting) + ` -m comment --comment ` +
fmt.Sprintf(postRoutingMasqChainCommentFormat, masqChain) + ` -m addrtype ! --dst-type LOCAL -j ` + string(masqChain) + `
-A ` + string(masqChain) + ` ` + nonMasqRuleComment + ` -d 169.254.0.0/16 -j RETURN
-A ` + string(masqChain) + ` ` + masqRuleComment + ` -j MASQUERADE
-A ` + string(masqChain) + ` ` + masqRuleComment + ` -j MASQUERADE --random-fully
COMMIT
`,
},
{
desc: "default config masquerading reserved ranges",
cfg: NewMasqConfigNoReservedRanges(),
want: `*nat
:` + string(utiliptables.ChainPostrouting) + ` - [0:0]
:` + string(masqChain) + ` - [0:0]
-A ` + string(utiliptables.ChainPostrouting) + ` -m comment --comment ` +
fmt.Sprintf(postRoutingMasqChainCommentFormat, masqChain) + ` -m addrtype ! --dst-type LOCAL -j ` + string(masqChain) + `
-A ` + string(masqChain) + ` ` + nonMasqRuleComment + ` -d 169.254.0.0/16 -j RETURN
-A ` + string(masqChain) + ` ` + nonMasqRuleComment + ` -d 10.0.0.0/8 -j RETURN
-A ` + string(masqChain) + ` ` + nonMasqRuleComment + ` -d 172.16.0.0/12 -j RETURN
-A ` + string(masqChain) + ` ` + nonMasqRuleComment + ` -d 192.168.0.0/16 -j RETURN
-A ` + string(masqChain) + ` ` + masqRuleComment + ` -j MASQUERADE
-A ` + string(masqChain) + ` ` + masqRuleComment + ` -j MASQUERADE --random-fully
COMMIT
`,
},
{
desc: "default config not masquerading reserved ranges",
cfg: NewMasqConfigWithReservedRanges(),
want: `*nat
:` + string(utiliptables.ChainPostrouting) + ` - [0:0]
:` + string(masqChain) + ` - [0:0]
-A ` + string(utiliptables.ChainPostrouting) + ` -m comment --comment ` +
fmt.Sprintf(postRoutingMasqChainCommentFormat, masqChain) + ` -m addrtype ! --dst-type LOCAL -j ` + string(masqChain) + `
-A ` + string(masqChain) + ` ` + nonMasqRuleComment + ` -d 169.254.0.0/16 -j RETURN
-A ` + string(masqChain) + ` ` + nonMasqRuleComment + ` -d 10.0.0.0/8 -j RETURN
-A ` + string(masqChain) + ` ` + nonMasqRuleComment + ` -d 172.16.0.0/12 -j RETURN
Expand All @@ -288,7 +322,7 @@ COMMIT
-A ` + string(masqChain) + ` ` + nonMasqRuleComment + ` -d 198.51.100.0/24 -j RETURN
-A ` + string(masqChain) + ` ` + nonMasqRuleComment + ` -d 203.0.113.0/24 -j RETURN
-A ` + string(masqChain) + ` ` + nonMasqRuleComment + ` -d 240.0.0.0/4 -j RETURN
-A ` + string(masqChain) + ` ` + masqRuleComment + ` -j MASQUERADE
-A ` + string(masqChain) + ` ` + masqRuleComment + ` -j MASQUERADE --random-fully
COMMIT
`,
},
Expand All @@ -301,10 +335,13 @@ COMMIT
},
},
want: `*nat
:` + string(utiliptables.ChainPostrouting) + ` - [0:0]
:` + string(masqChain) + ` - [0:0]
-A ` + string(utiliptables.ChainPostrouting) + ` -m comment --comment ` +
fmt.Sprintf(postRoutingMasqChainCommentFormat, masqChain) + ` -m addrtype ! --dst-type LOCAL -j ` + string(masqChain) + `
-A ` + string(masqChain) + ` ` + nonMasqRuleComment + ` -d 169.254.0.0/16 -j RETURN
-A ` + string(masqChain) + ` ` + nonMasqRuleComment + ` -d 10.244.0.0/16 -j RETURN
-A ` + string(masqChain) + ` ` + masqRuleComment + ` -j MASQUERADE
-A ` + string(masqChain) + ` ` + masqRuleComment + ` -j MASQUERADE --random-fully
COMMIT
`,
},
Expand All @@ -319,15 +356,18 @@ COMMIT
if !ok {
t.Errorf("MasqDaemon wasn't using the expected iptables mock")
}
if string(fipt.Lines) != tt.want {
t.Errorf("syncMasqRules wrote %q, want %q", string(fipt.Lines), tt.want)
buf := bytes.NewBuffer(nil)
fipt.SaveInto("nat", buf)
if string(buf.Bytes()) != string(tt.want) {
t.Errorf("syncMasqRules wrote %q, want %q", string(buf.Bytes()), tt.want)
}
})
}
}

// tests MasqDaemon.syncMasqRulesIPv6
func TestSyncMasqRulesIPv6(t *testing.T) {
masqChain = utiliptables.Chain("IP-MASQ-AGENT")
var syncMasqRulesIPv6Tests = []struct {
desc string // human readable description of the test
cfg *MasqConfig // Masq configuration to use
Expand All @@ -338,9 +378,12 @@ func TestSyncMasqRulesIPv6(t *testing.T) {
desc: "empty config",
cfg: &MasqConfig{},
want: `*nat
:` + string(utiliptables.ChainPostrouting) + ` - [0:0]
:` + string(masqChain) + ` - [0:0]
-A ` + string(utiliptables.ChainPostrouting) + ` -m comment --comment ` +
fmt.Sprintf(postRoutingMasqChainCommentFormat, masqChain) + ` -m addrtype ! --dst-type LOCAL -j ` + string(masqChain) + `
-A ` + string(masqChain) + ` ` + nonMasqRuleComment + ` -d fe80::/10 -j RETURN
-A ` + string(masqChain) + ` ` + masqRuleComment + ` -j MASQUERADE
-A ` + string(masqChain) + ` ` + masqRuleComment + ` -j MASQUERADE --random-fully
COMMIT
`,
},
Expand All @@ -353,19 +396,25 @@ COMMIT
},
},
want: `*nat
:` + string(utiliptables.ChainPostrouting) + ` - [0:0]
:` + string(masqChain) + ` - [0:0]
-A ` + string(utiliptables.ChainPostrouting) + ` -m comment --comment ` +
fmt.Sprintf(postRoutingMasqChainCommentFormat, masqChain) + ` -m addrtype ! --dst-type LOCAL -j ` + string(masqChain) + `
-A ` + string(masqChain) + ` ` + nonMasqRuleComment + ` -d fe80::/10 -j RETURN
-A ` + string(masqChain) + ` ` + nonMasqRuleComment + ` -d fc00::/7 -j RETURN
-A ` + string(masqChain) + ` ` + masqRuleComment + ` -j MASQUERADE
-A ` + string(masqChain) + ` ` + masqRuleComment + ` -j MASQUERADE --random-fully
COMMIT
`,
},
{
desc: "config has masqLinkLocalIPv6: true",
cfg: &MasqConfig{MasqLinkLocalIPv6: true},
want: `*nat
:` + string(utiliptables.ChainPostrouting) + ` - [0:0]
:` + string(masqChain) + ` - [0:0]
-A ` + string(masqChain) + ` ` + masqRuleComment + ` -j MASQUERADE
-A ` + string(utiliptables.ChainPostrouting) + ` -m comment --comment ` +
fmt.Sprintf(postRoutingMasqChainCommentFormat, masqChain) + ` -m addrtype ! --dst-type LOCAL -j ` + string(masqChain) + `
-A ` + string(masqChain) + ` ` + masqRuleComment + ` -j MASQUERADE --random-fully
COMMIT
`,
},
Expand All @@ -381,8 +430,10 @@ COMMIT
if !ok {
t.Errorf("MasqDaemon wasn't using the expected iptables mock")
}
if string(fipt6.Lines) != tt.want {
t.Errorf("syncMasqRulesIPv6 wrote %q, want %q", string(fipt6.Lines), tt.want)
buf := bytes.NewBuffer(nil)
fipt6.SaveInto("nat", buf)
if string(buf.Bytes()) != tt.want {
t.Errorf("syncMasqRulesIPv6 wrote %q, want %q", string(buf.Bytes()), tt.want)
}
})
}
Expand Down