Fix up usage of krb_server_keyfile GUC parameter.

secure_open_gssapi() installed the krb_server_keyfile setting as
KRB5_KTNAME unconditionally, so long as it's not empty.  However,
pg_GSS_recvauth() only installed it if KRB5_KTNAME wasn't set already,
leading to a troubling inconsistency: in theory, clients could see
different sets of server principal names depending on whether they
use GSSAPI encryption.  Always using krb_server_keyfile seems like
the right thing, so make both places do that.  Also fix up
secure_open_gssapi()'s lack of a check for setenv() failure ---
it's unlikely, surely, but security-critical actions are no place
to be sloppy.

Also improve the associated documentation.

This patch does nothing about secure_open_gssapi()'s use of setenv(),
and indeed causes pg_GSS_recvauth() to use it too.  That's nominally
against project portability rules, but since this code is only built
with --with-gssapi, I do not feel a need to do something about this
in the back branches.  A fix will be forthcoming for HEAD though.

Back-patch to v12 where GSSAPI encryption was introduced.  The
dubious behavior in pg_GSS_recvauth() goes back further, but it
didn't have anything to be inconsistent with, so let it be.

Discussion: https://postgr.es/m/2187460.1609263156@sss.pgh.pa.us

diff --git a/doc/src/sgml/client-auth.sgml b/doc/src/sgml/client-auth.sgml
index 92f474e8e6bd48c012268a7102b8a6f856089484..ccd748d264a14761906cba9314c55215b02e79e0 100644 (file)
--- a/doc/src/sgml/client-auth.sgml
+++ b/doc/src/sgml/client-auth.sgml
@@ -1262,11 +1262,7 @@ omicron         bryanh                  guest1
 
    
     The location of the server's keytab file is specified by the 
-    linkend="guc-krb-server-keyfile"/> configuration
-    parameter. The default is
-    FILE:/usr/local/pgsql/etc/krb5.keytab
-    (where the directory part is whatever was specified
-    as sysconfdir at build time).
+    linkend="guc-krb-server-keyfile"/> configuration parameter.
     For security reasons, it is recommended to use a separate keytab
     just for the PostgreSQL server rather
     than allowing the server to read the system keytab file.


diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml

index 8d72951dd09e93712aead03a5c0383c0f5893b71..97610de287d77d2dc4f91a3d3e9cb085a484d440 100644 (file)

--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -1035,10 +1035,16 @@ include_dir 'conf.d'
       
       
        
-        Sets the location of the Kerberos server key file. See
-        
-        for details. This parameter can only be set in the
+        Sets the location of the server's Kerberos key file.  The default is
+        FILE:/usr/local/pgsql/etc/krb5.keytab
+        (where the directory part is whatever was specified
+        as sysconfdir at build time; use
+        pg_config --sysconfdir to determine that).
+        If this parameter is set to an empty string, it is ignored and a
+        system-dependent default is used.
+        This parameter can only be set in the
         postgresql.conf file or on the server command line.
+        See  for more information.
        
       
      


diff --git a/src/backend/libpq/auth.c b/src/backend/libpq/auth.c

index 549c4c3d48a0602a95e26ad4bc660b932f498fda..45dcd65afaf0788f4f29cd3538bae05490f78b04 100644 (file)

--- a/src/backend/libpq/auth.c
+++ b/src/backend/libpq/auth.c
@@ -1054,29 +1054,18 @@ pg_GSS_recvauth(Port *port)
                (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
                 errmsg("GSSAPI is not supported in protocol version 2")));
 
