hello_ext: minor cleanup of extension shuffling code

This reduces the number of calls to gnutls_rnd(GNUTLS_RND_RANDOM)
based on the assumption that extension indices fit in uint8_t.

This also renames the priority string modifier from %NO_EXTS_SHUFFLE
to %NO_SHUFFLE_EXTENSIONS.

Signed-off-by: Daiki Ueno <ueno@gnu.org>

7 files changed, 52 insertions, 44 deletions

diff --git a/lib/gnutls_int.h b/lib/gnutls_int.h
index a3ee5e6f20..0d0b9a8e5e 100644
--- a/lib/gnutls_int.h
+++ b/lib/gnutls_int.h
@@ -356,6 +356,11 @@ verify(GNUTLS_EXTENSION_MAX < MAX_EXT_TYPES);
  */
 verify(GNUTLS_EXTENSION_MAX_VALUE - GNUTLS_EXTENSION_MAX >= 16);
 
+/* MAX_EXT_TYPES + 1 must fit in a single byte, to generate random
+ * permutation at once.
+ */
+verify(MAX_EXT_TYPES <= UINT8_MAX);
+
 /* The 'verify' symbol from <verify.h> is used extensively in the
  * code; undef it to avoid clash
  */
@@ -949,7 +954,7 @@ struct gnutls_priority_st {
 	bool no_extensions;
 
 	/* to disable extensions shuffling */
-	bool no_exts_shuffle;
+	bool no_shuffle_extensions;
 
 	safe_renegotiation_t sr;
 	bool min_record_version;
diff --git a/lib/hello_ext.c b/lib/hello_ext.c
index a15e91876a..a2159429a0 100644
--- a/lib/hello_ext.c
+++ b/lib/hello_ext.c
@@ -408,25 +408,32 @@ int hello_ext_send(void *_ctx, gnutls_buffer_st * buf)
 	return 0;
 }
 
