Merge branch 'random-name'

Author: benma

src/memory/memory.c

@@ -250,16 +250,55 @@ bool memory_set_device_name(const char* name)
     _read_chunk(CHUNK_1, chunk_bytes);
     util_zero(chunk.fields.device_name, sizeof(chunk.fields.device_name));
     snprintf((char*)&chunk.fields.device_name, MEMORY_DEVICE_NAME_MAX_LEN, "%s", name);
+
+    if (!rust_util_is_name_valid(chunk.fields.device_name, MEMORY_DEVICE_NAME_MAX_LEN)) {
+        return false;
+    }
     return _write_chunk(CHUNK_1, chunk.bytes);
 }
 
+static void _random_name(char* name_out)
+{
+    static char cached_name[MEMORY_DEVICE_NAME_MAX_LEN] = {0};
+
+    if (cached_name[0] == 0x00) {
+        // Generate 4 random uppercase letters
+        uint8_t random[32] = {0};
+        _interface_functions->random_32_bytes(random);
+        uint8_t letters[4];
+        for (size_t i = 0; i < sizeof(letters); i++) {
+            letters[i] = 'A' + (random[i] % 26);
+        }
+
+        // Format into cached name
+        snprintf(
+            cached_name,
+            MEMORY_DEVICE_NAME_MAX_LEN,
+            "BitBox %c%c%c%c",
+            letters[0],
+            letters[1],
+            letters[2],
+            letters[3]);
+    }
+
+    // Copy cached result to output
+    snprintf(name_out, MEMORY_DEVICE_NAME_MAX_LEN, "%s", cached_name);
+}
+
 void memory_get_device_name(char* name_out)
 {
     chunk_1_t chunk = {0};
     CLEANUP_CHUNK(chunk);
     _read_chunk(CHUNK_1, chunk_bytes);
-    if (chunk.fields.device_name[0] == 0xFF) {
-        snprintf(name_out, MEMORY_DEVICE_NAME_MAX_LEN, "%s", MEMORY_DEFAULT_DEVICE_NAME);
+    if (chunk.fields.device_name[0] == 0xFF ||
+        !rust_util_is_name_valid(chunk.fields.device_name, MEMORY_DEVICE_NAME_MAX_LEN)) {
+        if (memory_get_platform() == MEMORY_PLATFORM_BITBOX02_PLUS) {
+            // For Bluetooth, we want to use an unambiguous default name so this BitBox can be
+            // identified if multiple BitBoxes are advertising at the same time.
+            _random_name(name_out);
+        } else {
+            snprintf(name_out, MEMORY_DEVICE_NAME_MAX_LEN, "%s", MEMORY_DEFAULT_DEVICE_NAME);
+        }
     } else {
         snprintf(name_out, MEMORY_DEVICE_NAME_MAX_LEN, "%s", chunk.fields.device_name);
     }

src/memory/memory.h

@@ -66,12 +66,17 @@ extern const char* MEMORY_DEFAULT_DEVICE_NAME;
 // memory.c)
 #define MEMORY_DEVICE_NAME_MAX_LEN (64)
 
-// set device name. name is an utf8-encoded string, and null terminated. The max
-// size (including the null terminator) is MEMORY_DEVICE_NAME_MAX_LEN bytes.
+// set device name. name is null terminated. The name must be smaller or equal to
+// MEMORY_DEVICE_NAME_MAX_LEN (including the null terminator) and larger than 0 in size, consist of
+// printable ASCII characters only (and space), not start or end with whitespace, and contain no
+// whitespace other than space.
 USE_RESULT bool memory_set_device_name(const char* name);
 
-// name_out must have MEMORY_DEVICE_NAME_MAX_LEN bytes in size. Returns `MEMORY_DEFAULT_DEVICE_NAME`
-// if no device name is set.
+// name_out must have MEMORY_DEVICE_NAME_MAX_LEN bytes in size. If no device name is set, or if it
+// is invalid, we return:
+// - `MEMORY_DEFAULT_DEVICE_NAME` for non-bluetooth enabled BitBoxes
+// - "BitBox ABCD" for Bluetooth-enabled BitBoxes, where ABCD are four random uppercase letters.
+//    The name is cached in RAM, so the same random name is returned until reboot.
 void memory_get_device_name(char* name_out);
 
 /**

src/rust/bitbox02-rust-c/src/util.rs

@@ -25,6 +25,24 @@ pub extern "C" fn rust_util_zero(mut dst: BytesMut) {
     util::zero(dst.as_mut())
 }
 
+/// Calls `util::name::validate()` on the provided C string.
+/// SAFETY:
+/// `buf` must point to a valid buffer of size `max_len`.
+#[no_mangle]
+pub unsafe extern "C" fn rust_util_is_name_valid(buf: *const u8, max_len: usize) -> bool {
+    if max_len == 0 {
+        return false;
+    }
+    let slice = core::slice::from_raw_parts(buf, max_len);
+    match core::ffi::CStr::from_bytes_until_nul(slice) {
+        Ok(cstr) => match cstr.to_str() {
+            Ok(s) => util::name::validate(s, max_len - 1),
+            Err(_) => false,
+        },
+        Err(_) => false,
+    }
+}
+
 /// Convert bytes to hex representation
 ///
 /// * `buf` - bytes to convert to hex.
@@ -202,4 +220,22 @@ mod tests {
         let expected = b"LUC1eAJa5jW\0";
         assert_eq!(&result_buf[..expected.len()], expected);
     }
+
+    #[test]
+    // For clarity we want explicit null terminators in the strings below, not CStr literals
+    // `c"..."`.
+    #[allow(clippy::manual_c_str_literals)]
+    fn test_rust_util_is_name_valid() {
+        unsafe {
+            // Valid
+            assert!(rust_util_is_name_valid("foo\0".as_ptr(), 4));
+            assert!(rust_util_is_name_valid("foo\0........".as_ptr(), 12));
+
+            // Invalid
+            assert!(!rust_util_is_name_valid("fo\no\0".as_ptr(), 5));
+            assert!(!rust_util_is_name_valid("".as_ptr(), 0));
+            assert!(!rust_util_is_name_valid("foo\0".as_ptr(), 3));
+            assert!(!rust_util_is_name_valid("foo".as_ptr(), 3));
+        }
+    }
 }

test/unit-test/test_memory.c

@@ -18,15 +18,13 @@
 #include <cmocka.h>
 
 #include <memory/memory.h>
+#include <memory/memory_shared.h>
 
 #include <stdint.h>
 #include <stdio.h>
 #include <string.h>
 
-#define CHUNK_SIZE (16 * 512) // 8kB.
-
 #define FLASH_APP_DATA_LEN (0x000010000)
-#define FLASH_SHARED_DATA_START (0xe000)
 
 // chunk 0
 static const int _addr_factory_setup_done = 0;
@@ -386,18 +384,70 @@ static void _test_memory_get_device_name_default(void** state)
     EMPTYCHUNK(empty_chunk);
     expect_value(__wrap_memory_read_chunk_mock, chunk_num, 1);
     will_return(__wrap_memory_read_chunk_mock, empty_chunk);
+
+    EMPTYCHUNK(empty_shared_chunk);
+    will_return(__wrap_memory_read_shared_bootdata_mock, empty_shared_chunk);
+
     memory_get_device_name(name_out);
     assert_string_equal(MEMORY_DEFAULT_DEVICE_NAME, name_out);
 }
 
-static void _test_memory_get_device_name(void** state)
+// For Bluetooth devices (BitBox02+), the default name is "BitBox ABCD" where ABCD are four random
+// uppercase letters.
+static void _test_memory_get_device_name_default_bluetooth(void** state)
+{
+    uint8_t entropy[32] = {0};
+    memcpy(entropy, "\x00\x19\xFE\xFF", 4);
+
+    char name_out[MEMORY_DEVICE_NAME_MAX_LEN] = {0};
+    EMPTYCHUNK(empty_chunk);
+    expect_value(__wrap_memory_read_chunk_mock, chunk_num, 1);
+    will_return(__wrap_memory_read_chunk_mock, empty_chunk);
+
+    EMPTYCHUNK(empty_shared_chunk);
+    chunk_shared_t shared_chunk = {0};
+    memcpy(shared_chunk.bytes, empty_shared_chunk, CHUNK_SIZE);
+    shared_chunk.fields.platform = MEMORY_PLATFORM_BITBOX02_PLUS;
+    will_return(__wrap_memory_read_shared_bootdata_mock, shared_chunk.bytes);
+
+    will_return(_mock_random_32_bytes, entropy);
+
+    memory_get_device_name(name_out);
+    assert_string_equal("BitBox AZUV", name_out);
+
+    // Calling it again does not re-generate a new random name but reuses the already generated one.
+    expect_value(__wrap_memory_read_chunk_mock, chunk_num, 1);
+    will_return(__wrap_memory_read_chunk_mock, empty_chunk);
+    will_return(__wrap_memory_read_shared_bootdata_mock, shared_chunk.bytes);
+    memory_get_device_name(name_out);
+    assert_string_equal("BitBox AZUV", name_out);
+}
+
+static void _test_memory_get_device_name_invalid(void** state)
 {
     char name_out[MEMORY_DEVICE_NAME_MAX_LEN] = {0};
     EMPTYCHUNK(chunk);
     memset(chunk + _addr_device_name, 0, MEMORY_DEVICE_NAME_MAX_LEN);
     const char* device_name = "Äxxxxxxxxxxxxxx xxxxxxxxxxxxxxxxxxxxxx 漢字xxxxxxxxxxxxxxxxx";
     snprintf((char*)chunk + _addr_device_name, MEMORY_DEVICE_NAME_MAX_LEN, "%s", device_name);
 
+    EMPTYCHUNK(empty_shared_chunk);
+    will_return(__wrap_memory_read_shared_bootdata_mock, empty_shared_chunk);
+
+    expect_value(__wrap_memory_read_chunk_mock, chunk_num, 1);
+    will_return(__wrap_memory_read_chunk_mock, chunk);
+    memory_get_device_name(name_out);
+    assert_string_equal(MEMORY_DEFAULT_DEVICE_NAME, name_out);
+}
+
+static void _test_memory_get_device_name(void** state)
+{
+    char name_out[MEMORY_DEVICE_NAME_MAX_LEN] = {0};
+    EMPTYCHUNK(chunk);
+    memset(chunk + _addr_device_name, 0, MEMORY_DEVICE_NAME_MAX_LEN);
+    const char* device_name = "foo bar";
+    snprintf((char*)chunk + _addr_device_name, MEMORY_DEVICE_NAME_MAX_LEN, "%s", device_name);
+
     expect_value(__wrap_memory_read_chunk_mock, chunk_num, 1);
     will_return(__wrap_memory_read_chunk_mock, chunk);
     memory_get_device_name(name_out);
@@ -527,6 +577,8 @@ int main(void)
         cmocka_unit_test(_test_memory_set_mnemonic_passphrase_enabled),
         cmocka_unit_test(_test_memory_reset_hww),
         cmocka_unit_test(_test_memory_get_device_name_default),
+        cmocka_unit_test(_test_memory_get_device_name_default_bluetooth),
+        cmocka_unit_test(_test_memory_get_device_name_invalid),
         cmocka_unit_test(_test_memory_get_device_name),
         cmocka_unit_test(_test_memory_device_name),
         cmocka_unit_test(_test_memory_set_seed_birthdate),