Thread: [HACKERS] Allow pg_dumpall to work without pg_authid
Hi,
I would like to work on a patch to accommodate restricted environments (such as AWS RDS Postgres) which don't allow pg_authid access since their definition of Superuser is just a regular user with extra permissions.
Would you consider a patch to add a flag to work around this restriction, Or, do you prefer that this be maintained outside core?
I could add a flag such as --avoid-pgauthid (am open to options) that skips pg_authid and uses pg_user (but essentially resets all User passwords). Mostly this is better than not being able to get the dump at all.
I have a fork here (a few weeks old):
Thanks
Robins Tharakan
--
-
robins
Greetings, * Robins Tharakan (tharakan@gmail.com) wrote: > I would like to work on a patch to accommodate restricted environments > (such as AWS RDS Postgres) which don't allow pg_authid access since their > definition of Superuser is just a regular user with extra permissions. > > Would you consider a patch to add a flag to work around this restriction, > Or, do you prefer that this be maintained outside core? > > I could add a flag such as --avoid-pgauthid (am open to options) that skips > pg_authid and uses pg_user (but essentially resets all User passwords). > Mostly this is better than not being able to get the dump at all. If anything, it should use pg_roles, not pg_user. I don't really like the "--avoid-pgauthid" option, but "--no-passwords" would probably work. In general, this seems like a reasonable thing to add support for. Thanks! Stephen
On Sun, 19 Feb 2017 at 10:08 Stephen Frost <sfrost@snowman.net> wrote:
If anything, it should use pg_roles, not pg_user.
I don't really like the "--avoid-pgauthid" option, but "--no-passwords"
would probably work.
Am sorry, I meant pg_roles (FWIW, the github URL given earlier uses pg_roles).
'--no-passwords' is a good alternative.
Would submit a patch soon.
-
Robins
--
-
robins
On 19 February 2017 at 17:02, Robins Tharakan <tharakan@gmail.com> wrote:
On Sun, 19 Feb 2017 at 10:08 Stephen Frost <sfrost@snowman.net> wrote:If anything, it should use pg_roles, not pg_user.
I don't really like the "--avoid-pgauthid" option, but "--no-passwords"
would probably work.'--no-passwords' is a good alternative.Would submit a patch soon.
After reviewing further, it seems that merely adding a password related workaround wouldn't suffice. Further --no-password is already an alias for -w, so that flag is effectively taken.
Since the main restriction with AWS RDS is the unavailability of pg_authid, probably that is a better basis to name the flag on.
Attaching a patch to add a new flag (--no-pgauthid) to pg_dumpall that can dump Globals without needing pg_authid. So the following works with AWS RDS Postgres databases.
pg_dumpall --no-pgauthid --globals-only > a.sql
I'll create a Commitfest entry, if there aren't many objections.
-
robins
Attachment
Robins, * Robins Tharakan (tharakan@gmail.com) wrote: > On 19 February 2017 at 17:02, Robins Tharakan <tharakan@gmail.com> wrote: > > On Sun, 19 Feb 2017 at 10:08 Stephen Frost <sfrost@snowman.net> wrote: > >> If anything, it should use pg_roles, not pg_user. > >> > >> I don't really like the "--avoid-pgauthid" option, but "--no-passwords" > >> would probably work. > >> > > '--no-passwords' is a good alternative. > > Would submit a patch soon. > > > After reviewing further, it seems that merely adding a password related > workaround wouldn't suffice. Further --no-password is already an alias for > -w, so that flag is effectively taken. Ah, yes, that makes --no-passwords a bad name. The other changes to use pg_roles instead of pg_authid when rolpassword isn't being used look like they should just be changed to use pg_roles instead of using one or the other. That should be an independent patch from the one which adds the option we are discussing. > Since the main restriction with AWS RDS is the unavailability of pg_authid, > probably that is a better basis to name the flag on. I don't like the idea of having the catalog name drive the option name. For one thing, there's been some discussion of using column-level privs on catalogs, which would actually make it such that pg_authid could be queried by regular users for the public columns. Perhaps --no-role-passwords instead? > Attaching a patch to add a new flag (--no-pgauthid) to pg_dumpall that can > dump Globals without needing pg_authid. So the following works with AWS RDS > Postgres databases. > > pg_dumpall --no-pgauthid --globals-only > a.sql Does that then work with a non-superuser account on a regular PG instance also? If not, I'd like to suggest that we consider follow-on patches to provide options for whatever else currently requires superuser on a regular install. > I'll create a Commitfest entry, if there aren't many objections. Yes, please do create a commitfest entry for this. Thanks! Stephen
Stephen,
The other changes to use pg_roles instead of pg_authid when rolpassword
isn't being used look like they should just be changed to use pg_roles
instead of using one or the other. That should be an independent patch
from the one which adds the option we are discussing.
Sure. Attached are 2 patches, of which 1 patch just replaces pg_authid with
pg_roles in pg_dumpall. The only exceptions there are buildShSecLabels()
& pg_catalog.binary_upgrade_set_next_pg_authid_oid() which I thought
should still use pg_authid.
Perhaps --no-role-passwords instead?
Makes Sense. The updated patch uses this name.
> pg_dumpall --no-pgauthid --globals-only > a.sql
Does that then work with a non-superuser account on a regular PG
instance also? If not, I'd like to suggest that we consider follow-on
patches to provide options for whatever else currently requires
superuser on a regular install.
If I understand that correctly, the answer is Yes. I didn't test all db objects,
but trying to do a pg_dumpall using a non-priviledge user does successfully
complete with all existing users dumped successfully.
pg_dumpall --globals-only --no-role-password > a.sql
Yes, please do create a commitfest entry for this.
Created Commitfest entry.
-
robins
robins
Attachment
On Wed, Feb 22, 2017 at 06:33:10PM +1100, Robins Tharakan wrote: > Stephen, > > On 20 February 2017 at 08:50, Stephen Frost <sfrost@snowman.net> wrote: > > > The other changes to use pg_roles instead of pg_authid when rolpassword > > isn't being used look like they should just be changed to use pg_roles > > instead of using one or the other. That should be an independent patch > > from the one which adds the option we are discussing. > > > > Sure. Attached are 2 patches, of which 1 patch just replaces > pg_authid with pg_roles in pg_dumpall. The only exceptions > there are buildShSecLabels() & > pg_catalog.binary_upgrade_set_next_pg_authid_oid() which I thought > should still use pg_authid. Thanks for doing this! That pg_dumpall didn't work with RDS Postgres (and possibly others) was a pretty large wart. In future, could you please leave patches uncompressed so they're easier to see in the archives? Best, David. -- David Fetter <david(at)fetter(dot)org> http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: david(dot)fetter(at)gmail(dot)com Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate
On 22 February 2017 at 07:33, Robins Tharakan <tharakan@gmail.com> wrote: > Stephen, > > On 20 February 2017 at 08:50, Stephen Frost <sfrost@snowman.net> wrote: >> >> The other changes to use pg_roles instead of pg_authid when rolpassword >> isn't being used look like they should just be changed to use pg_roles >> instead of using one or the other. That should be an independent patch >> from the one which adds the option we are discussing. > > > Sure. Attached are 2 patches, of which 1 patch just replaces pg_authid with > pg_roles in pg_dumpall. The only exceptions there are buildShSecLabels() > & pg_catalog.binary_upgrade_set_next_pg_authid_oid() which I thought > should still use pg_authid. Patch, and life, is simpler if we use just one or the other, IMHO. >> Perhaps --no-role-passwords instead? >> > Makes Sense. The updated patch uses this name. > >> >> > pg_dumpall --no-pgauthid --globals-only > a.sql >> >> Does that then work with a non-superuser account on a regular PG >> instance also? If not, I'd like to suggest that we consider follow-on >> patches to provide options for whatever else currently requires >> superuser on a regular install. >> > If I understand that correctly, the answer is Yes. I didn't test all db > objects, > but trying to do a pg_dumpall using a non-priviledge user does successfully > complete with all existing users dumped successfully. > > pg_dumpall --globals-only --no-role-password > a.sql It looks to me like you'd need to use --no-security-labels as well, in most cases since that accesses pg_authid also. The patch seemed to be missing substantial chunks of coding, so I've added those also. I've also added checks to prevent it running with other mutually exclusive options. Reworded doc changes also. Tested, but no tests added for this. -- Simon Riggs http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Attachment
On Sun, Feb 19, 2017 at 12:24 AM, Robins Tharakan <tharakan@gmail.com> wrote: > I would like to work on a patch to accommodate restricted environments (such > as AWS RDS Postgres) which don't allow pg_authid access since their > definition of Superuser is just a regular user with extra permissions. > > Would you consider a patch to add a flag to work around this restriction, > Or, do you prefer that this be maintained outside core? I am a little surprised that this patch has gotten such a good reception. We haven't in the past been all that willing to accept core changes for the benefit of forks of PostgreSQL; extensions, sure, but forks? Maybe we should take the view that Amazon has broken this and Amazon ought to fix it, rather than making it our job to (try to) work around their bugs. On the other hand, maybe that approach is narrow-minded. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On 26 February 2017 at 19:26, Robert Haas <robertmhaas@gmail.com> wrote:
I am a little surprised that this patch has gotten such a good
reception. We haven't in the past been all that willing to accept
core changes for the benefit of forks of PostgreSQL; extensions, sure,
but forks? Maybe we should take the view that Amazon has broken this
and Amazon ought to fix it, rather than making it our job to (try to)
work around their bugs.
(Disclaimer: I work at the said company, although don't represent them
in any way. This patch is in my personal capacity)
To confirm, this did originate by trying to accommodate a fork. But what
I can say is that this doesn't appear to be a bug; what they call
Super-User isn't effectively one.
Personally, I think it would be wise to also consider that this fork has
a very large user-base and for that user-base, this 'is' Postgres. Further,
case-by-case exceptions still should be considered for important issues
(here, this relates to lock-in).
Either way, I could pull-back the patch if more people object.
-
robins
On Sun, Feb 26, 2017 at 3:43 PM, Robins Tharakan <tharakan@gmail.com> wrote: > To confirm, this did originate by trying to accommodate a fork. But what > I can say is that this doesn't appear to be a bug; what they call > Super-User isn't effectively one. How's that not a bug? I mean, it's reasonable for someone to want to restrict the superuser in a cloud environment, but if they restrict it so much that you can't take a backup with standard tools, I'd say they should also patch the tools, though maybe a better idea would be to restrict the superuser a bit less. My basic concern here is that I don't want half of our tools to end up with special-purpose flags that serve only to unbreak Amazon RDS. That can't be a good solution to anything. It will lead to extra work for us and confusion for users about whether they should be using them. People are going to see this --avoid-pgauthid and wonder why it's there. And the next time Amazon RDS breaks something, we'll get a different flag someplace else to fix that problem. If we call them all --unbreak-amazon-rds instead of things like --avoid-pgauthid, then it will be clear when they need to be used, but why would we accept the job of working around the defects in Amazon's fork of PG? I'm already responsible for helping maintain one fork of PostgreSQL, but I'm not under any illusion that I get to do that by putting changes that make that easier into the community code base. Plus, for that work, I get paid. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
How's that not a bug? I mean, it's reasonable for someone to want to
restrict the superuser in a cloud environment, but if they restrict it
so much that you can't take a backup with standard tools, I'd say they
should also patch the tools, though maybe a better idea would be to
restrict the superuser a bit less.
They 'should', and I completely agree; but that isn't the reality today.
Thus the patch.
Now it's understandable for the community to say 'not my problem' but
I'd say that depends on what it considers 'my problem'. If half the people
manage their installations on a cloud solution, it unwillingly becomes one.
Unrelated, it still does allow a non-superuser to take a dump sans the
password, which doesn't seem like a bad idea, since the patch
doesn't do anything more than dump from pg_roles, which is anyways
available to non-superusers.
I await if more object similarly.
-
robins
Robert, * Robert Haas (robertmhaas@gmail.com) wrote: > On Sun, Feb 19, 2017 at 12:24 AM, Robins Tharakan <tharakan@gmail.com> wrote: > > I would like to work on a patch to accommodate restricted environments (such > > as AWS RDS Postgres) which don't allow pg_authid access since their > > definition of Superuser is just a regular user with extra permissions. > > > > Would you consider a patch to add a flag to work around this restriction, > > Or, do you prefer that this be maintained outside core? > > I am a little surprised that this patch has gotten such a good > reception. We haven't in the past been all that willing to accept > core changes for the benefit of forks of PostgreSQL; extensions, sure, > but forks? Maybe we should take the view that Amazon has broken this > and Amazon ought to fix it, rather than making it our job to (try to) > work around their bugs. I suspect it's gotten a reasonably good reception because it's about making it possible to do more things as a non-superuser, which is something that we've got a number of different people working on currently, with two interesting patches from Dave geared towards doing that for monitoring. I have no doubt that there will be other users of this, which means that it isn't "for the benefit of forks" but for users of core too. > On the other hand, maybe that approach is narrow-minded. I agree that we shouldn't be accepting changes into core which are only applicable and helpful for forks of PG. Thanks! Stephen
On 28 February 2017 at 16:12, Stephen Frost <sfrost@snowman.net> wrote: > Robert, > > * Robert Haas (robertmhaas@gmail.com) wrote: >> On Sun, Feb 19, 2017 at 12:24 AM, Robins Tharakan <tharakan@gmail.com> wrote: >> > I would like to work on a patch to accommodate restricted environments (such >> > as AWS RDS Postgres) which don't allow pg_authid access since their >> > definition of Superuser is just a regular user with extra permissions. >> > >> > Would you consider a patch to add a flag to work around this restriction, >> > Or, do you prefer that this be maintained outside core? >> >> I am a little surprised that this patch has gotten such a good >> reception. We haven't in the past been all that willing to accept >> core changes for the benefit of forks of PostgreSQL; extensions, sure, >> but forks? Maybe we should take the view that Amazon has broken this >> and Amazon ought to fix it, rather than making it our job to (try to) >> work around their bugs. > > I suspect it's gotten a reasonably good reception because it's about > making it possible to do more things as a non-superuser, which is > something that we've got a number of different people working on > currently, with two interesting patches from Dave geared towards doing > that for monitoring. I have no doubt that there will be other users of > this, which means that it isn't "for the benefit of forks" but for users > of core too. If this was of benefit only to one company it would not get time or attention from me. I thought that would have been obvious enough, but if not, I'm happy to say it clearly. The enhanced patch by me removes any mention of specific vendors or approaches. I've edited the stated reason for the patch on the CF app, so its clearer as to why this might be acceptable. -- Simon Riggs http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On 28 February 2017 at 17:49, Simon Riggs <simon@2ndquadrant.com> wrote: > I've edited the stated reason for the patch on the CF app, so its > clearer as to why this might be acceptable. Robins, I'm looking to commit the patch version I posted, so I would like your comments that it does continue to solve the problems you raised. Thanks -- Simon Riggs http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On 5 March 2017 at 18:00, Simon Riggs <simon@2ndquadrant.com> wrote:
I'm looking to commit the patch version I posted, so I would like your
comments that it does continue to solve the problems you raised.
Thanks Simon, for confirming.
Yes, the updated patch does solve the problem.
-
robins
robins
Why this?
+ if (no_role_passwords && binary_upgrade)
+ {
+ fprintf(stderr, _("%s: options --no-role-passwords and
--binary-upgrade cannot be used together\n"),
+ progname);
+ fprintf(stderr, _("Try \"%s --help\" for more information.\n"),
+ progname);
+ exit_nicely(1);
+ }
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Thanks for nice patch related to AWS RDS.
Regards,
Sachin
On Sun, Mar 5, 2017 at 12:30 PM, Simon Riggs <simon@2ndquadrant.com> wrote:
On 28 February 2017 at 17:49, Simon Riggs <simon@2ndquadrant.com> wrote:
> I've edited the stated reason for the patch on the CF app, so its
> clearer as to why this might be acceptable.
Robins,
I'm looking to commit the patch version I posted, so I would like your
comments that it does continue to solve the problems you raised.
Thanks
--
Simon Riggs http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Sachin Kotwal
Greetings, * Sachin Kotwal (kotsachin@gmail.com) wrote: > Can we have backpatch this patch to PostgreSQL 9.6 and earlier releases ? No. This is a new feature and new features are not back-patched. Thanks! Stephen
Hi Stephen,
Thanks. I understand this is small but new feature and not bug fix.
But we should be able to backpatch if there is no dependency.
It will help users to get benefit of this feature for g96 and pg95 in RDS until they will have pg10 in RDS.
If It is against community policy then it is ok. I can understand.
Regards,
Sachin
On Wed, Mar 15, 2017 at 6:49 PM, Stephen Frost <sfrost@snowman.net> wrote:
Greetings,
* Sachin Kotwal (kotsachin@gmail.com) wrote:
> Can we have backpatch this patch to PostgreSQL 9.6 and earlier releases ?
No. This is a new feature and new features are not back-patched.
Thanks!
Stephen
Sachin Kotwal
Greetings, * Sachin Kotwal (kotsachin@gmail.com) wrote: > Thanks. I understand this is small but new feature and not bug fix. > But we should be able to backpatch if there is no dependency. No, it's a new feature and won't be back-patched. > It will help users to get benefit of this feature for g96 and pg95 in RDS > until they will have pg10 in RDS. There is no need to wait for pg10 to be in RDS to use PG10's pg_dumpall against RDS databases. pg_dump and pg_dumpall are very intentionally designed and intended to work against older versions of PG, so as soon as PG10 is released you'll be able to run PG10's pg_dumpall against your 9.6 or 9.5 RDS databases. > If It is against community policy then it is ok. I can understand. It is against community policy to back-patch features. Thanks! Stephen
On 3/13/17 16:41, Peter Eisentraut wrote:
> Why this?
No answer. Can we remove this chunk?
> + if (no_role_passwords && binary_upgrade)
> + {
> + fprintf(stderr, _("%s: options --no-role-passwords and
> --binary-upgrade cannot be used together\n"),
> + progname);
> + fprintf(stderr, _("Try \"%s --help\" for more information.\n"),
> + progname);
> + exit_nicely(1);
> + }
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:
> No answer. Can we remove this chunk?
>> + if (no_role_passwords && binary_upgrade)
Perhaps, but why? ISTM that trying to run pg_upgrade as non-superuser
is a nonstarter for a number of reasons, while if you're superuser you
do not need --no-role-passwords.
regards, tom lane
On 15 March 2017 at 21:56, Stephen Frost <sfrost@snowman.net> wrote: > Greetings, > > * Sachin Kotwal (kotsachin@gmail.com) wrote: >> Thanks. I understand this is small but new feature and not bug fix. >> But we should be able to backpatch if there is no dependency. > > No, it's a new feature and won't be back-patched. > >> It will help users to get benefit of this feature for g96 and pg95 in RDS >> until they will have pg10 in RDS. > > There is no need to wait for pg10 to be in RDS to use PG10's pg_dumpall > against RDS databases. pg_dump and pg_dumpall are very intentionally > designed and intended to work against older versions of PG, so as soon > as PG10 is released you'll be able to run PG10's pg_dumpall against your > 9.6 or 9.5 RDS databases. However, there's no guarantee that 9.5 or 9.6 will be able to _restore_ dumps made with Pg10's pg_dumpall and pg_dump. We don't have any output-compatibility in pg_dump to limit it to features from some $older_release . But ... you can always backpatch it yourself into older Pg and build your own pg_dump and pg_dumpall. -- Craig Ringer http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services
On 3/21/17 23:34, Tom Lane wrote: > Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes: >> No answer. Can we remove this chunk? > >>> + if (no_role_passwords && binary_upgrade) > > Perhaps, but why? ISTM that trying to run pg_upgrade as non-superuser > is a nonstarter for a number of reasons, while if you're superuser you > do not need --no-role-passwords. Well, this code was added, apparently without reason. We don't need to actively prohibit option combinations just because they are unusual. -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services