Proposal: Support custom authentication methods using hooks

Hi all,

On Thu, Feb 17, 2022 at 11:25 AM samay sharma <smilingsamay@gmail.com>
wrote:

Hi all,

I wanted to submit a patch to expose 2 new hooks (one for the
authentication check and another one for error reporting) in auth.c. These
will allow users to implement their own authentication methods for Postgres
or add custom logic around authentication.

A use case where this is useful are environments where you want
authentication to be centrally managed across different services. This is a
common deployment model for cloud providers where customers like to use
single sign on and authenticate across different services including
Postgres. Implementing this now is tricky as it requires syncing that
authentication method's credentials with Postgres (and that gets trickier
with TTL/expiry etc.). With these hooks, you can implement an extension to
check credentials directly using the authentication provider's APIs.

To enable this, I've proposed adding a new authentication method "custom"
which can be specified in pg_hba.conf and takes a mandatory argument
"provider" specifying which authentication provider to use. I've also moved
a couple static functions to headers so that extensions can call them.

Sample pg_hba.conf line to use a custom provider:

host all all ::1/128 custom
provider=test

As an example and a test case, I've added an extension named
test_auth_provider in src/test/modules which fetches credentials from
a pre-defined array. I've also added tap tests for the extension to test
this functionality.

One constraint in the current implementation is that we allow only one
authentication provider to be loaded at a time. In the future, we can add
more functionality to maintain an array of hooks and call the appropriate
one based on the provider name in the pg_hba line.

A couple of my tests are flaky and sometimes fail in CI. I think the
reason for that is I don't wait for pg_hba reload to be processed before
checking logs for error messages. I didn't find an immediate way to address
that and I'm looking into it but wanted to get an initial version out for
feedback on the approach taken and interfaces. Once those get finalized, I
can submit a patch to add docs as well.

I wanted to submit a v2 of my patches.

To fix the flaky tests, I decided to avoid checking the log files for
pg_hba reload errors and rely on the output of pg_hba_file_rules. While
doing that, I found two bugs (in my code) which were causing the custom
provider line to not be outputted correctly in pg_hba_file_rules.

This updated patch-set fixes those bugs and also uses pg_hba_file_rules to
check for errors arising due to improper configuration. After these
changes, I've stopped seeing CI failures.

Looking forward to feedback on the overall change and the approach taken.

Regards,
Samay

Hi Samay,

I wanted to submit a patch to expose 2 new hooks (one for the authentication check and another one for error reporting) in auth.c. These will allow users to implement their own authentication methods for Postgres or add custom logic around authentication.

I like the idea - PostgreSQL is all about extendability. Also, well
done with TAP tests and an example extension. This being said, I
didn't look at the code yet, but cfbot seems to be happy with it:
http://cfbot.cputube.org/

One constraint in the current implementation is that we allow only one authentication provider to be loaded at a time. In the future, we can add more functionality to maintain an array of hooks and call the appropriate one based on the provider name in the pg_hba line.

This sounds like a pretty severe and unnecessary limitation to me. Do
you think it would be difficult to bypass it in the first
implementation?

--
Best regards,
Aleksander Alekseev

On 2/24/22 04:16, Aleksander Alekseev wrote:

Hi Samay,

I wanted to submit a patch to expose 2 new hooks (one for the authentication check and another one for error reporting) in auth.c. These will allow users to implement their own authentication methods for Postgres or add custom logic around authentication.

I like the idea - PostgreSQL is all about extendability. Also, well
done with TAP tests and an example extension. This being said, I
didn't look at the code yet, but cfbot seems to be happy with it:
http://cfbot.cputube.org/

One constraint in the current implementation is that we allow only one authentication provider to be loaded at a time. In the future, we can add more functionality to maintain an array of hooks and call the appropriate one based on the provider name in the pg_hba line.

This sounds like a pretty severe and unnecessary limitation to me. Do
you think it would be difficult to bypass it in the first
implementation?

Yeah, I think we  would want a set of providers that could be looked up
at runtime.

I think this is likely to me material for release 16, so there's plenty
of time to get it right.

cheers

andrew

--
Andrew Dunstan
EDB: https://www.enterprisedb.com

