Protocol problem with GSSAPI encryption?

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

Protocol problem with GSSAPI encryption?

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

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'm not really familiar with the GSSAPI stuff so probably someone who is
should take a look.

--
Andrew (irc:RhodiumToad)


Reply | Threaded
Open this post in threaded view
|

Re: Protocol problem with GSSAPI encryption?

Peter Eisentraut-6
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


Reply | Threaded
Open this post in threaded view
|

Re: Protocol problem with GSSAPI encryption?

Andrew Gierth
>>>>> "Peter" == Peter Eisentraut <[hidden email]> 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)


diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
index 9ff2832c00..1b51d4916d 100644
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -404,7 +404,7 @@ static void BackendRun(Port *port) pg_attribute_noreturn();
 static void ExitPostmaster(int status) pg_attribute_noreturn();
 static int ServerLoop(void);
 static int BackendStartup(Port *port);
-static int ProcessStartupPacket(Port *port, bool secure_done);
+static int ProcessStartupPacket(Port *port, bool ssl_done, bool gss_done);
 static void SendNegotiateProtocolVersion(List *unrecognized_protocol_options);
 static void processCancelRequest(Port *port, void *pkt);
 static int initMasks(fd_set *rmask);
@@ -1914,11 +1914,15 @@ initMasks(fd_set *rmask)
  * send anything to the client, which would typically be appropriate
  * if we detect a communications failure.)
  *
- * Set secure_done when negotiation of an encrypted layer (currently, TLS or
- * GSSAPI) is already completed.
+ * Set ssl_done and/or gss_done when negotiation of an encrypted layer
+ * (currently, TLS or GSSAPI) is completed. A successful negotiation of either
+ * encryption layer sets both flags, but a rejected negotiation sets only the
+ * flag for that layer, since the client may wish to try the other one. We
+ * should make no assumption here about the order in which the client may make
+ * requests.
  */
 static int
-ProcessStartupPacket(Port *port, bool secure_done)
+ProcessStartupPacket(Port *port, bool ssl_done, bool gss_done)
 {
  int32 len;
  void   *buf;
@@ -1951,7 +1955,7 @@ ProcessStartupPacket(Port *port, bool secure_done)
  if (pq_getbytes(((char *) &len) + 1, 3) == EOF)
  {
  /* Got a partial length word, so bleat about that */
- if (!secure_done)
+ if (!ssl_done && !gss_done)
  ereport(COMMERROR,
  (errcode(ERRCODE_PROTOCOL_VIOLATION),
  errmsg("incomplete startup packet")));
@@ -2003,7 +2007,7 @@ ProcessStartupPacket(Port *port, bool secure_done)
  return STATUS_ERROR;
  }
 
- if (proto == NEGOTIATE_SSL_CODE && !secure_done)
+ if (proto == NEGOTIATE_SSL_CODE && !ssl_done)
  {
  char SSLok;
 
@@ -2032,11 +2036,14 @@ retry1:
  if (SSLok == 'S' && secure_open_server(port) == -1)
  return STATUS_ERROR;
 #endif
- /* regular startup packet, cancel, etc packet should follow... */
- /* but not another SSL negotiation request */
- return ProcessStartupPacket(port, true);
+ /*
+ * regular startup packet, cancel, etc packet should follow, but not
+ * another SSL negotiation request, and a GSS request should only
+ * follow if SSL was rejected (client may negotiate in either order)
+ */
+ return ProcessStartupPacket(port, true, SSLok == 'S');
  }
- else if (proto == NEGOTIATE_GSS_CODE && !secure_done)
+ else if (proto == NEGOTIATE_GSS_CODE && !gss_done)
  {
  char GSSok = 'N';
 #ifdef ENABLE_GSS
@@ -2059,8 +2066,12 @@ retry1:
  if (GSSok == 'G' && secure_open_gssapi(port) == -1)
  return STATUS_ERROR;
 #endif
- /* Won't ever see more than one negotiation request */
- return ProcessStartupPacket(port, true);
+ /*
+ * regular startup packet, cancel, etc packet should follow, but not
+ * another GSS negotiation request, and an SSL request should only
+ * follow if GSS was rejected (client may negotiate in either order)
+ */
+ return ProcessStartupPacket(port, GSSok == 'G', true);
  }
 
  /* Could add additional special packet types here */
@@ -4400,7 +4411,7 @@ BackendInitialize(Port *port)
  * Receive the startup packet (which might turn out to be a cancel request
  * packet).
  */
- status = ProcessStartupPacket(port, false);
+ status = ProcessStartupPacket(port, false, false);
 
  /*
  * Stop here if it was bad or a cancel packet.  ProcessStartupPacket
Reply | Threaded
Open this post in threaded view
|

Re: Protocol problem with GSSAPI encryption?

Stephen Frost
Greetings,

* Andrew Gierth ([hidden email]) wrote:

> >>>>> "Peter" == Peter Eisentraut <[hidden email]> 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

signature.asc (836 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Protocol problem with GSSAPI encryption?

Jakob Egger-2

> On 4. Dec 2019, at 06:24, Stephen Frost <[hidden email]> wrote:
>
> Greetings,
>
> * Andrew Gierth ([hidden email]) wrote:
>>>>>>> "Peter" == Peter Eisentraut <[hidden email]> 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:
https://www.postgresql.org/message-id/F27EEE9D-D04A-4B6B-B1F1-96EA4DD996D0@...


Jakob




0002-libpq-Retry-after-failed-ssl-gss-negotiation.patch (8K) Download Attachment