Protocol problem with GSSAPI encryption?

On 2019-12-01 02:13, Andrew Gierth wrote:

But ProcessStartupPacket assumes that the packet after a failed
negotiation of either kind will be the actual startup packet, so the SSL
connection request is rejected with "unsupported version 1234.5679".

I'm guessing this usually goes unnoticed because most people are
probably not set up to do GSSAPI, and those who are are probably ok with
using it for encryption. But if the client is set up for GSSAPI and the
server not, then trying to do an SSL connection will fail when it should
succeed, and PGGSSENCMODE=disable in the environment (or connect string)
is necessary to get the connection to succeed.

It seems to me that this is a bug in ProcessStartupPacket, which should
accept both GSS or SSL negotiation requests on a connection (in either
order). Maybe secure_done should be two flags rather than one?

I have also seen reports of that. I think your analysis is correct.

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

"Peter" == Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:

It seems to me that this is a bug in ProcessStartupPacket, which
should accept both GSS or SSL negotiation requests on a connection
(in either order). Maybe secure_done should be two flags rather than
one?

Peter> I have also seen reports of that. I think your analysis is
Peter> correct.

I figure something along these lines for the fix. Anyone in a position
to test this?

--
Andrew (irc:RhodiumToad)

Greetings,

* Andrew Gierth (andrew@tao11.riddles.org.uk) wrote:

"Peter" == Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:

It seems to me that this is a bug in ProcessStartupPacket, which
should accept both GSS or SSL negotiation requests on a connection
(in either order). Maybe secure_done should be two flags rather than
one?

Peter> I have also seen reports of that. I think your analysis is
Peter> correct.

I figure something along these lines for the fix. Anyone in a position
to test this?

At least at first blush, I tend to agree with your analysis and patch.

I'll see about getting this actually set up and tested in the next week
or so (and maybe there's some way to also manage to have a regression
test for it..).

Thanks!

Stephen

On 4. Dec 2019, at 06:24, Stephen Frost <sfrost@snowman.net> wrote:

Greetings,

* Andrew Gierth (andrew@tao11.riddles.org.uk) wrote:

"Peter" == Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:

It seems to me that this is a bug in ProcessStartupPacket, which
should accept both GSS or SSL negotiation requests on a connection
(in either order). Maybe secure_done should be two flags rather than
one?

Peter> I have also seen reports of that. I think your analysis is
Peter> correct.

I figure something along these lines for the fix. Anyone in a position
to test this?

At least at first blush, I tend to agree with your analysis and patch.

I agree with the patch, but this also needs to be fixed on the client side.
Otherwise libpq won't be able to connect to older servers.

I'm attaching a proposed second patch to detect the error on the client side and reconnect to this message.

This patch was first submitted as a separate thread here:
/messages/by-id/F27EEE9D-D04A-4B6B-B1F1-96EA4DD996D0@eggerapps.at

Jakob

On Sun, Dec 1, 2019 at 01:13:31AM +0000, Andrew Gierth wrote:

This came up recently on IRC, not sure if the report there was passed on
at all.

ProcessStartupPacket assumes that there will be only one negotiation
request for an encrypted connection, but libpq is capable of issuing
two: it will ask for GSS encryption first, if it looks like it will be
able to do GSSAPI, and if the server refuses that it will ask (on the
same connection) for SSL.

Are you saying that there is an additional round-trip for starting all
SSL connections because we now support GSSAPI, or this only happens if
libpq asks for GSSAPI?

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ As you are, so once was I.  As I am, so you will be. +
+                      Ancient Roman grave inscription +

Greetings,

* Bruce Momjian (bruce@momjian.us) wrote:

On Sun, Dec 1, 2019 at 01:13:31AM +0000, Andrew Gierth wrote:

This came up recently on IRC, not sure if the report there was passed on
at all.

ProcessStartupPacket assumes that there will be only one negotiation
request for an encrypted connection, but libpq is capable of issuing
two: it will ask for GSS encryption first, if it looks like it will be
able to do GSSAPI, and if the server refuses that it will ask (on the
same connection) for SSL.

Are you saying that there is an additional round-trip for starting all
SSL connections because we now support GSSAPI, or this only happens if
libpq asks for GSSAPI?

The way that this is intended to work is if, and only if, there's is a
valid GSS credentical cache (on the client side) will GSSAPI encryption
be attempted and then if that fails because the server doesn't support
GSSAPI encryption of it's not possible to acquire credentials for
whatever reason then we'll fall back to other methods.

