Обсуждение: [BUG] autovacuum may skip tables when session_authorization/role is set on database

Поиск
Список
Период
Сортировка

[BUG] autovacuum may skip tables when session_authorization/role is set on database

От
"Imseih (AWS), Sami"
Дата:

Hi,

 

A recent case in the field in which a database session_authorization is

altered to a non-superuser, non-owner of tables via alter database .. set session_authorization ..

caused autovacuum to skip tables.

 

The issue was discovered on 13.10, and the logs show such messages:

 

warning:  skipping "table1" --- only table or database owner can vacuum it

 

In HEAD, I can repro, but the message is now a bit different due to [1].

 

WARNING:  permission denied to vacuum "table1”, skipping it

 

It seems to me we should force an autovacuum worker to set the session userid to

a superuser.

 

Attached is a repro and a patch which sets the session user to the BOOTSTRAP superuser

at the start of the autovac worker.

 

Thoughts?

 

Regards,

 

Sami

Amazon Web Services (AWS)

 

 

[1] https://postgr.es/m/20220726.104712.912995710251150228.horikyota.ntt@gmail.com

 

Вложения

Re: [BUG] autovacuum may skip tables when session_authorization/role is set on database

От
Tom Lane
Дата:
"Imseih (AWS), Sami" <simseih@amazon.com> writes:
> A recent case in the field in which a database session_authorization is
> altered to a non-superuser, non-owner of tables via alter database .. set session_authorization ..
> caused autovacuum to skip tables.

That seems like an extremely not-bright idea.  What is the actual
use case for such a setting?  Doesn't it risk security problems?

> Attached is a repro and a patch which sets the session user to the BOOTSTRAP superuser
> at the start of the autovac worker.

I'm rather unimpressed by this proposal, first because there are
probably ten other ways to break autovac with ill-considered settings,
and second because if we do want to consider this a supported case,
what about other background processes?  They'd likely have issues
as well.

            regards, tom lane



Re: [BUG] autovacuum may skip tables when session_authorization/role is set on database

От
"Imseih (AWS), Sami"
Дата:
> What is the actual
> use case for such a setting? 

I don't have exact details on the use-case, bit this is not a common
use-case.

> Doesn't it risk security problems?

I cannot see how setting it on the database being more problematic than
setting it on a session level.


> I'm rather unimpressed by this proposal, first because there are
> probably ten other ways to break autovac with ill-considered settings,

There exists code in autovac that safeguard for such settings. For example,
statement_timeout, lock_timeout are disabled. There are a dozen or
more other settings that are overridden for autovac.

I see this being just another one to ensure that autovacuum always runs
as superuser.

> and second because if we do want to consider this a supported case,
> what about other background processes? They'd likely have issues
> as well.

I have not considered other background processes, but autovac is the only
one that I can think of which checks for relation permissions.

Regards,

Sami







On Thu, 14 Dec 2023 at 02:13, Imseih (AWS), Sami <simseih@amazon.com> wrote:
>
> Hi,
>
>
>
> A recent case in the field in which a database session_authorization is
>
> altered to a non-superuser, non-owner of tables via alter database .. set session_authorization ..
>
> caused autovacuum to skip tables.
>
>
>
> The issue was discovered on 13.10, and the logs show such messages:
>
>
>
> warning:  skipping "table1" --- only table or database owner can vacuum it
>
>
>
> In HEAD, I can repro, but the message is now a bit different due to [1].
>
>
>
> WARNING:  permission denied to vacuum "table1”, skipping it
>
>
>
> It seems to me we should force an autovacuum worker to set the session userid to
>
> a superuser.
>
>
>
> Attached is a repro and a patch which sets the session user to the BOOTSTRAP superuser
>
> at the start of the autovac worker.

Since there is not much interest on this patch, I have changed the
status with "Returned with Feedback". Feel free to propose a stronger
use case for the patch and add an entry for the same.

Regards,
Vignesh