Re: Allow non-superuser to cancel superuser tasks.
От | Leung, Anthony |
---|---|
Тема | Re: Allow non-superuser to cancel superuser tasks. |
Дата | |
Msg-id | 2C0DF031-EBC1-461A-B242-35912F8D0772@amazon.com обсуждение исходный текст |
Ответ на | Re: Allow non-superuser to cancel superuser tasks. ("Andrey M. Borodin" <x4mmm@yandex-team.ru>) |
Ответы |
Re: Allow non-superuser to cancel superuser tasks.
Re: Allow non-superuser to cancel superuser tasks. |
Список | pgsql-hackers |
> I don't think we should rely on !OidIsValid(proc->roleId) for signaling > autovacuum workers. That might not always be true, and I don't see any > need to rely on that otherwise. IMHO we should just add a few lines before > the existing code, which doesn't need to be changed at all: > > if (pg_stat_is_backend_autovac_worker(proc->backendId) && > !has_privs_of_role(GetUserId(), ROLE_PG_SIGNAL_AUTOVACUUM)) > return SIGNAL_BACKEND_NOAUTOVACUUM; I tried to add them above the existing code. When I test it locally, a user without pg_signal_autovacuum will actually failat this block because the user is not superuser and !OidIsValid(proc->roleId) is also true in the following: /* * Only allow superusers to signal superuser-owned backends. Any process * not advertising a role might have the importance of a superuser-owned * backend, so treat it that way. */ if ((!OidIsValid(proc->roleId) || superuser_arg(proc->roleId)) && !superuser()) return SIGNAL_BACKEND_NOSUPERUSER; This is what Im planning to do - If the backend is autovacuum worker and the user is not superuser or has pg_signal_autovacuumrole, we return the new value and provide the relevant error message /* * If the backend is autovacuum worker, allow user with privileges of the * pg_signal_autovacuum role to signal the backend. */ if (pgstat_get_backend_type(proc->backendId) == B_AUTOVAC_WORKER) { if (!has_privs_of_role(GetUserId(), ROLE_PG_SIGNAL_AUTOVACUUM) || !superuser()) return SIGNAL_BACKEND_NOAUTOVACUUM; } /* * Only allow superusers to signal superuser-owned backends. Any process * not advertising a role might have the importance of a superuser-owned * backend, so treat it that way. */ else if ((!OidIsValid(proc->roleId) || superuser_arg(proc->roleId)) && !superuser()) { return SIGNAL_BACKEND_NOSUPERUSER; } /* Users can signal backends they have role membership in. */ else if (!has_privs_of_role(GetUserId(), proc->roleId) && !has_privs_of_role(GetUserId(), ROLE_PG_SIGNAL_BACKEND)) { return SIGNAL_BACKEND_NOPERMISSION; } > We can add tests just like [0] with injection points. > I mean replace that "sleep 1" with something like "$node->wait_for_event('autovacuum worker', 'autocauum-runing');". > Currently we have no infrastructure to wait for autovacuum of particular table, but I think it's doable. > Also I do not like that test is changing system-wide autovac settings, AFAIR these settings can be set for particular table. Thanks for the suggestion. I will take a look at this. Let me also separate the test into a different patch file. -- Anthony Leung Amazon Web Services: https://aws.amazon.com
В списке pgsql-hackers по дате отправления:
Предыдущее
От: Thomas MunroДата:
Сообщение: Re: Cutting support for OpenSSL 1.0.1 and 1.0.2 in 17~?