Merge pull request #4097 from swagatbora90/fix-port-check-vsiravar

[Bugfix] Check if port is used in publish flag [duplicate of #2190]

Author: AkihiroSuda

cmd/nerdctl/container/container_run_network_linux_test.go

@@ -349,6 +349,61 @@ func TestUniqueHostPortAssignement(t *testing.T) {
 	}
 }
 
+func TestHostPortAlreadyInUse(t *testing.T) {
+	testCases := []struct {
+		hostPort      string
+		containerPort string
+	}{
+		{
+			hostPort:      "5000",
+			containerPort: "80/tcp",
+		},
+		{
+			hostPort:      "5000",
+			containerPort: "80/tcp",
+		},
+		{
+			hostPort:      "5000",
+			containerPort: "80/udp",
+		},
+		{
+			hostPort:      "5000",
+			containerPort: "80/sctp",
+		},
+	}
+
+	tID := testutil.Identifier(t)
+
+	for i, tc := range testCases {
+		tc := tc
+		tcName := fmt.Sprintf("%+v", tc)
+		t.Run(tcName, func(t *testing.T) {
+			if strings.Contains(tc.containerPort, "sctp") && rootlessutil.IsRootless() {
+				t.Skip("sctp is not supported in rootless mode")
+			}
+			testContainerName1 := fmt.Sprintf("%s-%d-1", tID, i)
+			testContainerName2 := fmt.Sprintf("%s-%d-2", tID, i)
+			base := testutil.NewBase(t)
+			t.Cleanup(func() {
+				base.Cmd("rm", "-f", testContainerName1, testContainerName2).AssertOK()
+			})
+			pFlag := fmt.Sprintf("%s:%s", tc.hostPort, tc.containerPort)
+			cmd1 := base.Cmd("run", "-d",
+				"--name", testContainerName1, "-p",
+				pFlag,
+				testutil.NginxAlpineImage)
+
+			cmd2 := base.Cmd("run", "-d",
+				"--name", testContainerName2, "-p",
+				pFlag,
+				testutil.NginxAlpineImage)
+
+			cmd1.AssertOK()
+			cmd2.AssertFail()
+		})
+	}
+}
+
 func TestRunPort(t *testing.T) {
 	baseTestRunPort(t, testutil.NginxAlpineImage, testutil.NginxAlpineIndexHTMLSnippet, true)
 }

pkg/portutil/port_allocate_linux.go

