Re: pgsql: Superuser can permit passwordless connections on postgres_fdw

I wrote:
> This is a bit messier.  But I think that the discrepancy is not
> really the fault of this patch: rather, it's a bug in the way the
> GSS support was put into libpq.  I thought we had a policy that
> all builds would recognize all possible parameters and then
> perhaps fail later.  Certainly the SSL parameters are implemented
> that way.  The #if's disabling GSS stuff in PQconninfoOptions[]
> are just broken, according to that policy.

Concretely, I think we ought to do (and back-patch) the attached.

I notice in testing this that the "nosuper" business added by
6136e94dc is broken in more ways than what the buildfarm is
complaining about: it leaves the role around at the end of the
test.  That's a HUGE violation of project policy, for security
reasons as well as the fact that it makes it impossible to run
"make installcheck" twice without getting different results.

            regards, tom lane

diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out
index 3d6e4ee..ed52ddd 100644
--- a/contrib/postgres_fdw/expected/postgres_fdw.out
+++ b/contrib/postgres_fdw/expected/postgres_fdw.out
@@ -132,7 +132,7 @@ CREATE FOREIGN TABLE ft6 (
 -- ===================================================================
 -- tests for validator
 -- ===================================================================
--- requiressl, krbsrvname and gsslib are omitted because they depend on
+-- requiressl is omitted because valid values depend on
 -- configure options
 ALTER SERVER testserver1 OPTIONS (
     use_remote_estimate 'false',
@@ -158,10 +158,10 @@ ALTER SERVER testserver1 OPTIONS (
     sslcert 'value',
     sslkey 'value',
     sslrootcert 'value',
-    sslcrl 'value'
+    sslcrl 'value',
     --requirepeer 'value',
-    -- krbsrvname 'value',
-    -- gsslib 'value',
+    krbsrvname 'value',
+    gsslib 'value'
     --replication 'value'
 );
 -- Error, invalid list syntax
@@ -8855,7 +8855,7 @@ DO $d$
     END;
 $d$;
 ERROR:  invalid option "password"
-HINT:  Valid options in this context are: service, passfile, channel_binding, connect_timeout, dbname, host, hostaddr,
port,options, application_name, keepalives, keepalives_idle, keepalives_interval, keepalives_count, tcp_user_timeout,
sslmode,sslcompression, sslcert, sslkey, sslrootcert, sslcrl, requirepeer, gssencmode, krbsrvname,
target_session_attrs,use_remote_estimate, fdw_startup_cost, fdw_tuple_cost, extensions, updatable, fetch_size 
+HINT:  Valid options in this context are: service, passfile, channel_binding, connect_timeout, dbname, host, hostaddr,
port,options, application_name, keepalives, keepalives_idle, keepalives_interval, keepalives_count, tcp_user_timeout,
sslmode,sslcompression, sslcert, sslkey, sslrootcert, sslcrl, requirepeer, gssencmode, krbsrvname, gsslib,
target_session_attrs,use_remote_estimate, fdw_startup_cost, fdw_tuple_cost, extensions, updatable, fetch_size 
 CONTEXT:  SQL statement "ALTER SERVER loopback_nopw OPTIONS (ADD password 'dummypw')"
 PL/pgSQL function inline_code_block line 3 at EXECUTE
 -- If we add a password for our user mapping instead, we should get a different
diff --git a/contrib/postgres_fdw/sql/postgres_fdw.sql b/contrib/postgres_fdw/sql/postgres_fdw.sql
index 33307d6..e0f4c72 100644
--- a/contrib/postgres_fdw/sql/postgres_fdw.sql
+++ b/contrib/postgres_fdw/sql/postgres_fdw.sql
@@ -145,7 +145,7 @@ CREATE FOREIGN TABLE ft6 (
 -- ===================================================================
 -- tests for validator
 -- ===================================================================
--- requiressl, krbsrvname and gsslib are omitted because they depend on
+-- requiressl is omitted because valid values depend on
 -- configure options
 ALTER SERVER testserver1 OPTIONS (
     use_remote_estimate 'false',
@@ -171,10 +171,10 @@ ALTER SERVER testserver1 OPTIONS (
     sslcert 'value',
     sslkey 'value',
     sslrootcert 'value',
-    sslcrl 'value'
+    sslcrl 'value',
     --requirepeer 'value',
-    -- krbsrvname 'value',
-    -- gsslib 'value',
+    krbsrvname 'value',
+    gsslib 'value'
     --replication 'value'
 );

diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml
index 4325946..66b09da 100644
--- a/doc/src/sgml/libpq.sgml
+++ b/doc/src/sgml/libpq.sgml
@@ -1747,8 +1747,10 @@ postgresql://%2Fvar%2Flib%2Fpostgresql/dbname
       <term><literal>gsslib</literal></term>
       <listitem>
        <para>
-        GSS library to use for GSSAPI authentication. Only used on Windows.
-        Set to <literal>gssapi</literal> to force libpq to use the GSSAPI
+        GSS library to use for GSSAPI authentication.
+        Currently this is disregarded except on Windows builds that include
+        both GSSAPI and SSPI support.  In that case, set
+        this to <literal>gssapi</literal> to cause libpq to use the GSSAPI
         library for authentication instead of the default SSPI.
        </para>
       </listitem>
diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
index 3ca7e05..cb3c431 100644
--- a/src/interfaces/libpq/fe-connect.c
+++ b/src/interfaces/libpq/fe-connect.c
@@ -304,6 +304,10 @@ static const internalPQconninfoOption PQconninfoOptions[] = {
         "SSL-Client-Key", "", 64,
     offsetof(struct pg_conn, sslkey)},

+    {"sslpassword", NULL, NULL, NULL,
+        "SSL-Client-Key-Password", "*", 20,
+    offsetof(struct pg_conn, sslpassword)},
+
     {"sslrootcert", "PGSSLROOTCERT", NULL, NULL,
         "SSL-Root-Certificate", "", 64,
     offsetof(struct pg_conn, sslrootcert)},
@@ -317,30 +321,21 @@ static const internalPQconninfoOption PQconninfoOptions[] = {
     offsetof(struct pg_conn, requirepeer)},

     /*
-     * Expose gssencmode similarly to sslmode - we can still handle "disable"
-     * and "prefer".
+     * As with SSL, all GSS options are exposed even in builds that don't have
+     * support.
      */
     {"gssencmode", "PGGSSENCMODE", DefaultGSSMode, NULL,
         "GSSENC-Mode", "", 7,    /* sizeof("disable") == 7 */
     offsetof(struct pg_conn, gssencmode)},

-#if defined(ENABLE_GSS) || defined(ENABLE_SSPI)
     /* Kerberos and GSSAPI authentication support specifying the service name */
     {"krbsrvname", "PGKRBSRVNAME", PG_KRB_SRVNAM, NULL,
         "Kerberos-service-name", "", 20,
     offsetof(struct pg_conn, krbsrvname)},
-#endif
-
-#if defined(ENABLE_GSS) && defined(ENABLE_SSPI)

-    /*
-     * GSSAPI and SSPI both enabled, give a way to override which is used by
-     * default
-     */
     {"gsslib", "PGGSSLIB", NULL, NULL,
         "GSS-library", "", 7,    /* sizeof("gssapi") = 7 */
     offsetof(struct pg_conn, gsslib)},
-#endif

     {"replication", NULL, NULL, NULL,
         "Replication", "D", 5,
@@ -351,10 +346,6 @@ static const internalPQconninfoOption PQconninfoOptions[] = {
         "Target-Session-Attrs", "", 11, /* sizeof("read-write") = 11 */
     offsetof(struct pg_conn, target_session_attrs)},

-    {"sslpassword", NULL, NULL, NULL,
-        "SSL-Client-Key-Password", "*", 20,
-    offsetof(struct pg_conn, sslpassword)},
-
     /* Terminating entry --- MUST BE LAST */
     {NULL, NULL, NULL, NULL,
     NULL, NULL, 0}
@@ -3983,6 +3974,8 @@ freePGconn(PGconn *conn)
         free(conn->sslcert);
     if (conn->sslkey)
         free(conn->sslkey);
+    if (conn->sslpassword)
+        free(conn->sslpassword);
     if (conn->sslrootcert)
         free(conn->sslrootcert);
     if (conn->sslcrl)
@@ -3991,14 +3984,14 @@ freePGconn(PGconn *conn)
         free(conn->sslcompression);
     if (conn->requirepeer)
         free(conn->requirepeer);
-    if (conn->connip)
-        free(conn->connip);
     if (conn->gssencmode)
         free(conn->gssencmode);
-#if defined(ENABLE_GSS) || defined(ENABLE_SSPI)
     if (conn->krbsrvname)
         free(conn->krbsrvname);
-#endif
+    if (conn->gsslib)
+        free(conn->gsslib);
+    if (conn->connip)
+        free(conn->connip);
 #ifdef ENABLE_GSS
     if (conn->gcred != GSS_C_NO_CREDENTIAL)
     {
@@ -4015,10 +4008,6 @@ freePGconn(PGconn *conn)
         conn->gctx = NULL;
     }
 #endif
-#if defined(ENABLE_GSS) && defined(ENABLE_SSPI)
-    if (conn->gsslib)
-        free(conn->gsslib);
-#endif
     /* Note that conn->Pfdebug is not ours to close or free */
     if (conn->last_query)
         free(conn->last_query);
@@ -4034,8 +4023,6 @@ freePGconn(PGconn *conn)
         free(conn->target_session_attrs);
     termPQExpBuffer(&conn->errorMessage);
     termPQExpBuffer(&conn->workBuffer);
-    if (conn->sslpassword)
-        free(conn->sslpassword);

     free(conn);

diff --git a/src/interfaces/libpq/libpq-int.h b/src/interfaces/libpq/libpq-int.h
index 7f5be7d..e99cbf3 100644
--- a/src/interfaces/libpq/libpq-int.h
+++ b/src/interfaces/libpq/libpq-int.h
@@ -359,13 +359,14 @@ struct pg_conn
     char       *sslcompression; /* SSL compression (0 or 1) */
     char       *sslkey;            /* client key filename */
     char       *sslcert;        /* client certificate filename */
+    char       *sslpassword;    /* client key file password */
     char       *sslrootcert;    /* root certificate filename */
     char       *sslcrl;            /* certificate revocation list filename */
     char       *requirepeer;    /* required peer credentials for local sockets */
-
-#if defined(ENABLE_GSS) || defined(ENABLE_SSPI)
+    char       *gssencmode;        /* GSS mode (require,prefer,disable) */
     char       *krbsrvname;        /* Kerberos service name */
-#endif
+    char       *gsslib;            /* What GSS library to use ("gssapi" or
+                                 * "sspi") */

     /* Type of connection to make.  Possible values: any, read-write. */
     char       *target_session_attrs;
@@ -484,7 +485,6 @@ struct pg_conn
 #endif                            /* USE_OPENSSL */
 #endif                            /* USE_SSL */

-    char       *gssencmode;        /* GSS mode (require,prefer,disable) */
 #ifdef ENABLE_GSS
     gss_ctx_id_t gctx;            /* GSS context */
     gss_name_t    gtarg_nam;        /* GSS target name */
@@ -496,10 +496,6 @@ struct pg_conn
 #endif

 #ifdef ENABLE_SSPI
-#ifdef ENABLE_GSS
-    char       *gsslib;            /* What GSS library to use ("gssapi" or
-                                 * "sspi") */
-#endif
     CredHandle *sspicred;        /* SSPI credentials handle */
     CtxtHandle *sspictx;        /* SSPI context */
     char       *sspitarget;        /* SSPI target name */
@@ -512,8 +508,6 @@ struct pg_conn

     /* Buffer for receiving various parts of messages */
     PQExpBufferData workBuffer; /* expansible string */
-
-    char *sslpassword; /* client key file password */
 };

 /* PGcancel stores all data necessary to cancel a connection. A copy of this