José, * José Arthur Benetasso Villanova (jose.art...@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
signature.asc
Description: Digital signature