Re: Log message for GSS connection is missing once connection authorization is successful.
| От | Bharath Rupireddy |
|---|---|
| Тема | Re: Log message for GSS connection is missing once connection authorization is successful. |
| Дата | |
| Msg-id | CALj2ACW7EgAQKhnTJAocqUkekR+sMy1TX8AK-FjWo+CMp9y6pw@mail.gmail.com обсуждение исходный текст |
| Ответ на | Re: Log message for GSS connection is missing once connection authorization is successful. (vignesh C <vignesh21@gmail.com>) |
| Ответы |
Re: Log message for GSS connection is missing once connection authorization is successful.
(vignesh C <vignesh21@gmail.com>)
|
| Список | pgsql-hackers |
Please add this to commitfest to not lose track of it.
I took a look at v2 patch, here are some comments.
On Thu, Oct 29, 2020 at 11:01 AM vignesh C <vignesh21@gmail.com> wrote:
>> Stephen also shared his thoughts for the above changes, I have
> provided an updated patch for the same in the previous mail. Please
> have a look and let me know if you have any comments.
>
> if (be_gssapi_get_auth(port) || be_gssapi_get_princ(port))
> connection authorized: GSS %s (principal=%s)
> With the first %s being: (authentication || encrypted || authenticated and encrypted)
>
1. Instead of just "on/off" after GSS %s in the log message, wouldn't it be informative if we have authenticated and/or encrypted as suggested by Stephen?
So the log message would look like this:
if(be_gssapi_get_auth(port))
replication connection authorized: user=bob application_name=foo GSS authenticated (principal=bar)
if(be_gssapi_get_enc(port))
replication connection authorized: user=bob application_name=foo GSS encrypted (principal=bar)
if(be_gssapi_get_auth(port) && be_gssapi_get_enc(port))
replication connection authorized: user=bob application_name=foo GSS authenticated and encrypted (principal=bar)
+#ifdef ENABLE_GSS
+ if (be_gssapi_get_auth(port) || be_gssapi_get_princ(port))
+ ereport(LOG,
+ (port->application_name != NULL
+ ? errmsg("replication connection authorized: user=%s application_name=%s GSS %s (principal=%s)",
+ port->user_name,
+ port->application_name,
+ be_gssapi_get_auth(port) || be_gssapi_get_enc(port) ? _("on") : _("off"),
+ be_gssapi_get_princ(port))
+ : errmsg("replication connection authorized: user=%s GSS %s (principal=%s)",
+ port->user_name,
+ be_gssapi_get_auth(port) || be_gssapi_get_enc(port) ? _("on") : _("off"),
+ be_gssapi_get_princ(port))));
+ else
2. I think the log message preparation looks a bit clumsy with ternary operators and duplicate log message texts(I know that we already do this for SSL). Can we have the log message prepared using StringInfoData data structure/APIs and use just a single ereport? This way, that part of the code looks cleaner.
Here's what I'm visualizing:
if (Log_connections)
{
StringInfoData msg;
if (am_walsender)
append("replication connection authorized: user=%s");
else
append("connection authorized: user=%s database=%s");
if (port->application_name)
append("application_name=%s");
#ifdef USE_SSL
if (port->ssl_in_use)
append("SSL enabled (protocol=%s, cipher=%s, bits=%d, compression=%s");
#elif defined(ENABLE_GSS)
blah,blah,blah
#endif
ereport (LOG, msg.data);
}
{
StringInfoData msg;
if (am_walsender)
append("replication connection authorized: user=%s");
else
append("connection authorized: user=%s database=%s");
if (port->application_name)
append("application_name=%s");
#ifdef USE_SSL
if (port->ssl_in_use)
append("SSL enabled (protocol=%s, cipher=%s, bits=%d, compression=%s");
#elif defined(ENABLE_GSS)
blah,blah,blah
#endif
ereport (LOG, msg.data);
}
3. + if (be_gssapi_get_auth(port) || be_gssapi_get_princ(port))
If be_gssapi_get_auth(port) returns false, I think there's no way that be_gssapi_get_princ(port) would return a non null value, see the comment. The function be_gssapi_get_princ() returns NULL if the auth is false, so the check if ( be_gssapi_get_princ(port)) would suffice.
gss_name_t name; /* GSSAPI client name */
char *princ; /* GSSAPI Principal used for auth, NULL if
* GSSAPI auth was not used */
bool auth; /* GSSAPI Authentication used */
bool enc; /* GSSAPI encryption in use */
Bharath Rupireddy.
EnterpriseDB: http://www.enterprisedb.com
В списке pgsql-hackers по дате отправления: