Skip to content

Commit d981074

Browse files
hlinnakahlinnaka
committed
Misc SCRAM code cleanups.
* Move computation of SaltedPassword to a separate function from scram_ClientOrServerKey(). This saves a lot of cycles in libpq, by computing SaltedPassword only once per authentication. (Computing SaltedPassword is expensive by design.) * Split scram_ClientOrServerKey() into two functions. Improves readability, by making the calling code less verbose. * Rename "server proof" to "server signature", to better match the nomenclature used in RFC 5802. * Rename SCRAM_SALT_LEN to SCRAM_DEFAULT_SALT_LEN, to make it more clear that the salt can be of any length, and the constant only specifies how long a salt we use when we generate a new verifier. Also rename SCRAM_ITERATIONS_DEFAULT to SCRAM_DEFAULT_ITERATIONS, for consistency. These things caught my eye while working on other upcoming changes.
1 parent b9a3ef5 commit d981074

File tree

4 files changed

+89
-71
lines changed

4 files changed

+89
-71
lines changed

‎src/backend/libpq/auth-scram.c

+19-17
Original file line numberDiff line numberDiff line change
@@ -396,7 +396,8 @@ scram_build_verifier(const char *username, const char *password,
396396
{
397397
char *prep_password = NULL;
398398
pg_saslprep_rc rc;
399-
char saltbuf[SCRAM_SALT_LEN];
399+
char saltbuf[SCRAM_DEFAULT_SALT_LEN];
400+
uint8 salted_password[SCRAM_KEY_LEN];
400401
uint8 keybuf[SCRAM_KEY_LEN];
401402
char *encoded_salt;
402403
char *encoded_storedkey;
@@ -414,24 +415,25 @@ scram_build_verifier(const char *username, const char *password,
414415
password = (const char *) prep_password;
415416

416417
if (iterations <= 0)
417-
iterations = SCRAM_ITERATIONS_DEFAULT;
418+
iterations = SCRAM_DEFAULT_ITERATIONS;
418419

419420
/* Generate salt, and encode it in base64 */
420-
if (!pg_backend_random(saltbuf, SCRAM_SALT_LEN))
421+
if (!pg_backend_random(saltbuf, SCRAM_DEFAULT_SALT_LEN))
421422
{
422423
ereport(LOG,
423424
(errcode(ERRCODE_INTERNAL_ERROR),
424425
errmsg("could not generate random salt")));
425426
return NULL;
426427
}
427428

428-
encoded_salt = palloc(pg_b64_enc_len(SCRAM_SALT_LEN) + 1);
429-
encoded_len = pg_b64_encode(saltbuf, SCRAM_SALT_LEN, encoded_salt);
429+
encoded_salt = palloc(pg_b64_enc_len(SCRAM_DEFAULT_SALT_LEN) + 1);
430+
encoded_len = pg_b64_encode(saltbuf, SCRAM_DEFAULT_SALT_LEN, encoded_salt);
430431
encoded_salt[encoded_len] = '\0';
431432

432433
/* Calculate StoredKey, and encode it in base64 */
433-
scram_ClientOrServerKey(password, saltbuf, SCRAM_SALT_LEN,
434-
iterations, SCRAM_CLIENT_KEY_NAME, keybuf);
434+
scram_SaltedPassword(password, saltbuf, SCRAM_DEFAULT_SALT_LEN,
435+
iterations, salted_password);
436+
scram_ClientKey(salted_password, keybuf);
435437
scram_H(keybuf, SCRAM_KEY_LEN, keybuf); /* StoredKey */
436438

437439
encoded_storedkey = palloc(pg_b64_enc_len(SCRAM_KEY_LEN) + 1);
@@ -440,8 +442,7 @@ scram_build_verifier(const char *username, const char *password,
440442
encoded_storedkey[encoded_len] = '\0';
441443

442444
/* And same for ServerKey */
443-
scram_ClientOrServerKey(password, saltbuf, SCRAM_SALT_LEN, iterations,
444-
SCRAM_SERVER_KEY_NAME, keybuf);
445+
scram_ServerKey(salted_password, keybuf);
445446

446447
encoded_serverkey = palloc(pg_b64_enc_len(SCRAM_KEY_LEN) + 1);
447448
encoded_len = pg_b64_encode((const char *) keybuf, SCRAM_KEY_LEN,
@@ -473,6 +474,7 @@ scram_verify_plain_password(const char *username, const char *password,
473474
char *salt;
474475
int saltlen;
475476
int iterations;
477+
uint8 salted_password[SCRAM_KEY_LEN];
476478
uint8 stored_key[SCRAM_KEY_LEN];
477479
uint8 server_key[SCRAM_KEY_LEN];
478480
uint8 computed_key[SCRAM_KEY_LEN];
@@ -502,9 +504,9 @@ scram_verify_plain_password(const char *username, const char *password,
502504
if (rc == SASLPREP_SUCCESS)
503505
password = prep_password;
504506

505-
/* Compute Server key based on the user-supplied plaintext password */
506-
scram_ClientOrServerKey(password, salt, saltlen, iterations,
507-
SCRAM_SERVER_KEY_NAME, computed_key);
507+
/* Compute Server Key based on the user-supplied plaintext password */
508+
scram_SaltedPassword(password, salt, saltlen, iterations, salted_password);
509+
scram_ServerKey(salted_password, computed_key);
508510

509511
if (prep_password)
510512
pfree(prep_password);
@@ -630,12 +632,12 @@ mock_scram_verifier(const char *username, int *iterations, char **salt,
630632
/* Generate deterministic salt */
631633
raw_salt = scram_MockSalt(username);
632634

633-
encoded_salt = (char *) palloc(pg_b64_enc_len(SCRAM_SALT_LEN) + 1);
634-
encoded_len = pg_b64_encode(raw_salt, SCRAM_SALT_LEN, encoded_salt);
635+
encoded_salt = (char *) palloc(pg_b64_enc_len(SCRAM_DEFAULT_SALT_LEN) + 1);
636+
encoded_len = pg_b64_encode(raw_salt, SCRAM_DEFAULT_SALT_LEN, encoded_salt);
635637
encoded_salt[encoded_len] = '\0';
636638

637639
*salt = encoded_salt;
638-
*iterations = SCRAM_ITERATIONS_DEFAULT;
640+
*iterations = SCRAM_DEFAULT_ITERATIONS;
639641

640642
/* StoredKey and ServerKey are not used in a doomed authentication */
641643
memset(stored_key, 0, SCRAM_KEY_LEN);
@@ -1179,7 +1181,7 @@ build_server_final_message(scram_state *state)
11791181
/*
11801182
* Determinisitcally generate salt for mock authentication, using a SHA256
11811183
* hash based on the username and a cluster-level secret key. Returns a
1182-
* pointer to a static buffer of size SCRAM_SALT_LEN.
1184+
* pointer to a static buffer of size SCRAM_DEFAULT_SALT_LEN.
11831185
*/
11841186
static char *
11851187
scram_MockSalt(const char *username)
@@ -1194,7 +1196,7 @@ scram_MockSalt(const char *username)
11941196
* not larger the SHA256 digest length. If the salt is smaller, the caller
11951197
* will just ignore the extra data))
11961198
*/
1197-
StaticAssertStmt(PG_SHA256_DIGEST_LENGTH >= SCRAM_SALT_LEN,
1199+
StaticAssertStmt(PG_SHA256_DIGEST_LENGTH >= SCRAM_DEFAULT_SALT_LEN,
11981200
"salt length greater than SHA256 digest length");
11991201

12001202
pg_sha256_init(&ctx);

‎src/common/scram-common.c

+33-18
Original file line numberDiff line numberDiff line change
@@ -98,23 +98,31 @@ scram_HMAC_final(uint8 *result, scram_HMAC_ctx *ctx)
9898
}
9999

100100
/*
101-
* Iterate hash calculation of HMAC entry using given salt.
102-
* scram_Hi() is essentially PBKDF2 (see RFC2898) with HMAC() as the
103-
* pseudorandom function.
101+
* Calculate SaltedPassword.
102+
*
103+
* The password should already be normalized by SASLprep.
104104
*/
105-
static void
106-
scram_Hi(const char *str, const char *salt, int saltlen, int iterations, uint8 *result)
105+
void
106+
scram_SaltedPassword(const char *password,
107+
const char *salt, int saltlen, int iterations,
108+
uint8 *result)
107109
{
108-
int str_len = strlen(str);
110+
int password_len = strlen(password);
109111
uint32 one = htonl(1);
110112
int i,
111113
j;
112114
uint8 Ui[SCRAM_KEY_LEN];
113115
uint8 Ui_prev[SCRAM_KEY_LEN];
114116
scram_HMAC_ctx hmac_ctx;
115117

118+
/*
119+
* Iterate hash calculation of HMAC entry using given salt. This is
120+
* essentially PBKDF2 (see RFC2898) with HMAC() as the pseudorandom
121+
* function.
122+
*/
123+
116124
/* First iteration */
117-
scram_HMAC_init(&hmac_ctx, (uint8 *) str, str_len);
125+
scram_HMAC_init(&hmac_ctx, (uint8 *) password, password_len);
118126
scram_HMAC_update(&hmac_ctx, salt, saltlen);
119127
scram_HMAC_update(&hmac_ctx, (char *) &one, sizeof(uint32));
120128
scram_HMAC_final(Ui_prev, &hmac_ctx);
@@ -123,7 +131,7 @@ scram_Hi(const char *str, const char *salt, int saltlen, int iterations, uint8 *
123131
/* Subsequent iterations */
124132
for (i = 2; i <= iterations; i++)
125133
{
126-
scram_HMAC_init(&hmac_ctx, (uint8 *) str, str_len);
134+
scram_HMAC_init(&hmac_ctx, (uint8 *) password, password_len);
127135
scram_HMAC_update(&hmac_ctx, (const char *) Ui_prev, SCRAM_KEY_LEN);
128136
scram_HMAC_final(Ui, &hmac_ctx);
129137
for (j = 0; j < SCRAM_KEY_LEN; j++)
@@ -148,20 +156,27 @@ scram_H(const uint8 *input, int len, uint8 *result)
148156
}
149157

150158
/*
151-
* Calculate ClientKey or ServerKey.
152-
*
153-
* The password should already be normalized by SASLprep.
159+
* Calculate ClientKey.
154160
*/
155161
void
156-
scram_ClientOrServerKey(const char *password,
157-
const char *salt, int saltlen, int iterations,
158-
const char *keystr, uint8 *result)
162+
scram_ClientKey(const uint8 *salted_password, uint8 *result)
163+
{
164+
scram_HMAC_ctx ctx;
165+
166+
scram_HMAC_init(&ctx, salted_password, SCRAM_KEY_LEN);
167+
scram_HMAC_update(&ctx, "Client Key", strlen("Client Key"));
168+
scram_HMAC_final(result, &ctx);
169+
}
170+
171+
/*
172+
* Calculate ServerKey.
173+
*/
174+
void
175+
scram_ServerKey(const uint8 *salted_password, uint8 *result)
159176
{
160-
uint8 keybuf[SCRAM_KEY_LEN];
161177
scram_HMAC_ctx ctx;
162178

163-
scram_Hi(password, salt, saltlen, iterations, keybuf);
164-
scram_HMAC_init(&ctx, keybuf, SCRAM_KEY_LEN);
165-
scram_HMAC_update(&ctx, keystr, strlen(keystr));
179+
scram_HMAC_init(&ctx, salted_password, SCRAM_KEY_LEN);
180+
scram_HMAC_update(&ctx, "Server Key", strlen("Server Key"));
166181
scram_HMAC_final(result, &ctx);
167182
}

‎src/include/common/scram-common.h

+6-9
Original file line numberDiff line numberDiff line change
@@ -29,14 +29,10 @@
2929
#define SCRAM_RAW_NONCE_LEN 10
3030

3131
/* length of salt when generating new verifiers */
32-
#define SCRAM_SALT_LEN 10
32+
#define SCRAM_DEFAULT_SALT_LEN 10
3333

3434
/* default number of iterations when generating verifier */
35-
#define SCRAM_ITERATIONS_DEFAULT 4096
36-
37-
/* Base name of keys used for proof generation */
38-
#define SCRAM_SERVER_KEY_NAME "Server Key"
39-
#define SCRAM_CLIENT_KEY_NAME "Client Key"
35+
#define SCRAM_DEFAULT_ITERATIONS 4096
4036

4137
/*
4238
* Context data for HMAC used in SCRAM authentication.
@@ -51,9 +47,10 @@ extern void scram_HMAC_init(scram_HMAC_ctx *ctx, const uint8 *key, int keylen);
5147
extern void scram_HMAC_update(scram_HMAC_ctx *ctx, const char *str, int slen);
5248
extern void scram_HMAC_final(uint8 *result, scram_HMAC_ctx *ctx);
5349

50+
extern void scram_SaltedPassword(const char *password, const char *salt,
51+
int saltlen, int iterations, uint8 *result);
5452
extern void scram_H(const uint8 *str, int len, uint8 *result);
55-
extern void scram_ClientOrServerKey(const char *password, const char *salt,
56-
int saltlen, int iterations,
57-
const char *keystr, uint8 *result);
53+
extern void scram_ClientKey(const uint8 *salted_password, uint8 *result);
54+
extern void scram_ServerKey(const uint8 *salted_password, uint8 *result);
5855

5956
#endif /* SCRAM_COMMON_H */

‎src/interfaces/libpq/fe-auth-scram.c

+31-27
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ typedef struct
4646
char *password;
4747

4848
/* We construct these */
49+
uint8 SaltedPassword[SCRAM_KEY_LEN];
4950
char *client_nonce;
5051
char *client_first_message_bare;
5152
char *client_final_message_without_proof;
@@ -59,7 +60,7 @@ typedef struct
5960

6061
/* These come from the server-final message */
6162
char *server_final_message;
62-
char ServerProof[SCRAM_KEY_LEN];
63+
char ServerSignature[SCRAM_KEY_LEN];
6364
} fe_scram_state;
6465

6566
static bool read_server_first_message(fe_scram_state *state, char *input,
@@ -70,7 +71,7 @@ static char *build_client_first_message(fe_scram_state *state,
7071
PQExpBuffer errormessage);
7172
static char *build_client_final_message(fe_scram_state *state,
7273
PQExpBuffer errormessage);
73-
static bool verify_server_proof(fe_scram_state *state);
74+
static bool verify_server_signature(fe_scram_state *state);
7475
static void calculate_client_proof(fe_scram_state *state,
7576
const char *client_final_message_without_proof,
7677
uint8 *result);
@@ -216,12 +217,12 @@ pg_fe_scram_exchange(void *opaq, char *input, int inputlen,
216217
goto error;
217218

218219
/*
219-
* Verify server proof, to make sure we're talking to the genuine
220-
* server. XXX: A fake server could simply not require
220+
* Verify server signature, to make sure we're talking to the
221+
* genuine server. XXX: A fake server could simply not require
221222
* authentication, though. There is currently no option in libpq
222223
* to reject a connection, if SCRAM authentication did not happen.
223224
*/
224-
if (verify_server_proof(state))
225+
if (verify_server_signature(state))
225226
*success = true;
226227
else
227228
{
@@ -486,12 +487,11 @@ read_server_first_message(fe_scram_state *state, char *input,
486487
* Read the final exchange message coming from the server.
487488
*/
488489
static bool
489-
read_server_final_message(fe_scram_state *state,
490-
char *input,
490+
read_server_final_message(fe_scram_state *state, char *input,
491491
PQExpBuffer errormessage)
492492
{
493-
char *encoded_server_proof;
494-
int server_proof_len;
493+
char *encoded_server_signature;
494+
int server_signature_len;
495495

496496
state->server_final_message = strdup(input);
497497
if (!state->server_final_message)
@@ -513,8 +513,8 @@ read_server_final_message(fe_scram_state *state,
513513
}
514514

515515
/* Parse the message. */
516-
encoded_server_proof = read_attr_value(&input, 'v', errormessage);
517-
if (encoded_server_proof == NULL)
516+
encoded_server_signature = read_attr_value(&input, 'v', errormessage);
517+
if (encoded_server_signature == NULL)
518518
{
519519
/* read_attr_value() has generated an error message */
520520
return false;
@@ -524,13 +524,13 @@ read_server_final_message(fe_scram_state *state,
524524
printfPQExpBuffer(errormessage,
525525
libpq_gettext("malformed SCRAM message (garbage at end of server-final-message)\n"));
526526

527-
server_proof_len = pg_b64_decode(encoded_server_proof,
528-
strlen(encoded_server_proof),
529-
state->ServerProof);
530-
if (server_proof_len != SCRAM_KEY_LEN)
527+
server_signature_len = pg_b64_decode(encoded_server_signature,
528+
strlen(encoded_server_signature),
529+
state->ServerSignature);
530+
if (server_signature_len != SCRAM_KEY_LEN)
531531
{
532532
printfPQExpBuffer(errormessage,
533-
libpq_gettext("malformed SCRAM message (invalid server proof)\n"));
533+
libpq_gettext("malformed SCRAM message (invalid server signature)\n"));
534534
return false;
535535
}
536536

@@ -552,8 +552,14 @@ calculate_client_proof(fe_scram_state *state,
552552
int i;
553553
scram_HMAC_ctx ctx;
554554

555-
scram_ClientOrServerKey(state->password, state->salt, state->saltlen,
556-
state->iterations, SCRAM_CLIENT_KEY_NAME, ClientKey);
555+
/*
556+
* Calculate SaltedPassword, and store it in 'state' so that we can reuse
557+
* it later in verify_server_signature.
558+
*/
559+
scram_SaltedPassword(state->password, state->salt, state->saltlen,
560+
state->iterations, state->SaltedPassword);
561+
562+
scram_ClientKey(state->SaltedPassword, ClientKey);
557563
scram_H(ClientKey, SCRAM_KEY_LEN, StoredKey);
558564

559565
scram_HMAC_init(&ctx, StoredKey, SCRAM_KEY_LEN);
@@ -575,19 +581,17 @@ calculate_client_proof(fe_scram_state *state,
575581
}
576582

577583
/*
578-
* Validate the server proof, received as part of the final exchange message
579-
* received from the server.
584+
* Validate the server signature, received as part of the final exchange
585+
* message received from the server.
580586
*/
581587
static bool
582-
verify_server_proof(fe_scram_state *state)
588+
verify_server_signature(fe_scram_state *state)
583589
{
584-
uint8 ServerSignature[SCRAM_KEY_LEN];
590+
uint8 expected_ServerSignature[SCRAM_KEY_LEN];
585591
uint8 ServerKey[SCRAM_KEY_LEN];
586592
scram_HMAC_ctx ctx;
587593

588-
scram_ClientOrServerKey(state->password, state->salt, state->saltlen,
589-
state->iterations, SCRAM_SERVER_KEY_NAME,
590-
ServerKey);
594+
scram_ServerKey(state->SaltedPassword, ServerKey);
591595

592596
/* calculate ServerSignature */
593597
scram_HMAC_init(&ctx, ServerKey, SCRAM_KEY_LEN);
@@ -602,9 +606,9 @@ verify_server_proof(fe_scram_state *state)
602606
scram_HMAC_update(&ctx,
603607
state->client_final_message_without_proof,
604608
strlen(state->client_final_message_without_proof));
605-
scram_HMAC_final(ServerSignature, &ctx);
609+
scram_HMAC_final(expected_ServerSignature, &ctx);
606610

607-
if (memcmp(ServerSignature, state->ServerProof, SCRAM_KEY_LEN) != 0)
611+
if (memcmp(expected_ServerSignature, state->ServerSignature, SCRAM_KEY_LEN) != 0)
608612
return false;
609613

610614
return true;

0 commit comments

Comments
 (0)