MIME-Version: 1.0
References: <CAAvxfHc-HHzONQ2oXdvhFF9ayRnidPwK+fVBhRzaBWYYLVQL-g@mail.gmail.com>
 <20230622034818.GA1077640@nathanxps13> <CAAvxfHdj5H88W99anUoFf4OaH+Oozs=e7kF1mhsegxpWOsvQFQ@mail.gmail.com>
 <20230623175416.GA1268820@nathanxps13> <CAAvxfHfSbyHp62bqtovsEKu4M3oCQ-mehfq0OJ7Fk3OqkLwXtw@mail.gmail.com>
 <CAAvxfHfzRhoD98UXKE4DZpR5iwznMiehwsuqMzYhfQcnTwQ0Mw@mail.gmail.com>
In-Reply-To: <CAAvxfHfzRhoD98UXKE4DZpR5iwznMiehwsuqMzYhfQcnTwQ0Mw@mail.gmail.com>
From: Joseph Koshakow <koshy44@gmail.com>
Date: Sat, 8 Jul 2023 16:44:06 -0400
Message-ID: <CAAvxfHdJdAb4QVj5nGv5tz8=QuhvxE4Y4B8yRLDdAEz1doU9jA@mail.gmail.com>
Subject: Re: Preventing non-superusers from altering session authorization
To: Nathan Bossart <nathandbossart@gmail.com>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Content-Type: multipart/alternative; boundary="0000000000002d85d505ffffd071"
Archived-At: <https://www.postgresql.org/message-id/CAAvxfHdJdAb4QVj5nGv5tz8%3DQuhvxE4Y4B8yRLDdAEz1doU9jA%40mail.gmail.com>
Precedence: bulk

--0000000000002d85d505ffffd071
Content-Type: text/plain; charset="UTF-8"

I've discovered an issue with this approach. Let's say you have some
session open that is connected as a superuser and you run the following
commands:

  - CREATE ROLE r1 LOGIN SUPERUSER;
  - CREATE ROLE r2;
  - CREATE ROLE r3;

Then you open another session connected with user r1 and run the
following commands:

  - SET SESSION AUTHROIZATION r2;
  - BEGIN;
  - SET SESSION AUTHORIZATION r3;

Then in your original session run:

  - ALTER ROLE r1 NOSUPERUSER;

Finally in the r1 session run:

  - CREATE TABLE t ();

Postgres will then panic with the following logs:

2023-07-08 16:33:27.787 EDT [157141] ERROR:  permission denied for schema
public at character 14
2023-07-08 16:33:27.787 EDT [157141] STATEMENT:  CREATE TABLE t ();
2023-07-08 16:33:27.787 EDT [157141] ERROR:  permission denied to set
session authorization
2023-07-08 16:33:27.787 EDT [157141] WARNING:  AbortTransaction while in
ABORT state
2023-07-08 16:33:27.787 EDT [157141] ERROR:  permission denied to set
session authorization
2023-07-08 16:33:27.787 EDT [157141] WARNING:  AbortTransaction while in
ABORT state
2023-07-08 16:33:27.787 EDT [157141] ERROR:  permission denied to set
session authorization
2023-07-08 16:33:27.787 EDT [157141] WARNING:  AbortTransaction while in
ABORT state
2023-07-08 16:33:27.787 EDT [157141] ERROR:  permission denied to set
session authorization
2023-07-08 16:33:27.787 EDT [157141] PANIC:  ERRORDATA_STACK_SIZE exceeded
2023-07-08 16:33:27.882 EDT [156878] LOG:  server process (PID 157141) was
terminated by signal 6: Aborted
2023-07-08 16:33:27.882 EDT [156878] DETAIL:  Failed process was running:
CREATE TABLE t ();

I think the issue here is that if a session loses the ability to set
their session authorization in the middle of a transaction, then
rolling back the transaction may fail and cause the server to panic.
That's probably what the deleted comment mean when it said:

