pg_basebackup ignores the existing data directory permissions

Started by Haribabu Kommialmost 7 years ago50 messages
#1Haribabu Kommi
Haribabu Kommi
kommi.haribabu@gmail.com
1 attachment(s)

Hi Hackers,

During the testing allow group access permissions on the standby database
directory,
one of my colleague found the issue, that pg_basebackup doesn't verify
whether the existing data directory has the required permissions or not?
This issue is not related allow group access permissions. This problem was
present for a long time, (I think from the time the pg_basebackup was
introduced).

Attached patch fixes the problem similar like initdb by changing the
permissions of the data
directory to the required permissions.

Regards,
Haribabu Kommi
Fujitsu Australia

Attachments:

0001-pg_basebackup-Correct-the-existing-directory-permiss.patchapplication/octet-stream; name=0001-pg_basebackup-Correct-the-existing-directory-permiss.patch
#2Michael Paquier
Michael Paquier
michael@paquier.xyz
In reply to: Haribabu Kommi (#1)
Re: pg_basebackup ignores the existing data directory permissions

On Tue, Feb 12, 2019 at 06:03:37PM +1100, Haribabu Kommi wrote:

During the testing allow group access permissions on the standby database
directory, one of my colleague found the issue, that pg_basebackup
doesn't verify whether the existing data directory has the required
permissions or not? This issue is not related allow group access
permissions. This problem was present for a long time, (I think from
the time the pg_basebackup was introduced).

In which case this would cause the postmaster to fail to start.

Attached patch fixes the problem similar like initdb by changing the
permissions of the data
directory to the required permissions.

It looks right to me and takes care of the case where group access is
allowed. Still, we have not seen any complains about the current
behavior either and pg_basebackup is around for some time already, so
I would tend to not back-patch that. Any thoughts from others?
--
Michael

#3Haribabu Kommi
Haribabu Kommi
kommi.haribabu@gmail.com
In reply to: Michael Paquier (#2)
Re: pg_basebackup ignores the existing data directory permissions

On Wed, Feb 13, 2019 at 12:42 PM Michael Paquier <michael@paquier.xyz>
wrote:

On Tue, Feb 12, 2019 at 06:03:37PM +1100, Haribabu Kommi wrote:

During the testing allow group access permissions on the standby database
directory, one of my colleague found the issue, that pg_basebackup
doesn't verify whether the existing data directory has the required
permissions or not? This issue is not related allow group access
permissions. This problem was present for a long time, (I think from
the time the pg_basebackup was introduced).

In which case this would cause the postmaster to fail to start.

Yes, the postmaster fails to start, but I feel if pg_basebackup takes care
of correcting the file permissions automatically like initdb, that will be
good.

Attached patch fixes the problem similar like initdb by changing the
permissions of the data
directory to the required permissions.

It looks right to me and takes care of the case where group access is
allowed. Still, we have not seen any complains about the current
behavior either and pg_basebackup is around for some time already, so
I would tend to not back-patch that. Any thoughts from others?

This should back-patch till 11 where the group access is introduced.
Because of lack of complaints, I agree with you that there is no need of
further back-patch.

Regards,
Haribabu Kommi
Fujitsu Australia

#4Michael Paquier
Michael Paquier
michael@paquier.xyz
In reply to: Haribabu Kommi (#3)
Re: pg_basebackup ignores the existing data directory permissions

On Wed, Feb 13, 2019 at 06:42:36PM +1100, Haribabu Kommi wrote:

This should back-patch till 11 where the group access is introduced.
Because of lack of complaints, I agree with you that there is no need of
further back-patch.

I am confused by the link with group access. The patch you are
sending is compatible down to v11, but we could also do it further
down by just using chmod with S_IRWXU on the target folder if it
exists and is empty.
--
Michael

#5Haribabu Kommi
Haribabu Kommi
kommi.haribabu@gmail.com
In reply to: Michael Paquier (#4)
Re: pg_basebackup ignores the existing data directory permissions

On Thu, Feb 14, 2019 at 3:04 PM Michael Paquier <michael@paquier.xyz> wrote:

On Wed, Feb 13, 2019 at 06:42:36PM +1100, Haribabu Kommi wrote:

This should back-patch till 11 where the group access is introduced.
Because of lack of complaints, I agree with you that there is no need of
further back-patch.

I am confused by the link with group access.

Apologies to confuse you by linking it with group access. This patch doesn't
have an interaction with group access. From v11 onwards, PostgreSQL server
accepts two set of permissions for the data directory because of group
access.

we have an application that is used to create the data directory with
owner access (0700), but with initdb group permissions option, it
automatically
converts to (0750) by the initdb. But pg_basebackup doesn't change it when
it tries to do a backup from a group access server.

The patch you are
sending is compatible down to v11, but we could also do it further
down by just using chmod with S_IRWXU on the target folder if it
exists and is empty.

Yes, I agree with you that by changing chmod as you said fixes it in the
back-branches.

Regards,
Haribabu Kommi
Fujitsu Australia

#6Michael Paquier
Michael Paquier
michael@paquier.xyz
In reply to: Haribabu Kommi (#5)
Re: pg_basebackup ignores the existing data directory permissions

On Thu, Feb 14, 2019 at 06:34:07PM +1100, Haribabu Kommi wrote:

we have an application that is used to create the data directory with

Well, initdb would do that happily, so there is no actual any need to
do that to begin with. Anyway..

owner access (0700), but with initdb group permissions option, it
automatically
converts to (0750) by the initdb. But pg_basebackup doesn't change it when
it tries to do a backup from a group access server.

So that's basically the opposite of the case I was thinking about,
where you create a path for a base backup with permissions strictly
higher than 700, say 755, and the base backup path does not have
enough restrictions. And in your case the permissions are too
restrictive because of the application creating the folder itself but
they should be relaxed if group access is enabled. Actually, that's
something that we may want to do consistently across all branches. If
an application calls pg_basebackup after creating a path, they most
likely change the permissions anyway to allow the postmaster to
start.
--
Michael

#7Magnus Hagander
Magnus Hagander
magnus@hagander.net
In reply to: Michael Paquier (#6)
Re: pg_basebackup ignores the existing data directory permissions

On Thu, Feb 14, 2019 at 9:10 AM Michael Paquier <michael@paquier.xyz> wrote:

On Thu, Feb 14, 2019 at 06:34:07PM +1100, Haribabu Kommi wrote:

we have an application that is used to create the data directory with

Well, initdb would do that happily, so there is no actual any need to
do that to begin with. Anyway..

owner access (0700), but with initdb group permissions option, it
automatically
converts to (0750) by the initdb. But pg_basebackup doesn't change it

when

it tries to do a backup from a group access server.

So that's basically the opposite of the case I was thinking about,
where you create a path for a base backup with permissions strictly
higher than 700, say 755, and the base backup path does not have
enough restrictions. And in your case the permissions are too
restrictive because of the application creating the folder itself but
they should be relaxed if group access is enabled. Actually, that's
something that we may want to do consistently across all branches. If
an application calls pg_basebackup after creating a path, they most
likely change the permissions anyway to allow the postmaster to
start.

I think it could be argued that neither initdb *or* pg_basebackup should
change the permissions on an existing directory, because the admin may have
done that intentionally. But when they do create the directory, they should
follow the same patterns.

--
Magnus Hagander
Me: https://www.hagander.net/ <http://www.hagander.net/&gt;
Work: https://www.redpill-linpro.com/ <http://www.redpill-linpro.com/&gt;

#8Haribabu Kommi
Haribabu Kommi
kommi.haribabu@gmail.com
In reply to: Magnus Hagander (#7)
Re: pg_basebackup ignores the existing data directory permissions

On Thu, Feb 14, 2019 at 8:57 PM Magnus Hagander <magnus@hagander.net> wrote:

On Thu, Feb 14, 2019 at 9:10 AM Michael Paquier <michael@paquier.xyz>
wrote:

On Thu, Feb 14, 2019 at 06:34:07PM +1100, Haribabu Kommi wrote:

we have an application that is used to create the data directory with

Well, initdb would do that happily, so there is no actual any need to
do that to begin with. Anyway..

owner access (0700), but with initdb group permissions option, it
automatically
converts to (0750) by the initdb. But pg_basebackup doesn't change it

when

it tries to do a backup from a group access server.

So that's basically the opposite of the case I was thinking about,
where you create a path for a base backup with permissions strictly
higher than 700, say 755, and the base backup path does not have
enough restrictions. And in your case the permissions are too
restrictive because of the application creating the folder itself but
they should be relaxed if group access is enabled. Actually, that's
something that we may want to do consistently across all branches. If
an application calls pg_basebackup after creating a path, they most
likely change the permissions anyway to allow the postmaster to
start.

I think it could be argued that neither initdb *or* pg_basebackup should
change the permissions on an existing directory, because the admin may have
done that intentionally. But when they do create the directory, they should
follow the same patterns.

Hmm, even if the administrator set some specific permissions to the data
directory,
PostgreSQL server doesn't allow server to start if the permissions are not
(0700)
for versions less than 11 and (0700 or 0750) for version 11 or later.

To let the user to use the PostgreSQL server, user must change the
permissions
of the data directory. So, I don't see a problem in changing the
permissions by these
tools.

Regards,
Haribabu Kommi
Fujitsu Australia

#9Michael Paquier
Michael Paquier
michael@paquier.xyz
In reply to: Haribabu Kommi (#8)
Re: pg_basebackup ignores the existing data directory permissions

On Thu, Feb 14, 2019 at 11:21:19PM +1100, Haribabu Kommi wrote:

On Thu, Feb 14, 2019 at 8:57 PM Magnus Hagander <magnus@hagander.net> wrote:

I think it could be argued that neither initdb *or* pg_basebackup should
change the permissions on an existing directory, because the admin may have
done that intentionally. But when they do create the directory, they should
follow the same patterns.

Hmm, even if the administrator set some specific permissions to the data
directory, PostgreSQL server doesn't allow server to start if the
permissions are not (0700) for versions less than 11 and (0700 or
0750) for version 11 or later.

Yes, particularly with pg_basebackup -R this adds an extra step in the
user flow.

To let the user to use the PostgreSQL server, user must change the
permissions of the data directory. So, I don't see a problem in
changing the permissions by these tools.

I certainly agree with the point of Magnus that both tools should
behave consistently, and I cannot actually imagine why it would be
useful for an admin to keep a more permissive data folder while all
the contents already have umasks set at the same level as the primary
(or what initdb has been told to use), but perhaps I lack imagination.
If we doubt about potential user impact, the usual, best, answer is to
let back-branches behave the way they do now, and only do something on
HEAD.
--
Michael

#10Kyotaro HORIGUCHI
Kyotaro HORIGUCHI
horiguchi.kyotaro@lab.ntt.co.jp
In reply to: Michael Paquier (#9)
Re: pg_basebackup ignores the existing data directory permissions

At Fri, 15 Feb 2019 08:15:24 +0900, Michael Paquier <michael@paquier.xyz> wrote in <20190214231524.GC2240@paquier.xyz>

On Thu, Feb 14, 2019 at 11:21:19PM +1100, Haribabu Kommi wrote:

On Thu, Feb 14, 2019 at 8:57 PM Magnus Hagander <magnus@hagander.net> wrote:

I think it could be argued that neither initdb *or* pg_basebackup should
change the permissions on an existing directory, because the admin may have
done that intentionally. But when they do create the directory, they should
follow the same patterns.

Hmm, even if the administrator set some specific permissions to the data
directory, PostgreSQL server doesn't allow server to start if the
permissions are not (0700) for versions less than 11 and (0700 or
0750) for version 11 or later.

Yes, particularly with pg_basebackup -R this adds an extra step in the
user flow.

I disagree that pg_basebackup rejects directories other than
specific permissions, since it is just a binary backup tool,
which is not exclusive to making replication-standby. It ought to
be runnable and actually runnable by any OS users even by root,
for who postgres rejects to start. As mentioned upthread, it is
safe-side failure that server rejects to run on it.

To let the user to use the PostgreSQL server, user must change the
permissions of the data directory. So, I don't see a problem in
changing the permissions by these tools.

I certainly agree with the point of Magnus that both tools should
behave consistently, and I cannot actually imagine why it would be
useful for an admin to keep a more permissive data folder while all
the contents already have umasks set at the same level as the primary
(or what initdb has been told to use), but perhaps I lack imagination.
If we doubt about potential user impact, the usual, best, answer is to
let back-branches behave the way they do now, and only do something on
HEAD.

initdb is to create a directory on which server works and rather
rejects existing directory, so I think the "incosistency" seems
fine.

I can live with some new options, say --create-New-directory or
--check-directory-Permission.

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center

#11Michael Paquier
Michael Paquier
michael@paquier.xyz
In reply to: Kyotaro HORIGUCHI (#10)
Re: pg_basebackup ignores the existing data directory permissions

On Fri, Feb 15, 2019 at 09:24:15AM +0900, Kyotaro HORIGUCHI wrote:

I disagree that pg_basebackup rejects directories other than
specific permissions, since it is just a binary backup tool,
which is not exclusive to making replication-standby. It ought to
be runnable and actually runnable by any OS users even by root,
for who postgres rejects to start. As mentioned upthread, it is
safe-side failure that server rejects to run on it.

Perhaps I do not fully understand your argument here. We do not
discuss about making pg_basebackup fail in any way, just of having it
adjust the umask of the target path so as users can simplify startups
using the generated base backup.
--
Michael

#12Haribabu Kommi
Haribabu Kommi
kommi.haribabu@gmail.com
In reply to: Michael Paquier (#9)
Re: pg_basebackup ignores the existing data directory permissions

On Fri, Feb 15, 2019 at 10:15 AM Michael Paquier <michael@paquier.xyz>
wrote:

On Thu, Feb 14, 2019 at 11:21:19PM +1100, Haribabu Kommi wrote:

On Thu, Feb 14, 2019 at 8:57 PM Magnus Hagander <magnus@hagander.net>

wrote:

I think it could be argued that neither initdb *or* pg_basebackup should
change the permissions on an existing directory, because the admin may

have

done that intentionally. But when they do create the directory, they

should

follow the same patterns.

Hmm, even if the administrator set some specific permissions to the data
directory, PostgreSQL server doesn't allow server to start if the
permissions are not (0700) for versions less than 11 and (0700 or
0750) for version 11 or later.

Yes, particularly with pg_basebackup -R this adds an extra step in the
user flow.

To let the user to use the PostgreSQL server, user must change the
permissions of the data directory. So, I don't see a problem in
changing the permissions by these tools.

I certainly agree with the point of Magnus that both tools should
behave consistently, and I cannot actually imagine why it would be
useful for an admin to keep a more permissive data folder while all
the contents already have umasks set at the same level as the primary
(or what initdb has been told to use), but perhaps I lack imagination.
If we doubt about potential user impact, the usual, best, answer is to
let back-branches behave the way they do now, and only do something on
HEAD.

I also agree that both inidb and pg_basebackup should behave same.
Our main concern is that standby data directory that doesn't follow
the primary data directory permissions can lead failures when the standby
gets promoted.

Lack of complaints from the users, how about making this change in the HEAD?

Regards,
Haribabu Kommi
Fujitsu Australia

#13Michael Paquier
Michael Paquier
michael@paquier.xyz
In reply to: Haribabu Kommi (#12)
Re: pg_basebackup ignores the existing data directory permissions

On Wed, Feb 20, 2019 at 03:16:48PM +1100, Haribabu Kommi wrote:

Lack of complaints from the users, how about making this change in the HEAD?

Fine by me. I would tend to patch pg_basebackup so as the target
folder gets the correct umask instead of nuking the chmod call in
initdb so I think that we are on the same page. Let's see what the
others think.
--
Michael

#14Magnus Hagander
Magnus Hagander
magnus@hagander.net
In reply to: Haribabu Kommi (#12)
Re: pg_basebackup ignores the existing data directory permissions

On Wed, Feb 20, 2019 at 5:17 AM Haribabu Kommi <kommi.haribabu@gmail.com>
wrote:

On Fri, Feb 15, 2019 at 10:15 AM Michael Paquier <michael@paquier.xyz>
wrote:

On Thu, Feb 14, 2019 at 11:21:19PM +1100, Haribabu Kommi wrote:

On Thu, Feb 14, 2019 at 8:57 PM Magnus Hagander <magnus@hagander.net>

wrote:

I think it could be argued that neither initdb *or* pg_basebackup

should

change the permissions on an existing directory, because the admin may

have

done that intentionally. But when they do create the directory, they

should

follow the same patterns.

Hmm, even if the administrator set some specific permissions to the data
directory, PostgreSQL server doesn't allow server to start if the
permissions are not (0700) for versions less than 11 and (0700 or
0750) for version 11 or later.

Yes, particularly with pg_basebackup -R this adds an extra step in the
user flow.

Perhaps we should make the enforcement of permissions conditional on -R?
OTOH that's documented as "write recovery.conf", but we could change that
to be "prepare for replication" or something?

To let the user to use the PostgreSQL server, user must change the

permissions of the data directory. So, I don't see a problem in
changing the permissions by these tools.

I certainly agree with the point of Magnus that both tools should
behave consistently, and I cannot actually imagine why it would be
useful for an admin to keep a more permissive data folder while all
the contents already have umasks set at the same level as the primary
(or what initdb has been told to use), but perhaps I lack imagination.
If we doubt about potential user impact, the usual, best, answer is to
let back-branches behave the way they do now, and only do something on
HEAD.

I also agree that both inidb and pg_basebackup should behave same.
Our main concern is that standby data directory that doesn't follow
the primary data directory permissions can lead failures when the standby
gets promoted.

I don't think that follows at all. There are many scenarios where you'd
want the standby to have different permissions than the primary. And I'm
not sure it's our business to enforce that. A much much more common mistake
people make is run pg_basebackup as the wrong user, thereby getting the
wrong owner of all files. But that doesn't mean we should enforce the
owner/group of the files.

--
Magnus Hagander
Me: https://www.hagander.net/ <http://www.hagander.net/&gt;
Work: https://www.redpill-linpro.com/ <http://www.redpill-linpro.com/&gt;

#15Haribabu Kommi
Haribabu Kommi
kommi.haribabu@gmail.com
In reply to: Magnus Hagander (#14)
Re: pg_basebackup ignores the existing data directory permissions

On Wed, Feb 20, 2019 at 7:40 PM Magnus Hagander <magnus@hagander.net> wrote:

On Wed, Feb 20, 2019 at 5:17 AM Haribabu Kommi <kommi.haribabu@gmail.com>
wrote:

On Fri, Feb 15, 2019 at 10:15 AM Michael Paquier <michael@paquier.xyz>
wrote:

On Thu, Feb 14, 2019 at 11:21:19PM +1100, Haribabu Kommi wrote:

On Thu, Feb 14, 2019 at 8:57 PM Magnus Hagander <magnus@hagander.net>

wrote:

I think it could be argued that neither initdb *or* pg_basebackup

should

change the permissions on an existing directory, because the admin

may have

done that intentionally. But when they do create the directory, they

should

follow the same patterns.

Hmm, even if the administrator set some specific permissions to the

data

directory, PostgreSQL server doesn't allow server to start if the
permissions are not (0700) for versions less than 11 and (0700 or
0750) for version 11 or later.

Yes, particularly with pg_basebackup -R this adds an extra step in the
user flow.

Perhaps we should make the enforcement of permissions conditional on -R?
OTOH that's documented as "write recovery.conf", but we could change that
to be "prepare for replication" or something?

Yes, the enforcement of permissions can be done only when -R option is
provided.
The documentation is changed in v12 already as "write configuration for
replication".

To let the user to use the PostgreSQL server, user must change the

permissions of the data directory. So, I don't see a problem in
changing the permissions by these tools.

I certainly agree with the point of Magnus that both tools should
behave consistently, and I cannot actually imagine why it would be
useful for an admin to keep a more permissive data folder while all
the contents already have umasks set at the same level as the primary
(or what initdb has been told to use), but perhaps I lack imagination.
If we doubt about potential user impact, the usual, best, answer is to
let back-branches behave the way they do now, and only do something on
HEAD.

I also agree that both inidb and pg_basebackup should behave same.
Our main concern is that standby data directory that doesn't follow
the primary data directory permissions can lead failures when the standby
gets promoted.

I don't think that follows at all. There are many scenarios where you'd
want the standby to have different permissions than the primary.

I really having a hard time to understand that how the different
permissions are possible?
I think of that the standby is having more restrict permissions. May be the
standby is not a
hot standby?

Can you please provide some more details?

Till v11, PostgreSQL allows the data directory permissions to be 0700 of
the directory, otherwise
server start fails, even if the pg_basebackup is successful. In my testing
I came to know that data
directory permissions less than 0700 e.g- 0300 also the server is started.
I feel the check of validating
data directory is checking whether are there any group permissions or not?
But it didn't whether the
current owner have all the permissions are not? Is this the scenario are
you expecting?

And I'm not sure it's our business to enforce that. A much much more
common mistake people make is run pg_basebackup as the wrong user, thereby
getting the wrong owner of all files. But that doesn't mean we should
enforce the owner/group of the files.

I didn't understand this point also clearly. The system user who executes
the pg_basebackup command,
all the database files are associated with that user. If that corresponding
user don't have permissions to
access that existing data folder, the backup fails.

Regards,
Haribabu Kommi
Fujitsu Australia

#16Peter Eisentraut
Peter Eisentraut
peter.eisentraut@2ndquadrant.com
In reply to: Haribabu Kommi (#15)
Re: pg_basebackup ignores the existing data directory permissions

pg_basebackup copies the data directory permission mode from the
upstream server. But it doesn't copy the ownership. So if say the
upstream server allows group access and things are owned by
postgres:postgres, and I want to make a copy for local development and
make a backup into a directory owned by peter:staff without group
access, then it would be inappropriate for pg_basebackup to change the
permissions on that directory.

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

#17Haribabu Kommi
Haribabu Kommi
kommi.haribabu@gmail.com
In reply to: Peter Eisentraut (#16)
Re: pg_basebackup ignores the existing data directory permissions

On Fri, Mar 8, 2019 at 11:59 PM Peter Eisentraut <
peter.eisentraut@2ndquadrant.com> wrote:

pg_basebackup copies the data directory permission mode from the
upstream server. But it doesn't copy the ownership. So if say the
upstream server allows group access and things are owned by
postgres:postgres, and I want to make a copy for local development and
make a backup into a directory owned by peter:staff without group
access, then it would be inappropriate for pg_basebackup to change the
permissions on that directory.

Yes, I agree that it may be a problem if the existing data directory
permissions
are 0700 to changing it to 0750. But it may not be a problem for the
scenarios,
where the existing data permissions >=0750, to the upstream permissions.
Because user must need to change anyway to start the server, otherwise
server
start fails, and also the files inside the data folder follows the
permissions of the
upstream data directory.

usually production systems follows same permissions are of upstream, I don't
see a problem in following the same for development environment also?

comments?

Regards,
Haribabu Kommi
Fujitsu Australia

#18Peter Eisentraut
Peter Eisentraut
peter.eisentraut@2ndquadrant.com
In reply to: Haribabu Kommi (#17)
Re: pg_basebackup ignores the existing data directory permissions

On 2019-03-09 02:19, Haribabu Kommi wrote:

Yes, I agree that it may be a problem if the existing data directory
permissions
are 0700 to changing it to 0750. But it may not be a problem for the
scenarios,
where the existing data permissions  >=0750, to the upstream permissions.
Because user must need to change anyway to start the server, otherwise
server
start fails, and also the files inside the data folder follows the
permissions of the
upstream data directory.

usually production systems follows same permissions are of upstream, I don't
see a problem in following the same for development environment also?

I think the potential problems of getting this wrong are bigger than the
issue we are trying to fix.

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

#19Haribabu Kommi
Haribabu Kommi
kommi.haribabu@gmail.com
In reply to: Peter Eisentraut (#18)
Re: pg_basebackup ignores the existing data directory permissions

On Fri, Mar 15, 2019 at 10:33 AM Peter Eisentraut <
peter.eisentraut@2ndquadrant.com> wrote:

On 2019-03-09 02:19, Haribabu Kommi wrote:

Yes, I agree that it may be a problem if the existing data directory
permissions
are 0700 to changing it to 0750. But it may not be a problem for the
scenarios,
where the existing data permissions >=0750, to the upstream permissions.
Because user must need to change anyway to start the server, otherwise
server
start fails, and also the files inside the data folder follows the
permissions of the
upstream data directory.

usually production systems follows same permissions are of upstream, I

don't

see a problem in following the same for development environment also?

I think the potential problems of getting this wrong are bigger than the
issue we are trying to fix.

Thanks for your opinion. I am not sure exactly what are the problems.
But anyway I can go with your suggestion.

How about changing the data directory permissions for the -R scenario?
if executing pg_basebackup on to an existing data directory is a common
scenario? or leave it?

Regards,
Haribabu Kommi
Fujitsu Australia

#20Robert Haas
Robert Haas
robertmhaas@gmail.com
In reply to: Peter Eisentraut (#18)
Re: pg_basebackup ignores the existing data directory permissions

On Thu, Mar 14, 2019 at 7:34 PM Peter Eisentraut
<peter.eisentraut@2ndquadrant.com> wrote:

I think the potential problems of getting this wrong are bigger than the
issue we are trying to fix.

I think the question is: how do we know what the user intended? If
the user wants the directory to be accessible only to the owner, then
we ought to set the permissions on the directory itself and of
everything inside it to 0700 (or 0600). If they want group access, we
should set everything to 0750 (or 0644). But how do we know what the
user wants?

Right now, we take the position that the user wants the individual
files to have the same mode that they do on the master, but the
directory should retain its existing permissions. That appears to be
pretty silly, because that might end up creating a bunch of files
inside the directory that are marked as group-readable while the
directory itself isn't; surely nobody wants that. Adopting this patch
would fix that inconsistency.

However, it might be better to go the other way. Maybe pg_basebackup
should decide whether group permission is appropriate for the
contained files and directories not by looking at the master, but by
looking at the directory into which it's writing. The basic objection
to this patch seems to be that we should not assume that the user got
the permissions on the existing directory wrong, and I think that
objection is fair, but if we accept it, then we should ask why we're
setting the permission of everything under that directory according to
some other methodology.

Another option would be to provide a pg_basebackup option to allow the
user to specify what they intended i.e. --[no-]group-read. (Tying it
to -R doesn't sound like a good decision to me.)

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

#21Stephen Frost
Stephen Frost
sfrost@snowman.net
In reply to: Robert Haas (#20)
Re: pg_basebackup ignores the existing data directory permissions

Greetings,

* Robert Haas (robertmhaas@gmail.com) wrote:

On Thu, Mar 14, 2019 at 7:34 PM Peter Eisentraut
<peter.eisentraut@2ndquadrant.com> wrote:

I think the potential problems of getting this wrong are bigger than the
issue we are trying to fix.

I think the question is: how do we know what the user intended? If
the user wants the directory to be accessible only to the owner, then
we ought to set the permissions on the directory itself and of
everything inside it to 0700 (or 0600). If they want group access, we
should set everything to 0750 (or 0644). But how do we know what the
user wants?

Right now, we take the position that the user wants the individual
files to have the same mode that they do on the master, but the
directory should retain its existing permissions. That appears to be
pretty silly, because that might end up creating a bunch of files
inside the directory that are marked as group-readable while the
directory itself isn't; surely nobody wants that. Adopting this patch
would fix that inconsistency.

However, it might be better to go the other way. Maybe pg_basebackup
should decide whether group permission is appropriate for the
contained files and directories not by looking at the master, but by
looking at the directory into which it's writing. The basic objection
to this patch seems to be that we should not assume that the user got
the permissions on the existing directory wrong, and I think that
objection is fair, but if we accept it, then we should ask why we're
setting the permission of everything under that directory according to
some other methodology.

Going based on the current setting of the directory seems defensible to
me, with the argument of "we trust you created the directory the way you
want the rest of the system to be".

Another option would be to provide a pg_basebackup option to allow the
user to specify what they intended i.e. --[no-]group-read. (Tying it
to -R doesn't sound like a good decision to me.)

I definitely think that we should add an option to allow the user to
tell us explicitly what they want here, even if we also go based on what
the created directory has (and in that case, we should make everything,
including the base directory, follow what the user asked for).

Thanks!

Stephen

#22Magnus Hagander
Magnus Hagander
magnus@hagander.net
In reply to: Stephen Frost (#21)
Re: pg_basebackup ignores the existing data directory permissions

On Mon, Mar 18, 2019 at 7:08 AM Stephen Frost <sfrost@snowman.net> wrote:

Greetings,

* Robert Haas (robertmhaas@gmail.com) wrote:

On Thu, Mar 14, 2019 at 7:34 PM Peter Eisentraut
<peter.eisentraut@2ndquadrant.com> wrote:

I think the potential problems of getting this wrong are bigger than

the

issue we are trying to fix.

I think the question is: how do we know what the user intended? If
the user wants the directory to be accessible only to the owner, then
we ought to set the permissions on the directory itself and of
everything inside it to 0700 (or 0600). If they want group access, we
should set everything to 0750 (or 0644). But how do we know what the
user wants?

Right now, we take the position that the user wants the individual
files to have the same mode that they do on the master, but the
directory should retain its existing permissions. That appears to be
pretty silly, because that might end up creating a bunch of files
inside the directory that are marked as group-readable while the
directory itself isn't; surely nobody wants that. Adopting this patch
would fix that inconsistency.

However, it might be better to go the other way. Maybe pg_basebackup
should decide whether group permission is appropriate for the
contained files and directories not by looking at the master, but by
looking at the directory into which it's writing. The basic objection
to this patch seems to be that we should not assume that the user got
the permissions on the existing directory wrong, and I think that
objection is fair, but if we accept it, then we should ask why we're
setting the permission of everything under that directory according to
some other methodology.

Going based on the current setting of the directory seems defensible to
me, with the argument of "we trust you created the directory the way you
want the rest of the system to be".

Which I believe is also how a plain unix cp (or tar or whatever) would
work, isn't it? I think that alone is a pretty strong reason to work the
same as those -- they're not entirely unsimilar.

Another option would be to provide a pg_basebackup option to allow the

user to specify what they intended i.e. --[no-]group-read. (Tying it
to -R doesn't sound like a good decision to me.)

I definitely think that we should add an option to allow the user to
tell us explicitly what they want here, even if we also go based on what
the created directory has (and in that case, we should make everything,
including the base directory, follow what the user asked for).

+1 for having an option to override whatever the default is.

--
Magnus Hagander
Me: https://www.hagander.net/ <http://www.hagander.net/&gt;
Work: https://www.redpill-linpro.com/ <http://www.redpill-linpro.com/&gt;

#23Michael Paquier
Michael Paquier
michael@paquier.xyz
In reply to: Magnus Hagander (#22)
Re: pg_basebackup ignores the existing data directory permissions

On Mon, Mar 18, 2019 at 08:32:44AM +0100, Magnus Hagander wrote:

On Mon, Mar 18, 2019 at 7:08 AM Stephen Frost <sfrost@snowman.net> wrote:

I definitely think that we should add an option to allow the user to
tell us explicitly what they want here, even if we also go based on what
the created directory has (and in that case, we should make everything,
including the base directory, follow what the user asked for).

+1 for having an option to override whatever the default is.

Based on the feedback gathered, having a separate option to enforce
the default and not touching the behavior implemented until now,
sounds fine to me.
--
Michael

#24Kyotaro HORIGUCHI
Kyotaro HORIGUCHI
horiguchi.kyotaro@lab.ntt.co.jp
In reply to: Michael Paquier (#23)
Re: pg_basebackup ignores the existing data directory permissions

At Mon, 18 Mar 2019 17:16:01 +0900, Michael Paquier <michael@paquier.xyz> wrote in <20190318081601.GI1885@paquier.xyz>

On Mon, Mar 18, 2019 at 08:32:44AM +0100, Magnus Hagander wrote:

On Mon, Mar 18, 2019 at 7:08 AM Stephen Frost <sfrost@snowman.net> wrote:

I definitely think that we should add an option to allow the user to
tell us explicitly what they want here, even if we also go based on what
the created directory has (and in that case, we should make everything,
including the base directory, follow what the user asked for).

+1 for having an option to override whatever the default is.

Based on the feedback gathered, having a separate option to enforce
the default and not touching the behavior implemented until now,
sounds fine to me.

FWIW +1 from me. That would be like cp -p.

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center

#25Peter Eisentraut
Peter Eisentraut
peter.eisentraut@2ndquadrant.com
In reply to: Robert Haas (#20)
Re: pg_basebackup ignores the existing data directory permissions

On 2019-03-16 15:29, Robert Haas wrote:

Another option would be to provide a pg_basebackup option to allow the
user to specify what they intended i.e. --[no-]group-read. (Tying it
to -R doesn't sound like a good decision to me.)

I was actually surprised to learn how it works right now. I would have
preferred that pg_basebackup require an explicit option to turn on group
access, similar to initdb.

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

#26Peter Eisentraut
Peter Eisentraut
peter.eisentraut@2ndquadrant.com
In reply to: Magnus Hagander (#22)
Re: pg_basebackup ignores the existing data directory permissions

On 2019-03-18 08:32, Magnus Hagander wrote:

Going based on the current setting of the directory seems defensible to
me, with the argument of "we trust you created the directory the way you
want the rest of the system to be".

Which I believe is also how a plain unix cp (or tar or whatever) would
work, isn't it? I think that alone is a pretty strong reason to work the
same as those -- they're not entirely unsimilar.

Those don't copy over the network. In the case of pg_basebackup, there
is nothing that ensures that the remote system has the same users or
groups or permission setup.

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

#27Robert Haas
Robert Haas
robertmhaas@gmail.com
In reply to: Michael Paquier (#23)
Re: pg_basebackup ignores the existing data directory permissions

On Mon, Mar 18, 2019 at 4:16 AM Michael Paquier <michael@paquier.xyz> wrote:

On Mon, Mar 18, 2019 at 08:32:44AM +0100, Magnus Hagander wrote:

On Mon, Mar 18, 2019 at 7:08 AM Stephen Frost <sfrost@snowman.net> wrote:

I definitely think that we should add an option to allow the user to
tell us explicitly what they want here, even if we also go based on what
the created directory has (and in that case, we should make everything,
including the base directory, follow what the user asked for).

+1 for having an option to override whatever the default is.

Based on the feedback gathered, having a separate option to enforce
the default and not touching the behavior implemented until now,
sounds fine to me.

That's not what I'm proposing. I think the behavior implemented until
now is not best, because the files within the directory should inherit
the directory's permissions, not the remote side's permissions.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

#28Peter Eisentraut
Peter Eisentraut
peter.eisentraut@2ndquadrant.com
In reply to: Robert Haas (#27)
Re: pg_basebackup ignores the existing data directory permissions

On 2019-03-18 14:47, Robert Haas wrote:

Based on the feedback gathered, having a separate option to enforce
the default and not touching the behavior implemented until now,
sounds fine to me.

That's not what I'm proposing. I think the behavior implemented until
now is not best, because the files within the directory should inherit
the directory's permissions, not the remote side's permissions.

I'm strongly in favor of keeping initdb and pg_basebackup options
similar and consistent. They are both ways to initialize data directories.

You'll note that initdb does not behave the way you describe. It's not
unreasonable behavior, but it's not the way it currently works.

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

#29Robert Haas
Robert Haas
robertmhaas@gmail.com
In reply to: Peter Eisentraut (#28)
Re: pg_basebackup ignores the existing data directory permissions

On Mon, Mar 18, 2019 at 11:36 AM Peter Eisentraut
<peter.eisentraut@2ndquadrant.com> wrote:

On 2019-03-18 14:47, Robert Haas wrote:

Based on the feedback gathered, having a separate option to enforce
the default and not touching the behavior implemented until now,
sounds fine to me.

That's not what I'm proposing. I think the behavior implemented until
now is not best, because the files within the directory should inherit
the directory's permissions, not the remote side's permissions.

I'm strongly in favor of keeping initdb and pg_basebackup options
similar and consistent. They are both ways to initialize data directories.

You'll note that initdb does not behave the way you describe. It's not
unreasonable behavior, but it's not the way it currently works.

So you want to default to no group access regardless of the directory
permissions, with an option to enable group access that must be
explicitly specified? That seems like a reasonable option to me; note
that initdb does seem to chdir() an existing directory.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

#30Michael Paquier
Michael Paquier
michael@paquier.xyz
In reply to: Robert Haas (#29)
Re: pg_basebackup ignores the existing data directory permissions

On Mon, Mar 18, 2019 at 11:45:05AM -0400, Robert Haas wrote:

So you want to default to no group access regardless of the directory
permissions, with an option to enable group access that must be
explicitly specified? That seems like a reasonable option to me; note
that initdb does seem to chdir() an existing directory.

Hm. We have been assuming that the contents of a base backup inherit
the permission of the source when using pg_basebackup because this
allows users to keep a nodes in a consistent state without deciding
which option to use. Do you mean that you would like to enforce the
permissions of only the root directory if it exists? Or the root
directory with all its contents? The former may be fine. The latter
is definitely not.
--
Michael

#31Haribabu Kommi
Haribabu Kommi
kommi.haribabu@gmail.com
In reply to: Michael Paquier (#30)
Re: pg_basebackup ignores the existing data directory permissions

On Tue, Mar 19, 2019 at 5:29 PM Michael Paquier <michael@paquier.xyz> wrote:

On Mon, Mar 18, 2019 at 11:45:05AM -0400, Robert Haas wrote:

So you want to default to no group access regardless of the directory
permissions, with an option to enable group access that must be
explicitly specified? That seems like a reasonable option to me; note
that initdb does seem to chdir() an existing directory.

Hm. We have been assuming that the contents of a base backup inherit
the permission of the source when using pg_basebackup because this
allows users to keep a nodes in a consistent state without deciding
which option to use. Do you mean that you would like to enforce the
permissions of only the root directory if it exists? Or the root
directory with all its contents? The former may be fine. The latter
is definitely not.

As per my understanding going through the discussion, the option is for
root directory with all its contents also.

How about the following change?

pg_basebackup --> copies the contents of the src directory (with group
access)
and even the root directory permissions.

pg_basebackup --no-group-access --> copies the contents of the src
directory
(with no group access) even for the root directory.

So the default behavior works for many people, others that needs restrict
behavior
can use the new option.

Regards,
Haribabu Kommi
Fujitsu Australia

#32Peter Eisentraut
Peter Eisentraut
peter.eisentraut@2ndquadrant.com
In reply to: Robert Haas (#29)
Re: pg_basebackup ignores the existing data directory permissions

On 2019-03-18 16:45, Robert Haas wrote:

I'm strongly in favor of keeping initdb and pg_basebackup options
similar and consistent. They are both ways to initialize data directories.

You'll note that initdb does not behave the way you describe. It's not
unreasonable behavior, but it's not the way it currently works.

So you want to default to no group access regardless of the directory
permissions, with an option to enable group access that must be
explicitly specified? That seems like a reasonable option to me; note
that initdb does seem to chdir() an existing directory.

I think that would have been my preference, but PG11 is already shipping
with the current behavior, so I'm not sure whether that should be changed.

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

#33Peter Eisentraut
Peter Eisentraut
peter.eisentraut@2ndquadrant.com
In reply to: Haribabu Kommi (#31)
Re: pg_basebackup ignores the existing data directory permissions

On 2019-03-19 08:34, Haribabu Kommi wrote:

How about the following change?

pg_basebackup  --> copies the contents of the src directory (with group
access) 
and even the root directory permissions.

pg_basebackup --no-group-access   --> copies the contents of the src
directory 
(with no group access) even for the root directory.

I'm OK with that. Perhaps a positive option --allow-group-access would
also be useful.

Let's make sure the behavior of these options is aligned with initdb.
And write tests for each variant.

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

#34Robert Haas
Robert Haas
robertmhaas@gmail.com
In reply to: Michael Paquier (#30)
Re: pg_basebackup ignores the existing data directory permissions

On Tue, Mar 19, 2019 at 2:29 AM Michael Paquier <michael@paquier.xyz> wrote:

Hm. We have been assuming that the contents of a base backup inherit
the permission of the source when using pg_basebackup because this
allows users to keep a nodes in a consistent state without deciding
which option to use. Do you mean that you would like to enforce the
permissions of only the root directory if it exists? Or the root
directory with all its contents? The former may be fine. The latter
is definitely not.

Why not?

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

#35Haribabu Kommi
Haribabu Kommi
kommi.haribabu@gmail.com
In reply to: Peter Eisentraut (#33)
Re: pg_basebackup ignores the existing data directory permissions

On Thu, Mar 21, 2019 at 3:02 AM Peter Eisentraut <
peter.eisentraut@2ndquadrant.com> wrote:

On 2019-03-19 08:34, Haribabu Kommi wrote:

How about the following change?

pg_basebackup --> copies the contents of the src directory (with group
access)
and even the root directory permissions.

pg_basebackup --no-group-access --> copies the contents of the src
directory
(with no group access) even for the root directory.

I'm OK with that. Perhaps a positive option --allow-group-access would
also be useful.

Do you want both --allow-group-access and --no-group-access options to
be added to pg_basebackup?

Without --allow-group-access is automatically --no-group-access?

Or you want pg_basebackup independently decide the group access irrespective
of the source directory?

Even if the source directory is "not group access", pg_basebackup
--allow-group-access
make it standby as "group access".

source directory is "group access", pg_basebackup --no-group-access make it
"no group access" standby directory.

Default behavior of pg_basebackup is just to copy same as source directory?

Let's make sure the behavior of these options is aligned with initdb.
And write tests for each variant.

OK.

Regards,
Haribabu Kommi
Fujitsu Australia

#36Michael Paquier
Michael Paquier
michael@paquier.xyz
In reply to: Robert Haas (#34)
Re: pg_basebackup ignores the existing data directory permissions

On Thu, Mar 21, 2019 at 02:56:24PM -0400, Robert Haas wrote:

On Tue, Mar 19, 2019 at 2:29 AM Michael Paquier <michael@paquier.xyz> wrote:

Hm. We have been assuming that the contents of a base backup inherit
the permission of the source when using pg_basebackup because this
allows users to keep a nodes in a consistent state without deciding
which option to use. Do you mean that you would like to enforce the
permissions of only the root directory if it exists? Or the root
directory with all its contents? The former may be fine. The latter
is definitely not.

Why not?

Because we have released v11 so as we respect the permissions set on
the source instead from which the backup is taken for all the folder's
content. If we begin to enforce it we may break some cases. If a new
option is introduced, it seems to me that the default should remain
what has been released with v11, but that it is additionally possible
to enforce group permissions or non-group permissions at will on the
backup taken for all the contents in the data folder, including the
root folder, created manually or not before running the pg_basebackup
command.
--
Michael

#37Haribabu Kommi
Haribabu Kommi
kommi.haribabu@gmail.com
In reply to: Michael Paquier (#36)
Re: pg_basebackup ignores the existing data directory permissions

On Fri, Mar 22, 2019 at 11:42 AM Michael Paquier <michael@paquier.xyz>
wrote:

On Thu, Mar 21, 2019 at 02:56:24PM -0400, Robert Haas wrote:

On Tue, Mar 19, 2019 at 2:29 AM Michael Paquier <michael@paquier.xyz>

wrote:

Hm. We have been assuming that the contents of a base backup inherit
the permission of the source when using pg_basebackup because this
allows users to keep a nodes in a consistent state without deciding
which option to use. Do you mean that you would like to enforce the
permissions of only the root directory if it exists? Or the root
directory with all its contents? The former may be fine. The latter
is definitely not.

Why not?

Because we have released v11 so as we respect the permissions set on
the source instead from which the backup is taken for all the folder's
content. If we begin to enforce it we may break some cases. If a new
option is introduced, it seems to me that the default should remain
what has been released with v11, but that it is additionally possible
to enforce group permissions or non-group permissions at will on the
backup taken for all the contents in the data folder, including the
root folder, created manually or not before running the pg_basebackup
command.

How about letting the pg_basebackup to decide group permissions of the
standby directory irrespective of the primary directory permissions.

Default - permissions are same as primary
--allow-group-access - standby directory have group access permissions
--no-group--access - standby directory doesn't have group permissions

The last two options behave irrespective of the primary directory
permissions.

opinions?

Regards,
Haribabu Kommi
Fujitsu Australia

#38Michael Paquier
Michael Paquier
michael@paquier.xyz
In reply to: Haribabu Kommi (#37)
Re: pg_basebackup ignores the existing data directory permissions

On Fri, Mar 22, 2019 at 02:45:24PM +1100, Haribabu Kommi wrote:

How about letting the pg_basebackup to decide group permissions of the
standby directory irrespective of the primary directory permissions.

Default - permissions are same as primary
--allow-group-access - standby directory have group access permissions
--no-group--access - standby directory doesn't have group permissions

The last two options behave irrespective of the primary directory
permissions.

Yes, I'd imagine that we would want to be able to define three
different behaviors, by either having a set of options, or a sinple
option with a switch, say --group-access:
- "inherit" causes the permissions to be inherited from the source
node, and that's the default.
- "none" enforces the default 0700/0600.
- "group" enforces group read access.
--
Michael

#39Peter Eisentraut
Peter Eisentraut
peter.eisentraut@2ndquadrant.com
In reply to: Michael Paquier (#38)
Re: pg_basebackup ignores the existing data directory permissions

On 2019-03-22 05:00, Michael Paquier wrote:

On Fri, Mar 22, 2019 at 02:45:24PM +1100, Haribabu Kommi wrote:

How about letting the pg_basebackup to decide group permissions of the
standby directory irrespective of the primary directory permissions.

Default - permissions are same as primary
--allow-group-access - standby directory have group access permissions
--no-group--access - standby directory doesn't have group permissions

The last two options behave irrespective of the primary directory
permissions.

Yes, I'd imagine that we would want to be able to define three
different behaviors, by either having a set of options, or a sinple
option with a switch, say --group-access:
- "inherit" causes the permissions to be inherited from the source
node, and that's the default.
- "none" enforces the default 0700/0600.
- "group" enforces group read access.

Yes, we could use those three behaviors.

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

#40Haribabu Kommi
Haribabu Kommi
kommi.haribabu@gmail.com
In reply to: Peter Eisentraut (#39)
1 attachment(s)
Re: pg_basebackup ignores the existing data directory permissions

On Sat, Mar 23, 2019 at 2:23 AM Peter Eisentraut <
peter.eisentraut@2ndquadrant.com> wrote:

On 2019-03-22 05:00, Michael Paquier wrote:

On Fri, Mar 22, 2019 at 02:45:24PM +1100, Haribabu Kommi wrote:

How about letting the pg_basebackup to decide group permissions of the
standby directory irrespective of the primary directory permissions.

Default - permissions are same as primary
--allow-group-access - standby directory have group access permissions
--no-group--access - standby directory doesn't have group permissions

The last two options behave irrespective of the primary directory
permissions.

Yes, I'd imagine that we would want to be able to define three
different behaviors, by either having a set of options, or a sinple
option with a switch, say --group-access:
- "inherit" causes the permissions to be inherited from the source
node, and that's the default.
- "none" enforces the default 0700/0600.
- "group" enforces group read access.

Yes, we could use those three behaviors.

Thanks for all your opinions, here I attached an updated patch as discussed.

New option -g --group-mode is added to pg_basebackup to specify the
group access permissions.

inherit - same permissions as source instance (default)
none - No group permissions irrespective of source instance
group - group permissions irrespective of source instance

With the above additional options, the pg_basebackup is able to control
the access permissions of the backup files, but when it comes to tar mode
all the files are sent from the server and stored as it is in backup, to
support
tar mode group access mode control, the BASE BACKUP protocol is
enhanced with new option GROUP_MODE 'none' or GROUP_MODE 'group'
to control the file permissions before they are sent to backup. Sending
GROUP_MODE to the server depends on the -g option received to the
pg_basebackup utility.

comments?

Regards,
Haribabu Kommi
Fujitsu Australia

Attachments:

0001-New-pg_basebackup-g-option-to-control-the-group-acce.patchapplication/octet-stream; name=0001-New-pg_basebackup-g-option-to-control-the-group-acce.patch
#41Robert Haas
Robert Haas
robertmhaas@gmail.com
In reply to: Michael Paquier (#36)
Re: pg_basebackup ignores the existing data directory permissions

On Thu, Mar 21, 2019 at 8:42 PM Michael Paquier <michael@paquier.xyz> wrote:

Why not?

Because we have released v11 so as we respect the permissions set on
the source instead from which the backup is taken for all the folder's
content. If we begin to enforce it we may break some cases. If a new
option is introduced, it seems to me that the default should remain
what has been released with v11, but that it is additionally possible
to enforce group permissions or non-group permissions at will on the
backup taken for all the contents in the data folder, including the
root folder, created manually or not before running the pg_basebackup
command.

I don't agree with that logic, because setting the permissions of the
content one way and the directory another cannot really be what anyone
wants.

If we're going to go with -g {inherit|none|group} then -g inherit
ought to do what was originally proposed on this thread -- i.e. set
the directory permissions to match the contents. I don't think that's
a change that can or should be back-patched, but we should make it
consistent as part of this cleanup.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

#42Michael Paquier
Michael Paquier
michael@paquier.xyz
In reply to: Robert Haas (#41)
Re: pg_basebackup ignores the existing data directory permissions

On Mon, Mar 25, 2019 at 09:08:23AM -0400, Robert Haas wrote:

If we're going to go with -g {inherit|none|group} then -g inherit
ought to do what was originally proposed on this thread -- i.e. set
the directory permissions to match the contents. I don't think that's
a change that can or should be back-patched, but we should make it
consistent as part of this cleanup.

No objections from me to do that actually. That's a small
compatibility break, but with the new options it is possible to live
with. Perhaps we should add into initdb the same switch? Except that
"inherit" makes no sense there.
--
Michael

#43Michael Paquier
Michael Paquier
michael@paquier.xyz
In reply to: Haribabu Kommi (#40)
Re: pg_basebackup ignores the existing data directory permissions

On Sun, Mar 24, 2019 at 10:30:47PM +1100, Haribabu Kommi wrote:

With the above additional options, the pg_basebackup is able to control
the access permissions of the backup files, but when it comes to tar mode
all the files are sent from the server and stored as it is in backup, to
support
tar mode group access mode control, the BASE BACKUP protocol is
enhanced with new option GROUP_MODE 'none' or GROUP_MODE 'group'
to control the file permissions before they are sent to backup. Sending
GROUP_MODE to the server depends on the -g option received to the
pg_basebackup utility.

Do we really want to extend the replication protocol to control that?
I am really questioning if we should keep this stuff isolated within
pg_basebackup or not. At the same time, it may be confusing to have
BASE_BACKUP only use the permissions inherited from the data
directory, so some input from folks maintaining an external backup
tool is welcome.
--
Michael

#44Haribabu Kommi
Haribabu Kommi
kommi.haribabu@gmail.com
In reply to: Michael Paquier (#43)
Re: pg_basebackup ignores the existing data directory permissions

On Tue, Mar 26, 2019 at 1:27 PM Michael Paquier <michael@paquier.xyz> wrote:

On Sun, Mar 24, 2019 at 10:30:47PM +1100, Haribabu Kommi wrote:

With the above additional options, the pg_basebackup is able to control
the access permissions of the backup files, but when it comes to tar mode
all the files are sent from the server and stored as it is in backup, to
support
tar mode group access mode control, the BASE BACKUP protocol is
enhanced with new option GROUP_MODE 'none' or GROUP_MODE 'group'
to control the file permissions before they are sent to backup. Sending
GROUP_MODE to the server depends on the -g option received to the
pg_basebackup utility.

Thanks for the review.

Do we really want to extend the replication protocol to control that?

As the backup data is passed in tar format and if the pg_basebackup
is also storing it in tar format, i feel changing the permissions on tar
creation is easier than regenerating the received tar with different
permissions at pg_basebackup side.

Other than tar format, changing only in pg_basebackup can support
independent group access permissions of the standby directory.

I am really questioning if we should keep this stuff isolated within

pg_basebackup or not. At the same time, it may be confusing to have
BASE_BACKUP only use the permissions inherited from the data
directory, so some input from folks maintaining an external backup
tool is welcome.

That would be good to hear what other external backup tool authors
think of this change.

Regards,
Haribabu Kommi
Fujitsu Australia

#45Peter Eisentraut
Peter Eisentraut
peter.eisentraut@2ndquadrant.com
In reply to: Michael Paquier (#43)
Re: pg_basebackup ignores the existing data directory permissions

On 2019-03-26 03:26, Michael Paquier wrote:

Do we really want to extend the replication protocol to control that?

Perhaps we are losing sight of the original problem, which is that if
you create the target directory with the wrong permissions then ... it
has the wrong permissions. And you are free to change the permissions
at any time. Many of the proposed solutions sound excessively
complicated relative to that.

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

#46David Steele
David Steele
david@pgmasters.net
In reply to: Haribabu Kommi (#44)
Re: Re: pg_basebackup ignores the existing data directory permissions

On 3/26/19 3:59 AM, Haribabu Kommi wrote:

I am really questioning if we should keep this stuff isolated within
pg_basebackup or not.  At the same time, it may be confusing to have
BASE_BACKUP only use the permissions inherited from the data
directory, so some input from folks maintaining an external backup
tool is welcome.

That would be good to hear what other external backup tool authors
think of this change.

I'm OK with the -g (inherit|none|group) option as implemented. I prefer
the default as it is (inherit), which makes sense since I wrote it that way.

Having BASE_BACKUP set the permissions inside the tar file seems OK as
well. I'm not aware of any external solutions that are using the
replication protocol directly - I believe they all use pg_basebackup, so
I don't think they would need to change anything.

Having said that, I think setting permissions is a pretty trivial thing
to do and there are plenty of possible scenarios that are still not
covered here. I have no objections to the patch but it seems like overkill.

Regards,
--
-David
david@pgmasters.net

#47Haribabu Kommi
Haribabu Kommi
kommi.haribabu@gmail.com
In reply to: Peter Eisentraut (#45)
Re: pg_basebackup ignores the existing data directory permissions

On Fri, Mar 29, 2019 at 9:05 PM Peter Eisentraut <
peter.eisentraut@2ndquadrant.com> wrote:

On 2019-03-26 03:26, Michael Paquier wrote:

Do we really want to extend the replication protocol to control that?

Perhaps we are losing sight of the original problem, which is that if
you create the target directory with the wrong permissions then ... it
has the wrong permissions. And you are free to change the permissions
at any time. Many of the proposed solutions sound excessively
complicated relative to that.

Yes, I agree that the proposed solution for fixing the original problem of
existing
data directory permissions with new group options to pg_basebackup is a
big.

why can't we just fix the permissions of the directory by default as per the
source instance? I feel with this change, it may be useful to many people
than problem.

From understanding of the thread discussion,

+1 by:

Michael Paquier
Robert Haas
Haribabu Kommi

-1 by:

Magnus Hagander
Peter Eisentraut

Does any one want to weigh their opinion on this patch to consider best
option for controlling the existing standby data directory permissions.

Regards,
Haribabu Kommi
Fujitsu Australia

#48Robert Haas
Robert Haas
robertmhaas@gmail.com
In reply to: Peter Eisentraut (#45)
Re: pg_basebackup ignores the existing data directory permissions

On Fri, Mar 29, 2019 at 6:05 AM Peter Eisentraut
<peter.eisentraut@2ndquadrant.com> wrote:

On 2019-03-26 03:26, Michael Paquier wrote:

Do we really want to extend the replication protocol to control that?

Perhaps we are losing sight of the original problem, which is that if
you create the target directory with the wrong permissions then ... it
has the wrong permissions. And you are free to change the permissions
at any time. Many of the proposed solutions sound excessively
complicated relative to that.

I don't think I agree with that characterization of the problem. I
mean, what do you mean by "wrong"? Perhaps you created the directory
with the "right" permissions, i.e. those you actually wanted, and then
pg_basebackup rather rudely insisted on ignoring them when it decided
how to set the permissions for the files inside that directory. On the
other hand, perhaps you wished to abdicate responsibility for security
decisions to whatever rule pg_basebackup uses, and it rather rudely
didn't bother to enforce that decision on the top level directory,
forcing you to think about a question you had decided to ignore.

I am not sure what solution is best here, but it is hard to imagine
that the status quo is the right thing.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

#49Alvaro Herrera
Alvaro Herrera
alvherre@2ndquadrant.com
In reply to: Robert Haas (#48)
Re: pg_basebackup ignores the existing data directory permissions

On 2019-Apr-03, Robert Haas wrote:

I am not sure what solution is best here, but it is hard to imagine
that the status quo is the right thing.

This patch has been dormant for months. There's been at lot of
discussion but it doesn't seem conclusive; it doesn't look like we know
what we actually want to do. Can I try to restart the discussion and
see if we can get to an agreement, so that somebody can implement it?
Failing that, it seems this patch would be Returned with Little Useful Feedback.

There seem to be multiple fine points here:

1. We want to have initdb and pg_basebackup behave consistently.

Maybe if we don't like that changing pg_basebackup would make it
behave differently to initdb, then we ought to change both tools'
default behavior, and give equivalent new options to both to select
the other(s?) behavior(s?). So I talk about "the tool" referring to
both initdb and pg_basebackup in the following.

2. Should the case of creating a new dir behave differently from using
an existing directory?

Probably for simplicity we want both cases to behave the same.
I mean that if an existing dir has group privs and we choose that the
default behavior is without group privs, then those would get removed
unless a cmd line arg is given. Contrariwise if we choose that group
perms are to be preserved if they exist, then we should create a new
dir with group privs unless an option is given.

3. Sometimes we want to have the tool keep the permissions of an
existing directory, but for pg_basebackup the user might sometimes
want to preserve the permissions of upstream instead.

It seems to me that we could choose the default to be the most secure
behavior (which AFAICT is not to have any group perms), and if the
user wants to preserve group perms in an existing dir (or give group
perms to a directory created by the tool) they can pass a bespoke
command line argument.

I think ultimately this means that upstream privs would go ignored by
pg_basebackup. Maybe we can add another cmdline option to enable
preserving such.

I hope I didn't completely misunderstand the thread -- always a
possibility.

--
Álvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

#50Michael Paquier
Michael Paquier
michael@paquier.xyz
In reply to: Alvaro Herrera (#49)
Re: pg_basebackup ignores the existing data directory permissions

On Wed, Sep 04, 2019 at 04:11:17PM -0400, Alvaro Herrera wrote:

This patch has been dormant for months. There's been at lot of
discussion but it doesn't seem conclusive; it doesn't look like we know
what we actually want to do. Can I try to restart the discussion and
see if we can get to an agreement, so that somebody can implement it?
Failing that, it seems this patch would be Returned with Little Useful
Feedback.

This thread has been idle for a couple of months, so I have marked the
CF entry as returned with little useful feedback.
--
Michael