Re: Increase psql's password buffer size

2020-02-19 Thread Tom Lane
Fujii Masao  writes:
> 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?

I can't get excited about this in the least.  For any "normal" use of
passwords, 100 bytes is surely far more than sufficient.  Furthermore,
if there is someone out there for whom it isn't sufficient, they're not
going to want to build custom versions of Postgres to change it.

If we think that longer passwords are actually a thing to be concerned
about, then what we need to do is change all these places to support
expansible buffers.  I'm not taking a position on whether that's worth
the trouble ... but I do take the position that just inserting a
#define is a waste of time.

regards, tom lane




Re: Increase psql's password buffer size

2020-02-19 Thread Fujii Masao



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 
Date: Fri, 12 Oct 2018 15:31:31 +
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;
boolhave_password = false;
-   charpassword[100];
+   charpassword[PROMPT_MAX_PASSWORD_LENGTH];
boolnew_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)
boolnew_pass;
boolsuccess = 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)
 {
-   charpwd1[100];
-   charpwd2[100];
+   charpwd1[PROMPT_MAX_PASSWORD_LENGTH];
+   charpwd2[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;
 intdbgetpassword = 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;
-   charpassbuf[100];
+   charpassbuf[PROMPT_MAX_PASSWORD_LENGTH];
boolnew_pass;
 
if (!reqdb)
@@ -246,7 +246,7 @@ ConnectDatabase(Archive *AHX,
 {
ArchiveHandle *AH = (ArchiveHandle *) AHX;
char   *password;
-   charpassbuf[100];
+   charpassbuf[PROMPT_MAX_PASSWORD_LENGTH];
boolnew_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 

Re: Increase psql's password buffer size

2020-01-22 Thread Daniel Gustafsson
> On 22 Jan 2020, at 01:41, David Fetter  wrote:
> 
> On Tue, Jan 21, 2020 at 07:05:47PM +0100, David Fetter wrote:
>> On Tue, Jan 21, 2020 at 10:23:59AM -0500, Bruce Momjian wrote:
>>> On Tue, Jan 21, 2020 at 04:19:13PM +0100, David Fetter wrote:
 On Tue, Jan 21, 2020 at 10:12:52AM -0500, Bruce Momjian wrote:
> I think we should be using a macro to define the maximum length, rather
> than have 100 used in various places.
 
 It's not just 100 in some places. It's different in different places,
 which goes to your point.
 
 How about using a system that doesn't meaningfully impose a maximum
 length? The shell variable is a const char *, so why not just
 re(p)alloc as needed?
>>> 
>>> Uh, how do you know how big to make the buffer that receives the read?
>> 
>> You can start at any size, possibly even 100, and then increase the
>> size in a loop along the lines of (untested)

It doesn't seem like a terribly safe pattern to have the client decide the read
buffer without disclosing the size, and have the server resize the input buffer
to an arbitrary size as input comes in.

>   my_size *= 2;
>   buf = (char *) realloc(buf, my_size);

I know it's just example code, but using buf as the input to realloc like this
risks a memleak when realloc fails as the original buf pointer is overwritten.
Using a temporary pointer for ther returnvalue avoids that, which is how
pg_repalloc and repalloc does it.

cheers ./daniel



Re: Increase psql's password buffer size

2020-01-21 Thread Fujii Masao




On 2020/01/22 9:41, David Fetter wrote:

On Tue, Jan 21, 2020 at 07:05:47PM +0100, David Fetter wrote:

On Tue, Jan 21, 2020 at 10:23:59AM -0500, Bruce Momjian wrote:

On Tue, Jan 21, 2020 at 04:19:13PM +0100, David Fetter wrote:

On Tue, Jan 21, 2020 at 10:12:52AM -0500, Bruce Momjian wrote:

I think we should be using a macro to define the maximum length, rather
than have 100 used in various places.


It's not just 100 in some places. It's different in different places,
which goes to your point.

How about using a system that doesn't meaningfully impose a maximum
length? The shell variable is a const char *, so why not just
re(p)alloc as needed?


Uh, how do you know how big to make the buffer that receives the read?


You can start at any size, possibly even 100, and then increase the
size in a loop along the lines of (untested)


That's possible, but I like having the (reasonable) upper limit on that
rather than arbitrary size.

Regards,

--
Fujii Masao
NTT DATA CORPORATION
Advanced Platform Technology Group
Research and Development Headquarters




Re: Increase psql's password buffer size

2020-01-21 Thread Fujii Masao




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.

Regards,

--
Fujii Masao
NTT DATA CORPORATION
Advanced Platform Technology Group
Research and Development Headquarters




Re: Increase psql's password buffer size

2020-01-21 Thread David Fetter
On Tue, Jan 21, 2020 at 07:05:47PM +0100, David Fetter wrote:
> On Tue, Jan 21, 2020 at 10:23:59AM -0500, Bruce Momjian wrote:
> > On Tue, Jan 21, 2020 at 04:19:13PM +0100, David Fetter wrote:
> > > On Tue, Jan 21, 2020 at 10:12:52AM -0500, Bruce Momjian wrote:
> > > > I think we should be using a macro to define the maximum length, rather
> > > > than have 100 used in various places.
> > > 
> > > It's not just 100 in some places. It's different in different places,
> > > which goes to your point.
> > > 
> > > How about using a system that doesn't meaningfully impose a maximum
> > > length? The shell variable is a const char *, so why not just
> > > re(p)alloc as needed?
> > 
> > Uh, how do you know how big to make the buffer that receives the read?
> 
> You can start at any size, possibly even 100, and then increase the
> size in a loop along the lines of (untested)

[and unworkable]

I should have tested the code, but my point about using rep?alloc()
remains.

Best,
David.

Working code:

int main(int argc, char **argv)
{
size_t my_size = 2,
   curr_size = 0;
char *buf;
int c;

buf = (char *) malloc(my_size);

printf("Enter a nice, long string.\n");

while( (c = getchar()) != '\0' )
{
buf[curr_size++] = c;
if (curr_size == my_size)
{
my_size *= 2;
buf = (char *) realloc(buf, my_size);
}
}
printf("The string %s is %zu bytes long.\n", buf, curr_size);
}
-- 
David Fetter  http://fetter.org/
Phone: +1 415 235 3778

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate




Re: Increase psql's password buffer size

2020-01-21 Thread David Fetter
On Tue, Jan 21, 2020 at 10:23:59AM -0500, Bruce Momjian wrote:
> On Tue, Jan 21, 2020 at 04:19:13PM +0100, David Fetter wrote:
> > On Tue, Jan 21, 2020 at 10:12:52AM -0500, Bruce Momjian wrote:
> > > I think we should be using a macro to define the maximum length, rather
> > > than have 100 used in various places.
> > 
> > It's not just 100 in some places. It's different in different places,
> > which goes to your point.
> > 
> > How about using a system that doesn't meaningfully impose a maximum
> > length? The shell variable is a const char *, so why not just
> > re(p)alloc as needed?
> 
> Uh, how do you know how big to make the buffer that receives the read?

You can start at any size, possibly even 100, and then increase the
size in a loop along the lines of (untested)

my_size = 100;
my_buf = char[my_size];
curr_size = 0;
while (c = getchar() != '\0')
{
my_buf[curr_size++] = c;
if (curr_size == my_size) /* If we want an absolute maximum,
 this'd be the place to test for it.
 */
{
my_size *= 2;
repalloc(my_buf, my_size);
}
}

Best,
David.
-- 
David Fetter  http://fetter.org/
Phone: +1 415 235 3778

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate




Re: Increase psql's password buffer size

2020-01-21 Thread Bruce Momjian
On Tue, Jan 21, 2020 at 04:19:13PM +0100, David Fetter wrote:
> On Tue, Jan 21, 2020 at 10:12:52AM -0500, Bruce Momjian wrote:
> > I think we should be using a macro to define the maximum length, rather
> > than have 100 used in various places.
> 
> It's not just 100 in some places. It's different in different places,
> which goes to your point.
> 
> How about using a system that doesn't meaningfully impose a maximum
> length? The shell variable is a const char *, so why not just
> re(p)alloc as needed?

Uh, how do you know how big to make the buffer that receives the read?

-- 
  Bruce Momjian  http://momjian.us
  EnterpriseDB http://enterprisedb.com

+ As you are, so once was I.  As I am, so you will be. +
+  Ancient Roman grave inscription +




Re: Increase psql's password buffer size

2020-01-21 Thread David Fetter
On Tue, Jan 21, 2020 at 10:12:52AM -0500, 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.

It's not just 100 in some places. It's different in different places,
which goes to your point.

How about using a system that doesn't meaningfully impose a maximum
length? The shell variable is a const char *, so why not just
re(p)alloc as needed?

Best,
David.
-- 
David Fetter  http://fetter.org/
Phone: +1 415 235 3778

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate




Re: Increase psql's password buffer size

2020-01-21 Thread Bruce Momjian
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.

-- 
  Bruce Momjian  http://momjian.us
  EnterpriseDB http://enterprisedb.com

+ As you are, so once was I.  As I am, so you will be. +
+  Ancient Roman grave inscription +




Re: Increase psql's password buffer size

2020-01-20 Thread Fujii Masao




On 2020/01/21 4:21, David Fetter wrote:

On Mon, Jan 20, 2020 at 07:44:25PM +0100, David Fetter wrote:

On Mon, Jan 20, 2020 at 01:12:35PM -0500, Tom Lane wrote:

David Fetter  writes:

At least two cloud providers are now stuffing large amounts of
information into the password field. This change makes it possible to
accommodate that usage in interactive sessions.


Like who?


AWS and Azure are two examples I know of.


It seems like a completely silly idea.  And if 2K is sane, why not
much more?


Good question. Does it make sense to rearrange these things so they're
allocated at runtime instead of compile time?


(I can't say that s/100/2048/ in one place is a particularly evil
change; what bothers me is the likelihood that there are other
places that won't cope with arbitrarily long passwords.  Not all of
them are necessarily under our control, either.)


I found one that is, so please find attached the next revision of the
patch.


I found another place that assumes 100 bytes and upped it to 2048.


There are other places that 100 bytes password length is assumed.
It's better to check the 0001 patch that posted in the following thread.
https://www.postgresql.org/message-id/09512c4f-8cb9-4021-b455-ef4c4f0d5...@amazon.com

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.

Regards,

--
Fujii Masao
NTT DATA CORPORATION
Advanced Platform Technology Group
Research and Development Headquarters




Re: Increase psql's password buffer size

2020-01-20 Thread David Fetter
On Mon, Jan 20, 2020 at 09:17:47PM +0100, Alexander Kukushkin wrote:
> Hi,
> 
> I think I should add my two cents.
> 
> On Mon, 20 Jan 2020 at 20:38, Tom Lane  wrote:
> >
> > > I found another place that assumes 100 bytes and upped it to 2048.
> 
> There one more place, in the code which is parsing .pgpass

What I found that seems like it might be related was on line 6900 of
src/interfaces/libpq/fe-connect.c (passwordFromFile):

#define LINELEN NAMEDATALEN*5

which is 315 (63*5) by default and isn't 100 on any sane setup. What
did I miss?

In any case, having the lengths be different in different places
seems sub-optimal. PGPASSWORD is just a const char *, so could be
quite long. The password prompted for by psql can be up to 100 bytes,
and the one read from .pgpass is bounded from above by 

315
- 4 (colons)
- 4 (shortest possible hostname)
- 4 (usual port number)
- 1 (shortest db name)
- 1 (shortest possible username)
---
301

> > So this is pretty much exactly what I expected.  And have you tried
> > it with e.g. PAM, or LDAP?
> >
> > I think the AWS guys are fools to imagine that this will work in very
> > many places, and I don't see why we should be leading the charge to
> > make it work for them.  What's the point of having a huge amount of
> > data in a password, anyway?
> 
> We at Zalando are using JWT tokens as passwords. JWT tokens are
> self-contained and therefore quite huge (up to 700-800 bytes in our
> case). Tokens have a limited lifetime (1 hour) and we are using PAM to
> verify them.
> Altogether the whole thing works like a charm. The only problem that
> it is not possible to copy the token into psql password prompt,
> but there is a workaround, export PGPASSWORD=verylongtokenstring &&
> psql
> 
> JWT: https://jwt.io/
> PAM module to verify OAuth tokens: https://github.com/CyberDem0n/pam-oauth2

This reminds me of a patch that implemented PGPASSCOMMAND.
https://www.postgresql.org/message-id/flat/CAE35ztOGZqgwae3mBA%3DL97pSg3kvin2xycQh%3Dir%3D5NiwCApiYQ%40mail.gmail.com

Discussion of that seems to have trailed off, though. My thought on
that was that it was making a decision about the presence of both a
.pgpass file and a PGPASSCOMMAND setting that it shouldn't have made,
i.e. it decided which took precedence.  I think it should fail when
presented with both, as there's not a single right answer that will
cover all cases.

Best,
David.
-- 
David Fetter  http://fetter.org/
Phone: +1 415 235 3778

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate




Re: Increase psql's password buffer size

2020-01-20 Thread Tom Lane
Alexander Kukushkin  writes:
> I think I should add my two cents.
> We at Zalando are using JWT tokens as passwords. JWT tokens are
> self-contained and therefore quite huge (up to 700-800 bytes in our
> case). Tokens have a limited lifetime (1 hour) and we are using PAM to
> verify them.
> Altogether the whole thing works like a charm. The only problem that
> it is not possible to copy the token into psql password prompt,
> but there is a workaround, export PGPASSWORD=verylongtokenstring &&
> psql

I remain unconvinced that this is a good design, as compared to the
alternative of hashing $large_secret_data down to a more typical
length for a password.

Quite aside from whether or not you run into any implementation
restrictions on password length, using externally-sourced secret
data directly as a password seems like a lousy idea from a pure
security standpoint.  What happens if somebody compromises your
database, or even just your connection to the database, and is
able to read out the raw password?  The damage is worse than the
ordinary case of just being able to get into your database account,
because now the attacker has info about a formerly-secure upstream
datum, which probably lets him into some other things.  It's not
unlike using the same password across multiple services.

In the case you describe, you're also exposing that data to wherever
the PAM mechanism is keeping its secrets, hence presenting an even
larger attack surface.  Hashing the data before it goes to any of
those places would go a long way towards mitigating the risk.

regards, tom lane




Re: Increase psql's password buffer size

2020-01-20 Thread Alexander Kukushkin
Hi,

I think I should add my two cents.

On Mon, 20 Jan 2020 at 20:38, Tom Lane  wrote:
>
> > I found another place that assumes 100 bytes and upped it to 2048.

There one more place, in the code which is parsing .pgpass

>
> So this is pretty much exactly what I expected.  And have you tried
> it with e.g. PAM, or LDAP?
>
> I think the AWS guys are fools to imagine that this will work in very
> many places, and I don't see why we should be leading the charge to
> make it work for them.  What's the point of having a huge amount of
> data in a password, anyway?

We at Zalando are using JWT tokens as passwords. JWT tokens are
self-contained and therefore quite huge (up to 700-800 bytes in our
case). Tokens have a limited lifetime (1 hour) and we are using PAM to
verify them.
Altogether the whole thing works like a charm. The only problem that
it is not possible to copy the token into psql password prompt,
but there is a workaround, export PGPASSWORD=verylongtokenstring &&
psql

JWT: https://jwt.io/
PAM module to verify OAuth tokens: https://github.com/CyberDem0n/pam-oauth2

Regards,
--
Alexander Kukushkin




Re: Increase psql's password buffer size

2020-01-20 Thread Tom Lane
David Fetter  writes:
> On Mon, Jan 20, 2020 at 07:44:25PM +0100, David Fetter wrote:
>> On Mon, Jan 20, 2020 at 01:12:35PM -0500, Tom Lane wrote:
>>> (I can't say that s/100/2048/ in one place is a particularly evil
>>> change; what bothers me is the likelihood that there are other
>>> places that won't cope with arbitrarily long passwords.  Not all of
>>> them are necessarily under our control, either.)

>> I found one that is, so please find attached the next revision of the
>> patch.

> I found another place that assumes 100 bytes and upped it to 2048.

So this is pretty much exactly what I expected.  And have you tried
it with e.g. PAM, or LDAP?

I think the AWS guys are fools to imagine that this will work in very
many places, and I don't see why we should be leading the charge to
make it work for them.  What's the point of having a huge amount of
data in a password, anyway?  You can't expect to get it back out
again, and there's no reason to believe that it adds any security
after a certain point.  If they want a bunch of different things
contributing to the password, OK, but they could just hash those
things together and thereby keep their submitted password to a length
that will work with most services.

regards, tom lane




Re: Increase psql's password buffer size

2020-01-20 Thread David Fetter
On Mon, Jan 20, 2020 at 07:44:25PM +0100, David Fetter wrote:
> On Mon, Jan 20, 2020 at 01:12:35PM -0500, Tom Lane wrote:
> > David Fetter  writes:
> > > At least two cloud providers are now stuffing large amounts of
> > > information into the password field. This change makes it possible to
> > > accommodate that usage in interactive sessions.
> > 
> > Like who?
> 
> AWS and Azure are two examples I know of.
> 
> > It seems like a completely silly idea.  And if 2K is sane, why not
> > much more?
> 
> Good question. Does it make sense to rearrange these things so they're
> allocated at runtime instead of compile time?
> 
> > (I can't say that s/100/2048/ in one place is a particularly evil
> > change; what bothers me is the likelihood that there are other
> > places that won't cope with arbitrarily long passwords.  Not all of
> > them are necessarily under our control, either.)
> 
> I found one that is, so please find attached the next revision of the
> patch.

I found another place that assumes 100 bytes and upped it to 2048.

Best,
David.
-- 
David Fetter  http://fetter.org/
Phone: +1 415 235 3778

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate
>From fb05bf709df0a67a63bca413cd7f0f276cab78b9 Mon Sep 17 00:00:00 2001
From: David Fetter 
Date: Mon, 20 Jan 2020 09:58:19 -0800
Subject: [PATCH v3] Increase psql's password buffer size
To: hackers
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="2.24.1"

This is a multi-part message in MIME format.
--2.24.1
Content-Type: text/plain; charset=UTF-8; format=fixed
Content-Transfer-Encoding: 8bit


At least two cloud providers are now stuffing large amounts of
information into the password field. This makes it possible to
accommodate that usage in interactive sessions.

diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index e111cee556..61386fe4ae 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -1827,8 +1827,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[2048];
+		char		pw2[2048];
 
 		simple_prompt("Enter new password: ", pw1, sizeof(pw1), false);
 		simple_prompt("Enter it again: ", pw2, sizeof(pw2), false);
@@ -2845,7 +2845,7 @@ copy_previous_query(PQExpBuffer query_buf, PQExpBuffer previous_buf)
 static char *
 prompt_for_password(const char *username)
 {
-	char		buf[100];
+	char		buf[2048];
 
 	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 3302bd4dd3..a7e3263979 100644
--- a/src/bin/psql/startup.c
+++ b/src/bin/psql/startup.c
@@ -120,7 +120,7 @@ main(int argc, char *argv[])
 	struct adhoc_opts options;
 	int			successResult;
 	bool		have_password = false;
-	char		password[100];
+	char		password[2048];
 	bool		new_pass;
 
 	pg_logging_init(argv[0]);

--2.24.1--




Re: Increase psql's password buffer size

2020-01-20 Thread David Fetter
On Mon, Jan 20, 2020 at 01:12:35PM -0500, Tom Lane wrote:
> David Fetter  writes:
> > At least two cloud providers are now stuffing large amounts of
> > information into the password field. This change makes it possible to
> > accommodate that usage in interactive sessions.
> 
> Like who?

AWS and Azure are two examples I know of.

> It seems like a completely silly idea.  And if 2K is sane, why not
> much more?

Good question. Does it make sense to rearrange these things so they're
allocated at runtime instead of compile time?

> (I can't say that s/100/2048/ in one place is a particularly evil
> change; what bothers me is the likelihood that there are other
> places that won't cope with arbitrarily long passwords.  Not all of
> them are necessarily under our control, either.)

I found one that is, so please find attached the next revision of the
patch.

Best,
David.
-- 
David Fetter  http://fetter.org/
Phone: +1 415 235 3778

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate
>From dfe72e1f7b3af646ba3d0bff017c9574eb54eb4c Mon Sep 17 00:00:00 2001
From: David Fetter 
Date: Mon, 20 Jan 2020 09:58:19 -0800
Subject: [PATCH v2] Increase psql's password buffer size
To: hackers
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="2.24.1"

This is a multi-part message in MIME format.
--2.24.1
Content-Type: text/plain; charset=UTF-8; format=fixed
Content-Transfer-Encoding: 8bit


At least two cloud providers are now stuffing large amounts of
information into the password field. This makes it possible to
accommodate that usage in interactive sessions.

diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index e111cee556..61386fe4ae 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -1827,8 +1827,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[2048];
+		char		pw2[2048];
 
 		simple_prompt("Enter new password: ", pw1, sizeof(pw1), false);
 		simple_prompt("Enter it again: ", pw2, sizeof(pw2), false);
@@ -2845,7 +2845,7 @@ copy_previous_query(PQExpBuffer query_buf, PQExpBuffer previous_buf)
 static char *
 prompt_for_password(const char *username)
 {
-	char		buf[100];
+	char		buf[2048];
 
 	if (username == NULL || username[0] == '\0')
 		simple_prompt("Password: ", buf, sizeof(buf), false);

--2.24.1--




Re: Increase psql's password buffer size

2020-01-20 Thread Tom Lane
David Fetter  writes:
> At least two cloud providers are now stuffing large amounts of
> information into the password field. This change makes it possible to
> accommodate that usage in interactive sessions.

Like who?  It seems like a completely silly idea.  And if 2K is sane,
why not much more?

(I can't say that s/100/2048/ in one place is a particularly evil change;
what bothers me is the likelihood that there are other places that won't
cope with arbitrarily long passwords.  Not all of them are necessarily
under our control, either.)

regards, tom lane




Increase psql's password buffer size

2020-01-20 Thread David Fetter
Folks,

At least two cloud providers are now stuffing large amounts of
information into the password field. This change makes it possible to
accommodate that usage in interactive sessions.

Best,
David.
-- 
David Fetter  http://fetter.org/
Phone: +1 415 235 3778

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate
>From 168c380548d1f97e4f6e9245851c22029931e8b5 Mon Sep 17 00:00:00 2001
From: David Fetter 
Date: Mon, 20 Jan 2020 09:58:19 -0800
Subject: [PATCH v1] Increase psql's password buffer size
To: hackers
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="2.24.1"

This is a multi-part message in MIME format.
--2.24.1
Content-Type: text/plain; charset=UTF-8; format=fixed
Content-Transfer-Encoding: 8bit


At least two cloud providers are now stuffing large amounts of
information into the password field. This makes it possible to
accommodate that usage in interactive sessions.

diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index e111cee556..84c2a0a37f 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -2845,7 +2845,7 @@ copy_previous_query(PQExpBuffer query_buf, PQExpBuffer previous_buf)
 static char *
 prompt_for_password(const char *username)
 {
-	char		buf[100];
+	char		buf[2048];
 
 	if (username == NULL || username[0] == '\0')
 		simple_prompt("Password: ", buf, sizeof(buf), false);

--2.24.1--