Skip to content

Commit

Permalink
Update rp_filter only when /proc is accessible
Browse files Browse the repository at this point in the history
  • Loading branch information
@SaranBalaji90
SaranBalaji90 committed Apr 6, 2020
1 parent 050e6ec commit 41acae5
Show file tree
Hide file tree
Showing 4 changed files with 89 additions and 64 deletions.
14 changes: 10 additions & 4 deletions pkg/networkutils/network.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -243,10 +243,16 @@ func (n *linuxNetwork) SetupHostNetwork(vpcCIDR *net.IPNet, vpcCIDRs []*string,
primaryIntfRPFilter := "net/ipv4/conf/" + primaryIntf + "/rp_filter"
const rpFilterLoose = "2"

log.Debugf("Setting RPF for primary interface: %s", primaryIntfRPFilter)
err = n.procSys.Set(primaryIntfRPFilter, rpFilterLoose)
if err != nil {
return errors.Wrapf(err, "failed to configure %s RPF check", primaryIntf)
if n.procSys.IsPathWriteAccessible(primaryIntfRPFilter) {
// Setting RPF will be removed from aws-node when we bump our major version.
log.Debugf("Setting RPF for primary interface: %s", primaryIntfRPFilter)
err = n.procSys.Set(primaryIntfRPFilter, rpFilterLoose)
if err != nil {
return errors.Wrapf(err, "failed to configure %s RPF check", primaryIntf)
}
} else {
// if aws-node run as un-privileged pods then /proc will be mounted as read only.
log.Infof("Skip updating RPF for primary interface: %s", primaryIntfRPFilter)
}
}

Expand Down
117 changes: 57 additions & 60 deletions pkg/networkutils/network_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -285,18 +285,8 @@ func TestSetupHostNetworkNodePortEnabled(t *testing.T) {
procSys: mockProcSys,
}

mockPrimaryInterfaceLookup(ctrl, mockNetLink)
mockNetLink.EXPECT().LinkSetMTU(gomock.Any(), testMTU).Return(nil)

var hostRule netlink.Rule
mockNetLink.EXPECT().NewRule().Return(&hostRule)
mockNetLink.EXPECT().RuleDel(&hostRule)
var mainENIRule netlink.Rule
mockNetLink.EXPECT().NewRule().Return(&mainENIRule)
mockNetLink.EXPECT().RuleDel(&mainENIRule)
mockNetLink.EXPECT().RuleAdd(&mainENIRule)

mockProcSys.EXPECT().Set("net/ipv4/conf/lo/rp_filter", "2").Return(nil)
setupNetLinkMocks(ctrl, mockNetLink)
setupProcSysMocks(mockProcSys, true)

var vpcCIDRs []*string

Expand Down Expand Up @@ -363,18 +353,8 @@ func TestSetupHostNetworkWithExcludeSNATCIDRs(t *testing.T) {
procSys: mockProcSys,
}

mockPrimaryInterfaceLookup(ctrl, mockNetLink)

mockNetLink.EXPECT().LinkSetMTU(gomock.Any(), testMTU).Return(nil)
var hostRule netlink.Rule
mockNetLink.EXPECT().NewRule().Return(&hostRule)
mockNetLink.EXPECT().RuleDel(&hostRule)
var mainENIRule netlink.Rule
mockNetLink.EXPECT().NewRule().Return(&mainENIRule)
mockNetLink.EXPECT().RuleDel(&mainENIRule)
mockNetLink.EXPECT().RuleAdd(&mainENIRule)

mockProcSys.EXPECT().Set("net/ipv4/conf/lo/rp_filter", "2").Return(nil)
setupNetLinkMocks(ctrl, mockNetLink)
setupProcSysMocks(mockProcSys, true)

var vpcCIDRs []*string
vpcCIDRs = []*string{aws.String("10.10.0.0/16"), aws.String("10.11.0.0/16")}
Expand Down Expand Up @@ -416,18 +396,8 @@ func TestSetupHostNetworkCleansUpStaleSNATRules(t *testing.T) {
},
procSys: mockProcSys,
}
mockPrimaryInterfaceLookup(ctrl, mockNetLink)

mockNetLink.EXPECT().LinkSetMTU(gomock.Any(), testMTU).Return(nil)
var hostRule netlink.Rule
mockNetLink.EXPECT().NewRule().Return(&hostRule)
mockNetLink.EXPECT().RuleDel(&hostRule)
var mainENIRule netlink.Rule
mockNetLink.EXPECT().NewRule().Return(&mainENIRule)
mockNetLink.EXPECT().RuleDel(&mainENIRule)
mockNetLink.EXPECT().RuleAdd(&mainENIRule)

mockProcSys.EXPECT().Set("net/ipv4/conf/lo/rp_filter", "2").Return(nil)
setupNetLinkMocks(ctrl, mockNetLink)
setupProcSysMocks(mockProcSys, true)

vpcCIDRs := []*string{aws.String("10.10.0.0/16"), aws.String("10.11.0.0/16")}
_ = mockIptables.Append("nat", "AWS-SNAT-CHAIN-0", "!", "-d", "10.10.0.0/16", "-m", "comment", "--comment", "AWS SNAT CHAN", "-j", "AWS-SNAT-CHAIN-1") //AWS SNAT CHAN proves backwards compatibility
Expand Down Expand Up @@ -477,18 +447,8 @@ func TestSetupHostNetworkExcludedSNATCIDRsIdempotent(t *testing.T) {
},
procSys: mockProcSys,
}
mockPrimaryInterfaceLookup(ctrl, mockNetLink)

