diff --git a/pkg/sentry/socket/netfilter/netfilter.go b/pkg/sentry/socket/netfilter/netfilter.go index ef907e1a4..5f38ae091 100644 --- a/pkg/sentry/socket/netfilter/netfilter.go +++ b/pkg/sentry/socket/netfilter/netfilter.go @@ -289,7 +289,8 @@ func SetEntries(task *kernel.Task, stk *stack.Stack, optVal []byte, ipv6 bool) * // - There are no chains without an unconditional final rule. // - There are no chains without an unconditional underflow rule. - return syserr.TranslateNetstackError(stk.IPTables().ReplaceTable(nameToID[replace.Name.String()], table, ipv6)) + stk.IPTables().ReplaceTable(nameToID[replace.Name.String()], table, ipv6) + return nil } // parseMatchers parses 0 or more matchers from optVal. optVal should contain diff --git a/pkg/tcpip/network/ipv4/ipv4_test.go b/pkg/tcpip/network/ipv4/ipv4_test.go index 7b8fd0af5..001f16f5b 100644 --- a/pkg/tcpip/network/ipv4/ipv4_test.go +++ b/pkg/tcpip/network/ipv4/ipv4_test.go @@ -2774,9 +2774,7 @@ func TestWriteStats(t *testing.T) { filter := ipt.GetTable(stack.FilterID, false /* ipv6 */) ruleIdx := filter.BuiltinChains[stack.Output] filter.Rules[ruleIdx].Target = &stack.DropTarget{} - if err := ipt.ReplaceTable(stack.FilterID, filter, false /* ipv6 */); err != nil { - t.Fatalf("failed to replace table: %s", err) - } + ipt.ReplaceTable(stack.FilterID, filter, false /* ipv6 */) }, allowPackets: math.MaxInt32, expectSent: 0, @@ -2790,9 +2788,7 @@ func TestWriteStats(t *testing.T) { filter := ipt.GetTable(stack.NATID, false /* ipv6 */) ruleIdx := filter.BuiltinChains[stack.Postrouting] filter.Rules[ruleIdx].Target = &stack.DropTarget{} - if err := ipt.ReplaceTable(stack.NATID, filter, false /* ipv6 */); err != nil { - t.Fatalf("failed to replace table: %s", err) - } + ipt.ReplaceTable(stack.NATID, filter, false /* ipv6 */) }, allowPackets: math.MaxInt32, expectSent: 0, @@ -2812,9 +2808,7 @@ func TestWriteStats(t *testing.T) { filter.Rules[ruleIdx].Matchers = []stack.Matcher{&limitedMatcher{nPackets - 1}} // Make sure the next rule is ACCEPT. filter.Rules[ruleIdx+1].Target = &stack.AcceptTarget{} - if err := ipt.ReplaceTable(stack.FilterID, filter, false /* ipv6 */); err != nil { - t.Fatalf("failed to replace table: %s", err) - } + ipt.ReplaceTable(stack.FilterID, filter, false /* ipv6 */) }, allowPackets: math.MaxInt32, expectSent: nPackets - 1, @@ -2834,9 +2828,7 @@ func TestWriteStats(t *testing.T) { filter.Rules[ruleIdx].Matchers = []stack.Matcher{&limitedMatcher{nPackets - 1}} // Make sure the next rule is ACCEPT. filter.Rules[ruleIdx+1].Target = &stack.AcceptTarget{} - if err := ipt.ReplaceTable(stack.NATID, filter, false /* ipv6 */); err != nil { - t.Fatalf("failed to replace table: %s", err) - } + ipt.ReplaceTable(stack.NATID, filter, false /* ipv6 */) }, allowPackets: math.MaxInt32, expectSent: nPackets - 1, @@ -3184,9 +3176,7 @@ func TestCloseLocking(t *testing.T) { stack.Postrouting: 3, }, } - if err := s.IPTables().ReplaceTable(stack.NATID, table, false /* ipv6 */); err != nil { - t.Fatalf("s.IPTables().ReplaceTable(...): %s", err) - } + s.IPTables().ReplaceTable(stack.NATID, table, false /* ipv6 */) e := channel.New(0, defaultMTU, "") if err := s.CreateNIC(nicID1, e); err != nil { diff --git a/pkg/tcpip/network/ipv6/ipv6_test.go b/pkg/tcpip/network/ipv6/ipv6_test.go index 17af81f9e..985cc0b7c 100644 --- a/pkg/tcpip/network/ipv6/ipv6_test.go +++ b/pkg/tcpip/network/ipv6/ipv6_test.go @@ -2530,9 +2530,7 @@ func TestWriteStats(t *testing.T) { filter := ipt.GetTable(stack.FilterID, true /* ipv6 */) ruleIdx := filter.BuiltinChains[stack.Output] filter.Rules[ruleIdx].Target = &stack.DropTarget{} - if err := ipt.ReplaceTable(stack.FilterID, filter, true /* ipv6 */); err != nil { - t.Fatalf("failed to replace table: %v", err) - } + ipt.ReplaceTable(stack.FilterID, filter, true /* ipv6 */) }, allowPackets: math.MaxInt32, expectSent: 0, @@ -2547,9 +2545,7 @@ func TestWriteStats(t *testing.T) { filter := ipt.GetTable(stack.NATID, true /* ipv6 */) ruleIdx := filter.BuiltinChains[stack.Postrouting] filter.Rules[ruleIdx].Target = &stack.DropTarget{} - if err := ipt.ReplaceTable(stack.NATID, filter, true /* ipv6 */); err != nil { - t.Fatalf("failed to replace table: %v", err) - } + ipt.ReplaceTable(stack.NATID, filter, true /* ipv6 */) }, allowPackets: math.MaxInt32, expectSent: 0, @@ -2569,9 +2565,7 @@ func TestWriteStats(t *testing.T) { filter.Rules[ruleIdx].Matchers = []stack.Matcher{&limitedMatcher{nPackets - 1}} // Make sure the next rule is ACCEPT. filter.Rules[ruleIdx+1].Target = &stack.AcceptTarget{} - if err := ipt.ReplaceTable(stack.FilterID, filter, true /* ipv6 */); err != nil { - t.Fatalf("failed to replace table: %v", err) - } + ipt.ReplaceTable(stack.FilterID, filter, true /* ipv6 */) }, allowPackets: math.MaxInt32, expectSent: nPackets - 1, @@ -2591,9 +2585,7 @@ func TestWriteStats(t *testing.T) { filter.Rules[ruleIdx].Matchers = []stack.Matcher{&limitedMatcher{nPackets - 1}} // Make sure the next rule is ACCEPT. filter.Rules[ruleIdx+1].Target = &stack.AcceptTarget{} - if err := ipt.ReplaceTable(stack.NATID, filter, true /* ipv6 */); err != nil { - t.Fatalf("failed to replace table: %v", err) - } + ipt.ReplaceTable(stack.NATID, filter, true /* ipv6 */) }, allowPackets: math.MaxInt32, expectSent: nPackets - 1, diff --git a/pkg/tcpip/stack/iptables.go b/pkg/tcpip/stack/iptables.go index da79f2bb5..a5f904970 100644 --- a/pkg/tcpip/stack/iptables.go +++ b/pkg/tcpip/stack/iptables.go @@ -231,7 +231,7 @@ func (it *IPTables) getTableRLocked(id TableID, ipv6 bool) Table { // ReplaceTable replaces or inserts table by name. It panics when an invalid id // is provided. -func (it *IPTables) ReplaceTable(id TableID, table Table, ipv6 bool) tcpip.Error { +func (it *IPTables) ReplaceTable(id TableID, table Table, ipv6 bool) { it.mu.Lock() defer it.mu.Unlock() // If iptables is being enabled, initialize the conntrack table and @@ -246,7 +246,6 @@ func (it *IPTables) ReplaceTable(id TableID, table Table, ipv6 bool) tcpip.Error } else { it.v4Tables[id] = table } - return nil } // A chainVerdict is what a table decides should be done with a packet. diff --git a/pkg/tcpip/stack/iptables_test.go b/pkg/tcpip/stack/iptables_test.go index 57948ad50..b85558513 100644 --- a/pkg/tcpip/stack/iptables_test.go +++ b/pkg/tcpip/stack/iptables_test.go @@ -115,9 +115,7 @@ func TestNATedConnectionReap(t *testing.T) { Postrouting: 5, }, } - if err := iptables.ReplaceTable(NATID, table, ipv6); err != nil { - t.Fatalf("ipt.ReplaceTable(%d, _, true): %s", NATID, err) - } + iptables.ReplaceTable(NATID, table, ipv6) // Stop the reaper if it is running so we can reap manually as it is started // on the first change to IPTables. @@ -299,9 +297,7 @@ func TestNATAlwaysPerformed(t *testing.T) { iptables := DefaultTables(clock, rand.New(rand.NewSource(0 /* seed */))) // Just to make sure the iptables is not short circuited. - if err := iptables.ReplaceTable(NATID, iptables.GetTable(NATID, ipv6), ipv6); err != nil { - t.Fatalf("ipt.ReplaceTable(%d, _, true): %s", NATID, err) - } + iptables.ReplaceTable(NATID, iptables.GetTable(NATID, ipv6), ipv6) pkt := v6PacketBuffer() @@ -414,9 +410,7 @@ func TestNATConflict(t *testing.T) { Postrouting: 5, }, } - if err := iptables.ReplaceTable(NATID, table, ipv6); err != nil { - t.Fatalf("ipt.ReplaceTable(%d, _, true): %s", NATID, err) - } + iptables.ReplaceTable(NATID, table, ipv6) // Create and finalize the connection. test.checkIPTables(t, iptables, v6PacketBufferWithSrcAddr(srcAddr), true /* lastHookOK */) diff --git a/pkg/tcpip/tests/integration/iptables_test.go b/pkg/tcpip/tests/integration/iptables_test.go index 49e1ef4af..dc480c11d 100644 --- a/pkg/tcpip/tests/integration/iptables_test.go +++ b/pkg/tcpip/tests/integration/iptables_test.go @@ -182,9 +182,7 @@ func TestIPTablesStatsForInput(t *testing.T) { filter.Rules[ruleIdx].Matchers = []stack.Matcher{&inputIfNameMatcher{nicName}} // Make sure the packet is not dropped by the next rule. filter.Rules[ruleIdx+1].Target = &stack.AcceptTarget{} - if err := ipt.ReplaceTable(stack.FilterID, filter, true /* ipv6 */); err != nil { - t.Fatalf("ipt.ReplaceTable(%d, _, %t): %s", stack.FilterID, true, err) - } + ipt.ReplaceTable(stack.FilterID, filter, true /* ipv6 */) }, genPacket: genPacketV6, proto: header.IPv6ProtocolNumber, @@ -203,9 +201,7 @@ func TestIPTablesStatsForInput(t *testing.T) { filter.Rules[ruleIdx].Target = &stack.DropTarget{} filter.Rules[ruleIdx].Matchers = []stack.Matcher{&inputIfNameMatcher{nicName}} filter.Rules[ruleIdx+1].Target = &stack.AcceptTarget{} - if err := ipt.ReplaceTable(stack.FilterID, filter, false /* ipv6 */); err != nil { - t.Fatalf("ipt.ReplaceTable(%d, _, %t): %s", stack.FilterID, false, err) - } + ipt.ReplaceTable(stack.FilterID, filter, false /* ipv6 */) }, genPacket: genPacketV4, proto: header.IPv4ProtocolNumber, @@ -223,9 +219,7 @@ func TestIPTablesStatsForInput(t *testing.T) { filter.Rules[ruleIdx].Filter = stack.IPHeaderFilter{InputInterface: anotherNicName} filter.Rules[ruleIdx].Target = &stack.DropTarget{} filter.Rules[ruleIdx+1].Target = &stack.AcceptTarget{} - if err := ipt.ReplaceTable(stack.FilterID, filter, true /* ipv6 */); err != nil { - t.Fatalf("ipt.ReplaceTable(%d, _, %t): %s", stack.FilterID, true, err) - } + ipt.ReplaceTable(stack.FilterID, filter, true /* ipv6 */) }, genPacket: genPacketV6, proto: header.IPv6ProtocolNumber, @@ -243,9 +237,7 @@ func TestIPTablesStatsForInput(t *testing.T) { filter.Rules[ruleIdx].Filter = stack.IPHeaderFilter{InputInterface: anotherNicName} filter.Rules[ruleIdx].Target = &stack.DropTarget{} filter.Rules[ruleIdx+1].Target = &stack.AcceptTarget{} - if err := ipt.ReplaceTable(stack.FilterID, filter, false /* ipv6 */); err != nil { - t.Fatalf("ipt.ReplaceTable(%d, _, %t): %s", stack.FilterID, false, err) - } + ipt.ReplaceTable(stack.FilterID, filter, false /* ipv6 */) }, genPacket: genPacketV4, proto: header.IPv4ProtocolNumber, @@ -266,9 +258,7 @@ func TestIPTablesStatsForInput(t *testing.T) { } filter.Rules[ruleIdx].Target = &stack.DropTarget{} filter.Rules[ruleIdx+1].Target = &stack.AcceptTarget{} - if err := ipt.ReplaceTable(stack.FilterID, filter, true /* ipv6 */); err != nil { - t.Fatalf("ipt.ReplaceTable(%d, _, %t): %s", stack.FilterID, true, err) - } + ipt.ReplaceTable(stack.FilterID, filter, true /* ipv6 */) }, genPacket: genPacketV6, proto: header.IPv6ProtocolNumber, @@ -289,9 +279,7 @@ func TestIPTablesStatsForInput(t *testing.T) { } filter.Rules[ruleIdx].Target = &stack.DropTarget{} filter.Rules[ruleIdx+1].Target = &stack.AcceptTarget{} - if err := ipt.ReplaceTable(stack.FilterID, filter, false /* ipv6 */); err != nil { - t.Fatalf("ipt.ReplaceTable(%d, _, %t): %s", stack.FilterID, false, err) - } + ipt.ReplaceTable(stack.FilterID, filter, false /* ipv6 */) }, genPacket: genPacketV4, proto: header.IPv4ProtocolNumber, @@ -309,9 +297,7 @@ func TestIPTablesStatsForInput(t *testing.T) { filter.Rules[ruleIdx].Target = &stack.DropTarget{} filter.Rules[ruleIdx].Matchers = []stack.Matcher{&inputIfNameMatcher{anotherNicName}} filter.Rules[ruleIdx+1].Target = &stack.AcceptTarget{} - if err := ipt.ReplaceTable(stack.FilterID, filter, true /* ipv6 */); err != nil { - t.Fatalf("ipt.ReplaceTable(%d, _, %t): %s", stack.FilterID, true, err) - } + ipt.ReplaceTable(stack.FilterID, filter, true /* ipv6 */) }, genPacket: genPacketV6, proto: header.IPv6ProtocolNumber, @@ -329,9 +315,7 @@ func TestIPTablesStatsForInput(t *testing.T) { filter.Rules[ruleIdx].Target = &stack.DropTarget{} filter.Rules[ruleIdx].Matchers = []stack.Matcher{&inputIfNameMatcher{anotherNicName}} filter.Rules[ruleIdx+1].Target = &stack.AcceptTarget{} - if err := ipt.ReplaceTable(stack.FilterID, filter, false /* ipv6 */); err != nil { - t.Fatalf("ipt.ReplaceTable(%d, _, %t): %s", stack.FilterID, false, err) - } + ipt.ReplaceTable(stack.FilterID, filter, false /* ipv6 */) }, genPacket: genPacketV4, proto: header.IPv4ProtocolNumber, @@ -474,9 +458,7 @@ func TestIPTableWritePackets(t *testing.T) { }, } - if err := s.IPTables().ReplaceTable(stack.FilterID, table, false /* ipv4 */); err != nil { - t.Fatalf("ReplaceTable(%d, _, false): %s", stack.FilterID, err) - } + s.IPTables().ReplaceTable(stack.FilterID, table, false /* ipv4 */) }, genPacket: func(r *stack.Route) stack.PacketBufferList { var pkts stack.PacketBufferList @@ -565,9 +547,7 @@ func TestIPTableWritePackets(t *testing.T) { }, } - if err := s.IPTables().ReplaceTable(stack.FilterID, table, true /* ipv6 */); err != nil { - t.Fatalf("ReplaceTable(%d, _, true): %s", stack.FilterID, err) - } + s.IPTables().ReplaceTable(stack.FilterID, table, true /* ipv6 */) }, genPacket: func(r *stack.Route) stack.PacketBufferList { var pkts stack.PacketBufferList @@ -722,9 +702,7 @@ func setupDropFilter(hook stack.Hook, f stack.IPHeaderFilter) func(*testing.T, * filter.Rules[ruleIdx].Target = &stack.DropTarget{NetworkProtocol: netProto} // Make sure the packet is not dropped by the next rule. filter.Rules[ruleIdx+1].Target = &stack.AcceptTarget{NetworkProtocol: netProto} - if err := ipt.ReplaceTable(stack.FilterID, filter, ipv6); err != nil { - t.Fatalf("ipt.ReplaceTable(%d, _, %t): %s", stack.FilterID, ipv6, err) - } + ipt.ReplaceTable(stack.FilterID, filter, ipv6) } } @@ -1221,9 +1199,7 @@ func setupNAT(t *testing.T, s *stack.Stack, netProto tcpip.NetworkProtocolNumber table.Rules[ruleIdx].Target = target // Make sure the packet is not dropped by the next rule. table.Rules[ruleIdx+1].Target = &stack.AcceptTarget{} - if err := ipt.ReplaceTable(stack.NATID, table, ipv6); err != nil { - t.Fatalf("ipt.ReplaceTable(%d, _, %t): %s", stack.NATID, ipv6, err) - } + ipt.ReplaceTable(stack.NATID, table, ipv6) } func setupDNAT(t *testing.T, s *stack.Stack, netProto tcpip.NetworkProtocolNumber, transProto tcpip.TransportProtocolNumber, target stack.Target) { @@ -1316,9 +1292,7 @@ func setupTwiceNAT(t *testing.T, s *stack.Stack, netProto tcpip.NetworkProtocolN }, } - if err := ipt.ReplaceTable(stack.NATID, table, ipv6); err != nil { - t.Fatalf("ipt.ReplaceTable(%d, _, %t): %s", stack.NATID, ipv6, err) - } + ipt.ReplaceTable(stack.NATID, table, ipv6) } type natType struct { @@ -2523,9 +2497,7 @@ func TestNATICMPError(t *testing.T) { }, } - if err := ipt.ReplaceTable(stack.NATID, table, ipv6); err != nil { - t.Fatalf("ipt.ReplaceTable(%d, _, %t): %s", stack.NATID, ipv6, err) - } + ipt.ReplaceTable(stack.NATID, table, ipv6) buf := transportType.buf @@ -2890,9 +2862,7 @@ func TestSNATHandlePortOrIdentConflicts(t *testing.T) { }, } - if err := ipt.ReplaceTable(stack.NATID, table, ipv6); err != nil { - t.Fatalf("ipt.ReplaceTable(%d, _, %t): %s", stack.NATID, ipv6, err) - } + ipt.ReplaceTable(stack.NATID, table, ipv6) for i, srcAddr := range test.srcAddrs { t.Run(fmt.Sprintf("Packet#%d", i), func(t *testing.T) { @@ -2970,9 +2940,7 @@ func TestLocallyRoutedPackets(t *testing.T) { ipv6 := test.netProto == ipv6.ProtocolNumber ipt := s.IPTables() filter := ipt.GetTable(stack.FilterID, ipv6) - if err := ipt.ReplaceTable(stack.FilterID, filter, ipv6); err != nil { - t.Fatalf("ipt.ReplaceTable(%d, _, %t): %s", stack.FilterID, ipv6, err) - } + ipt.ReplaceTable(stack.FilterID, filter, ipv6) } var wq waiter.Queue @@ -3295,9 +3263,7 @@ func TestRejectWith(t *testing.T) { filter.Rules[ruleIdx].Target = test.rejectTarget(t, s.NetworkProtocolInstance(test.netProto), rejectWith.val) // Make sure the packet is not dropped by the next rule. filter.Rules[ruleIdx+1].Target = &stack.AcceptTarget{} - if err := ipt.ReplaceTable(stack.FilterID, filter, ipv6); err != nil { - t.Fatalf("ipt.ReplaceTable(%d, _, %t): %s", stack.FilterID, ipv6, err) - } + ipt.ReplaceTable(stack.FilterID, filter, ipv6) } func() { diff --git a/pkg/tcpip/tests/integration/istio_test.go b/pkg/tcpip/tests/integration/istio_test.go index 10166c5ed..3a66b3724 100644 --- a/pkg/tcpip/tests/integration/istio_test.go +++ b/pkg/tcpip/tests/integration/istio_test.go @@ -314,9 +314,7 @@ func TestOutboundNATRedirect(t *testing.T) { NetworkProtocol: ipv4.ProtocolNumber, } tbl.Rules[ruleIdx+1].Target = &stack.AcceptTarget{} - if err := ipt.ReplaceTable(stack.NATID, tbl, false /* ipv6 */); err != nil { - t.Fatalf("ipt.ReplaceTable(%d, _, false): %s", stack.NATID, err) - } + ipt.ReplaceTable(stack.NATID, tbl, false /* ipv6 */) dialFunc := func(protocol, address string) (net.Conn, error) { host, port, err := net.SplitHostPort(address)