-   if (pg_krb_server_keyfile && strlen(pg_krb_server_keyfile) > 0)
+   /*
+    * Use the configured keytab, if there is one.  Unfortunately, Heimdal
+    * doesn't support the cred store extensions, so use the env var.
+    */
+   if (pg_krb_server_keyfile != NULL && pg_krb_server_keyfile[0] != '\0')
    {
-       /*
-        * Set default Kerberos keytab file for the Krb5 mechanism.
-        *
-        * setenv("KRB5_KTNAME", pg_krb_server_keyfile, 0); except setenv()
-        * not always available.
-        */
-       if (getenv("KRB5_KTNAME") == NULL)
+       if (setenv("KRB5_KTNAME", pg_krb_server_keyfile, 1) != 0)
        {
-           size_t      kt_len = strlen(pg_krb_server_keyfile) + 14;
-           char       *kt_path = malloc(kt_len);
-
-           if (!kt_path ||
-               snprintf(kt_path, kt_len, "KRB5_KTNAME=%s",
-                        pg_krb_server_keyfile) != kt_len - 2 ||
-               putenv(kt_path) != 0)
-           {
-               ereport(LOG,
-                       (errcode(ERRCODE_OUT_OF_MEMORY),
-                        errmsg("out of memory")));
-               return STATUS_ERROR;
-           }
+           /* The only likely failure cause is OOM, so use that errcode */
+           ereport(FATAL,
+                   (errcode(ERRCODE_OUT_OF_MEMORY),
+                    errmsg("could not set environment: %m")));
        }
    }
 


diff --git a/src/backend/libpq/be-secure-gssapi.c b/src/backend/libpq/be-secure-gssapi.c

index 36a28254bd31d206d74c9f0cbcbbeb4d70caca86..56d310e61a27e4db3b81531f42538a0b9f334cbc 100644 (file)

--- a/src/backend/libpq/be-secure-gssapi.c
+++ b/src/backend/libpq/be-secure-gssapi.c
@@ -525,8 +525,16 @@ secure_open_gssapi(Port *port)
     * Use the configured keytab, if there is one.  Unfortunately, Heimdal
     * doesn't support the cred store extensions, so use the env var.
     */
-   if (pg_krb_server_keyfile != NULL && strlen(pg_krb_server_keyfile) > 0)
-       setenv("KRB5_KTNAME", pg_krb_server_keyfile, 1);
+   if (pg_krb_server_keyfile != NULL && pg_krb_server_keyfile[0] != '\0')
+   {
+       if (setenv("KRB5_KTNAME", pg_krb_server_keyfile, 1) != 0)
+       {
+           /* The only likely failure cause is OOM, so use that errcode */
+           ereport(FATAL,
+                   (errcode(ERRCODE_OUT_OF_MEMORY),
+                    errmsg("could not set environment: %m")));
+       }
+   }
 
    while (true)
    {


diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample

index 7bd659dbfa458aa63cba3adfdf8abe95aac8ce95..1284925683ae2ed7363fdd6e783087634300f1c2 100644 (file)

--- a/src/backend/utils/misc/postgresql.conf.sample
+++ b/src/backend/utils/misc/postgresql.conf.sample
@@ -92,7 +92,7 @@
 #db_user_namespace = off
 
 # GSSAPI using Kerberos
-#krb_server_keyfile = ''
+#krb_server_keyfile = 'FILE:${sysconfdir}/krb5.keytab'
 #krb_caseins_users = off
 
 # - SSL -

diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 8d72951dd09e93712aead03a5c0383c0f5893b71..97610de287d77d2dc4f91a3d3e9cb085a484d440 100644 (file)
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -1035,10 +1035,16 @@ include_dir 'conf.d'
       
       
        
-        Sets the location of the Kerberos server key file. See
-        
-        for details. This parameter can only be set in the
+        Sets the location of the server's Kerberos key file.  The default is
+        FILE:/usr/local/pgsql/etc/krb5.keytab
+        (where the directory part is whatever was specified
+        as sysconfdir at build time; use
+        pg_config --sysconfdir to determine that).
+        If this parameter is set to an empty string, it is ignored and a
+        system-dependent default is used.
+        This parameter can only be set in the
         postgresql.conf file or on the server command line.
+        See  for more information.
        
       
      

diff --git a/src/backend/libpq/auth.c b/src/backend/libpq/auth.c
index 549c4c3d48a0602a95e26ad4bc660b932f498fda..45dcd65afaf0788f4f29cd3538bae05490f78b04 100644 (file)
--- a/src/backend/libpq/auth.c
+++ b/src/backend/libpq/auth.c
@@ -1054,29 +1054,18 @@ pg_GSS_recvauth(Port *port)
                (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
                 errmsg("GSSAPI is not supported in protocol version 2")));
 
