fix: reject direct connect url when connect-backend is set - #870
Open
aroh3006 wants to merge 1 commit into
Open
Conversation
Connect fell through to ConnectWithURL for any request that was not a bookmark, even in connect-backend mode. The backend is supposed to be the only thing deciding which database a session gets, but a client could just POST a url to /api/connect and reach any database the pgweb host can dial, bypassing the backend entirely. Now Connect rejects the direct url path with the same errNotPermitted used for bookmarks-only mode when connect-backend is configured, leaving the backend-gated and bookmark paths untouched. Added a test that hits /api/connect with connect-backend set and a url pointing at an unresolvable host. It fails against the old code (the handler actually tries to dial the host and returns a DNS error) and passes with the fix (rejected immediately, no connection attempt).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #869.
Connect(pkg/api/api.go) only guards the direct-url path withbookmark_idandBookmarksOnly, so in connect-backend mode(
--sessions --connect-backend=... --connect-token=...) a client canstill POST a
urlform value to/api/connectand connect straightto it, bypassing the backend entirely. The backend exists specifically
to decide which database a session is allowed to reach, so this
defeats the whole point of the feature and lets any client reach
whatever host and database the pgweb process can dial.
Connectnow rejects the direct url path with the existingerrNotPermittedwhencommand.Opts.ConnectBackendis set, the sameway it already does for
BookmarksOnly. The backend-gated route(
ConnectWithBackend) and bookmark connections are untouched.Test. Added
Test_Connect_RejectsDirectUrlWhenConnectBackendConfigured,which sets
ConnectBackendand POSTs aurlpointing at anunresolvable host. Ran it against the current code first: the handler
actually attempted the connection and returned a DNS lookup error
(proving it really does reach out to an attacker-chosen host), so the
test fails there and passes with this change. Full
pkg/apisuitestill passes.