Re: Log message for GSS connection is missing once connection authorization is successful.

On Thu, Oct 29, 2020 at 7:26 PM Stephen Frost <sfrost(at)snowman(dot)net> wrote:
>
> Greetings,
>
> * vignesh C (vignesh21(at)gmail(dot)com) wrote:
> > I have made a v2 patch based on the changes you have suggested. The
> > patch for the same is attached.
>
> > From b067cf823750f200102be0a0cad9a26a08e29a92 Mon Sep 17 00:00:00 2001
> > From: Vignesh C <vignesh21(at)gmail(dot)com>
> > Date: Wed, 28 Oct 2020 08:19:06 +0530
> > Subject: [PATCH v2] Log message for GSS connection is missing once connection
> > authorization is successful.
> >
> > Log message for GSS connection is missing once connection authorization is
> > successful. We have similar log message for SSL connections once the connection
> > authorization is successful. This message will help the user to identify the
> > connection that was selected from the logfile.
>
> Just to be clear- it's not that the message is 'missing', it's just not
> providing the (certainly useful) information about how the connection
> was authorized. The commit message should make it clear that what we're
> doing here is improving the connection authorization message for GSS
> authenticated or encrypted connections.
>

I have updated the commit message accordingly.

> > diff --git a/src/backend/utils/init/postinit.c b/src/backend/utils/init/postinit.c
> > index d4ab4c7..7980e92 100644
> > --- a/src/backend/utils/init/postinit.c
> > +++ b/src/backend/utils/init/postinit.c
> > @@ -267,6 +267,21 @@ PerformAuthentication(Port *port)
> > be_tls_get_compression(port) ? _("on") : _("off"))));
> > else
> > #endif
> > +#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
> > +#endif
>
> No, this isn't what I was suggesting. "on" and "off" really isn't
> communicating the details about the GSS-using connection. What I
> suggested before was something like:
>
> 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)) ? "authenticated and encrypted" : be_gssapi_get_auth(port) ? "authenticated" : "encrypted",
> be_gssapi_get_princ(port))
>
> Though I'll admit that perhaps there's something better which could be
> done here- but just 'on/off' certainly isn't that. Another option might
> be:
>
> errmsg("replication connection authorized: user=%s application_name=%s GSS authenticated: %s, encrypted: %s, principal: %s",
> port->user_name,
> port->application_name,
> be_gssapi_get_auth(port) ? "yes" : "no",
> be_gssapi_get_enc(port) ? "yes" : "no",
> be_gssapi_get_princ(port))
>

I like the above method that you suggested, I have changed it based on
the above.

> Also, it would be good to see if there's a way to add to the tests we
> have for GSSAPI authentication/encryption to show that we hit each of
> the possible cases and check that we get the correct messages in the log
> as a result.
>

I have added the log validation to the existing tests that are present
for authentication.

Attached v3 patch has the change for the same.

Regards,
Vignesh
EnterpriseDB: http://www.enterprisedb.com