On 2020/01/22 11:01, Fujii Masao wrote:
On 2020/01/22 0:12, Bruce Momjian wrote:
On Tue, Jan 21, 2020 at 02:42:07PM +0900, Fujii Masao wrote:
I have no strong opinion about the maximum length of password,
for now. But IMO it's worth committing that 0001 patch as the first step
for this problem.
Also IMO the more problematic thing is that psql silently truncates
the password specified in the prompt into 99B if its length is
more than 99B. I think that psql should emit a warning in this case
so that users can notice that.
I think we should be using a macro to define the maximum length, rather
than have 100 used in various places.
+1 as the first step for this issue. The patch that I mentioned
upthread actually does that.
Attached is the patch that Nathan proposed at [1] and I think that
it's worth applying. I'd like to add this to next CommitFest.
Thought?
[1]
https://www.postgresql.org/message-id/09512c4f-8cb9-4021-b455-ef4c4f0d5...@amazon.com
Regards,
--
Fujii Masao
NTT DATA CORPORATION
Advanced Platform Technology Group
Research and Development Headquarters
From ba8fb45bfad75ebba9b22bfaf397d2647a416f8e Mon Sep 17 00:00:00 2001
From: Nathan Bossart <bossa...@amazon.com>
Date: Fri, 12 Oct 2018 15:31:31 +0000
Subject: [PATCH v1 1/3] Refactor maximum password length enforced by client
utilities.
---
contrib/oid2name/oid2name.c | 2 +-
contrib/vacuumlo/vacuumlo.c | 2 +-
src/bin/initdb/initdb.c | 4 ++--
src/bin/pg_basebackup/streamutil.c | 2 +-
src/bin/pg_dump/pg_backup_db.c | 4 ++--
src/bin/pg_dump/pg_dumpall.c | 2 +-
src/bin/pgbench/pgbench.c | 2 +-
src/bin/psql/command.c | 6 +++---
src/bin/psql/startup.c | 2 +-
src/bin/scripts/common.c | 2 +-
src/bin/scripts/createuser.c | 4 ++--
src/common/saslprep.c | 4 ++--
src/include/postgres_fe.h | 11 +++++++++++
13 files changed, 29 insertions(+), 18 deletions(-)
diff --git a/contrib/oid2name/oid2name.c b/contrib/oid2name/oid2name.c
index aa122ca0e9..08904ffd12 100644
--- a/contrib/oid2name/oid2name.c
+++ b/contrib/oid2name/oid2name.c
@@ -285,7 +285,7 @@ sql_conn(struct options *my_opts)
{
PGconn *conn;
bool have_password = false;
- char password[100];
+ char password[PROMPT_MAX_PASSWORD_LENGTH];
bool new_pass;
PGresult *res;
diff --git a/contrib/vacuumlo/vacuumlo.c b/contrib/vacuumlo/vacuumlo.c
index 3075781abe..5acfa65289 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[PROMPT_MAX_PASSWORD_LENGTH];
/* Note: password can be carried over from a previous call */
if (param->pg_prompt == TRI_YES && !have_password)
diff --git a/src/bin/initdb/initdb.c b/src/bin/initdb/initdb.c
index ab5cb7f0c1..a9663531de 100644
--- a/src/bin/initdb/initdb.c
+++ b/src/bin/initdb/initdb.c
@@ -1528,8 +1528,8 @@ setup_auth(FILE *cmdfd)
static void
get_su_pwd(void)
{
- char pwd1[100];
- char pwd2[100];
+ char pwd1[PROMPT_MAX_PASSWORD_LENGTH];
+ char pwd2[PROMPT_MAX_PASSWORD_LENGTH];
if (pwprompt)
{
diff --git a/src/bin/pg_basebackup/streamutil.c
b/src/bin/pg_basebackup/streamutil.c
index 52f1e559b7..8ecb412ab8 100644
--- a/src/bin/pg_basebackup/streamutil.c
+++ b/src/bin/pg_basebackup/streamutil.c
@@ -51,7 +51,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[PROMPT_MAX_PASSWORD_LENGTH];
PGconn *conn = NULL;
/*
diff --git a/src/bin/pg_dump/pg_backup_db.c b/src/bin/pg_dump/pg_backup_db.c
index 5e32ee8a5b..402ee3cff1 100644
--- a/src/bin/pg_dump/pg_backup_db.c
+++ b/src/bin/pg_dump/pg_backup_db.c
@@ -126,7 +126,7 @@ _connectDB(ArchiveHandle *AH, const char *reqdb, const char
*requser)
const char *newdb;
const char *newuser;
char *password;
- char passbuf[100];
+ char passbuf[PROMPT_MAX_PASSWORD_LENGTH];
bool new_pass;
if (!reqdb)
@@ -246,7 +246,7 @@ ConnectDatabase(Archive *AHX,
{
ArchiveHandle *AH = (ArchiveHandle *) AHX;
char *password;
- char passbuf[100];
+ char passbuf[PROMPT_MAX_PASSWORD_LENGTH];
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 5176626476..0521f8c700 100644
--- a/src/bin/pg_dump/pg_dumpall.c
+++ b/src/bin/pg_dump/pg_dumpall.c
@@ -1536,7 +1536,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[PROMPT_MAX_PASSWORD_LENGTH];
if (prompt_password == TRI_YES && !have_password)
{
diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.c
index 436764b9c9..097666169b 100644
--- a/src/bin/pgbench/pgbench.c
+++ b/src/bin/pgbench/pgbench.c
@@ -1179,7 +1179,7 @@ doConnect(void)
PGconn *conn;
bool new_pass;
static bool have_password = false;
- static char password[100];
+ static char password[PROMPT_MAX_PASSWORD_LENGTH];
/*
* 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 5b4d54a442..879fa1f494 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -1790,8 +1790,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[PROMPT_MAX_PASSWORD_LENGTH];
+ char pw2[PROMPT_MAX_PASSWORD_LENGTH];
simple_prompt("Enter new password: ", pw1, sizeof(pw1), false);
simple_prompt("Enter it again: ", pw2, sizeof(pw2), false);
@@ -2808,7 +2808,7 @@ copy_previous_query(PQExpBuffer query_buf, PQExpBuffer
previous_buf)
static char *
prompt_for_password(const char *username)
{
- char buf[100];
+ char buf[PROMPT_MAX_PASSWORD_LENGTH];
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 be57574cd3..dfc1377382 100644
--- a/src/bin/psql/startup.c
+++ b/src/bin/psql/startup.c
@@ -100,7 +100,7 @@ main(int argc, char *argv[])
struct adhoc_opts options;
int successResult;
bool have_password = false;
- char password[100];
+ char password[PROMPT_MAX_PASSWORD_LENGTH];
bool new_pass;
set_pglocale_pgservice(argv[0], PG_TEXTDOMAIN("psql"));
diff --git a/src/bin/scripts/common.c b/src/bin/scripts/common.c
index ba6120706d..0b31bf3672 100644
--- a/src/bin/scripts/common.c
+++ b/src/bin/scripts/common.c
@@ -73,7 +73,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[PROMPT_MAX_PASSWORD_LENGTH];
if (!allow_password_reuse)
have_password = false;
diff --git a/src/bin/scripts/createuser.c b/src/bin/scripts/createuser.c
index 3420e62fdd..ced7386af8 100644
--- a/src/bin/scripts/createuser.c
+++ b/src/bin/scripts/createuser.c
@@ -66,7 +66,7 @@ main(int argc, char *argv[])
bool pwprompt = false;
char *newpassword = NULL;
char newuser_buf[128];
- char newpassword_buf[100];
+ char newpassword_buf[PROMPT_MAX_PASSWORD_LENGTH];
/* Tri-valued variables. */
enum trivalue createdb = TRI_DEFAULT,
@@ -201,7 +201,7 @@ main(int argc, char *argv[])
if (pwprompt)
{
- char pw2[100];
+ char pw2[PROMPT_MAX_PASSWORD_LENGTH];
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 4cf574fed8..1f90df250d 100644
--- a/src/common/saslprep.c
+++ b/src/common/saslprep.c
@@ -39,7 +39,7 @@
* 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
+#define SASLPREP_MAX_PASSWORD_LENGTH 1024
/*
* In backend, we will use palloc/pfree. In frontend, use malloc, and
@@ -1085,7 +1085,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) > SASLPREP_MAX_PASSWORD_LENGTH)
{
#ifndef FRONTEND
ereport(ERROR,
diff --git a/src/include/postgres_fe.h b/src/include/postgres_fe.h
index 14567953f2..0c9293f942 100644
--- a/src/include/postgres_fe.h
+++ b/src/include/postgres_fe.h
@@ -26,4 +26,15 @@
#include "common/fe_memutils.h"
+/*
+ * Limit on the length of passwords we will try to process. Note that this
does
+ * not restrict the creation of longer passwords via commands such as CREATE
+ * ROLE and ALTER ROLE. It merely restricts the length of passwords accepted
by
+ * client utility prompts (e.g. psql).
+ *
+ * One byte is reserved at the end for the '\0' byte, so the user-facing
maximum
+ * length is actually 99.
+ */
+#define PROMPT_MAX_PASSWORD_LENGTH 100
+
#endif /* POSTGRES_FE_H */
--
2.16.2
