Enable "pam" authentication support via HBA

[moench-tegeder]

Cursory code review has shown no reason why "pam" should not be
supported in pg_hba.
Add "pam" to the list of supported authentication methods in
the HBA file - only if generally available, of course - and
edit docs to match the new reality.

Fixes #501

[petere]

You need to update all the places where cf_auth_type == AUTH_PAM is checked. See src/objects.c and src/admin.c for missing places. The problem is that without these changes the takeover does not work correctly when you have a pg_hba.conf with mixed pam and on-pam entries. use_server_socket() in src/objects.c needs to decide when processing a server socket entry from SHOW FDS whether to look up the user in the normal user list or add it to the PAM user list. Right now it knows this from the global auth_type setting, because it's the same for all sockets. But if we allow a mix of PAM and non-PAM authentication, then we have to solve this in another way. I think a good way to do this would be to change SHOW FDS to ship a flag "this is a PAM user". (Updating the SHOW FDS output needs to be done with care to allow cross-version upgrades. I'm not sure about how to do that in detail.)

[petere]

This change is incorrect.

[petere]

This information is already available in client->client_auth_type. No need to look it up again.

[wibrt]

In a more general way:
the way to authenticate directly to postgres does not fully match the way one can authenticate against pgbouncer

Just thinking out freely, this may be wrong, but..:
what if there was a pam module "pam-postgresql"
which would allow pam to manage it's authentication against a postgresql instance;
(thus having pam delegate its authencation to postgres)
having this single pam module would facilitate pgbouncer all the authentication methods of postgres, also future ones.
(also other intermediates that support pam, could benefit from this)

A possible limitation:
what if pgbouncer connects to multiple different postgres instances?

[petere]

Just thinking out freely, this may be wrong, but..:
what if there was a pam module "pam-postgresql"
which would allow pam to manage it's authentication against a postgresql instance;

This exists, it's called pam-pgsql.

But I don't see what this has to do with this thread. If you do PAM authentication and use pam-pgsql for the PAM module, then you are mostly just replicating what auth_query does.

[wibrt]

as i understand it: pam-psql in this form has little added value; just as you mention.

what i mean: a pam module to check whether a user can authenticate against a postgresql cluster X, thus effectively authenticated against the pg_hba.conf of this postgresql cluster X;

the problem at hand to be solved is that the authentication methods for pgbouncer do not match 100% the authentication methods of postgresql, or a combination of them.
Example (in short): lets say you have mixture of pam, scram/md5, ldap/ad user authentication in your pg_hba.conf of postgres, there no straightforward way of using pgbouncer in this case (unless limited to certain auth methods; or running multiple pgbouncers on different ports for different auth methods)

So instead of adding auth methods to pgbouncer, maybe just one is enough,
a pam-module that delegates it

[petere]

I've studied this a bit today. One major issue that I have identified is how authenticating the same user sometimes via PAM and sometimes not works. Internally, PgBouncer tracks PAM and non-PAM users separately (see add_pam_user()). If we allow both PAM and non-PAM authentication at the same time, then we could have the same user name registered twice. This would first of all crash in some places, but when that's fixed, is that what we want? For example, you'd have potentially two pools with the same database and user name, except that one is for a PAM user and one is not. This would make a lot of things very confusing.

I think one way to move forward here would be to analyze whether we can get rid of add_pam_user() and keep the PAM users in the normal user tree, perhaps with a flag and a separate password field, or something like that.

[JelteF]

Given that #1052 changed the user management it should now be fine to have PAM users and non-PAM users in one system. The SHOW FDS thing we decided we don't care anymore about breaking because now we have a different method for online restarts.

So I decided to update this patch and address the other feedback from @petere. I intend to merge this soon given its simplicity.

Addressed the comments

[clarkbreyman]

The change to client.c:538 seems to be causing actual PAM auth segfaults by skipping the path that allocates the pool. I'm looking to see if there is an omission of setting client->auth_type to PAM when cf_auth_type is PAM.

@JelteF - see #1261

[clarkbreyman]

client->client_auth_type is not set until finish_set_pool() which is after the test in set_pool(). At the time of the test cf_auth_type is PAM but the client is still ANY.

[lalitvc]

Ohh I see the patch is reverted in #1291