Обсуждение: Log operating system user connecting via unix socket

José,

* José Arthur Benetasso Villanova (jose.arthur@gmail.com) wrote:
> Here in my work, we have about 100 PostgreSQL machines and about 20 users
> with superuser privileges.

Sounds pretty common.  What kind of superuser rights are they using?
What is the minimum set of rights that are required for these users
(which may break out into different groups, naturally).  We're looking
at ways to provide access for certain operations to non-superusers, to
reduce the number of superuser accounts required.

> This group of 20 people change constantly, so it's cumbersome create a role
> for each. Instead, we map all of then in pg_ident.conf.

Do these 20 individuals have 'regular' accounts also?  Have you
considered granting them a superuser role which they could 'set role'
to when they need to perform a superuser operation?

> The problem is: with current postgres log, I just know that a postgres user
> connect, but I don't know which one is in case that more than one user is
> logged in the server.

Understood, that's unfortunate.

> This simple log line can create the relations needed for an audit.
>
> Feel free to comment and criticize.

What I think we really want here is logging of the general 'system
user' for all auth methods instead of only for the 'peer' method.
Knowing who connected via Kerberos is quite valuable also, for example.

My thinking would be to add 'system_user' to the Peer struct and then
log the system user in PerformAuthentication.  Another thought might be
to replace peer_cn with 'peer_user' (it's the same thing for client cert
SSL connections, after all..) and then log it and also make it available
in pg_stat_activity.

These are a bit off-the-cuff comments, but hopefully make sense and
provide the right direction to be looking in.  The other thing to
consider is how this information is reflected in the CSV log and/or
log_line_prefix..

Thanks!

Stephen

Stephen Frost <sfrost@snowman.net> writes:
> What I think we really want here is logging of the general 'system
> user' for all auth methods instead of only for the 'peer' method.

Well, we don't really know that except in a small subset of auth
methods.  I agree that when we do know it, it's useful info to log.

My big beef with the proposed patch is that the log message is emitted
unconditionally.  There are lots and lots of users who feel that during
normal operation, *zero* log messages should get emitted.  Those villagers
would be on our doorsteps with pitchforks if we shipped this patch as-is.

I would propose that this information should be emitted only when
log_connections is enabled, and indeed that it should be part of the
log_connections message not a separate message.  So this leads to
thinking that somehow, the code for individual auth methods should
be able to return an "additional info" field for inclusion in
log_connections.  We already have such a concept for auth failures,
cf commit 5e0b5dcab.

> ... and also make it available in pg_stat_activity.

That's moving the goalposts quite a bit, and I'm not sure it's necessary
or even desirable.  Let's just get this added to log_connections output,
and then see if there's field demand for more.
        regards, tom lane

Tom,

* Tom Lane (tgl@sss.pgh.pa.us) wrote:
> Stephen Frost <sfrost@snowman.net> writes:
> > What I think we really want here is logging of the general 'system
> > user' for all auth methods instead of only for the 'peer' method.
>
> Well, we don't really know that except in a small subset of auth
> methods.  I agree that when we do know it, it's useful info to log.

Right.

> My big beef with the proposed patch is that the log message is emitted
> unconditionally.  There are lots and lots of users who feel that during
> normal operation, *zero* log messages should get emitted.  Those villagers
> would be on our doorsteps with pitchforks if we shipped this patch as-is.

Agreed.

> I would propose that this information should be emitted only when
> log_connections is enabled, and indeed that it should be part of the
> log_connections message not a separate message.  So this leads to
> thinking that somehow, the code for individual auth methods should
> be able to return an "additional info" field for inclusion in
> log_connections.  We already have such a concept for auth failures,
> cf commit 5e0b5dcab.

Apologies if it wasn't clear, but that's exactly what I was suggesting
by saying to add it to PerformAuthentication, which is where we emit
the connection info when log_connections is enabled.

> > ... and also make it available in pg_stat_activity.
>
> That's moving the goalposts quite a bit, and I'm not sure it's necessary
> or even desirable.  Let's just get this added to log_connections output,
> and then see if there's field demand for more.

This was in context of peer_cn, which is just a specific "system user"
value and which we're already showing in pg_stat_* info tables.  I'd
love to have the Kerberos principal available, but I don't think it'd
make sense to have a 'pg_stat_kerberos' just for that.

I agree that it's moving the goalposts for this patch and could be an
independent patch, but I don't see it as any different, from a
desirability and requirements perspective, than what we're doing for SSL
connections.

Thanks!

Stephen

José,

* José Arthur Benetasso Villanova (jose.arthur@gmail.com) wrote:
> I wrote 2 possible patches, both issuing a detail message only if
> log_connections is enabled.
>
> The first one using the Stephen Frost suggestion, inside the Port struct (I
> guess that this is the one, I coudn't find the Peer struct)

Ah, yes, apologies for the typo.

> The second one following the same approach of cf commit 5e0b5dcab, as
> pointed by Tom Lane.

This really isn't quite following along in the approach used by
5e0b5dcab, from my viewing of it.  I believe Tom was suggesting that an
essentially opaque value be returned to be included rather than what
you've done which codifies it as 'system_user'.

I'm not a fan of that approach though, as the mapping system we have in
pg_ident is generalized and this should be implemented generally by all
authentication methods which support mappings.

That's also why I was suggesting to get rid of peer_cn in the Port
structure in favor of having the 'system_user' or similar variable and
using it in all of these cases where we provide mapping support- then
all of the auth methods which support mappings would set that value,
including the existing SSL code.  You might need to check and see if
there's anything which depends on peer_cn being NULL for non-SSL
connections and adjust that logic, but hopefully that's not what we're
relying on.  I don't see anything like that on a quick glance through
the peer_cn usage.

Also, it looks like you missed one of the exit cases from
pg_SSPI_recvauth(), no?  You may need to refactor that code a bit to
provide easy access to what the system username used is, or simply make
sure to set the port->system_user value in both paths.

Lastly, are you sure that you have the right memory context for the
pstrdup() calls that you're making?  be-secure-openssl.c goes to some
effort to ensure that the memory allocated for peer_cn is in the
TopMemoryContext, but I don't see anything like that in the code
proposed, which makes me worried you didn't consider which memory
context you were allocating in.

Thanks!

Stephen