pg.ddx.io  pgsql-hackers@postgresql.org mailing list archive  
help / color / mirror / Atom feed
Should rolpassword be toastable?
27+ messages / 7 participants
[nested] [flat]

Commits from this thread:
  2024-10-07 8275325a06  Restrict password hash length.
  2024-09-21 6aa44060a3  Remove pg_authid's TOAST table.
* Should rolpassword be toastable?
@ 2023-09-23 12:00 Alexander Lakhin <exclusion@gmail.com>
  2023-09-23 14:39 ` Re: Should rolpassword be toastable? Tom Lane <tgl@sss.pgh.pa.us>
  0 siblings, 1 reply; 27+ messages in thread

From: Alexander Lakhin @ 2023-09-23 12:00 UTC (permalink / raw)
  To: pgsql-hackers

Hello hackers,

When playing with oversized tuples, I've found that it's possible to set
such oversized password for a user, that could not be validated.
For example:
SELECT format('CREATE ROLE test_user LOGIN PASSWORD ''SCRAM-SHA-256$' || repeat('0', 2000000) || 
'4096:NuDacwYSUxeOeFUEf3ivTQ==$Wgvq3OCYrJI6eUfvKlAzn4p/j3mzgWzXbVnWeFK1qhY=:r1qSP0j2QojCjLpFUjI0i6ckInvxJDKoyWnN3zF8WCM='';')
\gexec
-- the password is "pass"
(One can achieve the same result with a large salt size, for example, 2048.)

psql -U "test_user" -c "SELECT 1"
psql: error: connection to server on socket "/tmp/.s.PGSQL.5432" failed: FATAL:  cannot read pg_class without having 
selected a database

I've tried to set attstorage = 'p' for the rolpassword attribute forcefully
by dirty hacking genbki.pl, and as a result I get an error on CREATE ROLE:
ERROR:  row is too big: size 2000256, maximum size 8160

Best regards,
Alexander





^ permalink  raw  reply  [nested|flat] 27+ messages in thread

* Re: Should rolpassword be toastable?
  2023-09-23 12:00 Should rolpassword be toastable? Alexander Lakhin <exclusion@gmail.com>
@ 2023-09-23 14:39 ` Tom Lane <tgl@sss.pgh.pa.us>
  2023-09-23 18:00   ` Re: Should rolpassword be toastable? Alexander Lakhin <exclusion@gmail.com>
  0 siblings, 1 reply; 27+ messages in thread

From: Tom Lane @ 2023-09-23 14:39 UTC (permalink / raw)
  To: Alexander Lakhin <exclusion@gmail.com>; +Cc: pgsql-hackers

Alexander Lakhin <exclusion@gmail.com> writes:
> When playing with oversized tuples, I've found that it's possible to set
> such oversized password for a user, that could not be validated.
> For example:
> ...
> psql -U "test_user" -c "SELECT 1"
> psql: error: connection to server on socket "/tmp/.s.PGSQL.5432" failed: FATAL:  cannot read pg_class without having 
> selected a database

My inclination is to fix this by removing pg_authid's toast table.
I was not in favor of "let's attach a toast table to every catalog"
to begin with, and I think this failure graphically illustrates
why that was not as great an idea as some people thought.
I also don't think it's worth trying to make it really work.

I'm also now more than just slightly skeptical about whether
pg_database should have a toast table.  Has anybody tried,
say, storing a daticurules field wide enough to end up
out-of-line?

			regards, tom lane





^ permalink  raw  reply  [nested|flat] 27+ messages in thread

* Re: Should rolpassword be toastable?
  2023-09-23 12:00 Should rolpassword be toastable? Alexander Lakhin <exclusion@gmail.com>
  2023-09-23 14:39 ` Re: Should rolpassword be toastable? Tom Lane <tgl@sss.pgh.pa.us>
@ 2023-09-23 18:00   ` Alexander Lakhin <exclusion@gmail.com>
  2024-09-19 03:00     ` Re: Should rolpassword be toastable? Alexander Lakhin <exclusion@gmail.com>
  0 siblings, 1 reply; 27+ messages in thread

From: Alexander Lakhin @ 2023-09-23 18:00 UTC (permalink / raw)
  To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: pgsql-hackers

23.09.2023 17:39, Tom Lane wrote:
> I'm also now more than just slightly skeptical about whether
> pg_database should have a toast table.  Has anybody tried,
> say, storing a daticurules field wide enough to end up
> out-of-line?

I tried, but failed, because pg_database accessed in InitPostgres() before
assigning MyDatabaseId only via the function GetDatabaseTupleByOid(),
which doesn't unpack the database tuple.
Another access to a system catalog with unassigned MyDatabaseId might occur
in the has_privs_of_role() call, but pg_auth_members contains no toastable
attributes.
So for now only pg_authid is worthy of condemnation, AFAICS.

Best regards,
Alexander





^ permalink  raw  reply  [nested|flat] 27+ messages in thread

* Re: Should rolpassword be toastable?
  2023-09-23 12:00 Should rolpassword be toastable? Alexander Lakhin <exclusion@gmail.com>
  2023-09-23 14:39 ` Re: Should rolpassword be toastable? Tom Lane <tgl@sss.pgh.pa.us>
  2023-09-23 18:00   ` Re: Should rolpassword be toastable? Alexander Lakhin <exclusion@gmail.com>
@ 2024-09-19 03:00     ` Alexander Lakhin <exclusion@gmail.com>
  2024-09-19 14:22       ` Re: Should rolpassword be toastable? Nathan Bossart <nathandbossart@gmail.com>
  0 siblings, 1 reply; 27+ messages in thread

From: Alexander Lakhin @ 2024-09-19 03:00 UTC (permalink / raw)
  To: Tom Lane <tgl@sss.pgh.pa.us>; Nathan Bossart <nathandbossart@gmail.com>; +Cc: pgsql-hackers

Hello Tom and Nathan,

23.09.2023 21:00, Alexander Lakhin wrote:
> 23.09.2023 17:39, Tom Lane wrote:
>> I'm also now more than just slightly skeptical about whether
>> pg_database should have a toast table.  Has anybody tried,
>> say, storing a daticurules field wide enough to end up
>> out-of-line?
>
> So for now only pg_authid is worthy of condemnation, AFAICS.
>

Let me remind you of this issue in light of b52c4fc3c.
Yes, it's opposite, but maybe it makes sense to fix it now in the hope that
~1 year of testing will bring something helpful for both changes.

Best regards,
Alexander





^ permalink  raw  reply  [nested|flat] 27+ messages in thread

* Re: Should rolpassword be toastable?
  2023-09-23 12:00 Should rolpassword be toastable? Alexander Lakhin <exclusion@gmail.com>
  2023-09-23 14:39 ` Re: Should rolpassword be toastable? Tom Lane <tgl@sss.pgh.pa.us>
  2023-09-23 18:00   ` Re: Should rolpassword be toastable? Alexander Lakhin <exclusion@gmail.com>
  2024-09-19 03:00     ` Re: Should rolpassword be toastable? Alexander Lakhin <exclusion@gmail.com>
@ 2024-09-19 14:22       ` Nathan Bossart <nathandbossart@gmail.com>
  2024-09-19 14:31         ` Re: Should rolpassword be toastable? Tom Lane <tgl@sss.pgh.pa.us>
  0 siblings, 1 reply; 27+ messages in thread

From: Nathan Bossart @ 2024-09-19 14:22 UTC (permalink / raw)
  To: Alexander Lakhin <exclusion@gmail.com>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; pgsql-hackers

On Thu, Sep 19, 2024 at 06:00:00AM +0300, Alexander Lakhin wrote:
> 23.09.2023 21:00, Alexander Lakhin wrote:
>> So for now only pg_authid is worthy of condemnation, AFAICS.
> 
> Let me remind you of this issue in light of b52c4fc3c.
> Yes, it's opposite, but maybe it makes sense to fix it now in the hope that
> ~1 year of testing will bring something helpful for both changes.

Hm.  It does seem like there's little point in giving pg_authid a TOAST
table, as rolpassword is the only varlena column, and it obviously has
problems.  But wouldn't removing it just trade one unhelpful internal error
when trying to log in for another when trying to add a really long password
hash (which hopefully nobody is really trying to do in practice)?  I wonder
if we could make this a little more user-friendly.

-- 
nathan





^ permalink  raw  reply  [nested|flat] 27+ messages in thread

* Re: Should rolpassword be toastable?
  2023-09-23 12:00 Should rolpassword be toastable? Alexander Lakhin <exclusion@gmail.com>
  2023-09-23 14:39 ` Re: Should rolpassword be toastable? Tom Lane <tgl@sss.pgh.pa.us>
  2023-09-23 18:00   ` Re: Should rolpassword be toastable? Alexander Lakhin <exclusion@gmail.com>
  2024-09-19 03:00     ` Re: Should rolpassword be toastable? Alexander Lakhin <exclusion@gmail.com>
  2024-09-19 14:22       ` Re: Should rolpassword be toastable? Nathan Bossart <nathandbossart@gmail.com>
@ 2024-09-19 14:31         ` Tom Lane <tgl@sss.pgh.pa.us>
  2024-09-19 17:44           ` Re: Should rolpassword be toastable? Nathan Bossart <nathandbossart@gmail.com>
  0 siblings, 1 reply; 27+ messages in thread

From: Tom Lane @ 2024-09-19 14:31 UTC (permalink / raw)
  To: Nathan Bossart <nathandbossart@gmail.com>; +Cc: Alexander Lakhin <exclusion@gmail.com>; pgsql-hackers

Nathan Bossart <nathandbossart@gmail.com> writes:
> Hm.  It does seem like there's little point in giving pg_authid a TOAST
> table, as rolpassword is the only varlena column, and it obviously has
> problems.  But wouldn't removing it just trade one unhelpful internal error
> when trying to log in for another when trying to add a really long password
> hash (which hopefully nobody is really trying to do in practice)?  I wonder
> if we could make this a little more user-friendly.

We could put an arbitrary limit (say, half of BLCKSZ) on the length of
passwords.

			regards, tom lane





^ permalink  raw  reply  [nested|flat] 27+ messages in thread

* Re: Should rolpassword be toastable?
  2023-09-23 12:00 Should rolpassword be toastable? Alexander Lakhin <exclusion@gmail.com>
  2023-09-23 14:39 ` Re: Should rolpassword be toastable? Tom Lane <tgl@sss.pgh.pa.us>
  2023-09-23 18:00   ` Re: Should rolpassword be toastable? Alexander Lakhin <exclusion@gmail.com>
  2024-09-19 03:00     ` Re: Should rolpassword be toastable? Alexander Lakhin <exclusion@gmail.com>
  2024-09-19 14:22       ` Re: Should rolpassword be toastable? Nathan Bossart <nathandbossart@gmail.com>
  2024-09-19 14:31         ` Re: Should rolpassword be toastable? Tom Lane <tgl@sss.pgh.pa.us>
@ 2024-09-19 17:44           ` Nathan Bossart <nathandbossart@gmail.com>
  2024-09-19 21:52             ` Re: Should rolpassword be toastable? Nathan Bossart <nathandbossart@gmail.com>
  0 siblings, 1 reply; 27+ messages in thread

From: Nathan Bossart @ 2024-09-19 17:44 UTC (permalink / raw)
  To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Alexander Lakhin <exclusion@gmail.com>; pgsql-hackers

On Thu, Sep 19, 2024 at 10:31:15AM -0400, Tom Lane wrote:
> Nathan Bossart <nathandbossart@gmail.com> writes:
>> Hm.  It does seem like there's little point in giving pg_authid a TOAST
>> table, as rolpassword is the only varlena column, and it obviously has
>> problems.  But wouldn't removing it just trade one unhelpful internal error
>> when trying to log in for another when trying to add a really long password
>> hash (which hopefully nobody is really trying to do in practice)?  I wonder
>> if we could make this a little more user-friendly.
> 
> We could put an arbitrary limit (say, half of BLCKSZ) on the length of
> passwords.

Something like that could be good enough.  I was thinking about actually
validating that the hash had the correct form, but that might be a little
more complex than is warranted here.

-- 
nathan





^ permalink  raw  reply  [nested|flat] 27+ messages in thread

* Re: Should rolpassword be toastable?
  2023-09-23 12:00 Should rolpassword be toastable? Alexander Lakhin <exclusion@gmail.com>
  2023-09-23 14:39 ` Re: Should rolpassword be toastable? Tom Lane <tgl@sss.pgh.pa.us>
  2023-09-23 18:00   ` Re: Should rolpassword be toastable? Alexander Lakhin <exclusion@gmail.com>
  2024-09-19 03:00     ` Re: Should rolpassword be toastable? Alexander Lakhin <exclusion@gmail.com>
  2024-09-19 14:22       ` Re: Should rolpassword be toastable? Nathan Bossart <nathandbossart@gmail.com>
  2024-09-19 14:31         ` Re: Should rolpassword be toastable? Tom Lane <tgl@sss.pgh.pa.us>
  2024-09-19 17:44           ` Re: Should rolpassword be toastable? Nathan Bossart <nathandbossart@gmail.com>
@ 2024-09-19 21:52             ` Nathan Bossart <nathandbossart@gmail.com>
  2024-09-19 22:14               ` Re: Should rolpassword be toastable? Tom Lane <tgl@sss.pgh.pa.us>
  0 siblings, 1 reply; 27+ messages in thread

From: Nathan Bossart @ 2024-09-19 21:52 UTC (permalink / raw)
  To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Alexander Lakhin <exclusion@gmail.com>; pgsql-hackers

On Thu, Sep 19, 2024 at 12:44:32PM -0500, Nathan Bossart wrote:
> On Thu, Sep 19, 2024 at 10:31:15AM -0400, Tom Lane wrote:
>> We could put an arbitrary limit (say, half of BLCKSZ) on the length of
>> passwords.
> 
> Something like that could be good enough.  I was thinking about actually
> validating that the hash had the correct form, but that might be a little
> more complex than is warranted here.

Oh, actually, I see that we are already validating the hash, but you can
create valid SCRAM-SHA-256 hashes that are really long.  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.

-- 
nathan

Attachments:

