From 8649faeff121d4e7d11030c62056609e401b3e55 Mon Sep 17 00:00:00 2001 From: Michael Paquier Date: Fri, 1 Dec 2017 10:47:51 +0900 Subject: [PATCH 2/3] Add connection parameter "scramchannelbinding" This parameter can be used to enforce the value of the type of channel binding used during a SASL message exchange. This proves to be useful now to check code paths where an invalid channel binding type is used by a client, which is limited, but will be more useful to allow clients to enforce the channel binding type to tls-enpoint once it gets added. The default value is tls-unique, which is what RFC 5802 specifies. Clients can optionally specify an empty value which has as effect to not use channel binding and use SCRAM-SHA-256 as SASL mechanism chosen. More tests dedicated to SASL and channel binding are added as well to the SSL test suite, which is handy to check the validity of this patch. --- doc/src/sgml/libpq.sgml | 14 ++++++++++++++ src/interfaces/libpq/fe-auth-scram.c | 20 +++++++++++++++----- src/interfaces/libpq/fe-auth.c | 9 ++++++--- src/interfaces/libpq/fe-auth.h | 1 + src/interfaces/libpq/fe-connect.c | 10 ++++++++++ src/interfaces/libpq/libpq-int.h | 1 + src/test/ssl/t/002_scram.pl | 14 +++++++++++++- 7 files changed, 60 insertions(+), 9 deletions(-) diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml index 4703309254..8f1588d0c6 100644 --- a/doc/src/sgml/libpq.sgml +++ b/doc/src/sgml/libpq.sgml @@ -1222,6 +1222,20 @@ postgresql://%2Fvar%2Flib%2Fpostgresql/dbname + + scramchannelbinding + + + Controls the name of the channel binding type sent to server when doing + a message exchange for a SCRAM authentication. The list of channel + binding names supported by server are listed in + . An empty value allows client + to not use channel binding. The default value is + tls-unique. + + + + sslmode diff --git a/src/interfaces/libpq/fe-auth-scram.c b/src/interfaces/libpq/fe-auth-scram.c index 8acad6f150..66e01aefaa 100644 --- a/src/interfaces/libpq/fe-auth-scram.c +++ b/src/interfaces/libpq/fe-auth-scram.c @@ -93,6 +93,7 @@ pg_fe_scram_init(const char *username, const char *password, bool ssl_in_use, const char *sasl_mechanism, + const char *channel_binding_type, char *tls_finished_message, size_t tls_finished_len) { @@ -112,17 +113,14 @@ pg_fe_scram_init(const char *username, state->tls_finished_message = tls_finished_message; state->tls_finished_len = tls_finished_len; state->sasl_mechanism = strdup(sasl_mechanism); + state->channel_binding_type = channel_binding_type; + if (!state->sasl_mechanism) { free(state); return NULL; } - /* - * Store channel binding type. Only one type is currently supported. - */ - state->channel_binding_type = SCRAM_CHANNEL_BINDING_TLS_UNIQUE; - /* Normalize the password with SASLprep, if possible */ rc = pg_saslprep(password, &prep_password); if (rc == SASLPREP_OOM) @@ -375,6 +373,15 @@ build_client_first_message(fe_scram_state *state, PQExpBuffer errormessage) Assert(state->ssl_in_use); appendPQExpBuffer(&buf, "p=%s", state->channel_binding_type); } + else if (state->channel_binding_type == NULL || + strlen(state->channel_binding_type) == 0) + { + /* + * Client has chosen to not show to server that it supports channel + * binding. + */ + appendPQExpBuffer(&buf, "n"); + } else if (state->ssl_in_use) { /* @@ -489,6 +496,9 @@ build_client_final_message(fe_scram_state *state, PQExpBuffer errormessage) free(cbind_input); } + else if (state->channel_binding_type == NULL || + strlen(state->channel_binding_type) == 0) + appendPQExpBuffer(&buf, "c=biws"); /* base64 of "n,," */ else if (state->ssl_in_use) appendPQExpBuffer(&buf, "c=eSws"); /* base64 of "y,," */ else diff --git a/src/interfaces/libpq/fe-auth.c b/src/interfaces/libpq/fe-auth.c index 2cfdb7c125..5828d4207c 100644 --- a/src/interfaces/libpq/fe-auth.c +++ b/src/interfaces/libpq/fe-auth.c @@ -528,11 +528,13 @@ pg_SASL_init(PGconn *conn, int payloadlen) /* * Select the mechanism to use. Pick SCRAM-SHA-256-PLUS over anything - * else. Pick SCRAM-SHA-256 if nothing else has already been picked. - * If we add more mechanisms, a more refined priority mechanism might - * become necessary. + * else if a channel binding type is defined. Pick SCRAM-SHA-256 if + * nothing else has already been picked. If we add more mechanisms, a + * more refined priority mechanism might become necessary. */ if (conn->ssl_in_use && + conn->scramchannelbinding && + strlen(conn->scramchannelbinding) > 0 && strcmp(mechanism_buf.data, SCRAM_SHA256_PLUS_NAME) == 0) selected_mechanism = SCRAM_SHA256_PLUS_NAME; else if (strcmp(mechanism_buf.data, SCRAM_SHA256_NAME) == 0 && @@ -591,6 +593,7 @@ pg_SASL_init(PGconn *conn, int payloadlen) password, conn->ssl_in_use, selected_mechanism, + conn->scramchannelbinding, tls_finished, tls_finished_len); if (!conn->sasl_state) diff --git a/src/interfaces/libpq/fe-auth.h b/src/interfaces/libpq/fe-auth.h index 3e92410eae..db319ac071 100644 --- a/src/interfaces/libpq/fe-auth.h +++ b/src/interfaces/libpq/fe-auth.h @@ -27,6 +27,7 @@ extern void *pg_fe_scram_init(const char *username, const char *password, bool ssl_in_use, const char *sasl_mechanism, + const char *channel_binding_type, char *tls_finished_message, size_t tls_finished_len); extern void pg_fe_scram_free(void *opaq); diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c index 2c175a2a24..ecaa7a541e 100644 --- a/src/interfaces/libpq/fe-connect.c +++ b/src/interfaces/libpq/fe-connect.c @@ -71,6 +71,7 @@ static int ldapServiceLookup(const char *purl, PQconninfoOption *options, #endif #include "common/ip.h" +#include "common/scram-common.h" #include "mb/pg_wchar.h" #include "port/pg_bswap.h" @@ -122,6 +123,7 @@ static int ldapServiceLookup(const char *purl, PQconninfoOption *options, #define DefaultOption "" #define DefaultAuthtype "" #define DefaultTargetSessionAttrs "any" +#define DefaultSCRAMChannelBinding SCRAM_CHANNEL_BINDING_TLS_UNIQUE #ifdef USE_SSL #define DefaultSSLMode "prefer" #else @@ -262,6 +264,12 @@ static const internalPQconninfoOption PQconninfoOptions[] = { "TCP-Keepalives-Count", "", 10, /* strlen(INT32_MAX) == 10 */ offsetof(struct pg_conn, keepalives_count)}, + /* Set of options proper to SCRAM */ + {"scramchannelbinding", NULL, DefaultSCRAMChannelBinding, NULL, + "SCRAM-Channel-Binding", "", + 22, /* sizeof("tls-unique-for-telnet") == 22 */ + offsetof(struct pg_conn, scramchannelbinding)}, + /* * ssl options are allowed even without client SSL support because the * client can still handle SSL modes "disable" and "allow". Other @@ -3469,6 +3477,8 @@ freePGconn(PGconn *conn) free(conn->keepalives_interval); if (conn->keepalives_count) free(conn->keepalives_count); + if (conn->scramchannelbinding) + free(conn->scramchannelbinding); if (conn->sslmode) free(conn->sslmode); if (conn->sslcert) diff --git a/src/interfaces/libpq/libpq-int.h b/src/interfaces/libpq/libpq-int.h index 8412ee8160..b116ff8032 100644 --- a/src/interfaces/libpq/libpq-int.h +++ b/src/interfaces/libpq/libpq-int.h @@ -349,6 +349,7 @@ struct pg_conn * retransmits */ char *keepalives_count; /* maximum number of TCP keepalive * retransmits */ + char *scramchannelbinding; /* channel binding type used in SASL */ char *sslmode; /* SSL mode (require,prefer,allow,disable) */ char *sslcompression; /* SSL compression (0 or 1) */ char *sslkey; /* client key filename */ diff --git a/src/test/ssl/t/002_scram.pl b/src/test/ssl/t/002_scram.pl index 25f75bd52a..c8e9114d64 100644 --- a/src/test/ssl/t/002_scram.pl +++ b/src/test/ssl/t/002_scram.pl @@ -4,7 +4,7 @@ use strict; use warnings; use PostgresNode; use TestLib; -use Test::More tests => 1; +use Test::More tests => 4; use ServerSetup; use File::Copy; @@ -34,5 +34,17 @@ $ENV{PGPASSWORD} = "pass"; $common_connstr = "user=ssltestuser dbname=trustdb sslmode=require hostaddr=$SERVERHOSTADDR"; +# Default settings test_connect_ok($common_connstr, '', "SCRAM authentication with default channel binding"); + +# Channel bindings +test_connect_ok($common_connstr, + "scramchannelbinding=tls-unique", + "SCRAM authentication with tls-unique as channel binding"); +test_connect_ok($common_connstr, + "scramchannelbinding=''", + "SCRAM authentication without channel binding"); +test_connect_fails($common_connstr, + "scramchannelbinding=not-exists", + "SCRAM authentication with invalid channel binding"); -- 2.15.0