On Wed, Apr 04, 2018 at 08:03:54PM -0400, David Steele wrote: > On 4/2/18 2:28 AM, Michael Paquier wrote: >> The answer is no for me and likely the same for others, but I am raising >> the point for the archives. Should we relax >> check_ssl_key_file_permissions() for group permissions by the way from >> 0600 to 0640 when the file is owned by the user running Postgres? If we >> don't do that, then SSL private keys will need to be used with 0600, >> potentially breaking backups... At the same time this reduces the >> security of private keys but if the administrator is ready to accept >> group permissions that should be a risk he is ready to accept? > > I feel this should be considered in a separate patch. These files are > not created by initdb so it seems to be an admin/packaging issue. There > is always the option to locate the certs outside the data directory and > some distros do that by default.
OK, I agree with your final position. Let's treat it later in a separate thread, if need be. However this is not really mandatory. >> I am pretty sure that we want more documentation in pg_basebackup, >> pg_rewind and pg_resetwal telling that those consider grouping access. > > I think this makes sense for pg_basebackup, pg_receivewal, and > pg_recvlogical so I have added notes for those. Not sure I'm happy with > the language but at least we have something to bikeshed. > > It seems to me that pg_resetwal and pg_rewind should be expected to not > break the permissions in PGDATA, just as they do now. OK, I think that I am fine with this logic. > I decided that the constants were a bit confusing in general. What does > "default" mean, anyway? The value that user should have if he decides to not enforce it. > Instead I have created variables in file_perm.c > that hold the current file create mode, dir create mode, and mode mask. > All call sites use those variables (e.g. pg_dir_create_mode), which I > think are much clear. Hm. I personally find that even more confusing, especially with SetDataDirectoryCreatePerm which basically is the same as SetConfigOption for the backend. In your case pg_dir_create_mode is not aimed at remaining a constant as you may change it depending on if grouping is enabled or not... And all the other iterations of such variables in src/common/ are constants (see NumScanKeywords or forkNames[] for example), so I cannot see with a good eye this change. You also forgot a call to SetDataDirectoryCreatePerm or pg_dir_create_mode remains to its default. The interface of file_perm.h that you are introducing is not confusing anymore though.. -- Michael
signature.asc
Description: PGP signature