fail_for_long_scram_hash.patch (text/plain, 966B) download diff --git a/src/backend/libpq/crypt.c b/src/backend/libpq/crypt.c index 629e51e00b..a723e8219a 100644 --- a/src/backend/libpq/crypt.c +++ b/src/backend/libpq/crypt.c @@ -121,6 +121,19 @@ encrypt_password(PasswordType target_type, const char *role, if (guessed_type != PASSWORD_TYPE_PLAINTEXT) { + /* + * Valid SCRAM-SHA-256 hashes can have very long "iterations" and + * "salt" fields, but we don't want to store anything that might get + * TOASTed, since de-TOASTing won't work during authentication because + * we haven't selected a database yet and cannot read pg_class. 256 + * bytes should be more than enough for all practical use, so fail for + * anything longer. + */ + if (guessed_type == PASSWORD_TYPE_SCRAM_SHA_256 && + strlen(password) > 256) + ereport(ERROR, + (errmsg("SCRAM-SHA-256 hash is too long"))); + /* * Cannot convert an already-encrypted password from one format to * another, so return it as it is.
^ permalink raw reply [nested|flat] 27+ messages in thread

* Re: Should rolpassword be toastable?
  2023-09-23 12:00 Should rolpassword be toastable? Alexander Lakhin <exclusion@gmail.com>
  2023-09-23 14:39 ` Re: Should rolpassword be toastable? Tom Lane <tgl@sss.pgh.pa.us>
  2023-09-23 18:00   ` Re: Should rolpassword be toastable? Alexander Lakhin <exclusion@gmail.com>
  2024-09-19 03:00     ` Re: Should rolpassword be toastable? Alexander Lakhin <exclusion@gmail.com>
  2024-09-19 14:22       ` Re: Should rolpassword be toastable? Nathan Bossart <nathandbossart@gmail.com>
  2024-09-19 14:31         ` Re: Should rolpassword be toastable? Tom Lane <tgl@sss.pgh.pa.us>
  2024-09-19 17:44           ` Re: Should rolpassword be toastable? Nathan Bossart <nathandbossart@gmail.com>
  2024-09-19 21:52             ` Re: Should rolpassword be toastable? Nathan Bossart <nathandbossart@gmail.com>
@ 2024-09-19 22:14               ` Tom Lane <tgl@sss.pgh.pa.us>
  2024-09-19 23:37                 ` Re: Should rolpassword be toastable? Jonathan S. Katz <jkatz@postgresql.org>
  0 siblings, 1 reply; 27+ messages in thread

From: Tom Lane @ 2024-09-19 22:14 UTC (permalink / raw)
  To: Nathan Bossart <nathandbossart@gmail.com>; +Cc: Alexander Lakhin <exclusion@gmail.com>; pgsql-hackers

Nathan Bossart <nathandbossart@gmail.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.  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.)

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.

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

			regards, tom lane





^ permalink  raw  reply  [nested|flat] 27+ messages in thread

* Re: Should rolpassword be toastable?
  2023-09-23 12:00 Should rolpassword be toastable? Alexander Lakhin <exclusion@gmail.com>
  2023-09-23 14:39 ` Re: Should rolpassword be toastable? Tom Lane <tgl@sss.pgh.pa.us>
  2023-09-23 18:00   ` Re: Should rolpassword be toastable? Alexander Lakhin <exclusion@gmail.com>
  2024-09-19 03:00     ` Re: Should rolpassword be toastable? Alexander Lakhin <exclusion@gmail.com>
  2024-09-19 14:22       ` Re: Should rolpassword be toastable? Nathan Bossart <nathandbossart@gmail.com>
  2024-09-19 14:31         ` Re: Should rolpassword be toastable? Tom Lane <tgl@sss.pgh.pa.us>
  2024-09-19 17:44           ` Re: Should rolpassword be toastable? Nathan Bossart <nathandbossart@gmail.com>
  2024-09-19 21:52             ` Re: Should rolpassword be toastable? Nathan Bossart <nathandbossart@gmail.com>
  2024-09-19 22:14               ` Re: Should rolpassword be toastable? Tom Lane <tgl@sss.pgh.pa.us>
@ 2024-09-19 23:37                 ` Jonathan S. Katz <jkatz@postgresql.org>
  2024-09-20 02:46                   ` Re: Should rolpassword be toastable? Nathan Bossart <nathandbossart@gmail.com>
  0 siblings, 1 reply; 27+ messages in thread

From: Jonathan S. Katz @ 2024-09-19 23:37 UTC (permalink / raw)
  To: Tom Lane <tgl@sss.pgh.pa.us>; Nathan Bossart <nathandbossart@gmail.com>; +Cc: Alexander Lakhin <exclusion@gmail.com>; pgsql-hackers

On 9/19/24 6:14 PM, Tom Lane wrote:
> Nathan Bossart <nathandbossart@gmail.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.

>  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.)

+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?

> 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.

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

Jonathan


Attachments:

  OpenPGP_signature.asc (application/pgp-signature, 839B) download

^ permalink  raw  reply  [nested|flat] 27+ messages in thread

* Re: Should rolpassword be toastable?
  2023-09-23 12:00 Should rolpassword be toastable? Alexander Lakhin <exclusion@gmail.com>
  2023-09-23 14:39 ` Re: Should rolpassword be toastable? Tom Lane <tgl@sss.pgh.pa.us>
  2023-09-23 18:00   ` Re: Should rolpassword be toastable? Alexander Lakhin <exclusion@gmail.com>
  2024-09-19 03:00     ` Re: Should rolpassword be toastable? Alexander Lakhin <exclusion@gmail.com>
  2024-09-19 14:22       ` Re: Should rolpassword be toastable? Nathan Bossart <nathandbossart@gmail.com>
  2024-09-19 14:31         ` Re: Should rolpassword be toastable? Tom Lane <tgl@sss.pgh.pa.us>
  2024-09-19 17:44           ` Re: Should rolpassword be toastable? Nathan Bossart <nathandbossart@gmail.com>
  2024-09-19 21:52             ` Re: Should rolpassword be toastable? Nathan Bossart <nathandbossart@gmail.com>
  2024-09-19 22:14               ` Re: Should rolpassword be toastable? Tom Lane <tgl@sss.pgh.pa.us>
  2024-09-19 23:37                 ` Re: Should rolpassword be toastable? Jonathan S. Katz <jkatz@postgresql.org>
@ 2024-09-20 02:46                   ` Nathan Bossart <nathandbossart@gmail.com>
  2024-09-20 05:23                     ` Re: Should rolpassword be toastable? Michael Paquier <michael@paquier.xyz>
  0 siblings, 1 reply; 27+ messages in thread

From: Nathan Bossart @ 2024-09-20 02:46 UTC (permalink / raw)
  To: Jonathan S. Katz <jkatz@postgresql.org>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; Alexander Lakhin <exclusion@gmail.com>; pgsql-hackers

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@gmail.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

Attachments: download all (2 files, .tar.gz)

v2-0001-place-limit-on-password-hash-length.patch (text/plain, 6.6K) download From 0493783d99195080f5ef48f8b5d749b2a3979be6 Mon Sep 17 00:00:00 2001 From: Nathan Bossart <nathan@postgresql.org> Date: Thu, 19 Sep 2024 20:59:10 -0500 Subject: [PATCH v2 1/2] place limit on password hash length --- src/backend/libpq/crypt.c | 56 ++++++++++++++++++-------- src/include/libpq/crypt.h | 9 +++++ src/test/regress/expected/password.out | 7 ++++ src/test/regress/sql/password.sql | 4 ++ 4 files changed, 59 insertions(+), 17 deletions(-) diff --git a/src/backend/libpq/crypt.c b/src/backend/libpq/crypt.c index 629e51e00b..2cdc977bea 100644 --- a/src/backend/libpq/crypt.c +++ b/src/backend/libpq/crypt.c @@ -125,32 +125,54 @@ encrypt_password(PasswordType target_type, const char *role, * Cannot convert an already-encrypted password from one format to * another, so return it as it is. */ - return pstrdup(password); + encrypted_password = pstrdup(password); } - - switch (target_type) + else { - case PASSWORD_TYPE_MD5: - encrypted_password = palloc(MD5_PASSWD_LEN + 1); + switch (target_type) + { + case PASSWORD_TYPE_MD5: + encrypted_password = palloc(MD5_PASSWD_LEN + 1); - if (!pg_md5_encrypt(password, role, strlen(role), - encrypted_password, &errstr)) - elog(ERROR, "password encryption failed: %s", errstr); - return encrypted_password; + if (!pg_md5_encrypt(password, role, strlen(role), + encrypted_password, &errstr)) + elog(ERROR, "password encryption failed: %s", errstr); + break; - case PASSWORD_TYPE_SCRAM_SHA_256: - return pg_be_scram_build_secret(password); + case PASSWORD_TYPE_SCRAM_SHA_256: + encrypted_password = pg_be_scram_build_secret(password); + break; - case PASSWORD_TYPE_PLAINTEXT: - elog(ERROR, "cannot encrypt password with 'plaintext'"); + case PASSWORD_TYPE_PLAINTEXT: + elog(ERROR, "cannot encrypt password with 'plaintext'"); + encrypted_password = palloc(0); /* keep compiler quiet */ + break; + } } /* - * This shouldn't happen, because the above switch statements should - * handle every combination of source and target password types. + * Valid password hashes may be very long, but we don't want to store + * anything that might get TOASTed, since de-TOASTing won't work during + * authentication because we haven't selected a database yet and cannot + * read pg_class. 256 bytes should be more than enough for all practical + * use, so fail for anything longer. */ - elog(ERROR, "cannot encrypt password to requested type"); - return NULL; /* keep compiler quiet */ + if (strlen(encrypted_password) > MAX_ENCRYPTED_PASSWORD_LEN) + { + /* + * We don't expect any of our own hashing routines to produce hashes + * that are too long. + */ + Assert(guessed_type != PASSWORD_TYPE_PLAINTEXT); + + ereport(ERROR, + (errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED), + errmsg("encrypted password is too long"), + errdetail("Encrypted passwords must be no longer than %d characters.", + MAX_ENCRYPTED_PASSWORD_LEN))); + } + + return encrypted_password; } /* diff --git a/src/include/libpq/crypt.h b/src/include/libpq/crypt.h index f744de4d20..7a1099b917 100644 --- a/src/include/libpq/crypt.h +++ b/src/include/libpq/crypt.h @@ -15,6 +15,15 @@ #include "datatype/timestamp.h" +/* + * Valid password hashes may be very long, but we don't want to store anything + * that might get TOASTed, since de-TOASTing won't work during authentication + * because we haven't selected a database yet and cannot read pg_class. 256 + * bytes should be more than enough for all practical use, and our own password + * encryption routines should never produce hashes longer than this. + */ +#define MAX_ENCRYPTED_PASSWORD_LEN (256) + /* * Types of password hashes or secrets. * diff --git a/src/test/regress/expected/password.out b/src/test/regress/expected/password.out index 924d6e001d..8ab995a162 100644 --- a/src/test/regress/expected/password.out +++ b/src/test/regress/expected/password.out @@ -127,6 +127,13 @@ SELECT rolname, rolpassword not like '%A6xHKoH/494E941doaPOYg==%' as is_rolpassw regress_passwd_sha_len2 | t (3 rows) +-- Test that valid hashes that are too long are rejected +CREATE ROLE regress_passwd10 PASSWORD 'SCRAM-SHA-256$00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000004096:NuDacwYSUxeOeFUEf3ivTQ==$Wgvq3OCYrJI6eUfvKlAzn4p/j3mzgWzXbVnWeFK1qhY=:r1qSP0j2QojCjLpFUjI0i6ckInvxJDKoyWnN3zF8WCM='; +ERROR: encrypted password is too long +DETAIL: Encrypted passwords must be no longer than 256 characters. +ALTER ROLE regress_passwd9 PASSWORD 'SCRAM-SHA-256$00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000004096:NuDacwYSUxeOeFUEf3ivTQ==$Wgvq3OCYrJI6eUfvKlAzn4p/j3mzgWzXbVnWeFK1qhY=:r1qSP0j2QojCjLpFUjI0i6ckInvxJDKoyWnN3zF8WCM='; +ERROR: encrypted password is too long +DETAIL: Encrypted passwords must be no longer than 256 characters. DROP ROLE regress_passwd1; DROP ROLE regress_passwd2; DROP ROLE regress_passwd3; diff --git a/src/test/regress/sql/password.sql b/src/test/regress/sql/password.sql index bb82aa4aa2..442c903c00 100644 --- a/src/test/regress/sql/password.sql +++ b/src/test/regress/sql/password.sql @@ -95,6 +95,10 @@ SELECT rolname, rolpassword not like '%A6xHKoH/494E941doaPOYg==%' as is_rolpassw WHERE rolname LIKE 'regress_passwd_sha_len%' ORDER BY rolname; +-- Test that valid hashes that are too long are rejected +CREATE ROLE regress_passwd10 PASSWORD 'SCRAM-SHA-256$00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000004096:NuDacwYSUxeOeFUEf3ivTQ==$Wgvq3OCYrJI6eUfvKlAzn4p/j3mzgWzXbVnWeFK1qhY=:r1qSP0j2QojCjLpFUjI0i6ckInvxJDKoyWnN3zF8WCM='; +ALTER ROLE regress_passwd9 PASSWORD 'SCRAM-SHA-256$00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000004096:NuDacwYSUxeOeFUEf3ivTQ==$Wgvq3OCYrJI6eUfvKlAzn4p/j3mzgWzXbVnWeFK1qhY=:r1qSP0j2QojCjLpFUjI0i6ckInvxJDKoyWnN3zF8WCM='; + DROP ROLE regress_passwd1; DROP ROLE regress_passwd2; DROP ROLE regress_passwd3; -- 2.39.5 (Apple Git-154)
v2-0002-remove-pg_authid-s-TOAST-table.patch (text/plain, 9.6K) download From 535057307ac9ad47668bed4c48767e6d131391bf Mon Sep 17 00:00:00 2001 From: Nathan Bossart <nathan@postgresql.org> Date: Thu, 19 Sep 2024 21:09:30 -0500 Subject: [PATCH v2 2/2] remove pg_authid's TOAST table XXX: NEEDS CATVERSION BUMP --- src/backend/catalog/catalog.c | 4 +--- src/include/catalog/pg_authid.h | 2 -- src/test/regress/expected/create_index.out | 12 ++++++------ src/test/regress/expected/misc_sanity.out | 5 ++++- src/test/regress/expected/tablespace.out | 12 ++++++------ src/test/regress/sql/create_index.sql | 8 ++++---- src/test/regress/sql/misc_sanity.sql | 2 ++ src/test/regress/sql/tablespace.sql | 8 ++++---- 8 files changed, 27 insertions(+), 26 deletions(-) diff --git a/src/backend/catalog/catalog.c b/src/backend/catalog/catalog.c index 6c39434a30..d6b07a7865 100644 --- a/src/backend/catalog/catalog.c +++ b/src/backend/catalog/catalog.c @@ -300,9 +300,7 @@ IsSharedRelation(Oid relationId) relationId == TablespaceOidIndexId) return true; /* These are their toast tables and toast indexes */ - if (relationId == PgAuthidToastTable || - relationId == PgAuthidToastIndex || - relationId == PgDatabaseToastTable || + if (relationId == PgDatabaseToastTable || relationId == PgDatabaseToastIndex || relationId == PgDbRoleSettingToastTable || relationId == PgDbRoleSettingToastIndex || diff --git a/src/include/catalog/pg_authid.h b/src/include/catalog/pg_authid.h index e08863f78a..e846d75731 100644 --- a/src/include/catalog/pg_authid.h +++ b/src/include/catalog/pg_authid.h @@ -55,8 +55,6 @@ CATALOG(pg_authid,1260,AuthIdRelationId) BKI_SHARED_RELATION BKI_ROWTYPE_OID(284 */ typedef FormData_pg_authid *Form_pg_authid; -DECLARE_TOAST_WITH_MACRO(pg_authid, 4175, 4176, PgAuthidToastTable, PgAuthidToastIndex); - DECLARE_UNIQUE_INDEX(pg_authid_rolname_index, 2676, AuthIdRolnameIndexId, pg_authid, btree(rolname name_ops)); DECLARE_UNIQUE_INDEX_PKEY(pg_authid_oid_index, 2677, AuthIdOidIndexId, pg_authid, btree(oid oid_ops)); diff --git a/src/test/regress/expected/create_index.out b/src/test/regress/expected/create_index.out index cf6eac5734..22c8787e80 100644 --- a/src/test/regress/expected/create_index.out +++ b/src/test/regress/expected/create_index.out @@ -2665,9 +2665,9 @@ ERROR: cannot reindex system catalogs concurrently REINDEX INDEX CONCURRENTLY pg_class_oid_index; -- no catalog index ERROR: cannot reindex system catalogs concurrently -- These are the toast table and index of pg_authid. -REINDEX TABLE CONCURRENTLY pg_toast.pg_toast_1260; -- no catalog toast table +REINDEX TABLE CONCURRENTLY pg_toast.pg_toast_1262; -- no catalog toast table ERROR: cannot reindex system catalogs concurrently -REINDEX INDEX CONCURRENTLY pg_toast.pg_toast_1260_index; -- no catalog toast index +REINDEX INDEX CONCURRENTLY pg_toast.pg_toast_1262_index; -- no catalog toast index ERROR: cannot reindex system catalogs concurrently REINDEX SYSTEM CONCURRENTLY postgres; -- not allowed for SYSTEM ERROR: cannot reindex system catalogs concurrently @@ -2974,10 +2974,10 @@ ERROR: must be owner of schema schema_to_reindex RESET ROLE; GRANT USAGE ON SCHEMA pg_toast TO regress_reindexuser; SET SESSION ROLE regress_reindexuser; -REINDEX TABLE pg_toast.pg_toast_1260; -ERROR: permission denied for table pg_toast_1260 -REINDEX INDEX pg_toast.pg_toast_1260_index; -ERROR: permission denied for index pg_toast_1260_index +REINDEX TABLE pg_toast.pg_toast_1262; +ERROR: permission denied for table pg_toast_1262 +REINDEX INDEX pg_toast.pg_toast_1262_index; +ERROR: permission denied for index pg_toast_1262_index -- Clean up RESET ROLE; REVOKE USAGE ON SCHEMA pg_toast FROM regress_reindexuser; diff --git a/src/test/regress/expected/misc_sanity.out b/src/test/regress/expected/misc_sanity.out index 45e7492877..b032a3f476 100644 --- a/src/test/regress/expected/misc_sanity.out +++ b/src/test/regress/expected/misc_sanity.out @@ -40,6 +40,8 @@ WHERE refclassid = 0 OR refobjid = 0 OR -- 2. pg_largeobject and pg_largeobject_metadata. Large object catalogs -- and toast tables are mutually exclusive and large object data is handled -- as user data by pg_upgrade, which would cause failures. +-- 3. pg_authid, since its toast table cannot be accessed when it would be +-- needed, i.e., during authentication before we've selected a database. SELECT relname, attname, atttypid::regtype FROM pg_class c JOIN pg_attribute a ON c.oid = attrelid WHERE c.oid < 16384 AND @@ -53,12 +55,13 @@ ORDER BY 1, 2; pg_attribute | attfdwoptions | text[] pg_attribute | attmissingval | anyarray pg_attribute | attoptions | text[] + pg_authid | rolpassword | text pg_class | relacl | aclitem[] pg_class | reloptions | text[] pg_class | relpartbound | pg_node_tree pg_largeobject | data | bytea pg_largeobject_metadata | lomacl | aclitem[] -(9 rows) +(10 rows) -- system catalogs without primary keys -- diff --git a/src/test/regress/expected/tablespace.out b/src/test/regress/expected/tablespace.out index 9aabb85349..dd535d41a3 100644 --- a/src/test/regress/expected/tablespace.out +++ b/src/test/regress/expected/tablespace.out @@ -51,13 +51,13 @@ ERROR: cannot move system relation "pg_authid_rolname_index" REINDEX (TABLESPACE regress_tblspace) TABLE CONCURRENTLY pg_authid; ERROR: cannot reindex system catalogs concurrently -- toast relations, fail -REINDEX (TABLESPACE regress_tblspace) INDEX pg_toast.pg_toast_1260_index; -ERROR: cannot move system relation "pg_toast_1260_index" -REINDEX (TABLESPACE regress_tblspace) INDEX CONCURRENTLY pg_toast.pg_toast_1260_index; +REINDEX (TABLESPACE regress_tblspace) INDEX pg_toast.pg_toast_1262_index; +ERROR: cannot move system relation "pg_toast_1262_index" +REINDEX (TABLESPACE regress_tblspace) INDEX CONCURRENTLY pg_toast.pg_toast_1262_index; ERROR: cannot reindex system catalogs concurrently -REINDEX (TABLESPACE regress_tblspace) TABLE pg_toast.pg_toast_1260; -ERROR: cannot move system relation "pg_toast_1260_index" -REINDEX (TABLESPACE regress_tblspace) TABLE CONCURRENTLY pg_toast.pg_toast_1260; +REINDEX (TABLESPACE regress_tblspace) TABLE pg_toast.pg_toast_1262; +ERROR: cannot move system relation "pg_toast_1262_index" +REINDEX (TABLESPACE regress_tblspace) TABLE CONCURRENTLY pg_toast.pg_toast_1262; ERROR: cannot reindex system catalogs concurrently -- system catalog, fail REINDEX (TABLESPACE pg_global) TABLE pg_authid; diff --git a/src/test/regress/sql/create_index.sql b/src/test/regress/sql/create_index.sql index e296891cab..145860b6ec 100644 --- a/src/test/regress/sql/create_index.sql +++ b/src/test/regress/sql/create_index.sql @@ -1127,8 +1127,8 @@ COMMIT; REINDEX TABLE CONCURRENTLY pg_class; -- no catalog relation REINDEX INDEX CONCURRENTLY pg_class_oid_index; -- no catalog index -- These are the toast table and index of pg_authid. -REINDEX TABLE CONCURRENTLY pg_toast.pg_toast_1260; -- no catalog toast table -REINDEX INDEX CONCURRENTLY pg_toast.pg_toast_1260_index; -- no catalog toast index +REINDEX TABLE CONCURRENTLY pg_toast.pg_toast_1262; -- no catalog toast table +REINDEX INDEX CONCURRENTLY pg_toast.pg_toast_1262_index; -- no catalog toast index REINDEX SYSTEM CONCURRENTLY postgres; -- not allowed for SYSTEM REINDEX (CONCURRENTLY) SYSTEM postgres; -- ditto REINDEX (CONCURRENTLY) SYSTEM; -- ditto @@ -1305,8 +1305,8 @@ REINDEX SCHEMA schema_to_reindex; RESET ROLE; GRANT USAGE ON SCHEMA pg_toast TO regress_reindexuser; SET SESSION ROLE regress_reindexuser; -REINDEX TABLE pg_toast.pg_toast_1260; -REINDEX INDEX pg_toast.pg_toast_1260_index; +REINDEX TABLE pg_toast.pg_toast_1262; +REINDEX INDEX pg_toast.pg_toast_1262_index; -- Clean up RESET ROLE; diff --git a/src/test/regress/sql/misc_sanity.sql b/src/test/regress/sql/misc_sanity.sql index 16f3a7c0c1..135793871b 100644 --- a/src/test/regress/sql/misc_sanity.sql +++ b/src/test/regress/sql/misc_sanity.sql @@ -43,6 +43,8 @@ WHERE refclassid = 0 OR refobjid = 0 OR -- 2. pg_largeobject and pg_largeobject_metadata. Large object catalogs -- and toast tables are mutually exclusive and large object data is handled -- as user data by pg_upgrade, which would cause failures. +-- 3. pg_authid, since its toast table cannot be accessed when it would be +-- needed, i.e., during authentication before we've selected a database. SELECT relname, attname, atttypid::regtype FROM pg_class c JOIN pg_attribute a ON c.oid = attrelid diff --git a/src/test/regress/sql/tablespace.sql b/src/test/regress/sql/tablespace.sql index d274d9615e..c8b83788f0 100644 --- a/src/test/regress/sql/tablespace.sql +++ b/src/test/regress/sql/tablespace.sql @@ -40,10 +40,10 @@ REINDEX (TABLESPACE regress_tblspace) TABLE CONCURRENTLY pg_am; REINDEX (TABLESPACE regress_tblspace) TABLE pg_authid; REINDEX (TABLESPACE regress_tblspace) TABLE CONCURRENTLY pg_authid; -- toast relations, fail -REINDEX (TABLESPACE regress_tblspace) INDEX pg_toast.pg_toast_1260_index; -REINDEX (TABLESPACE regress_tblspace) INDEX CONCURRENTLY pg_toast.pg_toast_1260_index; -REINDEX (TABLESPACE regress_tblspace) TABLE pg_toast.pg_toast_1260; -REINDEX (TABLESPACE regress_tblspace) TABLE CONCURRENTLY pg_toast.pg_toast_1260; +REINDEX (TABLESPACE regress_tblspace) INDEX pg_toast.pg_toast_1262_index; +REINDEX (TABLESPACE regress_tblspace) INDEX CONCURRENTLY pg_toast.pg_toast_1262_index; +REINDEX (TABLESPACE regress_tblspace) TABLE pg_toast.pg_toast_1262; +REINDEX (TABLESPACE regress_tblspace) TABLE CONCURRENTLY pg_toast.pg_toast_1262; -- system catalog, fail REINDEX (TABLESPACE pg_global) TABLE pg_authid; REINDEX (TABLESPACE pg_global) TABLE CONCURRENTLY pg_authid; -- 2.39.5 (Apple Git-154)
^ permalink raw reply [nested|flat] 27+ messages in thread

