Re: [PATCH] Why is_admin_of_role() uses ROLERECURSE_MEMBERS rather than ROLERECURSE_PRIVS?

Started by preTham21 days ago2 messages
#1preTham
prezza672@gmail.com

Hi,
Hi, (found this in https://commitfest.postgresql.org/patch/6251/)

create user u1;
create user u2;
create user u3;
create user u4;
grant u2 to u1 with admin true ;
grant u3 to u2 with admin true ;
revoke inherit option for u2 from u1 ;
set session authorization u1;
grant u3 to u4;

PostgreSQL calls select_best_admin() internally. If that function returns
InvalidOid, I think it means that “The system tried all the possible
grantor roles (roles we belong to that have ADMIN OPTION), but none are
currently usable.” i.e the system couldn’t find a grantor role in the
current context, so it reports: "no possible grantors"

The "grant u3 to u4;" will report error "no possible grantors" rather than

"permission denied to grant role".

Is this the expected behavior

Again I think a “permission denied” would imply we tried as a specific
role, and that role doesn’t have permission. But here, Postgres never even
found which role we could be acting as. So from the system’s logic, it’s
not a denied action; it’s “no valid takers found to even attempt the
action.”

but the "no possible grantors" error can happen in my test case.

The main reason is that is_admin_of_role() and select_best_admin() use

different role recurse methods.

I think they should keep consistent, maybe both use ROLERECURSE_PRIVS?

Thoughts?

I think ROLERECURSE_MEMBERS traverses membership relationships among
roles regardless
of whether inheritance or session activation is in play, while
ROLERECURSE_PRIVS
recursively traverses active privileges. and only consider roles whose
privileges are currently usable i.e. those with inherit still true, or
roles that are currently SET ROLE’d into.

I believe is_admin_of_role() uses ROLERECURSE_MEMBERS because
is_admin_of_role() is not used for permission enforcement and Its purpose
is to answer checks like “does A have the ADMIN OPTION for B (anywhere in
the membership graph)?” So it needs to see all possible relationships, even
if the intermediate memberships are non-inheriting or currently inactive.

If A was once granted B WITH ADMIN OPTION, then regardless of INHERIT, A is
“an admin of B” from the system’s metadata perspective. Therefore, it
recurses through memberships unconditionally while ignoring session state.

select_best_admin() is called at execution time when we actually run a
GRANT or REVOKE command. At that moment, PostgreSQL must know: “Given the
roles the current user is effectively using right now, which one is a valid
grantor?”. Hence it can only follow active, usable privileges i.e. it uses
ROLERECURSE_PRIVS.

Regards,
Pretham

#2cca5507
cca5507@qq.com
In reply to: preTham (#1)
Re: [PATCH] Why is_admin_of_role() uses ROLERECURSE_MEMBERS ratherthan ROLERECURSE_PRIVS?

Hi,

Thank you for your reply.

PostgreSQL calls select_best_admin() internally. If that function returns
InvalidOid, I think it means that “The system tried all the possible
grantor roles (roles we belong to that have ADMIN OPTION), but none are
currently usable.” i.e the system couldn’t find a grantor role in the
current context, so it reports: "no possible grantors"

+1

Again I think a “permission denied” would imply we tried as a specific
role, and that role doesn’t have permission. But here, Postgres never even
found which role we could be acting as. So from the system’s logic, it’s
not a denied action; it’s “no valid takers found to even attempt the
action.”

Due to the "revoke inherit option for u2 from u1", I think reporting "permission denied ..."
here is reasonable. The "no possible grantors" is more like an internal error, and should
not be reported to users.

I believe is_admin_of_role() uses ROLERECURSE_MEMBERS because
is_admin_of_role() is not used for permission enforcement and Its purpose
is to answer checks like “does A have the ADMIN OPTION for B (anywhere in
the membership graph)?” So it needs to see all possible relationships, even
if the intermediate memberships are non-inheriting or currently inactive.

But many places report "permission denied ..." if is_admin_of_role() return false.

--
Regards,
ChangAo Chen