[BlueZ V2 PATCH 0/5] Replace random number generation function

[BlueZ V2 PATCH 0/5] Replace random number generation function

[Tedd Ho-Jeong An]

From: Tedd Ho-Jeong An <tedd.an@intel.com>

The Coverity scan reported (CWE-676):
  rand() should not be used for security-related applications, because
  linear congruential algorithms are too easy to break.

This series of patch replaces the standard random number generation
function, rand(), to getrandom() syscall, which provides more secure
random number than the standard rand() function.

Tedd Ho-Jeong An (5):
  emulator: Replace random number generation function
  peripheral: Replace random number generation function
  tools/btgatt-server: Replace random number generation function
  plugins: Replace random number generation function
  profiles/health: Replace random number generation function

 emulator/le.c          | 11 +++++++++--
 emulator/phy.c         | 10 ++++++++--
 peripheral/main.c      | 11 ++++++-----
 plugins/autopair.c     |  8 +++++++-
 profiles/health/hdp.c  | 11 +++++++----
 profiles/health/mcap.c | 17 +++++++++++++++--
 tools/btgatt-server.c  |  7 ++++++-
 7 files changed, 58 insertions(+), 17 deletions(-)

-- 
2.25.1

[BlueZ V2 PATCH 1/5] emulator: Replace random number generation function

[Tedd Ho-Jeong An]

From: Tedd Ho-Jeong An <tedd.an@intel.com>

This patch replaces the rand() function to the getrandom() syscall.

It was reported by the Coverity scan
  rand() should not be used for security-related applications, because
  linear congruential algorithms are too easy to break
---
 emulator/le.c  | 11 +++++++++--
 emulator/phy.c | 10 ++++++++--
 2 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/emulator/le.c b/emulator/le.c
index 07a44c5f1..f8f313f2c 100644
--- a/emulator/le.c
+++ b/emulator/le.c
@@ -20,6 +20,7 @@
 #include <sys/socket.h>
 #include <sys/un.h>
 #include <sys/uio.h>
+#include <sys/random.h>
 #include <time.h>
 
 #include "lib/bluetooth.h"
@@ -503,11 +504,17 @@ static void send_adv_pkt(struct bt_le *hci, uint8_t channel)
 
 static unsigned int get_adv_delay(void)
 {
+	unsigned int val;
+
 	/* The advertising delay is a pseudo-random value with a range
 	 * of 0 ms to 10 ms generated for each advertising event.
 	 */
-	srand(time(NULL));
-	return (rand() % 11);
+	if (getrandom(&val, sizeof(val), 0) < 0) {
+		/* If it fails to get the random number, use a static value */
+		val = 5;
+	}
+
+	return (val % 11);
 }
 
 static void adv_timeout_callback(int id, void *user_data)
diff --git a/emulator/phy.c b/emulator/phy.c
index 2ae6ad3a2..44cace438 100644
--- a/emulator/phy.c
+++ b/emulator/phy.c
@@ -19,6 +19,7 @@
 #include <stdlib.h>
 #include <string.h>
 #include <sys/socket.h>
+#include <sys/random.h>
 #include <netinet/in.h>
 #include <netinet/ip.h>
 #include <time.h>
@@ -173,8 +174,13 @@ struct bt_phy *bt_phy_new(void)
 	mainloop_add_fd(phy->rx_fd, EPOLLIN, phy_rx_callback, phy, NULL);
 
 	if (!get_random_bytes(&phy->id, sizeof(phy->id))) {
-		srandom(time(NULL));
-		phy->id = random();
+		if (getrandom(&phy->id, sizeof(phy->id), 0) < 0) {
+			mainloop_remove_fd(phy->rx_fd);
+			close(phy->tx_fd);
+			close(phy->rx_fd);
+			free(phy);
+			return NULL;
+		}
 	}
 
 	bt_phy_send(phy, BT_PHY_PKT_NULL, NULL, 0);
-- 
2.25.1

RE: Replace random number generation function

[bluez.test.bot]