* Re: Should rolpassword be toastable?
  2023-09-23 12:00 Should rolpassword be toastable? Alexander Lakhin <exclusion@gmail.com>
  2023-09-23 14:39 ` Re: Should rolpassword be toastable? Tom Lane <tgl@sss.pgh.pa.us>
  2023-09-23 18:00   ` Re: Should rolpassword be toastable? Alexander Lakhin <exclusion@gmail.com>
  2024-09-19 03:00     ` Re: Should rolpassword be toastable? Alexander Lakhin <exclusion@gmail.com>
  2024-09-19 14:22       ` Re: Should rolpassword be toastable? Nathan Bossart <nathandbossart@gmail.com>
  2024-09-19 14:31         ` Re: Should rolpassword be toastable? Tom Lane <tgl@sss.pgh.pa.us>
  2024-09-19 17:44           ` Re: Should rolpassword be toastable? Nathan Bossart <nathandbossart@gmail.com>
  2024-09-19 21:52             ` Re: Should rolpassword be toastable? Nathan Bossart <nathandbossart@gmail.com>
  2024-09-19 22:14               ` Re: Should rolpassword be toastable? Tom Lane <tgl@sss.pgh.pa.us>
  2024-09-19 23:37                 ` Re: Should rolpassword be toastable? Jonathan S. Katz <jkatz@postgresql.org>
  2024-09-20 02:46                   ` Re: Should rolpassword be toastable? Nathan Bossart <nathandbossart@gmail.com>
@ 2024-09-20 05:23                     ` Michael Paquier <michael@paquier.xyz>
  2024-09-20 14:06                       ` Re: Should rolpassword be toastable? Jonathan S. Katz <jkatz@postgresql.org>
  0 siblings, 1 reply; 27+ messages in thread

From: Michael Paquier @ 2024-09-20 05:23 UTC (permalink / raw)
  To: Nathan Bossart <nathandbossart@gmail.com>; +Cc: Jonathan S. Katz <jkatz@postgresql.org>; Tom Lane <tgl@sss.pgh.pa.us>; Alexander Lakhin <exclusion@gmail.com>; pgsql-hackers

On Thu, Sep 19, 2024 at 09:46:00PM -0500, Nathan Bossart wrote:
> On Thu, Sep 19, 2024 at 07:37:55PM -0400, Jonathan S. Katz wrote:
>>> 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.

Not sure.  Is this really something we absolutely need?  Sure, this
generates a better error when inserting a record too long to
pg_authid, but removing the toast relation is enough to avoid the
problems one would see when authenticating.  Not sure if this argument
is enough to count as an objection, just sharing some doubts :)

Removing the toast relation for pg_authid sounds good to me.

>  -- These are the toast table and index of pg_authid.
> -REINDEX TABLE CONCURRENTLY pg_toast.pg_toast_1260; -- no catalog toast table
> +REINDEX TABLE CONCURRENTLY pg_toast.pg_toast_1262; -- no catalog toast table
>  ERROR:  cannot reindex system catalogs concurrently
> -REINDEX INDEX CONCURRENTLY pg_toast.pg_toast_1260_index; -- no catalog toast index
> +REINDEX INDEX CONCURRENTLY pg_toast.pg_toast_1262_index; -- no catalog toast index

This comment should be refreshed as of s/pg_authid/pg_database/.
--
Michael

Attachments:

  signature.asc (application/pgp-signature, 832B) download

^ permalink  raw  reply  [nested|flat] 27+ messages in thread

