Thread

Commits

  1. Remove arbitrary restrictions on password length.

  2. Remove support for password_encryption='off' / 'plain'.

  1. Maximum password length

    Nathan Bossart <bossartn@amazon.com> — 2018-10-12T20:33:22Z

    Hello,
    
    I recently noticed a few restrictions on exceptionally long passwords
    that don't seem to be documented.  While scram-sha-256 has a limit of
    1,024 characters [0], other password-based authentication mechanisms
    do not seem to have a well-defined limit.  Furthermore, there is a
    1,000 character restriction on password messages [1], which limits the
    effective maximum length of the content of the message to 995
    characters (due to the '\0' byte and 4 bytes for the length of the
    message).  This 995 character restriction shouldn't impact md5 or
    scram-sha-256 authentication, but it will impact "password"
    authentication.  On top of all this, client utilities truncate
    passwords provided via prompt to 99 characters, so longer passwords
    must be provided via alternatives such as .pgpass and PGPASSWORD.
    
    I suspect these limits are acceptable for the vast majority of users,
    but it is presumably very confusing to users who attempt to use longer
    passwords.  For example, the truncation performed by client utilities
    like psql is done silently, specifying a scram-sha-256 password that
    is too long will result in a "password too long" message, and
    providing a password message longer than 995 characters will result in
    a "server closed the connection" error and an "invalid message length"
    log statement.
    
    I've attached 2 patches in an effort to clarify the upper bounds on
    password lengths:
        - 0001 refactors the hard-coded 100 character buffer size used for
          password prompts for client utilities into a
          PROMPT_MAX_PASSWORD_LENGTH macro in postgres_fe.h.
        - 0002 is an attempt at documenting the password length
          restrictions and suggested workarounds for longer passwords.
    
    I've also attached a third patch that increases the maximum length of
    password messages accepted by the server to 8,192 characters.  The
    current limit of 1,000 characters can be insufficient for very long
    passwords provided via "password" authentication.  IMO this server
    message limit is especially confusing for scram-sha-256 passwords, as
    they can be up to 1,024 characters long, but with "password"
    authentication, only 995 characters can be used to connect to the
    server.  Other forms of authentication similar to "password" (LDAP,
    RADIUS, PAM, BSD) are likewise impacted by the server message limit
    and may benefit from this increase.
    
    I am submitting these patches for consideration in commitfest 2018-11.
    
    Nathan
    
    [0] https://git.postgresql.org/gitweb/?p=postgresql.git;a=blob;f=src/common/saslprep.c;h=4cf574fed87ad830bcf8fdb105e37f8b4df0ee44;hb=HEAD#l42 
    [1] https://git.postgresql.org/gitweb/?p=postgresql.git;a=blob;f=src/backend/libpq/auth.c;h=85175655359829a2cf50dd883066bbb3d45e2286;hb=HEAD#l682
    
    
  2. Re: Maximum password length

    Stephen Frost <sfrost@snowman.net> — 2018-10-12T20:51:49Z

    Greetings,
    
    * Bossart, Nathan (bossartn@amazon.com) wrote:
    > I've attached 2 patches in an effort to clarify the upper bounds on
    > password lengths:
    >     - 0001 refactors the hard-coded 100 character buffer size used for
    >       password prompts for client utilities into a
    >       PROMPT_MAX_PASSWORD_LENGTH macro in postgres_fe.h.
    >     - 0002 is an attempt at documenting the password length
    >       restrictions and suggested workarounds for longer passwords.
    
    If we're going to do work in this area, why wouldn't we have the client
    tools and the server agree on the max length and then have them all be
    consistent..?
    
    Seems odd to decide that 100 character buffer size in the clients makes
    sense and then make the server support an 8k password.
    
    I'm also trying to figure out why it makes sense to support an 8k
    password and if we've really tried seeing what happens if pg_authid gets
    a toast table that's actually used for passwords...
    
    I'll note your patches neglected to include any tests...
    
    Thanks!
    
    Stephen
    
  3. Re: Maximum password length

    Isaac Morland <isaac.morland@gmail.com> — 2018-10-12T21:04:00Z

    On Fri, 12 Oct 2018 at 16:52, Stephen Frost <sfrost@snowman.net> wrote:
    
    
    > I'm also trying to figure out why it makes sense to support an 8k
    > password and if we've really tried seeing what happens if pg_authid gets
    > a toast table that's actually used for passwords...
    >
    
    pg_authid.rolpassword stores a hash, so the password length does not affect
    it.
    
    Of course, this also means that even in principle super-long passwords
    don't increase security, since one "can" (again, in principle) brute-force
    any password by guessing the first
    not-very-many-more-than-the-total-number-of-distinct-hashes possible
    passwords, starting with the shortest passwords and working up to longer
    passwords.
    
    It's also obvious that past a certain point, longer passwords don't help
    anyway, because it's already enough to have a password that can't be
    guessed in, say, the expected duration of the Earth's existence using all
    the computing power currently available in the world.
    
    I agree there should be a specific limit that is the same in libpq, on the
    server, and in the protocol. Maybe 128 characters, to get a nice round
    number? This is still way longer than the 32-byte SHA 256 hash. Or 64,
    which is still plenty but doesn't involve extending the current character
    buffer size to a longer value while still hugely exceeding the amount of
    information in the hash.
    
  4. Re: Maximum password length

    Nathan Bossart <bossartn@amazon.com> — 2018-10-12T21:14:57Z

    Hi Stephen,
    
    On 10/12/18, 3:52 PM, "Stephen Frost" <sfrost@snowman.net> wrote:
    > If we're going to do work in this area, why wouldn't we have the client
    > tools and the server agree on the max length and then have them all be
    > consistent..?
    > 
    > Seems odd to decide that 100 character buffer size in the clients makes
    > sense and then make the server support an 8k password.
    
    I considered this but wondered if expanding the buffers over 80x was
    too intrusive or if the 100 character limit had some historical
    purpose.  I'm happy to align everything if desired.
    
    > I'm also trying to figure out why it makes sense to support an 8k
    > password and if we've really tried seeing what happens if pg_authid gets
    > a toast table that's actually used for passwords...
    
    Since v10+ always stores passwords encrypted [0], I don't think it
    will require a TOAST table.
    
    > I'll note your patches neglected to include any tests...
    
    I will look into adding tests.  I've also been told that there may be
    some limits for the .pgpass file, so I am looking into that as well.
    
    Nathan
    
    [0] https://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=eb61136dc75a76caef8460fa939244d8593100f2
    
    
  5. Re: Maximum password length

    Stephen Frost <sfrost@snowman.net> — 2018-10-12T21:19:32Z

    Greetings,
    
    * Isaac Morland (isaac.morland@gmail.com) wrote:
    > On Fri, 12 Oct 2018 at 16:52, Stephen Frost <sfrost@snowman.net> wrote:
    > > I'm also trying to figure out why it makes sense to support an 8k
    > > password and if we've really tried seeing what happens if pg_authid gets
    > > a toast table that's actually used for passwords...
    > 
    > pg_authid.rolpassword stores a hash, so the password length does not affect
    > it.
    
    I had been thinking about storing of plaintext passwords, which we
    certainly used to do, but forgot that we actually did remove that,
    finally, so this specific point isn't a concern any longer, though of
    course the rest is.
    
    > Of course, this also means that even in principle super-long passwords
    > don't increase security, since one "can" (again, in principle) brute-force
    > any password by guessing the first
    > not-very-many-more-than-the-total-number-of-distinct-hashes possible
    > passwords, starting with the shortest passwords and working up to longer
    > passwords.
    
    Well, as you say, length doesn't matter here, if all you're doing is
    enumerating all possible responses to the server.
    
    > It's also obvious that past a certain point, longer passwords don't help
    > anyway, because it's already enough to have a password that can't be
    > guessed in, say, the expected duration of the Earth's existence using all
    > the computing power currently available in the world.
    
    Not sure I really am all that keen to get into that debate. :)
    
    > I agree there should be a specific limit that is the same in libpq, on the
    > server, and in the protocol. Maybe 128 characters, to get a nice round
    > number? This is still way longer than the 32-byte SHA 256 hash. Or 64,
    > which is still plenty but doesn't involve extending the current character
    > buffer size to a longer value while still hugely exceeding the amount of
    > information in the hash.
    
    I certainly don't think that we should break things which do work today,
    which would include long plaintext passwords sent by clients.
    
    Even if our clients don't support >100 character passwords, if the
    server does, then someone might be using one.
    
    Thanks!
    
    Stephen
    
  6. Re: Maximum password length

    Nathan Bossart <bossartn@amazon.com> — 2018-10-12T21:22:09Z

    Hi Isaac,
    
    On 10/12/18, 4:04 PM, "Isaac Morland" <isaac.morland@gmail.com> wrote:
    > I agree there should be a specific limit that is the same in libpq,
    > on the server, and in the protocol. Maybe 128 characters, to get a
    > nice round number? This is still way longer than the 32-byte SHA 256
    > hash. Or 64, which is still plenty but doesn't involve extending the
    > current character buffer size to a longer value while still hugely
    > exceeding the amount of information in the hash.
    
    My main motivation for suggesting the increase to 8k is to provide
    flexibility for alternative authentication methods like LDAP, RADIUS,
    PAM, and BSD.
    
    Nathan
    
    
  7. Re: Maximum password length

    Stephen Frost <sfrost@snowman.net> — 2018-10-12T21:22:14Z

    Greetings,
    
    * Bossart, Nathan (bossartn@amazon.com) wrote:
    > On 10/12/18, 3:52 PM, "Stephen Frost" <sfrost@snowman.net> wrote:
    > > If we're going to do work in this area, why wouldn't we have the client
    > > tools and the server agree on the max length and then have them all be
    > > consistent..?
    > > 
    > > Seems odd to decide that 100 character buffer size in the clients makes
    > > sense and then make the server support an 8k password.
    > 
    > I considered this but wondered if expanding the buffers over 80x was
    > too intrusive or if the 100 character limit had some historical
    > purpose.  I'm happy to align everything if desired.
    
    The way to sort that out would likely to be go look at the history...
    
    That said, assuming we do adjust the limit to be higher, it'd probably
    make more sense to allocate it and not just have it on the stack (which
    might be why it's the size it is today...).
    
    > > I'm also trying to figure out why it makes sense to support an 8k
    > > password and if we've really tried seeing what happens if pg_authid gets
    > > a toast table that's actually used for passwords...
    > 
    > Since v10+ always stores passwords encrypted [0], I don't think it
    > will require a TOAST table.
    
    Yeah, that was pointed out downthread, I'd forgotten that we (finally)
    got rid of storing plaintext passwords; sometimes it's difficult to
    believe that we've actually moved forward with something that some of us
    complained about many, many, many years ago. ;)
    
    > > I'll note your patches neglected to include any tests...
    > 
    > I will look into adding tests.  I've also been told that there may be
    > some limits for the .pgpass file, so I am looking into that as well.
    
    Ok.
    
    Thanks!
    
    Stephen
    
  8. Re: Maximum password length

    Tom Lane <tgl@sss.pgh.pa.us> — 2018-10-12T21:22:50Z

    Isaac Morland <isaac.morland@gmail.com> writes:
    > On Fri, 12 Oct 2018 at 16:52, Stephen Frost <sfrost@snowman.net> wrote:
    >> I'm also trying to figure out why it makes sense to support an 8k
    >> password and if we've really tried seeing what happens if pg_authid gets
    >> a toast table that's actually used for passwords...
    
    > ...
    > It's also obvious that past a certain point, longer passwords don't help
    > anyway, because it's already enough to have a password that can't be
    > guessed in, say, the expected duration of the Earth's existence using all
    > the computing power currently available in the world.
    
    And, of course, who is really going to type a password longer than a
    couple dozen characters?  And get it right reliably, when they can't
    see what they're typing?  But even if you assume the password is never
    manually entered but just lives in somebody's .pgpass, it's pointless
    to make it so long.  Then the attacker will just switch to brute-forcing
    the user's login password, or whereever along the chain there actually
    is a manually-entered password.
    
    I concur that we might as well standardize on something in the range
    of 64 to 100 characters.  1K is silly, even if somewhere there is a
    spec that allows it.
    
    			regards, tom lane
    
    
    
  9. Re: Maximum password length

    Stephen Frost <sfrost@snowman.net> — 2018-10-12T21:23:38Z

    Greetings,
    
    * Bossart, Nathan (bossartn@amazon.com) wrote:
    > On 10/12/18, 4:04 PM, "Isaac Morland" <isaac.morland@gmail.com> wrote:
    > > I agree there should be a specific limit that is the same in libpq,
    > > on the server, and in the protocol. Maybe 128 characters, to get a
    > > nice round number? This is still way longer than the 32-byte SHA 256
    > > hash. Or 64, which is still plenty but doesn't involve extending the
    > > current character buffer size to a longer value while still hugely
    > > exceeding the amount of information in the hash.
    > 
    > My main motivation for suggesting the increase to 8k is to provide
    > flexibility for alternative authentication methods like LDAP, RADIUS,
    > PAM, and BSD.
    
    Specific use-cases here would be better than hand-waving at "these other
    things."  Last I checked, all of those work with what we've got today
    and I don't recall hearing complaints about them not working due to this
    limit.
    
    Thanks!
    
    Stephen
    
  10. Re: Maximum password length

    Nathan Bossart <bossartn@amazon.com> — 2018-10-12T23:02:54Z

    On 10/12/18, 4:24 PM, "Stephen Frost" <sfrost@snowman.net> wrote:
    > * Bossart, Nathan (bossartn@amazon.com) wrote:
    >> My main motivation for suggesting the increase to 8k is to provide
    >> flexibility for alternative authentication methods like LDAP, RADIUS,
    >> PAM, and BSD.
    >
    > Specific use-cases here would be better than hand-waving at "these other
    > things."  Last I checked, all of those work with what we've got today
    > and I don't recall hearing complaints about them not working due to this
    > limit.
    
    The main one I am thinking of is generated security tokens.  It seems
    reasonable to me to limit md5 and scram-sha-256 passwords to a much
    shorter length, but I think the actual server message limit should be
    somewhat more flexible.
    
    Nathan
    
    
  11. Re: Maximum password length

    Tom Lane <tgl@sss.pgh.pa.us> — 2018-10-13T00:02:00Z

    "Bossart, Nathan" <bossartn@amazon.com> writes:
    > On 10/12/18, 4:24 PM, "Stephen Frost" <sfrost@snowman.net> wrote:
    >> Specific use-cases here would be better than hand-waving at "these other
    >> things."  Last I checked, all of those work with what we've got today
    >> and I don't recall hearing complaints about them not working due to this
    >> limit.
    
    > The main one I am thinking of is generated security tokens.  It seems
    > reasonable to me to limit md5 and scram-sha-256 passwords to a much
    > shorter length, but I think the actual server message limit should be
    > somewhat more flexible.
    
    Sure, but even a generated security token seems unlikely to be more
    than a couple dozen bytes long.  What's the actual use-case for tokens
    longer than that?  ISTM that a limit around 100 bytes already has a
    whole lot of headroom.
    
    			regards, tom lane
    
    
    
  12. Re: Maximum password length

    Nathan Bossart <bossartn@amazon.com> — 2018-10-13T04:30:05Z

    On 10/12/18, 7:02 PM, "Tom Lane" <tgl@sss.pgh.pa.us> wrote:
    > "Bossart, Nathan" <bossartn@amazon.com> writes:
    >> The main one I am thinking of is generated security tokens.  It seems
    >> reasonable to me to limit md5 and scram-sha-256 passwords to a much
    >> shorter length, but I think the actual server message limit should be
    >> somewhat more flexible.
    >
    > Sure, but even a generated security token seems unlikely to be more
    > than a couple dozen bytes long.  What's the actual use-case for tokens
    > longer than that?  ISTM that a limit around 100 bytes already has a
    > whole lot of headroom.
    
    I can't speak to the technical necessity of longer tokens, but several
    services provide them.  One specific example is the AWS Security Token
    Service.  The documentation for that service currently suggests that
    "the typical size is less than 4096 bytes..." [0].  I understand this
    alone doesn't warrant a change to PostgreSQL, but it seems valuable to
    me to ease this restriction on custom client authentication
    mechanisms.
    
    Regarding md5 and scram-sha-256 passwords, I'll look into establishing
    some sort of maximum password length that is well-documented and
    provides users with clear error messages.  My vote would be something
    like 128 characters just to be safe.  One interesting question is how
    we handle existing longer passwords after upgrading.  Maybe we could
    continue to allow longer passwords to be used for authentication and
    only restrict the length of new ones.
    
    Nathan
    
    [0] https://docs.aws.amazon.com/STS/latest/APIReference/API_GetSessionToken.html
    
    
  13. Re: Maximum password length

    Alexander Kukushkin <cyberdemn@gmail.com> — 2018-10-13T12:27:55Z

    On Sat, 13 Oct 2018 at 02:02, Tom Lane <tgl@sss.pgh.pa.us> wrote:
    
    > Sure, but even a generated security token seems unlikely to be more
    > than a couple dozen bytes long.  What's the actual use-case for tokens
    > longer than that?  ISTM that a limit around 100 bytes already has a
    > whole lot of headroom.
    
    Self-containing tokens, for example JWT, could be easily longer than 100 bytes.
    We at Zalando are using such tokens and the usual size of JWT token is
    600-700 bytes.
    
    It is not possible to "paste" such token into psql password prompt,
    because the input is truncated by 100 bytes.
    It is not possible to put it into ".pgpass" either, because it assumes
    that line could not be longer than 320 bytes (64*5)
    
    At the moment there are only two ways to use such tokens as a password:
    1. export PGPASSWORD=very_long.token
    2. specify the token(password) in the connection url
    
    Regards,
    --
    Alexander Kukushkin
    
    
    
  14. Re: Maximum password length

    Tom Lane <tgl@sss.pgh.pa.us> — 2020-09-01T00:54:53Z

    Alexander Kukushkin <cyberdemn@gmail.com> writes:
    > Self-containing tokens, for example JWT, could be easily longer than 100 bytes.
    > We at Zalando are using such tokens and the usual size of JWT token is
    > 600-700 bytes.
    
    > It is not possible to "paste" such token into psql password prompt,
    > because the input is truncated by 100 bytes.
    > It is not possible to put it into ".pgpass" either, because it assumes
    > that line could not be longer than 320 bytes (64*5)
    
    > At the moment there are only two ways to use such tokens as a password:
    > 1. export PGPASSWORD=very_long.token
    > 2. specify the token(password) in the connection url
    
    This thread seems to have fallen off the radar, but I got interested again
    now that we have a report of somebody else trying to use an 800-or-so-byte
    password [1], so I looked over Nathan's patches in some detail.
    
    I concur with Stephen's position that there ought to be just one upper
    limit not several.  At the same time, it's not clear to me that the
    password packet's length is closely related to the plaintext password
    limit when we're using SCRAM --- is there any case where the verifier
    string could exceed a few hundred bytes?
    
    Also, I'm not exactly convinced that we need to document the limit in the
    SGML docs, and I'm definitely down on repeating that info in 16 different
    places.  If we make the limit high enough to not be a problem, nobody is
    going to care exactly what it is.
    
    Therefore, I propose setting this up with a #define symbol in
    pg_config_manual.h and leaving it at that.  Giving documentation in
    pg_config_manual.h seems sufficient to me.  Attached is a revised
    version of Nathan's patches that does it like that.
    
    I set the proposed limit at 1024 bytes, but given that we now know
    of use-cases needing up to 800 bytes, maybe there should be a little
    more headroom?  I don't want to make it enormous, though, seeing that
    we're allocating static buffers of that size.
    
    Note this patch is intended to be applied over my patch at [2],
    since it modifies the test case added there.
    
    			regards, tom lane
    
    [1] https://www.postgresql.org/message-id/flat/CAOhmDze1nqG2vfegpSsTFCgaiFRsqgjO6yLsbmhroz2zGmJHog%40mail.gmail.com
    [2] https://www.postgresql.org/message-id/4187382.1598909041%40sss.pgh.pa.us
    
    
  15. Re: Maximum password length

    Tom Lane <tgl@sss.pgh.pa.us> — 2020-09-01T18:36:24Z

    I wrote:
    > Note this patch is intended to be applied over my patch at [2],
    > since it modifies the test case added there.
    
    I've now pushed that patch, so the patch in my previous mail should
    directly apply to HEAD.
    
    I'd originally been wondering whether we need to back-patch this patch.
    But unless someone wants to make a case for the max password length
    being more than 1024, it seems like this is mostly cleanup and could
    just be done in HEAD.  At 1024, the actual behavior of pg_saslprep()
    isn't changing at all, and the behavior of recv_password_packet() isn't
    changing by much.  The real impact is just that the places that prompt
    for a password will accept passwords up to 1K instead of 100 bytes.
    Which, TBH, seems like neatnik-ism rather than fixing anything useful.
    Surely nobody is going to manually enter passwords exceeding 100 bytes.
    And, since simple_prompt insists on reading /dev/tty not stdin, there
    is no very easy way to pass a machine-generated password through that
    code path.  The practical ways to deal with a long password are either
    to set it as PGPASSWORD (has always worked) or put it in .pgpass
    (works as of now).
    
    Anyway, I added this thread to the upcoming CF, in case anyone wants to
    discuss it further.
    
    			regards, tom lane
    
    
    
    
  16. Re: Maximum password length

    Nathan Bossart <bossartn@amazon.com> — 2020-09-01T20:15:59Z

    On 8/31/20, 5:55 PM, "Tom Lane" <tgl@sss.pgh.pa.us> wrote:
    > I set the proposed limit at 1024 bytes, but given that we now know
    > of use-cases needing up to 800 bytes, maybe there should be a little
    > more headroom?  I don't want to make it enormous, though, seeing that
    > we're allocating static buffers of that size.
    
    For the use-case described in [0], I ended up bumping the server-side
    limit in libpq/auth.c to 8192 bytes for RDS instances.  This appears
    to be the PqRecvBuffer size, too.  In any case, these tokens regularly
    exceed 1024 bytes, so I would definitely argue for more headroom if
    possible.  Otherwise, I like the idea of unifying all the limits.
    
    Nathan
    
    [0] https://www.postgresql.org/message-id/flat/CAOhmDze1nqG2vfegpSsTFCgaiFRsqgjO6yLsbmhroz2zGmJHog%40mail.gmail.com
    
    
  17. Re: Maximum password length

    Peter Eisentraut <peter.eisentraut@2ndquadrant.com> — 2020-09-01T21:15:20Z

    On 2020-09-01 02:54, Tom Lane wrote:
    > Therefore, I propose setting this up with a #define symbol in
    > pg_config_manual.h and leaving it at that.  Giving documentation in
    > pg_config_manual.h seems sufficient to me.  Attached is a revised
    > version of Nathan's patches that does it like that.
    > 
    > I set the proposed limit at 1024 bytes, but given that we now know
    > of use-cases needing up to 800 bytes, maybe there should be a little
    > more headroom?  I don't want to make it enormous, though, seeing that
    > we're allocating static buffers of that size.
    
    ISTM that it's only going to be a matter of time before that will be 
    exceeded.  Why have a limit at all?  Accept whatever StringInfo accepts.
    
    -- 
    Peter Eisentraut              http://www.2ndQuadrant.com/
    PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
    
    
    
    
  18. Re: Maximum password length

    Tom Lane <tgl@sss.pgh.pa.us> — 2020-09-01T21:58:39Z

    Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:
    > ISTM that it's only going to be a matter of time before that will be 
    > exceeded.  Why have a limit at all?  Accept whatever StringInfo accepts.
    
    Hmm, that would require some refactoring of simple_prompt for starters.
    I agree there's no hard reason why we have to have any specific limit,
    if we're willing to do that.
    
    			regards, tom lane
    
    
    
    
  19. Re: Maximum password length

    Tom Lane <tgl@sss.pgh.pa.us> — 2020-09-02T00:58:23Z

    I wrote:
    > Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:
    >> ISTM that it's only going to be a matter of time before that will be 
    >> exceeded.  Why have a limit at all?  Accept whatever StringInfo accepts.
    
    > Hmm, that would require some refactoring of simple_prompt for starters.
    
    To use StringInfo, we have to move sprompt.c into src/common/ where
    the stringinfo stuff lives; but that seems fine to me, because it had
    little if any business being in src/port/.  Here's a draft patch
    that does it that way.
    
    This could be refined; in particular, I think that most of the
    password-prompting sites could drop their separate have_password
    flags in favor of checking whether the password pointer is NULL
    or not.  That would likely also prove that some of the free(password)
    calls I sprinkled in are unnecessary.
    
    			regards, tom lane
    
    
  20. Re: Maximum password length

    Tom Lane <tgl@sss.pgh.pa.us> — 2020-09-03T17:18:17Z

    I wrote:
    > This could be refined; in particular, I think that most of the
    > password-prompting sites could drop their separate have_password
    > flags in favor of checking whether the password pointer is NULL
    > or not.  That would likely also prove that some of the free(password)
    > calls I sprinkled in are unnecessary.
    
    Hearing no objections to this general plan, I went ahead and did that
    cleanup.  This version seems committable to me.
    
    			regards, tom lane
    
    
  21. Re: Maximum password length

    Nathan Bossart <bossartn@amazon.com> — 2020-09-03T20:06:55Z

    On 9/3/20, 10:19 AM, "Tom Lane" <tgl@sss.pgh.pa.us> wrote:
    > Hearing no objections to this general plan, I went ahead and did that
    > cleanup.  This version seems committable to me.
    
                    FILE       *pwf = fopen(pwfilename, "r");
    -               int                     i;
    +               char            pwdbuf[8192];
    
    If I am reading correctly, this would be the only defined password
    length limit once this patch is applied.  While it's probably unlikely
    that this will cause problems for anybody anytime soon, is there any
    reason not to give this the same treatment as the .pgpass code and
    remove the line length limit altogether?
    
    Otherwise, the patch looks good to me.
    
    Nathan
    
    
  22. Re: Maximum password length

    Tom Lane <tgl@sss.pgh.pa.us> — 2020-09-03T21:07:24Z

    "Bossart, Nathan" <bossartn@amazon.com> writes:
    > On 9/3/20, 10:19 AM, "Tom Lane" <tgl@sss.pgh.pa.us> wrote:
    > +               char            pwdbuf[8192];
    
    > If I am reading correctly, this would be the only defined password
    > length limit once this patch is applied.  While it's probably unlikely
    > that this will cause problems for anybody anytime soon, is there any
    > reason not to give this the same treatment as the .pgpass code and
    > remove the line length limit altogether?
    
    Yeah, it just didn't quite seem worthwhile there, given the adjacent
    comment that clearly says that this is second-class-citizen code:
    
             * Ideally this should insist that the file not be world-readable.
             * However, this option is mainly intended for use on Windows where
             * file permissions may not exist at all, so we'll skip the paranoia
             * for now.
    
    If you insist, I'll change it, but it seems even less likely to ever
    matter to anybody than the changes to make simple_prompt accept
    indefinitely long passwords.  (Perhaps a reasonable compromise
    is to extend this comment to note that we're also not bothering
    to support indefinitely long passwords.)
    
    			regards, tom lane
    
    
    
    
  23. Re: Maximum password length

    Nathan Bossart <bossartn@amazon.com> — 2020-09-03T21:28:49Z

    On 9/3/20, 2:14 PM, "Tom Lane" <tgl@sss.pgh.pa.us> wrote:
    > If you insist, I'll change it, but it seems even less likely to ever
    > matter to anybody than the changes to make simple_prompt accept
    > indefinitely long passwords.  (Perhaps a reasonable compromise
    > is to extend this comment to note that we're also not bothering
    > to support indefinitely long passwords.)
    
    I don't feel strongly about this.  A comment in the code seems
    reasonable.
    
    Nathan
    
    
  24. Re: Maximum password length

    Tom Lane <tgl@sss.pgh.pa.us> — 2020-09-03T21:36:02Z

    "Bossart, Nathan" <bossartn@amazon.com> writes:
    > On 9/3/20, 2:14 PM, "Tom Lane" <tgl@sss.pgh.pa.us> wrote:
    >> If you insist, I'll change it, but it seems even less likely to ever
    >> matter to anybody than the changes to make simple_prompt accept
    >> indefinitely long passwords.  (Perhaps a reasonable compromise
    >> is to extend this comment to note that we're also not bothering
    >> to support indefinitely long passwords.)
    
    > I don't feel strongly about this.  A comment in the code seems
    > reasonable.
    
    Alvaro proposes nearby that we ought to have a src/common/ function
    to slurp an indefinitely long line from a file [1].  If we do that,
    it'd be entirely reasonable to make this code use that.  So maybe
    the right comment is "XXX FIXME later".
    
    			regards, tom lane
    
    [1] https://www.postgresql.org/message-id/20200903200842.GA11952%40alvherre.pgsql
    
    
    
    
  25. Re: Maximum password length

    Tom Lane <tgl@sss.pgh.pa.us> — 2020-09-03T22:46:35Z

    I wrote:
    > Alvaro proposes nearby that we ought to have a src/common/ function
    > to slurp an indefinitely long line from a file [1].  If we do that,
    > it'd be entirely reasonable to make this code use that.  So maybe
    > the right comment is "XXX FIXME later".
    
    Actually, on further thought, the obviously right thing to do here
    is to refactor simple_prompt into two functions: the inner one is
    basically like fgets except it returns a malloc'd, variable-size
    string, and then the outer one does the other stuff simple_prompt needs
    such as prompting and opening /dev/tty.  The inner function would
    serve initdb's need directly, and it would also have uses elsewhere,
    as per the other thread.
    
    I'll go make that happen.
    
    			regards, tom lane