mockNetLink.EXPECT().LinkSetMTU(gomock.Any(), testMTU).Return(nil)
var hostRule netlink.Rule
mockNetLink.EXPECT().NewRule().Return(&hostRule)
mockNetLink.EXPECT().RuleDel(&hostRule)
var mainENIRule netlink.Rule
mockNetLink.EXPECT().NewRule().Return(&mainENIRule)
mockNetLink.EXPECT().RuleDel(&mainENIRule)
mockNetLink.EXPECT().RuleAdd(&mainENIRule)

mockProcSys.EXPECT().Set("net/ipv4/conf/lo/rp_filter", "2").Return(nil)
setupNetLinkMocks(ctrl, mockNetLink)
setupProcSysMocks(mockProcSys, true)

_ = mockIptables.Append("nat", "AWS-SNAT-CHAIN-0", "!", "-d", "10.10.0.0/16", "-m", "comment", "--comment", "AWS SNAT CHAIN", "-j", "AWS-SNAT-CHAIN-1")
_ = mockIptables.Append("nat", "AWS-SNAT-CHAIN-1", "!", "-d", "10.11.0.0/16", "-m", "comment", "--comment", "AWS SNAT CHAIN", "-j", "AWS-SNAT-CHAIN-2")
Expand Down Expand Up @@ -537,18 +497,8 @@ func TestSetupHostNetworkMultipleCIDRs(t *testing.T) {
},
procSys: mockProcSys,
}
mockPrimaryInterfaceLookup(ctrl, mockNetLink)

mockNetLink.EXPECT().LinkSetMTU(gomock.Any(), testMTU).Return(nil)
var hostRule netlink.Rule
mockNetLink.EXPECT().NewRule().Return(&hostRule)
mockNetLink.EXPECT().RuleDel(&hostRule)
var mainENIRule netlink.Rule
mockNetLink.EXPECT().NewRule().Return(&mainENIRule)
mockNetLink.EXPECT().RuleDel(&mainENIRule)
mockNetLink.EXPECT().RuleAdd(&mainENIRule)

mockProcSys.EXPECT().Set("net/ipv4/conf/lo/rp_filter", "2").Return(nil)
setupNetLinkMocks(ctrl, mockNetLink)
setupProcSysMocks(mockProcSys, true)

var vpcCIDRs []*string
vpcCIDRs = []*string{aws.String("10.10.0.0/16"), aws.String("10.11.0.0/16")}
Expand Down Expand Up @@ -582,6 +532,53 @@ func TestIncrementIPv4Addr(t *testing.T) {
}
}

func TestSetupHostNetworkForUnprivilegedPod(t *testing.T) {
ctrl, mockNetLink, _, mockNS, mockIptables, mockProcSys := setup(t)
defer ctrl.Finish()

ln := &linuxNetwork{
useExternalSNAT: true,
nodePortSupportEnabled: true,
mainENIMark: defaultConnmark,
mtu: testMTU,

netLink: mockNetLink,
ns: mockNS,
newIptables: func() (iptablesIface, error) {
return mockIptables, nil
},
procSys: mockProcSys,
}
var vpcCIDRs []*string
setupNetLinkMocks(ctrl, mockNetLink)
setupProcSysMocks(mockProcSys, false)

err := ln.SetupHostNetwork(testENINetIPNet, vpcCIDRs, loopback, &testENINetIP)
assert.NoError(t, err)
}

func setupProcSysMocks(mockProcSys *mock_procsyswrapper.MockProcSys, shouldHaveWriteAccess bool) {
rpFilterPath := "net/ipv4/conf/lo/rp_filter"

mockProcSys.EXPECT().IsPathWriteAccessible(rpFilterPath).Return(shouldHaveWriteAccess)
if shouldHaveWriteAccess {
mockProcSys.EXPECT().Set(rpFilterPath, "2").Return(nil)
}
}

func setupNetLinkMocks(ctrl *gomock.Controller, mockNetLink *mock_netlinkwrapper.MockNetLink) {
mockPrimaryInterfaceLookup(ctrl, mockNetLink)
mockNetLink.EXPECT().LinkSetMTU(gomock.Any(), testMTU).Return(nil)

var hostRule netlink.Rule
mockNetLink.EXPECT().NewRule().Return(&hostRule)
mockNetLink.EXPECT().RuleDel(&hostRule)
var mainENIRule netlink.Rule
mockNetLink.EXPECT().NewRule().Return(&mainENIRule)
mockNetLink.EXPECT().RuleDel(&mainENIRule)
mockNetLink.EXPECT().RuleAdd(&mainENIRule)
}

type mockIptables struct {
// dataplaneState is a map from table name to chain name to slice of rulespecs
dataplaneState map[string]map[string][][]string
Expand Down
14 changes: 14 additions & 0 deletions pkg/procsyswrapper/mocks/procsys_mocks.go
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 8 additions & 0 deletions pkg/procsyswrapper/procsys.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,14 @@ package procsyswrapper

import (
"io/ioutil"

"golang.org/x/sys/unix"
)

type ProcSys interface {
Get(key string) (string, error)
Set(key, value string) error
IsPathWriteAccessible(key string) bool
}

type procSys struct {
Expand All @@ -42,3 +45,8 @@ func (p *procSys) Get(key string) (string, error) {
func (p *procSys) Set(key, value string) error {
return ioutil.WriteFile(p.path(key), []byte(value), 0644)
}

// IsPathWriteAccessible verifies if aws-node pod can write to the specified path
func (p *procSys) IsPathWriteAccessible(key string) bool {
return unix.Access(p.prefix + key, unix.W_OK) != nil
}

0 comments on commit 41acae5

Please sign in to comment.