Re: [HACKERS] pg_tablespace_size()
Tom Lane wrote: Magnus Hagander [EMAIL PROTECTED] writes: Tom Lane wrote: * Just remove the above-quoted lines. Superusers should be allowed to shoot themselves in the foot. (I'm not actually sure that there would be any bad consequences from putting an ordinary table into pg_global anyway. Is there ever *any* reason for doing this? Probably not a good one, and I suspect there would be some funny misbehaviors if you were to clone the database containing the table. The table would be physically shared but logically not. yuck. What I'm inclined to do about it is is adopt my suggestion #2 (move the location of the defense), since permission denied for a superuser is a pretty unhelpful error message anyway. Ok. Works for me. * Decide that we should allow anyone to do pg_tablespace_size('pg_global') and put in a special wart for that in dbsize.c. This wasn't part of the original agreement but maybe there's a case to be made for it. That's pretty much the same thing, right? Well, no, I was suggesting that we might want to special-case pg_global as a tablespace that anyone (superuser or no) could get the size of. This is actually independent of whether we change the aclmask behavior. Oh, ok, I see. Then my vote is for the other solution = not allowing anybody to do this. //Magnus ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [HACKERS] pg_tablespace_size()
Magnus Hagander [EMAIL PROTECTED] writes: Tom Lane wrote: * Just remove the above-quoted lines. Superusers should be allowed to shoot themselves in the foot. (I'm not actually sure that there would be any bad consequences from putting an ordinary table into pg_global anyway. Is there ever *any* reason for doing this? Probably not a good one, and I suspect there would be some funny misbehaviors if you were to clone the database containing the table. The table would be physically shared but logically not. What I'm inclined to do about it is is adopt my suggestion #2 (move the location of the defense), since permission denied for a superuser is a pretty unhelpful error message anyway. * Decide that we should allow anyone to do pg_tablespace_size('pg_global') and put in a special wart for that in dbsize.c. This wasn't part of the original agreement but maybe there's a case to be made for it. That's pretty much the same thing, right? Well, no, I was suggesting that we might want to special-case pg_global as a tablespace that anyone (superuser or no) could get the size of. This is actually independent of whether we change the aclmask behavior. regards, tom lane ---(end of broadcast)--- TIP 7: You can help support the PostgreSQL project by donating at http://www.postgresql.org/about/donate
Re: [HACKERS] pg_tablespace_size()
Tom Lane wrote: I wrote: [ squint... ] There is something wrong here, because a superuser should certainly pass the aclcheck test. I don't know where the bug is but this is not the correct fix. OK, after looking, the issue is this wart in pg_tablespace_aclmask(): /* * Only shared relations can be stored in global space; don't let even * superusers override this */ if (spc_oid == GLOBALTABLESPACE_OID !IsBootstrapProcessingMode()) return 0; /* Otherwise, superusers bypass all permission checking. */ Yup, that was my point. There are a number of ways that we could deal with this: * Just remove the above-quoted lines. Superusers should be allowed to shoot themselves in the foot. (I'm not actually sure that there would be any bad consequences from putting an ordinary table into pg_global anyway. I think I wrote the above code in fear that some parts of the system would equate reltablespace = pg_global with relisshared, but AFAICS that's not the case now.) Is there ever *any* reason for doing this? If there isn't, I don't think we should provide just that foot-gun. But if there is any case where it makes sense to do that, then the superuser should probably be allowed to do it. * Remove the above lines and instead put a defense into heap_create. This might be better design anyway since a more specific error could be reported. * Leave aclchk.c as-is and apply Magnus' patch to allow superusers to bypass the check in pg_tablespace_size. See foot-gun above. If we want to keep the check, I think that my patch is fine. If we don't, then taking out that code is better. * Decide that we should allow anyone to do pg_tablespace_size('pg_global') and put in a special wart for that in dbsize.c. This wasn't part of the original agreement but maybe there's a case to be made for it. That's pretty much the same thing, right? Since the acl check will check for pg_global, and if it's anything else, let superuser in. It's gotta be easier to read if it's just a plain superuser check, I think. //Magnus ---(end of broadcast)--- TIP 1: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [HACKERS] pg_tablespace_size()
Tom Lane wrote: Magnus Hagander [EMAIL PROTECTED] writes: Attached is a patch I want to apply for this. Toms message at http://archives.postgresql.org/pgsql-hackers/2007-10/msg00448.php makes me bring it up here before I apply it. [ squint... ] There is something wrong here, because a superuser should certainly pass the aclcheck test. I don't know where the bug is but this is not the correct fix. Are you sure? pg_tablespace_aclmask() has: /* * Only shared relations can be stored in global space; don't let even * superusers override this */ if (spc_oid == GLOBALTABLESPACE_OID !IsBootstrapProcessingMode()) return 0; And this is what causes the failure. I certainly didn't want to take out that check, so short-circuiting it in the way I did seemed right.. //Magnus ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [HACKERS] pg_tablespace_size()
Tom Lane wrote: Dave Page [EMAIL PROTECTED] writes: select pg_tablespace_size('pg_global'); ERROR: permission denied for tablespace pg_global Can this be fixed please? What's to be fixed? That's exactly the result I'd expect given the previously-agreed-to behavior for pg_tablespace_size. I know we said tablespace requires CREATE privs, but certainly the super-user should be able to use the function too. -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://postgres.enterprisedb.com + If your life is a hard drive, Christ can be your backup. + ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [HACKERS] pg_tablespace_size()
Magnus Hagander [EMAIL PROTECTED] writes: Attached is a patch I want to apply for this. Toms message at http://archives.postgresql.org/pgsql-hackers/2007-10/msg00448.php makes me bring it up here before I apply it. [ squint... ] There is something wrong here, because a superuser should certainly pass the aclcheck test. I don't know where the bug is but this is not the correct fix. regards, tom lane ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [HACKERS] pg_tablespace_size()
On Fri, Oct 12, 2007 at 12:20:08PM +0100, Dave Page wrote: Following on from Hiroshi's earlier post: http://archives.postgresql.org/pgsql-hackers/2007-10/msg00324.php Per http://archives.postgresql.org/pgsql-hackers/2007-08/msg01018.php, superusers should be able to use pg_tablespace_size() on any tablespace, however when logged in as postgres on a beta1 server, I get: -- Executing query: select pg_tablespace_size('pg_global'); ERROR: permission denied for tablespace pg_global Can this be fixed please? Attached is a patch I want to apply for this. Toms message at http://archives.postgresql.org/pgsql-hackers/2007-10/msg00448.php makes me bring it up here before I apply it. I think the correct behaviour is that superusers should be able to do it, and that's in the thread that Dave referred to. But since Tom said it's by design in the message above, I'd like confirmation that this is ok. Since I think that allowing superusers to do it is the only sensible thing, I will apply this patch unless someone objects :-) //Magnus Index: src/backend/utils/adt/dbsize.c === RCS file: /cvsroot/pgsql/src/backend/utils/adt/dbsize.c,v retrieving revision 1.14 diff -c -r1.14 dbsize.c *** src/backend/utils/adt/dbsize.c 29 Aug 2007 17:24:29 - 1.14 --- src/backend/utils/adt/dbsize.c 12 Oct 2007 14:38:32 - *** *** 164,171 /* * User must have CREATE privilege for target tablespace, either explicitly * granted or implicitly because it is default for current database. */ ! if (tblspcOid != MyDatabaseTableSpace) { aclresult = pg_tablespace_aclcheck(tblspcOid, GetUserId(), ACL_CREATE); if (aclresult != ACLCHECK_OK) --- 164,175 /* * User must have CREATE privilege for target tablespace, either explicitly * granted or implicitly because it is default for current database. +* +* Specifically check for superuser permissions here, since +* pg_tablespace_aclcheck() will deny access to pg_global even for +* superusers. */ ! if (tblspcOid != MyDatabaseTableSpace !superuser()) { aclresult = pg_tablespace_aclcheck(tblspcOid, GetUserId(), ACL_CREATE); if (aclresult != ACLCHECK_OK) ---(end of broadcast)--- TIP 9: In versions below 8.0, the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
[HACKERS] pg_tablespace_size()
Following on from Hiroshi's earlier post: http://archives.postgresql.org/pgsql-hackers/2007-10/msg00324.php Per http://archives.postgresql.org/pgsql-hackers/2007-08/msg01018.php, superusers should be able to use pg_tablespace_size() on any tablespace, however when logged in as postgres on a beta1 server, I get: -- Executing query: select pg_tablespace_size('pg_global'); ERROR: permission denied for tablespace pg_global Can this be fixed please? Regards, Dave ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [HACKERS] pg_tablespace_size()
I wrote: [ squint... ] There is something wrong here, because a superuser should certainly pass the aclcheck test. I don't know where the bug is but this is not the correct fix. OK, after looking, the issue is this wart in pg_tablespace_aclmask(): /* * Only shared relations can be stored in global space; don't let even * superusers override this */ if (spc_oid == GLOBALTABLESPACE_OID !IsBootstrapProcessingMode()) return 0; /* Otherwise, superusers bypass all permission checking. */ There are a number of ways that we could deal with this: * Just remove the above-quoted lines. Superusers should be allowed to shoot themselves in the foot. (I'm not actually sure that there would be any bad consequences from putting an ordinary table into pg_global anyway. I think I wrote the above code in fear that some parts of the system would equate reltablespace = pg_global with relisshared, but AFAICS that's not the case now.) * Remove the above lines and instead put a defense into heap_create. This might be better design anyway since a more specific error could be reported. * Leave aclchk.c as-is and apply Magnus' patch to allow superusers to bypass the check in pg_tablespace_size. * Decide that we should allow anyone to do pg_tablespace_size('pg_global') and put in a special wart for that in dbsize.c. This wasn't part of the original agreement but maybe there's a case to be made for it. Thoughts? regards, tom lane ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [HACKERS] pg_tablespace_size()
Dave Page [EMAIL PROTECTED] writes: In the message I quoted at the end of the long discussion on the topic you assured me it would work for superusers. Is there any reason the superuser shouldn't see the size of pg_global? Oh, you were superuser? Then something's broken, agreed. regards, tom lane ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [HACKERS] pg_tablespace_size()
Tom Lane wrote: Dave Page [EMAIL PROTECTED] writes: select pg_tablespace_size('pg_global'); ERROR: permission denied for tablespace pg_global Can this be fixed please? What's to be fixed? That's exactly the result I'd expect given the previously-agreed-to behavior for pg_tablespace_size. In the message I quoted at the end of the long discussion on the topic you assured me it would work for superusers. Is there any reason the superuser shouldn't see the size of pg_global? Regards, Dave. ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [HACKERS] pg_tablespace_size()
Dave Page [EMAIL PROTECTED] writes: select pg_tablespace_size('pg_global'); ERROR: permission denied for tablespace pg_global Can this be fixed please? What's to be fixed? That's exactly the result I'd expect given the previously-agreed-to behavior for pg_tablespace_size. regards, tom lane ---(end of broadcast)--- TIP 7: You can help support the PostgreSQL project by donating at http://www.postgresql.org/about/donate