Robert, * Robert Haas (robertmh...@gmail.com) wrote: > On Fri, Jan 27, 2017 at 11:34 AM, Stephen Frost <sfr...@snowman.net> wrote: > > * Robert Haas (robertmh...@gmail.com) wrote: > >> OK, fair enough. get_raw_page() is clearly not something that we > >> really want everybody to have access to by default, but if it were up > >> to me, I'd change the permissions check inside the function to do a > >> check for select privileges on the relation, and then I'd change the > >> SQL script to revoke access from everybody but the superuser. > > > > The way to do properly do this would not be to have some conflation of > > the right to execute get_raw_page() combined with a typically granted > > right like 'SELECT'. Instead, it would be to have an explicit RAW_PAGE > > or similar permission which could be GRANT'd to a user for a particular > > relation, and then we could change the superuser() check into a check > > against that right, and call it done. > > That's moving the goalposts a very large distance. > > >> Actually, I think that's Stephen should have done something similar > >> for pgstattuple in commit fd321a1dfd64d30bf1652ea6b39b654304f68ae4, ... > > > > Requiring pgstattuple() to check if a user has any rights on a table > > means that an existing non-superuser monitoring tool that's calling > > pgstattuple() for summary information would be required to have SELECT > > rights across, most likely, all tables, even though the monitoring > > user's got no need for that level of access. Now, we could argue that > > having access to just one column would be enough for that user, but > > that's still more access than the monitoring user needs, and there's > > still the issue of new tables (and default privileges don't currently > > support column-level privileges anyway). > > ...whereas here you don't want to move the goalposts at all. I can't > understand this. When I want the superuser to have the option to hand > out pg_ls_dir() access for all directories pg_ls_dir() can access, you > complain that's too broad. Yet your own previous commit, which allows > pgstattuple() access to be granted, makes no provision for any > granularity of access control at all. And I think there is an > excellent argument - which I have made - that pgstattuple() is more > likely to expose sensitive information than pg_ls_dir().
You're completely ignoring the use-cases for which these are being done. I've outlined the precise use-case for pgstattuple()'s usage across the entire database for which the admin has granted the EXECUTE access in. I've not yet seen a use-case for access to pg_ls_dir() across all directories pg_ls_dir() can access. My recollection is that you brought up pg_wal, but that's hardly every directory, and some hypothetical tool which could intelligently figure out what orphaned files exist. For the former, I would recommend a function for exactly that (or perhaps something better which provides more than just a count of files), for the latter, that's something that I would be very worried that someone trying to implement outside of core would get wrong or which could be version-dependent. We've already got some code in core to find files left over from a crash and deal with them and perhaps that could be expanded to deal with other cases. > Even if we someday have the capability for people to grant pg_ls_dir() > access on a directory-by-directory basis, I am sure there will still > be a way for them to grant access on all the directories to which > pg_ls_dir() can access today; after all, that's just two directories, > and their subdirectories. At most somebody would have to make two > GRANTs. Removing the hard-coded superuser() checks allows that use > case now, and doesn't prohibit you from implementing the other thing > later when and if and when we reach agreement on it. With an appropriate use-case for it, I wouldn't be against it. I linked to both use-cases and a provided patch for finer-grained access to pg_ls_dir() and friends three years ago, which got shot down. I'm not against it if the community wishes to reconsider that decision and support having filesystem-like access where there's an appropriate use-case for it, and with fine-grained access control provided to meet that use-case. Further, as it relates to goal-posts, if your goal is to let an admin grant out the ability to see how many files are in pg_wal, you're spending a great deal more effort than that would require. I wouldn't object (and would actually appreciate) a function which is able to do exactly that, and I'd go work with Greg S-M right away to make sure that check_postgres.pl knows about that function and uses it in PG10 and above. I do object to someone coming along and saying "let's rip out all the superuser() checks" and it would seem that I'm not alone. Thanks! Stephen
signature.asc
Description: Digital signature
