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

The following bug has been logged on the website:

Bug reference: 6412
Logged by: Andy Grimm
Email address: agrimm(at)gmail(dot)com
PostgreSQL version: 9.1.2
Operating system: Linux (Fedora)
Description:

When psql prompts for a password, it only reads the first 100 characters of
the password. The limit in fe-connect.c (for when .pgpass is used) is
weirder, a seemingly arbitrary 320 bytes for all fields combined. Other
(postgresql-jdbc, PyGreSQL, etc.) have no problem with a 512-byte password.
It would be nice to have these limits controlled by a constant, and for the
command to give an error or warning when a password is truncated.

On 27-01-2012 23:15, agrimm(at)gmail(dot)com wrote:
> When psql prompts for a password, it only reads the first 100 characters of
> the password. The limit in fe-connect.c (for when .pgpass is used) is
> weirder, a seemingly arbitrary 320 bytes for all fields combined. Other
> (postgresql-jdbc, PyGreSQL, etc.) have no problem with a 512-byte password.
> It would be nice to have these limits controlled by a constant, and for the
> command to give an error or warning when a password is truncated.
>
I don't see it as a bug but a limitation. Why do you need such a long
password? If you are not comfortable with this reasonable limit, look at
fe-connect.c -> PasswordFromFile() and change the LINELEN. More to the point,
AFAICS all of the PostgreSQL client prompts are limited to 100 bytes (look at
simple_prompt function); letting 220 bytes for host, port, database, and user.

--
Euler Taveira de Oliveira - Timbira http://www.timbira.com.br/
PostgreSQL: Consultoria, Desenvolvimento, Suporte 24x7 e Treinamento

On Sat, Jan 28, 2012 at 11:55 AM, Euler Taveira de Oliveira
<euler(at)timbira(dot)com> wrote:
> On 27-01-2012 23:15, agrimm(at)gmail(dot)com wrote:
>> When psql prompts for a password, it only reads the first 100 characters of
>> the password.  The limit in fe-connect.c (for when .pgpass is used) is
>> weirder, a seemingly arbitrary 320 bytes for all fields combined.  Other
>> (postgresql-jdbc, PyGreSQL, etc.) have no problem with a 512-byte password.
>> It would be nice to have these limits controlled by a constant, and for the
>> command to give an error or warning when a password is truncated.
>>
> I don't see it as a bug but a limitation.

First, thank you for the quick response.

IMHO, there is a subtle difference here. If psql raised an error
message on passwords exceeding 100 characters, I would understand your
perspective, but I think that simply truncating the password and
continuing on is a bug. I also think that hard-coding the number
"100" in several places is simply poor practice which should be
corrected, and that if there's good reason for that to be the password
length limit, it should be uniformly enforced.

Regardless, of whether it's a bug or feature, though, the fixes are
trivial, so I'm not sure what a strong argument _against_ the changes
would be.

>Why do you need such a long
> password?

The password is not of my choosing. It's an autogenerated sha hash of
an RSA key, and i've simply been the key to use.
While I agree that it's generally impractical to use such a long
password at the command line, more than 99% of the use of this
password is programmatic, and if I complain to the author that the
password is too long, he'll respond "it works for me with JDBC; you
are using broken tools.

> If you are not comfortable with this reasonable limit, look at
> fe-connect.c -> PasswordFromFile() and change the LINELEN. More to the point,
> AFAICS all of the PostgreSQL client prompts are limited to 100 bytes (look at
> simple_prompt function); letting 220 bytes for host, port, database, and user.

I looked at the code before I wrote up the issue, and I have written
and tested a patch. I've posted it here:

https://bugzilla.redhat.com/attachment.cgi?id=558061

As you might expect, it simply defines a constant called PASSWDLEN and
uses that in the calls to simple_prompt, as well as in initdb's
reading of pwfile (which inexplicably uses MAXPGPATH as the maximum
password length today).

Perhaps I should just submit the patch to pgsql-hackers ? I'm new to
the pgsql bug interaction process, so my apologies if filing a bug was
not the appropriate way to present the issue. I get Internal Server
Error messages when I attempt to subscribe to any of the pgsql mailing
lists, so this makes communication with the lists difficult.

--Andy

> --
>   Euler Taveira de Oliveira - Timbira       http://www.timbira.com.br/
>   PostgreSQL: Consultoria, Desenvolvimento, Suporte 24x7 e Treinamento

Euler Taveira de Oliveira <euler(at)timbira(dot)com> writes:
> I don't see it as a bug but a limitation. Why do you need such a long
> password?

Yeah, I think the reason we're not too consistent about this is that
nobody ever imagined that limits of 100 bytes or more would pose an
issue in practice. What's the use-case for passwords longer than
that?

regards, tom lane

