Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 861e967

Browse files
committed
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
1 parent 2392136 commit 861e967

File tree

5 files changed

+31
-32
lines changed

5 files changed

+31
-32
lines changed

doc/src/sgml/client-auth.sgml

+1-5
Original file line numberDiff line numberDiff line change
@@ -1262,11 +1262,7 @@ omicron bryanh guest1
12621262

12631263
<para>
12641264
The location of the server's keytab file is specified by the <xref
1265-
linkend="guc-krb-server-keyfile"/> configuration
1266-
parameter. The default is
1267-
<filename>FILE:/usr/local/pgsql/etc/krb5.keytab</filename>
1268-
(where the directory part is whatever was specified
1269-
as <varname>sysconfdir</varname> at build time).
1265+
linkend="guc-krb-server-keyfile"/> configuration parameter.
12701266
For security reasons, it is recommended to use a separate keytab
12711267
just for the <productname>PostgreSQL</productname> server rather
12721268
than allowing the server to read the system keytab file.

doc/src/sgml/config.sgml

+9-3
Original file line numberDiff line numberDiff line change
@@ -1035,10 +1035,16 @@ include_dir 'conf.d'
10351035
</term>
10361036
<listitem>
10371037
<para>
1038-
Sets the location of the Kerberos server key file. See
1039-
<xref linkend="gssapi-auth"/>
1040-
for details. This parameter can only be set in the
1038+
Sets the location of the server's Kerberos key file. The default is
1039+
<filename>FILE:/usr/local/pgsql/etc/krb5.keytab</filename>
1040+
(where the directory part is whatever was specified
1041+
as <varname>sysconfdir</varname> at build time; use
1042+
<literal>pg_config --sysconfdir</literal> to determine that).
1043+
If this parameter is set to an empty string, it is ignored and a
1044+
system-dependent default is used.
1045+
This parameter can only be set in the
10411046
<filename>postgresql.conf</filename> file or on the server command line.
1047+
See <xref linkend="gssapi-auth"/> for more information.
10421048
</para>
10431049
</listitem>
10441050
</varlistentry>

src/backend/libpq/auth.c

+10-21
Original file line numberDiff line numberDiff line change
@@ -1054,29 +1054,18 @@ pg_GSS_recvauth(Port *port)
10541054
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
10551055
errmsg("GSSAPI is not supported in protocol version 2")));
10561056

1057-
if (pg_krb_server_keyfile && strlen(pg_krb_server_keyfile) > 0)
1057+
/*
1058+
* Use the configured keytab, if there is one. Unfortunately, Heimdal
1059+
* doesn't support the cred store extensions, so use the env var.
1060+
*/
1061+
if (pg_krb_server_keyfile != NULL && pg_krb_server_keyfile[0] != '\0')
10581062
{
1059-
/*
1060-
* Set default Kerberos keytab file for the Krb5 mechanism.
1061-
*
1062-
* setenv("KRB5_KTNAME", pg_krb_server_keyfile, 0); except setenv()
1063-
* not always available.
1064-
*/
1065-
if (getenv("KRB5_KTNAME") == NULL)
1063+
if (setenv("KRB5_KTNAME", pg_krb_server_keyfile, 1) != 0)
10661064
{
1067-
size_t kt_len = strlen(pg_krb_server_keyfile) + 14;
1068-
char *kt_path = malloc(kt_len);
1069-
1070-
if (!kt_path ||
1071-
snprintf(kt_path, kt_len, "KRB5_KTNAME=%s",
1072-
pg_krb_server_keyfile) != kt_len - 2 ||
1073-
putenv(kt_path) != 0)
1074-
{
1075-
ereport(LOG,
1076-
(errcode(ERRCODE_OUT_OF_MEMORY),
1077-
errmsg("out of memory")));
1078-
return STATUS_ERROR;
1079-
}
1065+
/* The only likely failure cause is OOM, so use that errcode */
1066+
ereport(FATAL,
1067+
(errcode(ERRCODE_OUT_OF_MEMORY),
1068+
errmsg("could not set environment: %m")));
10801069
}
10811070
}
10821071

src/backend/libpq/be-secure-gssapi.c

+10-2
Original file line numberDiff line numberDiff line change
@@ -525,8 +525,16 @@ secure_open_gssapi(Port *port)
525525
* Use the configured keytab, if there is one. Unfortunately, Heimdal
526526
* doesn't support the cred store extensions, so use the env var.
527527
*/
528-
if (pg_krb_server_keyfile != NULL && strlen(pg_krb_server_keyfile) > 0)
529-
setenv("KRB5_KTNAME", pg_krb_server_keyfile, 1);
528+
if (pg_krb_server_keyfile != NULL && pg_krb_server_keyfile[0] != '\0')
529+
{
530+
if (setenv("KRB5_KTNAME", pg_krb_server_keyfile, 1) != 0)
531+
{
532+
/* The only likely failure cause is OOM, so use that errcode */
533+
ereport(FATAL,
534+
(errcode(ERRCODE_OUT_OF_MEMORY),
535+
errmsg("could not set environment: %m")));
536+
}
537+
}
530538

531539
while (true)
532540
{

src/backend/utils/misc/postgresql.conf.sample

+1-1
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@
9292
#db_user_namespace = off
9393

9494
# GSSAPI using Kerberos
95-
#krb_server_keyfile = ''
95+
#krb_server_keyfile = 'FILE:${sysconfdir}/krb5.keytab'
9696
#krb_caseins_users = off
9797

9898
# - SSL -

0 commit comments

Comments
 (0)