Re: Questions about the new subscription parameter: password_required
Hi, how about having links (instead of just password_required=false) in alter_subscription.sgml and logical-replication.sgml so the user can navigate easily back to the CREATE SUBSCRIPTION parameters "password_required" part. For example, alter_subscription.sgml does this already for "two_phase" and "copy_data" but not for "password_required" (??) == Kind Regards, Peter Smith Fujitsu Australia On Sat, Oct 14, 2023 at 5:57 AM Jeff Davis wrote: > > On Fri, 2023-10-13 at 11:18 +0200, Benoit Lobréau wrote: > > I tried adding a section in "Logical Replication > Subscription" with > > the text you suggested and links in the CREATE / ALTER SUBSRIPTION > > commands. > > > > Is it better ? > > > Minor comments: > > * Use possessive "its" instead of the contraction, i.e. "before > transferring its ownership". > * I like that docs cover the case where a password is specified, but > the remote server doesn't require one. But the warning is the wrong > place to explain that, it should be in the main behavioral description > in 31.2.2. > * The warning feels like it has too many negatives and confused me at > first. I struggled myself a bit to come up with something less > confusing, but perhaps less is more: "Ensure that password_required is > properly set before transferring ownership of a subscription to a non- > superuser, otherwise the subscription may start to fail." > * Missing space in the warning after "password_required = true" > * Mention that a non-superuser-owned subscription with > password_required = false is partially locked down, e.g. the owner > can't change the connection string any more. > * 31.2.2 could probably be in the CREATE SUBSCRIPTION docs instead, > and linked from the ALTER docs. That's fairly normal for other commands > and I'm not sure there needs to be a separate section in logical > replication. I don't have a strong opinion here. > > I like the changes; this is a big improvement. I'll leave it to Robert > to commit it, so that he can ensure it matches how he expected the > feature to be used and sufficiently covers the behavioral aspects. > > Regards, > Jeff Davis > > >
Re: Questions about the new subscription parameter: password_required
On Fri, 2023-10-13 at 11:18 +0200, Benoit Lobréau wrote: > I tried adding a section in "Logical Replication > Subscription" with > the text you suggested and links in the CREATE / ALTER SUBSRIPTION > commands. > > Is it better ? Minor comments: * Use possessive "its" instead of the contraction, i.e. "before transferring its ownership". * I like that docs cover the case where a password is specified, but the remote server doesn't require one. But the warning is the wrong place to explain that, it should be in the main behavioral description in 31.2.2. * The warning feels like it has too many negatives and confused me at first. I struggled myself a bit to come up with something less confusing, but perhaps less is more: "Ensure that password_required is properly set before transferring ownership of a subscription to a non- superuser, otherwise the subscription may start to fail." * Missing space in the warning after "password_required = true" * Mention that a non-superuser-owned subscription with password_required = false is partially locked down, e.g. the owner can't change the connection string any more. * 31.2.2 could probably be in the CREATE SUBSCRIPTION docs instead, and linked from the ALTER docs. That's fairly normal for other commands and I'm not sure there needs to be a separate section in logical replication. I don't have a strong opinion here. I like the changes; this is a big improvement. I'll leave it to Robert to commit it, so that he can ensure it matches how he expected the feature to be used and sufficiently covers the behavioral aspects. Regards, Jeff Davis
Re: Questions about the new subscription parameter: password_required
On 9/23/23 03:57, Jeff Davis wrote: IIUC there is really one use case here, which is for superuser to define a subscription including the connection, and then change the owner to a non-superuser to actually run it (without being able to touch the connection string itself). I'd just document that in its own section, and mention a few caveats / mistakes to avoid. For instance, when the superuser is defining the connection, don't forget to set password_required=false, so that when you reassign to a non-superuser then the connection doesn't break. Hi, I tried adding a section in "Logical Replication > Subscription" with the text you suggested and links in the CREATE / ALTER SUBSRIPTION commands. Is it better ? -- Benoit Lobréau Consultant http://dalibo.comFrom f3f1b0ce8617971b173ea901c9735d8357955aa2 Mon Sep 17 00:00:00 2001 From: benoit Date: Thu, 12 Oct 2023 16:45:11 +0200 Subject: [PATCH] Doc patch for password_required Add documentation regarding non-superuser subscriptions with password_required=true. --- doc/src/sgml/logical-replication.sgml | 32 +++ doc/src/sgml/ref/alter_subscription.sgml | 3 ++- doc/src/sgml/ref/create_subscription.sgml | 3 ++- 3 files changed, 36 insertions(+), 2 deletions(-) diff --git a/doc/src/sgml/logical-replication.sgml b/doc/src/sgml/logical-replication.sgml index 3b2fa1129e..c3faaf88cd 100644 --- a/doc/src/sgml/logical-replication.sgml +++ b/doc/src/sgml/logical-replication.sgml @@ -329,6 +329,38 @@ + + Password required + + +password_required is a subscription parameter which specifies whether +connections to the publisher made as a result of this subscription must +use password authentication. This setting is ignored when the subscription +is owned by a superuser and set to true by default. + + + +If you want to have a subscription managed by a non-superuser with a connection string without +a password, you have to set password_required = false before transferring it's +ownership. In that case, only superusers can modify the subscription. + +test_pub=# CREATE SUBSCRIPTION test_sub CONNECTION 'host=somehost port=5432 user=repli dbname=tests_pub' PUBLICATION test_pub WITH (password_required=false); +CREATE SUBSCRIPTION +test_pub=# ALTER SUBSCRIPTION test_sub OWNER TO new_sub_owner; +ALTER SUBSCRIPTION + + + + + + If the connection string doesn't contain a password or the publication + side doesn't require a password during authentication and you have set + password_required = truebefore transferring ownership, + the subscription will start failing. + + + + Examples: Set Up Logical Replication diff --git a/doc/src/sgml/ref/alter_subscription.sgml b/doc/src/sgml/ref/alter_subscription.sgml index a85e04e4d6..e061c96937 100644 --- a/doc/src/sgml/ref/alter_subscription.sgml +++ b/doc/src/sgml/ref/alter_subscription.sgml @@ -50,7 +50,8 @@ ALTER SUBSCRIPTION name RENAME TO < CREATE permission on the database. In addition, to alter the owner, you must be able to SET ROLE to the new owning role. If the subscription has - password_required=false, only superusers can modify it. + password_required=false, only superusers can modify it + (See ). diff --git a/doc/src/sgml/ref/create_subscription.sgml b/doc/src/sgml/ref/create_subscription.sgml index c1bafbfa06..33ad3d12c7 100644 --- a/doc/src/sgml/ref/create_subscription.sgml +++ b/doc/src/sgml/ref/create_subscription.sgml @@ -361,7 +361,8 @@ CREATE SUBSCRIPTION subscription_nametrue. Only superusers can set - this value to false. + this value to false + (See ). -- 2.41.0
Re: Questions about the new subscription parameter: password_required
On 9/26/23 19:00, Jeff Davis wrote: + If the ownership of a subscription with password_required=true + is transferred to a non-superuser, they will gain full control over the subscription + but will not be able to modify it's connection string. I think you mean false, right? No, but I was wrong. At the beginning of the thread, I was surprised was even possible to change the ownership to a non-superuser because It shouldn't work and commands like ENABLE didn't complain in the terminal. Then Robert Haas explained to me that it's ok because the superuser can do whatever he wants. I came back to it later and somehow convinced myself it was working. Sorry. + If the ownership of a subscription with password_required=true + has been transferred to a non-superuser, it must be reverted to a superuser for + the DROP operation to succeed. That's only needed if the superuser transfers a subscription with password_required=true to a non-superuser and the connection string does not contain a password. In that case, the subscription is already in a failing state, not just for DROP. Ideally we'd have some other warning in the docs not to do that -- maybe in CREATE and ALTER. Yes, I forgot the connection string bit. Also, if the subscription is in that kind of failing state, there are other ways to get out of it as well, like disabling it and setting connection=none, then dropping i The code in for DropSubscription in src/backend/commands/subscriptioncmds.c tries to connect before testing if the slot is NONE / NULL. So it doesn't work to DISABLE the subscription and set the slot to NONE. Robert Haas proposed something in the following message but I am a little out of my depth here ... https://www.postgresql.org/message-id/af9435ae-18df-6a9e-2374-2de23009518c%40dalibo.com The whole thing is fairly delicate. As soon as you work slightly outside of the intended use, password_required starts causing unexpected things to happen. As I said earlier, I think the best thing to do is to just have a section that describes when to use password_required, what specific things you should do to satisfy that case, and what caveats you should avoid. Something like: "If you want to have a subscription using a connection string without a password managed by a non-superuser, then: [ insert SQL steps here ]. Warning: if the connection string doesn't contain a password, make sure to set password_required=false before transferring ownership, otherwise it will start failing." Ok, I will do it that way. Would you prefer this section to be in the ALTER SUBSCRIPTION on the CREATE SUBSCIPTION doc ? -- Benoit Lobréau Consultant http://dalibo.com
Re: Questions about the new subscription parameter: password_required
On Tue, Sep 26, 2023 at 1:00 PM Jeff Davis wrote: > As I said earlier, I think the best thing to do is to just have a > section that describes when to use password_required, what specific > things you should do to satisfy that case, and what caveats you should > avoid. Something like: > > "If you want to have a subscription using a connection string without > a password managed by a non-superuser, then: [ insert SQL steps here ]. > Warning: if the connection string doesn't contain a password, make sure > to set password_required=false before transferring ownership, otherwise > it will start failing." > > Documenting the behavior is good, too, but I find the behavior > difficult to document, so examples will help. Yeah, I think something like that could make sense, with an appropriate amount of word-smithing. -- Robert Haas EDB: http://www.enterprisedb.com
Re: Questions about the new subscription parameter: password_required
On Tue, 2023-09-26 at 18:21 +0200, Benoit Lobréau wrote: > On 9/26/23 16:27, Benoit Lobréau wrote: > > I will try to come up with a documentation patch. > > This is my attempt at a documentation patch. > + If the ownership of a subscription with password_required=true + is transferred to a non-superuser, they will gain full control over the subscription + but will not be able to modify it's connection string. I think you mean false, right? + If the ownership of a subscription with password_required=true + has been transferred to a non-superuser, it must be reverted to a superuser for + the DROP operation to succeed. That's only needed if the superuser transfers a subscription with password_required=true to a non-superuser and the connection string does not contain a password. In that case, the subscription is already in a failing state, not just for DROP. Ideally we'd have some other warning in the docs not to do that -- maybe in CREATE and ALTER. Also, if the subscription is in that kind of failing state, there are other ways to get out of it as well, like disabling it and setting connection=none, then dropping it. The whole thing is fairly delicate. As soon as you work slightly outside of the intended use, password_required starts causing unexpected things to happen. As I said earlier, I think the best thing to do is to just have a section that describes when to use password_required, what specific things you should do to satisfy that case, and what caveats you should avoid. Something like: "If you want to have a subscription using a connection string without a password managed by a non-superuser, then: [ insert SQL steps here ]. Warning: if the connection string doesn't contain a password, make sure to set password_required=false before transferring ownership, otherwise it will start failing." Documenting the behavior is good, too, but I find the behavior difficult to document, so examples will help. Regards, Jeff Davis
Re: Questions about the new subscription parameter: password_required
On 9/26/23 16:27, Benoit Lobréau wrote: I will try to come up with a documentation patch. This is my attempt at a documentation patch. -- Benoit Lobréau Consultant http://dalibo.comFrom a73baa91032fff37ef039168c276508553830f86 Mon Sep 17 00:00:00 2001 From: benoit Date: Tue, 26 Sep 2023 18:07:47 +0200 Subject: [PATCH] Doc patch for require_password Add documentation to ALTER / DROP SUBSCRIPTION regarding non-superuser subscriptions with require_password=true. --- doc/src/sgml/ref/alter_subscription.sgml | 3 +++ doc/src/sgml/ref/drop_subscription.sgml | 6 ++ 2 files changed, 9 insertions(+) diff --git a/doc/src/sgml/ref/alter_subscription.sgml b/doc/src/sgml/ref/alter_subscription.sgml index a85e04e4d6..0bbe7e2335 100644 --- a/doc/src/sgml/ref/alter_subscription.sgml +++ b/doc/src/sgml/ref/alter_subscription.sgml @@ -51,6 +51,9 @@ ALTER SUBSCRIPTION name RENAME TO < to alter the owner, you must be able to SET ROLE to the new owning role. If the subscription has password_required=false, only superusers can modify it. + If the ownership of a subscription with password_required=true + is transferred to a non-superuser, they will gain full control over the subscription + but will not be able to modify it's connection string. diff --git a/doc/src/sgml/ref/drop_subscription.sgml b/doc/src/sgml/ref/drop_subscription.sgml index 2a67bdea91..8ec743abd0 100644 --- a/doc/src/sgml/ref/drop_subscription.sgml +++ b/doc/src/sgml/ref/drop_subscription.sgml @@ -102,6 +102,12 @@ DROP SUBSCRIPTION [ IF EXISTS ] nameDROP SUBSCRIPTION cannot be executed inside a transaction block. + + + If the ownership of a subscription with password_required=true + has been transferred to a non-superuser, it must be reverted to a superuser for + the DROP operation to succeed. + -- 2.41.0
Re: Questions about the new subscription parameter: password_required
On 9/22/23 21:58, Robert Haas wrote I think that there normally shouldn't be any problem here, because if form->subpasswordrequired is true, we expect that the connection string should contain a password which the remote side actually uses, or we expect the subscription to be owned by the superuser. If neither of those things is the case, then either the superuser made a subscription that doesn't use a password owned by a non-superuser without fixing subpasswordrequired, or else the configuration on the remote side has changed so that it now doesn't use the password when formerly it did. In the first case, perhaps it would be fine to go ahead and drop the slot, but in the second case I don't think it's OK from a security point view, because the command is going to behave the same way no matter who executes the drop command, and a non-superuser who drops the slot shouldn't be permitted to rely on the postgres processes's identity to do anything on a remote node -- including dropping a relation slot. So I tentatively think that this behavior is correct. I must admin I hadn't considered the implication when the configuration on the remote side has changed and we use a non-superuser. I see how it could be problematic. I will try to come up with a documentation patch. Maybe you're altering it in a way that doesn't involve any connections or any changes to the connection string? There's no security issue if, say, you rename it. I looked at the code again. Indeed, of the ALTER SUBSCRIPTION commands, only ALTER SUBSCRIPTION .. CONNECTION uses walrcv_check_conninfo(). I checked the other thread (Re: [16+] subscription can end up in inconsistent state [1]) and will try the patch. Is it the thread you where refering to earlier ? [1] https://www.postgresql.org/message-id/flat/5dff4caf26f45ce224a33a5e18e110b93a351b2f.camel%40j-davis.com#ff4a06505de317b1ad436b8102a69446 -- Benoit Lobréau Consultant http://dalibo.com
Re: Questions about the new subscription parameter: password_required
On Fri, 2023-09-22 at 08:36 -0400, Robert Haas wrote: > On Fri, Sep 22, 2023 at 4:25 AM Benoit Lobréau > wrote: > > Can we consider adding something like this to clarify? > > > > """ > > This parameter is enforced when the CREATE SUBSCRIPTION or ALTER > > SUBSCRIPTION .. CONNECTION commands are executed. Therefore, it's > > possible to alter the ownership of a subscription with > > password_required=true to a non-superuser. > > """ > > I'm not sure of the exact wording, but there was another recent > thread > complaining about this being unclear, so it seems like some > clarification is needed. IIUC there is really one use case here, which is for superuser to define a subscription including the connection, and then change the owner to a non-superuser to actually run it (without being able to touch the connection string itself). I'd just document that in its own section, and mention a few caveats / mistakes to avoid. For instance, when the superuser is defining the connection, don't forget to set password_required=false, so that when you reassign to a non-superuser then the connection doesn't break. Regards, Jeff Davis
Re: Questions about the new subscription parameter: password_required
On Fri, Sep 22, 2023 at 10:59 AM Benoit Lobréau
wrote:
> You're right, it comes from the connection to drop the slot.
>
> But the code in for DropSubscription in
> src/backend/commands/subscriptioncmds.c tries to connect before testing
> if the slot is NONE / NULL. So it doesn't work to DISABLE the
> subscription and set the slot to NONE.
So I'm seeing this:
if (!slotname && rstates == NIL)
{
table_close(rel, NoLock);
return;
}
load_file("libpqwalreceiver", false);
wrconn = walrcv_connect(conninfo, true, must_use_password,
subname, );
That looks like it's intended to return if there's nothing to do, and
the comments say as much. But that (!slotname && rstates == NIL) test
looks sketchy. It seems like we should bail out early if *either*
!slotname *or* rstates == NIL, or for that matter if all of the
rstates have rstate->relid == 0 or rstate->state ==
SUBREL_STATE_SYNCDONE. Maybe we need to push setting up the connection
inside the foreach(lc, rstates) loop and do it only once we're sure we
want to do something. Or at least, I don't understand why we don't
bail out immediately in all cases where slotname is NULL, regardless
of rstates. Am I missing something here?
> Reading the code, I think I understand why the postgres user cannot drop
> the slot:
>
> * the owner is sub_owner (not a superuser)
> * and form->subpasswordrequired is true
>
> Should there be a test to check if the user executing the query is
> superuser? maybe it's handled differently? (I am not very familiar with
> the code).
I think that there normally shouldn't be any problem here, because if
form->subpasswordrequired is true, we expect that the connection
string should contain a password which the remote side actually uses,
or we expect the subscription to be owned by the superuser. If neither
of those things is the case, then either the superuser made a
subscription that doesn't use a password owned by a non-superuser
without fixing subpasswordrequired, or else the configuration on the
remote side has changed so that it now doesn't use the password when
formerly it did. In the first case, perhaps it would be fine to go
ahead and drop the slot, but in the second case I don't think it's OK
from a security point view, because the command is going to behave the
same way no matter who executes the drop command, and a non-superuser
who drops the slot shouldn't be permitted to rely on the postgres
processes's identity to do anything on a remote node -- including
dropping a relation slot. So I tentatively think that this behavior is
correct.
> I dont understand (yet?) why I can do ALTER SUBSCRIPTIONs after changing
> the ownership to an unpriviledged user (must_use_password should be true
> also in that case).
Maybe you're altering it in a way that doesn't involve any connections
or any changes to the connection string? There's no security issue if,
say, you rename it.
--
Robert Haas
EDB: http://www.enterprisedb.com
Re: Questions about the new subscription parameter: password_required
On 9/22/23 14:36, Robert Haas wrote:
I haven't checked this, but I think what's happening here is that DROP
SUBSCRIPTION tries to drop the remote slot, which requires making a
connection, which can trigger the error. You might get different
results if you did ALTER SUBSCRIPTION ... SET (slot_name = none)
first.
You're right, it comes from the connection to drop the slot.
But the code in for DropSubscription in
src/backend/commands/subscriptioncmds.c tries to connect before testing
if the slot is NONE / NULL. So it doesn't work to DISABLE the
subscription and set the slot to NONE.
1522 >~~~must_use_password = !superuser_arg(subowner) &&
form->subpasswordrequired;
...
1685 >~~~wrconn = walrcv_connect(conninfo, true, must_use_password,
1 >~~~>~~~>~~~>~~~>~~~>~~~>~~~subname, );
2 >~~~if (wrconn == NULL)
3 >~~~{
4 >~~~>~~~if (!slotname)
5 >~~~>~~~{
6 >~~~>~~~>~~~/* be tidy */
7 >~~~>~~~>~~~list_free(rstates);
8 >~~~>~~~>~~~table_close(rel, NoLock);
9 >~~~>~~~>~~~return;
10 >~~~>~~~}
11 >~~~>~~~else
12 >~~~>~~~{
13 >~~~>~~~>~~~ReportSlotConnectionError(rstates, subid, slotname,
err);
14 >~~~>~~~}
15 >~~~}
Reading the code, I think I understand why the postgres user cannot drop
the slot:
* the owner is sub_owner (not a superuser)
* and form->subpasswordrequired is true
Should there be a test to check if the user executing the query is
superuser? maybe it's handled differently? (I am not very familiar with
the code).
I dont understand (yet?) why I can do ALTER SUBSCRIPTIONs after changing
the ownership to an unpriviledged user (must_use_password should be true
also in that case).
--
Benoit Lobréau
Consultant
http://dalibo.com
Re: Questions about the new subscription parameter: password_required
On Fri, Sep 22, 2023 at 4:25 AM Benoit Lobréau wrote: > Can we consider adding something like this to clarify? > > """ > This parameter is enforced when the CREATE SUBSCRIPTION or ALTER > SUBSCRIPTION .. CONNECTION commands are executed. Therefore, it's > possible to alter the ownership of a subscription with > password_required=true to a non-superuser. > """ I'm not sure of the exact wording, but there was another recent thread complaining about this being unclear, so it seems like some clarification is needed. [ adding Jeff Davis in case he wants to weigh in here ] > Is the DROP SUBSCRIPTION failure in password_required.log expected for > both superuser and non-superuser? > > Is the DROP SUBSCRIPTION success in password_required2.log expected? > (i.e., with password_require=false, the only action a non-superuser can > perform is dropping the subscription. Since they own it, it is > understandable). I haven't checked this, but I think what's happening here is that DROP SUBSCRIPTION tries to drop the remote slot, which requires making a connection, which can trigger the error. You might get different results if you did ALTER SUBSCRIPTION ... SET (slot_name = none) first. -- Robert Haas EDB: http://www.enterprisedb.com
Re: Questions about the new subscription parameter: password_required
On 9/21/23 20:29, Robert Haas wrote: Which one? I see 2 ALTER SUBSCRIPTION ... OWNER commands in password_required.log and 1 more in password_required2.log, but they're all performed by the superuser, who is entitled to do anything they want. Thank you for taking the time to respond! I expected the ALTER SUBSCRIPTION ... OWNER command in password_required.log to fail because the end result of the command is a non-superuser owning a subscription with password_required=true, but the connection string has no password keyword, and the authentication scheme used doesn't require one anyway. The description of the password_required parameter doesn't clearly state what will fail or when the configuration is enforced (during CREATE SUBSCRIPTION and ALTER SUBSCRIPTION .. CONNECTION): """ https://www.postgresql.org/docs/16/sql-createsubscription.html Specifies whether connections to the publisher made as a result of this subscription must use password authentication. This setting is ignored when the subscription is owned by a superuser. The default is true. Only superusers can set this value to false. """ The description of pg_subscription.subpasswordrequired doesn't either: """ https://www.postgresql.org/docs/16/catalog-pg-subscription.html If true, the subscription will be required to specify a password for authentication """ Can we consider adding something like this to clarify? """ This parameter is enforced when the CREATE SUBSCRIPTION or ALTER SUBSCRIPTION .. CONNECTION commands are executed. Therefore, it's possible to alter the ownership of a subscription with password_required=true to a non-superuser. """ Is the DROP SUBSCRIPTION failure in password_required.log expected for both superuser and non-superuser? Is the DROP SUBSCRIPTION success in password_required2.log expected? (i.e., with password_require=false, the only action a non-superuser can perform is dropping the subscription. Since they own it, it is understandable). -- Benoit Lobréau Consultant http://dalibo.com
Re: Questions about the new subscription parameter: password_required
On Thu, Sep 21, 2023 at 8:03 AM Benoit Lobréau wrote: > I am confused about the new subscription parameter: password_required. > > I have two instances. The publisher's pg_hba is configured too allow > connections without authentication. On the subscriber, I have an > unprivileged user with pg_create_subscription and CREATE on the database. > > I tried using a superuser to create a subsciption without setting the > password_required parameter (the default is true). Then I changed the > owner to the unprivileged user. > > This user can use the subscription without limitation (including ALTER > SUBSCRIPTION ENABLE / DISABLE). The \dRs+ metacommand shows that a > password is requiered, which is not the case (or it is but it's not > enforced). > > Is this normal? I was expecting the ALTER SUBSCRIPTION .. OWNER to fail. Which one? I see 2 ALTER SUBSCRIPTION ... OWNER commands in password_required.log and 1 more in password_required2.log, but they're all performed by the superuser, who is entitled to do anything they want. The intention here is that most subscriptions will have passwordrequired=true. If such a subscription is owned by a superuser, the superuser can still use them however they like. If owned by a non-superuser, they can use them however they like *provided* that the password must be used to authenticate. If the superuser wants a non-superuser to be able to own a subscription that doesn't use a password, the superuser can set that up by configuring passwordrequired=false. But then that non-superuser is not allowed to further manipulate that subscription. -- Robert Haas EDB: http://www.enterprisedb.com
