Re: [HACKERS] RE: [INTERFACES] Re: SSL patch

Magnus Hagander <mha(at)sollentuna(dot)net> writes:
> Well. The problem is that the client sends the StartupPacket without
> reading anything at all from the server, which means it is too late to
> do SSL negotiation after the StartupPacket. It contains the password
> (possibly in clear-text), which would be one of the most important
> things to protect.

Actually, the StartupPacket does *not* contain a password. But it does
contain a valid database name and user name, which might be useful
information to an attacker, so I agree it would be good to protect it.

> Just to clearify: the SSL-enabled server still accepts 6.6 clients that are
> compiled without SSL support, but it will not accept from 6.5 clients, as it
> is now.

Right. My feeling is that we must make it possible for a 6.6 server to
accept connections from 6.5 (and earlier) clients, or the upgrade will
be too painful at large sites.

> One possibility would be that the client sent a negotiation packet _before_
> it sent the startuppacket. It would be a little bit weird to have this
> negotiation initiated from the client, but perhaps possible. OTOH, this will
> break compatibility in the way that a 6.6 client will not be able to talk to
> a 6.5 server.

Not if the 6.6 client is smart about recovering from a connection
failure. It could work like this:

Client opens connection
Client sends SSL negotiation packet
6.5 server (or SSL-less 6.6 server) sends back error msg
and closes connection
Client says "oh well", opens a new connection, and
proceeds with non-secure connection protocol

(Of course, if the client only wanted a secure connection, it'd give up
instead of making the second connection attempt.) This'd be a little
bit inefficient for new clients talking to old servers, but that doesn't
seem like it is a fatal objection --- in the other three cases there
is no extra overhead.

In the case where the server does have SSL capability, it accepts the
SSL packet, then the SSL negotiation is completed, and finally the
usual sort of StartupPacket is sent and the connection proceeds.

Of course, if the client does not want to use a secure connection, it
just opens the connection and sends a StartupPacket to begin with.

The only dubious assumption I can see in this is that the server has to
be able to distinguish an initial SSL negotiation packet from a
StartupPacket (and from a CancelRequestPacket). We should ensure that
that is true by prefixing an identifying word to the normal contents of
an SSL packet. Or, if it seems easiest, we could simply have that
initial client message consist *only* of a packet that means
"BeginSSLProtocol", and then the server side is the one that actually
starts the SSL negotiation. That is almost like your current patch ---
the critical differences are that the initial client message for an SSL
connection has to be set up so that an old server will reject it
cleanly, and the client has to be prepared to retry if that happens.

I think I prefer having the client's first message include the first
step of SSL negotiation if possible, since that would save one
packet transfer during the setup process. But if it's too hard to
make the SSL libraries play that way, we don't have to.

In any case, the initial client message for a non-SSL connection should
be a plain StartupPacket, and for an SSL connection it must be something
that an old server will refuse. That means we want the first 8 bytes to
be a packet length count and then a word that does not look like any
acceptable protocol version number. (Compare the way that
CancelRequestPackets are recognized.) The data payload of the packet
would either be the initial SSL negotiation data, or empty if you decide
that the server must send the initial SSL message.

> I had it set for that from the beginning, but didn't like it. The way I had
> it done then broke the client API, which I considered even worse (AKA the
> client had to specify to libpq if it was to use SSL or not, which meant that
> the interface to PQsetdb was changed - not just a simple upgrade to the new
> libpq was possible).

You'll still have an API addition, no? Something to set the SSL
connection option as "do not use SSL", "must use SSL or fail", or "use
SSL if server supports it". The last is a reasonable default if the
client doesn't specify, but the client must be able to specify. I guess
this would only be possible via a conninfo string...

regards, tom lane