pam auth - add rhost item

On Mon, Oct 12, 2015 at 12:01 PM, kolo hhmow <grzsmp@gmail.com> wrote:

Wheter it would be a problem to set additional item (rhost) before
pam_authentication function in backend/libpq/auth.c?
It is very useful because you can restrict access to given ip address like
in mysql.
And this actually utilized in pam-pgsql, wich cannot be used because rhost
item is empty.

I can't understand what you are suggesting here. Perhaps you could be
more specific, or propose a patch.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Yes, sorry. I was in hurry when I posted this message.
I dont understand whay in CheckPAMAuth function only PAM_USER item is
adding to pam information before authenticate?
Wheter it would be a problem to set additional pam information like
PAM_RHOST which is very useful because we can use this item to restrict
access to this ip address.
I hope I'm more specific now and you will understand me.
Sorry, but I'm not native english speaker.
Patch in attachment, and link below to web-view on github:
https://github.com/grzsmp/postgres/commit/5e2b102ec6de27e786d627623dcb187e997609e4

On Tue, Oct 13, 2015 at 7:08 PM, Robert Haas <robertmhaas@gmail.com> wrote:

On Tue, Oct 13, 2015 at 4:12 PM, kolo hhmow <grzsmp@gmail.com> wrote:

Yes, sorry. I was in hurry when I posted this message.
I dont understand whay in CheckPAMAuth function only PAM_USER item is adding
to pam information before authenticate?
Wheter it would be a problem to set additional pam information like
PAM_RHOST which is very useful because we can use this item to restrict
access to this ip address.
I hope I'm more specific now and you will understand me.
Sorry, but I'm not native english speaker.
Patch in attachment, and link below to web-view on github:
https://github.com/grzsmp/postgres/commit/5e2b102ec6de27e786d627623dcb187e997609e4

I don't personally know much about PAM, but if you want to restrict
access by IP, you could do that in pg_hba.conf.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Yes, but this is very ugly solution, becasue you have to restart postgresql
daemon each time you have added a new user.
This solution which I propose is give an abbility to dinamicaly manage user
accounts without need to restart each time a user account entry has change.
When you have lot of actively users using postgresql service, you cannot
restart the server each time somebody add, or remove some user account
entry from the system.
This is whay we uses pam modules with pam-pgsql and with this patch.

On Wed, Oct 14, 2015 at 9:52 PM, Robert Haas <robertmhaas@gmail.com> wrote:

Yes, you right - my mistake.
But editing pg_hba.conf with lot of entries is little inconveniet. When
using pam modules with backend database like postgresql/or whatever
is more efficient and convenient - this is whay among others I need pass
client ip to pam modules, and then to backend database for example.
So I'm waiting for comments from others.
Thanks.

On Wed, Oct 14, 2015 at 9:52 PM, Robert Haas <robertmhaas@gmail.com> wrote:

On 14-10-2015 17:35, kolo hhmow wrote:

Yes, but this is very ugly solution, becasue you have to restart
postgresql daemon each time you have added a new user.

Restart != Reload. You can even do it using SQL.

This solution which I propose is give an abbility to dinamicaly manage
user accounts without need to restart each time a user account entry has
change.

Why do you want to double restrict the access? We already have HBA.
Also, you could complicate the management because you need to check two
different service configurations to figure out why foo user can't log
in. I'm not a PAM expert but my impression is that rhost is an optional
item. Therefore, advise PAM users to use HBA is a way to not complicate
the actual feature.

--
Euler Taveira Timbira - http://www.timbira.com.br/
PostgreSQL: Consultoria, Desenvolvimento, Suporte 24x7 e Treinamento

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Thu, Oct 15, 2015 at 1:45 AM, Euler Taveira <euler@timbira.com.br> wrote:

On 14-10-2015 17:35, kolo hhmow wrote:

Yes, but this is very ugly solution, becasue you have to restart
postgresql daemon each time you have added a new user.

Restart != Reload. You can even do it using SQL.

Yes, this is was my mistake.

This solution which I propose is give an abbility to dinamicaly manage

user accounts without need to restart each time a user account entry has
change.

Why do you want to double restrict the access? We already have HBA. Also,
you could complicate the management because you need to check two different
service configurations to figure out why foo user can't log in. I'm not a
PAM expert but my impression is that rhost is an optional item. Therefore,
advise PAM users to use HBA is a way to not complicate the actual feature.

I have already explained this in my previous post. Did you read this?

So why postgresql give users an abbility to use a pam modules, when in
other side there is advice to not use them?
Anyway.
I do not see any complication with this approach. Just use one
configuration entry in pg_hba.conf, and rest entries in some database
backend of pam module, which is most convenient with lot of entries than
editing pg_hba.conf.
Yes rhost is optional item, which is not actually set to pam information in
ofical source code and this is why I need add this patch.

