pgwire: refactor+simplify the connection set-up code #43837
Merged
craig[bot] merged 2 commits intocockroachdb:masterfrom Jan 9, 2020
Merged
pgwire: refactor+simplify the connection set-up code #43837craig[bot] merged 2 commits intocockroachdb:masterfrom
craig[bot] merged 2 commits intocockroachdb:masterfrom
Conversation
make `handleAuthentication()` handle all of authentication, instead of sharing the logic with `conn.go`. Release note: None
Member
The early SQL connection start-up sequence was a bit too deep in nested conditionals. The conditionals historically grew as special cases on top of special cases; however taking a step back it turns out the overall flow of the start-up is quite simple and the code should reflect that. (This makes it both more readable and more easy to troubleshoot and extend.) No functional change. Release note: none
madelynnblue
approved these changes
Jan 9, 2020
Contributor
Author
|
thank you! bors r+ |
Contributor
Build failed (retrying...) |
craig bot
pushed a commit
that referenced
this pull request
Jan 9, 2020
43837: pgwire: refactor+simplify the connection set-up code r=knz a=knz Ahead of handling #31113. The early SQL connection start-up sequence was a bit too deep in nested conditionals. The conditionals historically grew as special cases on top of special cases; however taking a step back it turns out the overall flow of the start-up is quite simple and the code should reflect that. (This makes it both more readable and more easy to troubleshoot and extend.) No functional change. Co-authored-by: Raphael 'kena' Poss <knz@thaumogen.net>
Contributor
Build succeeded |
craig bot
pushed a commit
that referenced
this pull request
Jan 9, 2020
43848: pgwire: properly support unix socket clients with authentication r=knz a=knz Fixes #31113. cc @rolandcrosby (All commits except for the last from #43837 and #43843) tldr: this patch makes unix sockets more production-ready, by enabling clients to use unix sockets in secure mode and enabling authentication over unix sockets. **Motivation:** [Unix domain sockets](https://en.wikipedia.org/wiki/Unix_domain_socket) are a way for a server process to accept direct in-memory connections from processes running on the same machine as the server. They are simpler and faster as they avoid the TCP/IP stack entirely. Unix sockets are used both to provide a local client interface for administrator users operating the system; as well as setting up more complex authentication systems using the following topology: ``` client ^ | (non-standard protocol) | .----------|--------------(server machine)--------------------------. | v | | ,----------------------. ,--------------------. | | | connection proxy | | server process | | | | and transport-level |<--(unix socket)-->| and authentication | | | | security | | (e.g. crdb) | | | `----------------------' `--------------------' | `-------------------------------------------------------------------' ``` **Description of this change:** CockroachDB already supports setting up a unix socket for use by clients running on the same machine, subject to regular Unix permission checks. Prior to this patch, support for unix sockets was incomplete: - it would work properly for insecure nodes/clusters; however, ... - ... in secure mode, it would also require a TLS handshake over the unix socket, which is neither supported by pg clients nor meaningful: unix domain sockets have transport-level security already. This patch extends/fixes support for unix sockets as follows: - it properly accepts client connections without TLS over unix sockets; - it subjects incoming unix socket connections to the standard HBA rule-based authentication selection (via the cluster setting `server.host_based_authentication.configuration`); - it changes the default HBA configuration to contain a default `local` rule that requires password authentication, in a way compatible with PostgreSQL; - it un-hides the `--socket` parameter from the output of `cockroach start --help`. Release note (cli change): Connections using Unix sockets are now accepted even when the server is running in secure more. (Consult `cockroach start --help` for details about the `--socket` parameter.) Release note (security): Connections using unix sockets are now subject to the HBA rules defined via the setting `server.host_based_authentication.configuration`, in a way compatible with PostgreSQL: incoming unix connections match `local` rules, whereas incoming TCP connections match `host` rules. The default HBA configuration used when the cluster setting is empty is now: host all root all cert host all all all cert-password local all all password Co-authored-by: Raphael 'kena' Poss <knz@thaumogen.net>
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
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.
Ahead of handling #31113.
The early SQL connection start-up sequence was a bit too deep in
nested conditionals.
The conditionals historically grew as special cases on top of special
cases; however taking a step back it turns out the overall flow of the
start-up is quite simple and the code should reflect that. (This makes
it both more readable and more easy to troubleshoot and extend.)
No functional change.