From bb3aad9105b1997bc088403437ac6294e22809d9 Mon Sep 17 00:00:00 2001 From: Nathan Bossart 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)