Re: Problem with ssl and psql in Postgresql 13

At Wed, 23 Dec 2020 17:34:05 -0500, Tom Lane <tgl@sss.pgh.pa.us> wrote in 
> Oh ... wait a second.  I've just managed to partially reproduce this
> report.  Assume that we have a working Kerberos environment on both

If I kninit'ed and pg_hba.conf doesn't have a hostgssenc line and has
a hostssl line, I got the following error from psql.

$ psql -d postgres --set=sslmode=require -Uhoriguti -hmyhost.mydomain
psql: error: FATAL:  no pg_hba.conf entry for host "192.168.56.101", user "horiguti", database "postgres", SSL off
FATAL:  no pg_hba.conf entry for host "192.168.56.101", user "horiguti", database "postgres", SSL off

The state machine enables gssenc if a valid credential is available
regardless of whether a valid gsshostenc definition is available or not.

I rembmer that I am annoyed a bit by this behavior, but I didn't care
of that since I thought that SSL and GSS are not used togehter in an
environemt.

> ends.  Then libpq will establish a connection with GSS encryption
> and continue on with the usual PG authentication exchange.  If that
> part fails, then this logic (fe_connect.c around line 3350):
> 
>                     /*
>                      * If gssencmode is "prefer" and we're using GSSAPI, retry
>                      * without it.
>                      */
>                     if (conn->gssenc && conn->gssencmode[0] == 'p')
>                     {
>                         /* postmaster expects us to drop the connection */
>                         conn->try_gss = false;
>                         pqDropConnection(conn, true);
>                         conn->status = CONNECTION_NEEDED;
>                         goto keep_going;
>                     }
>
> drops the connection and tries again without GSS.  But *we already
> cleared allow_ssl_try*, in pqsecure_open_gss, so the next try will
> not bother with SSL either.
> So this directly explains the original report: if the only applicable
> pg_hba.conf entry is "hostssl", right here is where libpq will see
> that error, and it will go around and fail again because the next
> try uses neither GSS nor SSL.
>
> However, in the tests Mikael ran after backing that pg_hba.conf entry
> off to just "host", pg_hba wouldn't have caused an authentication-stage
> failure, so it's not so clear why we'd have looped back at this step.

In the hostssl case, there are two errors emitted on the server side:

[24494] FATAL:  no pg_hba.conf entry for host "192.168.56.101", user "horiguti", database "postgres", SSL off
[24495] FATAL:  no pg_hba.conf entry for host "192.168.56.101", user "horiguti", database "postgres", SSL off

However, the first line is bogus. It should be like the following.

FATAL:  no pg_hba.conf entry for host "192.168.56.101", user "horiguti", database "postgres", GSSENC on
FATAL:  no pg_hba.conf entry for host "192.168.56.101", user "horiguti", database "postgres", SSL off

The attached the first patch does that.
The client picks up only the second error to show from psql.

If I changed the hostssl to just host, host line matches
GSSENC-connection and authentication succeeds without an error.

> We'd need to explain the later cases to have a full theory.  It's
> plenty plausible that something else caused an auth-stage failure,
> but if so, why would the second GSS-free try work?

Does my expample above works?

As the result, psql loses the cause of the first failure and shows the
bogus message. On the other hand the server doesn't know the second
try is the result of the failure of the first try. If we want to show
meaningful messages to users, psql should show both the two messages,
or need to modify the protocol to pass around the information on the
failure of the previous try.

> In any case, that allow_ssl_try reset in pqsecure_open_gss is definitely
> even more broken than I thought before.  I think we need to rejigger the
> state machine so that it doesn't do that at all, but instead skips SSL
> establishment when GSS is already active via an explicit test, rather
> than munging the state variables.

It could make things simpler, but I'm not sure though. Anyway the
states are needed since PQconnectPoll(the state machine) needs to once
exit for negotiation and each of the two paths needs a re-entry point.

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center
diff --git a/src/backend/libpq/auth.c b/src/backend/libpq/auth.c
index 3d80930968..204630e383 100644
--- a/src/backend/libpq/auth.c
+++ b/src/backend/libpq/auth.c
@@ -511,6 +511,26 @@ ClientAuthentication(Port *port)
                 }
                 else
                 {
+#ifdef ENABLE_GSS
+#ifdef USE_SSL
+                    ereport(FATAL,
+                            (errcode(ERRCODE_INVALID_AUTHORIZATION_SPECIFICATION),
+                             errmsg("no pg_hba.conf entry for host \"%s\", user \"%s\", database \"%s\", %s, %s",
+                                    hostinfo, port->user_name,
+                                    port->database_name,
+                                    port->ssl_in_use ? _("SSL on") : _("SSL off"),
+                                    port->gss->enc ? _("GSSENC on") : _("GSSENC off")),
+                             HOSTNAME_LOOKUP_DETAIL(port)));
+#else /* USE_SSL */
+                    ereport(FATAL,
+                            (errcode(ERRCODE_INVALID_AUTHORIZATION_SPECIFICATION),
+                             errmsg("no pg_hba.conf entry for host \"%s\", user \"%s\", database \"%s\", %s",
+                                    hostinfo, port->user_name,
+                                    port->database_name,
+                                    port->gss->enc ? _("GSSENC on") : _("GSSENC off")),
+                             HOSTNAME_LOOKUP_DETAIL(port)));
+#endif /* USE_SSL */
+#else  /* ENABLE_GSS */
 #ifdef USE_SSL
                     ereport(FATAL,
                             (errcode(ERRCODE_INVALID_AUTHORIZATION_SPECIFICATION),
@@ -526,6 +546,7 @@ ClientAuthentication(Port *port)
                                     hostinfo, port->user_name,
                                     port->database_name),
                              HOSTNAME_LOOKUP_DETAIL(port)));
+#endif
 #endif
                 }
                 break;