rootless-containers · AkihiroSuda · Feb 15, 2021 · Feb 11, 2021 · Feb 11, 2021 · Feb 11, 2021
diff --git a/pkg/port/builtin/parent/tcp/tcp.go b/pkg/port/builtin/parent/tcp/tcp.go
@@ -5,14 +5,15 @@ import (
 	"io"
 	"net"
 	"os"
+	"strconv"
 	"sync"
 
 	"github.com/rootless-containers/rootlesskit/pkg/port"
 	"github.com/rootless-containers/rootlesskit/pkg/port/builtin/msg"
 )
 
 func Run(socketPath string, spec port.Spec, stopCh <-chan struct{}, logWriter io.Writer) error {
-	ln, err := net.Listen("tcp", fmt.Sprintf("%s:%d", spec.ParentIP, spec.ParentPort))
+	ln, err := net.Listen("tcp", net.JoinHostPort(spec.ParentIP, strconv.Itoa(spec.ParentPort)))
 	if err != nil {
 		fmt.Fprintf(logWriter, "listen: %v\n", err)
 		return err

diff --git a/pkg/port/builtin/parent/udp/udp.go b/pkg/port/builtin/parent/udp/udp.go
@@ -1,10 +1,10 @@
 package udp
 
 import (
-	"fmt"
 	"io"
 	"net"
 	"os"
+	"strconv"
 
 	"github.com/pkg/errors"
 
@@ -14,7 +14,7 @@ import (
 )
 
 func Run(socketPath string, spec port.Spec, stopCh <-chan struct{}, logWriter io.Writer) error {
-	addr, err := net.ResolveUDPAddr("udp", fmt.Sprintf("%s:%d", spec.ParentIP, spec.ParentPort))
+	addr, err := net.ResolveUDPAddr("udp", net.JoinHostPort(spec.ParentIP, strconv.Itoa(spec.ParentPort)))
 	if err != nil {
 		return err
 	}

diff --git a/pkg/port/portutil/portutil.go b/pkg/port/portutil/portutil.go
@@ -4,76 +4,135 @@ import (
 	"net"
 	"strconv"
 	"strings"
+	"text/scanner"
 
 	"github.com/pkg/errors"
 
 	"github.com/rootless-containers/rootlesskit/pkg/port"
 )
 
-// ParsePortSpec parses a Docker-like representation of PortSpec.
+// ParsePortSpec parses a Docker-like representation of PortSpec, but with
+// support for both "parent IP" and "child IP" (optional);
 // e.g. "127.0.0.1:8080:80/tcp", or "127.0.0.1:8080:10.0.2.100:80/tcp"
-func ParsePortSpec(s string) (*port.Spec, error) {
-	splitBySlash := strings.SplitN(s, "/", 2)
-	if len(splitBySlash) != 2 {
-		return nil, errors.Errorf("unexpected PortSpec string: %q", s)
+//
+// Format is as follows:
+//
+//     <parent IP>:<parent port>[:<child IP>]:<child port>/<proto>
+//
+// Note that (child IP being optional) the format can either contain 5 or 4
+// components. When using IPv6 IP addresses, addresses must use square brackets
+// to prevent the colons being mistaken for delimiters. For example:
+//
+//     [::1]:8080:[::2]:80/udp
+func ParsePortSpec(portSpec string) (*port.Spec, error) {
+	const (
+		parentIP   = iota
+		parentPort = iota
+		childIP    = iota
+		childPort  = iota
+		proto      = iota
+	)
+
+	var (
+		s         scanner.Scanner
+		err       error
+		parts     = make([]string, 5)
+		index     = parentIP
+		delimiter = ':'
+	)
+
+	// First get the "proto" and "parent-port" at the end. These parts are
+	// required, whereas "ParentIP" is optional. Removing them first makes
+	// it easier to parse the remaining parts, as otherwise the third part
+	// could be _either_ an IP-address _or_ a Port.
+
+	// Get the proto
+	protoPos := strings.LastIndex(portSpec, "/")
+	if protoPos < 0 {
+		return nil, errors.Errorf("missing proto in PortSpec string: %q", portSpec)
 	}
-	proto := splitBySlash[1]
-	switch proto {
-	case "tcp", "udp", "sctp":
-	default:
-		return nil, errors.Errorf("unexpected Proto in PortSpec string: %q", s)
+	parts[proto] = portSpec[protoPos+1:]
+	err = validateProto(parts[proto])
+	if err != nil {
+		return nil, errors.Wrapf(err, "invalid PortSpec string: %q", portSpec)
 	}
 
-	splitByColon := strings.SplitN(splitBySlash[0], ":", 4)
-	switch len(splitByColon) {
-	case 3, 4:
-	default:
-		return nil, errors.Errorf("unexpected PortSpec string: %q", s)
+	// Get the parent port
+	portPos := strings.LastIndex(portSpec, ":")
+	if portPos < 0 {
+		return nil, errors.Errorf("unexpected PortSpec string: %q", portSpec)
 	}
+	parts[childPort] = portSpec[portPos+1 : protoPos]
+
+	// Scan the remainder "<IP-address>:<port>[:<IP-address>]"
+	s.Init(strings.NewReader(portSpec[:portPos]))
+
+	for tok := s.Scan(); tok != scanner.EOF; tok = s.Scan() {
+		if index > childPort {
+			return nil, errors.Errorf("unexpected PortSpec string: %q", portSpec)
+		}
 
-	parentIP := splitByColon[0]
-	if net.IP(parentIP) == nil {
-		return nil, errors.Errorf("unexpected ParentIP in PortSpec string: %q", s)
+		switch tok {
+		case '[':
+			// Start of IPv6 IP-address; value ends at closing bracket (])
+			delimiter = ']'
+			continue
+		case delimiter:
+			if delimiter == ']' {
+				// End of IPv6 IP-address
+				delimiter = ':'
+				// Skip the next token, which should be a colon delimiter (:)
+				tok = s.Scan()
+			}
+			index++
+			continue
+		default:
+			parts[index] += s.TokenText()
+		}
 	}
 
-	parentPort, err := strconv.Atoi(splitByColon[1])
-	if err != nil {
-		return nil, errors.Wrapf(err, "unexpected ParentPort in PortSpec string: %q", s)
+	if parts[parentIP] != "" && net.ParseIP(parts[parentIP]) == nil {
+		return nil, errors.Errorf("unexpected ParentIP in PortSpec string: %q", portSpec)
+	}
+	if parts[childIP] != "" && net.ParseIP(parts[childIP]) == nil {
+		return nil, errors.Errorf("unexpected ParentIP in PortSpec string: %q", portSpec)
 	}
 
-	var childIP string
-	if len(splitByColon) == 4 {
-		childIP = splitByColon[2]
-		if net.IP(childIP) == nil {
-			return nil, errors.Errorf("unexpected ChildIP in PortSpec string: %q", s)
-		}
+	ps := &port.Spec{
+		Proto:    parts[proto],
+		ParentIP: parts[parentIP],
+		ChildIP:  parts[childIP],
 	}
 
-	childPort, err := strconv.Atoi(splitByColon[len(splitByColon)-1])
+	ps.ParentPort, err = strconv.Atoi(parts[parentPort])
 	if err != nil {
-		return nil, errors.Wrapf(err, "unexpected ChildPort in PortSpec string: %q", s)
+		return nil, errors.Wrapf(err, "unexpected ChildPort in PortSpec string: %q", portSpec)
 	}
 
-	return &port.Spec{
-		Proto:      proto,
-		ParentIP:   parentIP,
-		ParentPort: parentPort,
-		ChildIP:    childIP,
-		ChildPort:  childPort,
-	}, nil
+	ps.ChildPort, err = strconv.Atoi(parts[childPort])
+	if err != nil {
+		return nil, errors.Wrapf(err, "unexpected ParentPort in PortSpec string: %q", portSpec)
+	}
+
+	return ps, nil
 }
 
 // ValidatePortSpec validates *port.Spec.
 // existingPorts can be optionally passed for detecting conflicts.
 func ValidatePortSpec(spec port.Spec, existingPorts map[int]*port.Status) error {
-	if spec.Proto != "tcp" && spec.Proto != "udp" {
-		return errors.Errorf("unknown proto: %q", spec.Proto)
+	if err := validateProto(spec.Proto); err != nil {
+		return err
 	}
 	if spec.ParentIP != "" {
 		if net.ParseIP(spec.ParentIP) == nil {
 			return errors.Errorf("invalid ParentIP: %q", spec.ParentIP)
 		}
 	}
+	if spec.ChildIP != "" {
+		if net.ParseIP(spec.ChildIP) == nil {
+			return errors.Errorf("invalid ChildIP: %q", spec.ChildIP)
+		}
+	}
 	if spec.ParentPort <= 0 || spec.ParentPort > 65535 {
 		return errors.Errorf("invalid ParentPort: %q", spec.ParentPort)
 	}
@@ -90,3 +149,12 @@ func ValidatePortSpec(spec port.Spec, existingPorts map[int]*port.Status) error
 	}
 	return nil
 }
+
+func validateProto(proto string) error {
+	switch proto {
+	case "tcp", "udp", "sctp":
+		return nil
+	default:
+		return errors.Errorf("unknown proto: %q", proto)
+	}
+}
diff --git a/pkg/port/portutil/portutil_test.go b/pkg/port/portutil/portutil_test.go
@@ -45,21 +45,43 @@ func TestParsePortSpec(t *testing.T) {
 			s: "8080",
 			// future version may support short formats like this
 		},
+		{
+			s: "[::1]:8080:80/tcp",
+			expected: &port.Spec{
+				Proto:      "tcp",
+				ParentIP:   "::1",
+				ParentPort: 8080,
+				ChildPort:  80,
+			},
+		},
+		{
+			s: "[::1]:8080:[::2]:80/udp",
+			expected: &port.Spec{
+				Proto:      "udp",
+				ParentIP:   "::1",
+				ParentPort: 8080,
+				ChildIP:    "::2",
+				ChildPort:  80,
+			},
+		},
 	}
 	for _, tc := range testCases {
-		got, err := ParsePortSpec(tc.s)
-		if tc.expected == nil {
-			if err == nil {
-				t.Fatalf("error is expected for %q", tc.s)
+		tc := tc
+		t.Run(tc.s, func(t *testing.T) {
+			got, err := ParsePortSpec(tc.s)
+			if tc.expected == nil {
+				if err == nil {
+					t.Fatalf("error is expected for %q", tc.s)
+				}
+			} else {
+				if err != nil {
+					t.Fatalf("got error for %q: %v", tc.s, err)
+				}
+				if !reflect.DeepEqual(got, tc.expected) {
+					t.Fatalf("expected %+v, got %+v", tc.expected, got)
+				}
 			}
-		} else {
-			if err != nil {
-				t.Fatalf("got error for %q: %v", tc.s, err)
-			}
-			if !reflect.DeepEqual(got, tc.expected) {
-				t.Fatalf("expected %+v, got %+v", tc.expected, got)
-			}
-		}
+		})
 	}
 }
 
@@ -96,7 +118,7 @@ func TestValidatePortSpec(t *testing.T) {
 
 	// proto must be supplied and must equal "udp" or "tcp"
 	invalidProtos := []string{"", "NaN", "TCP"}
-	validProtos := []string{"udp", "tcp"}
+	validProtos := []string{"udp", "tcp", "sctp"}
 	for _, p := range invalidProtos {
 		s := spec
 		s.Proto = p
@@ -111,6 +133,12 @@ func TestValidatePortSpec(t *testing.T) {
 
 	}
 
+	s := port.Spec{Proto: "tcp", ParentIP: "invalid", ParentPort: 80, ChildPort: 80}
+	assert.Error(t, ValidatePortSpec(s, existingPorts))
+
+	s = port.Spec{Proto: "tcp", ParentPort: 80, ChildIP: "invalid", ChildPort: 80}
+	assert.Error(t, ValidatePortSpec(s, existingPorts))
+
 	invalidPorts := []int{-200, 0, 1000000}
 	validPorts := []int{20, 500, 1337, 65000}
 
@@ -146,7 +174,7 @@ func TestValidatePortSpec(t *testing.T) {
 	// existing ports include tcp 10.10.10.10:8080, tcp *:80, no udp
 
 	// udp doesn't conflict with tcp
-	s := port.Spec{Proto: "udp", ParentPort: 80, ChildPort: 80}
+	s = port.Spec{Proto: "udp", ParentPort: 80, ChildPort: 80}
 	assert.NoError(t, ValidatePortSpec(s, existingPorts))
 
 	// same parent, same child, different IP has no conflict

diff --git a/pkg/port/socat/socat.go b/pkg/port/socat/socat.go
@@ -7,6 +7,7 @@ import (
 	"net"
 	"os"
 	"os/exec"
+	"strconv"
 	"sync"
 	"syscall"
 	"time"
@@ -149,21 +150,23 @@ func createSocatCmd(ctx context.Context, spec port.Spec, logWriter io.Writer, ch
 		}
 		ip = p.String()
 	}
-	var cmd *exec.Cmd
+	var (
+		cmd   *exec.Cmd
+		proto string
+		hp    = net.JoinHostPort(ip, strconv.Itoa(spec.ChildPort))
+	)
 	switch spec.Proto {
 	case "tcp":
-		cmd = exec.CommandContext(ctx,
-			"socat",
-			fmt.Sprintf("TCP-LISTEN:%d,bind=%s,reuseaddr,fork,rcvbuf=65536,sndbuf=65536", spec.ParentPort, ipStr),
-			fmt.Sprintf("EXEC:\"%s\",nofork",
-				fmt.Sprintf("nsenter -U -n --preserve-credentials -t %d socat STDIN TCP4:%s:%d", childPID, ip, spec.ChildPort)))
+		proto = "TCP"
 	case "udp":
-		cmd = exec.CommandContext(ctx,
-			"socat",
-			fmt.Sprintf("UDP-LISTEN:%d,bind=%s,reuseaddr,fork,rcvbuf=65536,sndbuf=65536", spec.ParentPort, ipStr),
-			fmt.Sprintf("EXEC:\"%s\",nofork",
-				fmt.Sprintf("nsenter -U -n --preserve-credentials -t %d socat STDIN UDP4:%s:%d", childPID, ip, spec.ChildPort)))
+		proto = "UDP"
 	}
+	cmd = exec.CommandContext(ctx,
+		"socat",
+		fmt.Sprintf("%s-LISTEN:%d,bind=%s,reuseaddr,fork,rcvbuf=65536,sndbuf=65536", proto, spec.ParentPort, ipStr),
+		fmt.Sprintf(`EXEC:"%s",nofork`,
+			fmt.Sprintf("nsenter -U -n --preserve-credentials -t %d socat STDIN %s4:%s", childPID, proto, hp)))
+
 	cmd.Env = os.Environ()
 	cmd.Stdout = logWriter
 	cmd.Stderr = logWriter