Re: Preventing non-superusers from altering session authorization
Joseph Koshakow <koshy44@gmail.com>
From: Joseph Koshakow <koshy44@gmail.com>
To: Nathan Bossart <nathandbossart@gmail.com>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2023-07-08T18:03:41Z
Lists: pgsql-hackers
Nathan Bossart <nathandbossart@gmail.com> wrote: > I see that RESET SESSION AUTHORIZATION > with a concurrently dropped role will FATAL with your patch but succeed > without it, which could be part of the reason. I didn't even realize it, but the change to superuser_arg() in v2 fixed this issue. The catalog lookup is only done if userid != AuthenticatedUserId. So RESET SESSION AUTHORIZATION with a concurrently dropped role will no longer FATAL. Thanks, Joe On Sat, Jul 1, 2023 at 11:33 AM Joseph Koshakow <koshy44@gmail.com> wrote: > >> That might be a good change? If the original authenticated role ID no > >> longer exists then we may want to return an error when trying to set > >> your session authorization to that role. > > > > I was curious why we don't block DROP ROLE if there are active sessions > for > > the role or terminate any such sessions as part of the command, and I > found > > this discussion from 2016: > > > > https://postgr.es/m/flat/56E87CD8.60007%40ohmu.fi > > Ah, that makes sense that we don't prevent DROP ROLE on active roles. > Though, we do error when you try and set your role or session > authorization to a dropped role. So erroring on RESET SESSION > AUTHORIZATION when the original role is dropped makes it consistent > with SET SESSION AUTHORIZATION TO <dropped-original-role>. On the other > hand it makes it inconsistent with RESET ROLE, which does not error on > a dropped role. > > - Joe Koshakow > > On Fri, Jun 23, 2023 at 1:54 PM Nathan Bossart <nathandbossart@gmail.com> > wrote: > >> On Thu, Jun 22, 2023 at 06:39:45PM -0400, Joseph Koshakow wrote: >> > On Wed, Jun 21, 2023 at 11:48 PM Nathan Bossart < >> nathandbossart@gmail.com> >> > wrote: >> >> I see that RESET SESSION AUTHORIZATION >> >> with a concurrently dropped role will FATAL with your patch but succeed >> >> without it, which could be part of the reason. >> > >> > That might be a good change? If the original authenticated role ID no >> > longer exists then we may want to return an error when trying to set >> > your session authorization to that role. >> >> I was curious why we don't block DROP ROLE if there are active sessions >> for >> the role or terminate any such sessions as part of the command, and I >> found >> this discussion from 2016: >> >> https://postgr.es/m/flat/56E87CD8.60007%40ohmu.fi >> >> -- >> Nathan Bossart >> Amazon Web Services: https://aws.amazon.com >> >
Commits
-
Fix privilege check for SET SESSION AUTHORIZATION.
- a0363ab7aafd 17.0 landed
-
Move privilege check for SET SESSION AUTHORIZATION.
- 9987a7bf3406 17.0 landed
-
Rename session_auth_is_superuser to current_role_is_superuser.
- 0fef87753828 17.0 landed