[flozano]

Description

This change adds passive port multiplexing so different client IPs can share a narrow passive port range without binding a fresh listener per PASV/EPSV session.

AI disclaimer

This implementation was coded with GPT/Codex assistance.

Why

I am trying to replace AWS Transfer. AWS Transfer's FTP server uses by default a very narrow port range for passive ports (8192-8200). The clients that connect to my server, unfortunately, have hard-coded in their firewalls this port range, and all of them upload at fixed periods (same hour). With current model, only 9 simultaneous passive transfers would be possible. This PR tries to move the limit from 9 simultaneous total to 9 per client IP.

Changes

What changed:

• add Settings.PassiveTransferPortMultiplexing
• keep one real passive listener per port and reserve it per client IP
• dispatch accepted data connections to the matching pending reservation by source IP
• keep the existing behavior unchanged unless multiplexing is enabled
• add tests for different-IP sharing, same-IP exhaustion, and FTP-level passive exhaustion behavior

Behavioral notes:

• different client IPs can reuse the same passive port concurrently
• the same client IP still needs a distinct passive port per pending passive reservation, which is the protocol limit when demultiplexing by IP only

More on that protocol limit:

• a "pending passive reservation" is the interval after the server replies to PASV/EPSV and before the incoming data connection is matched and the reservation is released
• the dispatcher only knows the passive port and the source IP of the incoming TCP connection
• if two pending reservations share both the same passive port and the same observed source IP, there is no FTP protocol information available at accept time to decide which control session should receive that connection
• this is why IP-only multiplexing can safely share a passive port across different client IPs, but not across multiple simultaneous pending reservations from the same observed IP

Examples:

• client A at 203.0.113.10 sends PASV and gets passive port 8192; client B at 198.51.100.20 also sends PASV and also gets 8192; this works because the accepted data connections arrive from different source IPs and can be demultiplexed safely
• client A at 203.0.113.10 sends PASV and gets 8192; before opening the data connection, another control session that is also seen by the server as 203.0.113.10 sends PASV; that second session must get another passive port, for example 8193
• if the passive range is 8192-8200 and 10 workers are all behind the same NAT public IP 203.0.113.10, then only 9 concurrent pending passive reservations are possible; the 10th cannot be distinguished on the same passive port and must wait or fail with no available passive port
• sequential reuse is fine: if 203.0.113.10 gets 8192, opens the data connection, and that reservation is consumed and released, a later PASV from 203.0.113.10 can reuse 8192 again

Other caveats / issues:

• the effective limit is based on the observed source IP, so many internal clients behind the same NAT/public egress IP are treated as one client for multiplexing purposes
• this feature improves passive port reuse across different source IPs, but it does not increase concurrency for a single public IP beyond the configured passive range size
• FTPS/TLS does not remove this limitation, because the listener still has to dispatch the TCP connection before any per-session data-channel state can be used as a demultiplexing key
• this PR intentionally keeps the current behavior unchanged unless PassiveTransferPortMultiplexing is enabled

Validation:

• go test ./... -count=1 -timeout 90s

[codecov]

Codecov Report

❌ Patch coverage is 84.77509% with 44 lines in your changes missing coverage. Please review.
✅ Project coverage is 87.22%. Comparing base (c73a72e) to head (d79dd42).

Files with missing lines	Patch %	Lines
passive_multiplexer.go	86.51%	23 Missing and 6 partials ⚠️
transfer_pasv.go	81.42%	7 Missing and 6 partials ⚠️
server.go	50.00%	1 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #623      +/-   ##
==========================================
- Coverage   87.29%   87.22%   -0.08%     
==========================================
  Files          12       13       +1     
  Lines        1842     2090     +248     
==========================================
+ Hits         1608     1823     +215     
- Misses        156      180      +24     
- Partials       78       87       +9     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[fclairamb]

I like this a lot!

[drakkan]

@flozano thanks for this, but before adding this feature in SFTPGo I would like to check the following (potential) issues:

• With two clients behind the same NAT on the same port: A reserves, A's data connection is dispatched (reservation removed from the map), B reserves the same IP/port, A's transfer ends and Close() deletes B's.
• Close drains connCh then closes closedCh; if deliver passes its first closedCh check before that and reaches the second select after, both cases are ready and the send may win, leaving the client's connection in connCh with nobody to read or close it.

Let me know if I'm wrong a just did a quick look at the code. Thank you

[flozano]

@drakkan you were right on both, thanks for checking. I reproduced them on v0.32.6:

• Stale Close() deleting the newer reservation: it reproduces every time. With 8 sessions from one IP on 9 ports, a 12 s load test lost 23 transfers and completed about 41% fewer than with multiplexing off.
• deliver racing Close(): it left a connection stranded in 1,112 of 3,000,000 rounds under forced contention, so it's rare but real.

I also found a third one: when sessions from different IPs use a port for the first time at the same moment, they race to bind it, and the loser skips a port it could have shared, or gets a 421 if there's no other port.

#692 fixes all three, with regression tests and before/after numbers.

Two limitations remain, and they matter for SFTPGo, because connections are matched by raw source IP before WrapPassiveListener runs:

• IPMatchDisabled (passive_connections_security=1) has no effect: a data connection from a different IP is closed. I tested this one.
• With PROXY protocol, a data connection arriving through the proxy has the proxy's IP, so it won't match. This one comes from reading the code, I haven't tested it with a real proxy.

I'm making drakkan/sftpgo#2253 warn about both.