Some more comments on this:
1. It feels weird that the combination of "gssencmode=require
sslnegotiation=direct" combination is forbidden. Sure, the ssl
negotiation will never happen with gssencmode=require, so the
sslnegotiation option has no effect. But by that token, should we also
forbid the combination "sslmode=disable sslnegotiation=direct"? I think
not. The sslnegotiation option should mean "if we are going to try SSL,
should we try it in direct or negotiated mode?"
2. Should we allow direct SSL only at the very beginning of a TCP
connection, or should we also allow it after we have requested GSS and
the server said no? Like this:
Client: GSSENCRequest
Server: 'N' (gss not supported)
Client: TLS client Hello
On one hand, why not? It saves you a round-trip in this case too. If we
don't allow it, the client will have to send SSLRequest and wait for
response, or reconnect to try direct SSL. On the other hand, flexibility
is not necessarily a good thing in security-critical code like this.
The patch set is confused on whether that's allowed or not. The server
rejects it. But if you use "gssencmode=prefer
sslnegotiation=requiredrect", libpq will attempt to do it, and fail.
3. With "sslmode=verify-full sslnegotiation=direct", if the direct SSL
connection fails because of a problem with the certificate, libpq will
try again in negotiated SSL mode. That seems pointless. If the server
responded to the direct TLS Client Hello message with a valid
ServerHello, that indicates that the server supports direct SSL. If
anything goes wrong after that, retrying in negotiated mode is not going
to help.
4. The number of combinations of sslmode, gssencmode and sslnegotiation
settings is scary. And we have very few tests for them.
Attached patch set addresses the above, but is very much WIP. I
refactored the state machine in libpq, to make the states and
transitions more clear. I think that helps, but it's still pretty
complex. I'm all ears for ideas on how to simplify it further.
I added a new test suite to test the different libpq options. See
src/test/libpq_encryption. I think this is very much needed, but I'm
still not very happy with the implementation. Some combinations are
still impossible to test, like connecting to an older server that
doesn't support direct SSL, or having the server respond with 'N' to
GSSEncRequest. I'd also like to check more details of each connection
attempt, like how many TCP connections are established, to check for
things like 3. above. Maybe we need to add more logging to libpq or the
server and check the logs after each test.
I'm tempted to implement a mock server from scratch that could easily be
instructed to accept/reject the connection at just the right places. But
that's a lot of work.
I'm going to put this down for now. The attached patch set is even more
raw than v6, but I'm including it here to "save the work".
--
Heikki Linnakangas
Neon (https://neon.tech)
