tests: Bluetooth: reproduce gatt settings bug

Reproduce #54470.

- client connects
- client writes to CF & CCC
- client bonds
- server reboots

After this, we would expect to have the CCC and CF values be correctly
restored from settings but they are not.

Additionally add the same test with BT_PRIVACY=y, which shows that the
feature is broken with that config (fixed in a subsequent commit).

Signed-off-by: Jonathan Rico <[email protected]>

Author: jori-nordic

tests/bluetooth/bsim_bt/bsim_test_gatt_settings/prj.conf

@@ -22,3 +22,5 @@ CONFIG_BT_SETTINGS=y
 
 CONFIG_BT_GATT_AUTO_DISCOVER_CCC=y
 CONFIG_BT_GATT_AUTO_RESUBSCRIBE=n
+
+CONFIG_BT_PRIVACY=n

tests/bluetooth/bsim_bt/bsim_test_gatt_settings/prj_2.conf

@@ -0,0 +1,26 @@
+CONFIG_BT=y
+CONFIG_BT_PERIPHERAL=y
+CONFIG_BT_CENTRAL=y
+CONFIG_BT_DEVICE_NAME="GATT settings"
+
+CONFIG_LOG=y
+CONFIG_ASSERT=y
+CONFIG_BT_TESTING=y
+
+CONFIG_BT_AUTO_PHY_UPDATE=n
+CONFIG_BT_AUTO_DATA_LEN_UPDATE=n
+CONFIG_BT_GAP_AUTO_UPDATE_CONN_PARAMS=n
+
+CONFIG_BT_SMP=y
+CONFIG_BT_GATT_CLIENT=y
+CONFIG_BT_GATT_DYNAMIC_DB=y
+CONFIG_BT_GATT_CACHING=y
+
+CONFIG_SETTINGS=y
+CONFIG_SETTINGS_CUSTOM=y
+CONFIG_BT_SETTINGS=y
+
+CONFIG_BT_GATT_AUTO_DISCOVER_CCC=y
+CONFIG_BT_GATT_AUTO_RESUBSCRIBE=n
+
+CONFIG_BT_PRIVACY=y

tests/bluetooth/bsim_bt/bsim_test_gatt_settings/src/client.c

@@ -24,17 +24,22 @@ void client_round_0(void)
 
 	conn = connect_as_peripheral();
 	printk("connected: conn %p\n", conn);
-	wait_bonded();
 
 	gatt_discover();
 	activate_robust_caching();
 	/* subscribe to the SC indication, so we don't have to ATT read to
 	 * become change-aware.
 	 */
 	gatt_subscribe_to_service_changed(true);
-
 	read_test_char(true);
 
+	/* We should normally wait until we are bonded to write the CCC / CF
+	 * characteristics, but here we bond after the fact on purpose, to
+	 * simulate a client that has this exact behavior.
+	 * The CCC and CF should still persist on reboot.
+	 */
+	wait_bonded();
+
 	disconnect(conn);
 }
 

tests/bluetooth/bsim_bt/bsim_test_gatt_settings/src/gatt_utils.c

@@ -70,8 +70,8 @@ static struct bt_gatt_attr test_attrs[] = {
 
 	BT_GATT_CHARACTERISTIC(&test_chrc_uuid.uuid,
 			       BT_GATT_CHRC_READ | BT_GATT_CHRC_WRITE,
-			       BT_GATT_PERM_READ_ENCRYPT |
-			       BT_GATT_PERM_WRITE_ENCRYPT,
+			       BT_GATT_PERM_READ |
+			       BT_GATT_PERM_WRITE,
 			       read_test, write_test, test_value),
 };
 
@@ -361,3 +361,13 @@ void read_test_char(bool expect_success)
 {
 	read_char(gatt_handles[TEST_CHAR], expect_success);
 }
+
+void gatt_clear_flags(void)
+{
+	UNSET_FLAG(flag_client_read);
+	UNSET_FLAG(flag_discovered);
+	UNSET_FLAG(flag_sc_indicated);
+	UNSET_FLAG(flag_sc_subscribed);
+	UNSET_FLAG(flag_written);
+	UNSET_FLAG(flag_read);
+}

tests/bluetooth/bsim_bt/bsim_test_gatt_settings/src/gatt_utils.h

@@ -14,3 +14,4 @@ void read_test_char(bool expect_success);
 void wait_for_client_read(void);
 void gatt_subscribe_to_service_changed(bool subscribe);
 void wait_for_sc_indication(void);
+void gatt_clear_flags(void);

tests/bluetooth/bsim_bt/bsim_test_gatt_settings/src/server.c

@@ -40,11 +40,10 @@ void server_round_0(void)
 	settings_load();
 
 	conn = connect_as_central();
+	wait_for_client_read();
+
 	printk("bonding\n");
 	bond(conn);
-
-	wait_for_client_read();
-	wait_disconnected();
 }
 
 void server_round_1(void)
@@ -161,6 +160,9 @@ void server_round_6(void)
 /* What is being tested: since this deals with settings it's not the rounds
  * themselves, but rather the transitions that test expected behavior.
  *
+ * Round 0 -> 1: test CCC / CF values written before bonding are stored to NVS
+ * if the server reboots before disconnecting.
+ *
  * Round 1 -> 2: test change-awareness is updated if GATT DB changes _after_ the
  * peer has disconnected. In round 2 we also make sure we receive the Service
  * Changed indication.

tests/bluetooth/bsim_bt/bsim_test_gatt_settings/src/utils.c

@@ -5,6 +5,7 @@
  */
 
 #include "utils.h"
