Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change 'proxy_protocol' default mode and behavior #31622

Merged
merged 23 commits into from
Sep 12, 2023

Conversation

AntonAM
Copy link
Contributor

@AntonAM AntonAM commented Sep 8, 2023

Now by default proxy_protocol is working in 'unspecified' mode and in that mode we mark incoming connection with setting source port = 0 and don't allow IP pinned connections (based on port = 0).
Also on mode now requires PROXY header.

Implements RFD146 https://github.com/gravitational/teleport-private/pull/991

@AntonAM AntonAM added the networking Network connectivity features/problems label Sep 8, 2023
@AntonAM AntonAM force-pushed the anton/proxy-protocol-defaults branch 5 times, most recently from 613f15f to d1138ad Compare September 11, 2023 14:29
Now by default `proxy_protocol` is unspecified and in that mode
we don't allow IP pinned connections and mark incoming conection with setting
source port = 0.
'on' mode now requires PROXY header.
@AntonAM AntonAM force-pushed the anton/proxy-protocol-defaults branch from d1138ad to 56cb383 Compare September 11, 2023 14:47
@AntonAM AntonAM marked this pull request as ready for review September 11, 2023 14:47
@github-actions github-actions bot added database-access Database access related issues and PRs kubernetes-access size/md labels Sep 11, 2023
lib/config/configuration.go Outdated Show resolved Hide resolved
lib/config/fileconf.go Outdated Show resolved Hide resolved
lib/multiplexer/multiplexer.go Outdated Show resolved Hide resolved
lib/multiplexer/multiplexer.go Outdated Show resolved Hide resolved
lib/multiplexer/multiplexer.go Outdated Show resolved Hide resolved
lib/multiplexer/multiplexer.go Outdated Show resolved Hide resolved
lib/multiplexer/multiplexer.go Outdated Show resolved Hide resolved
lib/multiplexer/multiplexer.go Outdated Show resolved Hide resolved
lib/authz/permissions.go Outdated Show resolved Hide resolved
lib/authz/permissions.go Show resolved Hide resolved
@AntonAM AntonAM force-pushed the anton/proxy-protocol-defaults branch from ca31d22 to 2378999 Compare September 11, 2023 22:44
There cases when Telelport will call itself and it can go directly,
avoiding load balancer, so connection will not have unsigned PROXY header.
@AntonAM AntonAM force-pushed the anton/proxy-protocol-defaults branch from 2378999 to 6ac57d8 Compare September 11, 2023 22:49
lib/authz/permissions.go Outdated Show resolved Hide resolved
lib/authz/permissions.go Outdated Show resolved Hide resolved
lib/authz/permissions.go Show resolved Hide resolved
lib/config/configuration.go Show resolved Hide resolved
lib/multiplexer/multiplexer.go Outdated Show resolved Hide resolved
lib/multiplexer/multiplexer.go Show resolved Hide resolved
lib/service/kubernetes.go Outdated Show resolved Hide resolved
lib/service/service.go Show resolved Hide resolved
lib/srv/alpnproxy/proxy.go Outdated Show resolved Hide resolved
Copy link
Collaborator

@r0mant r0mant left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm once remaining comments are addressed

@@ -336,6 +336,10 @@ var ErrIPPinningMissing = trace.AccessDenied("pinned IP is required for the user
// ErrIPPinningMismatch is returned when user's pinned IP doesn't match observed IP.
var ErrIPPinningMismatch = trace.AccessDenied("pinned IP doesn't match observed client IP")

// ErrIPPinningNotAllowed is returned when user's pinned IP doesn't match observed IP.
var ErrIPPinningNotAllowed = trace.AccessDenied("IP pinning is not allowed for connections behind L4 load balancers with " +
"PROXY protocol enabled without explicitly setting 'proxy_protocol: on' in the proxy_service or auth_service config.")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should it be proxy_service and auth_service config?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Depends on the client setup, if there's another LB between Proxy and Auth then it can be both, although usually I think it will be only one. I'll make it and/or.

@AntonAM AntonAM added this pull request to the merge queue Sep 12, 2023
Merged via the queue into master with commit 56d6ec4 Sep 12, 2023
21 checks passed
@AntonAM AntonAM deleted the anton/proxy-protocol-defaults branch September 12, 2023 21:59
@public-teleport-github-review-bot

@AntonAM See the table below for backport results.

Branch Result
branch/v14 Create PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport/branch/v14 database-access Database access related issues and PRs kubernetes-access networking Network connectivity features/problems size/md
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants