Re: Avoid orphaned objects dependencies, take 3

Hi everybody,

Apologies for jumping into this conversation.

Our customers have also encountered a similar issue with the
concurrent drop of a dependent object. In our code (in the Greengage
DB), we implemented a fix analogous to one of the first versions of
Bertrand's approach, using locking within the pg_depend code.

In the long term, however, we are interested in aligning with the
upstream Postgres code as much as possible.

Since there haven't been any recent updates in this thread, we were
wondering if there are any plans for the next steps regarding this
issue.

Thank you in advance for your insights.

Best regards,
Roman Eskin

06.06.2025 15:37, Bertrand Drouvot пишет:
> Hi,
> 
> On Tue, Jun 03, 2025 at 01:27:29PM -0400, Robert Haas wrote:
>> On Mon, Jun 2, 2025 at 10:02 AM Bertrand Drouvot
>> <bertranddrouvot.pg@gmail.com> wrote:
>>> So, we currently have 2 patterns:
>>>
>>> P1: permission checks on a referenced object is done before we call recordMultipleDependencies()
>>> P2: permission checks on a referenced object is done after we call recordMultipleDependencies()
>>>
>>> So, if we add locking in recordMultipleDependencies() then I think that P2 is worst
>>> than P1 (there is still the risk that the permissions changed by the time
>>> we reach recordMultipleDependencies() though).
>>
>> Hmm. I don't think I agree.
> 
> Thanks for sharing your thoughts!
> 
>> If I understand correctly, P2 only permits
>> users to take a lock on an object they shouldn't be able to touch,
>> permitting them to temporarily interfere with access to it. P1 permits
>> users to potentially perform a permanent catalog modification that
>> should have been blocked by the permissions system. To my knowledge,
>> we've never formally classified the former type of problem as a
>> security vulnerability, although maybe there's an argument that it is
>> one. We've filed CVEs for problems of the latter sort.
> 
> What I meant to say is that P2 is worse "by design" because it's "always" wrong
> to lock an object we don't have a permission on: so the permission should be
> checked first.
> 
> So we have:
> 
> P1:
> 
>   * check_permissions()
>   * permissions could change here
>   * Lock in recordMultipleDependencies()
> 
> P2:
> 
>   * Lock in recordMultipleDependencies()
>   * FWIW, permissions could change here too (for example, one could still "
> revoke usage on schema myschema1 from user1" while there is an AccessShareLock
> on schema myschema1)
>   * check_permissions()
> 
> But P2 sequence of events is "wrong" by design (to lock an object we may not
> have permissions on) that's what I meant.
> 
> Now if we look at it from a "pure" security angle (as you did) I agree that P1 is
> the worse because it could allow catalog change that should have been blocked by
> the permission check.  P2 would prevent that. I agree that we should focus on
> P1 then.
> 
> Let me try to list the P1 cases (instead of the P2 ones) so that we can focus on
> /discuss those.
> 
> Regards,
> 

-- 
	С уважением,
Роман Ескин | C Developer
+7-964-449-81-15 | _r.eskin@arenadata.io_ <mailto:r.eskin@arenadata.io>
_Arenadata_ <https://arenadata.tech/> | Подписывайтесь на нас в Telegram
<https://t.me/arenadata> и VK <https://vk.com/arenadata>!

<https://arenadata.tech/events/>

УВЕДОМЛЕНИЕ О КОНФИДЕНЦИАЛЬНОСТИ:
Это электронное сообщение и любые документы, приложенные к нему, могут
содержать конфиденциальную информацию и предназначаются только
адресату/адресатам. Настоящим уведомляем Вас о том, что Вы не можете
воспроизводить, распространять и разглашать содержимое данного
электронного сообщения кому-либо без письменного согласия ООО «Аренадата
Софтвер». Разглашение конфиденциальной информации строго запрещено. Если
Вы получили это сообщение по ошибке, пожалуйста, сообщите об этом
отправителю по электронной почте и немедленно удалите это сообщение.