EXECUTE Grant is not required to execute functions

[data-matt]

Describe the problem

EXECUTE Grant is not required to run an UDF.

To Reproduce

 CREATE FUNCTION function_test.test.my_add(a INT, b INT) RETURNS INT IMMUTABLE LEAKPROOF LANGUAGE SQL AS 'SELECT a + b';
-- As user2, execute the function
select function_test.test.my_add(1,2);
  my_add
----------
       3
(1 row)

revoke execute on function function_test.test.my_add from tester;
-- try again after revoke, still works
select function_test.test.my_add(1,2);
  my_add
----------
       3
(1 row)

 SELECT rolname FROM pg_catalog.pg_proc f                                                                                                     JOIN pg_catalog.pg_roles r ON f.proowner = r.oid                                                                                                                                 WHERE proname = 'my_add';
     rolname
------------------
  function_owner
(1 row)

select function_test.test.my_add(1,2);
  my_add
----------
       3

Expected behavior
Users can only use the function when granted/they have the privilege/they have inherited the privilege or they are the owner.

Afflicted versions 22.2+ (Please backport).

Jira issue: CRDB-28228

[data-matt]

For those concerned about access to underlying table data, the function does respect SELECT grant.

Example here:

root@localhost:26257/function_test> create table t1 ( c1 int );
root@localhost:26257/function_test> insert into t1 values ( 0 ) ;
root@localhost:26257/function_test> create function test_select () returns int language sql as 'select c1 from t1';
root@localhost:26257/function_test> set role tester;
root@localhost:26257/function_test> select test_select();

ERROR: user tester does not have SELECT privilege on relation t1
SQLSTATE: 42501

[mgartner]

@data-matt The example you posted behaves identically to Postgres. I'll try to come up with a working reproduction of the issue.

[rafiss]

Probably because of this:

PostgreSQL grants privileges on some types of objects to PUBLIC by default when the objects are created. No privileges are granted to PUBLIC by default on tables, table columns, sequences, foreign data wrappers, foreign servers, large objects, schemas, tablespaces, or configuration parameters. For other types of objects, the default privileges granted to PUBLIC are as follows: CONNECT and TEMPORARY (create temporary tables) privileges for databases; EXECUTE privilege for functions and procedures; and USAGE privilege for languages and data types (including domains). The object owner can, of course, REVOKE both default and expressly granted privileges.

As part of this fix, we should also add an explicit grant to public for functions that are created.

Here's a repro in PG:

postgres=# create user tester;

postgres=# create schema test;

postgres=# grant usage on schema test to tester;

postgres=#  CREATE FUNCTION test.my_add(a INT, b INT) RETURNS INT IMMUTABLE LEAKPROOF LANGUAGE SQL AS 'SELECT a + b';

postgres=# revoke execute on function test.my_add from public;

postgres=# set role tester;

postgres=> select test.my_add(1,2);
ERROR:  42501: permission denied for function my_add
LOCATION:  aclcheck_error, aclchk.c:3449

[rafiss]

I just realized that backporting the fix to enforce EXECUTE privileges is going to be disruptive. Since we weren't giving public the EXECUTE privilege, it means if we backport that change, then queries that previously would work could start getting privilege errors. So for the backport, we'll probably need a cluster setting that's named something like sql.auth.enforce_execute_privilege.enabled and it should be false in the backport.

[rafiss]

I think this also means that we should write an upgrade step that does GRANT EXECUTE ON <func> TO public during the upgrade to 23.2 for each user-defined-function.

[mgartner]

I just realized that backporting the fix to enforce EXECUTE privileges is going to be disruptive. Since we weren't giving public the EXECUTE privilege, it means if we backport that change, then queries that previously would work could start getting privilege errors. So for the backport, we'll probably need a cluster setting that's named something like sql.auth.enforce_execute_privilege.enabled and it should be false in the backport.

Should we just not backport?

I think this also means that we should write an upgrade step that does GRANT EXECUTE ON TO public during the upgrade to 23.2 for each user-defined-function.

Is this something the foundations team can do?

[mgartner]

I believe this is now completed. The fixes are far too invasive to backport, unfortunately.