Make default ACL be consistent --- ie, starting point for ChangeAcl

is the same as the access permissions granted when a relation's relacl
field is NULL, ie, owner=all rights, world=no rights.

diff --git a/src/backend/catalog/aclchk.c b/src/backend/catalog/aclchk.c
index ea2e5c282c12fc4f9f83999f86a0bc31c0a2d65d..9374eb6a38be390f18f6c00fb8d761d7162514e9 100644 (file)
--- a/src/backend/catalog/aclchk.c
+++ b/src/backend/catalog/aclchk.c
@@ -36,28 +36,8 @@
 static int32 aclcheck(char *relname, Acl *acl, AclId id,
                                          AclIdType idtype, AclMode mode);
 
-/*
- * Enable use of user relations in place of real system catalogs.
- */
-/*#define ACLDEBUG*/
-
-#ifdef ACLDEBUG
-/*
- * Fool the code below into thinking that "pgacls" is pg_class.
- * relname and relowner are in the same place, happily.
- */
-#undef Anum_pg_class_relacl
-#define Anum_pg_class_relacl                   3
-#undef Natts_pg_class
-#define Natts_pg_class                                 3
-#undef Name_pg_class
-#define Name_pg_class                                  "pgacls"
-#undef Name_pg_group
-#define Name_pg_group                                  "pggroup"
-#endif
-
 /* warning messages, now more explicit. */
