public inbox for [email protected]  
help / color / mirror / Atom feed
[PATCH v6 04/10] doc: Allow users to limit storage reserved by replication slots
4+ messages / 3 participants
[nested] [flat]

* [PATCH v6 04/10] doc: Allow users to limit storage reserved by replication slots
@ 2020-04-08 00:56 Justin Pryzby <[email protected]>
  0 siblings, 0 replies; 4+ messages in thread

From: Justin Pryzby @ 2020-04-08 00:56 UTC (permalink / raw)

commit c6550776394e25c1620bc8258427c8f1d448080d
Author: Alvaro Herrera <[email protected]>
---
 doc/src/sgml/config.sgml | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 7a7177c550..190157df0a 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -3900,9 +3900,9 @@ restore_command = 'copy "C:\\server\\archivedir\\%f" "%p"'  # Windows
         slots</link> are allowed to retain in the <filename>pg_wal</filename>
         directory at checkpoint time.
         If <varname>max_slot_wal_keep_size</varname> is -1 (the default),
-        replication slots retain unlimited amount of WAL files.  If
-        restart_lsn of a replication slot gets behind more than that megabytes
-        from the current LSN, the standby using the slot may no longer be able
+        replication slots may retain an unlimited amount of WAL files.  Otherwise, if
+        restart_lsn of a replication slot falls behind the current LSN by more
+        than the given size, the standby using the slot may no longer be able
         to continue replication due to removal of required WAL files. You
         can see the WAL availability of replication slots
         in <link linkend="view-pg-replication-slots">pg_replication_slots</link>.
-- 
2.17.0


--C+ts3FVlLX8+P6JN
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment; filename="v6-0005-doc-s-evade-avoid.patch"



^ permalink  raw  reply  [nested|flat] 4+ messages in thread

