From 8275325a06ed91c053e046422a193dc6d56a70c5 Mon Sep 17 00:00:00 2001 From: Nathan Bossart Date: Mon, 7 Oct 2024 10:56:16 -0500 Subject: [PATCH] Restrict password hash length. Commit 6aa44060a3 removed pg_authid's TOAST table because the only varlena column is rolpassword, which cannot be de-TOASTed during authentication because we haven't selected a database yet and cannot read pg_class. Since that change, attempts to set password hashes that require out-of-line storage will fail with a "row is too big" error. This error message might be confusing to users. This commit places a limit on the length of password hashes so that attempts to set long password hashes will fail with a more user-friendly error. The chosen limit of 512 bytes should be sufficient to avoid "row is too big" errors independent of BLCKSZ, but it should also be lenient enough for all reasonable use-cases (or at least all the use-cases we could imagine). Reviewed-by: Tom Lane, Jonathan Katz, Michael Paquier, Jacob Champion Discussion: https://postgr.es/m/89e8649c-eb74-db25-7945-6d6b23992394%40gmail.com --- 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 629e51e00be..b01525dc28a 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. 512 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 f744de4d202..0bb44004353 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. 512 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 (512) + /* * Types of password hashes or secrets. * diff --git a/src/test/regress/expected/password.out b/src/test/regress/expected/password.out index 924d6e001d4..df3857460c2 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$000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000004096:wNFxNSk1hAXBkgub8py3bg==$65zC6E+R0U7tiYTC9+Wtq4Thw6gUDj3eDCINij8TflU=:rC1I7tcVugrHEY2DT0iPjGyjM4aJxkMM9n8WBxtUtHU='; +ERROR: encrypted password is too long +DETAIL: Encrypted passwords must be no longer than 512 bytes. +ALTER ROLE regress_passwd9 PASSWORD 'SCRAM-SHA-256$000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000004096:wNFxNSk1hAXBkgub8py3bg==$65zC6E+R0U7tiYTC9+Wtq4Thw6gUDj3eDCINij8TflU=:rC1I7tcVugrHEY2DT0iPjGyjM4aJxkMM9n8WBxtUtHU='; +ERROR: encrypted password is too long +DETAIL: Encrypted passwords must be no longer than 512 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 bb82aa4aa24..e7a98045450 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$000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000004096:wNFxNSk1hAXBkgub8py3bg==$65zC6E+R0U7tiYTC9+Wtq4Thw6gUDj3eDCINij8TflU=:rC1I7tcVugrHEY2DT0iPjGyjM4aJxkMM9n8WBxtUtHU='; +ALTER ROLE regress_passwd9 PASSWORD 'SCRAM-SHA-256$000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000004096:wNFxNSk1hAXBkgub8py3bg==$65zC6E+R0U7tiYTC9+Wtq4Thw6gUDj3eDCINij8TflU=:rC1I7tcVugrHEY2DT0iPjGyjM4aJxkMM9n8WBxtUtHU='; + DROP ROLE regress_passwd1; DROP ROLE regress_passwd2; DROP ROLE regress_passwd3;