Dump public schema ownership & seclabels
/messages/by-id/20201031163518.GB4039133@rfd.leadboat.com gave $SUBJECT as
one of the constituent projects for changing the public schema default ACL.
This ended up being simple. Attached. I chose to omit the "ALTER SCHEMA
public OWNER TO" when the owner is the bootstrap superuser, like how we skip
acl GRANT/REVOKE when the ACL matches the one recorded in pg_init_privs. I
waffled on that; would it be better to make the OWNER TO unconditional?
Like ownership, we've not been dumping security labels on the public schema.
The way I fixed ownership fixed security labels implicitly. If anyone thinks
I should unbundle these two, let me know.
All this is arguably a fix for an ancient bug. Some sites may need to
compensate for the behavior change, so I plan not to back-patch.
Thanks,
nm
Attachments:
public-owner-dump-v1.patchtext/plain; charset=us-asciiDownload+60-14
On Tue, Dec 29, 2020 at 05:49:24AM -0800, Noah Misch wrote:
/messages/by-id/20201031163518.GB4039133@rfd.leadboat.com gave $SUBJECT as
one of the constituent projects for changing the public schema default ACL.
This ended up being simple. Attached.
This is defective; it fails to reproduce nspacl after "ALTER SCHEMA public
OWNER TO pg_write_server_files; REVOKE ALL ON SCHEMA public FROM
pg_write_server_files;". I will try again later.
On 12/30/20 12:59 PM, Noah Misch wrote:
On Tue, Dec 29, 2020 at 05:49:24AM -0800, Noah Misch wrote:
/messages/by-id/20201031163518.GB4039133@rfd.leadboat.com gave $SUBJECT as
one of the constituent projects for changing the public schema default ACL.
This ended up being simple. Attached.This is defective; it fails to reproduce nspacl after "ALTER SCHEMA public
OWNER TO pg_write_server_files; REVOKE ALL ON SCHEMA public FROM
pg_write_server_files;". I will try again later.
Could I ask you to also include COMMENTs when you try again, please?
--
Vik Fearing
On Sat, Jan 16, 2021 at 02:05:43PM +0100, Vik Fearing wrote:
On 12/30/20 12:59 PM, Noah Misch wrote:
On Tue, Dec 29, 2020 at 05:49:24AM -0800, Noah Misch wrote:
/messages/by-id/20201031163518.GB4039133@rfd.leadboat.com gave $SUBJECT as
one of the constituent projects for changing the public schema default ACL.
This ended up being simple. Attached.This is defective; it fails to reproduce nspacl after "ALTER SCHEMA public
OWNER TO pg_write_server_files; REVOKE ALL ON SCHEMA public FROM
pg_write_server_files;". I will try again later.Could I ask you to also include COMMENTs when you try again, please?
That may work. I had not expected to hear of a person changing the comment on
schema public. To what do you change it?
On 1/17/21 10:41 AM, Noah Misch wrote:
On Sat, Jan 16, 2021 at 02:05:43PM +0100, Vik Fearing wrote:
On 12/30/20 12:59 PM, Noah Misch wrote:
On Tue, Dec 29, 2020 at 05:49:24AM -0800, Noah Misch wrote:
/messages/by-id/20201031163518.GB4039133@rfd.leadboat.com gave $SUBJECT as
one of the constituent projects for changing the public schema default ACL.
This ended up being simple. Attached.This is defective; it fails to reproduce nspacl after "ALTER SCHEMA public
OWNER TO pg_write_server_files; REVOKE ALL ON SCHEMA public FROM
pg_write_server_files;". I will try again later.Could I ask you to also include COMMENTs when you try again, please?
That may work. I had not expected to hear of a person changing the comment on
schema public. To what do you change it?
It was a while ago and I don't remember because it didn't appear in the
dump so I stopped doing it. :(
Mine was an actual comment, but there are some tools out there that
(ab)use COMMENTs as crude metadata for what they do. For example:
https://postgresql-anonymizer.readthedocs.io/en/stable/declare_masking_rules/#declaring-rules-with-comments
--
Vik Fearing
On Sun, Jan 17, 2021 at 12:00:06PM +0100, Vik Fearing wrote:
On 1/17/21 10:41 AM, Noah Misch wrote:
On Sat, Jan 16, 2021 at 02:05:43PM +0100, Vik Fearing wrote:
On 12/30/20 12:59 PM, Noah Misch wrote:
On Tue, Dec 29, 2020 at 05:49:24AM -0800, Noah Misch wrote:
/messages/by-id/20201031163518.GB4039133@rfd.leadboat.com gave $SUBJECT as
one of the constituent projects for changing the public schema default ACL.
This ended up being simple. Attached.This is defective; it fails to reproduce nspacl after "ALTER SCHEMA public
OWNER TO pg_write_server_files; REVOKE ALL ON SCHEMA public FROM
pg_write_server_files;". I will try again later.
Fixed. The comment added to getNamespaces() explains what went wrong.
Incidentally, --no-acl is fragile without --no-owner, because any REVOKE
statements assume a particular owner. Since one can elect --no-owner at
restore time, we can't simply adjust the REVOKE statements constructed at dump
time. That's not new with this patch or specific to initdb-created objects.
Could I ask you to also include COMMENTs when you try again, please?
That may work. I had not expected to hear of a person changing the comment on
schema public. To what do you change it?It was a while ago and I don't remember because it didn't appear in the
dump so I stopped doing it. :(Mine was an actual comment, but there are some tools out there that
(ab)use COMMENTs as crude metadata for what they do. For example:
https://postgresql-anonymizer.readthedocs.io/en/stable/declare_masking_rules/#declaring-rules-with-comments
I've attached a separate patch for this, which applies atop the ownership
patch. This makes more restores fail for non-superusers, which is okay.
On Thu, Feb 11, 2021 at 04:08:34AM -0800, Noah Misch wrote:
On Sun, Jan 17, 2021 at 12:00:06PM +0100, Vik Fearing wrote:
On 1/17/21 10:41 AM, Noah Misch wrote:
On Sat, Jan 16, 2021 at 02:05:43PM +0100, Vik Fearing wrote:
On 12/30/20 12:59 PM, Noah Misch wrote:
On Tue, Dec 29, 2020 at 05:49:24AM -0800, Noah Misch wrote:
/messages/by-id/20201031163518.GB4039133@rfd.leadboat.com gave $SUBJECT as
one of the constituent projects for changing the public schema default ACL.
This ended up being simple. Attached.This is defective; it fails to reproduce nspacl after "ALTER SCHEMA public
OWNER TO pg_write_server_files; REVOKE ALL ON SCHEMA public FROM
pg_write_server_files;". I will try again later.Fixed. The comment added to getNamespaces() explains what went wrong.
Incidentally, --no-acl is fragile without --no-owner, because any REVOKE
statements assume a particular owner. Since one can elect --no-owner at
restore time, we can't simply adjust the REVOKE statements constructed at dump
time. That's not new with this patch or specific to initdb-created objects.Could I ask you to also include COMMENTs when you try again, please?
That may work. I had not expected to hear of a person changing the comment on
schema public. To what do you change it?It was a while ago and I don't remember because it didn't appear in the
dump so I stopped doing it. :(Mine was an actual comment, but there are some tools out there that
(ab)use COMMENTs as crude metadata for what they do. For example:
https://postgresql-anonymizer.readthedocs.io/en/stable/declare_masking_rules/#declaring-rules-with-commentsI've attached a separate patch for this, which applies atop the ownership
patch. This makes more restores fail for non-superusers, which is okay.
Oops, I botched a refactoring late in the development of that patch. Here's a
fixed pair of patches.
The following review has been posted through the commitfest application:
make installcheck-world: tested, passed
Implements feature: tested, passed
Spec compliant: tested, passed
Documentation: not tested
Hi,
I have tested this patch. This patch still applies and it works well.
Regards,
Asif
The new status of this patch is: Ready for Committer
On Sat, May 1, 2021 at 8:16 AM Asif Rehman <asifr.rehman@gmail.com> wrote:
The following review has been posted through the commitfest application:
make installcheck-world: tested, passed
Implements feature: tested, passed
Spec compliant: tested, passed
Documentation: not testedHi,
I have tested this patch. This patch still applies and it works well.
Regards,
AsifThe new status of this patch is: Ready for Committer
For public-schema-comment-dump-v2.patch :
+ if (ncomments == 0)
+ {
+ comments = &empty_comment;
+ ncomments = 1;
+ }
+ else if (strcmp(comments->descr, (fout->remoteVersion >= 80300 ?
+ "standard public schema" :
+ "Standard public schema")) == 0)
+ {
+ ncomments = 0;Is it possible that, in the case ncomments > 0, there are more than one
comment ?
If not, an assertion can be added in the second if block above.
Cheers
On Sat, May 01, 2021 at 09:43:36AM -0700, Zhihong Yu wrote:
On Sat, May 1, 2021 at 8:16 AM Asif Rehman <asifr.rehman@gmail.com> wrote:
The following review has been posted through the commitfest application:
make installcheck-world: tested, passed
Implements feature: tested, passed
Spec compliant: tested, passed
Documentation: not testedHi,
I have tested this patch. This patch still applies and it works well.
Regards,
AsifThe new status of this patch is: Ready for Committer
Thanks. Later, I saw that "pg_dump --schema=public" traditionally has yielded
"CREATE SCHEMA public" and "COMMENT ON SCHEMA public". I've updated the
patches to preserve that behavior.
I'll push this when v15 branches. I do think it's a bug fix and could argue
for including it in v14. On the other hand, I mailed three total patch
versions now known to be wrong, so it would be imprudent to count on no
surprises remaining.
For public-schema-comment-dump-v2.patch :
+ if (ncomments == 0) + { + comments = &empty_comment; + ncomments = 1; + } + else if (strcmp(comments->descr, (fout->remoteVersion >= 80300 ? + "standard public schema" : + "Standard public schema")) == 0) + { + ncomments = 0;Is it possible that, in the case ncomments > 0, there are more than one
comment ?
Yes, I think that's normal when the search terms include an objsubid (subid !=
InvalidOid).
On Sun, May 02, 2021 at 10:57:47PM -0700, Noah Misch wrote:
On Sat, May 01, 2021 at 09:43:36AM -0700, Zhihong Yu wrote:
On Sat, May 1, 2021 at 8:16 AM Asif Rehman <asifr.rehman@gmail.com> wrote:
The new status of this patch is: Ready for Committer
I'll push this when v15 branches.
Done. This upset one buildfarm member so far:
https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=gaur&dt=2021-06-29%2001%3A43%3A14
I don't know what happened there. Tom, could you post a tar of the
src/bin/pg_dump/tmp_check/tmp_test_* directory after a failed "make -C
src/bin/pg_dump check" on that machine?
Noah Misch <noah@leadboat.com> writes:
Done. This upset one buildfarm member so far:
https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=gaur&dt=2021-06-29%2001%3A43%3A14
I don't know what happened there. Tom, could you post a tar of the
src/bin/pg_dump/tmp_check/tmp_test_* directory after a failed "make -C
src/bin/pg_dump check" on that machine?
I'm too tired to look at it right now, but remembering that that's
running an old Perl version, I wonder if there's some Perl
incompatibility here.
regards, tom lane
On Tue, Jun 29, 2021 at 01:53:42AM -0400, Tom Lane wrote:
Noah Misch <noah@leadboat.com> writes:
Done. This upset one buildfarm member so far:
https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=gaur&dt=2021-06-29%2001%3A43%3A14I don't know what happened there. Tom, could you post a tar of the
src/bin/pg_dump/tmp_check/tmp_test_* directory after a failed "make -C
src/bin/pg_dump check" on that machine?I'm too tired to look at it right now, but remembering that that's
running an old Perl version, I wonder if there's some Perl
incompatibility here.
That's at least part of the problem, based on experiments on a machine with
Perl 5.8.4. That machine can't actually build PostgreSQL. I've pushed a
necessary fix, though I'm only about 80% confident about it being sufficient.
Noah Misch <noah@leadboat.com> writes:
On Tue, Jun 29, 2021 at 01:53:42AM -0400, Tom Lane wrote:
Noah Misch <noah@leadboat.com> writes:
Done. This upset one buildfarm member so far:
https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=gaur&dt=2021-06-29%2001%3A43%3A14
I'm too tired to look at it right now, but remembering that that's
running an old Perl version, I wonder if there's some Perl
incompatibility here.
That's at least part of the problem, based on experiments on a machine with
Perl 5.8.4. That machine can't actually build PostgreSQL. I've pushed a
necessary fix, though I'm only about 80% confident about it being sufficient.
gaur is still plugging away on a new run, but it got past the
pg_dump-check step, so I think you're good.
prairiedog has a similar-vintage Perl, so likely it would have shown the
problem too; but it's slow enough that it never saw the intermediate state
between these commits.
regards, tom lane