Re: BUG #6412: psql & fe-connect truncate passwords - Mailing list 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@timbira.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@postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-bugs


--
  Bruce Momjian  <bruce@momjian.us>        http://momjian.us
  EnterpriseDB                             http://enterprisedb.com

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