From: Michael Paquier <michael@paquier.xyz>
To: Alvaro Herrera <alvherre@2ndquadrant.com>
Cc: clemens@ladisch.de
Cc: pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #15833: defining a comment on a domain constraint fails with wrong OID
Date: Mon, 10 Jun 2019 15:28:55 +0900
Message-ID: <20190610062855.GE2199@paquier.xyz> (raw)
In-Reply-To: <20190607054233.GG1736@paquier.xyz>
References: <15833-808e11904835d26f@postgresql.org>
<20190605181502.GA10753@alvherre.pgsql>
<20190607054233.GG1736@paquier.xyz>
On Fri, Jun 07, 2019 at 02:42:33PM +0900, Michael Paquier wrote:
> Well, it wouldn't be a problem to do a syscache lookup and then use
> the type from contypid, no? It seems to me that it would be more
> consistent to just add a pg_domain_constraint_ownercheck() in aclchk.c
> as all the syscache lookups happen their for all the other objects
> types. What do you think?
Attached is what I have in mind. There are already tests at the
bottom of constraints.source checking for comments on both table and
domain constraints, so my proposal is to run them with a dedicated
role. What do you think?
--
Michael
Attachments:dom-constraint-comments-v1.patch (text/x-diff, 4.9K) downloaddiff --git a/src/backend/catalog/aclchk.c b/src/backend/catalog/aclchk.cindex 2797af35c3..a155b76276 100644--- a/src/backend/catalog/aclchk.c+++ b/src/backend/catalog/aclchk.c@@ -33,6 +33,7 @@
#include "catalog/pg_authid.h"
#include "catalog/pg_cast.h"
#include "catalog/pg_collation.h"
+#include "catalog/pg_constraint.h"
#include "catalog/pg_conversion.h"
#include "catalog/pg_database.h"
#include "catalog/pg_default_acl.h"
@@ -4800,6 +4801,38 @@ pg_type_ownercheck(Oid type_oid, Oid roleid)
return has_privs_of_role(roleid, ownerId);
}
+/*+ * Ownership check for a domain constraint (specified by OID).+ *+ * Note that this relies on the ownership of the associated type.+ */+bool+pg_domain_constraint_ownercheck(Oid con_oid, Oid roleid)+{+ HeapTuple tuple;+ Oid typeId;++ /* Superusers bypass all permission checking. */+ if (superuser_arg(roleid))+ return true;++ tuple = SearchSysCache1(CONSTROID, ObjectIdGetDatum(con_oid));+ if (!HeapTupleIsValid(tuple))+ ereport(ERROR,+ (errcode(ERRCODE_UNDEFINED_OBJECT),+ errmsg("constraint with OID %u does not exist", con_oid)));++ typeId = ((Form_pg_constraint) GETSTRUCT(tuple))->contypid;++ ReleaseSysCache(tuple);++ /*+ * Fallback to type ownership check in this case as this is what domain+ * constraints rely on.+ */+ return pg_type_ownercheck(typeId, roleid);+}+
/*
* Ownership check for an operator (specified by OID).
*/
diff --git a/src/backend/catalog/objectaddress.c b/src/backend/catalog/objectaddress.cindex 2235e5626f..427f954629 100644--- a/src/backend/catalog/objectaddress.c+++ b/src/backend/catalog/objectaddress.c@@ -2295,10 +2295,13 @@ check_object_ownership(Oid roleid, ObjectType objtype, ObjectAddress address,
case OBJECT_TYPE:
case OBJECT_DOMAIN:
case OBJECT_ATTRIBUTE:
- case OBJECT_DOMCONSTRAINT:
if (!pg_type_ownercheck(address.objectId, roleid))
aclcheck_error_type(ACLCHECK_NOT_OWNER, address.objectId);
break;
+ case OBJECT_DOMCONSTRAINT:+ if (!pg_domain_constraint_ownercheck(address.objectId, roleid))+ aclcheck_error_type(ACLCHECK_NOT_OWNER, address.objectId);+ break;
case OBJECT_AGGREGATE:
case OBJECT_FUNCTION:
case OBJECT_PROCEDURE:
diff --git a/src/include/utils/acl.h b/src/include/utils/acl.hindex f4c160ee72..9adfd72c09 100644--- a/src/include/utils/acl.h+++ b/src/include/utils/acl.h@@ -287,6 +287,7 @@ extern void removeExtObjInitPriv(Oid objoid, Oid classoid);
/* ownercheck routines just return true (owner) or false (not) */
extern bool pg_class_ownercheck(Oid class_oid, Oid roleid);
extern bool pg_type_ownercheck(Oid type_oid, Oid roleid);
+extern bool pg_domain_constraint_ownercheck(Oid type_oid, Oid roleid);
extern bool pg_oper_ownercheck(Oid oper_oid, Oid roleid);
extern bool pg_proc_ownercheck(Oid proc_oid, Oid roleid);
extern bool pg_language_ownercheck(Oid lan_oid, Oid roleid);
diff --git a/src/test/regress/input/constraints.source b/src/test/regress/input/constraints.sourceindex 0abf95577e..111e65475f 100644--- a/src/test/regress/input/constraints.source+++ b/src/test/regress/input/constraints.source@@ -518,6 +518,10 @@ ALTER TABLE deferred_excl ADD EXCLUDE (f1 WITH =);
DROP TABLE deferred_excl;
-- Comments
+-- Setup a low-level role to enforce non-superuser checks.+CREATE ROLE regress_constraint_comments;+SET SESSION AUTHORIZATION regress_constraint_comments;+
CREATE TABLE constraint_comments_tbl (a int CONSTRAINT the_constraint CHECK (a > 0));
CREATE DOMAIN constraint_comments_dom AS int CONSTRAINT the_constraint CHECK (value > 0);
@@ -537,3 +541,5 @@ COMMENT ON CONSTRAINT the_constraint ON DOMAIN constraint_comments_dom IS NULL;
DROP TABLE constraint_comments_tbl;
DROP DOMAIN constraint_comments_dom;
+RESET SESSION AUTHORIZATION;+DROP ROLE regress_constraint_comments;diff --git a/src/test/regress/output/constraints.source b/src/test/regress/output/constraints.sourceindex 465d6da0e0..6313a4d71e 100644--- a/src/test/regress/output/constraints.source+++ b/src/test/regress/output/constraints.source@@ -704,6 +704,9 @@ ERROR: could not create exclusion constraint "deferred_excl_f1_excl"
DETAIL: Key (f1)=(3) conflicts with key (f1)=(3).
DROP TABLE deferred_excl;
-- Comments
+-- Setup a low-level role to enforce non-superuser checks.+CREATE ROLE regress_constraint_comments;+SET SESSION AUTHORIZATION regress_constraint_comments;
CREATE TABLE constraint_comments_tbl (a int CONSTRAINT the_constraint CHECK (a > 0));
CREATE DOMAIN constraint_comments_dom AS int CONSTRAINT the_constraint CHECK (value > 0);
COMMENT ON CONSTRAINT the_constraint ON constraint_comments_tbl IS 'yes, the comment';
@@ -722,3 +725,5 @@ COMMENT ON CONSTRAINT the_constraint ON constraint_comments_tbl IS NULL;
COMMENT ON CONSTRAINT the_constraint ON DOMAIN constraint_comments_dom IS NULL;
DROP TABLE constraint_comments_tbl;
DROP DOMAIN constraint_comments_dom;
+RESET SESSION AUTHORIZATION;+DROP ROLE regress_constraint_comments;
signature.asc (application/pgp-signature, 832B) download
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-bugs@postgresql.org
Cc: michael@paquier.xyz, alvherre@2ndquadrant.com, clemens@ladisch.de, pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #15833: defining a comment on a domain constraint fails with wrong OID
In-Reply-To: <20190610062855.GE2199@paquier.xyz>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox