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

[YSQL] Incorrect permission check logic #23266

Closed
1 task done
andrei-mart opened this issue Jul 23, 2024 · 1 comment
Closed
1 task done

[YSQL] Incorrect permission check logic #23266

andrei-mart opened this issue Jul 23, 2024 · 1 comment
Assignees
@pao214
Labels
2.20 Backport Required 2024.1 Backport Required area/ysql Yugabyte SQL (YSQL) kind/bug This issue is a bug priority/medium Medium priority issue

Comments

@andrei-mart
Copy link
Contributor

andrei-mart commented Jul 23, 2024 •
edited by jira bot
Loading

Jira Link: DB-12192

Description

https://github.com/yugabyte/yugabyte-db/blob/master/src/postgres/src/backend/utils/adt/yb_lockfuncs.c#L59

Trivial bug in the permission check condition, permission denied if user is (not superuser and not yb_db_admin), not if user is (not superuser or not yb_db_admin). Or means user has to be both to pass.
The superuser is a yb_db_admin, that's why the bug went undiscovered for so long.

In fact, since superuser is a yb_db_admin, it is sufficient to check if user is (not yb_db_admin) to deny the permission.

Issue Type

kind/bug

Warning: Please confirm that this issue does not contain any sensitive information

  • I confirm this issue does not contain any sensitive information.
@andrei-mart andrei-mart added area/ysql Yugabyte SQL (YSQL) status/awaiting-triage Issue awaiting triage labels Jul 23, 2024
@yugabyte-ci yugabyte-ci added kind/bug This issue is a bug priority/medium Medium priority issue labels Jul 23, 2024
@sushantrmishra sushantrmishra removed the status/awaiting-triage Issue awaiting triage label Jul 23, 2024
pao214 added a commit that referenced this issue Jul 25, 2024
@pao214
[#23266] YSQL: Only require YB Admin privileges to run pg_locks
399f165 
Summary:
Do not prevent YB Admins who are not superusers from running pg_locks.
Jira: DB-12192

Test Plan:
Jenkins

Test case to let yb_db_admin query pg_locks.

```
./yb_build.sh --java-test TestPgAuthorization#testPgLocksAuthorization
```

Backport-through: 2.20

Reviewers: smishra, amartsinchyk

Reviewed By: amartsinchyk

Subscribers: yql

Differential Revision: https://phorge.dev.yugabyte.com/D36780
pao214 added a commit that referenced this issue Jul 25, 2024
@pao214
[BACKPORT 2.20][#23266] YSQL: Only require YB Admin privileges to run…
0376da6 
… pg_locks

Summary:
Original commit: 399f165 / D36780
Do not prevent YB Admins who are not superusers from running pg_locks.
Jira: DB-12192

Test Plan:
Jenkins

Test case to let yb_db_admin query pg_locks.

```
./yb_build.sh --java-test TestPgAuthorization#testPgLocksAuthorization
```

Backport-through: 2.20

Reviewers: smishra, amartsinchyk

Reviewed By: amartsinchyk

Subscribers: yql

Tags: #jenkins-ready

Differential Revision: https://phorge.dev.yugabyte.com/D36829
pao214 added a commit that referenced this issue Jul 25, 2024
@pao214
[BACKPORT 2024.1][#23266] YSQL: Only require YB Admin privileges to r…
02852c2 
…un pg_locks

Summary:
Original commit: 399f165 / D36780
Do not prevent YB Admins who are not superusers from running pg_locks.
Jira: DB-12192

Test Plan:
Jenkins

Test case to let yb_db_admin query pg_locks.

```
./yb_build.sh --java-test TestPgAuthorization#testPgLocksAuthorization
```

Backport-through: 2.20

Reviewers: smishra, amartsinchyk

Reviewed By: amartsinchyk

Subscribers: yql

Tags: #jenkins-ready

Differential Revision: https://phorge.dev.yugabyte.com/D36828
@pao214
Copy link
Contributor

pao214 commented Jul 25, 2024

Landed on master, 2024.1, 2.20.

@pao214 pao214 closed this as completed Jul 25, 2024
jasonyb pushed a commit that referenced this issue Jul 25, 2024
@yugabyte-ci @jaki
[BACKPORT pg15-cherrypicks] all: Bulk port from master - 63
c7619d6 
Summary:
 5aa0c0a [PLAT-14078] Add local provider test for update databases
 cdd97f8 remove ea badge (#23276)
 2813d78 [PLAT-14156][PLAT-14323]: Move all UI Driven flags to INTERNAL and remove YBM key as its not used
 49523f5 [PLAT-14733]: Add support for OIDC attributes jwt_jwks_path and jwt_jwks_url
 b039d1a [PLAT-14366] Basic local provider test for master auto failover
 700fd49 [#23275] docdb: Fix missing home icon on master UI
 89e434e [#13254] YSQL: import pgtap v1.3.3
 1b3585f [doc][cdc] Updated diagrams (#23262)
 399f165 [#23266] YSQL: Only require YB Admin privileges to run pg_locks
 5a4bbd4 [#19954] docdb: Register both tablet split children atomically
 b4c4294 [PLAT-14617] Add support for numerical search and enable extra search fields forxCluster
 84fb7ad [#22449] YSQL: wal2json YB specific changes
 afe84d4 [#13254] YSQL: add pgtap to build
 adf3c54 [#23272] YSQL, ASH: Fix incorrect popping of query id from nested query ids stack
 3b42c2e [docs] Add syntax documentation for logical replication (#23270)

Test Plan: Jenkins: rebase: pg15-cherrypicks

Reviewers: jason, tfoucher

Tags: #jenkins-ready

Differential Revision: https://phorge.dev.yugabyte.com/D36841
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@pao214 pao214

Labels
2.20 Backport Required 2024.1 Backport Required area/ysql Yugabyte SQL (YSQL) kind/bug This issue is a bug priority/medium Medium priority issue
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@andrei-mart @sushantrmishra @pao214 @yugabyte-ci