-/* should correspond to the order of the ACLCHK_* result codes above. */
+/* MUST correspond to the order of the ACLCHK_* result codes in acl.h. */
 char      *aclcheck_error_strings[] = {
        "No error.",
        "Permission denied.",
@@ -65,6 +45,7 @@ char     *aclcheck_error_strings[] = {
        "Must be table owner."
 };
 
+
 #ifdef ACLDEBUG_TRACE
 static
 dumpacl(Acl *acl)
@@ -84,7 +65,7 @@ dumpacl(Acl *acl)
 #endif
 
 /*
- *
+ * ChangeAcl
  */
 void
 ChangeAcl(char *relname,
@@ -96,12 +77,12 @@ ChangeAcl(char *relname,
                           *new_acl;
        Relation        relation;
        HeapTuple       tuple;
+       Datum           aclDatum;
        Datum           values[Natts_pg_class];
        char            nulls[Natts_pg_class];
        char            replaces[Natts_pg_class];
        Relation        idescs[Num_pg_class_indices];
        bool            isNull;
-       bool            free_old_acl = false;
 
        /*
         * Find the pg_class tuple matching 'relname' and extract the ACL. If
@@ -118,29 +99,20 @@ ChangeAcl(char *relname,
                         relname);
        }
 
-       old_acl = (Acl *) heap_getattr(tuple,
-                                                                  Anum_pg_class_relacl,
-                                                                  RelationGetDescr(relation),
-                                                                  &isNull);
+       aclDatum = SysCacheGetAttr(RELNAME, tuple, Anum_pg_class_relacl,
+                                                          &isNull);
        if (isNull)
        {
-#ifdef ACLDEBUG_TRACE
-               elog(DEBUG, "ChangeAcl: using default ACL");
-#endif
-               old_acl = acldefault(relname);
-               free_old_acl = true;
-       }
-
-       /* Need to detoast the old ACL for modification */
-       old_acl = DatumGetAclP(PointerGetDatum(old_acl));
+               /* No ACL, so build default ACL for rel */
+               AclId           ownerId;
 
-       if (ACL_NUM(old_acl) < 1)
+               ownerId = ((Form_pg_class) GETSTRUCT(tuple))->relowner;
+               old_acl = acldefault(relname, ownerId);
+       }
+       else
        {
-#ifdef ACLDEBUG_TRACE
-               elog(DEBUG, "ChangeAcl: old ACL has zero length");
-#endif
-               old_acl = acldefault(relname);
-               free_old_acl = true;
+               /* get a detoasted copy of the rel's ACL */
+               old_acl = DatumGetAclPCopy(aclDatum);
        }
 
 #ifdef ACLDEBUG_TRACE
@@ -173,8 +145,8 @@ ChangeAcl(char *relname,
        CatalogCloseIndices(Num_pg_class_indices, idescs);
 
        heap_close(relation, RowExclusiveLock);
-       if (free_old_acl)
-               pfree(old_acl);
+
+       pfree(old_acl);
        pfree(new_acl);
 }
 
@@ -264,9 +236,15 @@ aclcheck(char *relname, Acl *acl, AclId id, AclIdType idtype, AclMode mode)
        unsigned        num,
                                found_group;
 
-       /* if no acl is found, use world default */
+       /*
+        * If ACL is null, default to "OK" --- this should not happen,
+        * since caller should have inserted appropriate default
+        */
        if (!acl)
-               acl = acldefault(relname);
+       {
+               elog(DEBUG, "aclcheck: null ACL, returning 1");
+               return ACLCHECK_OK;
+       }
 
        num = ACL_NUM(acl);
        aidat = ACL_DAT(acl);
@@ -278,9 +256,7 @@ aclcheck(char *relname, Acl *acl, AclId id, AclIdType idtype, AclMode mode)
         */
        if (num < 1)
        {
-#if defined(ACLDEBUG_TRACE) || 1
                elog(DEBUG, "aclcheck: zero-length ACL, returning 1");
-#endif
                return ACLCHECK_OK;
        }
 
@@ -357,11 +333,12 @@ aclcheck(char *relname, Acl *acl, AclId id, AclIdType idtype, AclMode mode)
 int32
 pg_aclcheck(char *relname, Oid userid, AclMode mode)
 {
-       HeapTuple       tuple;
-       Acl                *acl = (Acl *) NULL;
        int32           result;
+       HeapTuple       tuple;
        char       *usename;
-       Relation        relation;
+       Datum           aclDatum;
+       bool            isNull;
+       Acl                *acl;
 
        tuple = SearchSysCacheTuple(SHADOWSYSID,
                                                                ObjectIdGetDatum(userid),
@@ -399,53 +376,31 @@ pg_aclcheck(char *relname, Oid userid, AclMode mode)
                return ACLCHECK_OK;
        }
 
-#ifndef ACLDEBUG
-       relation = heap_openr(RelationRelationName, RowExclusiveLock);
+       /*
+        * Normal case: get the relation's ACL from pg_class
+        */
        tuple = SearchSysCacheTuple(RELNAME,
                                                                PointerGetDatum(relname),
                                                                0, 0, 0);
        if (!HeapTupleIsValid(tuple))
-       {
-               elog(ERROR, "pg_aclcheck: class \"%s\" not found",
-                        relname);
-       }
-       if (!heap_attisnull(tuple, Anum_pg_class_relacl))
-       {
-               /* get a detoasted copy of the ACL */
-               acl = DatumGetAclPCopy(heap_getattr(tuple,
-                                                                                       Anum_pg_class_relacl,
-                                                                                       RelationGetDescr(relation),
-                                                                                       (bool *) NULL));
-       }
-       else
-       {
+               elog(ERROR, "pg_aclcheck: class \"%s\" not found", relname);
 
-               /*
-                * if the acl is null, by default the owner can do whatever he
-                * wants to with it
-                */
+       aclDatum = SysCacheGetAttr(RELNAME, tuple, Anum_pg_class_relacl,
+                                                          &isNull);
+       if (isNull)
+       {
+               /* No ACL, so build default ACL for rel */
                AclId           ownerId;
 
                ownerId = ((Form_pg_class) GETSTRUCT(tuple))->relowner;
-               acl = aclownerdefault(relname, ownerId);
+               acl = acldefault(relname, ownerId);
        }
-       heap_close(relation, RowExclusiveLock);
-#else
-       relation = heap_openr(RelationRelationName, RowExclusiveLock);
-       tuple = SearchSysCacheTuple(RELNAME,
-                                                               PointerGetDatum(relname),
-                                                               0, 0, 0);
-       if (HeapTupleIsValid(tuple) &&
-               !heap_attisnull(tuple, Anum_pg_class_relacl))
+       else
        {
-               /* get a detoasted copy of the ACL */
-               acl = DatumGetAclPCopy(heap_getattr(tuple,
-                                                                                       Anum_pg_class_relacl,
-                                                                                       RelationGetDescr(relation),
-                                                                                       (bool *) NULL));
+               /* get a detoasted copy of the rel's ACL */
+               acl = DatumGetAclPCopy(aclDatum);
        }
-       heap_close(relation, RowExclusiveLock);
-#endif
+
        result = aclcheck(relname, acl, userid, (AclIdType) ACL_IDTYPE_UID, mode);
        if (acl)
                pfree(acl);

diff --git a/src/backend/utils/adt/acl.c b/src/backend/utils/adt/acl.c
index e1e2700abd394a32965d535ac193bf0e3377cbf0..33f09b198c1537738ed310cda0ca01bb813458ac 100644 (file)
--- a/src/backend/utils/adt/acl.c
+++ b/src/backend/utils/adt/acl.c
@@ -334,12 +334,23 @@ aclitemgt(AclItem *a1, AclItem *a2)
                        (a1->ai_idtype == a2->ai_idtype && a1->ai_id > a2->ai_id));
 }
 
+
+/*
+ * acldefault()  --- create an ACL describing default access permissions
+ *
+ * Change this routine if you want to alter the default access policy for
+ * newly-created tables (or any table with a NULL acl entry in pg_class)
+ */
 Acl *
-aclownerdefault(char *relname, AclId ownerid)
+acldefault(char *relname, AclId ownerid)
 {
        Acl                *acl;
        AclItem    *aip;
 
+#define ACL_WORLD_DEFAULT              (ACL_NO)
+/* #define             ACL_WORLD_DEFAULT               (ACL_RD|ACL_WR|ACL_AP|ACL_RU) */
+#define ACL_OWNER_DEFAULT              (ACL_RD|ACL_WR|ACL_AP|ACL_RU)
+
        acl = makeacl(2);
        aip = ACL_DAT(acl);
        aip[0].ai_idtype = ACL_IDTYPE_WORLD;
@@ -351,19 +362,6 @@ aclownerdefault(char *relname, AclId ownerid)
        return acl;
 }
 
-Acl *
-acldefault(char *relname)
-{
-       Acl                *acl;
-       AclItem    *aip;
-
-       acl = makeacl(1);
-       aip = ACL_DAT(acl);
-       aip[0].ai_idtype = ACL_IDTYPE_WORLD;
-       aip[0].ai_id = ACL_ID_WORLD;
-       aip[0].ai_mode = IsSystemRelationName(relname) ? ACL_RD : ACL_WORLD_DEFAULT;
-       return acl;
-}
 
 /*
  * Add or replace an item in an ACL array.

diff --git a/src/include/utils/acl.h b/src/include/utils/acl.h
index 4df6e5a637708cdd5bfc55b889a89715143b9fb3..bc786c1bacccd9b3f2d04af170534b348bb1c45a 100644 (file)
--- a/src/include/utils/acl.h
+++ b/src/include/utils/acl.h
@@ -12,7 +12,7 @@
  * NOTES
  *       For backward-compatibility purposes we have to allow there
  *       to be a null ACL in a pg_class tuple.  This will be defined as
- *       meaning "no protection" (i.e., old catalogs get old semantics).
+ *       meaning "default protection" (i.e., whatever acldefault() returns).
  *
  *       The AclItems in an ACL array are currently kept in sorted order.
  *       Things will break hard if you change that without changing the
@@ -32,7 +32,7 @@
  */
 typedef uint32 AclId;
 
-#define ACL_ID_WORLD   0               /* XXX only idtype should be checked */
+#define ACL_ID_WORLD   0               /* placeholder for id in a WORLD acl item */
 
 /*
  * AclIdType   tag that describes if the AclId is a user, group, etc.
@@ -58,15 +58,6 @@ typedef uint8 AclMode;
 #define ACL_RU                 (1<<3)  /* place rules */
 #define N_ACL_MODES            4
 
-#define ACL_MODECHG_ADD                        1
-#define ACL_MODECHG_DEL                        2
-#define ACL_MODECHG_EQL                        3
-
-/* change this line if you want to set the default acl permission  */
-#define ACL_WORLD_DEFAULT              (ACL_NO)
-/* #define             ACL_WORLD_DEFAULT               (ACL_RD|ACL_WR|ACL_AP|ACL_RU) */
-#define ACL_OWNER_DEFAULT              (ACL_RD|ACL_WR|ACL_AP|ACL_RU)
-
 /*
  * AclItem
  */
@@ -143,6 +134,13 @@ typedef ArrayType IdList;
 #define PG_RETURN_IDLIST_P(x)      PG_RETURN_POINTER(x)
 
 
+/*
+ * ACL modification opcodes
+ */
+#define ACL_MODECHG_ADD                        1
+#define ACL_MODECHG_DEL                        2
+#define ACL_MODECHG_EQL                        3
+
 /* mode indicators for I/O */
 #define ACL_MODECHG_STR                        "+-="   /* list of valid characters */
 #define ACL_MODECHG_ADD_CHR            '+'
@@ -171,8 +169,8 @@ extern char *aclcheck_error_strings[];
 /*
  * routines used internally (parser, etc.)
  */
-extern Acl *aclownerdefault(char *relname, AclId ownerid);
-extern Acl *acldefault(char *relname);
+extern Acl *acldefault(char *relname, AclId ownerid);
+
 extern Acl *aclinsert3(Acl *old_acl, AclItem *mod_aip, unsigned modechg);
 
 extern char *aclmakepriv(char *old_privlist, char new_priv);