Обсуждение: Questions about the new subscription parameter: password_required
Hi, 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. When I try to drop the subscription with the unprivileged user or a superuser, I get an error: ERROR: password is required DETAIL: Non-superuser cannot connect if the server does not request a password. HINT: Target server's authentication method must be changed, or set password_required=false in the subscription parameters. I have to re-change the subscription owner to the superuser, to be able to drop it. (See password_required.sql and password_required.log) I tried the same setup and changed the connexion string to add an application_name with the unprivileged user. In this case, I am reminded that I need a password. I tried modifying password_required to false with the superuser and modify the connexion string with the unprivilege user again. It fails with: HINT: Subscriptions with the password_required option set to false may only be created or modified by the superuser. I think that this part works as intended. I tried dropping the subscription with the unprivilege user: it works. Is it normal (given the previous message)? (see password_required2.sql and password_required2.log) -- Benoit Lobréau Consultant http://dalibo.com
Вложения
On Thu, Sep 21, 2023 at 8:03 AM Benoit Lobréau <benoit.lobreau@dalibo.com> 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
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
On Fri, Sep 22, 2023 at 4:25 AM Benoit Lobréau <benoit.lobreau@dalibo.com> 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
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, &err); 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
On Fri, Sep 22, 2023 at 10:59 AM Benoit Lobréau <benoit.lobreau@dalibo.com> 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, &err); 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
On Fri, 2023-09-22 at 08:36 -0400, Robert Haas wrote: > On Fri, Sep 22, 2023 at 4:25 AM Benoit Lobréau > <benoit.lobreau@dalibo.com> 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
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
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.com
Вложения
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 <literal>password_required=true</literal> + 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 <literal>password_required=true</literal> + 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
On Tue, Sep 26, 2023 at 1:00 PM Jeff Davis <pgsql@j-davis.com> 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
On 9/26/23 19:00, Jeff Davis wrote: > + If the ownership of a subscription with > <literal>password_required=true</literal> > + 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 > <literal>password_required=true</literal> > + 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
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.com
Вложения
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
Hi, how about having links (instead of just <literal>password_required=false</literal>) 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 <pgsql@j-davis.com> 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 > > >