diff --git a/pkg/tcpip/ports/BUILD b/pkg/tcpip/ports/BUILD index b3f752062..fbd6ae08c 100644 --- a/pkg/tcpip/ports/BUILD +++ b/pkg/tcpip/ports/BUILD @@ -13,7 +13,6 @@ go_library( ], visibility = ["//visibility:public"], deps = [ - "//pkg/atomicbitops", "//pkg/rand", "//pkg/sync", "//pkg/tcpip", diff --git a/pkg/tcpip/ports/ports.go b/pkg/tcpip/ports/ports.go index 1642e142f..1e9040e4e 100644 --- a/pkg/tcpip/ports/ports.go +++ b/pkg/tcpip/ports/ports.go @@ -19,7 +19,6 @@ package ports import ( "math" - "gvisor.dev/gvisor/pkg/atomicbitops" "gvisor.dev/gvisor/pkg/rand" "gvisor.dev/gvisor/pkg/sync" "gvisor.dev/gvisor/pkg/tcpip" @@ -228,13 +227,6 @@ type PortManager struct { ephemeralMu sync.RWMutex firstEphemeral uint16 numEphemeral uint16 - - // hint is used to pick ports ephemeral ports in a stable order for - // a given port offset. - // - // hint must be accessed using the portHint/incPortHint helpers. - // TODO(gvisor.dev/issue/940): S/R this field. - hint atomicbitops.Uint32 } // NewPortManager creates new PortManager. @@ -264,38 +256,12 @@ func (pm *PortManager) PickEphemeralPort(rng rand.RNG, testPort PortTester) (por return pickEphemeralPort(rng.Uint32(), firstEphemeral, numEphemeral, testPort) } -// portHint atomically reads and returns the pm.hint value. -func (pm *PortManager) portHint() uint32 { - return pm.hint.Load() -} - -// incPortHint atomically increments pm.hint by 1. -func (pm *PortManager) incPortHint() { - pm.hint.Add(1) -} - -// PickEphemeralPortStable starts at the specified offset + pm.portHint and -// iterates over all ephemeral ports, allowing the caller to decide whether a -// given port is suitable for its needs and stopping when a port is found or an -// error occurs. -func (pm *PortManager) PickEphemeralPortStable(offset uint32, testPort PortTester) (port uint16, err tcpip.Error) { - pm.ephemeralMu.RLock() - firstEphemeral := pm.firstEphemeral - numEphemeral := pm.numEphemeral - pm.ephemeralMu.RUnlock() - - p, err := pickEphemeralPort(pm.portHint()+offset, firstEphemeral, numEphemeral, testPort) - if err == nil { - pm.incPortHint() - } - return p, err -} - // pickEphemeralPort starts at the offset specified from the FirstEphemeral port // and iterates over the number of ports specified by count and allows the // caller to decide whether a given port is suitable for its needs, and stopping // when a port is found or an error occurs. func pickEphemeralPort(offset uint32, first, count uint16, testPort PortTester) (port uint16, err tcpip.Error) { + // This implements Algorithm 1 as per RFC 6056 Section 3.3.1. for i := uint32(0); i < uint32(count); i++ { port := uint16(uint32(first) + (offset+i)%uint32(count)) ok, err := testPort(port) diff --git a/pkg/tcpip/ports/ports_test.go b/pkg/tcpip/ports/ports_test.go index c688bfd8f..49002fdb5 100644 --- a/pkg/tcpip/ports/ports_test.go +++ b/pkg/tcpip/ports/ports_test.go @@ -16,7 +16,6 @@ package ports import ( "math" - "math/rand" "testing" "github.com/google/go-cmp/cmp" @@ -434,70 +433,6 @@ func TestPickEphemeralPort(t *testing.T) { } } -func TestPickEphemeralPortStable(t *testing.T) { - const ( - firstEphemeral = 32000 - numEphemeralPorts = 1000 - ) - - for _, test := range []struct { - name string - f func(port uint16) (bool, tcpip.Error) - wantErr tcpip.Error - wantPort uint16 - }{ - { - name: "no-port-available", - f: func(port uint16) (bool, tcpip.Error) { - return false, nil - }, - wantErr: &tcpip.ErrNoPortAvailable{}, - }, - { - name: "port-tester-error", - f: func(port uint16) (bool, tcpip.Error) { - return false, &tcpip.ErrBadBuffer{} - }, - wantErr: &tcpip.ErrBadBuffer{}, - }, - { - name: "only-port-16042-available", - f: func(port uint16) (bool, tcpip.Error) { - if port == firstEphemeral+42 { - return true, nil - } - return false, nil - }, - wantPort: firstEphemeral + 42, - }, - { - name: "only-port-under-16000-available", - f: func(port uint16) (bool, tcpip.Error) { - if port < firstEphemeral { - return true, nil - } - return false, nil - }, - wantErr: &tcpip.ErrNoPortAvailable{}, - }, - } { - t.Run(test.name, func(t *testing.T) { - pm := NewPortManager() - if err := pm.SetPortRange(firstEphemeral, firstEphemeral+numEphemeralPorts); err != nil { - t.Fatalf("failed to set ephemeral port range: %s", err) - } - portOffset := uint32(rand.Int31n(int32(numEphemeralPorts))) - port, err := pm.PickEphemeralPortStable(portOffset, test.f) - if diff := cmp.Diff(test.wantErr, err); diff != "" { - t.Fatalf("unexpected error from PickEphemeralPort(..), (-want, +got):\n%s", diff) - } - if port != test.wantPort { - t.Errorf("got PickEphemeralPort(..) = (%d, nil); want (%d, nil)", port, test.wantPort) - } - }) - } -} - // TestOverflow addresses b/183593432, wherein an overflowing uint16 causes a // port allocation failure. func TestOverflow(t *testing.T) { diff --git a/pkg/tcpip/transport/tcp/endpoint.go b/pkg/tcpip/transport/tcp/endpoint.go index 5ce0ab9ab..7eada75e4 100644 --- a/pkg/tcpip/transport/tcp/endpoint.go +++ b/pkg/tcpip/transport/tcp/endpoint.go @@ -16,7 +16,6 @@ package tcp import ( "container/heap" - "encoding/binary" "fmt" "io" "math" @@ -29,7 +28,6 @@ import ( "gvisor.dev/gvisor/pkg/sleep" "gvisor.dev/gvisor/pkg/sync" "gvisor.dev/gvisor/pkg/tcpip" - "gvisor.dev/gvisor/pkg/tcpip/hash/jenkins" "gvisor.dev/gvisor/pkg/tcpip/header" "gvisor.dev/gvisor/pkg/tcpip/ports" "gvisor.dev/gvisor/pkg/tcpip/seqnum" @@ -2246,28 +2244,6 @@ func (e *endpoint) registerEndpoint(addr tcpip.FullAddress, netProto tcpip.Netwo // endpoint would be trying to connect to itself). sameAddr := e.TransportEndpointInfo.ID.LocalAddress == e.TransportEndpointInfo.ID.RemoteAddress - // Calculate a port offset based on the destination IP/port and - // src IP to ensure that for a given tuple (srcIP, destIP, - // destPort) the offset used as a starting point is the same to - // ensure that we can cycle through the port space effectively. - portBuf := make([]byte, 2) - binary.LittleEndian.PutUint16(portBuf, e.ID.RemotePort) - - h := jenkins.Sum32(e.protocol.portOffsetSecret) - for _, s := range [][]byte{ - e.ID.LocalAddress.AsSlice(), - e.ID.RemoteAddress.AsSlice(), - portBuf, - } { - // Per io.Writer.Write: - // - // Write must return a non-nil error if it returns n < len(p). - if _, err := h.Write(s); err != nil { - panic(err) - } - } - portOffset := h.Sum32() - var twReuse tcpip.TCPTimeWaitReuseOption if err := e.stack.TransportProtocolOption(ProtocolNumber, &twReuse); err != nil { panic(fmt.Sprintf("e.stack.TransportProtocolOption(%d, %#v) = %s", ProtocolNumber, &twReuse, err)) @@ -2284,7 +2260,7 @@ func (e *endpoint) registerEndpoint(addr tcpip.FullAddress, netProto tcpip.Netwo } bindToDevice := tcpip.NICID(e.ops.GetBindToDevice()) - if _, err := e.stack.PickEphemeralPortStable(portOffset, func(p uint16) (bool, tcpip.Error) { + if _, err := e.stack.PickEphemeralPort(e.stack.SecureRNG(), func(p uint16) (bool, tcpip.Error) { if sameAddr && p == e.TransportEndpointInfo.ID.RemotePort { return false, nil }