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.

Sign up for GitHub

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

Jump to bottom

sql: users can connect even if they don't exist in insecure mode #20717

Closed
jordanlewis opened this issue Dec 14, 2017 · 4 comments · Fixed by #20800
Closed

sql: users can connect even if they don't exist in insecure mode #20717

jordanlewis opened this issue Dec 14, 2017 · 4 comments · Fixed by #20800
Assignees
@vivekmenezes
Labels
C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior.

Comments

@jordanlewis
Copy link
Member 

[11:12]% cockroach sql --insecure -u nosuchuser
# Welcome to the cockroach SQL interface.
# All statements must be terminated by a semicolon.
# To exit: CTRL + D.
#
# Client version: CockroachDB CCL v1.2-alpha.20170901-2084-ge457f9a19 (darwin amd64, built 2017/11/30 14:50:40, go1.9.1)
# Server version: CockroachDB CCL v1.2-alpha.20171211-123-g6172ba8d6 (darwin amd64, built 2017/12/11 22:32:52, go1.9.1)
# Cluster ID: b6057ca9-c5ef-47f4-800c-60ebc3749eff
#
# Enter \? for a brief introduction.
#
nosuchuser@:26257/> select 1;
+---+
| 1 |
+---+
| 1 |
+---+
(1 row)

This problem is not present in secure mode:

[11:14]% ./cockroach sql -u nosuchuser
# Welcome to the cockroach SQL interface.
# All statements must be terminated by a semicolon.
# To exit: CTRL + D.
#
Enter password:
Error: pq: user nosuchuser does not exist
Failed running "sql"
@jordanlewis jordanlewis added the C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. label Dec 14, 2017
@mberhault mberhault assigned vivekmenezes and unassigned mberhault Dec 14, 2017
@mberhault
Copy link
Contributor

@vivek for triage.

Given that anyone can login as root in insecure mode, that's probably not a very high-priority issue.

@jordanlewis
Copy link
Member Author

Perhaps not super high-priority, but it seems like a very easy win. See this forum post for an example of why it might be helpful: https://forum.cockroachlabs.com/t/deleting-existing-sessions/1216/4

@mberhault
Copy link
Contributor

True. But existing sessions are different from login checks. Let's keep #20718 for the former and leave this issue for user existence checks at login time in insecure mode.

@vivekmenezes
Copy link
Contributor

anyone think this is something we need to fix for 2.0?

benesch added a commit to benesch/cockroach that referenced this issue Dec 18, 2017
@benesch
pgwire: check for user existence in insecure mode, too
dfe2107 
Previously, only secure mode would validate that the user connecting
actually existed. This isn't a security bug--anyone can log in as root
in insecure mode, for example--but was confusing nonetheless.

Fixes cockroachdb#20717.

Release note (bug fix): It is no longer possible to log in as a
nonexistent user in insecure mode.
@benesch benesch mentioned this issue Dec 18, 2017
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@vivekmenezes vivekmenezes

Labels
C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

pgwire: check for user existence in insecure mode, too benesch/cockroach
4 participants
@jordanlewis @benesch @mberhault @vivekmenezes