Merge pull request #56880 from MrHohn/kube-proxy-ipv6-fix

Automatic merge from submit-queue (batch tested with PRs 53689, 56880, 55856, 59289, 60249). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

Harden kube-proxy for unmatched IP versions

**What this PR does / why we need it**:
This PR makes kube-proxy omits & logs & emits event for unmatched IP versions configuration (IPv6 address in IPv4 mode or IPv4 address in IPv6 mode). 

**Which issue(s) this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close the issue(s) when PR gets merged)*:
Fixes #57219

**Special notes for your reviewer**:

**Release note**:

```release-note
Fix the issue in kube-proxy iptables/ipvs mode to properly handle incorrect IP version.
```
This commit is contained in:
Kubernetes Submit Queue
2018-02-28 00:00:29 -08:00
committed by GitHub
24 changed files with 1563 additions and 1146 deletions

View File

@@ -8,6 +8,7 @@ go_library(
importpath = "k8s.io/kubernetes/pkg/util/conntrack",
visibility = ["//visibility:public"],
deps = [
"//pkg/util/net:go_default_library",
"//vendor/k8s.io/api/core/v1:go_default_library",
"//vendor/k8s.io/utils/exec:go_default_library",
],
@@ -20,6 +21,7 @@ go_test(
],
embed = [":go_default_library"],
deps = [
"//pkg/util/net:go_default_library",
"//vendor/k8s.io/api/core/v1:go_default_library",
"//vendor/k8s.io/utils/exec:go_default_library",
"//vendor/k8s.io/utils/exec/testing:go_default_library",

View File

@@ -18,11 +18,11 @@ package conntrack
import (
"fmt"
"net"
"strconv"
"strings"
"k8s.io/api/core/v1"
utilnet "k8s.io/kubernetes/pkg/util/net"
"k8s.io/utils/exec"
)
@@ -31,17 +31,6 @@ import (
// NoConnectionToDelete is the error string returned by conntrack when no matching connections are found
const NoConnectionToDelete = "0 flow entries have been deleted"
// IsIPv6 returns true if the given ip address is a valid ipv6 address
func IsIPv6(netIP net.IP) bool {
return netIP != nil && netIP.To4() == nil
}
// IsIPv6String returns true if the given string is a valid ipv6 address
func IsIPv6String(ip string) bool {
netIP := net.ParseIP(ip)
return IsIPv6(netIP)
}
func protoStr(proto v1.Protocol) string {
return strings.ToLower(string(proto))
}
@@ -56,7 +45,7 @@ func parametersWithFamily(isIPv6 bool, parameters ...string) []string {
// ClearEntriesForIP uses the conntrack tool to delete the conntrack entries
// for the UDP connections specified by the given service IP
func ClearEntriesForIP(execer exec.Interface, ip string, protocol v1.Protocol) error {
parameters := parametersWithFamily(IsIPv6String(ip), "-D", "--orig-dst", ip, "-p", protoStr(protocol))
parameters := parametersWithFamily(utilnet.IsIPv6String(ip), "-D", "--orig-dst", ip, "-p", protoStr(protocol))
err := Exec(execer, parameters...)
if err != nil && !strings.Contains(err.Error(), NoConnectionToDelete) {
// TODO: Better handling for deletion failure. When failure occur, stale udp connection may not get flushed.
@@ -107,7 +96,7 @@ func ClearEntriesForPort(execer exec.Interface, port int, isIPv6 bool, protocol
// ClearEntriesForNAT uses the conntrack tool to delete the conntrack entries
// for connections specified by the {origin, dest} IP pair.
func ClearEntriesForNAT(execer exec.Interface, origin, dest string, protocol v1.Protocol) error {
parameters := parametersWithFamily(IsIPv6String(origin), "-D", "--orig-dst", origin, "--dst-nat", dest,
parameters := parametersWithFamily(utilnet.IsIPv6String(origin), "-D", "--orig-dst", origin, "--dst-nat", dest,
"-p", protoStr(protocol))
err := Exec(execer, parameters...)
if err != nil && !strings.Contains(err.Error(), NoConnectionToDelete) {

View File

@@ -18,11 +18,11 @@ package conntrack
import (
"fmt"
"net"
"strings"
"testing"
"k8s.io/api/core/v1"
utilnet "k8s.io/kubernetes/pkg/util/net"
"k8s.io/utils/exec"
fakeexec "k8s.io/utils/exec/testing"
)
@@ -119,7 +119,7 @@ func TestClearUDPConntrackForIP(t *testing.T) {
if err := ClearEntriesForIP(&fexec, tc.ip, v1.ProtocolUDP); err != nil {
t.Errorf("%s test case:, Unexpected error: %v", tc.name, err)
}
expectCommand := fmt.Sprintf("conntrack -D --orig-dst %s -p udp", tc.ip) + familyParamStr(IsIPv6String(tc.ip))
expectCommand := fmt.Sprintf("conntrack -D --orig-dst %s -p udp", tc.ip) + familyParamStr(utilnet.IsIPv6String(tc.ip))
execCommand := strings.Join(fcmd.CombinedOutputLog[svcCount], " ")
if expectCommand != execCommand {
t.Errorf("%s test case: Expect command: %s, but executed %s", tc.name, expectCommand, execCommand)
@@ -223,7 +223,7 @@ func TestDeleteUDPConnections(t *testing.T) {
if err != nil {
t.Errorf("%s test case: unexpected error: %v", tc.name, err)
}
expectCommand := fmt.Sprintf("conntrack -D --orig-dst %s --dst-nat %s -p udp", tc.origin, tc.dest) + familyParamStr(IsIPv6String(tc.origin))
expectCommand := fmt.Sprintf("conntrack -D --orig-dst %s --dst-nat %s -p udp", tc.origin, tc.dest) + familyParamStr(utilnet.IsIPv6String(tc.origin))
execCommand := strings.Join(fcmd.CombinedOutputLog[i], " ")
if expectCommand != execCommand {
t.Errorf("%s test case: Expect command: %s, but executed %s", tc.name, expectCommand, execCommand)
@@ -234,99 +234,3 @@ func TestDeleteUDPConnections(t *testing.T) {
t.Errorf("Expect command executed %d times, but got %d", svcCount, fexec.CommandCalls)
}
}
func TestIsIPv6String(t *testing.T) {
testCases := []struct {
ip string
expectIPv6 bool
}{
{
ip: "127.0.0.1",
expectIPv6: false,
},
{
ip: "192.168.0.0",
expectIPv6: false,
},
{
ip: "1.2.3.4",
expectIPv6: false,
},
{
ip: "bad ip",
expectIPv6: false,
},
{
ip: "::1",
expectIPv6: true,
},
{
ip: "fd00::600d:f00d",
expectIPv6: true,
},
{
ip: "2001:db8::5",
expectIPv6: true,
},
}
for i := range testCases {
isIPv6 := IsIPv6String(testCases[i].ip)
if isIPv6 != testCases[i].expectIPv6 {
t.Errorf("[%d] Expect ipv6 %v, got %v", i+1, testCases[i].expectIPv6, isIPv6)
}
}
}
func TestIsIPv6(t *testing.T) {
testCases := []struct {
ip net.IP
expectIPv6 bool
}{
{
ip: net.IPv4zero,
expectIPv6: false,
},
{
ip: net.IPv4bcast,
expectIPv6: false,
},
{
ip: net.ParseIP("127.0.0.1"),
expectIPv6: false,
},
{
ip: net.ParseIP("10.20.40.40"),
expectIPv6: false,
},
{
ip: net.ParseIP("172.17.3.0"),
expectIPv6: false,
},
{
ip: nil,
expectIPv6: false,
},
{
ip: net.IPv6loopback,
expectIPv6: true,
},
{
ip: net.IPv6zero,
expectIPv6: true,
},
{
ip: net.ParseIP("fd00::600d:f00d"),
expectIPv6: true,
},
{
ip: net.ParseIP("2001:db8::5"),
expectIPv6: true,
},
}
for i := range testCases {
isIPv6 := IsIPv6(testCases[i].ip)
if isIPv6 != testCases[i].expectIPv6 {
t.Errorf("[%d] Expect ipv6 %v, got %v", i+1, testCases[i].expectIPv6, isIPv6)
}
}
}

View File

@@ -1,3 +1,5 @@
load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test")
package(default_visibility = ["//visibility:public"])
filegroup(
@@ -15,3 +17,15 @@ filegroup(
],
tags = ["automanaged"],
)
go_library(
name = "go_default_library",
srcs = ["net.go"],
importpath = "k8s.io/kubernetes/pkg/util/net",
)
go_test(
name = "go_default_test",
srcs = ["net_test.go"],
embed = [":go_default_library"],
)

4
pkg/util/net/OWNERS Normal file
View File

@@ -0,0 +1,4 @@
reviewers:
- sig-network-reviewers
approvers:
- sig-network-approvers

61
pkg/util/net/net.go Normal file
View File

@@ -0,0 +1,61 @@
/*
Copyright 2018 The Kubernetes Authors.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/
package net
import (
"net"
)
// IsIPv6 returns if netIP is IPv6.
func IsIPv6(netIP net.IP) bool {
return netIP != nil && netIP.To4() == nil
}
// IsIPv6String returns if ip is IPv6.
func IsIPv6String(ip string) bool {
netIP := net.ParseIP(ip)
return IsIPv6(netIP)
}
// IsIPv6CIDR returns if cidr is IPv6.
// This assumes cidr is a valid CIDR.
func IsIPv6CIDR(cidr string) bool {
ip, _, _ := net.ParseCIDR(cidr)
return IsIPv6(ip)
}
// FilterIncorrectIPVersion filters out the incorrect IP version case from a slice of IP strings.
func FilterIncorrectIPVersion(ipStrings []string, isIPv6Mode bool) ([]string, []string) {
return filterWithCondition(ipStrings, isIPv6Mode, IsIPv6String)
}
// FilterIncorrectCIDRVersion filters out the incorrect IP version case from a slice of CIDR strings.
func FilterIncorrectCIDRVersion(ipStrings []string, isIPv6Mode bool) ([]string, []string) {
return filterWithCondition(ipStrings, isIPv6Mode, IsIPv6CIDR)
}
func filterWithCondition(strs []string, expectedCondition bool, conditionFunc func(string) bool) ([]string, []string) {
var corrects, incorrects []string
for _, str := range strs {
if conditionFunc(str) != expectedCondition {
incorrects = append(incorrects, str)
} else {
corrects = append(corrects, str)
}
}
return corrects, incorrects
}

286
pkg/util/net/net_test.go Normal file
View File

@@ -0,0 +1,286 @@
/*
Copyright 2018 The Kubernetes Authors.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/
package net
import (
"net"
"reflect"
"testing"
)
func TestIsIPv6String(t *testing.T) {
testCases := []struct {
ip string
expectIPv6 bool
}{
{
ip: "127.0.0.1",
expectIPv6: false,
},
{
ip: "192.168.0.0",
expectIPv6: false,
},
{
ip: "1.2.3.4",
expectIPv6: false,
},
{
ip: "bad ip",
expectIPv6: false,
},
{
ip: "::1",
expectIPv6: true,
},
{
ip: "fd00::600d:f00d",
expectIPv6: true,
},
{
ip: "2001:db8::5",
expectIPv6: true,
},
}
for i := range testCases {
isIPv6 := IsIPv6String(testCases[i].ip)
if isIPv6 != testCases[i].expectIPv6 {
t.Errorf("[%d] Expect ipv6 %v, got %v", i+1, testCases[i].expectIPv6, isIPv6)
}
}
}
func TestIsIPv6(t *testing.T) {
testCases := []struct {
ip net.IP
expectIPv6 bool
}{
{
ip: net.IPv4zero,
expectIPv6: false,
},
{
ip: net.IPv4bcast,
expectIPv6: false,
},
{
ip: net.ParseIP("127.0.0.1"),
expectIPv6: false,
},
{
ip: net.ParseIP("10.20.40.40"),
expectIPv6: false,
},
{
ip: net.ParseIP("172.17.3.0"),
expectIPv6: false,
},
{
ip: nil,
expectIPv6: false,
},
{
ip: net.IPv6loopback,
expectIPv6: true,
},
{
ip: net.IPv6zero,
expectIPv6: true,
},
{
ip: net.ParseIP("fd00::600d:f00d"),
expectIPv6: true,
},
{
ip: net.ParseIP("2001:db8::5"),
expectIPv6: true,
},
}
for i := range testCases {
isIPv6 := IsIPv6(testCases[i].ip)
if isIPv6 != testCases[i].expectIPv6 {
t.Errorf("[%d] Expect ipv6 %v, got %v", i+1, testCases[i].expectIPv6, isIPv6)
}
}
}
func TestIsIPv6CIDR(t *testing.T) {
testCases := []struct {
desc string
cidr string
expectResult bool
}{
{
desc: "ipv4 CIDR 1",
cidr: "10.0.0.0/8",
expectResult: false,
},
{
desc: "ipv4 CIDR 2",
cidr: "192.168.0.0/16",
expectResult: false,
},
{
desc: "ipv6 CIDR 1",
cidr: "::/1",
expectResult: true,
},
{
desc: "ipv6 CIDR 2",
cidr: "2000::/10",
expectResult: true,
},
{
desc: "ipv6 CIDR 3",
cidr: "2001:db8::/32",
expectResult: true,
},
}
for _, tc := range testCases {
res := IsIPv6CIDR(tc.cidr)
if res != tc.expectResult {
t.Errorf("%v: want IsIPv6CIDR=%v, got %v", tc.desc, tc.expectResult, res)
}
}
}
func TestFilterIncorrectIPVersion(t *testing.T) {
testCases := []struct {
desc string
isIPv6 bool
ipStrings []string
expectCorrects []string
expectIncorrects []string
}{
{
desc: "all ipv4 strings in ipv4 mode",
isIPv6: false,
ipStrings: []string{"10.0.0.1", "192.168.0.1", "127.0.0.1"},
expectCorrects: []string{"10.0.0.1", "192.168.0.1", "127.0.0.1"},
expectIncorrects: nil,
},
{
desc: "all ipv6 strings in ipv4 mode",
isIPv6: false,
ipStrings: []string{"::1", "fd00::600d:f00d", "2001:db8::5"},
expectCorrects: nil,
expectIncorrects: []string{"::1", "fd00::600d:f00d", "2001:db8::5"},
},
{
desc: "mixed versions in ipv4 mode",
isIPv6: false,
ipStrings: []string{"10.0.0.1", "192.168.0.1", "127.0.0.1", "::1", "fd00::600d:f00d", "2001:db8::5"},
expectCorrects: []string{"10.0.0.1", "192.168.0.1", "127.0.0.1"},
expectIncorrects: []string{"::1", "fd00::600d:f00d", "2001:db8::5"},
},
{
desc: "all ipv4 strings in ipv6 mode",
isIPv6: true,
ipStrings: []string{"10.0.0.1", "192.168.0.1", "127.0.0.1"},
expectCorrects: nil,
expectIncorrects: []string{"10.0.0.1", "192.168.0.1", "127.0.0.1"},
},
{
desc: "all ipv6 strings in ipv6 mode",
isIPv6: true,
ipStrings: []string{"::1", "fd00::600d:f00d", "2001:db8::5"},
expectCorrects: []string{"::1", "fd00::600d:f00d", "2001:db8::5"},
expectIncorrects: nil,
},
{
desc: "mixed versions in ipv6 mode",
isIPv6: true,
ipStrings: []string{"10.0.0.1", "192.168.0.1", "127.0.0.1", "::1", "fd00::600d:f00d", "2001:db8::5"},
expectCorrects: []string{"::1", "fd00::600d:f00d", "2001:db8::5"},
expectIncorrects: []string{"10.0.0.1", "192.168.0.1", "127.0.0.1"},
},
}
for _, tc := range testCases {
corrects, incorrects := FilterIncorrectIPVersion(tc.ipStrings, tc.isIPv6)
if !reflect.DeepEqual(tc.expectCorrects, corrects) {
t.Errorf("%v: want corrects=%v, got %v", tc.desc, tc.expectCorrects, corrects)
}
if !reflect.DeepEqual(tc.expectIncorrects, incorrects) {
t.Errorf("%v: want incorrects=%v, got %v", tc.desc, tc.expectIncorrects, incorrects)
}
}
}
func TestFilterIncorrectCIDRVersion(t *testing.T) {
testCases := []struct {
desc string
isIPv6 bool
cidrStrings []string
expectCorrects []string
expectIncorrects []string
}{
{
desc: "all ipv4 strings in ipv4 mode",
isIPv6: false,
cidrStrings: []string{"0.0.0.0/1", "1.0.0.0/1"},
expectCorrects: []string{"0.0.0.0/1", "1.0.0.0/1"},
expectIncorrects: nil,
},
{
desc: "all ipv6 strings in ipv4 mode",
isIPv6: false,
cidrStrings: []string{"2001:db8::/32", "2001:0db8:0123:4567::/64"},
expectCorrects: nil,
expectIncorrects: []string{"2001:db8::/32", "2001:0db8:0123:4567::/64"},
},
{
desc: "mixed versions in ipv4 mode",
isIPv6: false,
cidrStrings: []string{"0.0.0.0/1", "1.0.0.0/1", "2001:db8::/32", "2001:0db8:0123:4567::/64"},
expectCorrects: []string{"0.0.0.0/1", "1.0.0.0/1"},
expectIncorrects: []string{"2001:db8::/32", "2001:0db8:0123:4567::/64"},
},
{
desc: "all ipv4 strings in ipv6 mode",
isIPv6: true,
cidrStrings: []string{"0.0.0.0/1", "1.0.0.0/1"},
expectCorrects: nil,
expectIncorrects: []string{"0.0.0.0/1", "1.0.0.0/1"},
},
{
desc: "all ipv6 strings in ipv6 mode",
isIPv6: true,
cidrStrings: []string{"2001:db8::/32", "2001:0db8:0123:4567::/64"},
expectCorrects: []string{"2001:db8::/32", "2001:0db8:0123:4567::/64"},
expectIncorrects: nil,
},
{
desc: "mixed versions in ipv6 mode",
isIPv6: true,
cidrStrings: []string{"0.0.0.0/1", "1.0.0.0/1", "2001:db8::/32", "2001:0db8:0123:4567::/64"},
expectCorrects: []string{"2001:db8::/32", "2001:0db8:0123:4567::/64"},
expectIncorrects: []string{"0.0.0.0/1", "1.0.0.0/1"},
},
}
for _, tc := range testCases {
corrects, incorrects := FilterIncorrectCIDRVersion(tc.cidrStrings, tc.isIPv6)
if !reflect.DeepEqual(tc.expectCorrects, corrects) {
t.Errorf("%v: want corrects=%v, got %v", tc.desc, tc.expectCorrects, corrects)
}
if !reflect.DeepEqual(tc.expectIncorrects, incorrects) {
t.Errorf("%v: want incorrects=%v, got %v", tc.desc, tc.expectIncorrects, incorrects)
}
}
}