On Wed, Nov 29, 2017 at 1:04 AM, Peter Eisentraut <peter.eisentr...@2ndquadrant.com> wrote: > On 11/22/17 21:08, Michael Paquier wrote: >> Yes, agreed. This patch looks good to me. In fe-auth-scram.c, it would >> be also nice to add a comment to keep in sync the logics in >> build_client_first_message() and build_client_final_message() which >> assign the cbind flag value. > > Could you clarify what comment you would like to have added or changed?
Sure. Here is with the attached patch what I have in mind. The way cbind-flag is assigned in the client-first message should be kept in-sync with the way the client-final message builds the binding data in c=. It could be possible to add more sanity-checks based on assertions by keeping track of the cbind-flag assigned in the client-first message as your upthread patch is doing in the backend code, but I see a simple comment as a sufficient reminder. -- Michael
diff --git a/src/interfaces/libpq/fe-auth-scram.c b/src/interfaces/libpq/fe-auth-scram.c index 97db0b1faa..7ef5cc437e 100644 --- a/src/interfaces/libpq/fe-auth-scram.c +++ b/src/interfaces/libpq/fe-auth-scram.c @@ -437,6 +437,8 @@ build_client_final_message(fe_scram_state *state, PQExpBuffer errormessage) /* * Construct client-final-message-without-proof. We need to remember it * for verifying the server proof in the final step of authentication. + * This needs to be kept consistent with the cbind_flag handling when + * building the first client message in build_client_first_message(). */ if (strcmp(state->sasl_mechanism, SCRAM_SHA256_PLUS_NAME) == 0) {