mirrors/postgres
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Minor cleanup of backend SCRAM code.

Browse Source 
Free each SASL message after sending it. It's not a lot of wasted memory,
and it's short-lived, but the authentication code in general tries to
pfree() stuff, so let's follow the example.

Adding the pfree() revealed a little bug in build_server_first_message().
It attempts to keeps a copy of the sent message, but it was missing a
pstrdup(), so the pointer started to dangle, after adding the pfree()
into CheckSCRAMAuth().

Reword comments and debug messages slightly, while we're at it.

Reviewed by Michael Paquier.

Discussion: https://www.postgresql.org/message-id/6490b975-5ee1-6280-ac1d-af975b19fb9a@iki.fi
This commit is contained in:
Heikki Linnakangas 2017-04-13 17:44:15 +03:00
parent 3d5facfd9a
commit 00707fa582
2 changed files with 12 additions and 10 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
src/backend/libpq/auth-scram.c
View File

@ -161,10 +161,10 @@ static char *scram_MockSalt(const char *username);
* needs to be called before doing any exchange. It will be filled later * needs to be called before doing any exchange. It will be filled later
* after the beginning of the exchange with verifier data. * after the beginning of the exchange with verifier data.
* *
* 'username' is the provided by the client. 'shadow_pass' is the role's * 'username' is the username provided by the client in the startup message.
* password verifier, from pg_authid.rolpassword. If 'shadow_pass' is NULL, we * 'shadow_pass' is the role's password verifier, from pg_authid.rolpassword.
* still perform an authentication exchange, but it will fail, as if an * If 'shadow_pass' is NULL, we still perform an authentication exchange, but
* incorrect password was given. * it will fail, as if an incorrect password was given.
*/ */
void * void *
pg_be_scram_init(const char *username, const char *shadow_pass) pg_be_scram_init(const char *username, const char *shadow_pass)
@ -984,7 +984,7 @@ build_server_first_message(scram_state *state)
state->client_nonce, state->server_nonce, state->client_nonce, state->server_nonce,
state->salt, state->iterations); state->salt, state->iterations);
return state->server_first_message; return pstrdup(state->server_first_message);
} }

12
src/backend/libpq/auth.c
View File

@ -872,6 +872,8 @@ CheckSCRAMAuth(Port *port, char *shadow_pass, char **logdetail)
strlen(SCRAM_SHA256_NAME) + 1); strlen(SCRAM_SHA256_NAME) + 1);
/* /*
* Initialize the status tracker for message exchanges.
*
* If the user doesn't exist, or doesn't have a valid password, or it's * If the user doesn't exist, or doesn't have a valid password, or it's
* expired, we still go through the motions of SASL authentication, but * expired, we still go through the motions of SASL authentication, but
* tell the authentication method that the authentication is "doomed". * tell the authentication method that the authentication is "doomed".
@ -880,8 +882,6 @@ CheckSCRAMAuth(Port *port, char *shadow_pass, char **logdetail)
* This is because we don't want to reveal to an attacker what usernames * This is because we don't want to reveal to an attacker what usernames
* are valid, nor which users have a valid password. * are valid, nor which users have a valid password.
*/ */
/* Initialize the status tracker for message exchanges */
scram_opaq = pg_be_scram_init(port->user_name, shadow_pass); scram_opaq = pg_be_scram_init(port->user_name, shadow_pass);
/* /*
@ -918,7 +918,7 @@ CheckSCRAMAuth(Port *port, char *shadow_pass, char **logdetail)
return STATUS_ERROR; return STATUS_ERROR;
} }
elog(DEBUG4, "Processing received SASL token of length %d", buf.len); elog(DEBUG4, "Processing received SASL response of length %d", buf.len);
/* /*
* we pass 'logdetail' as NULL when doing a mock authentication, * we pass 'logdetail' as NULL when doing a mock authentication,
@ -931,14 +931,16 @@ CheckSCRAMAuth(Port *port, char *shadow_pass, char **logdetail)
/* input buffer no longer used */ /* input buffer no longer used */
pfree(buf.data); pfree(buf.data);
if (outputlen > 0) if (output)
{ {
/* /*
* Negotiation generated data to be sent to the client. * Negotiation generated data to be sent to the client.
*/ */
elog(DEBUG4, "sending SASL response token of length %u", outputlen); elog(DEBUG4, "sending SASL challenge of length %u", outputlen);
sendAuthRequest(port, AUTH_REQ_SASL_CONT, output, outputlen); sendAuthRequest(port, AUTH_REQ_SASL_CONT, output, outputlen);
pfree(output);
} }
} while (result == SASL_EXCHANGE_CONTINUE); } while (result == SASL_EXCHANGE_CONTINUE);