Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 8275325

Browse files
Restrict password hash length.
Commit 6aa4406 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
1 parent 022564f commit 8275325

File tree

4 files changed

+63
-18
lines changed

4 files changed

+63
-18
lines changed

src/backend/libpq/crypt.c

Lines changed: 42 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,7 @@ encrypt_password(PasswordType target_type, const char *role,
116116
const char *password)
117117
{
118118
PasswordType guessed_type = get_password_type(password);
119-
char *encrypted_password;
119+
char *encrypted_password = NULL;
120120
const char *errstr = NULL;
121121

122122
if (guessed_type != PASSWORD_TYPE_PLAINTEXT)
@@ -125,32 +125,56 @@ encrypt_password(PasswordType target_type, const char *role,
125125
* Cannot convert an already-encrypted password from one format to
126126
* another, so return it as it is.
127127
*/
128-
return pstrdup(password);
128+
encrypted_password = pstrdup(password);
129129
}
130-
131-
switch (target_type)
130+
else
132131
{
133-
case PASSWORD_TYPE_MD5:
134-
encrypted_password = palloc(MD5_PASSWD_LEN + 1);
132+
switch (target_type)
133+
{
134+
case PASSWORD_TYPE_MD5:
135+
encrypted_password = palloc(MD5_PASSWD_LEN + 1);
135136

136-
if (!pg_md5_encrypt(password, role, strlen(role),
137-
encrypted_password, &errstr))
138-
elog(ERROR, "password encryption failed: %s", errstr);
139-
return encrypted_password;
137+
if (!pg_md5_encrypt(password, role, strlen(role),
138+
encrypted_password, &errstr))
139+
elog(ERROR, "password encryption failed: %s", errstr);
140+
break;
140141

141-
case PASSWORD_TYPE_SCRAM_SHA_256:
142-
return pg_be_scram_build_secret(password);
142+
case PASSWORD_TYPE_SCRAM_SHA_256:
143+
encrypted_password = pg_be_scram_build_secret(password);
144+
break;
143145

144-
case PASSWORD_TYPE_PLAINTEXT:
145-
elog(ERROR, "cannot encrypt password with 'plaintext'");
146+
case PASSWORD_TYPE_PLAINTEXT:
147+
elog(ERROR, "cannot encrypt password with 'plaintext'");
148+
break;
149+
}
146150
}
147151

152+
Assert(encrypted_password);
153+
148154
/*
149-
* This shouldn't happen, because the above switch statements should
150-
* handle every combination of source and target password types.
155+
* Valid password hashes may be very long, but we don't want to store
156+
* anything that might need out-of-line storage, since de-TOASTing won't
157+
* work during authentication because we haven't selected a database yet
158+
* and cannot read pg_class. 512 bytes should be more than enough for all
159+
* practical use, so fail for anything longer.
151160
*/
152-
elog(ERROR, "cannot encrypt password to requested type");
153-
return NULL; /* keep compiler quiet */
161+
if (encrypted_password && /* keep compiler quiet */
162+
strlen(encrypted_password) > MAX_ENCRYPTED_PASSWORD_LEN)
163+
{
164+
/*
165+
* We don't expect any of our own hashing routines to produce hashes
166+
* that are too long.
167+
*/
168+
Assert(guessed_type != PASSWORD_TYPE_PLAINTEXT);
169+
170+
ereport(ERROR,
171+
(errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
172+
errmsg("encrypted password is too long"),
173+
errdetail("Encrypted passwords must be no longer than %d bytes.",
174+
MAX_ENCRYPTED_PASSWORD_LEN)));
175+
}
176+
177+
return encrypted_password;
154178
}
155179

156180
/*

src/include/libpq/crypt.h

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,16 @@
1515

1616
#include "datatype/timestamp.h"
1717

18+
/*
19+
* Valid password hashes may be very long, but we don't want to store anything
20+
* that might need out-of-line storage, since de-TOASTing won't work during
21+
* authentication because we haven't selected a database yet and cannot read
22+
* pg_class. 512 bytes should be more than enough for all practical use, and
23+
* our own password encryption routines should never produce hashes longer than
24+
* this.
25+
*/
26+
#define MAX_ENCRYPTED_PASSWORD_LEN (512)
27+
1828
/*
1929
* Types of password hashes or secrets.
2030
*

src/test/regress/expected/password.out

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,13 @@ SELECT rolname, rolpassword not like '%A6xHKoH/494E941doaPOYg==%' as is_rolpassw
127127
regress_passwd_sha_len2 | t
128128
(3 rows)
129129

130+
-- Test that valid hashes that are too long are rejected
131+
CREATE ROLE regress_passwd10 PASSWORD 'SCRAM-SHA-256$000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000004096:wNFxNSk1hAXBkgub8py3bg==$65zC6E+R0U7tiYTC9+Wtq4Thw6gUDj3eDCINij8TflU=:rC1I7tcVugrHEY2DT0iPjGyjM4aJxkMM9n8WBxtUtHU=';
132+
ERROR: encrypted password is too long
133+
DETAIL: Encrypted passwords must be no longer than 512 bytes.
134+
ALTER ROLE regress_passwd9 PASSWORD 'SCRAM-SHA-256$000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000004096:wNFxNSk1hAXBkgub8py3bg==$65zC6E+R0U7tiYTC9+Wtq4Thw6gUDj3eDCINij8TflU=:rC1I7tcVugrHEY2DT0iPjGyjM4aJxkMM9n8WBxtUtHU=';
135+
ERROR: encrypted password is too long
136+
DETAIL: Encrypted passwords must be no longer than 512 bytes.
130137
DROP ROLE regress_passwd1;
131138
DROP ROLE regress_passwd2;
132139
DROP ROLE regress_passwd3;

src/test/regress/sql/password.sql

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,10 @@ SELECT rolname, rolpassword not like '%A6xHKoH/494E941doaPOYg==%' as is_rolpassw
9595
WHERE rolname LIKE 'regress_passwd_sha_len%'
9696
ORDER BY rolname;
9797

98+
-- Test that valid hashes that are too long are rejected
99+
CREATE ROLE regress_passwd10 PASSWORD 'SCRAM-SHA-256$000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000004096:wNFxNSk1hAXBkgub8py3bg==$65zC6E+R0U7tiYTC9+Wtq4Thw6gUDj3eDCINij8TflU=:rC1I7tcVugrHEY2DT0iPjGyjM4aJxkMM9n8WBxtUtHU=';
100+
ALTER ROLE regress_passwd9 PASSWORD 'SCRAM-SHA-256$000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000004096:wNFxNSk1hAXBkgub8py3bg==$65zC6E+R0U7tiYTC9+Wtq4Thw6gUDj3eDCINij8TflU=:rC1I7tcVugrHEY2DT0iPjGyjM4aJxkMM9n8WBxtUtHU=';
101+
98102
DROP ROLE regress_passwd1;
99103
DROP ROLE regress_passwd2;
100104
DROP ROLE regress_passwd3;

0 commit comments

Comments
 (0)