Re: allowing "map" for password auth methods with clientcert=verify-full

Поиск
Список
Период
Сортировка
От Stephen Frost
Тема Re: allowing "map" for password auth methods with clientcert=verify-full
Дата
Msg-id 20211108203240.GN20998@tamriel.snowman.net
обсуждение исходный текст
Ответ на Re: allowing "map" for password auth methods with clientcert=verify-full  (Jacob Champion <pchampion@vmware.com>)
Ответы Re: allowing "map" for password auth methods with clientcert=verify-full  (Jacob Champion <pchampion@vmware.com>)
Список pgsql-hackers
Greetings,

* Jacob Champion (pchampion@vmware.com) wrote:
> On Wed, 2021-10-27 at 12:53 -0400, Jonathan S. Katz wrote:
> > The patch I propose just layers on top of the existing functionality --
> >   you could even argue that it's "fixing a bug" that we did not add the
> > current "map" support for the case of "clientcert=verify-full" given we
> > do introspect the certificate CN to see if it matches the SQL role name.
>
> Well, also to Tom's earlier point, though, this is a different sort of
> mapping. Which "map" should we use if someone combines
> clientcert=verify-full with an auth method which already uses a map
> itself? Does the DBA want to map the auth name, the cert name, or both?

My understanding of the mapping system with pg_ident has always been
that it's a mapping from the 'authenticated user' to the 'user in PG'
and the point is to check if that mapping is allowed but not to actually
change anything about who the ultimately logged in user is.

This is pretty clear and simple when the two are entirely disjoint- that
is, with 'peer' or 'gssapi' or 'cert', the authenticated user will be
whatever the authentication system thinks it is- the Unix username for
peer, the Kerberos principle for gssapi, the DN or CN for cert.

In all of the above cases, the username provided to us by the end user
is the role they're trying to log in as and the mapping is just there to
check if that's allowed based on the *authenticated username*.  Perhaps
not surprisingly, 21.2 could use some improvement on this, but it's
certainly the case that the mapping is only there as a permission check
and does not actually change who the user is logging into the system as.
Whatever username is in the startup packet is the role that the user
will be logged in as if they're allowed to log in as that role.

Now, when PG is involved in the authentication of the user, then I agree
that it gets more interesting.  That is- the user wants to log in as
user u1, they have a certificate that has a DN of u1/user/domain, and we
want to verify that they know the password and present the right
certificate- but which password do they need to know?

Today, there's really only one possible answer- the username in the
startup packet, because we don't have any concept of "authenticate to PG
as user u1 and then be logged in as user u2".  If we were to support a
mapping for scram or md5, we'd really need the user to tell us both who
to authenticate as and the user to log into PG as, and then we'd use
pg_ident.conf to ensure that such a mapping is allowed.  If we wanted to
implement something along the lines of "user authenticates as X but is
logged in as Y" automatically, that would need to be something other
than pg_ident.conf, imv.  I see that's been discussed a bit on the other
thread I was explicitly trying to ignore and glad that it's more-or-less
coming to the same conclusion.

Where does that leave us with what Jonathan is suggesting though?  For
my 2c, we shouldn't allow 'map=' to be used for scram or md5 because
it'll just confuse users, until and unless we actually do the PGAUTHUSER
thing and then we can allow 'map=' to check if that mapping is allowed
and the actual SCRAM PW check is done against PGAUTHUSER and then the
logged in user is the user as specified in the startup packet, assuming
that mapping is allowed.  For Jonathan's actual case though, we should
add a 'certmap' option instead and have that be explicitly for the case
where it's scram w/ clientcert=verify-full and then we check the mapping
of the DN/CN to the startup-packet username.  There's no reason this
couldn't also work with a 'map' specified and PGAUTHUSER set and SCRAM
used to verify against that at the same time.

> The current usermap support is piecemeal and I'd like to see it
> completed, but I think you may be painting yourself into a corner if
> you fix it in this way. (From a quick look at the patch, I'm also
> worried that this happens to work by accident, but that may just be
> FUD.)

I don't think it's an accident that it works, but a few comments and
a more explicit option for the user interface would be good.  Admins
would be confused if 'map=xyz' was accepted for SCRAM but then didn't
actually do anything.

> > I think in the context of doing any new work, I'd step back and ask what
> > problem is this solving? The main one I think of is an integration with
> > a SSO system has a credential with an identifier that does not match
> > it's credential in PostgreSQL? (That would be the case I was working on,
> > though said case was borrowed from our docs). Are there other cases?
> >
> > That said, what would make it easier to manage it then? Maybe a lot of
> > this is documenting and some expansion on what the pg_ident.conf file
> > can do (per Andrew's suggestion). And maybe a new name for said file.
>
> I agree that the authorization system is due for a tuneup, for what
> it's worth. Some of the comments from Magnus on my LDAP patch [1] kind
> of hinted in that direction as well, I think, even if my approach is
> rejected in the end.

Perhaps not a surprise, but I continue to be against the idea of adding
anything more to the insecure hack that is our LDAP auth method.  We
should be moving away from that, not adding to it.

That this would also require a new connection option / envvar to tell us
who the user wants to authenticate to LDAP as doesn't exactly make me
any more thrilled with it.

Just my 2c though and I know others don't necessarily agree with me on
this point.

Thanks,

Stephen

Вложения

В списке pgsql-hackers по дате отправления:

Предыдущее
От: Justin Pryzby
Дата:
Сообщение: Re: make tuplestore helper function
Следующее
От: Stephen Frost
Дата:
Сообщение: Re: [PATCH v2] use has_privs_for_role for predefined roles