[-- Attachment #1: Type: text/plain, Size: 939 bytes --]

This is automated email and please do not reply to this email!

Dear submitter,

Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=592617

---Test result---

Test Summary:
CheckPatch                    PASS      6.87 seconds
GitLint                       PASS      4.68 seconds
Prep - Setup ELL              PASS      42.33 seconds
Build - Prep                  PASS      0.59 seconds
Build - Configure             PASS      8.11 seconds
Build - Make                  PASS      179.48 seconds
Make Check                    PASS      8.97 seconds
Make Distcheck                PASS      204.63 seconds
Build w/ext ELL - Configure   PASS      7.72 seconds
Build w/ext ELL - Make        PASS      169.93 seconds
Incremental Build with patchesPASS      955.83 seconds



---
Regards,
Linux Bluetooth

[BlueZ V2 PATCH 2/5] peripheral: Replace random number generation function

[Tedd Ho-Jeong An]

From: Tedd Ho-Jeong An <tedd.an@intel.com>

This patch replaces the rand() function to the getrandom() syscall.

It was reported by the Coverity scan
  rand() should not be used for security-related applications, because
  linear congruential algorithms are too easy to break
---
 peripheral/main.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/peripheral/main.c b/peripheral/main.c
index 86b52236e..0f5210403 100644
--- a/peripheral/main.c
+++ b/peripheral/main.c
@@ -25,6 +25,7 @@
 #include <sys/stat.h>
 #include <sys/types.h>
 #include <sys/mount.h>
+#include <sys/random.h>
 
 #ifndef WAIT_ANY
 #define WAIT_ANY (-1)
@@ -191,11 +192,11 @@ int main(int argc, char *argv[])
 							addr, 6) < 0) {
 			printf("Generating new persistent static address\n");
 
-			addr[0] = rand();
-			addr[1] = rand();
-			addr[2] = rand();
-			addr[3] = 0x34;
-			addr[4] = 0x12;
+			if (getrandom(addr, sizeof(addr), 0) < 0) {
+				perror("Failed to get random static address");
+				return EXIT_FAILURE;
+			}
+			/* Overwrite the MSB to make it a static address */
 			addr[5] = 0xc0;
 
 			efivars_write("BluetoothStaticAddress",
-- 
2.25.1

[BlueZ V2 PATCH 3/5] tools/btgatt-server: Replace random number generation function

[Tedd Ho-Jeong An]

From: Tedd Ho-Jeong An <tedd.an@intel.com>

This patch replaces the rand() function to the getrandom() syscall.

It was reported by the Coverity scan
  rand() should not be used for security-related applications, because
  linear congruential algorithms are too easy to break
---
 tools/btgatt-server.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/tools/btgatt-server.c b/tools/btgatt-server.c
index 000145a3d..15d49a464 100644
--- a/tools/btgatt-server.c
+++ b/tools/btgatt-server.c
@@ -20,6 +20,7 @@
 #include <getopt.h>
 #include <unistd.h>
 #include <errno.h>
+#include <sys/random.h>
 
 #include "lib/bluetooth.h"
 #include "lib/hci.h"
@@ -284,9 +285,13 @@ static bool hr_msrmt_cb(void *user_data)
 	uint16_t len = 2;
 	uint8_t pdu[4];
 	uint32_t cur_ee;
+	uint32_t val;
+
+	if (getrandom(&val, sizeof(val), 0) < 0)
+		return false;
 
 	pdu[0] = 0x06;
-	pdu[1] = 90 + (rand() % 40);
+	pdu[1] = 90 + (val % 40);
 
 	if (expended_present) {
 		pdu[0] |= 0x08;
-- 
2.25.1

[BlueZ V2 PATCH 4/5] plugins: Replace random number generation function

[Tedd Ho-Jeong An]

From: Tedd Ho-Jeong An <tedd.an@intel.com>

This patch replaces the rand() function to the getrandom() syscall.

It was reported by the Coverity scan
  rand() should not be used for security-related applications, because
  linear congruential algorithms are too easy to break
---
 plugins/autopair.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/plugins/autopair.c b/plugins/autopair.c
index 665a4f4a6..a75ecebe4 100644
--- a/plugins/autopair.c
+++ b/plugins/autopair.c
@@ -17,6 +17,7 @@
 #include <fcntl.h>
 #include <unistd.h>
 #include <errno.h>
+#include <sys/random.h>
 
 #include <glib.h>
 
@@ -49,6 +50,7 @@ static ssize_t autopair_pincb(struct btd_adapter *adapter,
 	char pinstr[7];
 	char name[25];
 	uint32_t class;
+	uint32_t val;
 
 	ba2str(device_get_address(device), addr);
 
@@ -129,8 +131,12 @@ static ssize_t autopair_pincb(struct btd_adapter *adapter,
 			if (attempt >= 4)
 				return 0;
 
+			if (getrandom(&val, sizeof(val), 0) < 0) {
+				error("Failed to get a random pincode");
+				return 0;
+			}
 			snprintf(pinstr, sizeof(pinstr), "%06u",
-						rand() % 1000000);
+						val % 1000000);
 			*display = true;
 			memcpy(pinbuf, pinstr, 6);
 			return 6;
-- 
2.25.1

[BlueZ V2 PATCH 5/5] profiles/health: Replace random number generation function

[Tedd Ho-Jeong An]

From: Tedd Ho-Jeong An <tedd.an@intel.com>

This patch replaces the rand() function to the getrandom() syscall.

It was reported by the Coverity scan
  rand() should not be used for security-related applications, because
  linear congruential algorithms are too easy to break
---
 profiles/health/hdp.c  | 11 +++++++----
 profiles/health/mcap.c | 17 +++++++++++++++--
 2 files changed, 22 insertions(+), 6 deletions(-)

diff --git a/profiles/health/hdp.c b/profiles/health/hdp.c
index 6bc41946f..40b6cc18a 100644
--- a/profiles/health/hdp.c
+++ b/profiles/health/hdp.c
@@ -16,6 +16,7 @@
 #include <stdint.h>
 #include <stdbool.h>
 #include <unistd.h>
+#include <sys/random.h>
 
 #include <glib.h>
 
@@ -1484,13 +1485,15 @@ static void destroy_create_dc_data(gpointer data)
 static void *generate_echo_packet(void)
 {
 	uint8_t *buf;
-	int i;
 
 	buf = g_malloc(HDP_ECHO_LEN);
-	srand(time(NULL));
+	if (!buf)
+		return NULL;
 
-	for(i = 0; i < HDP_ECHO_LEN; i++)
-		buf[i] = rand() % UINT8_MAX;
+	if (getrandom(buf, HDP_ECHO_LEN, 0) < 0) {
+		g_free(buf);
+		return NULL;
+	}
 
 	return buf;
 }
diff --git a/profiles/health/mcap.c b/profiles/health/mcap.c
index 5161ef77c..aad0a08a3 100644
--- a/profiles/health/mcap.c
+++ b/profiles/health/mcap.c
@@ -19,6 +19,7 @@
 #include <errno.h>
 #include <unistd.h>
 #include <time.h>
+#include <sys/random.h>
 
 #include <glib.h>
 
@@ -1888,6 +1889,7 @@ gboolean mcap_create_mcl(struct mcap_instance *mi,
 {
 	struct mcap_mcl *mcl;
 	struct connect_mcl *con;
+	uint16_t val;
 
 	mcl = find_mcl(mi->mcls, addr);
 	if (mcl) {
@@ -1903,7 +1905,12 @@ gboolean mcap_create_mcl(struct mcap_instance *mi,
 		mcl->state = MCL_IDLE;
 		bacpy(&mcl->addr, addr);
 		set_default_cb(mcl);
-		mcl->next_mdl = (rand() % MCAP_MDLID_FINAL) + 1;
+		if (getrandom(&val, sizeof(val), 0) < 0) {
+			mcap_instance_unref(mcl->mi);
+			g_free(mcl);
+			return FALSE;
+		}
+		mcl->next_mdl = (val % MCAP_MDLID_FINAL) + 1;
 	}
 
 	mcl->ctrl |= MCAP_CTRL_CONN;
@@ -2013,6 +2020,7 @@ static void connect_mcl_event_cb(GIOChannel *chan, GError *gerr,
 	bdaddr_t dst;
 	char address[18], srcstr[18];
 	GError *err = NULL;
+	uint16_t val;
 
 	if (gerr)
 		return;
@@ -2041,7 +2049,12 @@ static void connect_mcl_event_cb(GIOChannel *chan, GError *gerr,
 		mcl->mi = mcap_instance_ref(mi);
 		bacpy(&mcl->addr, &dst);
 		set_default_cb(mcl);
-		mcl->next_mdl = (rand() % MCAP_MDLID_FINAL) + 1;
+		if (getrandom(&val, sizeof(val), 0) < 0) {
+			mcap_instance_unref(mcl->mi);
+			g_free(mcl);
+			goto drop;
+		}
+		mcl->next_mdl = (val % MCAP_MDLID_FINAL) + 1;
 	}
 
 	set_mcl_conf(chan, mcl);
-- 
2.25.1

Re: [BlueZ V2 PATCH 0/5] Replace random number generation function

[Luiz Augusto von Dentz]

Hi Tedd,

On Wed, Dec 8, 2021 at 5:29 PM Tedd Ho-Jeong An <hj.tedd.an@gmail.com> wrote:
>
> From: Tedd Ho-Jeong An <tedd.an@intel.com>
>
> The Coverity scan reported (CWE-676):
>   rand() should not be used for security-related applications, because
>   linear congruential algorithms are too easy to break.
>
> This series of patch replaces the standard random number generation
> function, rand(), to getrandom() syscall, which provides more secure
> random number than the standard rand() function.
>
> Tedd Ho-Jeong An (5):
>   emulator: Replace random number generation function
>   peripheral: Replace random number generation function
>   tools/btgatt-server: Replace random number generation function
>   plugins: Replace random number generation function
>   profiles/health: Replace random number generation function
>
>  emulator/le.c          | 11 +++++++++--
>  emulator/phy.c         | 10 ++++++++--
>  peripheral/main.c      | 11 ++++++-----
>  plugins/autopair.c     |  8 +++++++-
>  profiles/health/hdp.c  | 11 +++++++----
>  profiles/health/mcap.c | 17 +++++++++++++++--
>  tools/btgatt-server.c  |  7 ++++++-
>  7 files changed, 58 insertions(+), 17 deletions(-)
>
> --
> 2.25.1

Applied, thanks.

-- 
Luiz Augusto von Dentz

[BlueZ PATCH 1/5] emulator: Replace random number generation function

[Tedd Ho-Jeong An]

From: Tedd Ho-Jeong An <tedd.an@intel.com>

This patch replaces the rand() function to the l_getrandom() from ELL,
which uses the getrandom() system call.

It was reported by the Coverity scan
  rand() should not be used for security-related applications, because
  linear congruential algorithms are too easy to break
---
 Makefile.tools | 3 ++-
 emulator/le.c  | 4 ++--
 emulator/phy.c | 6 ++++--
 3 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/Makefile.tools b/Makefile.tools
index c7bdff83f..8312d4d27 100644
--- a/Makefile.tools
+++ b/Makefile.tools
@@ -93,7 +93,8 @@ emulator_btvirt_SOURCES = emulator/main.c monitor/bt.h \
 				emulator/phy.h emulator/phy.c \
 				emulator/amp.h emulator/amp.c \
 				emulator/le.h emulator/le.c
-emulator_btvirt_LDADD = lib/libbluetooth-internal.la src/libshared-mainloop.la
+emulator_btvirt_LDADD = lib/libbluetooth-internal.la src/libshared-mainloop.la \
+				src/libshared-ell.la $(ell_ldadd)
 
 emulator_b1ee_SOURCES = emulator/b1ee.c
 emulator_b1ee_LDADD = src/libshared-mainloop.la
diff --git a/emulator/le.c b/emulator/le.c
index 07a44c5f1..fed3a7815 100644
--- a/emulator/le.c
+++ b/emulator/le.c
@@ -21,6 +21,7 @@
 #include <sys/un.h>
 #include <sys/uio.h>
 #include <time.h>
+#include <ell/ell.h>
 
 #include "lib/bluetooth.h"
 #include "lib/hci.h"
@@ -506,8 +507,7 @@ static unsigned int get_adv_delay(void)
 	/* The advertising delay is a pseudo-random value with a range
 	 * of 0 ms to 10 ms generated for each advertising event.
 	 */
-	srand(time(NULL));
-	return (rand() % 11);
+	return (l_getrandom_uint32() % 11);
 }
 
 static void adv_timeout_callback(int id, void *user_data)
diff --git a/emulator/phy.c b/emulator/phy.c
index 2ae6ad3a2..570a9c975 100644
--- a/emulator/phy.c
+++ b/emulator/phy.c
@@ -22,6 +22,7 @@
 #include <netinet/in.h>
 #include <netinet/ip.h>
 #include <time.h>
+#include <ell/ell.h>
 
 #include "src/shared/util.h"
 #include "src/shared/mainloop.h"
@@ -152,6 +153,7 @@ static int create_tx_socket(void)
 struct bt_phy *bt_phy_new(void)
 {
 	struct bt_phy *phy;
+	uint64_t phy_id;
 
 	phy = calloc(1, sizeof(*phy));
 	if (!phy)
@@ -173,8 +175,8 @@ struct bt_phy *bt_phy_new(void)
 	mainloop_add_fd(phy->rx_fd, EPOLLIN, phy_rx_callback, phy, NULL);
 
 	if (!get_random_bytes(&phy->id, sizeof(phy->id))) {
-		srandom(time(NULL));
-		phy->id = random();
+		l_getrandom(&phy_id, sizeof(phy_id));
+		phy->id = phy_id;
 	}
 
 	bt_phy_send(phy, BT_PHY_PKT_NULL, NULL, 0);
-- 
2.25.1

RE: Replace random number generation function

[bluez.test.bot]

[-- Attachment #1: Type: text/plain, Size: 884 bytes --]

This is automated email and please do not reply to this email!

Dear submitter,

Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=591967

---Test result---

Test Summary:
CheckPatch                    PASS      6.41 seconds
GitLint                       PASS      4.35 seconds
Prep - Setup ELL              PASS      43.18 seconds
Build - Prep                  PASS      0.47 seconds
Build - Configure             PASS      8.22 seconds
Build - Make                  PASS      186.52 seconds
Make Check                    PASS      9.53 seconds
Make Distcheck                PASS      222.57 seconds
Build w/ext ELL - Configure   PASS      8.31 seconds
Build w/ext ELL - Make        PASS      176.44 seconds



---
Regards,
Linux Bluetooth