Re: [16+] subscription can end up in inconsistent state

On Tue, Sep 12, 2023 at 12:30 AM Jeff Davis <pgsql@j-davis.com> wrote:
>
> On Mon, 2023-09-11 at 08:59 +0530, Amit Kapila wrote:
> > Can we think of calling walrcv_check_conninfo() when the following
> > check is true (MySubscription->passwordrequired &&
> > !superuser_arg(MySubscription->owner))? IIUC this has to be done for
> > both apply_worker and tablesync_worker.
>
> To me, it would make sense to just have walrcv_connect() do both checks
> (a) and (b) -- rather than only check (b) -- when must_use_password is
> true. Separating these checks (which are really two parts of the same
> check) led to this problem in the first place.
>

Sounds reasonable to me. However, we need to think about the call to
walrcv_check_conninfo() in CreateSubscription(). Do we want to remove
that as anyway, we will do that check via walrcv_connect()? If we do
so, then there is another question. Currently, walrcv_check_conninfo
is being invoked even when we don't connect but do we need to do that
in the first place?

Another point is that if we want to unify such a check at the time of
walrcv_connect() then do we need to do it at the time of Alter
Subscription? I think it will probably be better to catch the problem
early but does removing it from Alter Subscription time and doing it
at connect time lead to security hazards?

> Another thought: we may also need to invalidate the subscription worker
> in cases where a user loses their superuser status.
>

Yeah, I think that will be a good idea, especially in this context
where it can also affect remote connection.

--
With Regards,
Amit Kapila.