From: Heikki Linnakangas Date: Wed, 7 Dec 2016 15:24:55 +0200 Subject: [PATCH 08/11] Rewrite nonce generation. In the server, the nonce was generated using only ASCII-printable characters, and the result was base64-encoded. The base64 encoding is pointless, if we use only ASCII-printable chars to begin with. Calling pg_strong_random() can be somewhat expensive, as with the /dev/urandom implementation, it has to open the device, read the bytes, and close, on every call. So avoid calling it in a loop, generating only one byte in each call. I went back to using base64-encoding method of turning the raw bytes into the final nonce. That was more convenient than writing something that encodes to the whole ASCII-printable range. That means that we're not using the whole range of chars allowed in the nonce, but I believe that doesn't make any difference. (Both the frontend and backend will still accept the full range from the other side of the connection). --- src/backend/libpq/auth-scram.c | 52 ++++++++----------------------- src/include/common/scram-common.h | 6 +++- src/include/libpq/libpq-be.h | 2 -- src/interfaces/libpq/fe-auth-scram.c | 60 ++++++++++-------------------------- 4 files changed, 34 insertions(+), 86 deletions(-) diff --git a/src/backend/libpq/auth-scram.c b/src/backend/libpq/auth-scram.c index 55c5efa..cda663b 100644 --- a/src/backend/libpq/auth-scram.c +++ b/src/backend/libpq/auth-scram.c @@ -72,7 +72,7 @@ typedef struct /* Server-side status fields */ char *server_first_message; - char *server_nonce; /* base64-encoded */ + char *server_nonce; char *server_signature; } scram_state; @@ -115,7 +115,6 @@ static bool verify_client_proof(scram_state *state); static bool verify_final_nonce(scram_state *state); static bool parse_scram_verifier(const char *verifier, char **salt, int *iterations, char **stored_key, char **server_key); -static void generate_nonce(char *out, int len); /* * build_error_message @@ -237,29 +236,6 @@ check_client_data(void *opaque, char **logdetail) char *passwd; TimestampTz vuntil = 0; bool vuntil_null; - int count = 0; - - /* compute the salt to use for computing responses */ - while (count < sizeof(MyProcPort->SASLSalt)) - { - char byte; - - if (!pg_backend_random(&byte, 1)) - { - *logdetail = psprintf(_("Could not generate random salt")); - return SASL_OTHER_ERROR; - } - - /* - * Only ASCII printable characters, except commas are accepted in - * the nonce. - */ - if (byte < '!' || byte > '~' || byte == ',') - continue; - - MyProcPort->SASLSalt[count] = byte; - count++; - } /* * Fetch details about role needed for password checks. @@ -450,7 +426,7 @@ scram_build_verifier(const char *username, const char *password, if (iterations <= 0) iterations = SCRAM_ITERATIONS_DEFAULT; - generate_nonce(salt, SCRAM_SALT_LEN); + pg_backend_random(salt, SCRAM_SALT_LEN); encoded_salt = palloc(pg_b64_enc_len(SCRAM_SALT_LEN) + 1); encoded_len = pg_b64_encode(salt, SCRAM_SALT_LEN, encoded_salt); @@ -806,9 +782,6 @@ verify_client_proof(scram_state *state) static char * build_server_first_message(scram_state *state) { - char nonce[SCRAM_NONCE_LEN]; - int encoded_len; - /* * server-first-message = * [reserved-mext ","] nonce "," salt "," @@ -830,10 +803,19 @@ build_server_first_message(scram_state *state) * * r=fyko+d2lbbFgONRv9qkxdawL3rfcNHYJY1ZVvWVs7j,s=QSXCR+Q6sek8bf92,i=4096 */ - generate_nonce(nonce, SCRAM_NONCE_LEN); + + /* + * Per the spec, the nonce may consist of any printable ASCII characters. + * For convenience, however, we don't use the whole range available, rather, + * we generate some random bytes, and base64 encode them. + */ + char raw_nonce[SCRAM_NONCE_LEN]; + int encoded_len; + + pg_backend_random(raw_nonce, SCRAM_NONCE_LEN); state->server_nonce = palloc(pg_b64_enc_len(SCRAM_NONCE_LEN) + 1); - encoded_len = pg_b64_encode(nonce, SCRAM_NONCE_LEN, state->server_nonce); + encoded_len = pg_b64_encode(raw_nonce, SCRAM_NONCE_LEN, state->server_nonce); if (encoded_len < 0) return NULL; @@ -964,11 +946,3 @@ build_server_final_message(scram_state *state) */ return psprintf("v=%s", server_signature_base64); } - -static void -generate_nonce(char *result, int len) -{ - /* Use the salt generated for SASL authentication */ - memset(result, 0, len); - memcpy(result, MyProcPort->SASLSalt, Min(sizeof(MyProcPort->SASLSalt), len)); -} diff --git a/src/include/common/scram-common.h b/src/include/common/scram-common.h index e9028fb..34c6527 100644 --- a/src/include/common/scram-common.h +++ b/src/include/common/scram-common.h @@ -21,7 +21,11 @@ /* length of HMAC */ #define SHA256_HMAC_B PG_SHA256_BLOCK_LENGTH -/* length of random nonce generated in the authentication exchange */ +/* + * Size of random nonce generated in the authentication exchange. This is + * in "raw" number of bytes, the actual nonces sent over the wire are + * encoded using only ASCII-printable characters. + */ #define SCRAM_NONCE_LEN 10 /* length of salt when generating new verifiers */ diff --git a/src/include/libpq/libpq-be.h b/src/include/libpq/libpq-be.h index 299aaca..66647ad 100644 --- a/src/include/libpq/libpq-be.h +++ b/src/include/libpq/libpq-be.h @@ -144,8 +144,6 @@ typedef struct Port * Information that needs to be held during the authentication cycle. */ HbaLine *hba; - char SASLSalt[10]; /* SASL password salt, size of - * SCRAM_SALT_LEN */ /* * Information that really has no business at all being in struct Port, diff --git a/src/interfaces/libpq/fe-auth-scram.c b/src/interfaces/libpq/fe-auth-scram.c index a1dc7f1..12884e5 100644 --- a/src/interfaces/libpq/fe-auth-scram.c +++ b/src/interfaces/libpq/fe-auth-scram.c @@ -61,7 +61,6 @@ static char *build_client_first_message(fe_scram_state *state, static char *build_client_final_message(fe_scram_state *state, PQExpBuffer errormessage); static bool verify_server_proof(fe_scram_state *state); -static bool generate_nonce(char *buf, int len); static void calculate_client_proof(fe_scram_state *state, const char *client_final_message_without_proof, uint8 *result); @@ -257,25 +256,35 @@ read_attr_value(char **input, char attr, PQExpBuffer errorMessage) static char * build_client_first_message(fe_scram_state *state, PQExpBuffer errormessage) { - char nonce[SCRAM_NONCE_LEN + 1]; + char raw_nonce[SCRAM_NONCE_LEN + 1]; char *buf; - char msglen; + char buflen; + int n; + int encoded_len; - if (!generate_nonce(nonce, SCRAM_NONCE_LEN)) + /* XXX: Check max length of username? */ + + /* + * Generate a "raw" nonce. This is converted to ASCII-printable form by + * base64-encoding it. + */ + if (!pg_strong_random(raw_nonce, SCRAM_NONCE_LEN)) { printfPQExpBuffer(errormessage, libpq_gettext("failed to generate nonce\n")); return NULL; } /* Generate message */ - msglen = 5 + strlen(state->username) + 3 + strlen(nonce); - buf = malloc(msglen + 1); + buflen = 5 + strlen(state->username) + 3 + pg_b64_enc_len(SCRAM_NONCE_LEN) + 1; + buf = malloc(buflen); if (buf == NULL) { printfPQExpBuffer(errormessage, libpq_gettext("out of memory\n")); return NULL; } - snprintf(buf, msglen + 1, "n,,n=%s,r=%s", state->username, nonce); + n = snprintf(buf, buflen, "n,,n=%s,r=", state->username); + encoded_len = pg_b64_encode(raw_nonce, SCRAM_NONCE_LEN, buf + n); + buf[n + encoded_len] = '\0'; state->client_first_message_bare = strdup(buf + 3); if (!state->client_first_message_bare) @@ -527,40 +536,3 @@ verify_server_proof(fe_scram_state *state) return true; } - -/* - * Generate nonce with some randomness. - * Returns true of nonce has been succesfully generated, and false - * otherwise. - */ -static bool -generate_nonce(char *buf, int len) -{ - int count = 0; - - /* compute the salt to use for computing responses */ - while (count < len) - { - char byte; - -#ifdef HAVE_STRONG_RANDOM - if (!pg_strong_random(&byte, 1)) - return false; -#else - byte = random() % 256; -#endif - - /* - * Only ASCII printable characters, except commas are accepted in - * the nonce. - */ - if (byte < '!' || byte > '~' || byte == ',') - continue; - - buf[count] = byte; - count++; - } - - buf[len] = '\0'; - return true; -} -- 2.10.2 --------------0DD988856548A7EC1968AEA8 Content-Type: text/x-patch; name="0009-Random-number-fixes.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="0009-Random-number-fixes.patch"