Re: [PATCH v12] GSSAPI encryption support

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Robbie Harwood <rharwood(at)redhat(dot)com>
Cc: Stephen Frost <sfrost(at)snowman(dot)net>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: [PATCH v12] GSSAPI encryption support
Date: 2016-04-06 21:10:31
Message-ID: 1530.1459977031@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Robbie Harwood <rharwood(at)redhat(dot)com> writes:
> I need to flush this any time we might be doing encryption because it
> needs to be in a separate request to _secure_write() from what follows
> it. We don't know whether we should be doing encryption until
> connection parameters are parsed; to put it another way,
> port->gss->encrypt will never be true here because it hasn't been parsed
> out of port->gss->gss_encrypt yet.

Wait a second. So the initial connection-request packet is necessarily
unencrypted under this scheme? That seems like a pretty substantial
step backwards from what happens with SSL. Even granting that stuff
like passwords won't be sent till later, the combination of user name
and database name might already be useful info to an eavesdropper.

I would think a design similar to the SSL one (special protocol version
to cause encryption negotiation before the actual connection request
is sent) would be better.

(If I'm totally misunderstanding the context here, my apologies. I've
not been following this thread.)

>> I'm aware that enlargeStringInfo() does check and handle the case where
>> the length ends up >1G, but that feels a bit grotty to me- are you sure
>> you want the generic enlargeStringInfo() to handle that case?

> This is a good point. We definitely have to draw the line somewhere; 1G
> is a high upper bound. Digging around the server code I don't see a
> precedent for what's a good size to stop at. There's
> PG_MAX_AUTH_TOKEN_LENGTH, which is 65k, and the password length in
> auth.c is 1k. Beyond that, SocketBackend() calls pq_getmessage() with
> no maximum length, which causes the enlargeStringInfo() size
> restrictions to be the only control (wrapped in a PG_TRY()).

Note that SocketBackend() only runs *after* we've accepted the user as
authorized. We should be a lot charier of what we're willing to accept
before authorization, IMO. Note MAX_STARTUP_PACKET_LENGTH, which is
only 10K.

regards, tom lane

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Petr Jelinek 2016-04-06 21:11:44 Re: VS 2015 support in src/tools/msvc
Previous Message David Rowley 2016-04-06 21:10:20 Re: Performance improvement for joins where outer side is unique