Add dummy proxy on port map

It is needed in cases when mapped port is already bound, or another application bind mapped port. All this will be undetected because we use iptables and not net.Listen. Signed-off-by: Alexander Morozov <lk4d4@docker.com>
moby · May 22, 2015 · f89a865 · f89a865
1 parent eb8273e
commit f89a865
Show file tree

Hide file tree

Showing 4 changed files with 133 additions and 10 deletions.
diff --git a/Makefile b/Makefile
@@ -45,7 +45,7 @@ run-tests:
 	    if ls $$dir/*.go &> /dev/null; then \
 		pushd . &> /dev/null ; \
 		cd $$dir ; \
-		$(shell which godep) go test ${INSIDECONTAINER} -test.parallel 3 -test.v -covermode=count -coverprofile=./profile.tmp ; \
+		$(shell which godep) go test ${INSIDECONTAINER} -test.race -test.parallel 3 -test.v -covermode=count -coverprofile=./profile.tmp ; \
 		ret=$$? ;\
 		if [ $$ret -ne 0 ]; then exit $$ret; fi ;\
 		popd &> /dev/null; \

diff --git a/portmapper/mapper.go b/portmapper/mapper.go
@@ -84,6 +84,8 @@ func (pm *PortMapper) Map(container net.Addr, hostIP net.IP, hostPort int, usePr
 
 		if useProxy {
 			m.userlandProxy = newProxy(proto, hostIP, allocatedHostPort, container.(*net.TCPAddr).IP, container.(*net.TCPAddr).Port)
+		} else {
+			m.userlandProxy = newDummyProxy(proto, hostIP, allocatedHostPort)
 		}
 	case *net.UDPAddr:
 		proto = "udp"
@@ -99,6 +101,8 @@ func (pm *PortMapper) Map(container net.Addr, hostIP net.IP, hostPort int, usePr
 
 		if useProxy {
 			m.userlandProxy = newProxy(proto, hostIP, allocatedHostPort, container.(*net.UDPAddr).IP, container.(*net.UDPAddr).Port)
+		} else {
+			m.userlandProxy = newDummyProxy(proto, hostIP, allocatedHostPort)
 		}
 	default:
 		return nil, ErrUnknownBackendAddressType
@@ -123,9 +127,7 @@ func (pm *PortMapper) Map(container net.Addr, hostIP net.IP, hostPort int, usePr
 
 	cleanup := func() error {
 		// need to undo the iptables rules before we return
-		if m.userlandProxy != nil {
-			m.userlandProxy.Stop()
-		}
+		m.userlandProxy.Stop()
 		pm.forward(iptables.Delete, m.proto, hostIP, allocatedHostPort, containerIP.String(), containerPort)
 		if err := pm.Allocator.ReleasePort(hostIP, m.proto, allocatedHostPort); err != nil {
 			return err
@@ -134,13 +136,11 @@ func (pm *PortMapper) Map(container net.Addr, hostIP net.IP, hostPort int, usePr
 		return nil
 	}
 
-	if m.userlandProxy != nil {
-		if err := m.userlandProxy.Start(); err != nil {
-			if err := cleanup(); err != nil {
-				return nil, fmt.Errorf("Error during port allocation cleanup: %v", err)
-			}
-			return nil, err
+	if err := m.userlandProxy.Start(); err != nil {
+		if err := cleanup(); err != nil {
+			return nil, fmt.Errorf("Error during port allocation cleanup: %v", err)
 		}
+		return nil, err
 	}
 
 	pm.currentMappings[key] = m

diff --git a/portmapper/mapper_test.go b/portmapper/mapper_test.go
@@ -2,6 +2,7 @@ package portmapper
 
 import (
 	"net"
+	"strings"
 	"testing"
 
 	"github.com/docker/libnetwork/iptables"
@@ -194,3 +195,77 @@ func TestMapAllPortsSingleInterface(t *testing.T) {
 		hosts = []net.Addr{}
 	}
 }
+
+func TestMapTCPDummyListen(t *testing.T) {
+	pm := New()
+	dstIP := net.ParseIP("0.0.0.0")
+	dstAddr := &net.TCPAddr{IP: dstIP, Port: 80}
+
+	// no-op for dummy
+	srcAddr := &net.TCPAddr{Port: 1080, IP: net.ParseIP("172.16.0.1")}
+
+	addrEqual := func(addr1, addr2 net.Addr) bool {
+		return (addr1.Network() == addr2.Network()) && (addr1.String() == addr2.String())
+	}
+
+	if host, err := pm.Map(srcAddr, dstIP, 80, false); err != nil {
+		t.Fatalf("Failed to allocate port: %s", err)
+	} else if !addrEqual(dstAddr, host) {
+		t.Fatalf("Incorrect mapping result: expected %s:%s, got %s:%s",
+			dstAddr.String(), dstAddr.Network(), host.String(), host.Network())
+	}
+	if _, err := net.Listen("tcp", "0.0.0.0:80"); err == nil {
+		t.Fatal("Listen on mapped port without proxy should fail")
+	} else {
+		if !strings.Contains(err.Error(), "address already in use") {
+			t.Fatalf("Error should be about address already in use, got %v", err)
+		}
+	}
+	if _, err := net.Listen("tcp", "0.0.0.0:81"); err != nil {
+		t.Fatal(err)
+	}
+	if host, err := pm.Map(srcAddr, dstIP, 81, false); err == nil {
+		t.Fatalf("Bound port shouldn't be allocated, but it was on: %v", host)
+	} else {
+		if !strings.Contains(err.Error(), "address already in use") {
+			t.Fatalf("Error should be about address already in use, got %v", err)
+		}
+	}
+}
+
+func TestMapUDPDummyListen(t *testing.T) {
+	pm := New()
+	dstIP := net.ParseIP("0.0.0.0")
+	dstAddr := &net.UDPAddr{IP: dstIP, Port: 80}
+
+	// no-op for dummy
+	srcAddr := &net.UDPAddr{Port: 1080, IP: net.ParseIP("172.16.0.1")}
+
+	addrEqual := func(addr1, addr2 net.Addr) bool {
+		return (addr1.Network() == addr2.Network()) && (addr1.String() == addr2.String())
+	}
+
+	if host, err := pm.Map(srcAddr, dstIP, 80, false); err != nil {
+		t.Fatalf("Failed to allocate port: %s", err)
+	} else if !addrEqual(dstAddr, host) {
+		t.Fatalf("Incorrect mapping result: expected %s:%s, got %s:%s",
+			dstAddr.String(), dstAddr.Network(), host.String(), host.Network())
+	}
+	if _, err := net.ListenUDP("udp", &net.UDPAddr{IP: dstIP, Port: 80}); err == nil {
+		t.Fatal("Listen on mapped port without proxy should fail")
+	} else {
+		if !strings.Contains(err.Error(), "address already in use") {
+			t.Fatalf("Error should be about address already in use, got %v", err)
+		}
+	}
+	if _, err := net.ListenUDP("udp", &net.UDPAddr{IP: dstIP, Port: 81}); err != nil {
+		t.Fatal(err)
+	}
+	if host, err := pm.Map(srcAddr, dstIP, 81, false); err == nil {
+		t.Fatalf("Bound port shouldn't be allocated, but it was on: %v", host)
+	} else {
+		if !strings.Contains(err.Error(), "address already in use") {
+			t.Fatalf("Error should be about address already in use, got %v", err)
+		}
+	}
+}
diff --git a/portmapper/proxy.go b/portmapper/proxy.go
@@ -3,6 +3,7 @@ package portmapper
 import (
 	"flag"
 	"fmt"
+	"io"
 	"io/ioutil"
 	"log"
 	"net"
@@ -159,3 +160,50 @@ func (p *proxyCommand) Stop() error {
 	}
 	return nil
 }
+
+// dummyProxy just listen on some port, it is needed to prevent accidental
+// port allocations on bound port, because without userland proxy we using
+// iptables rules and not net.Listen
+type dummyProxy struct {
+	listener io.Closer
+	addr     net.Addr
+}
+
+func newDummyProxy(proto string, hostIP net.IP, hostPort int) userlandProxy {
+	switch proto {
+	case "tcp":
+		addr := &net.TCPAddr{IP: hostIP, Port: hostPort}
+		return &dummyProxy{addr: addr}
+	case "udp":
+		addr := &net.UDPAddr{IP: hostIP, Port: hostPort}
+		return &dummyProxy{addr: addr}
+	}
+	return nil
+}
+
+func (p *dummyProxy) Start() error {
+	switch addr := p.addr.(type) {
+	case *net.TCPAddr:
+		l, err := net.ListenTCP("tcp", addr)
+		if err != nil {
+			return err
+		}
+		p.listener = l
+	case *net.UDPAddr:
+		l, err := net.ListenUDP("udp", addr)
+		if err != nil {
+			return err
+		}
+		p.listener = l
+	default:
+		return fmt.Errorf("Unknown addr type: %T", p.addr)
+	}
+	return nil
+}
+
+func (p *dummyProxy) Stop() error {
+	if p.listener != nil {
+		return p.listener.Close()
+	}
+	return nil
+}