* Re: Should rolpassword be toastable?
  2023-09-23 12:00 Should rolpassword be toastable? Alexander Lakhin <exclusion@gmail.com>
  2023-09-23 14:39 ` Re: Should rolpassword be toastable? Tom Lane <tgl@sss.pgh.pa.us>
  2023-09-23 18:00   ` Re: Should rolpassword be toastable? Alexander Lakhin <exclusion@gmail.com>
  2024-09-19 03:00     ` Re: Should rolpassword be toastable? Alexander Lakhin <exclusion@gmail.com>
  2024-09-19 14:22       ` Re: Should rolpassword be toastable? Nathan Bossart <nathandbossart@gmail.com>
  2024-09-19 14:31         ` Re: Should rolpassword be toastable? Tom Lane <tgl@sss.pgh.pa.us>
  2024-09-19 17:44           ` Re: Should rolpassword be toastable? Nathan Bossart <nathandbossart@gmail.com>
  2024-09-19 21:52             ` Re: Should rolpassword be toastable? Nathan Bossart <nathandbossart@gmail.com>
  2024-09-19 22:14               ` Re: Should rolpassword be toastable? Tom Lane <tgl@sss.pgh.pa.us>
  2024-09-19 23:37                 ` Re: Should rolpassword be toastable? Jonathan S. Katz <jkatz@postgresql.org>
  2024-09-20 02:46                   ` Re: Should rolpassword be toastable? Nathan Bossart <nathandbossart@gmail.com>
  2024-09-20 05:23                     ` Re: Should rolpassword be toastable? Michael Paquier <michael@paquier.xyz>
@ 2024-09-20 14:06                       ` Jonathan S. Katz <jkatz@postgresql.org>
  2024-09-20 16:09                         ` Re: Should rolpassword be toastable? Nathan Bossart <nathandbossart@gmail.com>
  0 siblings, 1 reply; 27+ messages in thread

From: Jonathan S. Katz @ 2024-09-20 14:06 UTC (permalink / raw)
  To: Michael Paquier <michael@paquier.xyz>; Nathan Bossart <nathandbossart@gmail.com>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; Alexander Lakhin <exclusion@gmail.com>; pgsql-hackers

On 9/20/24 1:23 AM, Michael Paquier wrote:
> On Thu, Sep 19, 2024 at 09:46:00PM -0500, Nathan Bossart wrote:
>> On Thu, Sep 19, 2024 at 07:37:55PM -0400, Jonathan S. Katz wrote:
>>>> 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.
> 
> Not sure.  Is this really something we absolutely need?  Sure, this
> generates a better error when inserting a record too long to
> pg_authid, but removing the toast relation is enough to avoid the
> problems one would see when authenticating.  Not sure if this argument
> is enough to count as an objection, just sharing some doubts :)

The errors from lack of TOAST are confusing to users. Why can't we have 
a user friendly error here?

Jonathan


Attachments:

  OpenPGP_signature.asc (application/pgp-signature, 839B) download

^ permalink  raw  reply  [nested|flat] 27+ messages in thread

* Re: Should rolpassword be toastable?
  2023-09-23 12:00 Should rolpassword be toastable? Alexander Lakhin <exclusion@gmail.com>
  2023-09-23 14:39 ` Re: Should rolpassword be toastable? Tom Lane <tgl@sss.pgh.pa.us>
  2023-09-23 18:00   ` Re: Should rolpassword be toastable? Alexander Lakhin <exclusion@gmail.com>
  2024-09-19 03:00     ` Re: Should rolpassword be toastable? Alexander Lakhin <exclusion@gmail.com>
  2024-09-19 14:22       ` Re: Should rolpassword be toastable? Nathan Bossart <nathandbossart@gmail.com>
  2024-09-19 14:31         ` Re: Should rolpassword be toastable? Tom Lane <tgl@sss.pgh.pa.us>
  2024-09-19 17:44           ` Re: Should rolpassword be toastable? Nathan Bossart <nathandbossart@gmail.com>
  2024-09-19 21:52             ` Re: Should rolpassword be toastable? Nathan Bossart <nathandbossart@gmail.com>
  2024-09-19 22:14               ` Re: Should rolpassword be toastable? Tom Lane <tgl@sss.pgh.pa.us>
  2024-09-19 23:37                 ` Re: Should rolpassword be toastable? Jonathan S. Katz <jkatz@postgresql.org>
  2024-09-20 02:46                   ` Re: Should rolpassword be toastable? Nathan Bossart <nathandbossart@gmail.com>
  2024-09-20 05:23                     ` Re: Should rolpassword be toastable? Michael Paquier <michael@paquier.xyz>
  2024-09-20 14:06                       ` Re: Should rolpassword be toastable? Jonathan S. Katz <jkatz@postgresql.org>
@ 2024-09-20 16:09                         ` Nathan Bossart <nathandbossart@gmail.com>
  2024-09-20 16:27                           ` Re: Should rolpassword be toastable? Tom Lane <tgl@sss.pgh.pa.us>
  0 siblings, 1 reply; 27+ messages in thread

From: Nathan Bossart @ 2024-09-20 16:09 UTC (permalink / raw)
  To: Jonathan S. Katz <jkatz@postgresql.org>; +Cc: Michael Paquier <michael@paquier.xyz>; Tom Lane <tgl@sss.pgh.pa.us>; Alexander Lakhin <exclusion@gmail.com>; pgsql-hackers

On Fri, Sep 20, 2024 at 10:06:28AM -0400, Jonathan S. Katz wrote:
> On 9/20/24 1:23 AM, Michael Paquier wrote:
>> Not sure.  Is this really something we absolutely need?  Sure, this
>> generates a better error when inserting a record too long to
>> pg_authid, but removing the toast relation is enough to avoid the
>> problems one would see when authenticating.  Not sure if this argument
>> is enough to count as an objection, just sharing some doubts :)
> 
> The errors from lack of TOAST are confusing to users. Why can't we have a
> user friendly error here?

If I wanted to argue against adding a user-friendly error, I'd point out
that it's highly unlikely anyone is actually trying to use super long
hashes unless they are trying to break things, and it's just another
arbitrary limit that we'll need to maintain/enforce.  But on the off-chance
that someone is building a custom driver that generates long hashes for
whatever reason, I'd imagine that a clear error would be more helpful than
"row is too big."

Here is a v3 patch set that fixes the test comment and a compiler warning
in cfbot.

-- 
nathan

Attachments: download all (2 files, .tar.gz)

v3-0001-place-limit-on-password-hash-length.patch (text/plain, 7.0K) download From bb3aad9105b1997bc088403437ac6294e22809d9 Mon Sep 17 00:00:00 2001 From: Nathan Bossart <nathan@postgresql.org> Date: Thu, 19 Sep 2024 20:59:10 -0500 Subject: [PATCH v3 1/2] place limit on password hash length --- src/backend/libpq/crypt.c | 60 ++++++++++++++++++-------- src/include/libpq/crypt.h | 10 +++++ src/test/regress/expected/password.out | 7 +++ src/test/regress/sql/password.sql | 4 ++ 4 files changed, 63 insertions(+), 18 deletions(-) diff --git a/src/backend/libpq/crypt.c b/src/backend/libpq/crypt.c index 629e51e00b..753e5c11da 100644 --- a/src/backend/libpq/crypt.c +++ b/src/backend/libpq/crypt.c @@ -116,7 +116,7 @@ encrypt_password(PasswordType target_type, const char *role, const char *password) { PasswordType guessed_type = get_password_type(password); - char *encrypted_password; + char *encrypted_password = NULL; const char *errstr = NULL; if (guessed_type != PASSWORD_TYPE_PLAINTEXT) @@ -125,32 +125,56 @@ encrypt_password(PasswordType target_type, const char *role, * Cannot convert an already-encrypted password from one format to * another, so return it as it is. */ - return pstrdup(password); + encrypted_password = pstrdup(password); } - - switch (target_type) + else { - case PASSWORD_TYPE_MD5: - encrypted_password = palloc(MD5_PASSWD_LEN + 1); + switch (target_type) + { + case PASSWORD_TYPE_MD5: + encrypted_password = palloc(MD5_PASSWD_LEN + 1); - if (!pg_md5_encrypt(password, role, strlen(role), - encrypted_password, &errstr)) - elog(ERROR, "password encryption failed: %s", errstr); - return encrypted_password; + if (!pg_md5_encrypt(password, role, strlen(role), + encrypted_password, &errstr)) + elog(ERROR, "password encryption failed: %s", errstr); + break; - case PASSWORD_TYPE_SCRAM_SHA_256: - return pg_be_scram_build_secret(password); + case PASSWORD_TYPE_SCRAM_SHA_256: + encrypted_password = pg_be_scram_build_secret(password); + break; - case PASSWORD_TYPE_PLAINTEXT: - elog(ERROR, "cannot encrypt password with 'plaintext'"); + case PASSWORD_TYPE_PLAINTEXT: + elog(ERROR, "cannot encrypt password with 'plaintext'"); + break; + } } + Assert(encrypted_password); + /* - * This shouldn't happen, because the above switch statements should - * handle every combination of source and target password types. + * Valid password hashes may be very long, but we don't want to store + * anything that might need out-of-line storage, since de-TOASTing won't + * work during authentication because we haven't selected a database yet + * and cannot read pg_class. 256 bytes should be more than enough for all + * practical use, so fail for anything longer. */ - elog(ERROR, "cannot encrypt password to requested type"); - return NULL; /* keep compiler quiet */ + if (encrypted_password && /* keep compiler quiet */ + strlen(encrypted_password) > MAX_ENCRYPTED_PASSWORD_LEN) + { + /* + * We don't expect any of our own hashing routines to produce hashes + * that are too long. + */ + Assert(guessed_type != PASSWORD_TYPE_PLAINTEXT); + + ereport(ERROR, + (errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED), + errmsg("encrypted password is too long"), + errdetail("Encrypted passwords must be no longer than %d characters.", + MAX_ENCRYPTED_PASSWORD_LEN))); + } + + return encrypted_password; } /* diff --git a/src/include/libpq/crypt.h b/src/include/libpq/crypt.h index f744de4d20..a2c99cd16a 100644 --- a/src/include/libpq/crypt.h +++ b/src/include/libpq/crypt.h @@ -15,6 +15,16 @@ #include "datatype/timestamp.h" +/* + * Valid password hashes may be very long, but we don't want to store anything + * that might need out-of-line storage, since de-TOASTing won't work during + * authentication because we haven't selected a database yet and cannot read + * pg_class. 256 bytes should be more than enough for all practical use, and + * our own password encryption routines should never produce hashes longer than + * this. + */ +#define MAX_ENCRYPTED_PASSWORD_LEN (256) + /* * Types of password hashes or secrets. * diff --git a/src/test/regress/expected/password.out b/src/test/regress/expected/password.out index 924d6e001d..8ab995a162 100644 --- a/src/test/regress/expected/password.out +++ b/src/test/regress/expected/password.out @@ -127,6 +127,13 @@ SELECT rolname, rolpassword not like '%A6xHKoH/494E941doaPOYg==%' as is_rolpassw regress_passwd_sha_len2 | t (3 rows) +-- Test that valid hashes that are too long are rejected +CREATE ROLE regress_passwd10 PASSWORD 'SCRAM-SHA-256$00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000004096:NuDacwYSUxeOeFUEf3ivTQ==$Wgvq3OCYrJI6eUfvKlAzn4p/j3mzgWzXbVnWeFK1qhY=:r1qSP0j2QojCjLpFUjI0i6ckInvxJDKoyWnN3zF8WCM='; +ERROR: encrypted password is too long +DETAIL: Encrypted passwords must be no longer than 256 characters. +ALTER ROLE regress_passwd9 PASSWORD 'SCRAM-SHA-256$00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000004096:NuDacwYSUxeOeFUEf3ivTQ==$Wgvq3OCYrJI6eUfvKlAzn4p/j3mzgWzXbVnWeFK1qhY=:r1qSP0j2QojCjLpFUjI0i6ckInvxJDKoyWnN3zF8WCM='; +ERROR: encrypted password is too long +DETAIL: Encrypted passwords must be no longer than 256 characters. DROP ROLE regress_passwd1; DROP ROLE regress_passwd2; DROP ROLE regress_passwd3; diff --git a/src/test/regress/sql/password.sql b/src/test/regress/sql/password.sql index bb82aa4aa2..442c903c00 100644 --- a/src/test/regress/sql/password.sql +++ b/src/test/regress/sql/password.sql @@ -95,6 +95,10 @@ SELECT rolname, rolpassword not like '%A6xHKoH/494E941doaPOYg==%' as is_rolpassw WHERE rolname LIKE 'regress_passwd_sha_len%' ORDER BY rolname; +-- Test that valid hashes that are too long are rejected +CREATE ROLE regress_passwd10 PASSWORD 'SCRAM-SHA-256$00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000004096:NuDacwYSUxeOeFUEf3ivTQ==$Wgvq3OCYrJI6eUfvKlAzn4p/j3mzgWzXbVnWeFK1qhY=:r1qSP0j2QojCjLpFUjI0i6ckInvxJDKoyWnN3zF8WCM='; +ALTER ROLE regress_passwd9 PASSWORD 'SCRAM-SHA-256$00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000004096:NuDacwYSUxeOeFUEf3ivTQ==$Wgvq3OCYrJI6eUfvKlAzn4p/j3mzgWzXbVnWeFK1qhY=:r1qSP0j2QojCjLpFUjI0i6ckInvxJDKoyWnN3zF8WCM='; + DROP ROLE regress_passwd1; DROP ROLE regress_passwd2; DROP ROLE regress_passwd3; -- 2.39.5 (Apple Git-154)
v3-0002-remove-pg_authid-s-TOAST-table.patch (text/plain, 9.8K) download From 7e1736701e270d40cc1009ff4760317a8a366663 Mon Sep 17 00:00:00 2001 From: Nathan Bossart <nathan@postgresql.org> Date: Thu, 19 Sep 2024 21:09:30 -0500 Subject: [PATCH v3 2/2] remove pg_authid's TOAST table XXX: NEEDS CATVERSION BUMP --- src/backend/catalog/catalog.c | 4 +--- src/include/catalog/pg_authid.h | 2 -- src/test/regress/expected/create_index.out | 14 +++++++------- src/test/regress/expected/misc_sanity.out | 5 ++++- src/test/regress/expected/tablespace.out | 12 ++++++------ src/test/regress/sql/create_index.sql | 10 +++++----- src/test/regress/sql/misc_sanity.sql | 2 ++ src/test/regress/sql/tablespace.sql | 8 ++++---- 8 files changed, 29 insertions(+), 28 deletions(-) diff --git a/src/backend/catalog/catalog.c b/src/backend/catalog/catalog.c index 6c39434a30..d6b07a7865 100644 --- a/src/backend/catalog/catalog.c +++ b/src/backend/catalog/catalog.c @@ -300,9 +300,7 @@ IsSharedRelation(Oid relationId) relationId == TablespaceOidIndexId) return true; /* These are their toast tables and toast indexes */ - if (relationId == PgAuthidToastTable || - relationId == PgAuthidToastIndex || - relationId == PgDatabaseToastTable || + if (relationId == PgDatabaseToastTable || relationId == PgDatabaseToastIndex || relationId == PgDbRoleSettingToastTable || relationId == PgDbRoleSettingToastIndex || diff --git a/src/include/catalog/pg_authid.h b/src/include/catalog/pg_authid.h index e08863f78a..e846d75731 100644 --- a/src/include/catalog/pg_authid.h +++ b/src/include/catalog/pg_authid.h @@ -55,8 +55,6 @@ CATALOG(pg_authid,1260,AuthIdRelationId) BKI_SHARED_RELATION BKI_ROWTYPE_OID(284 */ typedef FormData_pg_authid *Form_pg_authid; -DECLARE_TOAST_WITH_MACRO(pg_authid, 4175, 4176, PgAuthidToastTable, PgAuthidToastIndex); - DECLARE_UNIQUE_INDEX(pg_authid_rolname_index, 2676, AuthIdRolnameIndexId, pg_authid, btree(rolname name_ops)); DECLARE_UNIQUE_INDEX_PKEY(pg_authid_oid_index, 2677, AuthIdOidIndexId, pg_authid, btree(oid oid_ops)); diff --git a/src/test/regress/expected/create_index.out b/src/test/regress/expected/create_index.out index cf6eac5734..d3358dfc39 100644 --- a/src/test/regress/expected/create_index.out +++ b/src/test/regress/expected/create_index.out @@ -2664,10 +2664,10 @@ REINDEX TABLE CONCURRENTLY pg_class; -- no catalog relation ERROR: cannot reindex system catalogs concurrently REINDEX INDEX CONCURRENTLY pg_class_oid_index; -- no catalog index ERROR: cannot reindex system catalogs concurrently --- These are the toast table and index of pg_authid. -REINDEX TABLE CONCURRENTLY pg_toast.pg_toast_1260; -- no catalog toast table +-- These are the toast table and index of pg_database. +REINDEX TABLE CONCURRENTLY pg_toast.pg_toast_1262; -- no catalog toast table ERROR: cannot reindex system catalogs concurrently -REINDEX INDEX CONCURRENTLY pg_toast.pg_toast_1260_index; -- no catalog toast index +REINDEX INDEX CONCURRENTLY pg_toast.pg_toast_1262_index; -- no catalog toast index ERROR: cannot reindex system catalogs concurrently REINDEX SYSTEM CONCURRENTLY postgres; -- not allowed for SYSTEM ERROR: cannot reindex system catalogs concurrently @@ -2974,10 +2974,10 @@ ERROR: must be owner of schema schema_to_reindex RESET ROLE; GRANT USAGE ON SCHEMA pg_toast TO regress_reindexuser; SET SESSION ROLE regress_reindexuser; -REINDEX TABLE pg_toast.pg_toast_1260; -ERROR: permission denied for table pg_toast_1260 -REINDEX INDEX pg_toast.pg_toast_1260_index; -ERROR: permission denied for index pg_toast_1260_index +REINDEX TABLE pg_toast.pg_toast_1262; +ERROR: permission denied for table pg_toast_1262 +REINDEX INDEX pg_toast.pg_toast_1262_index; +ERROR: permission denied for index pg_toast_1262_index -- Clean up RESET ROLE; REVOKE USAGE ON SCHEMA pg_toast FROM regress_reindexuser; diff --git a/src/test/regress/expected/misc_sanity.out b/src/test/regress/expected/misc_sanity.out index 45e7492877..b032a3f476 100644 --- a/src/test/regress/expected/misc_sanity.out +++ b/src/test/regress/expected/misc_sanity.out @@ -40,6 +40,8 @@ WHERE refclassid = 0 OR refobjid = 0 OR -- 2. pg_largeobject and pg_largeobject_metadata. Large object catalogs -- and toast tables are mutually exclusive and large object data is handled -- as user data by pg_upgrade, which would cause failures. +-- 3. pg_authid, since its toast table cannot be accessed when it would be +-- needed, i.e., during authentication before we've selected a database. SELECT relname, attname, atttypid::regtype FROM pg_class c JOIN pg_attribute a ON c.oid = attrelid WHERE c.oid < 16384 AND @@ -53,12 +55,13 @@ ORDER BY 1, 2; pg_attribute | attfdwoptions | text[] pg_attribute | attmissingval | anyarray pg_attribute | attoptions | text[] + pg_authid | rolpassword | text pg_class | relacl | aclitem[] pg_class | reloptions | text[] pg_class | relpartbound | pg_node_tree pg_largeobject | data | bytea pg_largeobject_metadata | lomacl | aclitem[] -(9 rows) +(10 rows) -- system catalogs without primary keys -- diff --git a/src/test/regress/expected/tablespace.out b/src/test/regress/expected/tablespace.out index 9aabb85349..dd535d41a3 100644 --- a/src/test/regress/expected/tablespace.out +++ b/src/test/regress/expected/tablespace.out @@ -51,13 +51,13 @@ ERROR: cannot move system relation "pg_authid_rolname_index" REINDEX (TABLESPACE regress_tblspace) TABLE CONCURRENTLY pg_authid; ERROR: cannot reindex system catalogs concurrently -- toast relations, fail -REINDEX (TABLESPACE regress_tblspace) INDEX pg_toast.pg_toast_1260_index; -ERROR: cannot move system relation "pg_toast_1260_index" -REINDEX (TABLESPACE regress_tblspace) INDEX CONCURRENTLY pg_toast.pg_toast_1260_index; +REINDEX (TABLESPACE regress_tblspace) INDEX pg_toast.pg_toast_1262_index; +ERROR: cannot move system relation "pg_toast_1262_index" +REINDEX (TABLESPACE regress_tblspace) INDEX CONCURRENTLY pg_toast.pg_toast_1262_index; ERROR: cannot reindex system catalogs concurrently -REINDEX (TABLESPACE regress_tblspace) TABLE pg_toast.pg_toast_1260; -ERROR: cannot move system relation "pg_toast_1260_index" -REINDEX (TABLESPACE regress_tblspace) TABLE CONCURRENTLY pg_toast.pg_toast_1260; +REINDEX (TABLESPACE regress_tblspace) TABLE pg_toast.pg_toast_1262; +ERROR: cannot move system relation "pg_toast_1262_index" +REINDEX (TABLESPACE regress_tblspace) TABLE CONCURRENTLY pg_toast.pg_toast_1262; ERROR: cannot reindex system catalogs concurrently -- system catalog, fail REINDEX (TABLESPACE pg_global) TABLE pg_authid; diff --git a/src/test/regress/sql/create_index.sql b/src/test/regress/sql/create_index.sql index e296891cab..fe162cc7c3 100644 --- a/src/test/regress/sql/create_index.sql +++ b/src/test/regress/sql/create_index.sql @@ -1126,9 +1126,9 @@ REINDEX TABLE CONCURRENTLY concur_reindex_tab; COMMIT; REINDEX TABLE CONCURRENTLY pg_class; -- no catalog relation REINDEX INDEX CONCURRENTLY pg_class_oid_index; -- no catalog index --- These are the toast table and index of pg_authid. -REINDEX TABLE CONCURRENTLY pg_toast.pg_toast_1260; -- no catalog toast table -REINDEX INDEX CONCURRENTLY pg_toast.pg_toast_1260_index; -- no catalog toast index +-- These are the toast table and index of pg_database. +REINDEX TABLE CONCURRENTLY pg_toast.pg_toast_1262; -- no catalog toast table +REINDEX INDEX CONCURRENTLY pg_toast.pg_toast_1262_index; -- no catalog toast index REINDEX SYSTEM CONCURRENTLY postgres; -- not allowed for SYSTEM REINDEX (CONCURRENTLY) SYSTEM postgres; -- ditto REINDEX (CONCURRENTLY) SYSTEM; -- ditto @@ -1305,8 +1305,8 @@ REINDEX SCHEMA schema_to_reindex; RESET ROLE; GRANT USAGE ON SCHEMA pg_toast TO regress_reindexuser; SET SESSION ROLE regress_reindexuser; -REINDEX TABLE pg_toast.pg_toast_1260; -REINDEX INDEX pg_toast.pg_toast_1260_index; +REINDEX TABLE pg_toast.pg_toast_1262; +REINDEX INDEX pg_toast.pg_toast_1262_index; -- Clean up RESET ROLE; diff --git a/src/test/regress/sql/misc_sanity.sql b/src/test/regress/sql/misc_sanity.sql index 16f3a7c0c1..135793871b 100644 --- a/src/test/regress/sql/misc_sanity.sql +++ b/src/test/regress/sql/misc_sanity.sql @@ -43,6 +43,8 @@ WHERE refclassid = 0 OR refobjid = 0 OR -- 2. pg_largeobject and pg_largeobject_metadata. Large object catalogs -- and toast tables are mutually exclusive and large object data is handled -- as user data by pg_upgrade, which would cause failures. +-- 3. pg_authid, since its toast table cannot be accessed when it would be +-- needed, i.e., during authentication before we've selected a database. SELECT relname, attname, atttypid::regtype FROM pg_class c JOIN pg_attribute a ON c.oid = attrelid diff --git a/src/test/regress/sql/tablespace.sql b/src/test/regress/sql/tablespace.sql index d274d9615e..c8b83788f0 100644 --- a/src/test/regress/sql/tablespace.sql +++ b/src/test/regress/sql/tablespace.sql @@ -40,10 +40,10 @@ REINDEX (TABLESPACE regress_tblspace) TABLE CONCURRENTLY pg_am; REINDEX (TABLESPACE regress_tblspace) TABLE pg_authid; REINDEX (TABLESPACE regress_tblspace) TABLE CONCURRENTLY pg_authid; -- toast relations, fail -REINDEX (TABLESPACE regress_tblspace) INDEX pg_toast.pg_toast_1260_index; -REINDEX (TABLESPACE regress_tblspace) INDEX CONCURRENTLY pg_toast.pg_toast_1260_index; -REINDEX (TABLESPACE regress_tblspace) TABLE pg_toast.pg_toast_1260; -REINDEX (TABLESPACE regress_tblspace) TABLE CONCURRENTLY pg_toast.pg_toast_1260; +REINDEX (TABLESPACE regress_tblspace) INDEX pg_toast.pg_toast_1262_index; +REINDEX (TABLESPACE regress_tblspace) INDEX CONCURRENTLY pg_toast.pg_toast_1262_index; +REINDEX (TABLESPACE regress_tblspace) TABLE pg_toast.pg_toast_1262; +REINDEX (TABLESPACE regress_tblspace) TABLE CONCURRENTLY pg_toast.pg_toast_1262; -- system catalog, fail REINDEX (TABLESPACE pg_global) TABLE pg_authid; REINDEX (TABLESPACE pg_global) TABLE CONCURRENTLY pg_authid; -- 2.39.5 (Apple Git-154)
^ permalink raw reply [nested|flat] 27+ messages in thread

* Re: Should rolpassword be toastable?
  2023-09-23 12:00 Should rolpassword be toastable? Alexander Lakhin <exclusion@gmail.com>
  2023-09-23 14:39 ` Re: Should rolpassword be toastable? Tom Lane <tgl@sss.pgh.pa.us>
  2023-09-23 18:00   ` Re: Should rolpassword be toastable? Alexander Lakhin <exclusion@gmail.com>
  2024-09-19 03:00     ` Re: Should rolpassword be toastable? Alexander Lakhin <exclusion@gmail.com>
  2024-09-19 14:22       ` Re: Should rolpassword be toastable? Nathan Bossart <nathandbossart@gmail.com>
  2024-09-19 14:31         ` Re: Should rolpassword be toastable? Tom Lane <tgl@sss.pgh.pa.us>
  2024-09-19 17:44           ` Re: Should rolpassword be toastable? Nathan Bossart <nathandbossart@gmail.com>
  2024-09-19 21:52             ` Re: Should rolpassword be toastable? Nathan Bossart <nathandbossart@gmail.com>
  2024-09-19 22:14               ` Re: Should rolpassword be toastable? Tom Lane <tgl@sss.pgh.pa.us>
  2024-09-19 23:37                 ` Re: Should rolpassword be toastable? Jonathan S. Katz <jkatz@postgresql.org>
  2024-09-20 02:46                   ` Re: Should rolpassword be toastable? Nathan Bossart <nathandbossart@gmail.com>
  2024-09-20 05:23                     ` Re: Should rolpassword be toastable? Michael Paquier <michael@paquier.xyz>
  2024-09-20 14:06                       ` Re: Should rolpassword be toastable? Jonathan S. Katz <jkatz@postgresql.org>
  2024-09-20 16:09                         ` Re: Should rolpassword be toastable? Nathan Bossart <nathandbossart@gmail.com>