On 15-10-2015 05:41, kolo hhmow wrote:

I have already explained this in my previous post. Did you read this?

Yes, I do.

So why postgresql give users an abbility to use a pam modules, when in
other side there is advice to not use them?
Anyway.

Where is such advise? I can't see it in docs [1]http://www.postgresql.org/docs/current/static/auth-methods.html#AUTH-PAM.

I do not see any complication with this approach. Just use one
configuration entry in pg_hba.conf, and rest entries in some database
backend of pam module, which is most convenient with lot of entries than
editing pg_hba.conf.

Why don't you use a group role? I need just one entry in pg_hba.conf.

[1]: http://www.postgresql.org/docs/current/static/auth-methods.html#AUTH-PAM
[2]: http://www.postgresql.org/docs/current/static/role-membership.html

--
Euler Taveira Timbira - http://www.timbira.com.br/
PostgreSQL: Consultoria, Desenvolvimento, Suporte 24x7 e Treinamento

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Fri, Oct 16, 2015 at 2:47 PM, Euler Taveira <euler@timbira.com.br> wrote:

On 15-10-2015 05:41, kolo hhmow wrote:

I have already explained this in my previous post. Did you read this?

Yes, I do.

So why postgresql give users an abbility to use a pam modules, when in

other side there is advice to not use them?
Anyway.

Where is such advise? I can't see it in docs [1].

Not in docs. You gave such advice:
"Therefore, advise PAM users to use HBA is a way to not complicate the
actual feature".

I do not see any complication with this approach. Just use one

configuration entry in pg_hba.conf, and rest entries in some database
backend of pam module, which is most convenient with lot of entries than
editing pg_hba.conf.

Why don't you use a group role? I need just one entry in pg_hba.conf.

[1]
http://www.postgresql.org/docs/current/static/auth-methods.html#AUTH-PAM
[2] http://www.postgresql.org/docs/current/static/role-membership.html

Because cannot restrict from what ip address client can connet in such way.

You can restrict only whole group, not just individual member of such
group, or I misunderstand something.

On Fri, Oct 16, 2015 at 8:47 AM, Euler Taveira <euler@timbira.com.br> wrote:

On 15-10-2015 05:41, kolo hhmow wrote:

I have already explained this in my previous post. Did you read this?

Yes, I do.

So why postgresql give users an abbility to use a pam modules, when in
other side there is advice to not use them?
Anyway.

Where is such advise? I can't see it in docs [1].

I do not see any complication with this approach. Just use one
configuration entry in pg_hba.conf, and rest entries in some database
backend of pam module, which is most convenient with lot of entries than
editing pg_hba.conf.

Why don't you use a group role? I need just one entry in pg_hba.conf.

I feel like this discussion has taken an unhelpful turn. Surely you
can see that this is not necessarily an exact substitute for what kolo
hhmow wants to do. Yeah, he could decide to do something else
instead, but are you really confused about why he would want to do
this in PAM, or is this just a case of arguing that what we have is
good enough so let's not change anything or take suggestions? He's
not saying there's no workaround; he's just saying he'd like this
better.

I think some more interesting questions are:
- Did he implement this correctly?
- Would it break anything?
- Are there lots of other knobs we should expose too instead of just one?
- What would it take to turn this into a committable patch?
- Would the cost of exposing this and perhaps some other knobs cost
too much in performance for the number of people it would make happy?
- If so, should the behavior be GUC-controlled or is there
justification for arguing we should drop the whole patch?

I feel like we've got somebody new showing up to our community with an
idea that is not obviously stupid. If we want such people to stick
around, we should try to give their ideas a fair shake.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Robert Haas wrote:

I think some more interesting questions are:
- Did he implement this correctly?
- Would it break anything?
- Are there lots of other knobs we should expose too instead of just one?
- What would it take to turn this into a committable patch?
- Would the cost of exposing this and perhaps some other knobs cost
too much in performance for the number of people it would make happy?
- If so, should the behavior be GUC-controlled or is there
justification for arguing we should drop the whole patch?

I agree with this set of questions -- the idea behind the patch seemed
quite reasonable to me.

I feel like we've got somebody new showing up to our community with an
idea that is not obviously stupid. If we want such people to stick
around, we should try to give their ideas a fair shake.

+1 to this, too.

--
�lvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On 16-10-2015 10:37, Robert Haas wrote:

- Did he implement this correctly?

- Would it break anything?

I did not review the patch.

- Are there lots of other knobs we should expose too instead of just one?