-   if (pg_krb_server_keyfile && strlen(pg_krb_server_keyfile) > 0)
+   /*
+    * Use the configured keytab, if there is one.  Unfortunately, Heimdal
+    * doesn't support the cred store extensions, so use the env var.
+    */
+   if (pg_krb_server_keyfile != NULL && pg_krb_server_keyfile[0] != '\0')
    {
-       /*
-        * Set default Kerberos keytab file for the Krb5 mechanism.
-        *
-        * setenv("KRB5_KTNAME", pg_krb_server_keyfile, 0); except setenv()
-        * not always available.
-        */
-       if (getenv("KRB5_KTNAME") == NULL)
+       if (setenv("KRB5_KTNAME", pg_krb_server_keyfile, 1) != 0)
        {
-           size_t      kt_len = strlen(pg_krb_server_keyfile) + 14;
-           char       *kt_path = malloc(kt_len);
-
-           if (!kt_path ||
-               snprintf(kt_path, kt_len, "KRB5_KTNAME=%s",
-                        pg_krb_server_keyfile) != kt_len - 2 ||
-               putenv(kt_path) != 0)
-           {
-               ereport(LOG,
-                       (errcode(ERRCODE_OUT_OF_MEMORY),
-                        errmsg("out of memory")));
-               return STATUS_ERROR;
-           }
+           /* The only likely failure cause is OOM, so use that errcode */
+           ereport(FATAL,
+                   (errcode(ERRCODE_OUT_OF_MEMORY),
+                    errmsg("could not set environment: %m")));
        }
    }
 

diff --git a/src/backend/libpq/be-secure-gssapi.c b/src/backend/libpq/be-secure-gssapi.c
index 36a28254bd31d206d74c9f0cbcbbeb4d70caca86..56d310e61a27e4db3b81531f42538a0b9f334cbc 100644 (file)
--- a/src/backend/libpq/be-secure-gssapi.c
+++ b/src/backend/libpq/be-secure-gssapi.c
@@ -525,8 +525,16 @@ secure_open_gssapi(Port *port)
     * Use the configured keytab, if there is one.  Unfortunately, Heimdal
     * doesn't support the cred store extensions, so use the env var.
     */
-   if (pg_krb_server_keyfile != NULL && strlen(pg_krb_server_keyfile) > 0)
-       setenv("KRB5_KTNAME", pg_krb_server_keyfile, 1);
+   if (pg_krb_server_keyfile != NULL && pg_krb_server_keyfile[0] != '\0')
+   {
+       if (setenv("KRB5_KTNAME", pg_krb_server_keyfile, 1) != 0)
+       {
+           /* The only likely failure cause is OOM, so use that errcode */
+           ereport(FATAL,
+                   (errcode(ERRCODE_OUT_OF_MEMORY),
+                    errmsg("could not set environment: %m")));
+       }
+   }
 
    while (true)
    {

diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample
index 7bd659dbfa458aa63cba3adfdf8abe95aac8ce95..1284925683ae2ed7363fdd6e783087634300f1c2 100644 (file)
--- a/src/backend/utils/misc/postgresql.conf.sample
+++ b/src/backend/utils/misc/postgresql.conf.sample
@@ -92,7 +92,7 @@
 #db_user_namespace = off
 
 # GSSAPI using Kerberos
-#krb_server_keyfile = ''
+#krb_server_keyfile = 'FILE:${sysconfdir}/krb5.keytab'
 #krb_caseins_users = off
 
 # - SSL -