Hi Aleksander,

On Thu, Feb 24, 2022 at 1:17 AM Aleksander Alekseev <
aleksander@timescale.com> wrote:

Hi Samay,

I wanted to submit a patch to expose 2 new hooks (one for the

authentication check and another one for error reporting) in auth.c. These
will allow users to implement their own authentication methods for Postgres
or add custom logic around authentication.

I like the idea - PostgreSQL is all about extendability. Also, well
done with TAP tests and an example extension. This being said, I
didn't look at the code yet, but cfbot seems to be happy with it:
http://cfbot.cputube.org/

Thank you!

One constraint in the current implementation is that we allow only one

authentication provider to be loaded at a time. In the future, we can add
more functionality to maintain an array of hooks and call the appropriate
one based on the provider name in the pg_hba line.

This sounds like a pretty severe and unnecessary limitation to me. Do
you think it would be difficult to bypass it in the first
implementation?

Just to clarify, the limitation is around usage of multiple custom
providers but does allow users to use the existing methods with one custom
authentication method. The reasons I thought this is ok to start with are:
* It allows users to plugin a custom authentication method which they can't
do today.
* Users *generally* have an authentication method for their database (eg.
we use ldap for authentication, or we use md5 passwords for
authentication). While it is possible, I'm not sure how many users use
completely different authentication methods (other than standard ones like
password and trust) for different IPs/databases for their same instance.

So, I thought this should be good enough for a number of use cases.

I thought about adding support for multiple custom providers and the
approach I came up with is: Maintaining a list of all providers (their
names, check functions and error functions), making sure they are all valid
while parsing pg_hba.conf and calling the right one when we see the custom
keyword in pg_hba.conf based on name. I'm not sure (I haven't yet checked)
if we have other such hooks in Postgres where multiple extensions use them
and Postgres calls the right hook based on input (instead of just calling
whoever has the hook).

Therefore, I wanted to start with something simple so users can begin using
auth methods of their choice, and add multiple providers as an enhancement
after doing more research and coming up with the right way to implement it.

That being said, any thoughts on the approach I mentioned above? I'll
look into it and see if it's not too difficult to implement this.

Regards,
Samay

On Thu, 2022-02-17 at 11:25 -0800, samay sharma wrote:

To enable this, I've proposed adding a new authentication method
"custom" which can be specified in pg_hba.conf and takes a mandatory
argument "provider" specifying which authentication provider to use.
I've also moved a couple static functions to headers so that
extensions can call them.

Sample pg_hba.conf line to use a custom provider:

host all all ::1/128
custom provider=test

One caveat is that this only works given information available from
existing authentication methods, because that's all the client
supports. In practice, it seems to only be useful with plaintext
password authentication over an SSL connection.

I still like the approach though. There's a lot of useful stuff you can
do at authentication time with only the connection information and a
password. It could be useful to authenticate against different
services, or some kind of attack detection, etc.

Regards,
Jeff Davis

Jeff Davis <pgsql@j-davis.com> writes:

On Thu, 2022-02-17 at 11:25 -0800, samay sharma wrote:

To enable this, I've proposed adding a new authentication method
"custom" which can be specified in pg_hba.conf and takes a mandatory
argument "provider" specifying which authentication provider to use.

One caveat is that this only works given information available from
existing authentication methods, because that's all the client
supports. In practice, it seems to only be useful with plaintext
password authentication over an SSL connection.

... and, since we can't readily enforce that the client only sends
those cleartext passwords over suitably-encrypted connections, this
could easily be a net negative for security. Not sure that I think
it's a good idea.

regards, tom lane

Hi,

On 2022-02-24 17:02:45 -0800, Jeff Davis wrote:

On Thu, 2022-02-17 at 11:25 -0800, samay sharma wrote:
One caveat is that this only works given information available from
existing authentication methods, because that's all the client
supports. In practice, it seems to only be useful with plaintext
password authentication over an SSL connection.

Why is it restricted to that? You could do sasl negotiation as well from what
I can see? And that'd theoretically also allow to negotiate whether the client
supports different ways of doing auth? Not saying that that's easy, but I
don't think it's a fundamental restriction.