* Re: Preventing non-superusers from altering session authorization
@ 2023-06-22 03:48 Nathan Bossart <[email protected]>
  2023-06-22 22:39 ` Re: Preventing non-superusers from altering session authorization Joseph Koshakow <[email protected]>
  0 siblings, 1 reply; 4+ messages in thread

From: Nathan Bossart @ 2023-06-22 03:48 UTC (permalink / raw)
  To: Joseph Koshakow <[email protected]>; +Cc: PostgreSQL Hackers <[email protected]>

On Wed, Jun 21, 2023 at 04:28:43PM -0400, Joseph Koshakow wrote:
> +	roleTup = SearchSysCache1(AUTHOID, ObjectIdGetDatum(AuthenticatedUserId));
> +	if (!HeapTupleIsValid(roleTup))
> +		ereport(FATAL,
> +				(errcode(ERRCODE_INVALID_AUTHORIZATION_SPECIFICATION),
> +						errmsg("role with OID %u does not exist", AuthenticatedUserId)));
> +	rform = (Form_pg_authid) GETSTRUCT(roleTup);

I think "superuser_arg(AuthenticatedUserId)" would work here.

-- 
Nathan Bossart
Amazon Web Services: https://aws.amazon.com






^ permalink  raw  reply  [nested|flat] 4+ messages in thread

* Re: Preventing non-superusers from altering session authorization
  2023-06-22 03:48 Re: Preventing non-superusers from altering session authorization Nathan Bossart <[email protected]>
@ 2023-06-22 22:39 ` Joseph Koshakow <[email protected]>
  2023-06-23 17:54   ` Re: Preventing non-superusers from altering session authorization Nathan Bossart <[email protected]>
  0 siblings, 1 reply; 4+ messages in thread

From: Joseph Koshakow @ 2023-06-22 22:39 UTC (permalink / raw)
  To: Nathan Bossart <[email protected]>; +Cc: PostgreSQL Hackers <[email protected]>

On Wed, Jun 21, 2023 at 11:48 PM Nathan Bossart <[email protected]>
wrote:
>
>    On Wed, Jun 21, 2023 at 04:28:43PM -0400, Joseph Koshakow wrote:
>    > +     roleTup = SearchSysCache1(AUTHOID,
ObjectIdGetDatum(AuthenticatedUserId));
>    > +     if (!HeapTupleIsValid(roleTup))
>    > +             ereport(FATAL,
>    > +
(errcode(ERRCODE_INVALID_AUTHORIZATION_SPECIFICATION),
>    > +                                             errmsg("role with OID
%u does not exist", AuthenticatedUserId)));
>    > +     rform = (Form_pg_authid) GETSTRUCT(roleTup);
>
>    I think "superuser_arg(AuthenticatedUserId)" would work here.

Yep, that worked. I've attached a patch with this change.

> 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.

Thanks,
Joe Koshakow


Attachments:

  [text/x-patch] v2-0001-Prevent-non-superusers-from-altering-session-auth.patch (5.3K, ../../CAAvxfHdj5H88W99anUoFf4OaH+Oozs=e7kF1mhsegxpWOsvQFQ@mail.gmail.com/3-v2-0001-Prevent-non-superusers-from-altering-session-auth.patch)
  download | inline diff:
From 2b2817e3ea4f1541a781216afb7415435ca362a0 Mon Sep 17 00:00:00 2001
From: Joseph Koshakow <[email protected]>
Date: Thu, 15 Jun 2023 14:53:11 -0400
Subject: [PATCH] Prevent non-superusers from altering session auth

Previously, if a user connected with as a role that had the superuser
attribute, then they could always execute a SET SESSION AUTHORIZATION
statement for the duration of their session. Even if the role was
altered to set superuser to false, the user was still allowed to
execute SET SESSION AUTHORIZATION. This allowed them to set their
session role to some other superuser and effectively regain the
superuser privileges. They could even reset their own superuser
attribute to true.

This commit alters the privilege checks for SET SESSION AUTHORIZATION
such that a user can only execute it if the role they connected with is
currently a superuser. This prevents users from regaining superuser
privileges after it has been revoked.
---
 doc/src/sgml/ref/set_session_auth.sgml |  2 +-
 src/backend/utils/init/miscinit.c      | 21 +++++++--------------
 2 files changed, 8 insertions(+), 15 deletions(-)

diff --git a/doc/src/sgml/ref/set_session_auth.sgml b/doc/src/sgml/ref/set_session_auth.sgml
index f8fcafc194..94adab2468 100644
--- a/doc/src/sgml/ref/set_session_auth.sgml
+++ b/doc/src/sgml/ref/set_session_auth.sgml
@@ -51,7 +51,7 @@ RESET SESSION AUTHORIZATION
 
   <para>
    The session user identifier can be changed only if the initial session
-   user (the <firstterm>authenticated user</firstterm>) had the
+   user (the <firstterm>authenticated user</firstterm>) has the
    superuser privilege.  Otherwise, the command is accepted only if it
    specifies the authenticated user name.
   </para>
diff --git a/src/backend/utils/init/miscinit.c b/src/backend/utils/init/miscinit.c
index a604432126..4cef655703 100644
--- a/src/backend/utils/init/miscinit.c
+++ b/src/backend/utils/init/miscinit.c
@@ -467,7 +467,7 @@ ChangeToDataDir(void)
  * AuthenticatedUserId is determined at connection start and never changes.
  *
  * SessionUserId is initially the same as AuthenticatedUserId, but can be
- * changed by SET SESSION AUTHORIZATION (if AuthenticatedUserIsSuperuser).
+ * changed by SET SESSION AUTHORIZATION.
  * This is the ID reported by the SESSION_USER SQL function.
  *
  * OuterUserId is the current user ID in effect at the "outer level" (outside
@@ -492,8 +492,6 @@ static Oid	OuterUserId = InvalidOid;
 static Oid	CurrentUserId = InvalidOid;
 static const char *SystemUser = NULL;
 
-/* We also have to remember the superuser state of some of these levels */
-static bool AuthenticatedUserIsSuperuser = false;
 static bool SessionUserIsSuperuser = false;
 
 static int	SecurityRestrictionContext = 0;
@@ -731,6 +729,7 @@ InitializeSessionUserId(const char *rolename, Oid roleid)
 	HeapTuple	roleTup;
 	Form_pg_authid rform;
 	char	   *rname;
+	bool	   is_superuser;
 
 	/*
 	 * Don't do scans if we're bootstrapping, none of the system catalogs
@@ -770,10 +769,10 @@ InitializeSessionUserId(const char *rolename, Oid roleid)
 	rname = NameStr(rform->rolname);
 
 	AuthenticatedUserId = roleid;
-	AuthenticatedUserIsSuperuser = rform->rolsuper;
+	is_superuser = rform->rolsuper;
 
 	/* This sets OuterUserId/CurrentUserId too */
-	SetSessionUserId(roleid, AuthenticatedUserIsSuperuser);
+	SetSessionUserId(roleid, is_superuser);
 
 	/* Also mark our PGPROC entry with the authenticated user id */
 	/* (We assume this is an atomic store so no lock is needed) */
@@ -806,7 +805,7 @@ InitializeSessionUserId(const char *rolename, Oid roleid)
 		 * just document that the connection limit is approximate.
 		 */
 		if (rform->rolconnlimit >= 0 &&
-			!AuthenticatedUserIsSuperuser &&
+			!is_superuser &&
 			CountUserBackends(roleid) > rform->rolconnlimit)
 			ereport(FATAL,
 					(errcode(ERRCODE_TOO_MANY_CONNECTIONS),
@@ -818,7 +817,7 @@ InitializeSessionUserId(const char *rolename, Oid roleid)
 	SetConfigOption("session_authorization", rname,
 					PGC_BACKEND, PGC_S_OVERRIDE);
 	SetConfigOption("is_superuser",
-					AuthenticatedUserIsSuperuser ? "on" : "off",
+					is_superuser ? "on" : "off",
 					PGC_INTERNAL, PGC_S_DYNAMIC_DEFAULT);
 
 	ReleaseSysCache(roleTup);
@@ -841,7 +840,6 @@ InitializeSessionUserIdStandalone(void)
 	Assert(!OidIsValid(AuthenticatedUserId));
 
 	AuthenticatedUserId = BOOTSTRAP_SUPERUSERID;
-	AuthenticatedUserIsSuperuser = true;
 
 	SetSessionUserId(BOOTSTRAP_SUPERUSERID, true);
 }
@@ -893,11 +891,6 @@ system_user(PG_FUNCTION_ARGS)
  * that in case of multiple SETs in a single session, the original userid's
  * superuserness is what matters.  But we set the GUC variable is_superuser
  * to indicate whether the *current* session userid is a superuser.
- *
- * Note: this is not an especially clean place to do the permission check.
- * It's OK because the check does not require catalog access and can't
- * fail during an end-of-transaction GUC reversion, but we may someday
- * have to push it up into assign_session_authorization.
  */
 void
 SetSessionAuthorization(Oid userid, bool is_superuser)
@@ -906,7 +899,7 @@ SetSessionAuthorization(Oid userid, bool is_superuser)
 	Assert(OidIsValid(AuthenticatedUserId));
 
 	if (userid != AuthenticatedUserId &&
-		!AuthenticatedUserIsSuperuser)
+		!superuser_arg(AuthenticatedUserId))
 		ereport(ERROR,
 				(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
 				 errmsg("permission denied to set session authorization")));
-- 
2.34.1



^ permalink  raw  reply  [nested|flat] 4+ messages in thread

* Re: Preventing non-superusers from altering session authorization
  2023-06-22 03:48 Re: Preventing non-superusers from altering session authorization Nathan Bossart <[email protected]>
  2023-06-22 22:39 ` Re: Preventing non-superusers from altering session authorization Joseph Koshakow <[email protected]>
@ 2023-06-23 17:54   ` Nathan Bossart <[email protected]>
  0 siblings, 0 replies; 4+ messages in thread

From: Nathan Bossart @ 2023-06-23 17:54 UTC (permalink / raw)
  To: Joseph Koshakow <[email protected]>; +Cc: PostgreSQL Hackers <[email protected]>

On Thu, Jun 22, 2023 at 06:39:45PM -0400, Joseph Koshakow wrote:
> On Wed, Jun 21, 2023 at 11:48 PM Nathan Bossart <[email protected]>
> 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






^ permalink  raw  reply  [nested|flat] 4+ messages in thread


end of thread, other threads:[~2023-06-23 17:54 UTC | newest]

Thread overview: 4+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2020-04-08 00:56 [PATCH v6 04/10] doc: Allow users to limit storage reserved by replication slots Justin Pryzby <[email protected]>
2023-06-22 03:48 Re: Preventing non-superusers from altering session authorization Nathan Bossart <[email protected]>
2023-06-22 22:39 ` Re: Preventing non-superusers from altering session authorization Joseph Koshakow <[email protected]>
2023-06-23 17:54   ` Re: Preventing non-superusers from altering session authorization Nathan Bossart <[email protected]>

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox