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
