Re: [PoC/RFC] Multiple passwords, interval expirations
Hi! I'm interested in this feature presence in the PostgreSQL core. Will try to provide valuable review/comments/suggestions and other help. On Tue, 10 Oct 2023 at 16:17, Gurjeet Singh wrote: > > > On Mon, Oct 9, 2023 at 2:31 AM Gurjeet Singh wrote: > > > > > > Next steps: > > > - Break the patch into a series of smaller patches. > > > - Add TAP tests (test the ability to actually login with these passwords) > > > - Add/update documentation > > > - Add more regression tests > > Please see attached the v4 of the patchset that introduces the notion > of named passwords slots, namely 'first' and 'second' passwords, and > allows users to address each of these passwords separately for the > purposes of adding, dropping, or assigning expiration times. > > Apart from the changes described by each patch's commit title, one > significant change since v3 is that now (included in v4-0002...patch) > it is not allowed for a role to have a mix of a types of passwords. > When adding a password, the patch ensures that the password being > added uses the same hashing algorithm (md5 or scram-sha-256) as the > existing password, if any. Having all passwords of the same type > helps the server pick the corresponding authentication method during > connection attempt. > > The v3 patch also had a few bugs that were exposed by cfbot's > automatic run. All those bugs have now been fixed, and the latest run > on the v4 branch [1] on my private Git repo shows a clean run [1]. > > The list of patches, and their commit titles are as follows: > > > v4-0001-...patch Add new columns to pg_authid > > v4-0002-...patch Update password verification infrastructure to handle two > > passwords > > v4-0003-...patch Added SQL support for ALTER ROLE to manage two passwords > > v4-0004-...patch Updated pg_dumpall to support exporting a role's second > > password > > v4-0005-...patch Update system views pg_roles and pg_shadow > > v4-0006-...patch Updated pg_authid catalog documentation > > v4-0007-...patch Updated psql's describe-roles meta-command > > v4-0008-...patch Added documentation for ALTER ROLE command > > v4-0009-...patch Added TAP tests to prove that a role can use two passwords > > to login > > v4-0010-...patch pgindent run > > v4-0011-...patch Run pgperltidy on files changed by this patchset > > Running pgperltidy updated many perl files unrelated to this patch, so > in the last patch I chose to include only the one perl file that is > affected by this patchset. > > [1]: password_rollover_v4 (910f81be54) > https://github.com/gurjeet/postgres/commits/password_rollover_v4 > > [2]: https://cirrus-ci.com/build/4675613999497216 > > Best regards, > Gurjeet > http://Gurje.et Latest attachment does not apply to HEAD anymore. I have rebased them. While rebasing, a couple of minor changes were done: 1) Little correction in the `plain_crypt_verify` comment. IMO this sounds a little better and comprehensible, is it? > - * 'shadow_pass' is the user's correct password hash, as stored in > - * pg_authid's rolpassword or rolsecondpassword. > + * 'shadow_pass' is one of the user's correct password hashes, as stored in > + * pg_authid's. 2) in v4-0004: >/* note: rolconfig is dumped later */ > - if (server_version >= 90600) > + if (server_version >= 17) >printfPQExpBuffer(buf, > "SELECT oid, rolname, > rolsuper, rolinherit, " > "rolcreaterole, rolcreatedb, > " > - "rolcanlogin, rolconnlimit, > rolpassword, " > - "rolvaliduntil, > rolreplication, rolbypassrls, " > + "rolcanlogin, rolconnlimit, > " > + "rolpassword, > rolvaliduntil, " > + "rolsecondpassword, > rolsecondvaliduntil, " > + "rolreplication, > rolbypassrls, " > + > "pg_catalog.shobj_description(oid, '%s') as rolcomment, " > + "rolname = current_user AS > is_current_user " > + "FROM %s " > + "WHERE rolname !~ '^pg_' " > + "ORDER BY 2", role_catalog, > role_catalog); > + else if (server_version >= 90600) > + printfPQExpBuffer(buf, > + "SELECT oid, rolname, > rolsuper, rolinherit, " > + "rolcreaterole, > rolcreatedb, " > + "rolcanlogin, rolconnlimit, > " > + "rolpassword, > rolvaliduntil, " > +
Re: [PoC/RFC] Multiple passwords, interval expirations
On Sat, 27 Jan 2024 at 07:18, vignesh C wrote: > > On Tue, 10 Oct 2023 at 16:37, Gurjeet Singh wrote: > > > > > On Mon, Oct 9, 2023 at 2:31 AM Gurjeet Singh wrote: > > > > > > > > Next steps: > > > > - Break the patch into a series of smaller patches. > > > > - Add TAP tests (test the ability to actually login with these > > > > passwords) > > > > - Add/update documentation > > > > - Add more regression tests > > > > Please see attached the v4 of the patchset that introduces the notion > > of named passwords slots, namely 'first' and 'second' passwords, and > > allows users to address each of these passwords separately for the > > purposes of adding, dropping, or assigning expiration times. > > > > Apart from the changes described by each patch's commit title, one > > significant change since v3 is that now (included in v4-0002...patch) > > it is not allowed for a role to have a mix of a types of passwords. > > When adding a password, the patch ensures that the password being > > added uses the same hashing algorithm (md5 or scram-sha-256) as the > > existing password, if any. Having all passwords of the same type > > helps the server pick the corresponding authentication method during > > connection attempt. > > > > The v3 patch also had a few bugs that were exposed by cfbot's > > automatic run. All those bugs have now been fixed, and the latest run > > on the v4 branch [1] on my private Git repo shows a clean run [1]. > > > > The list of patches, and their commit titles are as follows: > > > > > v4-0001-...patch Add new columns to pg_authid > > > v4-0002-...patch Update password verification infrastructure to handle > > > two passwords > > > v4-0003-...patch Added SQL support for ALTER ROLE to manage two passwords > > > v4-0004-...patch Updated pg_dumpall to support exporting a role's second > > > password > > > v4-0005-...patch Update system views pg_roles and pg_shadow > > > v4-0006-...patch Updated pg_authid catalog documentation > > > v4-0007-...patch Updated psql's describe-roles meta-command > > > v4-0008-...patch Added documentation for ALTER ROLE command > > > v4-0009-...patch Added TAP tests to prove that a role can use two > > > passwords to login > > > v4-0010-...patch pgindent run > > > v4-0011-...patch Run pgperltidy on files changed by this patchset > > > > Running pgperltidy updated many perl files unrelated to this patch, so > > in the last patch I chose to include only the one perl file that is > > affected by this patchset. > > CFBot shows that the patch does not apply anymore as in [1]: > === Applying patches on top of PostgreSQL commit ID > 4d969b2f85e1fd00e860366f101fd3e3160aab41 === > === applying patch > ./v4-0002-Update-password-verification-infrastructure-to-ha.patch > ... > patching file src/backend/libpq/auth.c > Hunk #4 FAILED at 828. > Hunk #5 succeeded at 886 (offset -2 lines). > Hunk #6 succeeded at 907 (offset -2 lines). > 1 out of 6 hunks FAILED -- saving rejects to file src/backend/libpq/auth.c.rej > > Please post an updated version for the same. The patch which you submitted has been awaiting your attention for quite some time now. As such, we have moved it to "Returned with Feedback" and removed it from the reviewing queue. Depending on timing, this may be reversible. Kindly address the feedback you have received, and resubmit the patch to the next CommitFest. Regards, Vignesh
Re: [PoC/RFC] Multiple passwords, interval expirations
On Tue, 10 Oct 2023 at 16:37, Gurjeet Singh wrote: > > > On Mon, Oct 9, 2023 at 2:31 AM Gurjeet Singh wrote: > > > > > > Next steps: > > > - Break the patch into a series of smaller patches. > > > - Add TAP tests (test the ability to actually login with these passwords) > > > - Add/update documentation > > > - Add more regression tests > > Please see attached the v4 of the patchset that introduces the notion > of named passwords slots, namely 'first' and 'second' passwords, and > allows users to address each of these passwords separately for the > purposes of adding, dropping, or assigning expiration times. > > Apart from the changes described by each patch's commit title, one > significant change since v3 is that now (included in v4-0002...patch) > it is not allowed for a role to have a mix of a types of passwords. > When adding a password, the patch ensures that the password being > added uses the same hashing algorithm (md5 or scram-sha-256) as the > existing password, if any. Having all passwords of the same type > helps the server pick the corresponding authentication method during > connection attempt. > > The v3 patch also had a few bugs that were exposed by cfbot's > automatic run. All those bugs have now been fixed, and the latest run > on the v4 branch [1] on my private Git repo shows a clean run [1]. > > The list of patches, and their commit titles are as follows: > > > v4-0001-...patch Add new columns to pg_authid > > v4-0002-...patch Update password verification infrastructure to handle two > > passwords > > v4-0003-...patch Added SQL support for ALTER ROLE to manage two passwords > > v4-0004-...patch Updated pg_dumpall to support exporting a role's second > > password > > v4-0005-...patch Update system views pg_roles and pg_shadow > > v4-0006-...patch Updated pg_authid catalog documentation > > v4-0007-...patch Updated psql's describe-roles meta-command > > v4-0008-...patch Added documentation for ALTER ROLE command > > v4-0009-...patch Added TAP tests to prove that a role can use two passwords > > to login > > v4-0010-...patch pgindent run > > v4-0011-...patch Run pgperltidy on files changed by this patchset > > Running pgperltidy updated many perl files unrelated to this patch, so > in the last patch I chose to include only the one perl file that is > affected by this patchset. CFBot shows that the patch does not apply anymore as in [1]: === Applying patches on top of PostgreSQL commit ID 4d969b2f85e1fd00e860366f101fd3e3160aab41 === === applying patch ./v4-0002-Update-password-verification-infrastructure-to-ha.patch ... patching file src/backend/libpq/auth.c Hunk #4 FAILED at 828. Hunk #5 succeeded at 886 (offset -2 lines). Hunk #6 succeeded at 907 (offset -2 lines). 1 out of 6 hunks FAILED -- saving rejects to file src/backend/libpq/auth.c.rej Please post an updated version for the same. [1] - http://cfbot.cputube.org/patch_46_4432.log Regards, Vignesh
Re: [PoC/RFC] Multiple passwords, interval expirations
Greetings, * Jeff Davis (pg...@j-davis.com) wrote: > On Wed, 2023-10-18 at 14:48 -0400, Stephen Frost wrote: > > Right, we need more observability, agreed, but that's not strictly > > necessary of this patch and could certainly be added independently. > > Is > > there really a need to make this observability a requirement of this > > particular change? > > I won't draw a line in the sand, but it feels like something should be > there to help the user keep track of which password they might want to > keep. At least a "created on" date or something. Sure, no objection to adding that and seems like it should be fairly easy ... but then again, I tend to feel that we should do that for all of the objects in the system and we've got some strong feelings against doing that from others. Perhaps this case is different to them, in which case, great, but if it's not, it'd be unfortunate for this feature to get bogged down due to that. > > > (Aside: is the uniqueness of the salt enforced in the current > > > patch?) > > > > Err, the salt has to be *identical* for each password of a given > > user, > > not unique, so I'm a bit confused here. > > Sorry, my mistake. Sure, no worries. > If the client needs to use the same salt as existing passwords, can you > still use PQencryptPasswordConn() on the client to avoid sending the > plaintext password to the server? Short answer, yes ... but seems that wasn't actually done yet. Requires a bit of additional work, since the client needs to get the existing salt (note that as part of the SCRAM typical exchange, the client is provided with the salt, so this isn't exposing anything new) to use to construct what is then sent to the server to store. We'll also need to decide how to handle the case if the client tries to send a password that doesn't have the same salt as the existing ones (regardless of how many passwords we end up supporting). Perhaps we require the user, through the grammar, to make clear if they want to add a password, and then error if they don't provide a matching salt, or if they want to remove existing passwords and replace with the new one. Thanks, Stephen signature.asc Description: PGP signature
Re: [PoC/RFC] Multiple passwords, interval expirations
On Wed, 2023-10-18 at 14:48 -0400, Stephen Frost wrote: > Right, we need more observability, agreed, but that's not strictly > necessary of this patch and could certainly be added independently. > Is > there really a need to make this observability a requirement of this > particular change? I won't draw a line in the sand, but it feels like something should be there to help the user keep track of which password they might want to keep. At least a "created on" date or something. > > (Aside: is the uniqueness of the salt enforced in the current > > patch?) > > Err, the salt has to be *identical* for each password of a given > user, > not unique, so I'm a bit confused here. Sorry, my mistake. If the client needs to use the same salt as existing passwords, can you still use PQencryptPasswordConn() on the client to avoid sending the plaintext password to the server? Regards, Jeff Davis
Re: [PoC/RFC] Multiple passwords, interval expirations
Greetings, * Jeff Davis (pg...@j-davis.com) wrote: > On Tue, 2023-10-17 at 22:52 -0400, Stephen Frost wrote: > > > Reading back through the thread, from a user perspective, the primary > > one seems to be that passwords are expected to be named. I'm > > surprised > > this is being brought up as such a serious concern. Lots and lots > > and > > lots of things in the system require naming, after all, and the idea > > that this is somehow harder or more of an issue is quite odd to me. > > In the simplest intended use case, the names will be arbitrary and > temporary. It's easy for me to imagine someone wondering "was I > supposed to delete 'bear' or 'lion'?". For indexes and other objects, > there's a lot more to go on, easily visible with \d. Sure, agreed. > Now, obviously that is not the end of the world, and the user could > prevent that problem a number of different ways. And we can do things > like improve the monitoring of password use, and store the password > creation time, to help users if they are confused. So I don't raise > concerns about naming as an objection to the feature overall, but > rather a concern that we aren't getting it quite right. Right, we need more observability, agreed, but that's not strictly necessary of this patch and could certainly be added independently. Is there really a need to make this observability a requirement of this particular change? > Maybe a name should be entirely optional, more like a comment, and the > passwords can be referenced by the salt? The salt needs to be unique > for a given user anyway. I proposed an approach in the email you replied to explicitly suggesting a way we could make the name be optional, so, sure, I'm generally on board with that idea. Note that it'd be optional for the user to provide and then we'd simply generate one for them and then that name is what would be used to refer to that password later. > (Aside: is the uniqueness of the salt enforced in the current patch?) Err, the salt has to be *identical* for each password of a given user, not unique, so I'm a bit confused here. Thanks, Stephen signature.asc Description: PGP signature
Re: [PoC/RFC] Multiple passwords, interval expirations
On Tue, 2023-10-17 at 22:52 -0400, Stephen Frost wrote: > Reading back through the thread, from a user perspective, the primary > one seems to be that passwords are expected to be named. I'm > surprised > this is being brought up as such a serious concern. Lots and lots > and > lots of things in the system require naming, after all, and the idea > that this is somehow harder or more of an issue is quite odd to me. In the simplest intended use case, the names will be arbitrary and temporary. It's easy for me to imagine someone wondering "was I supposed to delete 'bear' or 'lion'?". For indexes and other objects, there's a lot more to go on, easily visible with \d. Now, obviously that is not the end of the world, and the user could prevent that problem a number of different ways. And we can do things like improve the monitoring of password use, and store the password creation time, to help users if they are confused. So I don't raise concerns about naming as an objection to the feature overall, but rather a concern that we aren't getting it quite right. Maybe a name should be entirely optional, more like a comment, and the passwords can be referenced by the salt? The salt needs to be unique for a given user anyway. (Aside: is the uniqueness of the salt enforced in the current patch?) Regards, Jeff Davis
Re: [PoC/RFC] Multiple passwords, interval expirations
Greetings, * Jeff Davis (pg...@j-davis.com) wrote: > On Tue, 2023-10-17 at 16:20 -0400, Stephen Frost wrote: > > Agreed that it's a bad idea to design to support 2 and only 2. > > I don't disagree, but it's difficult to come up with syntax that: > > (a) supports N passwords > (b) makes the ordinary cases simple and documentable > (c) helps users avoid mistakes (at least in the simple cases) > (d) makes sense passwords with and without validity period > (e) handles the challenging cases Undoubtably biased ... but I don't agree that this is so difficult. What points have been raised as a downside of the originally proposed approach, specifically? Reading back through the thread, from a user perspective, the primary one seems to be that passwords are expected to be named. I'm surprised this is being brought up as such a serious concern. Lots and lots and lots of things in the system require naming, after all, and the idea that this is somehow harder or more of an issue is quite odd to me. > One challenging case is that we cannot allow the mixing of password > protocols (e.g. MD5 & SCRAM), because the authentication exchange only > gets one chance at success. If someone ends up with 7 MD5 passwords, > and they'd like to migrate to SCRAM, then we can't allow them to > migrate one password at a time (because then the other passwords would > break). I'd like to see what the SQL for doing this should look like. I've got absolutely no interest in supporting md5- it's high time to rip that out and be happy that it's gone. We nearly did it last year and I'm really hoping we accomplish that this year. I'm open to the idea that we may need to support some new SCRAM version or an alternative mechanism in the future, but it's long past time to spend any time worrying about md5. As for how difficult it is to deal with supporting an alternative in the future- that's going to depend a great deal on what that alternative is and I don't know that we can really code to handle that as part of this effort in a sensible way, and trying to code for "anything" is going to likely make this much more complicated, and probably rife with security issues too. > > If > > nothing else, there's the very simple case that the user needs to do > > another password rotation ... and they look and discover that the old > > password is still being used and that if they took it away, things > > would > > break, but they need time to run down which system is still using it > > while still needing to perform the migration for the other systems > > that > > are correctly being updated- boom, need 3 for that case. > > That sounds like a reasonable use case. I don't know if we should make > it a requirement, but if we come up with something reasonable that > supports this case I'm fine with it. Ideally, it would still be easy to > see when you are making a mistake (e.g. forgetting to ever remove > passwords). We have monitoring for many, many parts of the system and this would be a good thing to monitor also- not just at a per-password level but also at an overall role/user level, as you have a similar issue there and we don't properly provide users with any way to check reasonably "hey, when was the last time this user logged in?". No, trawling through ancient logs, if you even have them, isn't a proper solution to this problem. > > There's other > > use-cases that could be interesting though- presumably we'd log which > > password is used to authenticate and then users could have a fleet of > > web servers which each have their own password but log into the same > > PG > > user and they could happily rotate the passwords independently for > > all > > of those systems. > > > > if they're all > > logging in with the same role and just a different password, that's > > not > > going to happen. > > I'm not sure this is a great idea. Can you point to some precedent > here? It's already the case that lots and lots and lots of systems out there log into PG using the same username/password. With this, we're at least offering them the ability to vary the password and keep the user the same. We've even seen this be asked for in other ways- the ability to use distinct Kerberos or LDAP identifies to log into the same user in the database, see pg_ident.conf and various suggestions for how to bring that to LDAP too. Other systems also support the ability to have a group of users in LDAP, or such, be allowed to log into a specific database user. One big reason for this is that then you know everyong logging into that account has exactly the same access to things- because that's the lowest level at which access can be granted. Having a way to support this similar capability but for passwords is certainly useful. > > Giving users the option of not having to specify a name and letting > > the > > system come up with one (similar to what we do for indexes and such) > > could work out pretty decently, imv. I'd have that be optional >
Re: [PoC/RFC] Multiple passwords, interval expirations
On Tue, 2023-10-17 at 16:20 -0400, Stephen Frost wrote: > Agreed that it's a bad idea to design to support 2 and only 2. I don't disagree, but it's difficult to come up with syntax that: (a) supports N passwords (b) makes the ordinary cases simple and documentable (c) helps users avoid mistakes (at least in the simple cases) (d) makes sense passwords with and without validity period (e) handles the challenging cases One challenging case is that we cannot allow the mixing of password protocols (e.g. MD5 & SCRAM), because the authentication exchange only gets one chance at success. If someone ends up with 7 MD5 passwords, and they'd like to migrate to SCRAM, then we can't allow them to migrate one password at a time (because then the other passwords would break). I'd like to see what the SQL for doing this should look like. > If > nothing else, there's the very simple case that the user needs to do > another password rotation ... and they look and discover that the old > password is still being used and that if they took it away, things > would > break, but they need time to run down which system is still using it > while still needing to perform the migration for the other systems > that > are correctly being updated- boom, need 3 for that case. That sounds like a reasonable use case. I don't know if we should make it a requirement, but if we come up with something reasonable that supports this case I'm fine with it. Ideally, it would still be easy to see when you are making a mistake (e.g. forgetting to ever remove passwords). > There's other > use-cases that could be interesting though- presumably we'd log which > password is used to authenticate and then users could have a fleet of > web servers which each have their own password but log into the same > PG > user and they could happily rotate the passwords independently for > all > of those systems. > > if they're all > logging in with the same role and just a different password, that's > not > going to happen. I'm not sure this is a great idea. Can you point to some precedent here? > Giving users the option of not having to specify a name and letting > the > system come up with one (similar to what we do for indexes and such) > could work out pretty decently, imv. I'd have that be optional > though- > if the user wants to specify a name, then they should be allowed to > do > so. Can you describe how some basic use cases should work with example SQL? > > > * the identifier of the current password always changing (perhaps > > fine > > if it'a a "created at" ID?) > > I'm not following what the issue is you're getting at here. I just meant that when rotating passwords, you have to keep coming up with new names, so the "current" or "primary" one would not have a consistent name. Regards, Jeff Davis
Re: [PoC/RFC] Multiple passwords, interval expirations
Greetings, * Nathan Bossart (nathandboss...@gmail.com) wrote: > On Wed, Oct 04, 2023 at 10:41:15PM -0700, Gurjeet Singh wrote: > > The patches posted in this thread so far attempt to add the ability to > > allow the user to have an arbitrary number of passwords. I believe > > that allowing arbitrary number of passwords is not only unnecessary, > > but the need to name passwords, the need to store them in a shared > > catalog, etc. may actually create problems in the field. The > > users/admins will have to choose names for passwords, which they > > didn't have to previously. The need to name them may also lead to > > users storing password-hints in the password names (e.g. 'mom''s > > birthday', 'ex''s phone number', 'third password'), rendering the > > passwords weak. > > > > Moreover, allowing an arbitrarily many number of passwords will > > require us to provide additional infrastructure to solve problems like > > observability (which passwords are currently in use, and which ones > > have been effectively forgotten by applications), or create a nuisance > > for admins that can create more problems than it solves. > > IMHO neither of these problems seems insurmountable. Besides advising > against using hints as names, we could also automatically generate safe > names, or even disallow user-provided names entirely. And adding > observability for passwords seems worthwhile anyway. Agreed, particularly on adding observability for password use. Regardless of what we do, I feel pretty strongly that we need that. That said, having this handled in a separate catalog feels like a just generally better idea than shoving it all into pg_authid as we extend things to include information like "last used date", "last used source IP", etc. Providing this observability purely through logging strikes me as a terrible idea. I don't find the concern about names as 'hints' to be at all compelling. Having a way to avoid having names may have some value, but only if we can come up with something reasonable. > > So I propose that the feature should allow no more than 2 passwords > > for a role, each with their own validity periods. This eliminates the > > need to name passwords, because at any given time there are no more > > than 2 passwords; current one, and old one. This also eliminates the > > need for a shared catalog to hold passwords, because with the limit of > > 2 imposed, we can store the old password and its validity period in > > additional columns in the pg_authid table. > > Another approach could be to allow an abritrary number of passwords but to > also allow administrators to limit how many passwords can be associated to > each role. That way, we needn't restrict this feature to 2 passwords for > everyone. Perhaps 2 should be the default, but in any case, IMO we > shouldn't design to only support 2. Agreed that it's a bad idea to design to support 2 and only 2. If nothing else, there's the very simple case that the user needs to do another password rotation ... and they look and discover that the old password is still being used and that if they took it away, things would break, but they need time to run down which system is still using it while still needing to perform the migration for the other systems that are correctly being updated- boom, need 3 for that case. There's other use-cases that could be interesting though- presumably we'd log which password is used to authenticate and then users could have a fleet of web servers which each have their own password but log into the same PG user and they could happily rotate the passwords independently for all of those systems. I don't propose this as some use-case just for the purpose of argument- not sharing passwords across a bunch of systems is absolutely a good stance when it comes to security, and due to the way permissions and roles work in PG, being able to have both distinct passwords with explicitly logged indication of which system used what password to log in while not having to deal with possibly permission differences due to using actually independent roles is valuable. That is, each server using a separate role to log in could lead to some servers having access to something or other while others don't pretty easily- if they're all logging in with the same role and just a different password, that's not going to happen. * Jeff Davis (pg...@j-davis.com) wrote: > Using a number seems weird to me because either: > > (a) if the number is always increasing you'd have to look to find the > number of the new slot to add and the old slot to remove; or > (b) if switched between two numbers (say 0 and 1), it would be error > prone because you'd have to remember which is the old one that can be > safely replaced Yeah, a number doesn't strike me as very good either. > Maybe a password is best described by its validity period rather than a > name? But what about passwords that don't expire? The validity idea is interesting but falls down
Re: [PoC/RFC] Multiple passwords, interval expirations
On Mon, Oct 9, 2023 at 2:31 AM Gurjeet Singh wrote:
>
> Next steps:
> - Break the patch into a series of smaller patches.
Please see attached the same v3 patch, but now split into 3 separate
patches. Each patch in the series depends on the previous patch to
have been applied. I have made sure that each patch passes `make
check` individually.
First patch adds the two new columns, rolsecondpassword and
rolsecondvaliduntil to the pg_authid shared catalog. This patch also
updates the corresponding pg_authid.dat file to set these values to
null for the rows populated during bootstrap. Finally, it adds code to
CreateRole() to set these columns' values to NULL for a role being
created.
The second patch updates the password extraction, verification
functions as well as authentication functions to honor the second
password, if any. There is more detailed description in the commit
message/body of the patch.
The third patch adds the SQL support to the ALTER ROLE command which
allows manipulation of both, the rolpassword and rolsecondpassword,
columns and their respective expiration timestamps,
rol[second]validuntil. This patch also adds regression tests for the
new SQL command, demonstrating the use of the new grammar.
v3-0001-Add-new-columns-to-pg_authid.patch
v3-0002-Update-password-verification-infrastructure-to-ha.patch
v3-0003-Added-SQL-support-for-ALTER-ROLE-to-manage-two-pa.patch
Best regards,
Gurjeet
http://Gurje.et
From bc7c35e53e421157c9425c198bc2557ad118a650 Mon Sep 17 00:00:00 2001
From: Gurjeet Singh
Date: Mon, 9 Oct 2023 11:36:05 -0700
Subject: [PATCH v3 1/3] Add new columns to pg_authid
Add two columns to pg_authid, namely rolsecondpassword and
rolsecondvaliduntil. These columns are added in preparation for the
password-rollover feature. These columns will store the hash and the
expiration time, repspectively, of a second password that the role can
use for login authentication.
---
src/backend/commands/user.c | 4 +++
src/include/catalog/pg_authid.dat | 45 ---
src/include/catalog/pg_authid.h | 2 ++
3 files changed, 36 insertions(+), 15 deletions(-)
diff --git a/src/backend/commands/user.c b/src/backend/commands/user.c
index ce77a055e5..0afaf74865 100644
--- a/src/backend/commands/user.c
+++ b/src/backend/commands/user.c
@@ -452,9 +452,13 @@ CreateRole(ParseState *pstate, CreateRoleStmt *stmt)
else
new_record_nulls[Anum_pg_authid_rolpassword - 1] = true;
+ new_record_nulls[Anum_pg_authid_rolsecondpassword - 1] = true;
+
new_record[Anum_pg_authid_rolvaliduntil - 1] = validUntil_datum;
new_record_nulls[Anum_pg_authid_rolvaliduntil - 1] = validUntil_null;
+ new_record_nulls[Anum_pg_authid_rolsecondvaliduntil - 1] = true;
+
new_record[Anum_pg_authid_rolbypassrls - 1] = BoolGetDatum(bypassrls);
/*
diff --git a/src/include/catalog/pg_authid.dat b/src/include/catalog/pg_authid.dat
index 6b4a0d..b326e48376 100644
--- a/src/include/catalog/pg_authid.dat
+++ b/src/include/catalog/pg_authid.dat
@@ -23,76 +23,91 @@
rolname => 'POSTGRES', rolsuper => 't', rolinherit => 't',
rolcreaterole => 't', rolcreatedb => 't', rolcanlogin => 't',
rolreplication => 't', rolbypassrls => 't', rolconnlimit => '-1',
- rolpassword => '_null_', rolvaliduntil => '_null_' },
+ rolpassword => '_null_', rolvaliduntil => '_null_',
+ rolsecondpassword => '_null_', rolsecondvaliduntil => '_null_' },
{ oid => '6171', oid_symbol => 'ROLE_PG_DATABASE_OWNER',
rolname => 'pg_database_owner', rolsuper => 'f', rolinherit => 't',
rolcreaterole => 'f', rolcreatedb => 'f', rolcanlogin => 'f',
rolreplication => 'f', rolbypassrls => 'f', rolconnlimit => '-1',
- rolpassword => '_null_', rolvaliduntil => '_null_' },
+ rolpassword => '_null_', rolvaliduntil => '_null_',
+ rolsecondpassword => '_null_', rolsecondvaliduntil => '_null_' },
{ oid => '6181', oid_symbol => 'ROLE_PG_READ_ALL_DATA',
rolname => 'pg_read_all_data', rolsuper => 'f', rolinherit => 't',
rolcreaterole => 'f', rolcreatedb => 'f', rolcanlogin => 'f',
rolreplication => 'f', rolbypassrls => 'f', rolconnlimit => '-1',
- rolpassword => '_null_', rolvaliduntil => '_null_' },
+ rolpassword => '_null_', rolvaliduntil => '_null_',
+ rolsecondpassword => '_null_', rolsecondvaliduntil => '_null_' },
{ oid => '6182', oid_symbol => 'ROLE_PG_WRITE_ALL_DATA',
rolname => 'pg_write_all_data', rolsuper => 'f', rolinherit => 't',
rolcreaterole => 'f', rolcreatedb => 'f', rolcanlogin => 'f',
rolreplication => 'f', rolbypassrls => 'f', rolconnlimit => '-1',
- rolpassword => '_null_', rolvaliduntil => '_null_' },
+ rolpassword => '_null_', rolvaliduntil => '_null_',
+ rolsecondpassword => '_null_', rolsecondvaliduntil => '_null_' },
{ oid => '3373', oid_symbol => 'ROLE_PG_MONITOR',
rolname => 'pg_monitor', rolsuper => 'f', rolinherit => 't',
rolcreaterole => 'f', rolcreatedb => 'f', rolcanlogin => 'f',
rolreplication => 'f', rolbypassrls => 'f', rolconnlimit => '-1',
-
Re: [PoC/RFC] Multiple passwords, interval expirations
On Sun, Oct 8, 2023 at 1:01 PM Gurjeet Singh wrote: > > On Fri, Oct 6, 2023 at 1:29 PM Jeff Davis wrote: > > > > On Thu, 2023-10-05 at 14:28 -0700, Gurjeet Singh wrote: > > > > > This way there's a notion of a 'new' and 'old' passwords. > > > > IIUC, you are proposing that there are exactly two slots, NEW and OLD. > > When adding a password, OLD must be unset and it moves NEW to OLD, and > > adds the new password in NEW. DROP only works on OLD. Is that right? > > Yes, that's what I was proposing. But thinking a bit more about it, > the _implicit_ shuffling of labels 'new' and 'old' doesn't feel right > to me. The password that used to be referred to as 'new' now > automatically gets labeled 'old'. > > > It's close to the idea of deprecation, except that adding a new > > password implicitly deprecates the existing one. I'm not sure about > > that -- it could be confusing. > > +1 > I believe we should fix the _names_ of the slots the 2 passwords are > stored in, and provide commands that manipulate those slots by > respective names; the commands should not implicitly move the > passwords between the slots. Additionally, we may provide functions > that provide observability info for these slots. I propose the slot > names FIRST and SECOND (I picked these because these keywords/tokens > already exist in the grammar, but not yet sure if the grammar rules > would allow their use; feel free to propose better names). FIRST > refers to the the existing slot, namely rolpassword. SECOND refers to > the new slot we'd add, that is, a pgauthid column named > rolsecondpassword. The existing commands, or when neither FIRST nor > SECOND are specified, the commands apply to the existing slot, a.k.a. > FIRST. Please see attached the patch that implements this proposal. The patch is named password_rollover_v3.diff, breaking from the name 'multiple_passwords...', since this patch limits itself to address the password-rollover feature. The multiple_password* series of patches had removed a critical functionality, which I believe is crucial from security perspective. When a user does not exist, or has no passwords, or has passwords that have expired, we must pretend to perform authentication (network packet exchanges) as normally as possible, so that the absence of user, or lack of (or expiration of) passwords is not revealed to an attacker. I have restored the original behaviour in the CheckPWChallengeAuth() function; see commit aba99df407 [2]. I looked for any existing keywords that may better fit the purpose of naming the slots, better than FIRST and SECOND, but I could not find any. Instead of DROP to remove the passwords, I tried DELETE and the grammar/bison did not complain about it; so DELETE is an option, too, but I feel DROP FIRST/SECOND/ALL PASSWORD is a better companion of ADD FIRST/SECOND PASSWORD syntax, in the same vein as ADD/DROP COLUMN. The doc changes are still missing, but the regression tests and the comments therein should provide a good idea of the user interface of this new feature. Documenting this behaviour in a succinct manner feels difficult; so ideas welcome for how to inform the reader that now a role is accompanied by two slots to store the passwords, and that the old commands operate on the first slot, and to operate on the second password slot one must use the new syntax. I guess it would be best to start the relevant section with "To support gradual password rollovers, Postgres provides the ability to store 2 active passwords at the same time. The passwords are referred to as FIRST and SECOND password. Each of these passwords can be changed independently, and each of these can have an associated expiration time, if necessary." Since these new commands are only available to ALTER ROLE (and not to CREATE ROLE), the corresponding command doc page also needs to be updated. Next steps: - Break the patch into a series of smaller patches. - Add TAP tests (test the ability to actually login with these passwords) - Add/update documentation - Add more regression tests The patch progress can be followed on the Git branch password_rollover_v3 [1]. This branch begins uses multiple_passwords_v4 as starting point, and removes unnecessary code (commit 326f60225f [3]) The v1 (and tombstone of v2) patches of password_rollover never finished as the consensus changed while they were in progress, but they exist as sibling branches of the v3 branch. [1]: https://github.com/gurjeet/postgres/commits/password_rollover_v3 [2]: https://github.com/gurjeet/postgres/commit/aba99df407a523357db2813f0eea0b45dbeb6006 [3]: https://github.com/gurjeet/postgres/commit/326f60225f0e660338fc9c276c8728dc10db435b Best regards, Gurjeet http://Gurje.et password_rollover_v3.diff Description: Binary data
Re: [PoC/RFC] Multiple passwords, interval expirations
On Fri, Oct 6, 2023 at 1:29 PM Jeff Davis wrote:
>
> On Thu, 2023-10-05 at 14:28 -0700, Gurjeet Singh wrote:
>
> > This way there's a notion of a 'new' and 'old' passwords.
>
> IIUC, you are proposing that there are exactly two slots, NEW and OLD.
> When adding a password, OLD must be unset and it moves NEW to OLD, and
> adds the new password in NEW. DROP only works on OLD. Is that right?
Yes, that's what I was proposing. But thinking a bit more about it,
the _implicit_ shuffling of labels 'new' and 'old' doesn't feel right
to me. The password that used to be referred to as 'new' now
automatically gets labeled 'old'.
> It's close to the idea of deprecation, except that adding a new
> password implicitly deprecates the existing one. I'm not sure about
> that -- it could be confusing.
+1
> We could also try using a verb like "expire" that could be coupled with
> a date, and that way all old passwords would always have some validity
> period.
Forcing the users to pick an expiry date for a password they intend to
rollover, when an expiry date did not exist before for that password,
feels like adding more burden to their password rollover decision
making. The dates and rules of password rollover may be a part of a
system external to their database, (wiki, docs, calendar, etc.) which
now they will be forced to translate into a timestamp to specify in
the rollover commands.
I believe we should fix the _names_ of the slots the 2 passwords are
stored in, and provide commands that manipulate those slots by
respective names; the commands should not implicitly move the
passwords between the slots. Additionally, we may provide functions
that provide observability info for these slots. I propose the slot
names FIRST and SECOND (I picked these because these keywords/tokens
already exist in the grammar, but not yet sure if the grammar rules
would allow their use; feel free to propose better names). FIRST
refers to the the existing slot, namely rolpassword. SECOND refers to
the new slot we'd add, that is, a pgauthid column named
rolsecondpassword. The existing commands, or when neither FIRST nor
SECOND are specified, the commands apply to the existing slot, a.k.a.
FIRST.
The user interface might look like the following:
-- Create a user, as usual
CREATE ROLE u1 PASSWORD 'p1' VALID UNTIL '2020/01/01';
-- This automatically occupies the 'first' slot
-- Add another password that the user can use for authentication.
ALTER ROLE u1 ADD SECOND PASSWORD 'p2' VALID UNTIL '2021/01/01';
-- Change both the passwords' validity independently; this solves the
-- problem with the previous '2-element stack' approach, where we
-- could not address the password at the bottom of the stack.
ALTER ROLE u1 SECOND PASSWORD VALID UNTIL '2022/01/01';
ALTER ROLE u1 [ [ FIRST ] PASSWORD ] VALID UNTIL '2022/01/01';
-- If, for some reason, the user wants to get rid of the latest password added.
ALTER ROLE u1 DROP SECOND PASSWORD;
-- Add a new password (p3) in 'second' slot
ALTER ROLE u1 ADD SECOND PASSWORD 'p3' VALID UNTIL '2023/01/01';
-- Attempting to add a password while the respective slot is occupied
-- results in error
ALTER ROLE u1 ADD [ [ FIRST ] PASSWORD ] 'p4' VALID UNTIL '2024/01/01';
ERROR: first password already exists
ALTER ROLE u1 ADD SECOND PASSWORD 'p4' VALID UNTIL '2024/01/01';
ERROR: second password already exists
-- Users can use this function to check whether a password slot is occupied
=> select password_exists('u1', 'first');
password_exists
-
t
=> select password_exists('u1', 'second');
password_exists
-
t
-- Remove all passwords from the role. Both, 'first' and 'second',
passwords are removed.
ALTER ROLE u1 DROP ALL PASSWORD;
Best regards,
Gurjeet
http://Gurje.et
Re: [PoC/RFC] Multiple passwords, interval expirations
On Sun, Oct 8, 2023 at 10:50:15AM -0700, Gurjeet Singh wrote: > On Sun, Oct 8, 2023 at 10:29 AM Bruce Momjian wrote: > > > > I was speaking of autoremoving in cases where we are creating a new one, > > and taking the previous new one and making it the old one, if that was > > not clear. > > Yes, I think I understood it differently. I understood it to mean that > this behaviour would apply to all passwords, those created by existing > commands, as well as to those created by new commands for rollover use > case. Whereas you meant this autoremove behaviour to apply only to > those passwords created by/for rollover related commands. I hope I've > understood your proposal correctly this time around :-) Yes, it is only during the addition of a new password when the previous new password becomes the new old password. The previous old password would need to have an rolvaliduntil in the past. > I believe the passwords created by rollover feature should > behave by the same rules as the rules for passwords created by > existing CREATE/ALTER ROLE commands. If we implement the behaviour to > delete expired passwords, then I believe that behaviour should apply > to all passwords, irrespective of which command/feature was used to > create a password. This would only apply when we are moving the previous new password to old and the old one is removed. -- Bruce Momjian https://momjian.us EDB https://enterprisedb.com Only you can decide what is important to you.
Re: [PoC/RFC] Multiple passwords, interval expirations
On Sun, Oct 8, 2023 at 10:29 AM Bruce Momjian wrote: > > I was speaking of autoremoving in cases where we are creating a new one, > and taking the previous new one and making it the old one, if that was > not clear. Yes, I think I understood it differently. I understood it to mean that this behaviour would apply to all passwords, those created by existing commands, as well as to those created by new commands for rollover use case. Whereas you meant this autoremove behaviour to apply only to those passwords created by/for rollover related commands. I hope I've understood your proposal correctly this time around :-) I believe the passwords created by rollover feature should behave by the same rules as the rules for passwords created by existing CREATE/ALTER ROLE commands. If we implement the behaviour to delete expired passwords, then I believe that behaviour should apply to all passwords, irrespective of which command/feature was used to create a password. Best regards, Gurjeet http://Gurje.et
Re: [PoC/RFC] Multiple passwords, interval expirations
On Sun, Oct 8, 2023 at 10:24:42AM -0700, Gurjeet Singh wrote: > On Fri, Oct 6, 2023 at 1:46 PM Bruce Momjian wrote: > > > > On Fri, Oct 6, 2023 at 01:20:03PM -0700, Jeff Davis wrote: > > > The basic problem, as I see it, is: how do we keep users from > > > accidentally dropping the wrong password? Generated unique names or > > > > I thought we could auto-remove old password if the valid-until date is > > in the past. > > Autoremoving expired passwords will surprise users, and not in a good > way. Making a password, even an expired one, disappear from the system > will lead to astonishment. Among uses of an expired password are cases > of it acting like a tombstone, and the case where the user may want to > extend the validity of a password, instead of having to create a new > one and change application configuration(s) to specify the new > password. I was speaking of autoremoving in cases where we are creating a new one, and taking the previous new one and making it the old one, if that was not clear. -- Bruce Momjian https://momjian.us EDB https://enterprisedb.com Only you can decide what is important to you.
Re: [PoC/RFC] Multiple passwords, interval expirations
On Fri, Oct 6, 2023 at 1:46 PM Bruce Momjian wrote: > > On Fri, Oct 6, 2023 at 01:20:03PM -0700, Jeff Davis wrote: > > The basic problem, as I see it, is: how do we keep users from > > accidentally dropping the wrong password? Generated unique names or > > I thought we could auto-remove old password if the valid-until date is > in the past. Autoremoving expired passwords will surprise users, and not in a good way. Making a password, even an expired one, disappear from the system will lead to astonishment. Among uses of an expired password are cases of it acting like a tombstone, and the case where the user may want to extend the validity of a password, instead of having to create a new one and change application configuration(s) to specify the new password. Best regards, Gurjeet http://Gurje.et
Re: [PoC/RFC] Multiple passwords, interval expirations
On Fri, Oct 6, 2023 at 01:20:03PM -0700, Jeff Davis wrote: > The basic problem, as I see it, is: how do we keep users from > accidentally dropping the wrong password? Generated unique names or I thought we could auto-remove old password if the valid-until date is in the past. You would need a separate ALTER command to sets its date in the past without that. Also, defining a new password could require setting the expiration date of the old password to make future additions easier. For pg_authid, I was thinking of columns: ADD rolpassword_old ADD rolvaliduntil_old EXISTS rolpassword EXISTS rolvaliduntil I did blog about the password rotation problem and suggested certificates: https://momjian.us/main/blogs/pgblog/2020.html#July_17_2020 -- Bruce Momjian https://momjian.us EDB https://enterprisedb.com Only you can decide what is important to you.
Re: [PoC/RFC] Multiple passwords, interval expirations
On Thu, 2023-10-05 at 14:28 -0700, Gurjeet Singh wrote: > This way there's a notion of a 'new' and 'old' passwords. IIUC, you are proposing that there are exactly two slots, NEW and OLD. When adding a password, OLD must be unset and it moves NEW to OLD, and adds the new password in NEW. DROP only works on OLD. Is that right? It's close to the idea of deprecation, except that adding a new password implicitly deprecates the existing one. I'm not sure about that -- it could be confusing. We could also try using a verb like "expire" that could be coupled with a date, and that way all old passwords would always have some validity period. That might make it a bit easier to manage if we do need more than two passwords. Regards, Jeff Davis
Re: [PoC/RFC] Multiple passwords, interval expirations
On Fri, 2023-10-06 at 14:26 -0500, Nathan Bossart wrote: > I guess it's more of the latter. Perhaps one potential use case > would be > short-lived credentials that are created on demand. Such a password > might > only be valid for something like 15 minutes, and many users might > have the > ability to request a password for the database role. I don't know > whether > there is a ton of demand for such a use case, and it might already be > solvable by just creating separate roles. In any case, if there's > general > agreement that we only want to target the rotation use case, that's > fine by > me. The basic problem, as I see it, is: how do we keep users from accidentally dropping the wrong password? Generated unique names or numbers don't solve that problem. Auto-incrementing or a created-at timestamp solves it in the sense that you can at least look at a system view and see if there's a newer one, but it's a little awkward. A validity period is a good fit if all passwords have a validity period and we don't change it, but gets awkward otherwise. I'm also worried about two kinds of clutter: * old passwords not being garbage-collected * the identifier of the current password always changing (perhaps fine if it'a a "created at" ID?) Regards, Jeff Davis
Re: [PoC/RFC] Multiple passwords, interval expirations
On Thu, Oct 05, 2023 at 01:09:36PM -0700, Jeff Davis wrote: > On Thu, 2023-10-05 at 14:04 -0500, Nathan Bossart wrote: >> That way, we needn't restrict this feature to 2 passwords for >> everyone. Perhaps 2 should be the default, but in any case, IMO we >> shouldn't design to only support 2. > > Are there use cases for lots of passwords, or is it just a matter of > not introducing an artificial limitation? I guess it's more of the latter. Perhaps one potential use case would be short-lived credentials that are created on demand. Such a password might only be valid for something like 15 minutes, and many users might have the ability to request a password for the database role. I don't know whether there is a ton of demand for such a use case, and it might already be solvable by just creating separate roles. In any case, if there's general agreement that we only want to target the rotation use case, that's fine by me. -- Nathan Bossart Amazon Web Services: https://aws.amazon.com
Re: [PoC/RFC] Multiple passwords, interval expirations
On Thu, Oct 5, 2023 at 1:09 PM Jeff Davis wrote: > > > > > I think this would be mighty confusing to users since it's not clear > > that > > adding a password will potentially invalidate a current password > > (which > > might be actively in use), but only if there are already 2 in the > > stack. I > > worry that such a desіgn might be too closely tailored to the > > implementation details. If we proceed with this design, perhaps we > > should > > consider ERROR-ing if a user tries to add a third password. > > I agree that the proposed language is confusing, especially because ADD > causes a password to be added and another one to be removed. But > perhaps there are some non-confusing ways to expose a similar idea. How about a language like the following (I haven't tried if this will work in the grammar we have): CREATE ROLE u1 PASSWORD 'p1'; ALTER ROLE u1ADD NEW PASSWORD 'p2'; ALTER ROLE u1 ADD NEW PASSOWRD 'p3'; ERROR: Cannot have more than 2 passwords at the same time. ALTER ROLE u1 DROP OLD PASSWORD; ALTER ROLE u1 ADD NEW PASSOWRD 'p3'; -- succeeds; forgets password 'p1'; p2 and p3 can be used to login ALTER ROLE u1 DROP NEW PASSWORD; -- forgets password 'p3'. Only 'p2' can be used to login ALTER ROLE u1 ADD NEW PASSOWRD 'p4'; -- succeeds; 'p2' and 'p4' can be used to login -- Set the valid-until of 'new' (p4) password ALTER ROLE u1 VALID UNTIL '2024/01/01'; -- If we need the ability to change valid-until of both old and new, we may allow something like the following. ALTER ROLE u1 [_NEW_ | OLD] VALID UNTIL '2024/01/01'; This way there's a notion of a 'new' and 'old' passwords. User cannot add a third password without explicitly dropping one of existing passwords (either old or new). At any time the user can choose to drop the old or the new password. Adding a new password will mark the current password as 'old'; if there's only old password (because 'new' was dropped) the 'old' password will remain intact and the new one will be placed in 'current'/new spot. So in normal course of operation, even for automated jobs, the expected flow to roll over the passwords would be: ALTER USER u1 DROP OLD PASSWORD; -- success if there is an old password; otherwise NOTICE: no old password ALTER USER u1 ADD NEW PASSWORD 'new-secret'; > The thing I like about Gurjeet's proposal is that it's well-targeted at > a specific use case rather than trying to be too general. That makes it > a little easier to avoid certain problems like having a process that > adds passwords and never removes the old ones (leading to weird > problems like 47000 passwords for one user). > > But it also feels strange to be limited to two -- perhaps the password > rotation schedule or policy just doesn't work with a limit of two, or > perhaps that introduces new kinds of mistakes. > > Another idea: what if we introduce the notion of deprecating a > password? I'll have to think more about it, but perhaps my above proposal addresses the use case you describe. > if we allow multiple deprecated passwords, we'd still have to come up > with some way to address them (names, numbers, validity period, > something). But by isolating the problem to deprecated passwords only, > it feels like the system is still being restored to a clean state with > at most one single current password. The awkwardness is contained to > old passwords which will hopefully go away soon anyway and not > represent permanent clutter. +1 Best regards, Gurjeet http://Gurje.et
Re: [PoC/RFC] Multiple passwords, interval expirations
On Thu, Oct 5, 2023 at 12:04 PM Nathan Bossart wrote: > > On Wed, Oct 04, 2023 at 10:41:15PM -0700, Gurjeet Singh wrote: > > The patches posted in this thread so far attempt to add the ability to > > allow the user to have an arbitrary number of passwords. I believe > > that allowing arbitrary number of passwords is not only unnecessary, > > but the need to name passwords, the need to store them in a shared > > catalog, etc. may actually create problems in the field. The > > users/admins will have to choose names for passwords, which they > > didn't have to previously. The need to name them may also lead to > > users storing password-hints in the password names (e.g. 'mom''s > > birthday', 'ex''s phone number', 'third password'), rendering the > > passwords weak. > > > > Moreover, allowing an arbitrarily many number of passwords will > > require us to provide additional infrastructure to solve problems like > > observability (which passwords are currently in use, and which ones > > have been effectively forgotten by applications), or create a nuisance > > for admins that can create more problems than it solves. > > IMHO neither of these problems seems insurmountable. Agreed. > Besides advising > against using hints as names, we could also automatically generate safe > names, or even disallow user-provided names entirely. Somehow naming passwords doesn't feel palatable to me. > And adding > observability for passwords seems worthwhile anyway. Agreed. > > So I propose that the feature should allow no more than 2 passwords > > for a role, each with their own validity periods. This eliminates the > > need to name passwords, because at any given time there are no more > > than 2 passwords; current one, and old one. This also eliminates the > > need for a shared catalog to hold passwords, because with the limit of > > 2 imposed, we can store the old password and its validity period in > > additional columns in the pg_authid table. > > Another approach could be to allow an abritrary number of passwords but to > also allow administrators to limit how many passwords can be associated to > each role. That way, we needn't restrict this feature to 2 passwords for > everyone. Perhaps 2 should be the default, but in any case, IMO we > shouldn't design to only support 2. I don't see a real use case to support more than 2 passwords. Allowing an arbitrary number of passwords might look good and clean from aesthetics and documentation perspective (no artificially enforced limits, as in the zero-one-infinity rule), but in absence of real use cases for that many passwords, I'm afraid we might end up with a feature that creates more and worse problems for the users than it solves. > > In essence, we create a stack that can hold 2 passwords. Pushing an > > element when it's full will make it forget the bottom element. Popping > > the stack makes it forget the top element, and the only remaining > > element, if any, becomes the top. > > I think this would be mighty confusing to users since it's not clear that > adding a password will potentially invalidate a current password (which > might be actively in use), but only if there are already 2 in the stack. Fair point. We can aid the user by emitting a NOTICE (or a WARNING) message whenever an old password is removed from the system because of addition of a new password. > I > worry that such a desіgn might be too closely tailored to the > implementation details. If we proceed with this design, perhaps we should > consider ERROR-ing if a user tries to add a third password. I did not have a stack in mind when developing the use case and the grammar, so implementation details did not drive this design. This new design was more of a response to the manageability nightmare that I could see the old approach may lead to. When writing the email I thought mentioning the stack analogy may make it easier to develop a mental model. I certainly won't suggest using it in the docs for explanation :-) > > -- If, for some reason, the user wants to get rid of the latest password > > added. > > -- Remove 'p2' (current password). The old password (p1), will be restored > > to > > -- rolpassword, along with its valid-until value. > > ALTER ROLE u1 PASSWORD NULL; > > -- This may come as a surprise to some users, because currently they expect > > the > > -- user to completely lose the ability to use passwords for login after this > > -- command. To get the old behavior, the user must now use the ALL PASSWORD > > -- NULL incantation; see below. > > -- Issuing this command one more time will remove even the restored > > password, > > -- hence leaving the user with no passwords. > > Is it possible to remove the oldest password added without removing the > latest password added? In the patch I have so far, ALTER ROLE u1 ADD PASSWORD '' (empty string) will drop the old password (what you asked for), and move the current password to rololdpassword (which is not exactly what you asked
Re: [PoC/RFC] Multiple passwords, interval expirations
On Thu, 2023-10-05 at 14:04 -0500, Nathan Bossart wrote: > IMHO neither of these problems seems insurmountable. Besides > advising > against using hints as names, we could also automatically generate > safe > names, or even disallow user-provided names entirely. I'd like to see what this looks like as a user-interface. Using a name seems weird because of the reasons Gurjeet mentioned. Using a number seems weird to me because either: (a) if the number is always increasing you'd have to look to find the number of the new slot to add and the old slot to remove; or (b) if switched between two numbers (say 0 and 1), it would be error prone because you'd have to remember which is the old one that can be safely replaced Maybe a password is best described by its validity period rather than a name? But what about passwords that don't expire? > And adding > observability for passwords seems worthwhile anyway. That might be useful just to know whether a user's password is even being used -- in case the admin makes a mistake and some other auth method is being used. Also it would help to know when a password can safely be removed. > > That way, we needn't restrict this feature to 2 passwords for > everyone. Perhaps 2 should be the default, but in any case, IMO we > shouldn't design to only support 2. Are there use cases for lots of passwords, or is it just a matter of not introducing an artificial limitation? Would it ever make sense to have a role that has two permanent passwords, or would that be an abuse of this feature? Any use cases I can think of would be better solved with multiple user that are part of the same group. > > I think this would be mighty confusing to users since it's not clear > that > adding a password will potentially invalidate a current password > (which > might be actively in use), but only if there are already 2 in the > stack. I > worry that such a desіgn might be too closely tailored to the > implementation details. If we proceed with this design, perhaps we > should > consider ERROR-ing if a user tries to add a third password. I agree that the proposed language is confusing, especially because ADD causes a password to be added and another one to be removed. But perhaps there are some non-confusing ways to expose a similar idea. The thing I like about Gurjeet's proposal is that it's well-targeted at a specific use case rather than trying to be too general. That makes it a little easier to avoid certain problems like having a process that adds passwords and never removes the old ones (leading to weird problems like 47000 passwords for one user). But it also feels strange to be limited to two -- perhaps the password rotation schedule or policy just doesn't work with a limit of two, or perhaps that introduces new kinds of mistakes. Another idea: what if we introduce the notion of deprecating a password? To remove a password, it would have to be deprecated first, and maybe that would cause a LOG or WARNING message to be emitted when used, or show up differently in some system view. And perhaps you could have at most one non-deprecated password. That would give a workflow something like (I'm not proposing these exact keywords): CREATE USER foo PASSWORD 'secret1'; ALTER USER foo DEPRECATE PASSWORD; -- 'secret1' still works ALTER USER foo PASSWORD 'secret2'; -- 'secret1' or 'secret2' works ... fix some applications SET log_deprecated_password_use = WARNING; ... WARNING: deprecated password used for user 'foo' ... fix some applications you forgot about ... warnings quiet down ALTER USER foo DROP DEPRECATED PASSWORD; -- only 'secret2' works If the user wants to un-deprecate a password (before they drop it, of course), they can do something like: ALTER USER foo PASSWORD NULL; -- 'secret2' removed ALTER USER foo UNDEPRECATE PASSWORD; -- 'secret1' restored if we allow multiple deprecated passwords, we'd still have to come up with some way to address them (names, numbers, validity period, something). But by isolating the problem to deprecated passwords only, it feels like the system is still being restored to a clean state with at most one single current password. The awkwardness is contained to old passwords which will hopefully go away soon anyway and not represent permanent clutter. Regards, Jeff Davis
Re: [PoC/RFC] Multiple passwords, interval expirations
On Wed, Oct 04, 2023 at 10:41:15PM -0700, Gurjeet Singh wrote: > The patches posted in this thread so far attempt to add the ability to > allow the user to have an arbitrary number of passwords. I believe > that allowing arbitrary number of passwords is not only unnecessary, > but the need to name passwords, the need to store them in a shared > catalog, etc. may actually create problems in the field. The > users/admins will have to choose names for passwords, which they > didn't have to previously. The need to name them may also lead to > users storing password-hints in the password names (e.g. 'mom''s > birthday', 'ex''s phone number', 'third password'), rendering the > passwords weak. > > Moreover, allowing an arbitrarily many number of passwords will > require us to provide additional infrastructure to solve problems like > observability (which passwords are currently in use, and which ones > have been effectively forgotten by applications), or create a nuisance > for admins that can create more problems than it solves. IMHO neither of these problems seems insurmountable. Besides advising against using hints as names, we could also automatically generate safe names, or even disallow user-provided names entirely. And adding observability for passwords seems worthwhile anyway. > So I propose that the feature should allow no more than 2 passwords > for a role, each with their own validity periods. This eliminates the > need to name passwords, because at any given time there are no more > than 2 passwords; current one, and old one. This also eliminates the > need for a shared catalog to hold passwords, because with the limit of > 2 imposed, we can store the old password and its validity period in > additional columns in the pg_authid table. Another approach could be to allow an abritrary number of passwords but to also allow administrators to limit how many passwords can be associated to each role. That way, we needn't restrict this feature to 2 passwords for everyone. Perhaps 2 should be the default, but in any case, IMO we shouldn't design to only support 2. > In essence, we create a stack that can hold 2 passwords. Pushing an > element when it's full will make it forget the bottom element. Popping > the stack makes it forget the top element, and the only remaining > element, if any, becomes the top. I think this would be mighty confusing to users since it's not clear that adding a password will potentially invalidate a current password (which might be actively in use), but only if there are already 2 in the stack. I worry that such a desіgn might be too closely tailored to the implementation details. If we proceed with this design, perhaps we should consider ERROR-ing if a user tries to add a third password. > -- If, for some reason, the user wants to get rid of the latest password > added. > -- Remove 'p2' (current password). The old password (p1), will be restored to > -- rolpassword, along with its valid-until value. > ALTER ROLE u1 PASSWORD NULL; > -- This may come as a surprise to some users, because currently they expect > the > -- user to completely lose the ability to use passwords for login after this > -- command. To get the old behavior, the user must now use the ALL PASSWORD > -- NULL incantation; see below. > -- Issuing this command one more time will remove even the restored password, > -- hence leaving the user with no passwords. Is it possible to remove the oldest password added without removing the latest password added? -- Nathan Bossart Amazon Web Services: https://aws.amazon.com
Re: [PoC/RFC] Multiple passwords, interval expirations
I had an idea to simplify this feature/patch and after some validation in internal discussions, I am posting the new approach here. I'd appreciate any feedback and comments. To begin with, the feature we are chasing is to make it convenient for the users to rollover their passwords. Currently there is no easy way to rollover passwords without increasing the risk of an application outage. After a password change, the users/admins have to rush to change the password in all locations where it is stored. There is a window of time where if the application password is not changed to the new one, and the application tries to connect/reconnect for any reason, the application will fail authentication and lead to an outage. I personally haven't seen any attempts by any application/driver/framework to solve this problem in the wild, so following is just me theorizing how one may solve this problem on the application side; there may be other ways in which others may solve this problem. The application may be written in such a way that upon password authentication failure, it tries again with a second password. The application's configuration file (or environment variables) may allow specifying 2 passwords at the same time, and the application will keep trying these 2 passwords alternatively until it succeeds or the user restarts it with a new configuration. With such a logic in place in their application, the users may first change the configuration of all the instances of the application to hold the new password along with the old/current working password, and only then change the password in the database. This way, in the event of an application instance start/restart either the old password will succeed, or the new password will. There may be other ways to solve this problem, but I can't imagine any of those ways to be convenient and straightforward. At least not as convenient as it can be if the database itself allowed for storing both the passwords, and honored both passwords at the same time, while allowing to associate a separate validity period with each of the passwords. The patches posted in this thread so far attempt to add the ability to allow the user to have an arbitrary number of passwords. I believe that allowing arbitrary number of passwords is not only unnecessary, but the need to name passwords, the need to store them in a shared catalog, etc. may actually create problems in the field. The users/admins will have to choose names for passwords, which they didn't have to previously. The need to name them may also lead to users storing password-hints in the password names (e.g. 'mom''s birthday', 'ex''s phone number', 'third password'), rendering the passwords weak. Moreover, allowing an arbitrarily many number of passwords will require us to provide additional infrastructure to solve problems like observability (which passwords are currently in use, and which ones have been effectively forgotten by applications), or create a nuisance for admins that can create more problems than it solves. So I propose that the feature should allow no more than 2 passwords for a role, each with their own validity periods. This eliminates the need to name passwords, because at any given time there are no more than 2 passwords; current one, and old one. This also eliminates the need for a shared catalog to hold passwords, because with the limit of 2 imposed, we can store the old password and its validity period in additional columns in the pg_authid table. The patches so far also add a notion of max validity period of passwords, which only a superuser can override. I believe this is a useful feature, but that feature can be dealt with separately, independent of password rollover feature. So in the newer patches I will not include the relevant GUC and code. With the above being said, following is the user interface I can think of that can allow for various operations that users may need to perform to rollover their passwords. The 'ADD PASSWORD' and 'ALL PASSWORD' are additions to the grammar. rololdpassword and rololdvaliduntil will be new columns in pg_authid that will hold the old password and its valid-until value. In essence, we create a stack that can hold 2 passwords. Pushing an element when it's full will make it forget the bottom element. Popping the stack makes it forget the top element, and the only remaining element, if any, becomes the top. -- Create a user, as usual CREATE ROLE u1 PASSWORD 'p1' VALID UNTIL '2020/01/01'; -- Add another password that the user can use for authentication. This moves -- the 'p1' password hash and its valid-until value to rololdpassword and -- rololdvaliduntil, respectively. ALTER ROLE u1 ADD PASSWORD 'p2' VALID UNTIL '2021/01/01'; -- Change the password 'p2's (current password's) validity ALTER ROLE u1 VALID UNTIL '2022/01/01'; -- Note that currently I don't have a proposal for how to change the old -- password's validity period, without deleting the latest/main
Re: [PoC/RFC] Multiple passwords, interval expirations
On Mon, 2023-09-25 at 00:31 -0700, Gurjeet Singh wrote: > Please see attached v4 of the patch. The patch takes care of rebase > to > the master/17-devel branch, and includes some changes, too. FWIW I got some failures applying. I didn't investigate much, and instead I looked at your git branch (7a35619e). > Moreover, before the patch, in case of CheckPasswordAuth(), the error > (if any) would have been thrown _after_ network communication done by > sendAuthRequest() call. But with this patch, the error is thrown > before the network interaction, hence this changes the order of > network interaction and the error message. This may have security > implications, too, but I'm unable to articulate one right now. You mean before v3 or before v4? Is this currently a problem in v4? > Open question: If a client is capable of providing just md5 passwords > handshake, and because of pg_hba.conf setting, or because the role > has > at least one SCRAM password (essentially the 3rd case you mention > above: pg_hba md5 + md5 and scram pws -> scram), the server will > respond with a SASL/SCRAM authentication response, and that would > break the backwards compatibility and will deny access to the client. > Does this make it necessary to use a newer libpq/client library? Perhaps you can try the MD5 passwords first, and only if they fail, move on to try scram passwords? > Comments? IIUC, for the case of multiple scram passwords, we use the salt to select the right scram password, and then proceed from there? I'm not very excited about the idea of naming passwords, or having passwords with default names. I can't think of anything better right now, so it might be OK. > - Add tests > - Add/update documentation These are needed to provide better review. Regards, Jeff Davis
Re: [PoC/RFC] Multiple passwords, interval expirations
Greetings, On Fri, Jul 1, 2022 at 10:51 Brindle, Joshua wrote: > > On 6/30/22 8:20 PM, Stephen Frost wrote: > > * Gurjeet Singh (gurj...@singh.im) wrote: > >> I am planning on picking it up next week; right now picking up steam, > >> and reviewing a different, smaller patch. > > Great! Glad that others are interested in this. > > > >> At his behest, I had a conversation with Joshua (OP), and have his > >> support to pick up and continue working on this patch. I have a some > >> ideas of my own, on what this patch should do, but since I haven't > >> fully reviewed the (bulky) patch, I'll reserve my proposals until I > >> wrap my head around it. > > I'd be curious as to your thought as to what the patch should be doing. > > Joshua and I had discussed it at some length as he was working on it. > > > Adding myself to the CC list here /waves Hi! I gave Gurjeet a bit of a brain dump on what I had planned (and what > we'd talked about), though he's free to take it in a different direction > if he wants. Perhaps though would certainly like this to patch to be useful for the use-cases that we had discussed, naturally. :) >> Please expect some activity on this patch towards the end of next week. > > I've gone ahead and updated it, cleaned up a couple things, and make it > > so that check-world actually passes with it. Attached is an updated > > version and I'll add it to the July commitfest. > > Ah, thanks. Hopefully it wasn't too horrible of a rebase. Wasn’t too bad.. needs more clean-up, there was some white space issues and some simple re-base stuff, but then the support for “md5” pg_hba option was broken for users with SCRAM passwords because we weren’t checking if there was a SCRAM pw stored and upgrading to SCRAM in that case. That’s the main case that I fixed. We will need to document this though, of course. The patch I submitted should basically do: pg_hba md5 + md5-only pws -> md5 auth used pg_hba md5 + scram-only pws -> scram pg_hba md5 + md5 and scram pws -> scram pg_hba scram -> scram Not sure if we need to try and do something to make it possible to have pg_hba md5 + mixed pws and have md5 used but it’s tricky as we would have to know on the server side early on if that’s what we want to do. We could add an option to md5 to say “only do md5” maybe but I’m also inclined to not bother and tell people to just get moved to scram already. For my 2c, I’d also like to move to having a separate column for the PW type from the actual secret but that’s largely an independent change. Thanks! Stephen >
Re: [PoC/RFC] Multiple passwords, interval expirations
I am planning on picking it up next week; right now picking up steam, and reviewing a different, smaller patch. At his behest, I had a conversation with Joshua (OP), and have his support to pick up and continue working on this patch. I have a some ideas of my own, on what this patch should do, but since I haven't fully reviewed the (bulky) patch, I'll reserve my proposals until I wrap my head around it. Please expect some activity on this patch towards the end of next week. BCC: Joshua's new work email. Best regards, Gurjeet http://Gurje.et On Wed, Jun 29, 2022 at 2:27 PM Stephen Frost wrote: > > Greetings, > > On Wed, Jun 29, 2022 at 17:22 Jacob Champion wrote: >> >> On 4/8/22 10:04, Joshua Brindle wrote: >> > It's unclear if I will be able to continue working on this featureset, >> > this email address will be inactive after today. >> >> I'm assuming the answer to this was "no". Is there any interest out >> there to pick this up for the July CF? > > > Short answer to that is yes, I’m interested in continuing this (though > certainly would welcome it if there are others who are also interested, and > may be able to bring someone else to help work on it too but that might be > more August / September time frame). > > Thanks, > > Stephen
Re: [PoC/RFC] Multiple passwords, interval expirations
Greetings, On Wed, Jun 29, 2022 at 17:22 Jacob Champion wrote: > On 4/8/22 10:04, Joshua Brindle wrote: > > It's unclear if I will be able to continue working on this featureset, > > this email address will be inactive after today. > > I'm assuming the answer to this was "no". Is there any interest out > there to pick this up for the July CF? Short answer to that is yes, I’m interested in continuing this (though certainly would welcome it if there are others who are also interested, and may be able to bring someone else to help work on it too but that might be more August / September time frame). Thanks, Stephen >
Re: [PoC/RFC] Multiple passwords, interval expirations
On 4/8/22 10:04, Joshua Brindle wrote: > It's unclear if I will be able to continue working on this featureset, > this email address will be inactive after today. I'm assuming the answer to this was "no". Is there any interest out there to pick this up for the July CF? --Jacob
Re: [PoC/RFC] Multiple passwords, interval expirations
On Wed, Mar 2, 2022 at 9:58 AM Joshua Brindle wrote: > > This is not intended for PG15. > > Attached are a proof of concept patchset to implement multiple valid > passwords, which have independent expirations, set by a GUC or SQL > using an interval. > > postgres=# select * from pg_auth_password ; > roleid | name | >password > | expiration > +-+--- > +--- > 10 | __def__ | > SCRAM-SHA-256$4096:yGiHIYPwc2az7xj/7TIyTA==$OQL/AEcEY1yOCNbrZEj4zDvNnOLpIqltOW1uQvosLvc=:9VRRppuIkSrwhiBN5ePy8wB1y > zDa/2uX0WUx6gXi93E= | > 16384 | __def__ | > SCRAM-SHA-256$4096:AA==$1Ivp4d+vAWxowpuGEn05KR9lxyGOms3yy85k3D7XpBg=:k8xUjU6xrJG17PMGa/Zya6pAE > /M7pEDaoIFmWvNIEUg= | 2022-03-02 06:52:31.217193-08 > 16384 | newone | > SCRAM-SHA-256$4096:AA==$WK3+41CCGDognSnZrtpHhv00z9LuVUjHR1hWq8T1+iE=:w2C5GuhgiEB7wXqPxYfxBKB+e > hm4h6Oeif1uzpPIFVk= | 2022-03-03 06:47:53.728159-08 > (3 rows) There's obviously a salt problem here that I'll need to fix that apparently snuck in at the last rebase, but this brings up one aspect of the patchset I didn't mention in the original email: For the SCRAM protocol to work as is with existing clients the salt for each password must be the same. Right now ALTER USER will find and reuse the salt, but a user passing in a pre-computed SCRAM secret currently has no way to get the salt. for \password (we'll need a new one that takes a password name) I was thinking libpq could hold onto the salt that was used to log in, but for outside computation we'll need some way for the client to request it. None of that is done yet.
