Re: [PATCH] Add CHECK_FOR_INTERRUPTS in scram_SaltedPassword loop.

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