Thread: Does recovery write to backup_label ?
Just saw this in a PG 11.6 cluster starting a recovery: 2020-02-07 10:45:40 EST FATAL: 42501: could not open file "backup_label": Permission denied 2020-02-07 10:45:40 EST LOCATION: fsync_fname_ext, fd.c:3531 The label file was written with mode 0400 by a script that got the contents from pg_stop_backup(boolean,boolean). But during recovery, it is being poked at by fsync_fname_ext which wants to open it O_RDWR. I had assumed the label file would be treated as readonly during recovery. If the file needs to have 0600 permissions, should there be a note in the nonexclusive-mode backup docs to say so? Regards, -Chap
Hi, On 2020-02-07 11:08:48 -0500, Chapman Flack wrote: > Just saw this in a PG 11.6 cluster starting a recovery: > > 2020-02-07 10:45:40 EST FATAL: 42501: could not open file > "backup_label": Permission denied > 2020-02-07 10:45:40 EST LOCATION: fsync_fname_ext, fd.c:3531 Well, we generally assume that files in the data directory are writable, with a very few exceptions. And we do need to be able rename backup_label to backup_label.old. Which strictly speaking doesn't require write permissions on the file - but I assume that's what triggers the issue here. There's IIRC platforms that don't like fsyncing files with a O_RDONLY fd, so if we want to rename safely (which entails fsyncing beforehand), we don't have much choice. > I had assumed the label file would be treated as readonly > during recovery. It is IIRC documented that it does get renamed... > If the file needs to have 0600 permissions, should there be > a note in the nonexclusive-mode backup docs to say so? I'm not convinced that that's useful. The default is that everything needs to be writable by postgres. The exceptions should be noted if anything, not the default. Greetings, Andres Freund
On 2/7/20 2:55 PM, Andres Freund wrote: >> If the file needs to have 0600 permissions, should there be >> a note in the nonexclusive-mode backup docs to say so? > > I'm not convinced that that's useful. The default is that everything > needs to be writable by postgres. The exceptions should be noted if > anything, not the default. Could this arguably be a special case, as most things in the datadir are put there by postgres, but the backup_label is now to be put there (and not even 'there' there, but added as a final step only to a 'backup-copy-of-there' there) by the poor schmuck who reads the non-exclusive backup docs as saying "retrieve this content from pg_stop_backup() and preserve in a file named backup_label" and can't think of any obvious reason to put write permission on a file that preserves immutable history in a backup? Regards, -Chap
On Sat, Feb 8, 2020 at 5:05 AM Chapman Flack <chap@anastigmatix.net> wrote: > > On 2/7/20 2:55 PM, Andres Freund wrote: > > >> If the file needs to have 0600 permissions, should there be > >> a note in the nonexclusive-mode backup docs to say so? > > > > I'm not convinced that that's useful. The default is that everything > > needs to be writable by postgres. The exceptions should be noted if > > anything, not the default. +1 > Could this arguably be a special case, as most things in the datadir > are put there by postgres, but the backup_label is now to be put there > (and not even 'there' there, but added as a final step only to a > 'backup-copy-of-there' there) by the poor schmuck who reads the > non-exclusive backup docs as saying "retrieve this content from > pg_stop_backup() and preserve in a file named backup_label" and can't > think of any obvious reason to put write permission on a file > that preserves immutable history in a backup? I have no strong objection to add more note about permissions of the files that the users put in the data directory. If we really do that, it'd be better to note about not only backup_label but also other files like tablespace_map, recovery.signal, promotion trigger file, etc. Regards, -- Fujii Masao
On 2/7/20 8:06 PM, Fujii Masao wrote: > On Sat, Feb 8, 2020 at 5:05 AM Chapman Flack <chap@anastigmatix.net> wrote: >> >> On 2/7/20 2:55 PM, Andres Freund wrote: >> >>>> If the file needs to have 0600 permissions, should there be >>>> a note in the nonexclusive-mode backup docs to say so? >>> >>> I'm not convinced that that's useful. The default is that everything >>> needs to be writable by postgres. The exceptions should be noted if >>> anything, not the default. > > +1 +1. In theory it would be more secure to only allow rename, but since Postgres can overwrite any other file in the cluster I don't see much value in making an exception for this file. >> Could this arguably be a special case, as most things in the datadir >> are put there by postgres, but the backup_label is now to be put there >> (and not even 'there' there, but added as a final step only to a >> 'backup-copy-of-there' there) by the poor schmuck who reads the >> non-exclusive backup docs as saying "retrieve this content from >> pg_stop_backup() and preserve in a file named backup_label" and can't >> think of any obvious reason to put write permission on a file >> that preserves immutable history in a backup? > > I have no strong objection to add more note about permissions > of the files that the users put in the data directory. If we really > do that, it'd be better to note about not only backup_label > but also other files like tablespace_map, recovery.signal, > promotion trigger file, etc. More documentation seems like a good idea, especially since non-exclusive backup requires the app to choose/set permissions. Regards, -- -David david@pgmasters.net