Re: BUG #6412: psql & fe-connect truncate passwords

From: Bruce Momjian <bruce(at)momjian(dot)us>
To: Andy Grimm <agrimm(at)gmail(dot)com>
Cc: Euler Taveira de Oliveira <euler(at)timbira(dot)com>, PostgreSQL Bugs <pgsql-bugs(at)postgresql(dot)org>
Subject: Re: BUG #6412: psql & fe-connect truncate passwords
Date: 2012-08-27 16:12:25
Message-ID: 20120827161225.GL11088@momjian.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-bugs


Did we want this patch applied? Not enough demand?

---------------------------------------------------------------------------

On Wed, Feb 15, 2012 at 12:09:12AM -0500, Andy Grimm wrote:
> On Sat, Jan 28, 2012 at 7:47 PM, Euler Taveira de Oliveira
> <euler(at)timbira(dot)com> wrote:
> > On 28-01-2012 18:55, Andy Grimm wrote:
> >> It's not uniform between the client and the server, though.
> >>
> > The server doesn't impose a hard limit for password length and AFAICS it
> > should not because we aim for backward compatibility.
> >
> >> It sounds like you are suggesting
> >> that rather than increase the limit in the simple_prompt calls, you'd
> >> prefer to decrease the limit read from pwfile?  That doesn't
> >> particularly help me.
> >>
> > No, I am not. So there are three concerns here: (i) increase the limit for
> > simple_prompt() and (ii) raise an error when we reach that limit and (iii) fix
> > the PasswordFromFile(). Looking at your patch, it seems to fix only (i).
>
> Sorry that it's been a couple of weeks, but I have gotten around to
> working on a patch that address more of these concerns. The attached
> patch should
>
> 1) allow arbitrary length passwords to be read from a file via initdb --pwfile
> 2) allow the client to accept a password of arbitrary length at the
> password prompt
> 3) allow a password of arbitrary length in a pgpass file
>
> In #2 I say "allow the client to accept", because there's a
> pq_getmessage call in src/backend/libpq/auth.c which limits the
> password message length to 1000 characters. Changing that part of the
> code should allow longer passwords, but there may be other lurking
> backend issues after that, and I'm not concerned about going beyond
> 1000 at this point.
>
> --Andy
>
> >> require understanding of what the real password length limit in a
> >> database is.
> >>
> > There is no such limit; it is stored in a text datatype.
> >
> >
> > --
> >   Euler Taveira de Oliveira - Timbira       http://www.timbira.com.br/
> >   PostgreSQL: Consultoria, Desenvolvimento, Suporte 24x7 e Treinamento