I have heard, however, that the Applie GSS libraries are both outright
broken (they lie about a valid credential cache existing- claiming one
does even when that's clearly not the case, based on klist..), and
deprecated (so they aren't likely going to fix them either..). We're
currently looking to see if there's a way to basically detect the Apple
GSS libraries and refuse to build if we discover that's what we're
building against. I'm not sure what other choice we really have...

If you gdb psql, without a Kerberos credential cache, on a system that
has a working GSS library, you'll note that pg_GSS_have_cred_cache()
returns false, meaning we skip over the GSS startup code in
PQconnectPoll() (and drop down to trying to do SSL next).

Thanks,

Stephen

"Bruce" == Bruce Momjian <bruce@momjian.us> writes:

This came up recently on IRC, not sure if the report there was
passed on at all.

ProcessStartupPacket assumes that there will be only one negotiation
request for an encrypted connection, but libpq is capable of issuing
two: it will ask for GSS encryption first, if it looks like it will
be able to do GSSAPI, and if the server refuses that it will ask (on
the same connection) for SSL.

Bruce> Are you saying that there is an additional round-trip for
Bruce> starting all SSL connections because we now support GSSAPI, or
Bruce> this only happens if libpq asks for GSSAPI?

The problem only occurs if libpq thinks it might be able to do GSSAPI,
but the server does not. Without the patch I proposed or something like
it, this case fails to connect at all; with it, there will be an extra
round-trip. Explicitly disabling GSSAPI encryption in the connection
string or environment avoids the issue.

The exact condition for libpq seems to be a successful call to
gss_acquire_cred, but I'm not familiar with GSS in general.

--
Andrew (irc:RhodiumToad)

On Fri, Dec 20, 2019 at 06:14:09PM +0000, Andrew Gierth wrote:

"Bruce" == Bruce Momjian <bruce@momjian.us> writes:

This came up recently on IRC, not sure if the report there was
passed on at all.

ProcessStartupPacket assumes that there will be only one negotiation
request for an encrypted connection, but libpq is capable of issuing
two: it will ask for GSS encryption first, if it looks like it will
be able to do GSSAPI, and if the server refuses that it will ask (on
the same connection) for SSL.

Bruce> Are you saying that there is an additional round-trip for
Bruce> starting all SSL connections because we now support GSSAPI, or
Bruce> this only happens if libpq asks for GSSAPI?

The problem only occurs if libpq thinks it might be able to do GSSAPI,
but the server does not. Without the patch I proposed or something like
it, this case fails to connect at all; with it, there will be an extra
round-trip. Explicitly disabling GSSAPI encryption in the connection
string or environment avoids the issue.

The exact condition for libpq seems to be a successful call to
gss_acquire_cred, but I'm not familiar with GSS in general.

Thanks for the clarification from you and Stephen.

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ As you are, so once was I.  As I am, so you will be. +
+                      Ancient Roman grave inscription +

"Stephen" == Stephen Frost <sfrost@snowman.net> writes:

I figure something along these lines for the fix. Anyone in a
position to test this?

Stephen> At least at first blush, I tend to agree with your analysis
Stephen> and patch.

Stephen> I'll see about getting this actually set up and tested in the
Stephen> next week or so (and maybe there's some way to also manage to
Stephen> have a regression test for it..).

*poke*

--
Andrew (irc:RhodiumToad)

On Fri, Feb 21, 2020 at 12:35:03AM +0000, Andrew Gierth wrote:

"Stephen" == Stephen Frost <sfrost@snowman.net> writes:

I figure something along these lines for the fix. Anyone in a
position to test this?

Stephen> At least at first blush, I tend to agree with your analysis
Stephen> and patch.

Stephen> I'll see about getting this actually set up and tested in the
Stephen> next week or so (and maybe there's some way to also manage to
Stephen> have a regression test for it..).

*poke*

Second *poke*.
--
Michael

Greetings,

* Stephen Frost (sfrost@snowman.net) wrote:

* Andrew Gierth (andrew@tao11.riddles.org.uk) wrote:

"Peter" == Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:

It seems to me that this is a bug in ProcessStartupPacket, which
should accept both GSS or SSL negotiation requests on a connection
(in either order). Maybe secure_done should be two flags rather than
one?

Peter> I have also seen reports of that. I think your analysis is
Peter> correct.

I figure something along these lines for the fix. Anyone in a position
to test this?

At least at first blush, I tend to agree with your analysis and patch.

I'll see about getting this actually set up and tested in the next week
or so (and maybe there's some way to also manage to have a regression
test for it..).

After testing this and playing around with it a bit, I've gone ahead and
pushed it.

Thanks!

Stephen