Обсуждение: pg_basebackup ignores the existing data directory permissions

Поиск
Список
Период
Сортировка

pg_basebackup ignores the existing data directory permissions

От
Haribabu Kommi
Дата:
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
Вложения

Re: pg_basebackup ignores the existing data directory permissions

От
Michael Paquier
Дата:
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

Вложения

Re: pg_basebackup ignores the existing data directory permissions

От
Haribabu Kommi
Дата:

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

Re: pg_basebackup ignores the existing data directory permissions

От
Michael Paquier
Дата:
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

Вложения

Re: pg_basebackup ignores the existing data directory permissions

От
Haribabu Kommi
Дата:

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

Re: pg_basebackup ignores the existing data directory permissions

От
Michael Paquier
Дата:
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

Вложения

Re: pg_basebackup ignores the existing data directory permissions

От
Magnus Hagander
Дата:
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.
 
--

Re: pg_basebackup ignores the existing data directory permissions

От
Haribabu Kommi
Дата:

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

Re: pg_basebackup ignores the existing data directory permissions

От
Michael Paquier
Дата:
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

Вложения

Re: pg_basebackup ignores the existing data directory permissions

От
Kyotaro HORIGUCHI
Дата:
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



Re: pg_basebackup ignores the existing data directory permissions

От
Michael Paquier
Дата:
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

Вложения

Re: pg_basebackup ignores the existing data directory permissions

От
Haribabu Kommi
Дата:

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

Re: pg_basebackup ignores the existing data directory permissions

От
Michael Paquier
Дата:
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

Вложения

Re: pg_basebackup ignores the existing data directory permissions

От
Magnus Hagander
Дата:

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.

--

Re: pg_basebackup ignores the existing data directory permissions

От
Haribabu Kommi
Дата:

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

Re: pg_basebackup ignores the existing data directory permissions

От
Peter Eisentraut
Дата:
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


Re: pg_basebackup ignores the existing data directory permissions

От
Haribabu Kommi
Дата:

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

Re: pg_basebackup ignores the existing data directory permissions

От
Peter Eisentraut
Дата:
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


Re: pg_basebackup ignores the existing data directory permissions

От
Haribabu Kommi
Дата:

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

Re: pg_basebackup ignores the existing data directory permissions

От
Robert Haas
Дата:
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


Re: pg_basebackup ignores the existing data directory permissions

От
Stephen Frost
Дата:
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

Вложения

Re: pg_basebackup ignores the existing data directory permissions

От
Magnus Hagander
Дата:
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.
 
--

Re: pg_basebackup ignores the existing data directory permissions

От
Michael Paquier
Дата:
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

Вложения

Re: pg_basebackup ignores the existing data directory permissions

От
Kyotaro HORIGUCHI
Дата:
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



Re: pg_basebackup ignores the existing data directory permissions

От
Peter Eisentraut
Дата:
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


Re: pg_basebackup ignores the existing data directory permissions

От
Peter Eisentraut
Дата:
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


Re: pg_basebackup ignores the existing data directory permissions

От
Robert Haas
Дата:
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


Re: pg_basebackup ignores the existing data directory permissions

От
Peter Eisentraut
Дата:
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


Re: pg_basebackup ignores the existing data directory permissions

От
Robert Haas
Дата:
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


Re: pg_basebackup ignores the existing data directory permissions

От
Michael Paquier
Дата:
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

Вложения

Re: pg_basebackup ignores the existing data directory permissions

От
Haribabu Kommi
Дата:

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

Re: pg_basebackup ignores the existing data directory permissions

От
Peter Eisentraut
Дата:
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


Re: pg_basebackup ignores the existing data directory permissions

От
Peter Eisentraut
Дата:
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


Re: pg_basebackup ignores the existing data directory permissions

От
Robert Haas
Дата:
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


Re: pg_basebackup ignores the existing data directory permissions

От
Haribabu Kommi
Дата:

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

Re: pg_basebackup ignores the existing data directory permissions

От
Michael Paquier
Дата:
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

Вложения

Re: pg_basebackup ignores the existing data directory permissions

От
Haribabu Kommi
Дата:

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

Re: pg_basebackup ignores the existing data directory permissions

От
Michael Paquier
Дата:
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

Вложения

Re: pg_basebackup ignores the existing data directory permissions

От
Peter Eisentraut
Дата:
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


Re: pg_basebackup ignores the existing data directory permissions

От
Haribabu Kommi
Дата:
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
Вложения

Re: pg_basebackup ignores the existing data directory permissions

От
Robert Haas
Дата:
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


Re: pg_basebackup ignores the existing data directory permissions

От
Michael Paquier
Дата:
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

Вложения

Re: pg_basebackup ignores the existing data directory permissions

От
Michael Paquier
Дата:
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

Вложения

Re: pg_basebackup ignores the existing data directory permissions

От
Haribabu Kommi
Дата:

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

Re: pg_basebackup ignores the existing data directory permissions

От
Peter Eisentraut
Дата:
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



Re: Re: pg_basebackup ignores the existing data directory permissions

От
David Steele
Дата:
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



Re: pg_basebackup ignores the existing data directory permissions

От
Haribabu Kommi
Дата:

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

Re: pg_basebackup ignores the existing data directory permissions

От
Robert Haas
Дата:
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



Re: pg_basebackup ignores the existing data directory permissions

От
Alvaro Herrera
Дата:
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



Re: pg_basebackup ignores the existing data directory permissions

От
Michael Paquier
Дата:
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

Вложения