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

Thanks Stephen for your comments.

On Wed, Oct 28, 2020 at 9:44 PM Stephen Frost <sfrost(at)snowman(dot)net> wrote:
>
> Greetings,
>
> * vignesh C (vignesh21(at)gmail(dot)com) wrote:
> > Log message for GSS connection is missing once connection
> > authorization is successful. We have similar log messages 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. I'm not sure if this log message was
> > intentionally left out due to some reason for GSS.
> > If the above analysis looks correct, then please find a patch that
> > adds log for gss connections.
> >
> > Thoughts?
>
> I agree with logging the principal and if GSS encryption is being used
> or not as part of the connection authorized message. Not logging the
> principal isn't great and has been something I've wanted to fix for a
> while, so glad to see someone else is thinking about this.
>
> > From 95c906b9eaf1493ad10ac65d6cf7b27a7dd6acb9 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 v1] 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.
> > ---
> > src/backend/utils/init/postinit.c | 29 +++++++++++++++++++++++++++++
> > 1 file changed, 29 insertions(+)
> >
> > diff --git a/src/backend/utils/init/postinit.c b/src/backend/utils/init/postinit.c
> > index d4ab4c7..0fd38b7 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_enc(port))
>
> This is checking if GSS *encryption* is being used.
>
> > + ereport(LOG,
> > + (port->application_name != NULL
> > + ? errmsg("replication connection authorized: user=%s application_name=%s GSS enabled (gssapi autorization=%s, principal=%s)",
> > + port->user_name,
> > + port->application_name,
> > + be_gssapi_get_auth(port) ? _("on") : _("off"),
> > + be_gssapi_get_princ(port))
> > + : errmsg("replication connection authorized: user=%s GSS enabled (gssapi autorization=%s, principal=%s)",
> > + port->user_name,
> > + be_gssapi_get_auth(port) ? _("on") : _("off"),
> > + be_gssapi_get_princ(port))));
>
> This is checking if GSS *authentication* was used.
>
> You can certainly have GSS authentication used without encryption, and
> you can (though I'm not sure how useful it really is) have GSS
> encryption with 'trust' authentication, so we should really break this
> out into their own sets of checks, which would look something like:
>
> 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)
>
> Or something along those lines, I would think.
>
> I don't think 'enabled' is a good term to use here.
>

I have made a v2 patch based on the changes you have suggested. The
patch for the same is attached.

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