+#include "gatt_utils.h"
 #include <zephyr/sys/__assert.h>
 #include <zephyr/bluetooth/hci.h>
 
@@ -30,6 +31,7 @@ static void disconnected(struct bt_conn *conn, uint8_t reason)
 {
 	bt_conn_unref(conn);
 	UNSET_FLAG(flag_is_connected);
+	gatt_clear_flags();
 }
 
 static void connected(struct bt_conn *conn, uint8_t err)

tests/bluetooth/bsim_bt/bsim_test_gatt_settings/test_scripts/_compile.sh

@@ -11,3 +11,6 @@ source "${bash_source_dir}/_env.sh"
 # Place yourself in the test's root (i.e. ./../)
 west build -b nrf52_bsim -d build_test && \
     cp build_test/zephyr/zephyr.exe "${test_exe}"
+
+west build -b nrf52_bsim -d build_test_2 -- -DCONF_FILE=prj_2.conf && \
+    cp build_test_2/zephyr/zephyr.exe "${test_2_exe}"

tests/bluetooth/bsim_bt/bsim_test_gatt_settings/test_scripts/_env.sh

@@ -10,3 +10,4 @@ test_name="$(basename "$(realpath "$bash_source_dir/..")")"
 bsim_bin="${BSIM_OUT_PATH}/bin"
 BOARD="${BOARD:-nrf52_bsim}"
 test_exe="${bsim_bin}/bs_${BOARD}_tests_bluetooth_bsim_bt_${test_name}_prj_conf"
+test_2_exe="${bsim_bin}/bs_${BOARD}_tests_bluetooth_bsim_bt_${test_name}_prj_2_conf"

tests/bluetooth/bsim_bt/bsim_test_gatt_settings/test_scripts/run_gatt_settings_2.sh

@@ -0,0 +1,62 @@
+#!/usr/bin/env bash
+# Copyright 2022 Nordic Semiconductor ASA
+# SPDX-License-Identifier: Apache-2.0
+
+set -eu
+bash_source_dir="$(realpath "$(dirname "${BASH_SOURCE[0]}")")"
+
+source "${bash_source_dir}/_env.sh"
+simulation_id="${test_name}_2"
+verbosity_level=2
+process_ids=""
+exit_code=0
+
+function Execute() {
+    if [ ! -f $1 ]; then
+        echo -e "  \e[91m$(pwd)/$(basename $1) cannot be found (did you forget to\
+ compile it?)\e[39m"
+        exit 1
+    fi
+    $@ &
+    process_ids="$process_ids $!"
+}
+
+: "${BSIM_OUT_PATH:?BSIM_OUT_PATH must be defined}"
+
+cd ${BSIM_OUT_PATH}/bin
+
+# Remove the files used by the custom SETTINGS backend
+echo "remove settings files ${simulation_id}_*.log"
+rm ${simulation_id}_*.log || true
+
+Execute ./bs_2G4_phy_v1 -v=${verbosity_level} -s="${simulation_id}" -D=8 -sim_length=30e6
+
+# Only one `server` device is really running at a time. This is a necessary hack
+# because bsim doesn't support plugging devices in and out of a running
+# simulation, but this test needs a way to power-cycle the `server` device a few
+# times.
+#
+# Each device will wait until the previous instance (called 'test round') has
+# finished executing before starting up.
+Execute "$test_2_exe" -v=${verbosity_level} \
+    -s="${simulation_id}" -d=0 -testid=server -RealEncryption=1 -argstest 0 6 "server_2"
+Execute "$test_2_exe" -v=${verbosity_level} \
+    -s="${simulation_id}" -d=1 -testid=server -RealEncryption=1 -argstest 1 6 "server_2"
+Execute "$test_2_exe" -v=${verbosity_level} \
+    -s="${simulation_id}" -d=2 -testid=server -RealEncryption=1 -argstest 2 6 "server_2"
+Execute "$test_2_exe" -v=${verbosity_level} \
+    -s="${simulation_id}" -d=3 -testid=server -RealEncryption=1 -argstest 3 6 "server_2"
+Execute "$test_2_exe" -v=${verbosity_level} \
+    -s="${simulation_id}" -d=4 -testid=server -RealEncryption=1 -argstest 4 6 "server_2"
+Execute "$test_2_exe" -v=${verbosity_level} \
+    -s="${simulation_id}" -d=5 -testid=server -RealEncryption=1 -argstest 5 6 "server_2"
+Execute "$test_2_exe" -v=${verbosity_level} \
+    -s="${simulation_id}" -d=6 -testid=server -RealEncryption=1 -argstest 6 6 "server_2"
+
+Execute "$test_2_exe" -v=${verbosity_level} \
+    -s="${simulation_id}" -d=7 -testid=client -RealEncryption=1 -argstest 0 0 "client_2"
+
+for process_id in $process_ids; do
+  wait $process_id || let "exit_code=$?"
+done
+exit $exit_code #the last exit code != 0