Thread: Why ACL_EXECUTE is checked on FindConversion()?
When FindConversion() is called, it also checks current user's ACL_EXECUTE privilege on the conproc of the fetched conversion. Why this check is applied on FindConversion(), instead of FindDefaultConversion()? The FindConversion() returns the OID of conversion for the given name and namespace, or InvalidOid if not found or user does not have ACL_EXECUTE privilege. It is called from DropConversionsCommand(), RenameConversion(), AlterConversionOwner() and CommentConversion(), to obtain OID of the target conversion to be modified by DDL statement. On the other hand, FindDefaultConversionProc() does not apply such kind of permission checks, though it is called from SetClientEncoding(). The conversion procedure is implicitly called when user communicates to the server backend, so it seems to me quite natural if FindDefaultConversionProc() checks user's ACL_EXECUTE privilege. But it is checked when we lookup the target conversion on DDL statement. It's unclear for me what is the intension of this check. Thanks, -- OSS Platform Development Division, NEC KaiGai Kohei <kaigai@ak.jp.nec.com>
KaiGai Kohei <kaigai@ak.jp.nec.com> writes:
> When FindConversion() is called, it also checks current user's ACL_EXECUTE
> privilege on the conproc of the fetched conversion.
> Why this check is applied on FindConversion(), instead of FindDefaultConversion()?
Does seem pretty inconsistent, doesn't it?
The original idea may have been to provide a substitute for a USAGE
ACL check on conversions, in which case it's not totally insane: if
you make a conversion default then you're implicitly granting it to
public. But there's no documentation about this.
Offhand I see no really good reason to have a usage check on
conversions, and would be happy with removing this one. The function
permission check at CREATE CONVERSION time ought to be sufficient.
regards, tom lane
Tom Lane wrote: > KaiGai Kohei <kaigai@ak.jp.nec.com> writes: >> When FindConversion() is called, it also checks current user's ACL_EXECUTE >> privilege on the conproc of the fetched conversion. > >> Why this check is applied on FindConversion(), instead of FindDefaultConversion()? > > Does seem pretty inconsistent, doesn't it? Anyway, I could not understand the intention of the checks, rather than inconsistency. So, I doubted it might be a bug, if it intended to provide a permission something like ACL_USAGE on conversion. > The original idea may have been to provide a substitute for a USAGE > ACL check on conversions, in which case it's not totally insane: if > you make a conversion default then you're implicitly granting it to > public. But there's no documentation about this. If ACL_EXECUTE checks is deployed on FindDefaultConversion(), it performs as if something like ACL_USAGE permission. However, FindConversion() is called from ALTER or DROP CONVERSION, so it controls DDL statements in addition to its ownership. Please note that this check (ACL_EXECUTE) is not applied when user choose a certain conversion. > Offhand I see no really good reason to have a usage check on > conversions, and would be happy with removing this one. The function > permission check at CREATE CONVERSION time ought to be sufficient. It seems to me reasonable. If user does not have ACL_EXECUTE privilege on the function used in the conversion, he does not alter or drop the conversion, even if he has ownership of the conversion. This behavior is hard to understand. Thanks, -- OSS Platform Development Division, NEC KaiGai Kohei <kaigai@ak.jp.nec.com>
This patch fixes a problem discussed earlier. Now, FindConversion() which is only called from FindConversionByName() checks ACL_EXECUTE permission on the conversion procedure matched. If not allowed, it performs as if the given conversion does not exist (InvalidOid shall be returned). The FindConversionByName() is called from: - CommentConversion() - DropConversionsCommand() - RenameConversion() - AlterConversionOwner() All of them also check ownership of the conversion eventually. In addition, CreateConversionCommand() checks ACL_EXECUTE permission on the conversion procedure when creation time, independently. Also see the related discussion: http://archives.postgresql.org/pgsql-hackers/2009-08/msg01361.php http://archives.postgresql.org/pgsql-hackers/2009-08/msg01392.php http://archives.postgresql.org/pgsql-hackers/2009-08/msg01420.php Thanks, (2009/08/20 8:50), KaiGai Kohei wrote: > Tom Lane wrote: >> KaiGai Kohei<kaigai@ak.jp.nec.com> writes: >>> When FindConversion() is called, it also checks current user's ACL_EXECUTE >>> privilege on the conproc of the fetched conversion. >> >>> Why this check is applied on FindConversion(), instead of FindDefaultConversion()? >> >> Does seem pretty inconsistent, doesn't it? > > Anyway, I could not understand the intention of the checks, rather > than inconsistency. So, I doubted it might be a bug, if it intended > to provide a permission something like ACL_USAGE on conversion. > >> The original idea may have been to provide a substitute for a USAGE >> ACL check on conversions, in which case it's not totally insane: if >> you make a conversion default then you're implicitly granting it to >> public. But there's no documentation about this. > > If ACL_EXECUTE checks is deployed on FindDefaultConversion(), it performs > as if something like ACL_USAGE permission. However, FindConversion() is > called from ALTER or DROP CONVERSION, so it controls DDL statements > in addition to its ownership. > Please note that this check (ACL_EXECUTE) is not applied when user choose > a certain conversion. > >> Offhand I see no really good reason to have a usage check on >> conversions, and would be happy with removing this one. The function >> permission check at CREATE CONVERSION time ought to be sufficient. > > It seems to me reasonable. > > If user does not have ACL_EXECUTE privilege on the function used in > the conversion, he does not alter or drop the conversion, even if he > has ownership of the conversion. This behavior is hard to understand. > > Thanks, -- OSS Platform Development Division, NEC KaiGai Kohei <kaigai@ak.jp.nec.com>