Re: [PATCH] Add CHECK_FOR_INTERRUPTS in scram_SaltedPassword loop. - Mailing list pgsql-hackers

From Michael Paquier
Subject Re: [PATCH] Add CHECK_FOR_INTERRUPTS in scram_SaltedPassword loop.
Date
Msg-id ZWFLwCjp8-DvBJ7n@paquier.xyz
Whole thread Raw
In response to Re: [PATCH] Add CHECK_FOR_INTERRUPTS in scram_SaltedPassword loop.  (Aleksander Alekseev <aleksander@timescale.com>)
Responses Re: [PATCH] Add CHECK_FOR_INTERRUPTS in scram_SaltedPassword loop.
Re: [PATCH] Add CHECK_FOR_INTERRUPTS in scram_SaltedPassword loop.
List pgsql-hackers
On Thu, Nov 23, 2023 at 11:19:51AM +0300, Aleksander Alekseev wrote:
>>> I don't think it would be useful to limit this at an arbitrary point, iteration
>>> count can be set per password and if someone wants a specific password to be
>>> super-hard to brute force then why should we limit that?
>>
>> I agree with that. Maybe some users do want a super-hard password.
>> RFC 7677 and RFC 5802 don't specify the maximum number of iterations.
>
> That's a fairly good point. However we are not obligated not to
> implement everything that is missing in RFC. Also in fact we already
> limit the number of iterations to INT_MAX.

INT_MAX, as in the limit that we have for integer GUCs and the
routines building the hashed entry, so the Postgres internals are what
defines the limit here.  I doubt that we'll see cases where somebody
would want more than that, but who knows in 10/20 years.

> If we decide to limit this number even further the actual problem is
> to figure out what the new practical limit would be. Regardless of the
> chosen number there is a possibility of breaking backward
> compatibility for certain users.

No idea what the limit should be if it were to be lowered down, but
I suspect that even a new lower limit could be an issue for hosts in
the low-end specs when it comes to DOS.  It's not like there are no
ways to eat CPU when you are already logged in.

> For this reason I believe merging the proposed patch would be the
> right move at this point. It doesn't make anything worse for the
> existing users and solves a potential problem for some of them.

Yeah, agreed.  Being stuck on a potential large tight loops is
something we tend to avoid in the backend, so I agree that this is a
thing to keep in the backend especially because we have
scram_iterations and that it is user-settable.

I think that we should backpatch that down to v16 at least where the
GUC has been introduced as it's more like a nuisance if one sets the
GUC to an incorrect value, and I'd like to apply the patch this way.
Any objections or comments regarding that?
--
Michael

Attachment

pgsql-hackers by date:

Previous
From: Alexander Korotkov
Date:
Subject: Re: POC, WIP: OR-clause support for indexes
Next
From: Alexander Korotkov
Date:
Subject: Re: POC, WIP: OR-clause support for indexes