Re: Maximum password length - Mailing list pgsql-hackers

Alexander Kukushkin <cyberdemn@gmail.com> writes:
> Self-containing tokens, for example JWT, could be easily longer than 100 bytes.
> We at Zalando are using such tokens and the usual size of JWT token is
> 600-700 bytes.

> It is not possible to "paste" such token into psql password prompt,
> because the input is truncated by 100 bytes.
> It is not possible to put it into ".pgpass" either, because it assumes
> that line could not be longer than 320 bytes (64*5)

> At the moment there are only two ways to use such tokens as a password:
> 1. export PGPASSWORD=very_long.token
> 2. specify the token(password) in the connection url

This thread seems to have fallen off the radar, but I got interested again
now that we have a report of somebody else trying to use an 800-or-so-byte
password [1], so I looked over Nathan's patches in some detail.

I concur with Stephen's position that there ought to be just one upper
limit not several.  At the same time, it's not clear to me that the
password packet's length is closely related to the plaintext password
limit when we're using SCRAM --- is there any case where the verifier
string could exceed a few hundred bytes?

Also, I'm not exactly convinced that we need to document the limit in the
SGML docs, and I'm definitely down on repeating that info in 16 different
places.  If we make the limit high enough to not be a problem, nobody is
going to care exactly what it is.

Therefore, I propose setting this up with a #define symbol in
pg_config_manual.h and leaving it at that.  Giving documentation in
pg_config_manual.h seems sufficient to me.  Attached is a revised
version of Nathan's patches that does it like that.

I set the proposed limit at 1024 bytes, but given that we now know
of use-cases needing up to 800 bytes, maybe there should be a little
more headroom?  I don't want to make it enormous, though, seeing that
we're allocating static buffers of that size.

Note this patch is intended to be applied over my patch at [2],
since it modifies the test case added there.

            regards, tom lane

[1] https://www.postgresql.org/message-id/flat/CAOhmDze1nqG2vfegpSsTFCgaiFRsqgjO6yLsbmhroz2zGmJHog%40mail.gmail.com
[2] https://www.postgresql.org/message-id/4187382.1598909041%40sss.pgh.pa.us

diff --git a/contrib/oid2name/oid2name.c b/contrib/oid2name/oid2name.c
index 91b7958c48..c83355c6e6 100644
--- a/contrib/oid2name/oid2name.c
+++ b/contrib/oid2name/oid2name.c
@@ -294,7 +294,7 @@ sql_conn(struct options *my_opts)
 {
     PGconn       *conn;
     bool        have_password = false;
-    char        password[100];
+    char        password[MAX_PG_PASSWORD_LEN + 1];
     bool        new_pass;
     PGresult   *res;

diff --git a/contrib/vacuumlo/vacuumlo.c b/contrib/vacuumlo/vacuumlo.c
index e4019fafaa..74d72ca40a 100644
--- a/contrib/vacuumlo/vacuumlo.c
+++ b/contrib/vacuumlo/vacuumlo.c
@@ -70,7 +70,7 @@ vacuumlo(const char *database, const struct _param *param)
     bool        new_pass;
     bool        success = true;
     static bool have_password = false;
-    static char password[100];
+    static char password[MAX_PG_PASSWORD_LEN + 1];

     /* Note: password can be carried over from a previous call */
     if (param->pg_prompt == TRI_YES && !have_password)
diff --git a/src/backend/libpq/auth.c b/src/backend/libpq/auth.c
index 02b6c3f127..af3ceb945a 100644
--- a/src/backend/libpq/auth.c
+++ b/src/backend/libpq/auth.c
@@ -697,8 +697,12 @@ recv_password_packet(Port *port)
             return NULL;        /* EOF */
     }

+    /*
+     * Receive the password.  Allow the password proper (not counting the null
+     * terminator or message length word) to be up to MAX_PG_PASSWORD_LEN.
+     */
     initStringInfo(&buf);