-static inline void swap_exts(extensions_t * exts1, extensions_t * exts2)
+static inline void swap_exts(extensions_t * exts, size_t index1, size_t index2)
 {
-	extensions_t temp = *exts1;
-	*exts1 = *exts2;
-	*exts2 = temp;
+	extensions_t temp = exts[index1];
+	exts[index1] = exts[index2];
+	exts[index2] = temp;
 }
 
 static
 int shuffle_exts(extensions_t * exts, size_t size)
 {
-	/* generating random permutation of extensions */
-	extensions_t rnd_n;
-	for (size_t i = size - 1; i > 0; i--) {
-		int ret = gnutls_rnd(GNUTLS_RND_RANDOM, (void *)&rnd_n,
-				     sizeof(extensions_t));
-		if (ret < 0)
-			return ret;
-		extensions_t j = rnd_n % (i + 1);
-		swap_exts(&exts[i], &exts[j]);
+	uint8_t permutation[MAX_EXT_TYPES];
+	size_t i;
+	int ret;
+
+	assert(size <= MAX_EXT_TYPES);
+
+	/* Generate random permutation, assuming MAX_EXT_TYPES <
+	 * UINT8_MAX.
+	 */
+	ret = gnutls_rnd(GNUTLS_RND_RANDOM, permutation, size);
+	if (ret < 0)
+		return gnutls_assert_val(ret);
+
+	for (i = size - 1; i > 0; i--) {
+		extensions_t j = permutation[i] % (i + 1);
+		swap_exts(exts, i, j);
 	}
 
 	return 0;
@@ -441,6 +448,8 @@ _gnutls_gen_hello_extensions(gnutls_session_t session,
 	int pos, ret;
 	size_t i;
 	hello_ext_ctx_st ctx;
+	/* To shuffle extension sending order */
+	extensions_t indices[MAX_EXT_TYPES];
 
 	msg &= GNUTLS_EXT_FLAG_SET_ONLY_FLAGS_MASK;
 
@@ -470,37 +479,31 @@ _gnutls_gen_hello_extensions(gnutls_session_t session,
 			     ret - 4);
 	}
 
-	/* To shuffle extension sending order */
-	extensions_t shuffled_exts[MAX_EXT_TYPES];
-
 	/* Initializing extensions array */
 	for (i = 0; i < MAX_EXT_TYPES; i++) {
-		shuffled_exts[i] = i;
+		indices[i] = i;
 	}
 
-	/* ordering dumbfw and pre_shared_key as last extensions */
-	swap_exts(&shuffled_exts[MAX_EXT_TYPES - 2],
-		  &shuffled_exts[GNUTLS_EXTENSION_DUMBFW]);
-	swap_exts(&shuffled_exts[MAX_EXT_TYPES - 1],
-		  &shuffled_exts[GNUTLS_EXTENSION_PRE_SHARED_KEY]);
-
-	if (session->internals.priorities->no_exts_shuffle == 1)
-		goto next;
+	if (!session->internals.priorities->no_shuffle_extensions) {
+		/* Ordering padding and pre_shared_key as last extensions */
+		swap_exts(indices, MAX_EXT_TYPES - 2, GNUTLS_EXTENSION_DUMBFW);
+		swap_exts(indices, MAX_EXT_TYPES - 1,
+			  GNUTLS_EXTENSION_PRE_SHARED_KEY);
 
-	ret = shuffle_exts(shuffled_exts, MAX_EXT_TYPES - 2);
-	if (ret < 0)
-		return gnutls_assert_val(ret);
+		ret = shuffle_exts(indices, MAX_EXT_TYPES - 2);
+		if (ret < 0)
+			return gnutls_assert_val(ret);
+	}
 
- next:
 	/* hello_ext_send() ensures we don't send duplicates, in case
 	 * of overridden extensions */
-	for (size_t r = 0; r < MAX_EXT_TYPES; r++) {
-		i = shuffled_exts[r];
-		if (!extfunc[i])
+	for (i = 0; i < MAX_EXT_TYPES; i++) {
+		size_t ii = indices[i];
+		if (!extfunc[ii])
 			continue;
 
-		ctx.ext = extfunc[i];
-		ret = _gnutls_extv_append(buf, extfunc[i]->tls_id,
+		ctx.ext = extfunc[ii];
+		ret = _gnutls_extv_append(buf, extfunc[ii]->tls_id,
 					  &ctx, hello_ext_send);
 		if (ret < 0)
 			return gnutls_assert_val(ret);
diff --git a/lib/priority.c b/lib/priority.c
index 966de4d83d..0455b87ee8 100644
--- a/lib/priority.c
+++ b/lib/priority.c
@@ -1081,9 +1081,9 @@ static void disable_tls13_compat_mode(gnutls_priority_t c)
 	c->tls13_compat_mode = false;
 }
 
-static void enable_no_exts_shuffle(gnutls_priority_t c)
+static void enable_no_shuffle_extensions(gnutls_priority_t c)
 {
-	c->no_exts_shuffle = 1;
+	c->no_shuffle_extensions = 1;
 }
 
 static void dummy_func(gnutls_priority_t c)
diff --git a/lib/priority_options.gperf b/lib/priority_options.gperf
index d69c3b4b11..a8f182728b 100644
--- a/lib/priority_options.gperf
+++ b/lib/priority_options.gperf
@@ -43,4 +43,4 @@ NEW_PADDING, dummy_func
 DEBUG_ALLOW_KEY_USAGE_VIOLATIONS, enable_server_key_usage_violations
 ALLOW_SMALL_RECORDS, enable_allow_small_records
 DISABLE_TLS13_COMPAT_MODE, disable_tls13_compat_mode
-NO_EXTS_SHUFFLE, enable_no_exts_shuffle
+NO_SHUFFLE_EXTENSIONS, enable_no_shuffle_extensions
diff --git a/tests/tls13-early-data.c b/tests/tls13-early-data.c
index d7ed79b3bf..0676dc2002 100644
--- a/tests/tls13-early-data.c
+++ b/tests/tls13-early-data.c
@@ -91,8 +91,8 @@ extern unsigned int _gnutls_global_version;
  * selected during the initial handshake, not the resuming handshakes.
  */
 # define SESSIONS 3
-# define TLS13_AES_128_GCM "NONE:+VERS-TLS1.3:+AES-128-GCM:+AEAD:+SIGN-RSA-PSS-RSAE-SHA384:+GROUP-SECP256R1:%NO_EXTS_SHUFFLE"
-# define TLS13_CHACHA20_POLY1305 "NONE:+VERS-TLS1.3:+CHACHA20-POLY1305:+AEAD:+SIGN-RSA-PSS-RSAE-SHA384:+GROUP-SECP256R1:%NO_EXTS_SHUFFLE"
+# define TLS13_AES_128_GCM "NONE:+VERS-TLS1.3:+AES-128-GCM:+AEAD:+SIGN-RSA-PSS-RSAE-SHA384:+GROUP-SECP256R1:%NO_SHUFFLE_EXTENSIONS"
+# define TLS13_CHACHA20_POLY1305 "NONE:+VERS-TLS1.3:+CHACHA20-POLY1305:+AEAD:+SIGN-RSA-PSS-RSAE-SHA384:+GROUP-SECP256R1:%NO_SHUFFLE_EXTENSIONS"
 
 static const
 gnutls_datum_t hrnd = { (void *)
diff --git a/tests/tls13/prf-early.c b/tests/tls13/prf-early.c
index 0df9a18abd..a55970cf3c 100644
--- a/tests/tls13/prf-early.c
+++ b/tests/tls13/prf-early.c
@@ -183,9 +183,9 @@ static void client(int sds[])
 		 */
 		gnutls_init(&session, GNUTLS_CLIENT);
 
-		/* Use default priorities, sets %NO_EXTS_SHUFFLE */
+		/* Use default priorities, sets %NO_SHUFFLE_EXTENSIONS */
 		ret = gnutls_priority_set_direct(session,
-						 "NONE:+VERS-TLS1.3:+AES-256-GCM:+AEAD:+SIGN-RSA-PSS-RSAE-SHA384:+GROUP-SECP256R1:%NO_EXTS_SHUFFLE",
+						 "NONE:+VERS-TLS1.3:+AES-256-GCM:+AEAD:+SIGN-RSA-PSS-RSAE-SHA384:+GROUP-SECP256R1:%NO_SHUFFLE_EXTENSIONS",
 						 &err);
 		if (ret < 0) {
 			fail("client: priority set failed (%s): %s\n",
diff --git a/tests/tls13/prf.c b/tests/tls13/prf.c
index 003ed69b01..877f45867e 100644
--- a/tests/tls13/prf.c
+++ b/tests/tls13/prf.c
@@ -196,9 +196,9 @@ static void client(int fd)
 	 */
 	gnutls_init(&session, GNUTLS_CLIENT);
 
-	/* Use default priorities, sets %NO_EXTS_SHUFFLE */
+	/* Use default priorities, sets %NO_SHUFFLE_EXTENSIONS */
 	ret = gnutls_priority_set_direct(session,
-					 "NONE:+VERS-TLS1.3:+AES-256-GCM:+AEAD:+SIGN-RSA-PSS-RSAE-SHA384:+GROUP-SECP256R1:%NO_EXTS_SHUFFLE",
+					 "NONE:+VERS-TLS1.3:+AES-256-GCM:+AEAD:+SIGN-RSA-PSS-RSAE-SHA384:+GROUP-SECP256R1:%NO_SHUFFLE_EXTENSIONS",
 					 &err);
 
 	if (ret < 0) {