On 28-01-2012 14:32, Andy Grimm wrote:
> IMHO, there is a subtle difference here. If psql raised an error
> message on passwords exceeding 100 characters, I would understand your
> perspective, but I think that simply truncating the password and
> continuing on is a bug. I also think that hard-coding the number
> "100" in several places is simply poor practice which should be
> corrected, and that if there's good reason for that to be the password
> length limit, it should be uniformly enforced.
>
It is uniform on all of the bundled client tools. The source can always be
improved; such a constant is one of those improvements.

> The password is not of my choosing. It's an autogenerated sha hash of
> an RSA key, and i've simply been the key to use.
> While I agree that it's generally impractical to use such a long
> password at the command line, more than 99% of the use of this
> password is programmatic, and if I complain to the author that the
> password is too long, he'll respond "it works for me with JDBC; you
> are using broken tools.
>
So the "broken" part is the password file, right? I won't expect someone with
such a long password typing or (of course) copy/paste it, will I? Again,
patches are welcome.

> I looked at the code before I wrote up the issue, and I have written
> and tested a patch. I've posted it here:
>
> https://bugzilla.redhat.com/attachment.cgi?id=558061
>
Please, post a patch here, we don't follow other bug trackers.

> Perhaps I should just submit the patch to pgsql-hackers ? I'm new to
> the pgsql bug interaction process, so my apologies if filing a bug was
> not the appropriate way to present the issue. I get Internal Server
> Error messages when I attempt to subscribe to any of the pgsql mailing
> lists, so this makes communication with the lists difficult.
>
Bugs are tracked here but when it is not a bug but an improvement, we just
redirect this thread to -hackers.

--
Euler Taveira de Oliveira - Timbira http://www.timbira.com.br/
PostgreSQL: Consultoria, Desenvolvimento, Suporte 24x7 e Treinamento

On Sat, Jan 28, 2012 at 1:50 PM, Euler Taveira de Oliveira
<euler(at)timbira(dot)com> wrote:
> On 28-01-2012 14:32, Andy Grimm wrote:
>> IMHO, there is a subtle difference here.  If psql raised an error
>> message on passwords exceeding 100 characters, I would understand your
>> perspective, but I think that simply truncating the password and
>> continuing on is a bug.  I also think that hard-coding the number
>> "100" in several places is simply poor practice which should be
>> corrected, and that if there's good reason for that to be the password
>> length limit, it should be uniformly enforced.
>>
> It is uniform on all of the bundled client tools. The source can always be
> improved; such a constant is one of those improvements.

It's not uniform between the client and the server, though. The
server will allow passwords to be set which essentially make the
bundled client tools useless. Even if pwfile enforced a shorter
password, you'd still be able to set a long password with "alter user
...", and I have no idea what the limit is via that mechanism.

>> The password is not of my choosing.  It's an autogenerated sha hash of
>> an RSA key, and i've simply been the key to use.
>> While I agree that it's generally impractical to use such a long
>> password at the command line, more than 99% of the use of this
>> password is programmatic, and if I complain to the author that the
>> password is too long, he'll respond "it works for me with JDBC; you
>> are using broken tools.
>>
> So the "broken" part is the password file, right? I won't expect someone with
> such a long password typing or (of course) copy/paste it, will I? Again,
> patches are welcome.

I guess our opinions differ here. 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.

>> I looked at the code before I wrote up the issue, and I have written
>> and tested a patch.  I've posted it here:
>>
>> https://bugzilla.redhat.com/attachment.cgi?id=558061
>>
> Please, post a patch here, we don't follow other bug trackers.

A patch is attached. I must confess that it doesn't solve the
underlying problem, but merely "scratches my itch", so to speak. I
simply created a constant set to 1024, since that's what initdb's
pwfile option had already allowed you to set, and did not tackle the
issue of what happens when the password is truncated. Dealing with
truncation properly will be less trivial than this patch, and will
require understanding of what the real password length limit in a
database is.

>> Perhaps I should just submit the patch to pgsql-hackers ?  I'm new to
>> the pgsql bug interaction process, so my apologies if filing a bug was
>> not the appropriate way to present the issue.  I get Internal Server
>> Error messages when I attempt to subscribe to any of the pgsql mailing
>> lists, so this makes communication with the lists difficult.
>>
> Bugs are tracked here but when it is not a bug but an improvement, we just
> redirect this thread to -hackers.

Ok, thanks for explaining.

--Andy

>
> --
>   Euler Taveira de Oliveira - Timbira       http://www.timbira.com.br/
>   PostgreSQL: Consultoria, Desenvolvimento, Suporte 24x7 e Treinamento

Excerpts from Andy Grimm's message of sáb ene 28 14:32:24 -0300 2012:

