Commit f3049a60 authored by Peter Eisentraut's avatar Peter Eisentraut

Refactor channel binding code to fetch cbind_data only when necessary

As things stand now, channel binding data is fetched from OpenSSL and
saved into the SCRAM exchange context for any SSL connection attempted
for a SCRAM authentication, resulting in data fetched but not used if no
channel binding is used or if a different channel binding type is used
than what the data is here for.

Refactor the code in such a way that binding data is fetched from the
SSL stack only when a specific channel binding is used for both the
frontend and the backend.  In order to achieve that, save the libpq
connection context directly in the SCRAM exchange state, and add a
dependency to SSL in the low-level SCRAM routines.

This makes the interface in charge of initializing the SCRAM context
cleaner as all its data comes from either PGconn* (for frontend) or
Port* (for the backend).

Author: Michael Paquier <michael.paquier@gmail.com>
parent 3ad2afc2
......@@ -110,10 +110,8 @@ typedef struct
const char *username; /* username from startup packet */
Port *port;
char cbind_flag;
bool ssl_in_use;
const char *tls_finished_message;
size_t tls_finished_len;
char *channel_binding_type;
int iterations;
......@@ -172,21 +170,15 @@ static char *scram_mock_salt(const char *username);
* it will fail, as if an incorrect password was given.
*/
void *
pg_be_scram_init(const char *username,
const char *shadow_pass,
bool ssl_in_use,
const char *tls_finished_message,
size_t tls_finished_len)
pg_be_scram_init(Port *port,
const char *shadow_pass)
{
scram_state *state;
bool got_verifier;
state = (scram_state *) palloc0(sizeof(scram_state));
state->port = port;
state->state = SCRAM_AUTH_INIT;
state->username = username;
state->ssl_in_use = ssl_in_use;
state->tls_finished_message = tls_finished_message;
state->tls_finished_len = tls_finished_len;
state->channel_binding_type = NULL;
/*
......@@ -209,7 +201,7 @@ pg_be_scram_init(const char *username,
*/
ereport(LOG,
(errmsg("invalid SCRAM verifier for user \"%s\"",
username)));
state->port->user_name)));
got_verifier = false;
}
}
......@@ -220,7 +212,7 @@ pg_be_scram_init(const char *username,
* authentication with an MD5 hash.)
*/
state->logdetail = psprintf(_("User \"%s\" does not have a valid SCRAM verifier."),
state->username);
state->port->user_name);
got_verifier = false;
}
}
......@@ -242,8 +234,8 @@ pg_be_scram_init(const char *username,
*/
if (!got_verifier)
{
mock_scram_verifier(username, &state->iterations, &state->salt,
state->StoredKey, state->ServerKey);
mock_scram_verifier(state->port->user_name, &state->iterations,
&state->salt, state->StoredKey, state->ServerKey);
state->doomed = true;
}
......@@ -815,7 +807,7 @@ read_client_first_message(scram_state *state, char *input)
* it supports channel binding, which in this implementation is
* the case if a connection is using SSL.
*/
if (state->ssl_in_use)
if (state->port->ssl_in_use)
ereport(ERROR,
(errcode(ERRCODE_INVALID_AUTHORIZATION_SPECIFICATION),
errmsg("SCRAM channel binding negotiation error"),
......@@ -839,7 +831,7 @@ read_client_first_message(scram_state *state, char *input)
{
char *channel_binding_type;
if (!state->ssl_in_use)
if (!state->port->ssl_in_use)
{
/*
* Without SSL, we don't support channel binding.
......@@ -1120,8 +1112,9 @@ read_client_final_message(scram_state *state, char *input)
*/
if (strcmp(state->channel_binding_type, SCRAM_CHANNEL_BINDING_TLS_UNIQUE) == 0)
{
cbind_data = state->tls_finished_message;
cbind_data_len = state->tls_finished_len;
#ifdef USE_SSL
cbind_data = be_tls_get_peer_finished(state->port, &cbind_data_len);
#endif
}
else
{
......
......@@ -873,8 +873,6 @@ CheckSCRAMAuth(Port *port, char *shadow_pass, char **logdetail)
int inputlen;
int result;
bool initial;
char *tls_finished = NULL;
size_t tls_finished_len = 0;
/*
* SASL auth is not supported for protocol versions before 3, because it
......@@ -915,17 +913,6 @@ CheckSCRAMAuth(Port *port, char *shadow_pass, char **logdetail)
sendAuthRequest(port, AUTH_REQ_SASL, sasl_mechs, p - sasl_mechs + 1);
pfree(sasl_mechs);
#ifdef USE_SSL
/*
* Get data for channel binding.
*/
if (port->ssl_in_use)
{
tls_finished = be_tls_get_peer_finished(port, &tls_finished_len);
}
#endif
/*
* Initialize the status tracker for message exchanges.
*
......@@ -937,11 +924,7 @@ CheckSCRAMAuth(Port *port, char *shadow_pass, char **logdetail)
* This is because we don't want to reveal to an attacker what usernames
* are valid, nor which users have a valid password.
*/
scram_opaq = pg_be_scram_init(port->user_name,
shadow_pass,
port->ssl_in_use,
tls_finished,
tls_finished_len);
scram_opaq = pg_be_scram_init(port, shadow_pass);
/*
* Loop through SASL message exchange. This exchange can consist of
......
......@@ -13,15 +13,15 @@
#ifndef PG_SCRAM_H
#define PG_SCRAM_H
#include "libpq/libpq-be.h"
/* Status codes for message exchange */
#define SASL_EXCHANGE_CONTINUE 0
#define SASL_EXCHANGE_SUCCESS 1
#define SASL_EXCHANGE_FAILURE 2
/* Routines dedicated to authentication */
extern void *pg_be_scram_init(const char *username, const char *shadow_pass,
bool ssl_in_use, const char *tls_finished_message,
size_t tls_finished_len);
extern void *pg_be_scram_init(Port *port, const char *shadow_pass);
extern int pg_be_scram_exchange(void *opaq, char *input, int inputlen,
char **output, int *outputlen, char **logdetail);
......
This diff is collapsed.
......@@ -491,8 +491,6 @@ pg_SASL_init(PGconn *conn, int payloadlen)
bool success;
const char *selected_mechanism;
PQExpBufferData mechanism_buf;
char *tls_finished = NULL;
size_t tls_finished_len = 0;
char *password;
initPQExpBuffer(&mechanism_buf);
......@@ -570,32 +568,15 @@ pg_SASL_init(PGconn *conn, int payloadlen)
goto error;
}
#ifdef USE_SSL
/*
* Get data for channel binding.
*/
if (strcmp(selected_mechanism, SCRAM_SHA256_PLUS_NAME) == 0)
{
tls_finished = pgtls_get_finished(conn, &tls_finished_len);
if (tls_finished == NULL)
goto oom_error;
}
#endif
/*
* Initialize the SASL state information with all the information gathered
* during the initial exchange.
*
* Note: Only tls-unique is supported for the moment.
*/
conn->sasl_state = pg_fe_scram_init(conn->pguser,
conn->sasl_state = pg_fe_scram_init(conn,
password,
conn->ssl_in_use,
selected_mechanism,
conn->scram_channel_binding,
tls_finished,
tls_finished_len);
selected_mechanism);
if (!conn->sasl_state)
goto oom_error;
......@@ -603,7 +584,7 @@ pg_SASL_init(PGconn *conn, int payloadlen)
pg_fe_scram_exchange(conn->sasl_state,
NULL, -1,
&initialresponse, &initialresponselen,
&done, &success, &conn->errorMessage);
&done, &success);
if (done && !success)
goto error;
......@@ -684,7 +665,7 @@ pg_SASL_continue(PGconn *conn, int payloadlen, bool final)
pg_fe_scram_exchange(conn->sasl_state,
challenge, payloadlen,
&output, &outputlen,
&done, &success, &conn->errorMessage);
&done, &success);
free(challenge); /* don't need the input anymore */
if (final && !done)
......
......@@ -23,17 +23,13 @@ extern int pg_fe_sendauth(AuthRequest areq, int payloadlen, PGconn *conn);
extern char *pg_fe_getauthname(PQExpBuffer errorMessage);
/* Prototypes for functions in fe-auth-scram.c */
extern void *pg_fe_scram_init(const char *username,
extern void *pg_fe_scram_init(PGconn *conn,
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);
const char *sasl_mechanism);
extern void pg_fe_scram_free(void *opaq);
extern void pg_fe_scram_exchange(void *opaq, char *input, int inputlen,
char **output, int *outputlen,
bool *done, bool *success, PQExpBuffer errorMessage);
bool *done, bool *success);
extern char *pg_fe_scram_build_verifier(const char *password);
#endif /* FE_AUTH_H */
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment