Re: [PoC/RFC] Multiple passwords, interval expirations

Started by Stephen Frostabout 2 years ago7 messages
#1Stephen Frost
sfrost@snowman.net

Greetings,

* Nathan Bossart (nathandbossart@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 (pgsql@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 when you want multiple
passwords that have the same validity period (or which all don't have
any expiration).

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.

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.

Yup, +100 on this.

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

I don't see logs or warnings as being at all useful for this kind of
thing. Making it available through a view would work- but I don't think
we need to go into the deprecation language ourselves as part of the
grammer and instead should let users write their own queries against the
views we provide to see what passwords are being used and what aren't.

* Nathan Bossart (nathandbossart@gmail.com) wrote:

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.

Agreed, this seems like another good example of why we shouldn't design
this for just 2.

* Jeff Davis (pgsql@j-davis.com) wrote:

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.

Providing admins a way of seeing which passwords have been recently used
seems like a clear way to minimize, at least, the risk of the wrong
password being dropped. Allowing them to control the name feels like
it's another good way to minimize the risk since it's something they can
define and they can include in the name whatever info they need to
figure out if it's the one that's no longer being used, or not. Forcing
the use of numbers or of validation periods seems like it'd make it
harder on users to avoid this risk, not easier.

Allowing per-password expiration is another way to address the issue of
the wrong password being removed by essentially taking the password away
and seeing what breaks while having an easy way to bring it back if
something important stops working.

I'm also worried about two kinds of clutter:

* old passwords not being garbage-collected

I'm not terribly concerned with this if the password's validity has
passed and therefore it can't be used any longer. I could agree with
the concern about it being an issue if the passwords all stay valid
forever. I'll point out that we arguably have this problem with roles
already today and it doesn't seem to be a huge issue- PG has no idea how
long that user is actually valid for, the admin needs to have something
external to PG to deal with this.

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

Thanks,

Stephen

#2Jeff Davis
pgsql@j-davis.com
In reply to: Stephen Frost (#1)

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

#3Stephen Frost
sfrost@snowman.net
In reply to: Jeff Davis (#2)

Greetings,

* Jeff Davis (pgsql@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
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?

As mentioned, we have this general idea already; a simplified CREATE
INDEX syntax can be used as an example:

CREATE INDEX [ [ IF NOT EXISTS ] name ] ON table_name ...

and so we could have:

CREATE PASS [ [ IF NOT EXISTS ] name ] FOR role ...

Giving the user the option of providing a name, but just generating one
if the user declines to include a name. Naturally, users would have to
have some way to look up the names but that doesn't seem difficult to
provide such as through some \du command, along with appropriate views,
just like we have for indexes.

I do generally feel like the original patch was lacking when it came to
syntax and password management functions, but I don't think the answer
to that is to just throw away the whole concept of multiple passwords in
favor of only supporting a very limited number of them. I'd think we'd
work towards improving on the syntax to support what we think users will
need to make all of this work smoothly.

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

Do we need a current or primary? How is that defined, exactly? If we
want it, we could surely create it and make it work, but we'd need to
have a good understand of just what it is and why it's the 'current' or
'primary' and I'm not sure that we've actually got a good definition for
that and I'm not convinced yet that we should.

Thanks,

Stephen

#4Jeff Davis
pgsql@j-davis.com
In reply to: Stephen Frost (#3)

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

#5Stephen Frost
sfrost@snowman.net
In reply to: Jeff Davis (#4)

Greetings,

* Jeff Davis (pgsql@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

#6Jeff Davis
pgsql@j-davis.com
In reply to: Stephen Frost (#5)

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

#7Stephen Frost
sfrost@snowman.net
In reply to: Jeff Davis (#6)

Greetings,

* Jeff Davis (pgsql@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