HAWQ-1249. Don't do ACL checks on segments

Project: http://git-wip-us.apache.org/repos/asf/incubator-hawq/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-hawq/commit/2f5910f2
Tree: http://git-wip-us.apache.org/repos/asf/incubator-hawq/tree/2f5910f2
Diff: http://git-wip-us.apache.org/repos/asf/incubator-hawq/diff/2f5910f2

Branch: refs/heads/2.1.0.0-incubating
Commit: 2f5910f2b0c2877e524c4c428ed963255c176378
Parents: 8d22582
Author: Chunling Wang <[email protected]>
Authored: Mon Jan 9 14:35:11 2017 +0800
Committer: Chunling Wang <[email protected]>
Committed: Mon Jan 9 14:35:11 2017 +0800

----------------------------------------------------------------------
 src/backend/catalog/aclchk.c        | 85 +++++++++++++++++++++++---------
 src/backend/executor/execMain.c     | 37 +-------------
 src/backend/parser/parse_relation.c | 35 +++----------
 3 files changed, 72 insertions(+), 85 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-hawq/blob/2f5910f2/src/backend/catalog/aclchk.c
----------------------------------------------------------------------
diff --git a/src/backend/catalog/aclchk.c b/src/backend/catalog/aclchk.c
index d19a045..01a4f94 100644
--- a/src/backend/catalog/aclchk.c
+++ b/src/backend/catalog/aclchk.c
@@ -224,8 +224,9 @@ restrict_and_check_grant(bool is_grant, AclMode 
avail_goptions, bool all_privs,
         * If we found no grant options, consider whether to issue a hard error.
         * Per spec, having any privilege at all on the object will get you by
         * here.
+        * QE bypass all permission checking.
         */
-       if (avail_goptions == ACL_NO_RIGHTS)
+       if (avail_goptions == ACL_NO_RIGHTS && Gp_role != GP_ROLE_EXECUTE)
        {
          if (enable_ranger && !fallBackToNativeCheck(objkind, objectId, 
grantorId)) {
            if (pg_rangercheck(objkind, objectId, grantorId,
@@ -2948,9 +2949,9 @@ pg_class_aclmask(Oid table_oid, Oid roleid,
                }
        }
        /*
-        * Otherwise, superusers or on QE bypass all permission-checking.
+        * Otherwise, superusers bypass all permission-checking.
         */
-       if (GP_ROLE_EXECUTE == Gp_role || superuser_arg(roleid))
+       if (superuser_arg(roleid))
        {
 #ifdef ACLDEBUG
                elog(DEBUG2, "OID %u is superuser, home free", roleid);
@@ -3006,8 +3007,8 @@ pg_database_aclmask(Oid db_oid, Oid roleid,
        Oid                     ownerId;
        cqContext  *pcqCtx;
 
-       /* Superusers or on QE bypass all permission checking. */
-       if (GP_ROLE_EXECUTE == Gp_role || superuser_arg(roleid))
+       /* Superusers bypass all permission checking. */
+       if (superuser_arg(roleid))
                return mask;
 
        /*
@@ -3069,8 +3070,8 @@ pg_proc_aclmask(Oid proc_oid, Oid roleid,
        Oid                     ownerId;
        cqContext  *pcqCtx;
 
-       /* Superusers or on QE bypass all permission checking. */
-       if (GP_ROLE_EXECUTE == Gp_role || superuser_arg(roleid))
+       /* Superusers bypass all permission checking. */
+       if (superuser_arg(roleid))
                return mask;
 
        /*
@@ -3131,8 +3132,8 @@ pg_language_aclmask(Oid lang_oid, Oid roleid,
        Oid                     ownerId;
        cqContext  *pcqCtx;
 
-       /* Superusers or on QE bypass all permission checking. */
-       if (GP_ROLE_EXECUTE == Gp_role || superuser_arg(roleid))
+       /* Superusers bypass all permission checking. */
+       if (superuser_arg(roleid))
                return mask;
 
        /*
@@ -3194,8 +3195,8 @@ pg_namespace_aclmask(Oid nsp_oid, Oid roleid,
        Oid                     ownerId;
        cqContext  *pcqCtx;
 
-       /* Superusers or on QE bypass all permission checking. */
-       if (GP_ROLE_EXECUTE == Gp_role || superuser_arg(roleid))
+       /* Superusers bypass all permission checking. */
+       if (superuser_arg(roleid))
                return mask;
 
        /*
@@ -3293,8 +3294,8 @@ pg_tablespace_aclmask(Oid spc_oid, Oid roleid,
        if (spc_oid == GLOBALTABLESPACE_OID && 
!(IsBootstrapProcessingMode()||gp_upgrade_mode))
                return 0;
 
-       /* Otherwise, superusers or on QE bypass all permission checking. */
-       if (GP_ROLE_EXECUTE == Gp_role || superuser_arg(roleid))
+       /* Superusers bypass all permission checking. */
+       if (superuser_arg(roleid))
                return mask;
 
        /*
@@ -3366,8 +3367,8 @@ pg_foreign_data_wrapper_aclmask(Oid fdw_oid, Oid roleid,
 
        Form_pg_foreign_data_wrapper fdwForm;
 
-       /* Bypass permission checks for superusers or on QE */
-       if (GP_ROLE_EXECUTE == Gp_role || superuser_arg(roleid))
+       /* Superusers bypass all permission checking. */
+       if (superuser_arg(roleid))
                return mask;
 
        /*
@@ -3435,8 +3436,8 @@ pg_foreign_server_aclmask(Oid srv_oid, Oid roleid,
 
        Form_pg_foreign_server srvForm;
 
-       /* Bypass permission checks for superusers or on QE */
-       if (GP_ROLE_EXECUTE == Gp_role || superuser_arg(roleid))
+       /* Superusers bypass all permission checking. */
+       if (superuser_arg(roleid))
                return mask;
 
        /*
@@ -3505,10 +3506,10 @@ pg_extprotocol_aclmask(Oid ptcOid, Oid roleid,
        cqContext       cqc;
        cqContext  *pcqCtx;
 
-       /* Bypass permission checks for superusers or on QE */
-       if (GP_ROLE_EXECUTE == Gp_role || superuser_arg(roleid))
+       /* Superusers bypass all permission checking. */
+       if (superuser_arg(roleid))
                return mask;
-       
+
        rel = heap_open(ExtprotocolRelationId, AccessShareLock);
 
        pcqCtx = caql_beginscan(
@@ -3585,8 +3586,8 @@ pg_filesystem_aclmask(Oid fsysOid, Oid roleid,
        ScanKeyData entry[1];
 
 
-       /* Bypass permission checks for superusers or on QE */
-       if (GP_ROLE_EXECUTE == Gp_role || superuser_arg(roleid))
+       /* Bypass permission checks for superusers */
+       if (superuser_arg(roleid))
                return mask;
        
        /*
@@ -3788,6 +3789,10 @@ pg_filesystem_nativecheck(Oid fsysid, Oid roleid, 
AclMode mode)
 AclResult
 pg_class_aclcheck(Oid table_oid, Oid roleid, AclMode mode)
 {
+  /* Bypass all permission checking on QE. */
+  if (Gp_role == GP_ROLE_EXECUTE)
+    return ACLCHECK_OK;
+
   if(enable_ranger && !fallBackToNativeCheck(ACL_KIND_CLASS, table_oid, 
roleid))
   {
     return pg_rangercheck(ACL_KIND_CLASS, table_oid, roleid, mode, 
ACLMASK_ANY);
@@ -3804,6 +3809,10 @@ pg_class_aclcheck(Oid table_oid, Oid roleid, AclMode 
mode)
 AclResult
 pg_database_aclcheck(Oid db_oid, Oid roleid, AclMode mode)
 {
+  /* Bypass all permission checking on QE. */
+  if (Gp_role == GP_ROLE_EXECUTE)
+    return ACLCHECK_OK;
+
   if(enable_ranger && !fallBackToNativeCheck(ACL_KIND_DATABASE, db_oid, 
roleid))
    {
      return pg_rangercheck(ACL_KIND_DATABASE, db_oid, roleid, mode, 
ACLMASK_ANY);
@@ -3820,6 +3829,10 @@ pg_database_aclcheck(Oid db_oid, Oid roleid, AclMode 
mode)
 AclResult
 pg_proc_aclcheck(Oid proc_oid, Oid roleid, AclMode mode)
 {
+  /* Bypass all permission checking on QE. */
+  if (Gp_role == GP_ROLE_EXECUTE)
+    return ACLCHECK_OK;
+
   if(enable_ranger && !fallBackToNativeCheck(ACL_KIND_PROC, proc_oid, roleid))
   {
     return pg_rangercheck(ACL_KIND_PROC, proc_oid, roleid, mode, ACLMASK_ANY);
@@ -3836,6 +3849,10 @@ pg_proc_aclcheck(Oid proc_oid, Oid roleid, AclMode mode)
 AclResult
 pg_language_aclcheck(Oid lang_oid, Oid roleid, AclMode mode)
 {
+  /* Bypass all permission checking on QE. */
+  if (Gp_role == GP_ROLE_EXECUTE)
+    return ACLCHECK_OK;
+
   if(enable_ranger && !fallBackToNativeCheck(ACL_KIND_LANGUAGE, lang_oid, 
roleid))
   {
     return pg_rangercheck(ACL_KIND_LANGUAGE, lang_oid, roleid, mode, 
ACLMASK_ANY);
@@ -3852,6 +3869,10 @@ pg_language_aclcheck(Oid lang_oid, Oid roleid, AclMode 
mode)
 AclResult
 pg_namespace_aclcheck(Oid nsp_oid, Oid roleid, AclMode mode)
 {
+  /* Bypass all permission checking on QE. */
+  if (Gp_role == GP_ROLE_EXECUTE)
+    return ACLCHECK_OK;
+
   if(enable_ranger && !fallBackToNativeCheck(ACL_KIND_NAMESPACE, nsp_oid, 
roleid))
   {
     return pg_rangercheck(ACL_KIND_NAMESPACE, nsp_oid, roleid, mode, 
ACLMASK_ANY);
@@ -3868,6 +3889,10 @@ pg_namespace_aclcheck(Oid nsp_oid, Oid roleid, AclMode 
mode)
 AclResult
 pg_tablespace_aclcheck(Oid spc_oid, Oid roleid, AclMode mode)
 {
+  /* Bypass all permission checking on QE. */
+  if (Gp_role == GP_ROLE_EXECUTE)
+    return ACLCHECK_OK;
+
   if(enable_ranger && !fallBackToNativeCheck(ACL_KIND_TABLESPACE, spc_oid, 
roleid))
   {
     return pg_rangercheck(ACL_KIND_TABLESPACE, spc_oid, roleid, mode, 
ACLMASK_ANY);
@@ -3885,6 +3910,10 @@ pg_tablespace_aclcheck(Oid spc_oid, Oid roleid, AclMode 
mode)
 AclResult
 pg_foreign_data_wrapper_aclcheck(Oid fdw_oid, Oid roleid, AclMode mode)
 {
+  /* Bypass all permission checking on QE. */
+  if (Gp_role == GP_ROLE_EXECUTE)
+    return ACLCHECK_OK;
+
   if(enable_ranger && !fallBackToNativeCheck(ACL_KIND_FDW, fdw_oid, roleid))
   {
     return pg_rangercheck(ACL_KIND_FDW, fdw_oid, roleid, mode, ACLMASK_ANY);
@@ -3902,6 +3931,10 @@ pg_foreign_data_wrapper_aclcheck(Oid fdw_oid, Oid 
roleid, AclMode mode)
 AclResult
 pg_foreign_server_aclcheck(Oid srv_oid, Oid roleid, AclMode mode)
 {
+  /* Bypass all permission checking on QE. */
+  if (Gp_role == GP_ROLE_EXECUTE)
+    return ACLCHECK_OK;
+
   if(enable_ranger && !fallBackToNativeCheck(ACL_KIND_FOREIGN_SERVER, srv_oid, 
roleid))
   {
     return pg_rangercheck(ACL_KIND_FOREIGN_SERVER, srv_oid, roleid, mode, 
ACLMASK_ANY);
@@ -3919,6 +3952,10 @@ pg_foreign_server_aclcheck(Oid srv_oid, Oid roleid, 
AclMode mode)
 AclResult
 pg_extprotocol_aclcheck(Oid ptcid, Oid roleid, AclMode mode)
 {
+  /* Bypass all permission checking on QE. */
+  if (Gp_role == GP_ROLE_EXECUTE)
+    return ACLCHECK_OK;
+
   if(enable_ranger && !fallBackToNativeCheck(ACL_KIND_EXTPROTOCOL, ptcid, 
roleid))
   {
     return pg_rangercheck(ACL_KIND_EXTPROTOCOL, ptcid, roleid, mode, 
ACLMASK_ANY);
@@ -3935,6 +3972,10 @@ pg_extprotocol_aclcheck(Oid ptcid, Oid roleid, AclMode 
mode)
 AclResult
 pg_filesystem_aclcheck(Oid fsysid, Oid roleid, AclMode mode)
 {
+  /* Bypass all permission checking on QE. */
+  if (Gp_role == GP_ROLE_EXECUTE)
+    return ACLCHECK_OK;
+
   if(enable_ranger && !fallBackToNativeCheck(ACL_KIND_FILESYSTEM, fsysid, 
roleid))
   {
     return pg_rangercheck(ACL_KIND_FILESYSTEM, fsysid, roleid, mode, 
ACLMASK_ANY);

http://git-wip-us.apache.org/repos/asf/incubator-hawq/blob/2f5910f2/src/backend/executor/execMain.c
----------------------------------------------------------------------
diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c
index 30f6d09..666d16f 100644
--- a/src/backend/executor/execMain.c
+++ b/src/backend/executor/execMain.c
@@ -1912,45 +1912,10 @@ InitPlan(QueryDesc *queryDesc, int eflags)
         * rangetable here --- subplan RTEs will be checked during
         * ExecInitSubPlan().
         */
-       if (operation != CMD_SELECT ||
-                       (Gp_role != GP_ROLE_EXECUTE &&
-                        !(shouldDispatch && 
cdbpathlocus_querysegmentcatalogs)))
+       if (Gp_role != GP_ROLE_EXECUTE)
        {
                ExecCheckRTPerms(plannedstmt->rtable);
        }
-       else
-       {
-               /*
-                * We don't check the rights here, so we can query pg_statistic 
even if we are a non-privileged user.
-                * This shouldn't cause a problem, because 
"cdbpathlocus_querysegmentcatalogs" can only be true if we
-                * are doing special catalog queries for ANALYZE.  Otherwise, 
the QD will execute the normal access right
-                * check.  This does open a security hole, as it's possible for 
a hacker to connect to a segdb with GP_ROLE_EXECUTE,
-                * (at least, in theory, although it isn't easy) and then do a 
query.  But all they can see is
-                * pg_statistic and pg_class, and pg_class is normally readable 
by everyone.
-                */
-
-               ListCell *lc = NULL;
-
-               foreach(lc, plannedstmt->rtable)
-               {
-                       RangeTblEntry *rte = lfirst(lc);
-
-                       if (rte->rtekind != RTE_RELATION)
-                               continue;
-
-                       if (rte->requiredPerms == 0)
-                               continue;
-
-                       /*
-                        * Ignore access rights check on pg_statistic and 
pg_class, so
-                        * the QD can retreive the statistics from the QEs.
-                        */
-                       if (rte->relid != StatisticRelationId && rte->relid != 
RelationRelationId)
-                       {
-                               ExecCheckRTEPerms(rte);
-                       }
-               }
-       }
 
        /*
         * get information from query descriptor

http://git-wip-us.apache.org/repos/asf/incubator-hawq/blob/2f5910f2/src/backend/parser/parse_relation.c
----------------------------------------------------------------------
diff --git a/src/backend/parser/parse_relation.c 
b/src/backend/parser/parse_relation.c
index 7dbe496..f9444ef 100644
--- a/src/backend/parser/parse_relation.c
+++ b/src/backend/parser/parse_relation.c
@@ -2841,33 +2841,14 @@ ExecCheckRTEPerms(RangeTblEntry *rte)
        /*
         * We must have *all* the requiredPerms bits, so use aclmask not 
aclcheck.
         */
-       if (enable_ranger && !fallBackToNativeCheck(ACL_KIND_CLASS, relOid, 
userid))
-       {
-         elog(LOG, "ExecCheckRTEPerms: here");
-         /* ranger check required permission should all be approved.*/
-    if (pg_rangercheck(ACL_KIND_CLASS, relOid, userid, requiredPerms, 
ACLMASK_ALL)
-        != RANGERCHECK_OK)
-    {
-      /*
-       * If the table is a partition, return an error message that includes
-       * the name of the parent table.
-       */
-      const char *rel_name = get_rel_name_partition(relOid);
-      aclcheck_error(ACLCHECK_NO_PRIV, ACL_KIND_CLASS, rel_name);
-    }
-       }
-       else
-       {
-         if (pg_class_aclmask(relOid, userid, requiredPerms, ACLMASK_ALL)
-               != requiredPerms)
-    {
-      /*
-       * If the table is a partition, return an error message that includes
-       * the name of the parent table.
-       */
-      const char *rel_name = get_rel_name_partition(relOid);
-      aclcheck_error(ACLCHECK_NO_PRIV, ACL_KIND_CLASS, rel_name);
-    }
+       if (pg_class_aclmask(relOid, userid, requiredPerms, ACLMASK_ALL)
+                       != requiredPerms) {
+               /*
+                * If the table is a partition, return an error message that 
includes
+                * the name of the parent table.
+                */
+               const char *rel_name = get_rel_name_partition(relOid);
+               aclcheck_error(ACLCHECK_NO_PRIV, ACL_KIND_CLASS, rel_name);
        }
 }
 

Reply via email to