> diff --git a/src/bin/initdb/initdb.c b/src/bin/initdb/initdb.c
> index 99cf5b4..047b25e 100644
> --- a/src/bin/initdb/initdb.c
> +++ b/src/bin/initdb/initdb.c
> @@ -1303,8 +1303,8 @@ get_set_pwd(void)
> /*
> * Read password from terminal
> */
> - pwd1 = simple_prompt("Enter new superuser password: ", 100, false);
> - pwd2 = simple_prompt("Enter it again: ", 100, false);
> + pwd1 = simple_prompt("Enter new superuser password: ", MAX_PASSWD, false);
> + pwd2 = simple_prompt("Enter it again: ", MAX_PASSWD, false);
> if (strcmp(pwd1, pwd2) != 0)
> {
> fprintf(stderr, _("Passwords didn't match.\n"));
> @@ -1323,7 +1323,7 @@ get_set_pwd(void)
> * for now.
> */
> FILE *pwf = fopen(pwfilename, "r");
> - char pwdbuf[MAXPGPATH];
> + char *pwdbuf = calloc(1,1), buf[1024];
> int i;
>
> if (!pwf)
> @@ -1332,7 +1332,27 @@ get_set_pwd(void)
> progname, pwfilename, strerror(errno));
> exit_nicely();
> }
> - if (!fgets(pwdbuf, sizeof(pwdbuf), pwf))
> +
> + do
> + {
> + if (fgets(buf, sizeof(buf), pwf) == NULL)
> + break;
> + pwdbuf = realloc( pwdbuf, strlen(pwdbuf)+1+strlen(buf) );
> + if (!pwdbuf)
> + {
> + // Out of memory ?
> + fprintf(stderr, _("%s: could not read password from file \"%s\": %s\n"),
> + progname, pwfilename, strerror(errno));
> + exit_nicely();
> + }
> + strcat( pwdbuf, buf);
> + i = strlen(pwdbuf);
> + } while (strlen(buf) > 0 && pwdbuf[i-1] != '\n');
> +
> + while (i > 0 && (pwdbuf[i - 1] == '\r' || pwdbuf[i - 1] == '\n'))
> + pwdbuf[--i] = '\0';
> +
> + if (!i)
> {
> fprintf(stderr, _("%s: could not read password from file \"%s\": %s\n"),
> progname, pwfilename, strerror(errno));
> @@ -1340,10 +1360,6 @@ get_set_pwd(void)
> }
> fclose(pwf);
>
> - i = strlen(pwdbuf);
> - while (i > 0 && (pwdbuf[i - 1] == '\r' || pwdbuf[i - 1] == '\n'))
> - pwdbuf[--i] = '\0';
> -
> pwd1 = xstrdup(pwdbuf);
>
> }
> diff --git a/src/bin/pg_dump/pg_backup_db.c b/src/bin/pg_dump/pg_backup_db.c
> index 09da892..4f64672 100644
> --- a/src/bin/pg_dump/pg_backup_db.c
> +++ b/src/bin/pg_dump/pg_backup_db.c
> @@ -143,7 +143,7 @@ _connectDB(ArchiveHandle *AH, const char *reqdb, const char *requser)
>
> if (AH->promptPassword == TRI_YES && password == NULL)
> {
> - password = simple_prompt("Password: ", 100, false);
> + password = simple_prompt("Password: ", MAX_PASSWD, false);
> if (password == NULL)
> die_horribly(AH, modulename, "out of memory\n");
> }
> @@ -195,7 +195,7 @@ _connectDB(ArchiveHandle *AH, const char *reqdb, const char *requser)
> free(password);
>
> if (AH->promptPassword != TRI_NO)
> - password = simple_prompt("Password: ", 100, false);
> + password = simple_prompt("Password: ", MAX_PASSWD, false);
> else
> die_horribly(AH, modulename, "connection needs password\n");
>
> @@ -242,7 +242,7 @@ ConnectDatabase(Archive *AHX,
>
> if (prompt_password == TRI_YES && password == NULL)
> {
> - password = simple_prompt("Password: ", 100, false);
> + password = simple_prompt("Password: ", MAX_PASSWD, false);
> if (password == NULL)
> die_horribly(AH, modulename, "out of memory\n");
> }
> @@ -288,7 +288,7 @@ ConnectDatabase(Archive *AHX,
> prompt_password != TRI_NO)
> {
> PQfinish(AH->connection);
> - password = simple_prompt("Password: ", 100, false);
> + password = simple_prompt("Password: ", MAX_PASSWD, false);
> if (password == NULL)
> die_horribly(AH, modulename, "out of memory\n");
> new_pass = true;
> diff --git a/src/bin/pg_dump/pg_dumpall.c b/src/bin/pg_dump/pg_dumpall.c
> index 4c93667..496b131 100644
> --- a/src/bin/pg_dump/pg_dumpall.c
> +++ b/src/bin/pg_dump/pg_dumpall.c
> @@ -1687,7 +1687,7 @@ connectDatabase(const char *dbname, const char *pghost, const char *pgport,
> static char *password = NULL;
>
> if (prompt_password == TRI_YES && !password)
> - password = simple_prompt("Password: ", 100, false);
> + password = simple_prompt("Password: ", MAX_PASSWD, false);
>
> /*
> * Start the connection. Loop until we have a password if requested by
> @@ -1733,7 +1733,7 @@ connectDatabase(const char *dbname, const char *pghost, const char *pgport,
> prompt_password != TRI_NO)
> {
> PQfinish(conn);
> - password = simple_prompt("Password: ", 100, false);
> + password = simple_prompt("Password: ", MAX_PASSWD, false);
> new_pass = true;
> }
> } while (new_pass);
> diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
> index 8421ad0..4f347a4 100644
> --- a/src/bin/psql/command.c
> +++ b/src/bin/psql/command.c
> @@ -895,8 +895,8 @@ exec_command(const char *cmd,
> char *pw1;
> char *pw2;
>
> - pw1 = simple_prompt("Enter new password: ", 100, false);
> - pw2 = simple_prompt("Enter it again: ", 100, false);
> + pw1 = simple_prompt("Enter new password: ", MAX_PASSWD, false);
> + pw2 = simple_prompt("Enter it again: ", MAX_PASSWD, false);
>
> if (strcmp(pw1, pw2) != 0)
> {
> @@ -1462,7 +1462,7 @@ prompt_for_password(const char *username)
> char *result;
>
> if (username == NULL)
> - result = simple_prompt("Password: ", 100, false);
> + result = simple_prompt("Password: ", MAX_PASSWD, false);
> else
> {
> char *prompt_text;
> @@ -1470,7 +1470,7 @@ prompt_for_password(const char *username)
> prompt_text = malloc(strlen(username) + 100);
> snprintf(prompt_text, strlen(username) + 100,
> _("Password for user %s: "), username);
> - result = simple_prompt(prompt_text, 100, false);
> + result = simple_prompt(prompt_text, MAX_PASSWD, false);
> free(prompt_text);
> }
>
> diff --git a/src/bin/psql/startup.c b/src/bin/psql/startup.c
> index aff5772..eebbddc 100644
> --- a/src/bin/psql/startup.c
> +++ b/src/bin/psql/startup.c
> @@ -174,7 +174,7 @@ main(int argc, char *argv[])
> }
>
> if (pset.getPassword == TRI_YES)
> - password = simple_prompt(password_prompt, 100, false);
> + password = simple_prompt(password_prompt, MAX_PASSWD, false);
>
> /* loop until we have a password if requested by backend */
> do
> @@ -213,7 +213,7 @@ main(int argc, char *argv[])
> pset.getPassword != TRI_NO)
> {
> PQfinish(pset.db);
> - password = simple_prompt(password_prompt, 100, false);
> + password = simple_prompt(password_prompt, MAX_PASSWD, false);
> new_pass = true;
> }
> } while (new_pass);
> diff --git a/src/bin/scripts/common.c b/src/bin/scripts/common.c
> index 1a5284e..ebf4af2 100644
> --- a/src/bin/scripts/common.c
> +++ b/src/bin/scripts/common.c
> @@ -100,7 +100,7 @@ connectDatabase(const char *dbname, const char *pghost, const char *pgport,
> bool new_pass;
>
> if (prompt_password == TRI_YES)
> - password = simple_prompt("Password: ", 100, false);
> + password = simple_prompt("Password: ", MAX_PASSWD, false);
>
> /*
> * Start the connection. Loop until we have a password if requested by
> @@ -152,7 +152,7 @@ connectDatabase(const char *dbname, const char *pghost, const char *pgport,
> prompt_password != TRI_NO)
> {
> PQfinish(conn);
> - password = simple_prompt("Password: ", 100, false);
> + password = simple_prompt("Password: ", MAX_PASSWD, false);
> new_pass = true;
> }
> } while (new_pass);
> diff --git a/src/bin/scripts/createuser.c b/src/bin/scripts/createuser.c
> index 20a1a52..d9c3edb 100644
> --- a/src/bin/scripts/createuser.c
> +++ b/src/bin/scripts/createuser.c
> @@ -197,8 +197,8 @@ main(int argc, char *argv[])
> char *pw1,
> *pw2;
>
> - pw1 = simple_prompt("Enter password for new role: ", 100, false);
> - pw2 = simple_prompt("Enter it again: ", 100, false);
> + pw1 = simple_prompt("Enter password for new role: ", MAX_PASSWD, false);
> + pw2 = simple_prompt("Enter it again: ", MAX_PASSWD, false);
> if (strcmp(pw1, pw2) != 0)
> {
> fprintf(stderr, _("Passwords didn't match.\n"));
> diff --git a/src/include/pg_config_manual.h b/src/include/pg_config_manual.h
> index ac45ee6..4cef51f 100644
> --- a/src/include/pg_config_manual.h
> +++ b/src/include/pg_config_manual.h
> @@ -23,6 +23,20 @@
> #define NAMEDATALEN 64
>
> /*
> + * Maximum password length via command line tools
> + *
> + * If 0, no maximum password length is enforced.
> + * If greater than 0, this defines the maximum number of characters
> + * which will be read as input for a password prompt. Input in
> + * excess of this maximum will be silently ignored.
> + *
> + * The database itself does not have a password length limit,
> + * regardless of this setting.
> + *
> + */
> +#define MAX_PASSWD 0
> +
> +/*
> * Maximum number of arguments to a function.
> *
> * The minimum value is 8 (GIN indexes use 8-argument support functions).
> diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
> index 27a9805..a0f5ec9 100644
> --- a/src/interfaces/libpq/fe-connect.c
> +++ b/src/interfaces/libpq/fe-connect.c
> @@ -4905,22 +4905,31 @@ PasswordFromFile(char *hostname, char *port, char *dbname, char *username)
>
> while (!feof(fp) && !ferror(fp))
> {
> - char *t = buf,
> + char *t = calloc(1,sizeof(char)),
> *ret,
> *p1,
> *p2;
> int len;
>
> - if (fgets(buf, sizeof(buf), fp) == NULL)
> - break;
>
> - len = strlen(buf);
> + do
> + {
> + if ( fgets(buf, LINELEN, fp) == NULL)
> + break;
> + t = realloc(t, strlen(t)+1+strlen(buf));
> + /* Out of memory? */
> + if( !t )
> + return NULL;
> + strcat(t, buf);
> + len = strlen(t);
> + } while (strlen(buf) > 0 && t[len-1] != '\n');
> +
> if (len == 0)
> continue;
>
> /* Remove trailing newline */
> - if (buf[len - 1] == '\n')
> - buf[len - 1] = 0;
> + while ( len > 0 && (t[len-1] == '\n' || t[len-1] == '\r'))
> + t[--len] = 0;
>
> if ((t = pwdfMatchesString(t, hostname)) == NULL ||
> (t = pwdfMatchesString(t, port)) == NULL ||
> diff --git a/src/port/sprompt.c b/src/port/sprompt.c
> index 7baa26e..aafec28 100644
> --- a/src/port/sprompt.c
> +++ b/src/port/sprompt.c
> @@ -38,7 +38,10 @@ char *
> simple_prompt(const char *prompt, int maxlen, bool echo)
> {
> int length;
> + int buflen;
> + int bufsize = 1024;
> char *destination;
> + char buf[bufsize];
> FILE *termin,
> *termout;
>
> @@ -52,7 +55,11 @@ simple_prompt(const char *prompt, int maxlen, bool echo)
> #endif
> #endif
>
> - destination = (char *) malloc(maxlen + 1);
> + if (maxlen > 0) {
> + destination = (char *) calloc(1, sizeof(char));
> + } else {
> + destination = (char *) malloc((maxlen + 1) * sizeof(char));
> + }
> if (!destination)
> return NULL;
>
> @@ -108,21 +115,34 @@ simple_prompt(const char *prompt, int maxlen, bool echo)
> fflush(termout);
> }
>
> - if (fgets(destination, maxlen + 1, termin) == NULL)
> - destination[0] = '\0';
> -
> - length = strlen(destination);
> - if (length > 0 && destination[length - 1] != '\n')
> - {
> - /* eat rest of the line */
> - char buf[128];
> - int buflen;
> + if (maxlen > 0) {
> + if (fgets(destination, maxlen + 1, termin) == NULL)
> + destination[0] = '\0';
>
> + length = strlen(destination);
> + if (length > 0 && destination[length - 1] != '\n')
> + {
> + /* eat rest of the line */
> + do
> + {
> + if (fgets(buf, bufsize, termin) == NULL)
> + break;
> + buflen = strlen(buf);
> + } while (buflen > 0 && buf[buflen - 1] != '\n');
> + }
> +
> + } else {
> do
> {
> - if (fgets(buf, sizeof(buf), termin) == NULL)
> + if (fgets(buf, bufsize, termin) == NULL)
> break;
> buflen = strlen(buf);
> + destination = realloc( destination, strlen(destination)+1+buflen );
> + /* Out of memory ? */
> + if( !destination )
> + return NULL;
> + strcat( destination, buf );
> + length = strlen(destination);
> } while (buflen > 0 && buf[buflen - 1] != '\n');
> }
>

>
> --
> Sent via pgsql-bugs mailing list (pgsql-bugs(at)postgresql(dot)org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-bugs

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ It's impossible for everything to be true. +

In response to

Responses

Browse pgsql-bugs by date

  From Date Subject
Next Message Alvaro Herrera 2012-08-27 16:33:15 Re: BUG #6412: psql & fe-connect truncate passwords
Previous Message iann 2012-08-27 15:34:10 BUG #7507: pg_restore silently fails when restoring a db with the --create flag and no user.