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