Skip to content

Commit

Permalink
Remove arbitrary restrictions on password length.
Browse files Browse the repository at this point in the history 
This patch started out with the goal of harmonizing various arbitrary
limits on password length, but after awhile a better idea emerged:
let's just get rid of those fixed limits.

recv_password_packet() has an arbitrary limit on the packet size,
which we don't really need, so just drop it.  (Note that this doesn't
really affect anything for MD5 or SCRAM password verification, since
those will hash the user's password to something shorter anyway.
It does matter for auth methods that require a cleartext password.)

Likewise remove the arbitrary error condition in pg_saslprep().

The remaining limits are mostly in client-side code that prompts
for passwords.  To improve those, refactor simple_prompt() so that
it allocates its own result buffer that can be made as big as
necessary.  Actually, it proves best to make a separate routine
pg_get_line() that has essentially the semantics of fgets(), except
that it allocates a suitable result buffer and hence will never
return a truncated line.  (pg_get_line has a lot of potential
applications to replace randomly-sized fgets buffers elsewhere,
but I'll leave that for another patch.)

I built pg_get_line() atop stringinfo.c, which requires moving
that code to src/common/; but that seems fine since it was a poor
fit for src/port/ anyway.

This patch is mostly mine, but it owes a good deal to Nathan Bossart
who pressed for a solution to the password length problem and
created a predecessor patch.  Also thanks to Peter Eisentraut and
Stephen Frost for ideas and discussion.

Discussion: https://postgr.es/m/[email protected]
  • Loading branch information
@tglsfdc
tglsfdc committed Sep 4, 2020
1 parent be4b0c0 commit 67a472d
Show file tree
Hide file tree
Showing 21 changed files with 221 additions and 167 deletions.
11 changes: 5 additions & 6 deletions contrib/oid2name/oid2name.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include "catalog/pg_class_d.h"
#include "common/connect.h"
#include "common/logging.h"
#include "common/string.h"
#include "getopt_long.h"
#include "libpq-fe.h"
#include "pg_getopt.h"
Expand Down Expand Up @@ -293,8 +294,7 @@ PGconn *
sql_conn(struct options *my_opts)
{
PGconn *conn;
bool have_password = false;
char password[100];
char *password = NULL;
bool new_pass;
PGresult *res;

Expand All @@ -316,7 +316,7 @@ sql_conn(struct options *my_opts)
keywords[2] = "user";
values[2] = my_opts->username;
keywords[3] = "password";
values[3] = have_password ? password : NULL;
values[3] = password;
keywords[4] = "dbname";
values[4] = my_opts->dbname;
keywords[5] = "fallback_application_name";
Expand All @@ -336,11 +336,10 @@ sql_conn(struct options *my_opts)

if (PQstatus(conn) == CONNECTION_BAD &&
PQconnectionNeedsPassword(conn) &&
!have_password)
!password)
{
PQfinish(conn);
simple_prompt("Password: ", password, sizeof(password), false);
have_password = true;
password = simple_prompt("Password: ", false);
new_pass = true;
}
} while (new_pass);
Expand Down
18 changes: 7 additions & 11 deletions contrib/vacuumlo/vacuumlo.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
#include "catalog/pg_class_d.h"
#include "common/connect.h"
#include "common/logging.h"
#include "common/string.h"
#include "getopt_long.h"
#include "libpq-fe.h"
#include "pg_getopt.h"
Expand Down Expand Up @@ -69,15 +70,11 @@ vacuumlo(const char *database, const struct _param *param)
int i;
bool new_pass;
bool success = true;
static bool have_password = false;
static char password[100];
static char *password = NULL;

/* Note: password can be carried over from a previous call */
if (param->pg_prompt == TRI_YES && !have_password)
{
simple_prompt("Password: ", password, sizeof(password), false);
have_password = true;
}
if (param->pg_prompt == TRI_YES && !password)
password = simple_prompt("Password: ", false);

/*
* Start the connection. Loop until we have a password if requested by
Expand All @@ -97,7 +94,7 @@ vacuumlo(const char *database, const struct _param *param)
keywords[2] = "user";
values[2] = param->pg_user;
keywords[3] = "password";
values[3] = have_password ? password : NULL;
values[3] = password;
keywords[4] = "dbname";
values[4] = database;
keywords[5] = "fallback_application_name";
Expand All @@ -115,12 +112,11 @@ vacuumlo(const char *database, const struct _param *param)

if (PQstatus(conn) == CONNECTION_BAD &&
PQconnectionNeedsPassword(conn) &&
!have_password &&
!password &&
param->pg_prompt != TRI_NO)
{
PQfinish(conn);
simple_prompt("Password: ", password, sizeof(password), false);
have_password = true;
password = simple_prompt("Password: ", false);
new_pass = true;
}
} while (new_pass);
Expand Down
2 changes: 1 addition & 1 deletion src/backend/libpq/auth.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -698,7 +698,7 @@ recv_password_packet(Port *port)
}

initStringInfo(&buf);
if (pq_getmessage(&buf, 1000)) /* receive password */
if (pq_getmessage(&buf, 0)) /* receive password */
{
/* EOF - pq_getmessage already logged a suitable message */
pfree(buf.data);
Expand Down
21 changes: 11 additions & 10 deletions src/bin/initdb/initdb.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@
#include "common/file_utils.h"
#include "common/logging.h"
#include "common/restricted_token.h"
#include "common/string.h"
#include "common/username.h"
#include "fe_utils/string_utils.h"
#include "getaddrinfo.h"
Expand Down Expand Up @@ -1481,23 +1482,25 @@ setup_auth(FILE *cmdfd)
static void
get_su_pwd(void)
{
char pwd1[100];
char pwd2[100];
char *pwd1;

if (pwprompt)
{
/*
* Read password from terminal
*/
char *pwd2;

printf("\n");
fflush(stdout);
simple_prompt("Enter new superuser password: ", pwd1, sizeof(pwd1), false);
simple_prompt("Enter it again: ", pwd2, sizeof(pwd2), false);
pwd1 = simple_prompt("Enter new superuser password: ", false);
pwd2 = simple_prompt("Enter it again: ", false);
if (strcmp(pwd1, pwd2) != 0)
{
fprintf(stderr, _("Passwords didn't match.\n"));
exit(1);
}
free(pwd2);
}
else
{
Expand All @@ -1510,15 +1513,15 @@ get_su_pwd(void)
* for now.
*/
FILE *pwf = fopen(pwfilename, "r");
int i;

if (!pwf)
{
pg_log_error("could not open file \"%s\" for reading: %m",
pwfilename);
exit(1);
}
if (!fgets(pwd1, sizeof(pwd1), pwf))
pwd1 = pg_get_line(pwf);
if (!pwd1)
{
if (ferror(pwf))
pg_log_error("could not read password from file \"%s\": %m",
Expand All @@ -1530,12 +1533,10 @@ get_su_pwd(void)
}
fclose(pwf);

i = strlen(pwd1);
while (i > 0 && (pwd1[i - 1] == '\r' || pwd1[i - 1] == '\n'))
pwd1[--i] = '\0';
(void) pg_strip_crlf(pwd1);
}

superuser_password = pg_strdup(pwd1);
superuser_password = pwd1;
}

/*
Expand Down
13 changes: 7 additions & 6 deletions src/bin/pg_basebackup/streamutil.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
#include "common/fe_memutils.h"
#include "common/file_perm.h"
#include "common/logging.h"
#include "common/string.h"
#include "datatype/timestamp.h"
#include "port/pg_bswap.h"
#include "pqexpbuffer.h"
Expand Down Expand Up @@ -49,8 +50,7 @@ char *dbuser = NULL;
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 = NULL;
PGconn *conn = NULL;

/*
Expand Down Expand Up @@ -150,20 +150,21 @@ GetConnection(void)
}

/* If -W was given, force prompt for password, but only the first time */
need_password = (dbgetpassword == 1 && !have_password);
need_password = (dbgetpassword == 1 && !password);

do
{
/* Get a new password if appropriate */
if (need_password)
{
simple_prompt("Password: ", password, sizeof(password), false);
have_password = true;
if (password)
free(password);
password = simple_prompt("Password: ", false);
need_password = false;
}

/* Use (or reuse, on a subsequent connection) password if we have it */
if (have_password)
if (password)
{
keywords[i] = "password";
values[i] = password;
Expand Down
28 changes: 14 additions & 14 deletions src/bin/pg_dump/pg_backup_db.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
#endif

#include "common/connect.h"
#include "common/string.h"
#include "dumputils.h"
#include "fe_utils/string_utils.h"
#include "parallel.h"
Expand Down Expand Up @@ -122,7 +123,6 @@ _connectDB(ArchiveHandle *AH, const char *reqdb, const char *requser)
const char *newdb;
const char *newuser;
char *password;
char passbuf[100];
bool new_pass;

if (!reqdb)
Expand All @@ -141,10 +141,7 @@ _connectDB(ArchiveHandle *AH, const char *reqdb, const char *requser)
password = AH->savedPassword;

if (AH->promptPassword == TRI_YES && password == NULL)
{
simple_prompt("Password: ", passbuf, sizeof(passbuf), false);
password = passbuf;
}
password = simple_prompt("Password: ", false);

initPQExpBuffer(&connstr);
appendPQExpBufferStr(&connstr, "dbname=");
Expand Down Expand Up @@ -191,8 +188,9 @@ _connectDB(ArchiveHandle *AH, const char *reqdb, const char *requser)

if (AH->promptPassword != TRI_NO)
{
simple_prompt("Password: ", passbuf, sizeof(passbuf), false);
password = passbuf;
if (password && password != AH->savedPassword)
free(password);
password = simple_prompt("Password: ", false);
}
else
fatal("connection needs password");
Expand All @@ -201,6 +199,9 @@ _connectDB(ArchiveHandle *AH, const char *reqdb, const char *requser)
}
} while (new_pass);

if (password && password != AH->savedPassword)
free(password);

/*
* We want to remember connection's actual password, whether or not we got
* it by prompting. So we don't just store the password variable.
Expand Down Expand Up @@ -242,7 +243,6 @@ ConnectDatabase(Archive *AHX,
{
ArchiveHandle *AH = (ArchiveHandle *) AHX;
char *password;
char passbuf[100];
bool new_pass;

if (AH->connection)
Expand All @@ -251,10 +251,8 @@ ConnectDatabase(Archive *AHX,
password = AH->savedPassword;

if (prompt_password == TRI_YES && password == NULL)
{
simple_prompt("Password: ", passbuf, sizeof(passbuf), false);
password = passbuf;
}
password = simple_prompt("Password: ", false);

AH->promptPassword = prompt_password;

/*
Expand Down Expand Up @@ -293,8 +291,7 @@ ConnectDatabase(Archive *AHX,
prompt_password != TRI_NO)
{
PQfinish(AH->connection);
simple_prompt("Password: ", passbuf, sizeof(passbuf), false);
password = passbuf;
password = simple_prompt("Password: ", false);
new_pass = true;
}
} while (new_pass);
Expand All @@ -309,6 +306,9 @@ ConnectDatabase(Archive *AHX,
PQclear(ExecuteSqlQueryForSingleRow((Archive *) AH,
ALWAYS_SECURE_SEARCH_PATH_SQL));

if (password && password != AH->savedPassword)
free(password);

/*
* We want to remember connection's actual password, whether or not we got
* it by prompting. So we don't just store the password variable.
Expand Down
18 changes: 7 additions & 11 deletions src/bin/pg_dump/pg_dumpall.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
#include "common/connect.h"
#include "common/file_utils.h"
#include "common/logging.h"
#include "common/string.h"
#include "dumputils.h"
#include "fe_utils/string_utils.h"
#include "getopt_long.h"
Expand Down Expand Up @@ -1643,14 +1644,10 @@ connectDatabase(const char *dbname, const char *connection_string,
const char **keywords = NULL;
const char **values = NULL;
PQconninfoOption *conn_opts = NULL;
static bool have_password = false;
static char password[100];
static char *password = NULL;

if (prompt_password == TRI_YES && !have_password)
{
simple_prompt("Password: ", password, sizeof(password), false);
have_password = true;
}
if (prompt_password == TRI_YES && !password)
password = simple_prompt("Password: ", false);

/*
* Start the connection. Loop until we have a password if requested by
Expand Down Expand Up @@ -1730,7 +1727,7 @@ connectDatabase(const char *dbname, const char *connection_string,
values[i] = pguser;
i++;
}
if (have_password)
if (password)
{
keywords[i] = "password";
values[i] = password;
Expand All @@ -1757,12 +1754,11 @@ connectDatabase(const char *dbname, const char *connection_string,

if (PQstatus(conn) == CONNECTION_BAD &&
PQconnectionNeedsPassword(conn) &&
!have_password &&
!password &&
prompt_password != TRI_NO)
{
PQfinish(conn);
simple_prompt("Password: ", password, sizeof(password), false);
have_password = true;
password = simple_prompt("Password: ", false);
new_pass = true;
}
} while (new_pass);
Expand Down
11 changes: 5 additions & 6 deletions src/bin/pgbench/pgbench.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@

#include "common/int.h"
#include "common/logging.h"
#include "common/string.h"
#include "fe_utils/cancel.h"
#include "fe_utils/conditional.h"
#include "getopt_long.h"
Expand Down Expand Up @@ -1174,8 +1175,7 @@ doConnect(void)
{
PGconn *conn;
bool new_pass;
static bool have_password = false;
static char password[100];
static char *password = NULL;

/*
* Start the connection. Loop until we have a password if requested by
Expand All @@ -1195,7 +1195,7 @@ doConnect(void)
keywords[2] = "user";
values[2] = login;
keywords[3] = "password";
values[3] = have_password ? password : NULL;
values[3] = password;
keywords[4] = "dbname";
values[4] = dbName;
keywords[5] = "fallback_application_name";
Expand All @@ -1215,11 +1215,10 @@ doConnect(void)

if (PQstatus(conn) == CONNECTION_BAD &&
PQconnectionNeedsPassword(conn) &&
!have_password)
!password)
{
PQfinish(conn);
simple_prompt("Password: ", password, sizeof(password), false);
have_password = true;
password = simple_prompt("Password: ", false);
new_pass = true;
}
} while (new_pass);
Expand Down
Loading

0 comments on commit 67a472d

Please sign in to comment.