Date: Wed, 30 Dec 2020 10:00:40 -0500
From: Stephen Frost <sfrost@snowman.net>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Gustavsson Mikael <mikael.gustavsson@smhi.se>,
	Magnus Hagander <magnus@hagander.net>,
	Kyotaro Horiguchi <horikyota.ntt@gmail.com>,
	"pgsql-general@postgresql.org" <pgsql-general@postgresql.org>,
	Svensson Peter <peter.svensson@smhi.se>
Subject: Re: Problem with ssl and psql in Postgresql 13
Message-ID: <20201230150039.GZ27507@tamriel.snowman.net>
References: <737386.1608757126@sss.pgh.pa.us>
 <20201223211448.GR27507@tamriel.snowman.net>
 <738715.1608758847@sss.pgh.pa.us>
 <20201223214534.GS27507@tamriel.snowman.net>
 <740093.1608760567@sss.pgh.pa.us>
 <742641.1608762845@sss.pgh.pa.us>
 <1308863.1609016410@sss.pgh.pa.us>
 <1333636.1609020377@sss.pgh.pa.us>
 <1528843.1609108374@sss.pgh.pa.us>
 <2187460.1609263156@sss.pgh.pa.us>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="6TME3aayZmn2Ikqb"
Content-Disposition: inline
In-Reply-To: <2187460.1609263156@sss.pgh.pa.us>
User-Agent: Mutt/1.5.24 (2015-08-30)
Precedence: bulk


--6TME3aayZmn2Ikqb
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

Greetings,

First, thanks a lot for working on all of this and improving things!

* Tom Lane (tgl@sss.pgh.pa.us) wrote:
> I've got one last complaint about the backend GSS code: we are doing
> things randomly differently in the two places that install
> krb_server_keyfile as the active KRB5_KTNAME value.  secure_open_gssapi()
> sets KRB5_KTNAME unconditionally (and doesn't bother to check for error,
> either, not a good thing in a security-critical operation).  But the older
> code in pg_GSS_recvauth() is written to not override KRB5_KTNAME if it's
> already set.  This of-course-totally-undocumented behavior seems like a
> fairly bad idea to me: as things stand, the client-side choice of whether
> to initiate GSS encryption or not could result in two different server
> keytabs being used.

I agree that we should be consistent and that always setting the
environment variable if krb_server_keyfile has been set is the right
approach.

> I think we'd be best off to always override KRB5_KTNAME if we have a
> nonempty krb_server_keyfile setting, so the attached proposed patch
> makes both functions do it the same way.  (I did not make an effort
> to remove the dependency on setenv, given the nearby thread to
> standardize on that.)

+1.

> I'm not sure whether there's any documentation change that needs to
> be made.  The docs don't suggest that you're allowed to set
> krb_server_keyfile to an empty string in the first place, so maybe
> we needn't explain what happens if you do.

Perhaps saying something about 'system default' or 'taken from the
environment' might make sense.  That said, the system default almost
always ends up not being usable since it'll be owned by root and we
won't run as root.  Perhaps there are some who prefer to set it via the
environment variable, though I don't think I've ever seen that myself.

I didn't look too closely at the patch itself but on a once-over it
seemed fine to me.

Thanks,

Stephen

--6TME3aayZmn2Ikqb
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1

iQIcBAEBCgAGBQJf7JYXAAoJEO1sijiDR2RVay8P/2mY5yImna4qFVPk5qkXlAf3
BToDGOD+CTn6Ozcpzma/rPgL4JA/0ufqj3t+rwbZg7EoPd5WLB1QO6VYo5ggRpxL
L+mW0FwTOd9BdjS7u3Ghrlk0D47qByt4OPJWHhKwaHvTTPfwBdOAwTVAWXqaL481
Vape6ysglzN8B5UR1Dr7RMXgnjA++3FUILUQSd1iWSLkXjC8JHqOtr5W6dpLn9dQ
AFPeNL6QqgAFLp4Tr+RmoEm22GbCdj4Y+y63PeBRcZmXq/sn0HOK9q9oJm5ta8kJ
hyPlU8lxodK2qVuKg2ef5nAr0JardHdfasXVwItLZMmYm/gvvMSgid0LzI4V1FbW
XHJiaBnI2A21WthLFd92FU1iyT0XIxzMvx7SSP50s4FzSJ81HERTAWBlVIiiUoNc
hLPuPFWB3hbkQ6LvnaF6fovlCGsiGgFe851rkn2teiCXN7EPESq+OAO+TDEZhvqb
r2BGNixigA4bN+O65yWVXyU+pjrcZgLPY76z6PgHK2nDbgfMuPTFHCgG0au37Ayp
AoPUtf8RW2eVB6A1esw/WNVpcrPd2N4VyGrD4vbWZvIBmueC+5Wb7dp2kLhw+A6Y
PRQA+yUn4D4zfO1kVR0p6oY4mk72A2FxAdWJQsz2IrDjuS/jSm8PPGFpelMM7J4V
EVp3sJtjyDqX/vrMEeUX
=BzsO
-----END PGP SIGNATURE-----

--6TME3aayZmn2Ikqb--