@@ -25,11 +25,14 @@ import (
 
 const (
 	// This port range is compatible with Docker, FYI https://github.com/moby/moby/blob/eb9e42a09ee123af1d95bf7d46dd738258fa2109/libnetwork/portallocator/portallocator_unix.go#L7-L12
-	allocateEnd = 60999
+	allocateEnd = uint64(60999)
+
+	tcpTimeWait  = 6 //TIME_WAIT state is represented by the value 6 in /proc/net/tcp
+	tcpCloseWait = 8 //CLOSE_WAIT state is represented by the value 8 in /proc/net/tcp
 )
 
 var (
-	allocateStart = 49153
+	allocateStart = uint64(49153)
 )
 
 func filter(ss []procnet.NetworkDetail, filterFunc func(detail procnet.NetworkDetail) bool) (ret []procnet.NetworkDetail) {
@@ -42,24 +45,56 @@ func filter(ss []procnet.NetworkDetail, filterFunc func(detail procnet.NetworkDe
 }
 
 func portAllocate(protocol string, ip string, count uint64) (uint64, uint64, error) {
-	netprocData, err := procnet.ReadStatsFileData(protocol)
+	usedPorts, err := getUsedPorts(ip, protocol)
 	if err != nil {
 		return 0, 0, err
 	}
-	netprocItems := procnet.Parse(netprocData)
+
+	start := allocateStart
+	if count > allocateEnd-allocateStart+1 {
+		return 0, 0, fmt.Errorf("can not allocate %d ports", count)
+	}
+	for start < allocateEnd {
+		needReturn := true
+		for i := start; i < start+count; i++ {
+			if _, ok := usedPorts[i]; ok {
+				needReturn = false
+				break
+			}
+		}
+		if needReturn {
+			allocateStart = start + count
+			return start, start + count - 1, nil
+		}
+		start += count
+	}
+	return 0, 0, fmt.Errorf("there is not enough %d free ports", count)
+}
+
+func getUsedPorts(ip string, protocol string) (map[uint64]bool, error) {
+	netprocItems := []procnet.NetworkDetail{}
+
+	if protocol == "tcp" || protocol == "udp" {
+		netprocData, err := procnet.ReadStatsFileData(protocol)
+		if err != nil {
+			return nil, err
+		}
+		netprocItems = append(netprocItems, procnet.Parse(netprocData)...)
+	}
+
 	// In some circumstances, when we bind address like "0.0.0.0:80", we will get the formation of ":::80" in /proc/net/tcp6.
 	// So we need some trick to process this situation.
 	if protocol == "tcp" {
 		tempTCPV6Data, err := procnet.ReadStatsFileData("tcp6")
 		if err != nil {
-			return 0, 0, err
+			return nil, err
 		}
 		netprocItems = append(netprocItems, procnet.Parse(tempTCPV6Data)...)
 	}
 	if protocol == "udp" {
 		tempUDPV6Data, err := procnet.ReadStatsFileData("udp6")
 		if err != nil {
-			return 0, 0, err
+			return nil, err
 		}
 		netprocItems = append(netprocItems, procnet.Parse(tempUDPV6Data)...)
 	}
@@ -73,36 +108,26 @@ func portAllocate(protocol string, ip string, count uint64) (uint64, uint64, err
 
 	usedPort := make(map[uint64]bool)
 	for _, value := range netprocItems {
+		// Skip ports in TIME_WAIT or CLOSE_WAIT state
+		if protocol == "tcp" && (value.State == tcpTimeWait || value.State == tcpCloseWait) {
+			// In rootless mode, Rootlesskit creates extra socket connections to proxy traffic from the host network namespace
+			// to the container namespace. Proxy TCP connections can remain in TIME_WAIT state for 10-20 seconds even when the
+			// container is stopped/removed, which is standard TCP behavior. These ports are actually available for allocation
+			// despite appearing in /proc/net/tcp.
+			continue
+		}
 		usedPort[value.LocalPort] = true
 	}
 
 	ipTableItems, err := iptable.ReadIPTables("nat")
 	if err != nil {
-		return 0, 0, err
+		return nil, err
 	}
 	destinationPorts := iptable.ParseIPTableRules(ipTableItems)
 
 	for _, port := range destinationPorts {
 		usedPort[port] = true
 	}
 
-	start := uint64(allocateStart)
-	if count > uint64(allocateEnd-allocateStart+1) {
-		return 0, 0, fmt.Errorf("can not allocate %d ports", count)
-	}
-	for start < allocateEnd {
-		needReturn := true
-		for i := start; i < start+count; i++ {
-			if _, ok := usedPort[i]; ok {
-				needReturn = false
-				break
-			}
-		}
-		if needReturn {
-			allocateStart = int(start + count)
-			return start, start + count - 1, nil
-		}
-		start += count
-	}
-	return 0, 0, fmt.Errorf("there is not enough %d free ports", count)
+	return usedPort, nil
 }

pkg/portutil/port_allocate_other.go

@@ -23,3 +23,7 @@ import "fmt"
 func portAllocate(protocol string, ip string, count uint64) (uint64, uint64, error) {
 	return 0, 0, fmt.Errorf("auto port allocate are not support Non-Linux platform yet")
 }
+
+func getUsedPorts(ip string, protocol string) (map[uint64]bool, error) {
+	return nil, nil
+}

pkg/portutil/portutil.go

@@ -101,6 +101,16 @@ func ParseFlagP(s string) ([]cni.PortMapping, error) {
 		if err != nil {
 			return nil, fmt.Errorf("invalid hostPort: %s", hostPort)
 		}
+		var usedPorts map[uint64]bool
+		usedPorts, err = getUsedPorts(ip, proto)
+		if err != nil {
+			return nil, err
+		}
+		for i := startHostPort; i <= endHostPort; i++ {
+			if usedPorts[i] {
+				return nil, fmt.Errorf("bind for %s:%d failed: port is already allocated", ip, i)
+			}
+		}
 	}
 	if hostPort != "" && (endPort-startPort) != (endHostPort-startHostPort) {
 		if endPort != startPort {

pkg/portutil/procnet/procnet.go

@@ -27,6 +27,7 @@ import (
 type NetworkDetail struct {
 	LocalIP   net.IP
 	LocalPort uint64
+	State     int
 }
 
 func Parse(data []string) (results []NetworkDetail) {
@@ -37,9 +38,19 @@ func Parse(data []string) (results []NetworkDetail) {
 		if err != nil {
 			continue
 		}
+
+		state := 0
+		if len(lineData) > 2 {
+			stateHex, err := strconv.ParseInt(lineData[3], 16, 32)
+			if err == nil {
+				state = int(stateHex)
+			}
+		}
+
 		results = append(results, NetworkDetail{
 			LocalIP:   ip,
 			LocalPort: uint64(port),
+			State:     state,
 		})
 	}
 	return results