Re: [PATCH v12] GSSAPI encryption support

Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> writes:

> 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.

(Apologies for the wall of text that follows. My GSSAPI encryption
support has gone through three major redesigns, so I've got a fair bit
to say about it at this point, and it's probably better that I say too
much than too little. The short version is that GSSAPI works
differently than SSL and username is sent in the clear no matter what,
but this isn't a problem.)

Yes, by necessity. The username must be sent in the clear, even if only
as part of the GSSAPI handshake (i.e., the GSSAPI username will appear
in plantext in the GSSAPI blobs which are otherwise encrypted). GSSAPI
performs authentication before it can start encryption.

In this design, the contents of the Startup Message are the only
non-authentication related information sent in the clear. This
contains: username (which we need anyway), database, application_name,
and I add gss_encrypt.

Why does it look this way? Fallback support. We already have GSSAPI
authentication code in the project, and unfortunately we can't fix the
multitude of older clients that won't have encryption support, whatever
form it takes.

What if we didn't need fallback support, though? Doing it with a
special protocol version, as in the SSL/TLS case, would cause a separate
path for authentication to occur, during which everything would look
pretty much the same, except we wouldn't send database. We would then
complete the auth handshake, and in a separate exchange, pass in the
database information. Only then could we perform authorization
checking. Authorization checking is currently coupled with the
authentication as well; we would need a way to bypass the normal auth
sequence and enter encryption.

Bottom line is that designing similarly to SSL/TLS doesn't really make
sense because the two schemes work differently. Typically, usernames
are not considered sensitive information unless one is worried about the
security of the authentication information that goes with them. For
instance, MIT Kerberos will reveal the difference between "username not
found" and the equivalent of "bad password". I don't know how much
admins care about the database names being in the clear; I suspect it
doesn't matter much because just knowing their names isn't enough to
connect. Even if it's something people care about, this is still far
better than no encryption at all (which is the current GSSAPI behavior),
and would be better addressed by supporting connecting without
specifying a database immediately anyway.

>>> 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.

Correct, we're only talking about packets that are sent after
authentication+authorization are complete. Before authentication is
complete, the GSSAPI encryption codepaths are not taken.