On 11/19/17 23:08, Michael Paquier wrote:
> When using "n" or "y", the data sent by the client to the server about
> the use of channel binding is a base64-encoded string of respectively
> "n,," (biws) and "y,," (eSws). However, as noticed by Peter E here, a
> v10 server is able to allow connections with "n,,", but not with
> "y,,":
> https://www.postgresql.org/message-id/887b6fb7-15fe-239e-2aad-5911d2b09...@2ndquadrant.com
> 
> When trying to connect to a v11 client based on current HEAD to a v10
> server using SSL, then the connection would fail. The attached patch,
> for REL_10_STABLE, allows a server to accept as well as input "eSws",
> which is a combination that can now happen. This way, a v10 server
> accepts connections from a v11 and newer client with SSL.

I noticed what I think is an omission in the current v11 code.  We also
need to check whether the channel binding flag (n/y/p) encoded in the
client-final-message is the same one used in the client-first-message.
See attached patch.  This would also affect what we might end up
backpatching.

-- 
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
From 7db5626b0dd082ec782188759316fb345df37999 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pete...@gmx.net>
Date: Wed, 22 Nov 2017 14:02:57 -0500
Subject: [PATCH] Check channel binding flag at end of SCRAM exchange

We need to check whether the channel-binding flag encoded in the
client-final-message is the same one sent in the client-first-message.
---
 src/backend/libpq/auth-scram.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/src/backend/libpq/auth-scram.c b/src/backend/libpq/auth-scram.c
index 22103ce479..bfde6b746d 100644
--- a/src/backend/libpq/auth-scram.c
+++ b/src/backend/libpq/auth-scram.c
@@ -110,6 +110,7 @@ typedef struct
 
        const char *username;           /* username from startup packet */
 
+       char            cbind_flag;
        bool            ssl_in_use;
        const char *tls_finished_message;
        size_t          tls_finished_len;
@@ -788,6 +789,7 @@ read_client_first_message(scram_state *state, char *input)
         * Read gs2-cbind-flag.  (For details see also RFC 5802 Section 6 
"Channel
         * Binding".)
         */
+       state->cbind_flag = *input;
        switch (*input)
        {
                case 'n':
@@ -1108,6 +1110,8 @@ read_client_final_message(scram_state *state, char *input)
                char       *b64_message;
                int                     b64_message_len;
 
+               Assert(state->cbind_flag == 'p');
+
                /*
                 * Fetch data appropriate for channel binding type
                 */
@@ -1152,10 +1156,11 @@ read_client_final_message(scram_state *state, char 
*input)
                /*
                 * If we are not using channel binding, the binding data is 
expected
                 * to always be "biws", which is "n,," base64-encoded, or 
"eSws",
-                * which is "y,,".
+                * which is "y,,".  We also have to check whether the flag is 
the same
+                * one that the client originally sent.
                 */
-               if (strcmp(channel_binding, "biws") != 0 &&
-                       strcmp(channel_binding, "eSws") != 0)
+               if (!(strcmp(channel_binding, "biws") == 0 && state->cbind_flag 
== 'n') &&
+                       !(strcmp(channel_binding, "eSws") == 0 && 
state->cbind_flag == 'y'))
                        ereport(ERROR,
                                        (errcode(ERRCODE_PROTOCOL_VIOLATION),
                                         (errmsg("unexpected SCRAM 
channel-binding attribute in client-final-message"))));
-- 
2.15.0

Reply via email to