@ 2024-09-20 16:27                           ` Tom Lane <tgl@sss.pgh.pa.us>
  2024-09-21 20:25                             ` Re: Should rolpassword be toastable? Nathan Bossart <nathandbossart@gmail.com>
  0 siblings, 1 reply; 27+ messages in thread

From: Tom Lane @ 2024-09-20 16:27 UTC (permalink / raw)
  To: Nathan Bossart <nathandbossart@gmail.com>; +Cc: Jonathan S. Katz <jkatz@postgresql.org>; Michael Paquier <michael@paquier.xyz>; Alexander Lakhin <exclusion@gmail.com>; pgsql-hackers

Nathan Bossart <nathandbossart@gmail.com> writes:
> Here is a v3 patch set that fixes the test comment and a compiler warning
> in cfbot.

Nitpick: the message should say "%d bytes" not "%d characters",
because we're counting bytes.  Passes an eyeball check otherwise.

			regards, tom lane





^ permalink  raw  reply  [nested|flat] 27+ messages in thread

* Re: Should rolpassword be toastable?
  2023-09-23 12:00 Should rolpassword be toastable? Alexander Lakhin <exclusion@gmail.com>
  2023-09-23 14:39 ` Re: Should rolpassword be toastable? Tom Lane <tgl@sss.pgh.pa.us>
  2023-09-23 18:00   ` Re: Should rolpassword be toastable? Alexander Lakhin <exclusion@gmail.com>
  2024-09-19 03:00     ` Re: Should rolpassword be toastable? Alexander Lakhin <exclusion@gmail.com>
  2024-09-19 14:22       ` Re: Should rolpassword be toastable? Nathan Bossart <nathandbossart@gmail.com>
  2024-09-19 14:31         ` Re: Should rolpassword be toastable? Tom Lane <tgl@sss.pgh.pa.us>
  2024-09-19 17:44           ` Re: Should rolpassword be toastable? Nathan Bossart <nathandbossart@gmail.com>
  2024-09-19 21:52             ` Re: Should rolpassword be toastable? Nathan Bossart <nathandbossart@gmail.com>
  2024-09-19 22:14               ` Re: Should rolpassword be toastable? Tom Lane <tgl@sss.pgh.pa.us>
  2024-09-19 23:37                 ` Re: Should rolpassword be toastable? Jonathan S. Katz <jkatz@postgresql.org>
  2024-09-20 02:46                   ` Re: Should rolpassword be toastable? Nathan Bossart <nathandbossart@gmail.com>
  2024-09-20 05:23                     ` Re: Should rolpassword be toastable? Michael Paquier <michael@paquier.xyz>
  2024-09-20 14:06                       ` Re: Should rolpassword be toastable? Jonathan S. Katz <jkatz@postgresql.org>
  2024-09-20 16:09                         ` Re: Should rolpassword be toastable? Nathan Bossart <nathandbossart@gmail.com>
  2024-09-20 16:27                           ` Re: Should rolpassword be toastable? Tom Lane <tgl@sss.pgh.pa.us>
@ 2024-09-21 20:25                             ` Nathan Bossart <nathandbossart@gmail.com>
  2024-10-03 21:27                               ` Re: Should rolpassword be toastable? Nathan Bossart <nathandbossart@gmail.com>
  0 siblings, 1 reply; 27+ messages in thread

From: Nathan Bossart @ 2024-09-21 20:25 UTC (permalink / raw)
  To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Jonathan S. Katz <jkatz@postgresql.org>; Michael Paquier <michael@paquier.xyz>; Alexander Lakhin <exclusion@gmail.com>; pgsql-hackers

On Fri, Sep 20, 2024 at 12:27:41PM -0400, Tom Lane wrote:
> Nitpick: the message should say "%d bytes" not "%d characters",
> because we're counting bytes.  Passes an eyeball check otherwise.

Thanks for reviewing.  I went ahead and committed 0002 since it seems like
there's consensus on that one.  I've attached a rebased version of 0001
with s/characters/bytes.

-- 
nathan

Attachments:

v4-0001-place-limit-on-password-hash-length.patch (text/plain, 7.0K) download From f039f02cc35d57862af64648d4152693e52fbee2 Mon Sep 17 00:00:00 2001 From: Nathan Bossart <nathan@postgresql.org> Date: Thu, 19 Sep 2024 20:59:10 -0500 Subject: [PATCH v4 1/1] place limit on password hash length --- src/backend/libpq/crypt.c | 60 ++++++++++++++++++-------- src/include/libpq/crypt.h | 10 +++++ src/test/regress/expected/password.out | 7 +++ src/test/regress/sql/password.sql | 4 ++ 4 files changed, 63 insertions(+), 18 deletions(-) diff --git a/src/backend/libpq/crypt.c b/src/backend/libpq/crypt.c index 629e51e00b..05d289977f 100644 --- a/src/backend/libpq/crypt.c +++ b/src/backend/libpq/crypt.c @@ -116,7 +116,7 @@ encrypt_password(PasswordType target_type, const char *role, const char *password) { PasswordType guessed_type = get_password_type(password); - char *encrypted_password; + char *encrypted_password = NULL; const char *errstr = NULL; if (guessed_type != PASSWORD_TYPE_PLAINTEXT) @@ -125,32 +125,56 @@ encrypt_password(PasswordType target_type, const char *role, * Cannot convert an already-encrypted password from one format to * another, so return it as it is. */ - return pstrdup(password); + encrypted_password = pstrdup(password); } - - switch (target_type) + else { - case PASSWORD_TYPE_MD5: - encrypted_password = palloc(MD5_PASSWD_LEN + 1); + switch (target_type) + { + case PASSWORD_TYPE_MD5: + encrypted_password = palloc(MD5_PASSWD_LEN + 1); - if (!pg_md5_encrypt(password, role, strlen(role), - encrypted_password, &errstr)) - elog(ERROR, "password encryption failed: %s", errstr); - return encrypted_password; + if (!pg_md5_encrypt(password, role, strlen(role), + encrypted_password, &errstr)) + elog(ERROR, "password encryption failed: %s", errstr); + break; - case PASSWORD_TYPE_SCRAM_SHA_256: - return pg_be_scram_build_secret(password); + case PASSWORD_TYPE_SCRAM_SHA_256: + encrypted_password = pg_be_scram_build_secret(password); + break; - case PASSWORD_TYPE_PLAINTEXT: - elog(ERROR, "cannot encrypt password with 'plaintext'"); + case PASSWORD_TYPE_PLAINTEXT: + elog(ERROR, "cannot encrypt password with 'plaintext'"); + break; + } } + Assert(encrypted_password); + /* - * This shouldn't happen, because the above switch statements should - * handle every combination of source and target password types. + * Valid password hashes may be very long, but we don't want to store + * anything that might need out-of-line storage, since de-TOASTing won't + * work during authentication because we haven't selected a database yet + * and cannot read pg_class. 256 bytes should be more than enough for all + * practical use, so fail for anything longer. */ - elog(ERROR, "cannot encrypt password to requested type"); - return NULL; /* keep compiler quiet */ + if (encrypted_password && /* keep compiler quiet */ + strlen(encrypted_password) > MAX_ENCRYPTED_PASSWORD_LEN) + { + /* + * We don't expect any of our own hashing routines to produce hashes + * that are too long. + */ + Assert(guessed_type != PASSWORD_TYPE_PLAINTEXT); + + ereport(ERROR, + (errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED), + errmsg("encrypted password is too long"), + errdetail("Encrypted passwords must be no longer than %d bytes.", + MAX_ENCRYPTED_PASSWORD_LEN))); + } + + return encrypted_password; } /* diff --git a/src/include/libpq/crypt.h b/src/include/libpq/crypt.h index f744de4d20..a2c99cd16a 100644 --- a/src/include/libpq/crypt.h +++ b/src/include/libpq/crypt.h @@ -15,6 +15,16 @@ #include "datatype/timestamp.h" +/* + * Valid password hashes may be very long, but we don't want to store anything + * that might need out-of-line storage, since de-TOASTing won't work during + * authentication because we haven't selected a database yet and cannot read + * pg_class. 256 bytes should be more than enough for all practical use, and + * our own password encryption routines should never produce hashes longer than + * this. + */ +#define MAX_ENCRYPTED_PASSWORD_LEN (256) + /* * Types of password hashes or secrets. * diff --git a/src/test/regress/expected/password.out b/src/test/regress/expected/password.out index 924d6e001d..dbfe92a27d 100644 --- a/src/test/regress/expected/password.out +++ b/src/test/regress/expected/password.out @@ -127,6 +127,13 @@ SELECT rolname, rolpassword not like '%A6xHKoH/494E941doaPOYg==%' as is_rolpassw regress_passwd_sha_len2 | t (3 rows) +-- Test that valid hashes that are too long are rejected +CREATE ROLE regress_passwd10 PASSWORD 'SCRAM-SHA-256$00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000004096:NuDacwYSUxeOeFUEf3ivTQ==$Wgvq3OCYrJI6eUfvKlAzn4p/j3mzgWzXbVnWeFK1qhY=:r1qSP0j2QojCjLpFUjI0i6ckInvxJDKoyWnN3zF8WCM='; +ERROR: encrypted password is too long +DETAIL: Encrypted passwords must be no longer than 256 bytes. +ALTER ROLE regress_passwd9 PASSWORD 'SCRAM-SHA-256$00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000004096:NuDacwYSUxeOeFUEf3ivTQ==$Wgvq3OCYrJI6eUfvKlAzn4p/j3mzgWzXbVnWeFK1qhY=:r1qSP0j2QojCjLpFUjI0i6ckInvxJDKoyWnN3zF8WCM='; +ERROR: encrypted password is too long +DETAIL: Encrypted passwords must be no longer than 256 bytes. DROP ROLE regress_passwd1; DROP ROLE regress_passwd2; DROP ROLE regress_passwd3; diff --git a/src/test/regress/sql/password.sql b/src/test/regress/sql/password.sql index bb82aa4aa2..442c903c00 100644 --- a/src/test/regress/sql/password.sql +++ b/src/test/regress/sql/password.sql @@ -95,6 +95,10 @@ SELECT rolname, rolpassword not like '%A6xHKoH/494E941doaPOYg==%' as is_rolpassw WHERE rolname LIKE 'regress_passwd_sha_len%' ORDER BY rolname; +-- Test that valid hashes that are too long are rejected +CREATE ROLE regress_passwd10 PASSWORD 'SCRAM-SHA-256$00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000004096:NuDacwYSUxeOeFUEf3ivTQ==$Wgvq3OCYrJI6eUfvKlAzn4p/j3mzgWzXbVnWeFK1qhY=:r1qSP0j2QojCjLpFUjI0i6ckInvxJDKoyWnN3zF8WCM='; +ALTER ROLE regress_passwd9 PASSWORD 'SCRAM-SHA-256$00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000004096:NuDacwYSUxeOeFUEf3ivTQ==$Wgvq3OCYrJI6eUfvKlAzn4p/j3mzgWzXbVnWeFK1qhY=:r1qSP0j2QojCjLpFUjI0i6ckInvxJDKoyWnN3zF8WCM='; + DROP ROLE regress_passwd1; DROP ROLE regress_passwd2; DROP ROLE regress_passwd3; -- 2.39.5 (Apple Git-154)
^ permalink raw reply [nested|flat] 27+ messages in thread