I also can imagine things like using selinux labeling of connections.

We have several useful authentication technologies built ontop of plaintext
exchange. Radius, Ldap, Pam afaics could be implemented as an extension?

Greetings,

Andres Freund

On Thu, 2022-02-24 at 19:47 -0800, Andres Freund wrote:

Why is it restricted to that? You could do sasl negotiation as well
from what
I can see? And that'd theoretically also allow to negotiate whether
the client
supports different ways of doing auth? Not saying that that's easy,
but I
don't think it's a fundamental restriction.

Good point! It would only work with enhanced clients though -- maybe in
the future we'd make libpq pluggable with new auth methods?

We have several useful authentication technologies built ontop of
plaintext
exchange. Radius, Ldap, Pam afaics could be implemented as an
extension?

Yes, and it means that we won't have to extend that list in core in the
future when new methods become popular.

Regards,
Jeff Davis

On Thu, 2022-02-24 at 20:47 -0500, Tom Lane wrote:

... and, since we can't readily enforce that the client only sends
those cleartext passwords over suitably-encrypted connections, this
could easily be a net negative for security. Not sure that I think
it's a good idea.

I don't understand your point. Can't you just use "hostssl" rather than
"host"?

Also there are some useful cases that don't really require SSL, like
when the client and host are on the same machine, or if you have a
network secured some other way.

Regards,
Jeff Davis

Jeff Davis <pgsql@j-davis.com> writes:

On Thu, 2022-02-24 at 20:47 -0500, Tom Lane wrote:

... and, since we can't readily enforce that the client only sends
those cleartext passwords over suitably-encrypted connections, this
could easily be a net negative for security. Not sure that I think
it's a good idea.

I don't understand your point. Can't you just use "hostssl" rather than
"host"?

My point is that sending cleartext passwords over the wire is an
insecure-by-definition protocol that we shouldn't be encouraging
more use of.

regards, tom lane

Hi,

On 2022-02-25 09:33:45 -0800, Jeff Davis wrote:

On Thu, 2022-02-24 at 20:47 -0500, Tom Lane wrote:

... and, since we can't readily enforce that the client only sends
those cleartext passwords over suitably-encrypted connections, this
could easily be a net negative for security. Not sure that I think
it's a good idea.

I don't understand your point. Can't you just use "hostssl" rather than
"host"?

And the extension could check Port->ssl_in_use before sendAuthRequest(AUTH_REQ_PASSWORD)
if it wanted to restrict it.

Greetings,

Andres Freund

On 2/25/22 12:39 PM, Tom Lane wrote:

Jeff Davis <pgsql@j-davis.com> writes:

On Thu, 2022-02-24 at 20:47 -0500, Tom Lane wrote:

... and, since we can't readily enforce that the client only sends
those cleartext passwords over suitably-encrypted connections, this
could easily be a net negative for security. Not sure that I think
it's a good idea.

I don't understand your point. Can't you just use "hostssl" rather than
"host"?

My point is that sending cleartext passwords over the wire is an
insecure-by-definition protocol that we shouldn't be encouraging
more use of.

This is my general feeling as well. We just spent a bunch of effort
adding, refining, and making SCRAM the default method. I think doing
anything that would drive more use of sending plaintext passwords, even
over TLS, is counter to that.

I do understand arguments for (e.g. systems that require checking
password complexity), but I wonder if it's better for us to delegate
that to an external auth system. Regardless, I can get behind Andres'
point to "check Port->ssl_in_use before sendAuthRequest(AUTH_REQ_PASSWORD)".

I'm generally in favor of being able to support additional
authentication methods, the first one coming to mind is supporting OIDC.
Having a pluggable auth infrastructure could possibly make such efforts
easier. I'm definitely intrigued.

Jonathan

On Fri, 2022-02-25 at 12:39 -0500, Tom Lane wrote:

My point is that sending cleartext passwords over the wire is an
insecure-by-definition protocol that we shouldn't be encouraging
more use of.

We can require custom auth entries in pg_hba.conf to also specify
local, hostssl or hostgssenc.

It might annoy people who have a network secured at some other layer,
or who have the client on the same machine as the host. We could allow
plain "host" if someone specifies "customplain" explicitly.

