From: Robbie Harwood <rharwood@redhat.com>
To: Stephen Frost <sfrost@snowman.net>
Cc: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
Cc: pgsql-hackers@postgresql.org, Nico Williams <nico@cryptonector.com>
Cc: Michael Paquier <michael@paquier.xyz>
Cc: Alvaro Herrera <alvherre@2ndquadrant.com>
Cc: David Steele <david@pgmasters.net>
Subject: Re: [PATCH v22] GSSAPI encryption support
Date: Mon, 01 Apr 2019 11:13:24 -0400
Message-ID: <jlgv9zx4v3f.fsf@redhat.com> (raw)
In-Reply-To: <20190330142404.GO6197@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>
Stephen Frost <sfrost@snowman.net> writes:
> * Robbie Harwood (rharwood@redhat.com) wrote:
>> Stephen Frost <sfrost@snowman.net> writes:
>>
>> 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.
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.
> 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.
Good idea.
>> 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.
I'm happy with this! Appreciate you exploring my concerns.
Thanks,
--Robbie
Attachments:
signature.asc (application/pgp-signature, 832B) download
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: rharwood@redhat.com, sfrost@snowman.net, peter.eisentraut@2ndquadrant.com, nico@cryptonector.com, michael@paquier.xyz, alvherre@2ndquadrant.com, david@pgmasters.net
Subject: Re: [PATCH v22] GSSAPI encryption support
In-Reply-To: <jlgv9zx4v3f.fsf@redhat.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox