Hi all, Per the recent discussions around support of new SSL implementations for Postgres, it is becoming rather clear to me that the backend needs to be a bit smarter with the way it needs to decide if it should publish or not SCRAM-SHA-256-PLUS in the list that the clients can use to choose a SASL mechanism for authentication.
This has been discussed here in the MacOS SSL implementation: https://www.postgresql.org/message-id/20180122014637.GE22690%40paquier.xyz As well as here for GnuTLS: https://www.postgresql.org/message-id/CAB7nPqRJteyoyyj8E__v1D1SMoj8jpv6ZPyHuc7Md45%2BED-uMA%40mail.gmail.com Attached is a patch which is an attempt to make this choice cleaner for new SSL implementations. As we are (rightly!) calling the APIs to fetch the channel binding data only when necessary, I think that we need an extra API for SSL implementations to let the server decide if channel binding mechanisms should be published or not. There could be multiple possibilities, like making this API return only a boolean flag. However I have made a more extensible choice by having each SSL implementation build a list of supported channel bindings. This matters for each implementation as: - GnuTLS only supports tls-unique. - OpenSSL supports both tls-unique and tls-server-end-point. - MacOS would support none. However there is as well the argument that this list's contents are not directly used now, and based on what I saw from the MacOS SSL and GnuTLS patches that would not be the case after either. I am adding that to the next CF for consideration. Thoughts? -- Michael
diff --git a/src/backend/libpq/auth.c b/src/backend/libpq/auth.c index 746d7cbb8a..ecaab21e13 100644 --- a/src/backend/libpq/auth.c +++ b/src/backend/libpq/auth.c @@ -873,6 +873,7 @@ CheckSCRAMAuth(Port *port, char *shadow_pass, char **logdetail) int inputlen; int result; bool initial; + List *channel_bindings = NIL; /* * SASL auth is not supported for protocol versions before 3, because it @@ -898,7 +899,17 @@ CheckSCRAMAuth(Port *port, char *shadow_pass, char **logdetail) strlen(SCRAM_SHA256_NAME) + 3); p = sasl_mechs; - if (port->ssl_in_use) +#ifdef USE_SSL + /* + * Get the list of channel binding types supported by this SSL + * implementation to determine if server should publish -PLUS + * mechanisms or not. + */ + channel_bindings = be_tls_list_channel_bindings(); +#endif + + if (port->ssl_in_use && + list_length(channel_bindings) > 0) { strcpy(p, SCRAM_SHA256_PLUS_NAME); p += strlen(SCRAM_SHA256_PLUS_NAME) + 1; diff --git a/src/backend/libpq/be-secure-openssl.c b/src/backend/libpq/be-secure-openssl.c index fc6e8a0a88..95511a61b3 100644 --- a/src/backend/libpq/be-secure-openssl.c +++ b/src/backend/libpq/be-secure-openssl.c @@ -58,6 +58,7 @@ #include <openssl/ec.h> #endif +#include "common/scram-common.h" #include "libpq/libpq.h" #include "miscadmin.h" #include "pgstat.h" @@ -1215,6 +1216,18 @@ be_tls_get_peerdn_name(Port *port, char *ptr, size_t len) ptr[0] = '\0'; } +/* + * Routine to get the list of channel binding types available in this SSL + * implementation. For OpenSSL, both tls-unique and tls-server-end-point + * are supported. + */ +List * +be_tls_list_channel_bindings(void) +{ + return list_make2(pstrdup(SCRAM_CHANNEL_BINDING_TLS_UNIQUE), + pstrdup(SCRAM_CHANNEL_BINDING_TLS_END_POINT)); +} + /* * Routine to get the expected TLS Finished message information from the * client, useful for authorization when doing channel binding. diff --git a/src/include/libpq/libpq-be.h b/src/include/libpq/libpq-be.h index 49cb263110..3c37e800c1 100644 --- a/src/include/libpq/libpq-be.h +++ b/src/include/libpq/libpq-be.h @@ -209,6 +209,7 @@ extern bool be_tls_get_compression(Port *port); extern void be_tls_get_version(Port *port, char *ptr, size_t len); extern void be_tls_get_cipher(Port *port, char *ptr, size_t len); extern void be_tls_get_peerdn_name(Port *port, char *ptr, size_t len); +extern List *be_tls_list_channel_bindings(void); extern char *be_tls_get_peer_finished(Port *port, size_t *len); extern char *be_tls_get_certificate_hash(Port *port, size_t *len); #endif
signature.asc
Description: PGP signature