Regards,
Jeff Davis

Jeff Davis <pgsql@j-davis.com> writes:

On Fri, 2022-02-25 at 12:39 -0500, Tom Lane wrote:

My point is that sending cleartext passwords over the wire is an
insecure-by-definition protocol that we shouldn't be encouraging
more use of.

We can require custom auth entries in pg_hba.conf to also specify
local, hostssl or hostgssenc.

That's just a band-aid, though. It does nothing for the other
reasons not to want cleartext passwords, notably that you might
be giving your password to a compromised server.

I'm happy to add support for custom auth methods if they can use
a protocol that's safer than cleartext-password. But if that's the
only feasible option, then we're just encouraging people to use
insecure methods.

I also have in mind here that there has been discussion of giving
libpq a feature to refuse, on the client side, to send cleartext
passwords. (I don't recall right now if that's been mentioned
publicly or only among the security team, but it's definitely
under consideration.)

So I think this proposal needs more thought. A server-side hook
alone is not a useful improvement IMO; it's closer to being an
attractive nuisance.

regards, tom lane

On Fri, 2022-02-25 at 13:40 -0500, Jonathan S. Katz wrote:

I'm generally in favor of being able to support additional
authentication methods, the first one coming to mind is supporting OIDC.
Having a pluggable auth infrastructure could possibly make such efforts
easier. I'm definitely intrigued.

I'm hoping to dust off my OAuth patch and see if it can be ported on
top of this proposal.

--Jacob

Hi,

On 2022-02-25 14:10:39 -0500, Tom Lane wrote:

I'm happy to add support for custom auth methods if they can use
a protocol that's safer than cleartext-password. But if that's the
only feasible option, then we're just encouraging people to use
insecure methods.

It looks like scram can be used without much trouble. All the necessary
infrastructure to implement it without duplication appears to be public. The
plugin would need to get a secret from whereever and call
CheckSASLAuth(&pg_be_scram_mech, port, shadow_pass, logdetail);

or if it needs to do something more complicated than CheckSASLAuth(), it can
use AUTH_REQ_SASL{,_FIN CONT} itself.

Of course whether it's viable depends on what the custom auth method wants to
do. But it's not a restriction of the authentication plugin model.

Greetings,

Andres Freund

On Fri, 2022-02-25 at 14:10 -0500, Tom Lane wrote:

I'm happy to add support for custom auth methods if they can use
a protocol that's safer than cleartext-password. But if that's the
only feasible option, then we're just encouraging people to use
insecure methods.

FWIW, I'd like to be able to use a token in the password field, and
then authenticate that token. So I didn't intend to send an actual
password in plaintext.

Maybe we should have a new "token" connection parameter so that libpq
can allow sending a token plaintext but refuse to send a password in
plaintext?

I also have in mind here that there has been discussion of giving
libpq a feature to refuse, on the client side, to send cleartext
passwords.

I am generally in favor of that idea, but I'm not sure that will
completely resolve the issue. For instance, should it also refuse MD5?

Regards,
Jeff Davis

On 28.02.22 00:17, Jeff Davis wrote:

I also have in mind here that there has been discussion of giving
libpq a feature to refuse, on the client side, to send cleartext
passwords.

I am generally in favor of that idea, but I'm not sure that will
completely resolve the issue. For instance, should it also refuse MD5?

Presumably that feature could be more generally "refuse these
authentication mechanisms" rather than only one hardcoded one.

On 17.02.22 20:25, samay sharma wrote:

A use case where this is useful are environments where you want
authentication to be centrally managed across different services. This
is a common deployment model for cloud providers where customers like to
use single sign on and authenticate across different services including
Postgres. Implementing this now is tricky as it requires syncing that
authentication method's credentials with Postgres (and that gets
trickier with TTL/expiry etc.). With these hooks, you can implement an
extension to check credentials directly using the
authentication provider's APIs.

We already have a variety of authentication mechanisms that support
central management: LDAP, PAM, Kerberos, Radius. What other mechanisms
are people thinking about implementing using these hooks? Maybe there
are a bunch of them, in which case a hook system might be sensible, but
if there are only one or two plausible ones, we could also just make
them built in.