generate 20-byte lowercase hexadecimal session identifiers

Signed-off-by: Hans Zandbelt <[email protected]>

ChangeLog

@@ -1,3 +1,6 @@
+12/22/2023
+- generate 20-byte lowercase hexadecimal session identifiers
+
 12/20/2023
 - generate or propagate traceparent header using OIDCTraceParent; closes #1152; thanks @studersi
 - include hostname,port and process id in User-Agent header on outgoing requests

src/metrics.c

@@ -595,14 +595,14 @@ static inline apr_interval_time_t oidc_metrics_interval(server_rec *s) {
 
 unsigned int oidc_metric_random_int(unsigned int mod) {
 	unsigned int v;
-	apr_generate_random_bytes((unsigned char *)&v, sizeof(v));
+	oidc_util_random_bytes((unsigned char *)&v, sizeof(v));
 	return v % mod;
 }
 
 /*
  * thread that periodically writes the local data into the shared memory
  */
-static void * APR_THREAD_FUNC oidc_metrics_thread_run(apr_thread_t *thread, void *data) {
+static void *APR_THREAD_FUNC oidc_metrics_thread_run(apr_thread_t *thread, void *data) {
 	server_rec *s = (server_rec *)data;
 
 	/* sleep for a short random time <1s so child processes write-lock on a different frequency */

src/mod_auth_openidc.h

@@ -898,6 +898,9 @@ void oidc_config_check_x_forwarded(request_rec *r, const apr_byte_t x_forwarded_
 int oidc_jq_filter_cache_ttl(request_rec *r);
 
 // oidc_util.c
+apr_byte_t oidc_util_random_bytes(unsigned char *buf, apr_size_t length);
+apr_byte_t oidc_util_generate_random_bytes(request_rec *r, unsigned char *buf, apr_size_t length);
+apr_byte_t oidc_proto_generate_random_hex_string(request_rec *r, char **hex_str, int byte_len);
 int oidc_strnenvcmp(const char *a, const char *b, int len);
 int oidc_base64url_encode(request_rec *r, char **dst, const char *src, int src_len, int remove_padding);
 int oidc_base64url_decode(apr_pool_t *pool, char **dst, const char *src);

src/proto.c

@@ -60,63 +60,13 @@
 
 extern module AP_MODULE_DECLARE_DATA auth_openidc_module;
 
-static apr_status_t oidc_proto_generate_random_bytes(request_rec *r, unsigned char *buf, apr_size_t length) {
-	apr_status_t rv = APR_SUCCESS;
-
-#ifndef USE_URANDOM
-
-	oidc_debug(r, "apr_generate_random_bytes call for %" APR_SIZE_T_FMT " bytes", length);
-	rv = apr_generate_random_bytes(buf, length);
-	oidc_debug(r, "apr_generate_random_bytes returned");
-
-#else
-
-	int fd = -1;
-
-	do {
-		apr_ssize_t rc;
-
-		if (fd == -1) {
-			fd = open(DEV_RANDOM, O_RDONLY);
-			if (fd == -1)
-				return errno;
-		}
-
-		do {
-			oidc_debug(r, "read %s for %" APR_SIZE_T_FMT " bytes", DEV_RANDOM, length);
-			rc = read(fd, buf, length);
-			oidc_debug(r, "read %s returned: %" APR_SIZE_T_FMT, DEV_RANDOM " bytes", rc);
-		} while (rc == -1 && errno == EINTR);
-
-		if (rc < 0) {
-			int errnum = errno;
-			close(fd);
-			return errnum;
-		} else if (rc == 0) {
-			close(fd);
-			fd = -1; /* force open() again */
-		} else {
-			buf += rc;
-			length -= rc;
-		}
-	} while (length > 0);
-
-	close(fd);
-
-	rv = APR_SUCCESS;
-
-#endif
-
-	return rv;
-}
-
 /*
  * generate a random string value value of a specified length
  */
 apr_byte_t oidc_proto_generate_random_string(request_rec *r, char **output, int len) {
 	unsigned char *bytes = apr_pcalloc(r->pool, len);
-	if (oidc_proto_generate_random_bytes(r, bytes, len) != APR_SUCCESS) {
-		oidc_error(r, "oidc_proto_generate_random_bytes returned an error");
+	if (oidc_util_generate_random_bytes(r, bytes, len) != TRUE) {
+		oidc_error(r, "oidc_util_generate_random_bytes returned an error");
 		return FALSE;
 	}
 	if (oidc_base64url_encode(r, output, (const char *)bytes, len, TRUE) <= 0) {

src/session.c

@@ -97,8 +97,7 @@ static apr_byte_t oidc_session_decode(request_rec *r, oidc_cfg *c, oidc_session_
  * generate a unique identifier for a session
  */
 void oidc_session_id_new(request_rec *r, oidc_session_t *z) {
-	oidc_proto_generate_random_string(r, &z->uuid, 20);
-	// for (char *p = z->uuid ; (p && *p); ++p) *p = tolower(*p);
+	oidc_proto_generate_random_hex_string(r, &z->uuid, 20);
 }
 
 /*

src/util.c

@@ -56,6 +56,81 @@
 /* hrm, should we get rid of this by adding parameters to the (3) functions? */
 extern module AP_MODULE_DECLARE_DATA auth_openidc_module;
 
+apr_byte_t oidc_util_random_bytes(unsigned char *buf, apr_size_t length) {
+	apr_byte_t rv = TRUE;
+
+#ifndef USE_URANDOM
+
+	rv = (apr_generate_random_bytes(buf, length) == APR_SUCCESS);
+
+#else
+
+	int fd = -1;
+
+	do {
+		apr_ssize_t rc;
+
+		if (fd == -1) {
+			fd = open(DEV_RANDOM, O_RDONLY);
+			if (fd == -1)
+				return errno;
+		}
+
+		do {
+			rc = read(fd, buf, length);
+		} while (rc == -1 && errno == EINTR);
+
+		if (rc < 0) {
+			int errnum = errno;
+			close(fd);
+			return errnum;
+		} else if (rc == 0) {
+			close(fd);
+			fd = -1; /* force open() again */
+		} else {
+			buf += rc;
+			length -= rc;
+		}
+	} while (length > 0);
+
+	close(fd);
+
+	rv = TRUE;
+
+#endif
+
+	return rv;
+}
+
+apr_byte_t oidc_util_generate_random_bytes(request_rec *r, unsigned char *buf, apr_size_t length) {
+	apr_byte_t rv = TRUE;
+	const char *gen = NULL;
+#ifndef USE_URANDOM
+	gen = "apr";
+#else
+	gen = DEV_RANDOM;
+#endif
+	oidc_debug(r, "oidc_util_random_bytes [%s] call for %" APR_SIZE_T_FMT " bytes", gen, length);
+	rv = oidc_util_random_bytes(buf, length);
+	oidc_debug(r, "oidc_util_random_bytes returned: %d", rv);
+
+	return rv;
+}
+
+apr_byte_t oidc_proto_generate_random_hex_string(request_rec *r, char **hex_str, int byte_len) {
+	unsigned char *bytes = apr_pcalloc(r->pool, byte_len);
+	int i = 0;
+	if (oidc_util_generate_random_bytes(r, bytes, byte_len) != TRUE) {
+		oidc_error(r, "oidc_util_generate_random_bytes returned an error");
+		return FALSE;
+	}
+	*hex_str = "";
+	for (i = 0; i < byte_len; i++)
+		*hex_str = apr_psprintf(r->pool, "%s%02x", *hex_str, bytes[i]);
+
+	return TRUE;
+}
+
 /*
  * base64url encode a string
  */