Date: Tue, 2 Apr 2019 12:41:24 -0400
From: Stephen Frost <sfrost@snowman.net>
To: Robbie Harwood <rharwood@redhat.com>
Cc: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>,
	pgsql-hackers@postgresql.org, Nico Williams <nico@cryptonector.com>,
	Michael Paquier <michael@paquier.xyz>,
	Alvaro Herrera <alvherre@2ndquadrant.com>,
	David Steele <david@pgmasters.net>
Subject: Re: [PATCH v22] GSSAPI encryption support
Message-ID: <20190402164124.GZ6197@tamriel.snowman.net>
References: <jlg1tgq1ktm.fsf@thriss.redhat.com>
 <jlgy38mir81.fsf@redhat.com>
 <5ec3f734-810e-36e0-030f-af92efe135da@2ndquadrant.com>
 <20190223162751.GO6197@tamriel.snowman.net>
 <jlg4l8gpsgk.fsf@redhat.com>
 <20190316102110.GU6197@tamriel.snowman.net>
 <jlgftre3bbb.fsf@redhat.com>
 <20190330142404.GO6197@tamriel.snowman.net>
 <jlgv9zx4v3f.fsf@redhat.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="bPbpEglh0dw8VbL0"
Content-Disposition: inline
In-Reply-To: <jlgv9zx4v3f.fsf@redhat.com>
User-Agent: Mutt/1.5.24 (2015-08-30)
Precedence: bulk


--bPbpEglh0dw8VbL0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

Greetings,

* Robbie Harwood (rharwood@redhat.com) wrote:
> Stephen Frost <sfrost@snowman.net> writes:
> > * Robbie Harwood (rharwood@redhat.com) wrote:
> >> Stephen Frost <sfrost@snowman.net> writes:
> >>=20
> >> I wanted to note a couple things about this approach.  It now uses
> >> one more buffer than before (in contrast to the previous approach,
> >> which reused a buffer for received data that was encrypted and
> >> decrypted).
> >
> > Yeah, I don't see that as too much of an issue and it certainly seems
> > cleaner and simpler to reason about to me, which seems worth the
> > modest additional buffer cost.  That's certainly something we could
> > change if others feel differently.
> >
> >> Since these are static fixed size buffers, this increases the total
> >> steady-state memory usage by 16k as opposed to re-using the buffer.
> >> This may be fine; I don't know how tight RAM is here.
> >
> > It seems unlikely to be an issue to me- and I would contend that the
> > prior implementation didn't actually take any steps to prevent the
> > other side from sending packets of nearly arbitrary size (up to 1G),
> > so while the steady-state memory usage of the prior implementation was
> > less when everyone was playing nicely, it could have certainly been
> > abused.  I'm a lot happier having an explicit cap on how much memory
> > will be used, even if that cap is a bit higher.
>=20
> Yeah, that's definitely a fair point.  We could combine the two
> approaches, but I don't really see a reason to if it's unlikely to be an
> issue - as you say, this is more readable.  It can always be a follow-on
> if needed.

Agreed, we could do that later if it seems like it would be helpful.

> > I did add in a simple pg_stat_gssapi view, modeled on pg_stat_ssl, so
> > that you can check server-side if GSSAPI was used for authentication
> > and/or encryption, and what principal was used if GSSAPI was used for
> > authentication.
>=20
> Good idea.

Thanks. :)  I definitely like being able to see from the backend side of
things when a connection is encrypted or not, and being able to see the
principal used for authentication is really nice too.

> >> Again, these are nits, and I think I'm okay with the changes.
> >
> > Great, thanks again for reviewing!
> >
> > Updated patch attached, if you could take another look through it,
> > that'd be great.
>=20
> I'm happy with this!  Appreciate you exploring my concerns.

Fantastic, thanks so much for working on this over the years!  Unless
there's any further comments, I'm going to push this tomorrow.

Thanks again!

Stephen

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

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

iQIcBAEBCgAGBQJco5C0AAoJEO1sijiDR2RVvk4P/01+fm5JI+j00Raugqi0z607
eo/bl+3Ekdm4jPbPdHm1e5vfewpZ5VClvwfV7guTBjXGdu0zpVc6sQkxivbj3Cqy
spcef+ODeEUhrqmJ0cO+u8KYY2eN5PrxFhNL24lLv00JUW3fh/stIO47vnsSU5Tl
Jk8sCOqUERreSqFeG1Ak5sin4/qR4GPjxa7L0lEETtYK6unlmi/i4LPkq1CPNYQV
RRcPlmQlH2ItVGobxK76gZshUjNaZsojAw0oHrg/yyTRSvYH7dZQAuDHXuxRWJ0B
4Jp0+qUJ/p81vHiQhXYtePrc7D5e/G4AOxWhsp4LITpGVe5lPfcgx1d5A43Ai5ST
6r2OEn239ADgjqbMxbBgL5jfuLsHzRlxdx7aiGbf3jF1VCDqfICLACtjkThNQ7QN
Y+aKo2GoffyttNzz1+l++E85zGnEwmoGDDDtQBK/Qz6OlzFWeItcoREJ1DEsW9mf
GIshatTb9KH0/3+uXNCet2Q0iJD3aKjebzPXsQuF2/5rlmJJMQkH1GnvifFYYq/+
2nh+gIz+Tb4c/e+7qQ9bZRlLVfSkZSK5CHRmWZacBelBhFvq5bvEpVuvWfx/NWQd
GBuIR8eeQLg2/tXuip88SmrS4k9r3vde2X1tK7A9fBd8LhpGy9fJQ7QKF/U9bg5O
c9fwr2zXaEi5j3SWH7rt
=cxGr
-----END PGP SIGNATURE-----

--bPbpEglh0dw8VbL0--