-    if (pq_getmessage(&buf, 1000))    /* receive password */
+    if (pq_getmessage(&buf, MAX_PG_PASSWORD_LEN + 1 + 4))
     {
         /* EOF - pq_getmessage already logged a suitable message */
         pfree(buf.data);
diff --git a/src/bin/initdb/initdb.c b/src/bin/initdb/initdb.c
index 786672b1b6..a7ac748dd0 100644
--- a/src/bin/initdb/initdb.c
+++ b/src/bin/initdb/initdb.c
@@ -1481,8 +1481,8 @@ setup_auth(FILE *cmdfd)
 static void
 get_su_pwd(void)
 {
-    char        pwd1[100];
-    char        pwd2[100];
+    char        pwd1[MAX_PG_PASSWORD_LEN + 1];
+    char        pwd2[MAX_PG_PASSWORD_LEN + 1];

     if (pwprompt)
     {
diff --git a/src/bin/pg_basebackup/streamutil.c b/src/bin/pg_basebackup/streamutil.c
index c08003e7f2..27423f460c 100644
--- a/src/bin/pg_basebackup/streamutil.c
+++ b/src/bin/pg_basebackup/streamutil.c
@@ -50,7 +50,7 @@ char       *dbport = NULL;
 char       *dbname = NULL;
 int            dbgetpassword = 0;    /* 0=auto, -1=never, 1=always */
 static bool have_password = false;
-static char password[100];
+static char password[MAX_PG_PASSWORD_LEN + 1];
 PGconn       *conn = NULL;

 /*
diff --git a/src/bin/pg_dump/pg_backup_db.c b/src/bin/pg_dump/pg_backup_db.c
index 94af11b80a..9b2d8bbe50 100644
--- a/src/bin/pg_dump/pg_backup_db.c
+++ b/src/bin/pg_dump/pg_backup_db.c
@@ -122,7 +122,7 @@ _connectDB(ArchiveHandle *AH, const char *reqdb, const char *requser)
     const char *newdb;
     const char *newuser;
     char       *password;
-    char        passbuf[100];
+    char        passbuf[MAX_PG_PASSWORD_LEN + 1];
     bool        new_pass;

     if (!reqdb)
@@ -242,7 +242,7 @@ ConnectDatabase(Archive *AHX,
 {
     ArchiveHandle *AH = (ArchiveHandle *) AHX;
     char       *password;
-    char        passbuf[100];
+    char        passbuf[MAX_PG_PASSWORD_LEN + 1];
     bool        new_pass;

     if (AH->connection)
diff --git a/src/bin/pg_dump/pg_dumpall.c b/src/bin/pg_dump/pg_dumpall.c
index 2c82b39af0..463996d8df 100644
--- a/src/bin/pg_dump/pg_dumpall.c
+++ b/src/bin/pg_dump/pg_dumpall.c
@@ -1644,7 +1644,7 @@ connectDatabase(const char *dbname, const char *connection_string,
     const char **values = NULL;
     PQconninfoOption *conn_opts = NULL;
     static bool have_password = false;
-    static char password[100];
+    static char password[MAX_PG_PASSWORD_LEN + 1];

     if (prompt_password == TRI_YES && !have_password)
     {
diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.c
index 08a5947a9e..dbe11331ab 100644
--- a/src/bin/pgbench/pgbench.c
+++ b/src/bin/pgbench/pgbench.c
@@ -1175,7 +1175,7 @@ doConnect(void)
     PGconn       *conn;
     bool        new_pass;
     static bool have_password = false;
-    static char password[100];
+    static char password[MAX_PG_PASSWORD_LEN + 1];

     /*
      * Start the connection.  Loop until we have a password if requested by
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index 9902a4a2ba..2c6880d02e 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -1964,8 +1964,8 @@ exec_command_password(PsqlScanState scan_state, bool active_branch)
     {
         char       *opt0 = psql_scan_slash_option(scan_state,
                                                   OT_SQLID, NULL, true);
-        char        pw1[100];
-        char        pw2[100];
+        char        pw1[MAX_PG_PASSWORD_LEN + 1];
+        char        pw2[MAX_PG_PASSWORD_LEN + 1];

         simple_prompt("Enter new password: ", pw1, sizeof(pw1), false);
         simple_prompt("Enter it again: ", pw2, sizeof(pw2), false);
@@ -2982,7 +2982,7 @@ copy_previous_query(PQExpBuffer query_buf, PQExpBuffer previous_buf)
 static char *
 prompt_for_password(const char *username)
 {
-    char        buf[100];
+    char        buf[MAX_PG_PASSWORD_LEN + 1];

     if (username == NULL || username[0] == '\0')
         simple_prompt("Password: ", buf, sizeof(buf), false);
diff --git a/src/bin/psql/startup.c b/src/bin/psql/startup.c
index 3302bd4dd3..127919ee10 100644
--- a/src/bin/psql/startup.c
+++ b/src/bin/psql/startup.c
@@ -120,7 +120,7 @@ main(int argc, char *argv[])
     struct adhoc_opts options;
     int            successResult;
     bool        have_password = false;
-    char        password[100];
+    char        password[MAX_PG_PASSWORD_LEN + 1];
     bool        new_pass;

     pg_logging_init(argv[0]);
diff --git a/src/bin/scripts/common.c b/src/bin/scripts/common.c
index 420d0d11a5..447a61d201 100644
--- a/src/bin/scripts/common.c
+++ b/src/bin/scripts/common.c
@@ -69,7 +69,7 @@ connectDatabase(const char *dbname, const char *pghost,
     PGconn       *conn;
     bool        new_pass;
     static bool have_password = false;
-    static char password[100];
+    static char password[MAX_PG_PASSWORD_LEN + 1];

     if (!allow_password_reuse)
         have_password = false;
diff --git a/src/bin/scripts/createuser.c b/src/bin/scripts/createuser.c
index 9ced079ac7..70c346271e 100644
--- a/src/bin/scripts/createuser.c
+++ b/src/bin/scripts/createuser.c
@@ -64,7 +64,7 @@ main(int argc, char *argv[])
     bool        pwprompt = false;
     char       *newpassword = NULL;
     char        newuser_buf[128];
-    char        newpassword_buf[100];
+    char        newpassword_buf[MAX_PG_PASSWORD_LEN + 1];

     /* Tri-valued variables.  */
     enum trivalue createdb = TRI_DEFAULT,
@@ -206,7 +206,7 @@ main(int argc, char *argv[])

     if (pwprompt)
     {
-        char        pw2[100];
+        char        pw2[MAX_PG_PASSWORD_LEN + 1];

         simple_prompt("Enter password for new role: ",
                       newpassword_buf, sizeof(newpassword_buf), false);
diff --git a/src/common/saslprep.c b/src/common/saslprep.c
index 2dedf6b0fb..6b4967efc2 100644
--- a/src/common/saslprep.c
+++ b/src/common/saslprep.c
@@ -29,12 +29,6 @@
 #include "common/unicode_norm.h"
 #include "mb/pg_wchar.h"

-/*
- * Limit on how large password's we will try to process.  A password
- * larger than this will be treated the same as out-of-memory.
- */
-#define MAX_PASSWORD_LENGTH        1024
-
 /*
  * In backend, we will use palloc/pfree.  In frontend, use malloc, and
  * return SASLPREP_OOM on out-of-memory.
@@ -1079,7 +1073,7 @@ pg_saslprep(const char *input, char **output)
     *output = NULL;

     /* Check that the password isn't stupendously long */
-    if (strlen(input) > MAX_PASSWORD_LENGTH)
+    if (strlen(input) > MAX_PG_PASSWORD_LEN)
     {
 #ifndef FRONTEND
         ereport(ERROR,
diff --git a/src/include/pg_config_manual.h b/src/include/pg_config_manual.h
index 705dc69c06..d2b9255a60 100644
--- a/src/include/pg_config_manual.h
+++ b/src/include/pg_config_manual.h
@@ -97,6 +97,18 @@
  */
 #define MAXPGPATH        1024

+/*
+ * MAX_PG_PASSWORD_LEN: maximum length of a password string.
+ *
+ * This is applied indiscriminately to both plaintext and hashed passwords,
+ * so it must be large enough to accommodate all hashed-password formats.
+ * There are known use-cases in which the password is a machine-generated
+ * security token ranging up to around 800 bytes.
+ * Note that some auth methods have a tighter limit, for example RADIUS
+ * auth is limited to 128-byte passwords according to the RADIUS standard.
+ */
+#define MAX_PG_PASSWORD_LEN        1024
+
 /*
  * PG_SOMAXCONN: maximum accept-queue length limit passed to
  * listen(2).  You'd think we should use SOMAXCONN from
diff --git a/src/test/authentication/t/001_password.pl b/src/test/authentication/t/001_password.pl
index 1b4323fe2a..6a95814784 100644
--- a/src/test/authentication/t/001_password.pl
+++ b/src/test/authentication/t/001_password.pl
@@ -60,13 +60,14 @@ $node->start;

 # Create 3 roles with different password methods for each one. The same
 # password is used for all of them.
+my $password = 'a' . 'b' x 1000 . 'c';
 $node->safe_psql('postgres',
-    "SET password_encryption='scram-sha-256'; CREATE ROLE scram_role LOGIN PASSWORD 'pass';"
+    "SET password_encryption='scram-sha-256'; CREATE ROLE scram_role LOGIN PASSWORD '$password';"
 );
 $node->safe_psql('postgres',
-    "SET password_encryption='md5'; CREATE ROLE md5_role LOGIN PASSWORD 'pass';"
+    "SET password_encryption='md5'; CREATE ROLE md5_role LOGIN PASSWORD '$password';"
 );
-$ENV{"PGPASSWORD"} = 'pass';
+$ENV{"PGPASSWORD"} = $password;

 # For "trust" method, all users should be able to connect.
 reset_pg_hba($node, 'trust');
@@ -108,7 +109,7 @@ $ENV{"PGPASSFILE"} = $pgpassfile;

 append_to_file($pgpassfile, qq!
 # This very long comment is just here to exercise handling of long lines in the file. This very long comment is just
hereto exercise handling of long lines in the file. This very long comment is just here to exercise handling of long
linesin the file. This very long comment is just here to exercise handling of long lines in the file. This very long
commentis just here to exercise handling of long lines in the file. 
-*:*:postgres:scram_role:pass:this is not part of the password.
+*:*:postgres:scram_role:$password:this is not part of the password.
 !);
 chmod 0600, $pgpassfile or die;

@@ -116,8 +117,11 @@ reset_pg_hba($node, 'password');
 test_role($node, 'scram_role', 'password from pgpass', 0);
 test_role($node, 'md5_role',   'password from pgpass', 2);

+my $modpassword = $password;
+$modpassword =~ s/b/\\b/g;
+
 append_to_file($pgpassfile, qq!
-*:*:*:md5_role:p\\ass
+*:*:*:md5_role:$modpassword
 !);

 test_role($node, 'md5_role',   'password from pgpass', 0);