> Perhaps I should just submit the patch to pgsql-hackers ? I'm new to
> the pgsql bug interaction process, so my apologies if filing a bug was
> not the appropriate way to present the issue. I get Internal Server
> Error messages when I attempt to subscribe to any of the pgsql mailing
> lists, so this makes communication with the lists difficult.

Err, it's not the first time I hear about this, but I haven't been able
to detect a problem anywhere. Exactly how are you trying to subscribe?

--
Álvaro Herrera <alvherre(at)commandprompt(dot)com>
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

On Sat, Jan 28, 2012 at 5:48 PM, Alvaro Herrera
<alvherre(at)commandprompt(dot)com> wrote:
>
> Excerpts from Andy Grimm's message of sáb ene 28 14:32:24 -0300 2012:
>
>> Perhaps I should just submit the patch to pgsql-hackers ?  I'm new to
>> the pgsql bug interaction process, so my apologies if filing a bug was
>> not the appropriate way to present the issue.  I get Internal Server
>> Error messages when I attempt to subscribe to any of the pgsql mailing
>> lists, so this makes communication with the lists difficult.
>
> Err, it's not the first time I hear about this, but I haven't been able
> to detect a problem anywhere.  Exactly how are you trying to subscribe?

Thanks for the concern, Alvaro. As it turns out, despite the 500 ISE
message returned in my browser, I did receive a confirmation email and
was able to subscribe to this list, so it's not entirely broken. As
for how I tried to subscribe, I simply went to
http://www.postgresql.org/mailpref/pgsql-bugs (which is the link
provided at http://archives.postgresql.org/pgsql-bugs/ ).

--Andy

> --
> Álvaro Herrera <alvherre(at)commandprompt(dot)com>
> The PostgreSQL Company - Command Prompt, Inc.
> PostgreSQL Replication, Consulting, Custom Development, 24x7 support

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).

> 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

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

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. +

Excerpts from Bruce Momjian's message of lun ago 27 12:12:25 -0400 2012:
>
> Did we want this patch applied? Not enough demand?

I think it should be in the next commitfest for discussion. I don't see
any reason to reject it. I think it needs some fixes, though, so a
formal review process is called for.

--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

Bruce Momjian <bruce(at)momjian(dot)us> writes:
> Did we want this patch applied? Not enough demand?

It seems a bit overengineered at this point. I realize Andy wasn't the
one pushing to support arbitrary-length passwords originally, but I
can't see adding this much code for that. I don't even see the value
of allowing them to be more than 100 characters ...

regards, tom lane

On Sat, Jan 28, 2012 at 01:47:04PM -0500, Tom Lane wrote:
> Euler Taveira de Oliveira <euler(at)timbira(dot)com> writes:
> > I don't see it as a bug but a limitation. Why do you need such a long
> > password?
>
> Yeah, I think the reason we're not too consistent about this is that
> nobody ever imagined that limits of 100 bytes or more would pose an
> issue in practice. What's the use-case for passwords longer than
> that?

Thanks for all the feedback. I know it is a pain for me to re-ask these
questions, but it allows us to know that these issues have gotten
sufficient thought.

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

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

On Mon, Aug 27, 2012 at 12:33 PM, Alvaro Herrera
<alvherre(at)2ndquadrant(dot)com> wrote:
> Excerpts from Bruce Momjian's message of lun ago 27 12:12:25 -0400 2012:
>>
>> Did we want this patch applied? Not enough demand?
>
> I think it should be in the next commitfest for discussion. I don't see
> any reason to reject it. I think it needs some fixes, though, so a
> formal review process is called for.

+1. I added it.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On 15.02.2012 07:09, Andy Grimm wrote:
> 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.

Thanks for the patch. A few comments:

* Most of the simple_prompt() calls are for passwords, which now have no
limit, but there's a few others. How about we remove the maxlen argument
altogether, and just have it always return a malloc'd string that can be
arbitrarily long. (maybe with a sanity-check limit within
simple_prompt(), like 100k)

* .pg_service.conf handling still has a fixed limit on line length of
256 bytes. See parseServiceInfo() in fe-connect. I think we should lift
that limit too, for the sake of consistency. You can pass a password in
the service file, too.

* Missed a few simple_prompt() calls in contrib (oid2name, vacuumlo,
pgbench)

- Heikki

Andy Grimm escribió:

> 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

This patch is in an awkward position. It was submitted back in February
and then ignored for months. It got added by Robert into this
commitfest, but the submitter hasn't gotten involved at all. Then
Heikki posted a review a month ao and pointed out a number of issues
which are still waiting to be fixed. If this patch is something we
want, then perhaps somebody else should grab it, fix the issues, and
resubmit to the next commitfest.

If we don't want it (and some have suggested) then we just quietly let
the thread die here.

I am marking it Returned with Feedback for now.

--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services