* Re: Should rolpassword be toastable?
  2023-09-23 12:00 Should rolpassword be toastable? Alexander Lakhin <exclusion@gmail.com>
  2023-09-23 14:39 ` Re: Should rolpassword be toastable? Tom Lane <tgl@sss.pgh.pa.us>
  2023-09-23 18:00   ` Re: Should rolpassword be toastable? Alexander Lakhin <exclusion@gmail.com>
  2024-09-19 03:00     ` Re: Should rolpassword be toastable? Alexander Lakhin <exclusion@gmail.com>
  2024-09-19 14:22       ` Re: Should rolpassword be toastable? Nathan Bossart <nathandbossart@gmail.com>
  2024-09-19 14:31         ` Re: Should rolpassword be toastable? Tom Lane <tgl@sss.pgh.pa.us>
  2024-09-19 17:44           ` Re: Should rolpassword be toastable? Nathan Bossart <nathandbossart@gmail.com>
  2024-09-19 21:52             ` Re: Should rolpassword be toastable? Nathan Bossart <nathandbossart@gmail.com>
  2024-09-19 22:14               ` Re: Should rolpassword be toastable? Tom Lane <tgl@sss.pgh.pa.us>
  2024-09-19 23:37                 ` Re: Should rolpassword be toastable? Jonathan S. Katz <jkatz@postgresql.org>
  2024-09-20 02:46                   ` Re: Should rolpassword be toastable? Nathan Bossart <nathandbossart@gmail.com>
  2024-09-20 05:23                     ` Re: Should rolpassword be toastable? Michael Paquier <michael@paquier.xyz>
  2024-09-20 14:06                       ` Re: Should rolpassword be toastable? Jonathan S. Katz <jkatz@postgresql.org>
  2024-09-20 16:09                         ` Re: Should rolpassword be toastable? Nathan Bossart <nathandbossart@gmail.com>
  2024-09-20 16:27                           ` Re: Should rolpassword be toastable? Tom Lane <tgl@sss.pgh.pa.us>
  2024-09-21 20:25                             ` Re: Should rolpassword be toastable? Nathan Bossart <nathandbossart@gmail.com>
@ 2024-10-03 21:27                               ` Nathan Bossart <nathandbossart@gmail.com>
  2024-10-03 21:39                                 ` Re: Should rolpassword be toastable? Tom Lane <tgl@sss.pgh.pa.us>
  0 siblings, 1 reply; 27+ messages in thread

From: Nathan Bossart @ 2024-10-03 21:27 UTC (permalink / raw)
  To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Jonathan S. Katz <jkatz@postgresql.org>; Michael Paquier <michael@paquier.xyz>; Alexander Lakhin <exclusion@gmail.com>; pgsql-hackers

On Sat, Sep 21, 2024 at 03:25:54PM -0500, Nathan Bossart wrote:
> Thanks for reviewing.  I went ahead and committed 0002 since it seems like
> there's consensus on that one.  I've attached a rebased version of 0001
> with s/characters/bytes.

For the reasons discussed upthread [0], I can't bring myself to add an
arbitrary limit to the password hash length.  I am going to leave 0001
uncommitted for now.

[0] https://postgr.es/m/Zu2eT2H8OT3OXauc%40nathan

-- 
nathan





^ permalink  raw  reply  [nested|flat] 27+ messages in thread

* Re: Should rolpassword be toastable?
  2023-09-23 12:00 Should rolpassword be toastable? Alexander Lakhin <exclusion@gmail.com>
  2023-09-23 14:39 ` Re: Should rolpassword be toastable? Tom Lane <tgl@sss.pgh.pa.us>
  2023-09-23 18:00   ` Re: Should rolpassword be toastable? Alexander Lakhin <exclusion@gmail.com>
  2024-09-19 03:00     ` Re: Should rolpassword be toastable? Alexander Lakhin <exclusion@gmail.com>
  2024-09-19 14:22       ` Re: Should rolpassword be toastable? Nathan Bossart <nathandbossart@gmail.com>
  2024-09-19 14:31         ` Re: Should rolpassword be toastable? Tom Lane <tgl@sss.pgh.pa.us>
  2024-09-19 17:44           ` Re: Should rolpassword be toastable? Nathan Bossart <nathandbossart@gmail.com>
  2024-09-19 21:52             ` Re: Should rolpassword be toastable? Nathan Bossart <nathandbossart@gmail.com>
  2024-09-19 22:14               ` Re: Should rolpassword be toastable? Tom Lane <tgl@sss.pgh.pa.us>
  2024-09-19 23:37                 ` Re: Should rolpassword be toastable? Jonathan S. Katz <jkatz@postgresql.org>
  2024-09-20 02:46                   ` Re: Should rolpassword be toastable? Nathan Bossart <nathandbossart@gmail.com>
  2024-09-20 05:23                     ` Re: Should rolpassword be toastable? Michael Paquier <michael@paquier.xyz>
  2024-09-20 14:06                       ` Re: Should rolpassword be toastable? Jonathan S. Katz <jkatz@postgresql.org>
  2024-09-20 16:09                         ` Re: Should rolpassword be toastable? Nathan Bossart <nathandbossart@gmail.com>
  2024-09-20 16:27                           ` Re: Should rolpassword be toastable? Tom Lane <tgl@sss.pgh.pa.us>
  2024-09-21 20:25                             ` Re: Should rolpassword be toastable? Nathan Bossart <nathandbossart@gmail.com>
  2024-10-03 21:27                               ` Re: Should rolpassword be toastable? Nathan Bossart <nathandbossart@gmail.com>
@ 2024-10-03 21:39                                 ` Tom Lane <tgl@sss.pgh.pa.us>
  2024-10-03 22:17                                   ` Re: Should rolpassword be toastable? Nathan Bossart <nathandbossart@gmail.com>
  0 siblings, 1 reply; 27+ messages in thread

From: Tom Lane @ 2024-10-03 21:39 UTC (permalink / raw)
  To: Nathan Bossart <nathandbossart@gmail.com>; +Cc: Jonathan S. Katz <jkatz@postgresql.org>; Michael Paquier <michael@paquier.xyz>; Alexander Lakhin <exclusion@gmail.com>; pgsql-hackers

Nathan Bossart <nathandbossart@gmail.com> writes:
> For the reasons discussed upthread [0], I can't bring myself to add an
> arbitrary limit to the password hash length.  I am going to leave 0001
> uncommitted for now.
> [0] https://postgr.es/m/Zu2eT2H8OT3OXauc%40nathan

I'm confused, as in [0] you said

>> ...  But on the off-chance
>> that someone is building a custom driver that generates long hashes for
>> whatever reason, I'd imagine that a clear error would be more helpful than
>> "row is too big."

I agree with the idea that complaining about the password being too
long is far more intelligible than that.  Another problem with
leaving it as it stands in HEAD is that the effective limit is now
platform-specific, if not indeed dependent on the phase of the moon
(or at least, the other contents of the pg_authid row).  I fear it
would be very easy to construct cases where a password is accepted
on one machine but fails with "row is too big" on another.  A
uniform limit seems much less fraught with surprises.

			regards, tom lane





^ permalink  raw  reply  [nested|flat] 27+ messages in thread

* Re: Should rolpassword be toastable?
  2023-09-23 12:00 Should rolpassword be toastable? Alexander Lakhin <exclusion@gmail.com>
  2023-09-23 14:39 ` Re: Should rolpassword be toastable? Tom Lane <tgl@sss.pgh.pa.us>
  2023-09-23 18:00   ` Re: Should rolpassword be toastable? Alexander Lakhin <exclusion@gmail.com>
  2024-09-19 03:00     ` Re: Should rolpassword be toastable? Alexander Lakhin <exclusion@gmail.com>
  2024-09-19 14:22       ` Re: Should rolpassword be toastable? Nathan Bossart <nathandbossart@gmail.com>
  2024-09-19 14:31         ` Re: Should rolpassword be toastable? Tom Lane <tgl@sss.pgh.pa.us>
  2024-09-19 17:44           ` Re: Should rolpassword be toastable? Nathan Bossart <nathandbossart@gmail.com>
  2024-09-19 21:52             ` Re: Should rolpassword be toastable? Nathan Bossart <nathandbossart@gmail.com>
  2024-09-19 22:14               ` Re: Should rolpassword be toastable? Tom Lane <tgl@sss.pgh.pa.us>
  2024-09-19 23:37                 ` Re: Should rolpassword be toastable? Jonathan S. Katz <jkatz@postgresql.org>
  2024-09-20 02:46                   ` Re: Should rolpassword be toastable? Nathan Bossart <nathandbossart@gmail.com>
  2024-09-20 05:23                     ` Re: Should rolpassword be toastable? Michael Paquier <michael@paquier.xyz>
  2024-09-20 14:06                       ` Re: Should rolpassword be toastable? Jonathan S. Katz <jkatz@postgresql.org>
  2024-09-20 16:09                         ` Re: Should rolpassword be toastable? Nathan Bossart <nathandbossart@gmail.com>
  2024-09-20 16:27                           ` Re: Should rolpassword be toastable? Tom Lane <tgl@sss.pgh.pa.us>
  2024-09-21 20:25                             ` Re: Should rolpassword be toastable? Nathan Bossart <nathandbossart@gmail.com>
  2024-10-03 21:27                               ` Re: Should rolpassword be toastable? Nathan Bossart <nathandbossart@gmail.com>
  2024-10-03 21:39                                 ` Re: Should rolpassword be toastable? Tom Lane <tgl@sss.pgh.pa.us>
@ 2024-10-03 22:17                                   ` Nathan Bossart <nathandbossart@gmail.com>
  2024-10-03 22:24                                     ` Re: Should rolpassword be toastable? Tom Lane <tgl@sss.pgh.pa.us>
  0 siblings, 1 reply; 27+ messages in thread

From: Nathan Bossart @ 2024-10-03 22:17 UTC (permalink / raw)
  To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Jonathan S. Katz <jkatz@postgresql.org>; Michael Paquier <michael@paquier.xyz>; Alexander Lakhin <exclusion@gmail.com>; pgsql-hackers

On Thu, Oct 03, 2024 at 05:39:06PM -0400, Tom Lane wrote:
> I agree with the idea that complaining about the password being too
> long is far more intelligible than that.  Another problem with
> leaving it as it stands in HEAD is that the effective limit is now
> platform-specific, if not indeed dependent on the phase of the moon
> (or at least, the other contents of the pg_authid row).  I fear it
> would be very easy to construct cases where a password is accepted
> on one machine but fails with "row is too big" on another.  A
> uniform limit seems much less fraught with surprises.

I don't mind proceeding with the patch if there is strong support for it.
I wavered only because it's hard to be confident that we are choosing the
right limit.  AFAICT 256 bytes ought to be sufficient to avoid "row is too
big" errors independent of BLCKSZ today, but maybe someone will add another
varlena column in the future that breaks it.  Or maybe we add a new
password hashing method that produces longer strings.  Or maybe someone is
doing something really out there like storing additional information in the
salt.  I don't have any reason to believe that any of these things are
happening or are likely to happen anytime soon, but they seem similar in
likelihood to someone building a custom driver that generates ginormous
hashes.  But I can also buy the argument that none of this is a strong
enough reason to avoid making the error message nicer...

-- 
nathan





^ permalink  raw  reply  [nested|flat] 27+ messages in thread

* Re: Should rolpassword be toastable?
  2023-09-23 12:00 Should rolpassword be toastable? Alexander Lakhin <exclusion@gmail.com>
  2023-09-23 14:39 ` Re: Should rolpassword be toastable? Tom Lane <tgl@sss.pgh.pa.us>
  2023-09-23 18:00   ` Re: Should rolpassword be toastable? Alexander Lakhin <exclusion@gmail.com>
  2024-09-19 03:00     ` Re: Should rolpassword be toastable? Alexander Lakhin <exclusion@gmail.com>
  2024-09-19 14:22       ` Re: Should rolpassword be toastable? Nathan Bossart <nathandbossart@gmail.com>
  2024-09-19 14:31         ` Re: Should rolpassword be toastable? Tom Lane <tgl@sss.pgh.pa.us>
  2024-09-19 17:44           ` Re: Should rolpassword be toastable? Nathan Bossart <nathandbossart@gmail.com>
  2024-09-19 21:52             ` Re: Should rolpassword be toastable? Nathan Bossart <nathandbossart@gmail.com>
  2024-09-19 22:14               ` Re: Should rolpassword be toastable? Tom Lane <tgl@sss.pgh.pa.us>
  2024-09-19 23:37                 ` Re: Should rolpassword be toastable? Jonathan S. Katz <jkatz@postgresql.org>
  2024-09-20 02:46                   ` Re: Should rolpassword be toastable? Nathan Bossart <nathandbossart@gmail.com>
  2024-09-20 05:23                     ` Re: Should rolpassword be toastable? Michael Paquier <michael@paquier.xyz>
  2024-09-20 14:06                       ` Re: Should rolpassword be toastable? Jonathan S. Katz <jkatz@postgresql.org>
  2024-09-20 16:09                         ` Re: Should rolpassword be toastable? Nathan Bossart <nathandbossart@gmail.com>
  2024-09-20 16:27                           ` Re: Should rolpassword be toastable? Tom Lane <tgl@sss.pgh.pa.us>
  2024-09-21 20:25                             ` Re: Should rolpassword be toastable? Nathan Bossart <nathandbossart@gmail.com>
  2024-10-03 21:27                               ` Re: Should rolpassword be toastable? Nathan Bossart <nathandbossart@gmail.com>
  2024-10-03 21:39                                 ` Re: Should rolpassword be toastable? Tom Lane <tgl@sss.pgh.pa.us>
  2024-10-03 22:17                                   ` Re: Should rolpassword be toastable? Nathan Bossart <nathandbossart@gmail.com>
@ 2024-10-03 22:24                                     ` Tom Lane <tgl@sss.pgh.pa.us>
  2024-10-03 23:29                                       ` Re: Should rolpassword be toastable? Jacob Champion <jacob.champion@enterprisedb.com>
  0 siblings, 1 reply; 27+ messages in thread

From: Tom Lane @ 2024-10-03 22:24 UTC (permalink / raw)
  To: Nathan Bossart <nathandbossart@gmail.com>; +Cc: Jonathan S. Katz <jkatz@postgresql.org>; Michael Paquier <michael@paquier.xyz>; Alexander Lakhin <exclusion@gmail.com>; pgsql-hackers

Nathan Bossart <nathandbossart@gmail.com> writes:
> I don't mind proceeding with the patch if there is strong support for it.
> I wavered only because it's hard to be confident that we are choosing the
> right limit.

I'm not that fussed about it; surely 256 is more than anyone is using?
If not, we'll get push-back and then we can have a discussion about the
correct limit that's informed by more than guesswork.

> ... But I can also buy the argument that none of this is a strong
> enough reason to avoid making the error message nicer...

There's that, and there's also the fact that if you assume someone is
using $sufficiently-long-passwords then we might have broken their
use-case already.  We can't have much of a conversation here without
a concrete case to look at.

			regards, tom lane





^ permalink  raw  reply  [nested|flat] 27+ messages in thread

