From 79125ead2a6a234086844bb42f06d49603fe6ca0 Mon Sep 17 00:00:00 2001
|
From: Tom Lane <tgl@sss.pgh.pa.us>
|
Date: Mon, 8 Nov 2021 11:14:56 -0500
|
Subject: [PATCH 1/2] libpq: reject extraneous data after SSL or GSS encryption
|
handshake.
|
|
libpq collects up to a bufferload of data whenever it reads data from
|
the socket. When SSL or GSS encryption is requested during startup,
|
any additional data received with the server's yes-or-no reply
|
remained in the buffer, and would be treated as already-decrypted data
|
once the encryption handshake completed. Thus, a man-in-the-middle
|
with the ability to inject data into the TCP connection could stuff
|
some cleartext data into the start of a supposedly encryption-protected
|
database session.
|
|
This could probably be abused to inject faked responses to the
|
client's first few queries, although other details of libpq's behavior
|
make that harder than it sounds. A different line of attack is to
|
exfiltrate the client's password, or other sensitive data that might
|
be sent early in the session. That has been shown to be possible with
|
a server vulnerable to CVE-2021-23214.
|
|
To fix, throw a protocol-violation error if the internal buffer
|
is not empty after the encryption handshake.
|
|
Our thanks to Jacob Champion for reporting this problem.
|
|
Security: CVE-2021-23222
|
|
Upstream-Status: Backport[https://github.com/postgres/postgres/commit/160c0258802d10b0600d7671b1bbea55d8e17d45]
|
CVE: CVE-2021-23222
|
|
Signed-off-by: Changqing Li <changqing.li@windriver.com>
|
---
|
doc/src/sgml/protocol.sgml | 28 ++++++++++++++++++++++++++++
|
src/interfaces/libpq/fe-connect.c | 26 ++++++++++++++++++++++++++
|
2 files changed, 54 insertions(+)
|
|
diff --git a/doc/src/sgml/protocol.sgml b/doc/src/sgml/protocol.sgml
|
index e26619e1b5..b692648fca 100644
|
--- a/doc/src/sgml/protocol.sgml
|
+++ b/doc/src/sgml/protocol.sgml
|
@@ -1471,6 +1471,20 @@ SELCT 1/0;<!-- this typo is intentional -->
|
and proceed without requesting <acronym>SSL</acronym>.
|
</para>
|
|
+ <para>
|
+ When <acronym>SSL</acronym> encryption can be performed, the server
|
+ is expected to send only the single <literal>S</literal> byte and then
|
+ wait for the frontend to initiate an <acronym>SSL</acronym> handshake.
|
+ If additional bytes are available to read at this point, it likely
|
+ means that a man-in-the-middle is attempting to perform a
|
+ buffer-stuffing attack
|
+ (<ulink url="https://www.postgresql.org/support/security/CVE-2021-23222/">CVE-2021-23222</ulink>).
|
+ Frontends should be coded either to read exactly one byte from the
|
+ socket before turning the socket over to their SSL library, or to
|
+ treat it as a protocol violation if they find they have read additional
|
+ bytes.
|
+ </para>
|
+
|
<para>
|
An initial SSLRequest can also be used in a connection that is being
|
opened to send a CancelRequest message.
|
@@ -1532,6 +1546,20 @@ SELCT 1/0;<!-- this typo is intentional -->
|
encryption.
|
</para>
|
|
+ <para>
|
+ When <acronym>GSSAPI</acronym> encryption can be performed, the server
|
+ is expected to send only the single <literal>G</literal> byte and then
|
+ wait for the frontend to initiate a <acronym>GSSAPI</acronym> handshake.
|
+ If additional bytes are available to read at this point, it likely
|
+ means that a man-in-the-middle is attempting to perform a
|
+ buffer-stuffing attack
|
+ (<ulink url="https://www.postgresql.org/support/security/CVE-2021-23222/">CVE-2021-23222</ulink>).
|
+ Frontends should be coded either to read exactly one byte from the
|
+ socket before turning the socket over to their GSSAPI library, or to
|
+ treat it as a protocol violation if they find they have read additional
|
+ bytes.
|
+ </para>
|
+
|
<para>
|
An initial GSSENCRequest can also be used in a connection that is being
|
opened to send a CancelRequest message.
|
diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
|
index f80f4e98d8..57aee95183 100644
|
--- a/src/interfaces/libpq/fe-connect.c
|
+++ b/src/interfaces/libpq/fe-connect.c
|
@@ -3076,6 +3076,19 @@ keep_going: /* We will come back to here until there is
|
pollres = pqsecure_open_client(conn);
|
if (pollres == PGRES_POLLING_OK)
|
{
|
+ /*
|
+ * At this point we should have no data already buffered.
|
+ * If we do, it was received before we performed the SSL
|
+ * handshake, so it wasn't encrypted and indeed may have
|
+ * been injected by a man-in-the-middle.
|
+ */
|
+ if (conn->inCursor != conn->inEnd)
|
+ {
|
+ appendPQExpBufferStr(&conn->errorMessage,
|
+ libpq_gettext("received unencrypted data after SSL response\n"));
|
+ goto error_return;
|
+ }
|
+
|
/* SSL handshake done, ready to send startup packet */
|
conn->status = CONNECTION_MADE;
|
return PGRES_POLLING_WRITING;
|
@@ -3175,6 +3188,19 @@ keep_going: /* We will come back to here until there is
|
pollres = pqsecure_open_gss(conn);
|
if (pollres == PGRES_POLLING_OK)
|
{
|
+ /*
|
+ * At this point we should have no data already buffered.
|
+ * If we do, it was received before we performed the GSS
|
+ * handshake, so it wasn't encrypted and indeed may have
|
+ * been injected by a man-in-the-middle.
|
+ */
|
+ if (conn->inCursor != conn->inEnd)
|
+ {
|
+ appendPQExpBufferStr(&conn->errorMessage,
|
+ libpq_gettext("received unencrypted data after GSSAPI encryption response\n"));
|
+ goto error_return;
|
+ }
|
+
|
/* All set for startup packet */
|
conn->status = CONNECTION_MADE;
|
return PGRES_POLLING_WRITING;
|
--
|
2.17.1
|