> * It's OK because the check does not require catalog access and can't
> * fail during an end-of-transaction GUC reversion

Interestingly, if the r1 session manually types `ROLLBACK` instead of
executing a command that fails, then everything is fine and there's no
panic. I'm not familiar enough with transaction handling to know why
there would be a difference there.

Thanks,
Joe Koshakow

--0000000000002d85d505ffffd071
Content-Type: text/html; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

<div dir=3D"ltr">I&#39;ve discovered an issue with this approach. Let&#39;s=
 say you have some<br>session open that is connected as a superuser and you=
 run the following commands:<br><br>=C2=A0 - CREATE ROLE r1 LOGIN SUPERUSER=
;<br>=C2=A0 - CREATE ROLE r2;<br>=C2=A0 - CREATE ROLE r3;<br><br>Then you o=
pen another session connected with user r1 and run the<br>following command=
s:<br><br>=C2=A0 - SET SESSION AUTHROIZATION r2;<br>=C2=A0 - BEGIN;<br>=C2=
=A0 - SET SESSION AUTHORIZATION r3;<br><br>Then in your original session ru=
n:<br><br>=C2=A0 - ALTER ROLE r1 NOSUPERUSER;<br><br>Finally in the r1 sess=
ion run:<br><br>=C2=A0 - CREATE TABLE t ();<br><br>Postgres will then panic=
 with the following logs:<br><br>2023-07-08 16:33:27.787 EDT [157141] ERROR=
: =C2=A0permission denied for schema public at character 14<br>2023-07-08 1=
6:33:27.787 EDT [157141] STATEMENT: =C2=A0CREATE TABLE t ();<br>2023-07-08 =
16:33:27.787 EDT [157141] ERROR: =C2=A0permission denied to set session aut=
horization<br>2023-07-08 16:33:27.787 EDT [157141] WARNING: =C2=A0AbortTran=
saction while in ABORT state<br>2023-07-08 16:33:27.787 EDT [157141] ERROR:=
 =C2=A0permission denied to set session authorization<br>2023-07-08 16:33:2=
7.787 EDT [157141] WARNING: =C2=A0AbortTransaction while in ABORT state<br>=
2023-07-08 16:33:27.787 EDT [157141] ERROR: =C2=A0permission denied to set =
session authorization<br>2023-07-08 16:33:27.787 EDT [157141] WARNING: =C2=
=A0AbortTransaction while in ABORT state<br>2023-07-08 16:33:27.787 EDT [15=
7141] ERROR: =C2=A0permission denied to set session authorization<br>2023-0=
7-08 16:33:27.787 EDT [157141] PANIC: =C2=A0ERRORDATA_STACK_SIZE exceeded<b=
r>2023-07-08 16:33:27.882 EDT [156878] LOG: =C2=A0server process (PID 15714=
1) was terminated by signal 6: Aborted<br>2023-07-08 16:33:27.882 EDT [1568=
78] DETAIL: =C2=A0Failed process was running: CREATE TABLE t ();<br><br>I t=
hink the issue here is that if a session loses the ability to set<br>their =
session authorization in the middle of a transaction, then<br>rolling back =
the transaction may fail and cause the server to panic.<br>That&#39;s proba=
bly what the deleted comment mean when it said:<br><br>&gt; * It&#39;s OK b=
ecause the check does not require catalog access and can&#39;t<br><div>&gt;=
 * fail during an end-of-transaction GUC reversion</div><div><br></div><div=
>Interestingly, if the r1 session manually types `ROLLBACK` instead of<br>e=
xecuting a command that fails, then everything is fine and there&#39;s no<b=
r>panic. I&#39;m not familiar enough with transaction handling to know why<=
br>there would be a difference there.</div><div><br></div><div>Thanks,</div=
><div>Joe Koshakow<br></div></div>

--0000000000002d85d505ffffd071--