* Re: Should rolpassword be toastable?
  2023-09-23 12:00 Should rolpassword be toastable? Alexander Lakhin <exclusion@gmail.com>
  2023-09-23 14:39 ` Re: Should rolpassword be toastable? Tom Lane <tgl@sss.pgh.pa.us>
  2023-09-23 18:00   ` Re: Should rolpassword be toastable? Alexander Lakhin <exclusion@gmail.com>
  2024-09-19 03:00     ` Re: Should rolpassword be toastable? Alexander Lakhin <exclusion@gmail.com>
  2024-09-19 14:22       ` Re: Should rolpassword be toastable? Nathan Bossart <nathandbossart@gmail.com>
  2024-09-19 14:31         ` Re: Should rolpassword be toastable? Tom Lane <tgl@sss.pgh.pa.us>
  2024-09-19 17:44           ` Re: Should rolpassword be toastable? Nathan Bossart <nathandbossart@gmail.com>
  2024-09-19 21:52             ` Re: Should rolpassword be toastable? Nathan Bossart <nathandbossart@gmail.com>
  2024-09-19 22:14               ` Re: Should rolpassword be toastable? Tom Lane <tgl@sss.pgh.pa.us>
  2024-09-19 23:37                 ` Re: Should rolpassword be toastable? Jonathan S. Katz <jkatz@postgresql.org>
  2024-09-20 02:46                   ` Re: Should rolpassword be toastable? Nathan Bossart <nathandbossart@gmail.com>
  2024-09-20 05:23                     ` Re: Should rolpassword be toastable? Michael Paquier <michael@paquier.xyz>
  2024-09-20 14:06                       ` Re: Should rolpassword be toastable? Jonathan S. Katz <jkatz@postgresql.org>
  2024-09-20 16:09                         ` Re: Should rolpassword be toastable? Nathan Bossart <nathandbossart@gmail.com>
  2024-09-20 16:27                           ` Re: Should rolpassword be toastable? Tom Lane <tgl@sss.pgh.pa.us>
  2024-09-21 20:25                             ` Re: Should rolpassword be toastable? Nathan Bossart <nathandbossart@gmail.com>
  2024-10-03 21:27                               ` Re: Should rolpassword be toastable? Nathan Bossart <nathandbossart@gmail.com>
  2024-10-03 21:39                                 ` Re: Should rolpassword be toastable? Tom Lane <tgl@sss.pgh.pa.us>
  2024-10-03 22:17                                   ` Re: Should rolpassword be toastable? Nathan Bossart <nathandbossart@gmail.com>
  2024-10-03 22:24                                     ` Re: Should rolpassword be toastable? Tom Lane <tgl@sss.pgh.pa.us>
@ 2024-10-03 23:29                                       ` Jacob Champion <jacob.champion@enterprisedb.com>
  2024-10-04 02:17                                         ` Re: Should rolpassword be toastable? Jonathan S. Katz <jkatz@postgresql.org>
  0 siblings, 1 reply; 27+ messages in thread

From: Jacob Champion @ 2024-10-03 23:29 UTC (permalink / raw)
  To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Nathan Bossart <nathandbossart@gmail.com>; Jonathan S. Katz <jkatz@postgresql.org>; Michael Paquier <michael@paquier.xyz>; Alexander Lakhin <exclusion@gmail.com>; pgsql-hackers

On Thu, Oct 3, 2024 at 3:25 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> Nathan Bossart <nathandbossart@gmail.com> writes:
> > I don't mind proceeding with the patch if there is strong support for it.
> > I wavered only because it's hard to be confident that we are choosing the
> > right limit.
>
> I'm not that fussed about it; surely 256 is more than anyone is using?
> If not, we'll get push-back and then we can have a discussion about the
> correct limit that's informed by more than guesswork.

+1.

Next up is probably SCRAM-SHA-512, which should still have smaller
entries than that -- 222 bytes, I think, with 128-bit salts and a
5-digit iteration count?

--Jacob





^ permalink  raw  reply  [nested|flat] 27+ messages in thread

* Re: Should rolpassword be toastable?
  2023-09-23 12:00 Should rolpassword be toastable? Alexander Lakhin <exclusion@gmail.com>
  2023-09-23 14:39 ` Re: Should rolpassword be toastable? Tom Lane <tgl@sss.pgh.pa.us>
  2023-09-23 18:00   ` Re: Should rolpassword be toastable? Alexander Lakhin <exclusion@gmail.com>
  2024-09-19 03:00     ` Re: Should rolpassword be toastable? Alexander Lakhin <exclusion@gmail.com>
  2024-09-19 14:22       ` Re: Should rolpassword be toastable? Nathan Bossart <nathandbossart@gmail.com>
  2024-09-19 14:31         ` Re: Should rolpassword be toastable? Tom Lane <tgl@sss.pgh.pa.us>
  2024-09-19 17:44           ` Re: Should rolpassword be toastable? Nathan Bossart <nathandbossart@gmail.com>
  2024-09-19 21:52             ` Re: Should rolpassword be toastable? Nathan Bossart <nathandbossart@gmail.com>
  2024-09-19 22:14               ` Re: Should rolpassword be toastable? Tom Lane <tgl@sss.pgh.pa.us>
  2024-09-19 23:37                 ` Re: Should rolpassword be toastable? Jonathan S. Katz <jkatz@postgresql.org>
  2024-09-20 02:46                   ` Re: Should rolpassword be toastable? Nathan Bossart <nathandbossart@gmail.com>
  2024-09-20 05:23                     ` Re: Should rolpassword be toastable? Michael Paquier <michael@paquier.xyz>
  2024-09-20 14:06                       ` Re: Should rolpassword be toastable? Jonathan S. Katz <jkatz@postgresql.org>
  2024-09-20 16:09                         ` Re: Should rolpassword be toastable? Nathan Bossart <nathandbossart@gmail.com>
  2024-09-20 16:27                           ` Re: Should rolpassword be toastable? Tom Lane <tgl@sss.pgh.pa.us>
  2024-09-21 20:25                             ` Re: Should rolpassword be toastable? Nathan Bossart <nathandbossart@gmail.com>
  2024-10-03 21:27                               ` Re: Should rolpassword be toastable? Nathan Bossart <nathandbossart@gmail.com>
  2024-10-03 21:39                                 ` Re: Should rolpassword be toastable? Tom Lane <tgl@sss.pgh.pa.us>
  2024-10-03 22:17                                   ` Re: Should rolpassword be toastable? Nathan Bossart <nathandbossart@gmail.com>
  2024-10-03 22:24                                     ` Re: Should rolpassword be toastable? Tom Lane <tgl@sss.pgh.pa.us>
  2024-10-03 23:29                                       ` Re: Should rolpassword be toastable? Jacob Champion <jacob.champion@enterprisedb.com>
@ 2024-10-04 02:17                                         ` Jonathan S. Katz <jkatz@postgresql.org>
  2024-10-04 02:33                                           ` Re: Should rolpassword be toastable? Tom Lane <tgl@sss.pgh.pa.us>
  0 siblings, 1 reply; 27+ messages in thread

From: Jonathan S. Katz @ 2024-10-04 02:17 UTC (permalink / raw)
  To: Jacob Champion <jacob.champion@enterprisedb.com>; Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Nathan Bossart <nathandbossart@gmail.com>; Michael Paquier <michael@paquier.xyz>; Alexander Lakhin <exclusion@gmail.com>; pgsql-hackers

On 10/3/24 7:29 PM, Jacob Champion wrote:
> On Thu, Oct 3, 2024 at 3:25 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>>
>> Nathan Bossart <nathandbossart@gmail.com> writes:
>>> I don't mind proceeding with the patch if there is strong support for it.
>>> I wavered only because it's hard to be confident that we are choosing the
>>> right limit.
>>
>> I'm not that fussed about it; surely 256 is more than anyone is using?
>> If not, we'll get push-back and then we can have a discussion about the
>> correct limit that's informed by more than guesswork.
> 
> +1.
> 
> Next up is probably SCRAM-SHA-512, which should still have smaller
> entries than that -- 222 bytes, I think, with 128-bit salts and a
> 5-digit iteration count?

The challenge is that salts can be an arbitrary length, even today (as 
can the iterator value, though IIRC I think we check if it's in int 
bounds, and a large iterator becomes pretty impractical for usage).

Probabalistically, it's unlikely there are many very large salts in the 
wild (though I don't have data on that) and most folks are using the 
default length, but that probability isn't 0.

I think Tom's initial suggestion (BLCKSZ/2) is better than 256, given we 
really don't know what' out there in the wild, and this could end up 
being a breaking change. Every other type in pg_authid is pretty small. 
That said, I'm also imagining other things we may add that could require 
TOAST support (remembering previous passwords? storing multiple 
passwords options)?

Thanks,

Jonathan


Attachments:

  OpenPGP_signature.asc (application/pgp-signature, 839B) download

^ permalink  raw  reply  [nested|flat] 27+ messages in thread

* Re: Should rolpassword be toastable?
  2023-09-23 12:00 Should rolpassword be toastable? Alexander Lakhin <exclusion@gmail.com>
  2023-09-23 14:39 ` Re: Should rolpassword be toastable? Tom Lane <tgl@sss.pgh.pa.us>
  2023-09-23 18:00   ` Re: Should rolpassword be toastable? Alexander Lakhin <exclusion@gmail.com>
  2024-09-19 03:00     ` Re: Should rolpassword be toastable? Alexander Lakhin <exclusion@gmail.com>
  2024-09-19 14:22       ` Re: Should rolpassword be toastable? Nathan Bossart <nathandbossart@gmail.com>
  2024-09-19 14:31         ` Re: Should rolpassword be toastable? Tom Lane <tgl@sss.pgh.pa.us>
  2024-09-19 17:44           ` Re: Should rolpassword be toastable? Nathan Bossart <nathandbossart@gmail.com>
  2024-09-19 21:52             ` Re: Should rolpassword be toastable? Nathan Bossart <nathandbossart@gmail.com>
  2024-09-19 22:14               ` Re: Should rolpassword be toastable? Tom Lane <tgl@sss.pgh.pa.us>
  2024-09-19 23:37                 ` Re: Should rolpassword be toastable? Jonathan S. Katz <jkatz@postgresql.org>
  2024-09-20 02:46                   ` Re: Should rolpassword be toastable? Nathan Bossart <nathandbossart@gmail.com>
  2024-09-20 05:23                     ` Re: Should rolpassword be toastable? Michael Paquier <michael@paquier.xyz>
  2024-09-20 14:06                       ` Re: Should rolpassword be toastable? Jonathan S. Katz <jkatz@postgresql.org>
  2024-09-20 16:09                         ` Re: Should rolpassword be toastable? Nathan Bossart <nathandbossart@gmail.com>
  2024-09-20 16:27                           ` Re: Should rolpassword be toastable? Tom Lane <tgl@sss.pgh.pa.us>
  2024-09-21 20:25                             ` Re: Should rolpassword be toastable? Nathan Bossart <nathandbossart@gmail.com>
  2024-10-03 21:27                               ` Re: Should rolpassword be toastable? Nathan Bossart <nathandbossart@gmail.com>
  2024-10-03 21:39                                 ` Re: Should rolpassword be toastable? Tom Lane <tgl@sss.pgh.pa.us>
  2024-10-03 22:17                                   ` Re: Should rolpassword be toastable? Nathan Bossart <nathandbossart@gmail.com>
  2024-10-03 22:24                                     ` Re: Should rolpassword be toastable? Tom Lane <tgl@sss.pgh.pa.us>
  2024-10-03 23:29                                       ` Re: Should rolpassword be toastable? Jacob Champion <jacob.champion@enterprisedb.com>
  2024-10-04 02:17                                         ` Re: Should rolpassword be toastable? Jonathan S. Katz <jkatz@postgresql.org>
@ 2024-10-04 02:33                                           ` Tom Lane <tgl@sss.pgh.pa.us>
  2024-10-04 14:47                                             ` Re: Should rolpassword be toastable? Nathan Bossart <nathandbossart@gmail.com>
  0 siblings, 1 reply; 27+ messages in thread

From: Tom Lane @ 2024-10-04 02:33 UTC (permalink / raw)
  To: Jonathan S. Katz <jkatz@postgresql.org>; +Cc: Jacob Champion <jacob.champion@enterprisedb.com>; Nathan Bossart <nathandbossart@gmail.com>; Michael Paquier <michael@paquier.xyz>; Alexander Lakhin <exclusion@gmail.com>; pgsql-hackers

"Jonathan S. Katz" <jkatz@postgresql.org> writes:
> I think Tom's initial suggestion (BLCKSZ/2) is better than 256, given we 
> really don't know what' out there in the wild, and this could end up 
> being a breaking change. Every other type in pg_authid is pretty small. 

I'm having second thoughts about that though, based on the argument
that we don't really want a platform-dependent limit here.
Admittedly, nobody changes BLCKSZ on production systems, but it's
still theoretically an issue.  I don't have a problem with selecting
a larger limit such as 512 or 1024 though.

> That said, I'm also imagining other things we may add that could require 
> TOAST support (remembering previous passwords? storing multiple 
> passwords options)?

Things like previous passwords probably don't need to be accessed
during authentication, so there are at least a couple of ways we
could do that:
* put the previous passwords in an auxiliary table;
* put back pg_authid's toast table, but mark rolpassword as
"STORAGE MAIN" so it doesn't go to toast, while letting columns
that don't need to be touched at startup go there.

However, if you wanted to allow multiple passwords I'm not
sure about a good way.

			regards, tom lane





^ permalink  raw  reply  [nested|flat] 27+ messages in thread

* Re: Should rolpassword be toastable?
  2023-09-23 12:00 Should rolpassword be toastable? Alexander Lakhin <exclusion@gmail.com>
  2023-09-23 14:39 ` Re: Should rolpassword be toastable? Tom Lane <tgl@sss.pgh.pa.us>
  2023-09-23 18:00   ` Re: Should rolpassword be toastable? Alexander Lakhin <exclusion@gmail.com>
  2024-09-19 03:00     ` Re: Should rolpassword be toastable? Alexander Lakhin <exclusion@gmail.com>
  2024-09-19 14:22       ` Re: Should rolpassword be toastable? Nathan Bossart <nathandbossart@gmail.com>
  2024-09-19 14:31         ` Re: Should rolpassword be toastable? Tom Lane <tgl@sss.pgh.pa.us>
  2024-09-19 17:44           ` Re: Should rolpassword be toastable? Nathan Bossart <nathandbossart@gmail.com>
  2024-09-19 21:52             ` Re: Should rolpassword be toastable? Nathan Bossart <nathandbossart@gmail.com>
  2024-09-19 22:14               ` Re: Should rolpassword be toastable? Tom Lane <tgl@sss.pgh.pa.us>
  2024-09-19 23:37                 ` Re: Should rolpassword be toastable? Jonathan S. Katz <jkatz@postgresql.org>
  2024-09-20 02:46                   ` Re: Should rolpassword be toastable? Nathan Bossart <nathandbossart@gmail.com>
  2024-09-20 05:23                     ` Re: Should rolpassword be toastable? Michael Paquier <michael@paquier.xyz>
  2024-09-20 14:06                       ` Re: Should rolpassword be toastable? Jonathan S. Katz <jkatz@postgresql.org>
  2024-09-20 16:09                         ` Re: Should rolpassword be toastable? Nathan Bossart <nathandbossart@gmail.com>
  2024-09-20 16:27                           ` Re: Should rolpassword be toastable? Tom Lane <tgl@sss.pgh.pa.us>
  2024-09-21 20:25                             ` Re: Should rolpassword be toastable? Nathan Bossart <nathandbossart@gmail.com>
  2024-10-03 21:27                               ` Re: Should rolpassword be toastable? Nathan Bossart <nathandbossart@gmail.com>
  2024-10-03 21:39                                 ` Re: Should rolpassword be toastable? Tom Lane <tgl@sss.pgh.pa.us>
  2024-10-03 22:17                                   ` Re: Should rolpassword be toastable? Nathan Bossart <nathandbossart@gmail.com>
  2024-10-03 22:24                                     ` Re: Should rolpassword be toastable? Tom Lane <tgl@sss.pgh.pa.us>
  2024-10-03 23:29                                       ` Re: Should rolpassword be toastable? Jacob Champion <jacob.champion@enterprisedb.com>
  2024-10-04 02:17                                         ` Re: Should rolpassword be toastable? Jonathan S. Katz <jkatz@postgresql.org>
  2024-10-04 02:33                                           ` Re: Should rolpassword be toastable? Tom Lane <tgl@sss.pgh.pa.us>
@ 2024-10-04 14:47                                             ` Nathan Bossart <nathandbossart@gmail.com>
  2024-10-06 09:42                                               ` Re: Should rolpassword be toastable? Hannu Krosing <hannuk@google.com>
  0 siblings, 1 reply; 27+ messages in thread

From: Nathan Bossart @ 2024-10-04 14:47 UTC (permalink / raw)
  To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Jonathan S. Katz <jkatz@postgresql.org>; Jacob Champion <jacob.champion@enterprisedb.com>; Michael Paquier <michael@paquier.xyz>; Alexander Lakhin <exclusion@gmail.com>; pgsql-hackers

On Thu, Oct 03, 2024 at 10:33:04PM -0400, Tom Lane wrote:
> "Jonathan S. Katz" <jkatz@postgresql.org> writes:
>> I think Tom's initial suggestion (BLCKSZ/2) is better than 256, given we 
>> really don't know what' out there in the wild, and this could end up 
>> being a breaking change. Every other type in pg_authid is pretty small. 
> 
> I'm having second thoughts about that though, based on the argument
> that we don't really want a platform-dependent limit here.
> Admittedly, nobody changes BLCKSZ on production systems, but it's
> still theoretically an issue.  I don't have a problem with selecting
> a larger limit such as 512 or 1024 though.

