Re: Should rolpassword be toastable?

On Thu, Sep 19, 2024 at 07:37:55PM -0400, Jonathan S. Katz wrote:
> On 9/19/24 6:14 PM, Tom Lane wrote:
>> Nathan Bossart <nathandbossart(at)gmail(dot)com> writes:
>> > Oh, actually, I see that we are already validating the hash, but you can
>> > create valid SCRAM-SHA-256 hashes that are really long.
>
> You _can_, but it's up to a driver or a very determined user to do this, as
> it involves creating a very long salt.

I can't think of any reason to support this, unless we want Alexander to
find more bugs.

>> So putting an
>> > arbitrary limit (patch attached) is probably the correct path forward. I'd
>> > also remove pg_authid's TOAST table while at it.
>>
>> Shouldn't we enforce the limit in every case in encrypt_password,
>> not just this one? (I do agree that encrypt_password is an okay
>> place to enforce it.)

Yeah, that seems like a good idea. I've attached a more fleshed-out patch
set that applies the limit in all cases.

> +1; if there's any breakage, my guess is it would be on very long plaintext
> passwords, but that would be from a very old upgrade?

IIUC there's zero support for plain-text passwords in newer versions, and
any that remain in older clusters will be silently converted to a hash by
pg_upgrade.

>> I think you will get pushback from a limit of 256 bytes --- I seem
>> to recall discussion of actual use-cases where people were using
>> strings of a couple of kB. Whatever the limit is, the error message
>> had better cite it explicitly.
>
> I think it's OK to be a bit generous with the limit. Also, currently oru
> hashes are 256-bit (I know the above says byte), but this could increase
> should we support larger hashes.

Hm. Are you thinking of commit 67a472d? That one removed the password
length restrictions in client-side code and password message packets, which
I think is entirely separate from the lengths of the hashes stored in
rolpassword.

>> Also, the ereport call needs an errcode.
>> ERRCODE_PROGRAM_LIMIT_EXCEEDED is probably suitable.

This is added in v2.

--
nathan