Re: [PATCH v22] GSSAPI encryption support

From: Robbie Harwood <rharwood(at)redhat(dot)com>
To: Stephen Frost <sfrost(at)snowman(dot)net>
Cc: Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com>, pgsql-hackers(at)postgresql(dot)org, Nico Williams <nico(at)cryptonector(dot)com>, Michael Paquier <michael(at)paquier(dot)xyz>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, David Steele <david(at)pgmasters(dot)net>
Subject: Re: [PATCH v22] GSSAPI encryption support
Date: 2019-04-01 15:13:24
Message-ID: jlgv9zx4v3f.fsf@redhat.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Stephen Frost <sfrost(at)snowman(dot)net> writes:

> * Robbie Harwood (rharwood(at)redhat(dot)com) wrote:
>> Stephen Frost <sfrost(at)snowman(dot)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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message David Rowley 2019-04-01 15:23:20 Re: COPY FROM WHEN condition
Previous Message Tom Lane 2019-04-01 14:29:14 Re: speeding up planning with partitions