>From 4d3a59ae1cb5742499c71b0c1e048d30dcef6836 Mon Sep 17 00:00:00 2001
From: Heikki Linnakangas <heikki.linnakangas@iki.fi>
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