We are providing PAM_USER and PAM_CONV. The complete list of options are
[1]: http://pubs.opengroup.org/onlinepubs/8329799/pam_set_item.htm
(app) to user bar (PAM).

- What would it take to turn this into a committable patch?

Review?

- Would the cost of exposing this and perhaps some other knobs cost
too much in performance for the number of people it would make happy?

No.

- If so, should the behavior be GUC-controlled or is there
justification for arguing we should drop the whole patch?

The patch always set PAM_RHOST, ie. it means I can't disable it (at the
postgres side). Is it a problem? Of course the PAM module can provide a
way to ignore it but it is not our business.

I feel like we've got somebody new showing up to our community with an
idea that is not obviously stupid. If we want such people to stick
around, we should try to give their ideas a fair shake.

I share the same feeling. I wasn't trying to throw a cold water on it.

[1]: http://pubs.opengroup.org/onlinepubs/8329799/pam_set_item.htm

--
Euler Taveira Timbira - http://www.timbira.com.br/
PostgreSQL: Consultoria, Desenvolvimento, Suporte 24x7 e Treinamento

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Fri, Oct 16, 2015 at 10:50 AM, Euler Taveira <euler@timbira.com.br> wrote:

I feel like we've got somebody new showing up to our community with an
idea that is not obviously stupid. If we want such people to stick
around, we should try to give their ideas a fair shake.

I share the same feeling. I wasn't trying to throw a cold water on it.

OK, I felt like that's what you were doing. Sorry if I
misinterpreted, and thanks for clarifying.

kolo hhmow: I suggest adding your patch to
https://commitfest.postgresql.org/action/commitfest_view/open so it
doesn't get forgotten about. Hopefully someone who knows more about
this area will volunteer to review.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Ok.
Thak you all!
:)

On Fri, Oct 16, 2015 at 5:20 PM, Robert Haas <robertmhaas@gmail.com> wrote:

On Sat, Oct 17, 2015 at 1:00 AM, kolo hhmow <grzsmp@gmail.com> wrote:

Ok.
Thak you all!

This patch was listed twice in the CF app. I removed the duplicated
entry and let this one alive:
https://commitfest.postgresql.org/7/392/
Could you add your name as an author please?
--
Michael

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On 10/13/15 4:12 PM, kolo hhmow wrote:

Yes, sorry. I was in hurry when I posted this message.
I dont understand whay in CheckPAMAuth function only PAM_USER item is
adding to pam information before authenticate?
Wheter it would be a problem to set additional pam information like
PAM_RHOST which is very useful because we can use this item to restrict
access to this ip address.

Your implementation uses NI_NUMERICHOST, but the documentation of
pam_set_item speaks of a "hostname". Which is correct?

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Well, this is not matter since pam_set_item expect this argument as a
string.
Besides there is not always possible to get map from ip address to hostname.
So hostname is just a synonim for whatever information you cat get about
remote machine.

ps. sorry for delay answer.

Hi,

On 11/25/2015 01:45 PM, Grzegorz Sampolski wrote:

Well, this is not matter since pam_set_item expect this argument as a
string.
Besides there is not always possible to get map from ip address to
hostname. So hostname is just a synonim for whatever information you
cat get about remote machine.

I'm no PAM guru, but I don't see how this implies that we should
entirely abandon FQDN if it's available. Other tools relying on PAM have
to face the same question, so how do they address it?

For example this [1]https://fedorahosted.org/sssd/ticket/908 sssd ticket suggests that for example OpenSSH makes
this configurable - when UseDNS=yes then it attempts to resolve the IP
address to a FQDN, with UseDNS=no it passes the IP address without
attempting to use DNS.

[1]: https://fedorahosted.org/sssd/ticket/908

So maybe we need a knob for this, similar to UseDNS in OpenSSH?

Otherwise, the patch seems fine to me, except for whitespace issues.
Please, make sure you use tabs for indentation (and not spaces).

regards

--
Tomas Vondra http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Actually, one more thing - the patch should probably update the docs
too, because client-auth.sgml currently says this in the "auth-pam" section:

<para>
...
PAM is used only to validate user name/password pairs.
...
</para>

I believe that's no longer true, because the patch adds PAM_RHOST to the
user/password fields.

Regarding the other PAM_* fields, none of them strikes me as very useful
for our use case.

In a broader sense, I think this patch is quite desirable, despite being
rather simple (which is good). I certainly don't agree with suggestions
that we can already do things like this through pg_hba.conf. If we're
providing PAM authentication, let's make it as complete/useful as
possible. In some cases modifying PAM may not be feasible - e.g. some
management systems rely on PAM as much as possible, and doing changes in
other ways is a major hassle.

regards

--
Tomas Vondra http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers