Обсуждение: [MASSMAIL] pgsql: With gssencmode='require', check credential cache before connect

With gssencmode='require', check credential cache before connecting

Previously, libpq would establish the TCP connection, and then
immediately disconnect if the credentials were not available.  The
same thing happened if you tried to use a Unix domain socket with
gssencmode=require. Check those conditions before establishing the TCP
connection.

This is a very minor issue, but my motivation to do this now is that
I'm about to add more detail to the tests for encryption negotiation.
This makes the case of gssencmode=require but no credentials
configured fail at the same stage as with gssencmode=require and
GSSAPI support not compiled at all. That avoids having to deal with
variations in expected output depending on build options.

Discussion: https://www.postgresql.org/message-id/CAEze2Wja8VUoZygCepwUeiCrWa4jP316k0mvJrOW4PFmWP0Tcw@mail.gmail.com

Branch
------
master

Details
-------
https://git.postgresql.org/pg/commitdiff/20f9b61cc1926775b1ceb25196df942efaf8bdd2

Modified Files
--------------
src/interfaces/libpq/fe-connect.c | 32 ++++++++++++++++++++++++++++++--
1 file changed, 30 insertions(+), 2 deletions(-)

Hello.

At Sun, 07 Apr 2024 23:50:08 +0000, Heikki Linnakangas <heikki.linnakangas@iki.fi> wrote in 
> With gssencmode='require', check credential cache before connecting

This commit adds the following error message (indentations are adjusted):

+    libpq_append_conn_error(conn,
+        "GSSAPI encryption required but it is not supported over a local socket)");

The closing parenthesis at the end of the message seems to be a
leftover from editing.

diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
index 4bd523ec6e..e35bdc4036 100644
--- a/src/interfaces/libpq/fe-connect.c
+++ b/src/interfaces/libpq/fe-connect.c
@@ -2927,7 +2927,7 @@ keep_going:                        /* We will come back to here until there is
                         if (conn->raddr.addr.ss_family == AF_UNIX)
                         {
                             libpq_append_conn_error(conn,
-                                                    "GSSAPI encryption required but it is not supported over a local
socket)");
+                                                    "GSSAPI encryption required but it is not supported over a local
socket");
                             goto error_return;
                         }
                         if (conn->gcred == GSS_C_NO_CREDENTIAL)


About the following message:

+            libpq_append_conn_error(conn, "could not set ssl alpn extension: %s", err);

I'm not sure about the policy for writing acronyms in lowercase, but
other occurrences of ALPN (in backend code) seem to be written in
uppercase.

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center

On 09/04/2024 04:46, Kyotaro Horiguchi wrote:
> Hello.
> 
> At Sun, 07 Apr 2024 23:50:08 +0000, Heikki Linnakangas <heikki.linnakangas@iki.fi> wrote in
>> With gssencmode='require', check credential cache before connecting
> 
> This commit adds the following error message (indentations are adjusted):
> 
> +    libpq_append_conn_error(conn,
> +        "GSSAPI encryption required but it is not supported over a local socket)");
> 
> The closing parenthesis at the end of the message seems to be a
> leftover from editing.

Fixed, thanks!

> About the following message:
> 
> +            libpq_append_conn_error(conn, "could not set ssl alpn extension: %s", err);
> 
> I'm not sure about the policy for writing acronyms in lowercase, but
> other occurrences of ALPN (in backend code) seem to be written in
> uppercase.

Changed to uppercase. I also changed "ssl" to uppercase, for consistency 
with the "could not set SSL Server Name Indication (SNI)" message earlier.

To be even more consistent, we should perhaps spell out "SSL 
Application-Layer Protocol Negotiation (ALPN)", but that's pretty long 
and I don't think it really helps the user. It really should not fail, 
and there isn't anything the user can really do if that fails. Anyone 
who doesn't already know what ALPN is will need to google it anyway.

-- 
Heikki Linnakangas
Neon (https://neon.tech)

At Tue, 9 Apr 2024 08:14:53 +0300, Heikki Linnakangas <hlinnaka@iki.fi> wrote in 
> On 09/04/2024 04:46, Kyotaro Horiguchi wrote:
> > Hello.
> > At Sun, 07 Apr 2024 23:50:08 +0000, Heikki Linnakangas
> > <heikki.linnakangas@iki.fi> wrote in
> >> With gssencmode='require', check credential cache before connecting
> > This commit adds the following error message (indentations are
> > adjusted):
> > +    libpq_append_conn_error(conn,
> > + "GSSAPI encryption required but it is not supported over a local
> > socket)");
> > The closing parenthesis at the end of the message seems to be a
> > leftover from editing.
> 
> Fixed, thanks!
> 
> > About the following message:
> > + libpq_append_conn_error(conn, "could not set ssl alpn extension:
> > %s", err);
> > I'm not sure about the policy for writing acronyms in lowercase, but
> > other occurrences of ALPN (in backend code) seem to be written in
> > uppercase.
> 
> Changed to uppercase. I also changed "ssl" to uppercase, for
> consistency with the "could not set SSL Server Name Indication (SNI)"
> message earlier.

(I didn't consider SSL..)

> To be even more consistent, we should perhaps spell out "SSL
> Application-Layer Protocol Negotiation (ALPN)", but that's pretty long
> and I don't think it really helps the user. It really should not fail,
> and there isn't anything the user can really do if that fails. Anyone
> who doesn't already know what ALPN is will need to google it anyway.

I think so, too.

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center