Thanks Stephen for your comments.
On Wed, Oct 28, 2020 at 9:44 PM Stephen Frost <sfr...@snowman.net> wrote:
>
> Greetings,
>
> * vignesh C (vignes...@gmail.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 <vignes...@gmail.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
From b067cf823750f200102be0a0cad9a26a08e29a92 Mon Sep 17 00:00:00 2001
From: Vignesh C <vignes...@gmail.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.
---
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..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
ereport(LOG,
(port->application_name != NULL
? errmsg("replication connection authorized: user=%s application_name=%s",
@@ -295,6 +310,20 @@ 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("connection authorized: user=%s database=%s application_name=%s GSS %s (principal=%s)",
+ port->user_name, port->database_name, port->application_name,
+ be_gssapi_get_auth(port) || be_gssapi_get_enc(port) ? _("on") : _("off"),
+ be_gssapi_get_princ(port))
+ : errmsg("connection authorized: user=%s database=%s GSS %s (principal=%s)",
+ port->user_name, port->database_name,
+ be_gssapi_get_auth(port) || be_gssapi_get_enc(port) ? _("on") : _("off"),
+ be_gssapi_get_princ(port))));
+ else
+#endif
ereport(LOG,
(port->application_name != NULL
? errmsg("connection authorized: user=%s database=%s application_name=%s",
--
1.8.3.1