Since BLCKSZ can be as low as 1024, I think 512 would be a good choice.

> However, if you wanted to allow multiple passwords I'm not
> sure about a good way.

The most recent proposal I'm aware of [0] did seem to target that use-case.
One option might be to move rolpassword to a different catalog.  In any
case, I don't think it matters much for the patch at hand.

[0] https://postgr.es/m/CAGB%2BVh5SQQorNDEKP%2B0G%3DsmxHRhbhs%2BVkmQWD5Vh98fmn8X4dg%40mail.gmail.com

-- 
nathan





^ permalink  raw  reply  [nested|flat] 27+ messages in thread

* Re: Should rolpassword be toastable?
  2023-09-23 12:00 Should rolpassword be toastable? Alexander Lakhin <exclusion@gmail.com>
  2023-09-23 14:39 ` Re: Should rolpassword be toastable? Tom Lane <tgl@sss.pgh.pa.us>
  2023-09-23 18:00   ` Re: Should rolpassword be toastable? Alexander Lakhin <exclusion@gmail.com>
  2024-09-19 03:00     ` Re: Should rolpassword be toastable? Alexander Lakhin <exclusion@gmail.com>
  2024-09-19 14:22       ` Re: Should rolpassword be toastable? Nathan Bossart <nathandbossart@gmail.com>
  2024-09-19 14:31         ` Re: Should rolpassword be toastable? Tom Lane <tgl@sss.pgh.pa.us>
  2024-09-19 17:44           ` Re: Should rolpassword be toastable? Nathan Bossart <nathandbossart@gmail.com>
  2024-09-19 21:52             ` Re: Should rolpassword be toastable? Nathan Bossart <nathandbossart@gmail.com>
  2024-09-19 22:14               ` Re: Should rolpassword be toastable? Tom Lane <tgl@sss.pgh.pa.us>
  2024-09-19 23:37                 ` Re: Should rolpassword be toastable? Jonathan S. Katz <jkatz@postgresql.org>
  2024-09-20 02:46                   ` Re: Should rolpassword be toastable? Nathan Bossart <nathandbossart@gmail.com>
  2024-09-20 05:23                     ` Re: Should rolpassword be toastable? Michael Paquier <michael@paquier.xyz>
  2024-09-20 14:06                       ` Re: Should rolpassword be toastable? Jonathan S. Katz <jkatz@postgresql.org>
  2024-09-20 16:09                         ` Re: Should rolpassword be toastable? Nathan Bossart <nathandbossart@gmail.com>
  2024-09-20 16:27                           ` Re: Should rolpassword be toastable? Tom Lane <tgl@sss.pgh.pa.us>
  2024-09-21 20:25                             ` Re: Should rolpassword be toastable? Nathan Bossart <nathandbossart@gmail.com>
  2024-10-03 21:27                               ` Re: Should rolpassword be toastable? Nathan Bossart <nathandbossart@gmail.com>
  2024-10-03 21:39                                 ` Re: Should rolpassword be toastable? Tom Lane <tgl@sss.pgh.pa.us>
  2024-10-03 22:17                                   ` Re: Should rolpassword be toastable? Nathan Bossart <nathandbossart@gmail.com>
  2024-10-03 22:24                                     ` Re: Should rolpassword be toastable? Tom Lane <tgl@sss.pgh.pa.us>
  2024-10-03 23:29                                       ` Re: Should rolpassword be toastable? Jacob Champion <jacob.champion@enterprisedb.com>
  2024-10-04 02:17                                         ` Re: Should rolpassword be toastable? Jonathan S. Katz <jkatz@postgresql.org>
  2024-10-04 02:33                                           ` Re: Should rolpassword be toastable? Tom Lane <tgl@sss.pgh.pa.us>
  2024-10-04 14:47                                             ` Re: Should rolpassword be toastable? Nathan Bossart <nathandbossart@gmail.com>
@ 2024-10-06 09:42                                               ` Hannu Krosing <hannuk@google.com>
  2024-10-06 13:12                                                 ` Re: Should rolpassword be toastable? Nathan Bossart <nathandbossart@gmail.com>
  0 siblings, 1 reply; 27+ messages in thread

From: Hannu Krosing @ 2024-10-06 09:42 UTC (permalink / raw)
  To: Nathan Bossart <nathandbossart@gmail.com>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; Jonathan S. Katz <jkatz@postgresql.org>; Jacob Champion <jacob.champion@enterprisedb.com>; Michael Paquier <michael@paquier.xyz>; Alexander Lakhin <exclusion@gmail.com>; pgsql-hackers

On Fri, Oct 4, 2024 at 4:48 PM Nathan Bossart <nathandbossart@gmail.com> wrote:
>
..
> Since BLCKSZ can be as low as 1024, I think 512 would be a good choice.
>


Where did you get the minimal value of 1024 from ?

I vaguely remember someone testing with 256 at some point in the past

---
Hannu





^ permalink  raw  reply  [nested|flat] 27+ messages in thread

* Re: Should rolpassword be toastable?
  2023-09-23 12:00 Should rolpassword be toastable? Alexander Lakhin <exclusion@gmail.com>
  2023-09-23 14:39 ` Re: Should rolpassword be toastable? Tom Lane <tgl@sss.pgh.pa.us>
  2023-09-23 18:00   ` Re: Should rolpassword be toastable? Alexander Lakhin <exclusion@gmail.com>
  2024-09-19 03:00     ` Re: Should rolpassword be toastable? Alexander Lakhin <exclusion@gmail.com>
  2024-09-19 14:22       ` Re: Should rolpassword be toastable? Nathan Bossart <nathandbossart@gmail.com>
  2024-09-19 14:31         ` Re: Should rolpassword be toastable? Tom Lane <tgl@sss.pgh.pa.us>
  2024-09-19 17:44           ` Re: Should rolpassword be toastable? Nathan Bossart <nathandbossart@gmail.com>
  2024-09-19 21:52             ` Re: Should rolpassword be toastable? Nathan Bossart <nathandbossart@gmail.com>
  2024-09-19 22:14               ` Re: Should rolpassword be toastable? Tom Lane <tgl@sss.pgh.pa.us>
  2024-09-19 23:37                 ` Re: Should rolpassword be toastable? Jonathan S. Katz <jkatz@postgresql.org>
  2024-09-20 02:46                   ` Re: Should rolpassword be toastable? Nathan Bossart <nathandbossart@gmail.com>
  2024-09-20 05:23                     ` Re: Should rolpassword be toastable? Michael Paquier <michael@paquier.xyz>
  2024-09-20 14:06                       ` Re: Should rolpassword be toastable? Jonathan S. Katz <jkatz@postgresql.org>
  2024-09-20 16:09                         ` Re: Should rolpassword be toastable? Nathan Bossart <nathandbossart@gmail.com>
  2024-09-20 16:27                           ` Re: Should rolpassword be toastable? Tom Lane <tgl@sss.pgh.pa.us>
  2024-09-21 20:25                             ` Re: Should rolpassword be toastable? Nathan Bossart <nathandbossart@gmail.com>
  2024-10-03 21:27                               ` Re: Should rolpassword be toastable? Nathan Bossart <nathandbossart@gmail.com>
  2024-10-03 21:39                                 ` Re: Should rolpassword be toastable? Tom Lane <tgl@sss.pgh.pa.us>
  2024-10-03 22:17                                   ` Re: Should rolpassword be toastable? Nathan Bossart <nathandbossart@gmail.com>
  2024-10-03 22:24                                     ` Re: Should rolpassword be toastable? Tom Lane <tgl@sss.pgh.pa.us>
  2024-10-03 23:29                                       ` Re: Should rolpassword be toastable? Jacob Champion <jacob.champion@enterprisedb.com>
  2024-10-04 02:17                                         ` Re: Should rolpassword be toastable? Jonathan S. Katz <jkatz@postgresql.org>
  2024-10-04 02:33                                           ` Re: Should rolpassword be toastable? Tom Lane <tgl@sss.pgh.pa.us>
  2024-10-04 14:47                                             ` Re: Should rolpassword be toastable? Nathan Bossart <nathandbossart@gmail.com>
  2024-10-06 09:42                                               ` Re: Should rolpassword be toastable? Hannu Krosing <hannuk@google.com>
@ 2024-10-06 13:12                                                 ` Nathan Bossart <nathandbossart@gmail.com>
  2024-10-07 15:58                                                   ` Re: Should rolpassword be toastable? Nathan Bossart <nathandbossart@gmail.com>
  0 siblings, 1 reply; 27+ messages in thread

From: Nathan Bossart @ 2024-10-06 13:12 UTC (permalink / raw)
  To: Hannu Krosing <hannuk@google.com>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; Jonathan S. Katz <jkatz@postgresql.org>; Jacob Champion <jacob.champion@enterprisedb.com>; Michael Paquier <michael@paquier.xyz>; Alexander Lakhin <exclusion@gmail.com>; pgsql-hackers

On Sun, Oct 06, 2024 at 11:42:53AM +0200, Hannu Krosing wrote:
> On Fri, Oct 4, 2024 at 4:48 PM Nathan Bossart <nathandbossart@gmail.com> wrote:
>> Since BLCKSZ can be as low as 1024, I think 512 would be a good choice.
> 
> Where did you get the minimal value of 1024 from ?

	https://www.postgresql.org/docs/devel/install-make.html#CONFIGURE-OPTION-WITH-BLOCKSIZE

-- 
nathan





^ permalink  raw  reply  [nested|flat] 27+ messages in thread

* Re: Should rolpassword be toastable?
  2023-09-23 12:00 Should rolpassword be toastable? Alexander Lakhin <exclusion@gmail.com>
  2023-09-23 14:39 ` Re: Should rolpassword be toastable? Tom Lane <tgl@sss.pgh.pa.us>
  2023-09-23 18:00   ` Re: Should rolpassword be toastable? Alexander Lakhin <exclusion@gmail.com>
  2024-09-19 03:00     ` Re: Should rolpassword be toastable? Alexander Lakhin <exclusion@gmail.com>
  2024-09-19 14:22       ` Re: Should rolpassword be toastable? Nathan Bossart <nathandbossart@gmail.com>
  2024-09-19 14:31         ` Re: Should rolpassword be toastable? Tom Lane <tgl@sss.pgh.pa.us>
  2024-09-19 17:44           ` Re: Should rolpassword be toastable? Nathan Bossart <nathandbossart@gmail.com>
  2024-09-19 21:52             ` Re: Should rolpassword be toastable? Nathan Bossart <nathandbossart@gmail.com>
  2024-09-19 22:14               ` Re: Should rolpassword be toastable? Tom Lane <tgl@sss.pgh.pa.us>
  2024-09-19 23:37                 ` Re: Should rolpassword be toastable? Jonathan S. Katz <jkatz@postgresql.org>
  2024-09-20 02:46                   ` Re: Should rolpassword be toastable? Nathan Bossart <nathandbossart@gmail.com>
  2024-09-20 05:23                     ` Re: Should rolpassword be toastable? Michael Paquier <michael@paquier.xyz>
  2024-09-20 14:06                       ` Re: Should rolpassword be toastable? Jonathan S. Katz <jkatz@postgresql.org>
  2024-09-20 16:09                         ` Re: Should rolpassword be toastable? Nathan Bossart <nathandbossart@gmail.com>
  2024-09-20 16:27                           ` Re: Should rolpassword be toastable? Tom Lane <tgl@sss.pgh.pa.us>
  2024-09-21 20:25                             ` Re: Should rolpassword be toastable? Nathan Bossart <nathandbossart@gmail.com>
  2024-10-03 21:27                               ` Re: Should rolpassword be toastable? Nathan Bossart <nathandbossart@gmail.com>
  2024-10-03 21:39                                 ` Re: Should rolpassword be toastable? Tom Lane <tgl@sss.pgh.pa.us>
  2024-10-03 22:17                                   ` Re: Should rolpassword be toastable? Nathan Bossart <nathandbossart@gmail.com>
  2024-10-03 22:24                                     ` Re: Should rolpassword be toastable? Tom Lane <tgl@sss.pgh.pa.us>
  2024-10-03 23:29                                       ` Re: Should rolpassword be toastable? Jacob Champion <jacob.champion@enterprisedb.com>
  2024-10-04 02:17                                         ` Re: Should rolpassword be toastable? Jonathan S. Katz <jkatz@postgresql.org>
  2024-10-04 02:33                                           ` Re: Should rolpassword be toastable? Tom Lane <tgl@sss.pgh.pa.us>
  2024-10-04 14:47                                             ` Re: Should rolpassword be toastable? Nathan Bossart <nathandbossart@gmail.com>
  2024-10-06 09:42                                               ` Re: Should rolpassword be toastable? Hannu Krosing <hannuk@google.com>
  2024-10-06 13:12                                                 ` Re: Should rolpassword be toastable? Nathan Bossart <nathandbossart@gmail.com>
@ 2024-10-07 15:58                                                   ` Nathan Bossart <nathandbossart@gmail.com>
  0 siblings, 0 replies; 27+ messages in thread

From: Nathan Bossart @ 2024-10-07 15:58 UTC (permalink / raw)
  To: Hannu Krosing <hannuk@google.com>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; Jonathan S. Katz <jkatz@postgresql.org>; Jacob Champion <jacob.champion@enterprisedb.com>; Michael Paquier <michael@paquier.xyz>; Alexander Lakhin <exclusion@gmail.com>; pgsql-hackers

Committed with the limit set to 512 bytes.  We have plenty of time to
adjust this limit as needed before it takes effect in v18.

-- 
nathan





^ permalink  raw  reply  [nested|flat] 27+ messages in thread


end of thread, other threads:[~2024-10-07 15:58 UTC | newest]

Thread overview: 27+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2023-09-23 12:00 Should rolpassword be toastable? Alexander Lakhin <exclusion@gmail.com>
2023-09-23 14:39 ` Tom Lane <tgl@sss.pgh.pa.us>
2023-09-23 18:00   ` Alexander Lakhin <exclusion@gmail.com>
2024-09-19 03:00     ` Alexander Lakhin <exclusion@gmail.com>
2024-09-19 14:22       ` Nathan Bossart <nathandbossart@gmail.com>
2024-09-19 14:31         ` Tom Lane <tgl@sss.pgh.pa.us>
2024-09-19 17:44           ` Nathan Bossart <nathandbossart@gmail.com>
2024-09-19 21:52             ` Nathan Bossart <nathandbossart@gmail.com>
2024-09-19 22:14               ` Tom Lane <tgl@sss.pgh.pa.us>
2024-09-19 23:37                 ` Jonathan S. Katz <jkatz@postgresql.org>
2024-09-20 02:46                   ` Nathan Bossart <nathandbossart@gmail.com>
2024-09-20 05:23                     ` Michael Paquier <michael@paquier.xyz>
2024-09-20 14:06                       ` Jonathan S. Katz <jkatz@postgresql.org>
2024-09-20 16:09                         ` Nathan Bossart <nathandbossart@gmail.com>
2024-09-20 16:27                           ` Tom Lane <tgl@sss.pgh.pa.us>
2024-09-21 20:25                             ` Nathan Bossart <nathandbossart@gmail.com>
2024-10-03 21:27                               ` Nathan Bossart <nathandbossart@gmail.com>
2024-10-03 21:39                                 ` Tom Lane <tgl@sss.pgh.pa.us>
2024-10-03 22:17                                   ` Nathan Bossart <nathandbossart@gmail.com>
2024-10-03 22:24                                     ` Tom Lane <tgl@sss.pgh.pa.us>
2024-10-03 23:29                                       ` Jacob Champion <jacob.champion@enterprisedb.com>
2024-10-04 02:17                                         ` Jonathan S. Katz <jkatz@postgresql.org>
2024-10-04 02:33                                           ` Tom Lane <tgl@sss.pgh.pa.us>
2024-10-04 14:47                                             ` Nathan Bossart <nathandbossart@gmail.com>
2024-10-06 09:42                                               ` Hannu Krosing <hannuk@google.com>
2024-10-06 13:12                                                 ` Nathan Bossart <nathandbossart@gmail.com>
2024-10-07 15:58                                                   ` Nathan Bossart <nathandbossart@gmail.com>

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox