Skip to content

Fixes potential nil return from multiplexer.Conn.RemoteAddr()#54578

Merged
eriktate merged 1 commit intomasterfrom
eriktate/check-for-nil-proxyline-source
May 7, 2025
Merged

Fixes potential nil return from multiplexer.Conn.RemoteAddr()#54578
eriktate merged 1 commit intomasterfrom
eriktate/check-for-nil-proxyline-source

Conversation

@eriktate
Copy link
Copy Markdown
Contributor

@eriktate eriktate commented May 6, 2025

This PR fixes a regression that made it possible for multiplexer.Conn.RemoteAddr() to return a nil net.Addr. This bug was introduced by the addition of the proxy_protocol_allow_downgrade feature and happens when a genuine class E source address appears in a PROXY header. The existing behavior assumed we would find an original source address TLV whenever a class E source address was present since they're technically reserved for experimental purposes. However GKE has a feature that allows for networking using class E addresses. I'm also fairly certain this same behavior would have appeared in conjunction with CloudFlare's pseudo IPv4 feature which was the original inspiration for proxy_protocol_allow_downgrade. The fix is to fallback to the proxy line's source address when an original address isn't found in the TLVs.

changelog: Fixed an issue preventing connections due to missing client IPs when using class E address space with GKE or CloudFlare pseudo IPv4 forward headers.

@github-actions github-actions Bot requested review from EdwardDowling and zmb3 May 6, 2025 20:50
@eriktate eriktate force-pushed the eriktate/check-for-nil-proxyline-source branch from c876933 to 216a315 Compare May 6, 2025 21:33
Comment thread lib/multiplexer/wrapper_test.go Outdated
@public-teleport-github-review-bot public-teleport-github-review-bot Bot removed the request for review from EdwardDowling May 6, 2025 22:48
@eriktate eriktate force-pushed the eriktate/check-for-nil-proxyline-source branch from 216a315 to b32c0dd Compare May 7, 2025 15:26
@eriktate eriktate enabled auto-merge May 7, 2025 15:26
original source address in a ProxyLine's TLVs
@eriktate eriktate force-pushed the eriktate/check-for-nil-proxyline-source branch from b32c0dd to e276abf Compare May 7, 2025 15:27
@eriktate eriktate added this pull request to the merge queue May 7, 2025
Merged via the queue into master with commit 19cdf00 May 7, 2025
40 checks passed
@eriktate eriktate deleted the eriktate/check-for-nil-proxyline-source branch May 7, 2025 16:09
@backport-bot-workflows
Copy link
Copy Markdown
Contributor

@eriktate See the table below for backport results.

Branch Result
branch/v16 Create PR
branch/v17 Create PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants