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