Merge pull request #53 from LedgerHQ/develop

v1.6.4

Makefile

@@ -33,7 +33,7 @@ PATH_APP_LOAD_PARAMS += "5262163'"  # int("PKS".encode("ascii").hex(), 16)
 
 APPVERSION_M=1
 APPVERSION_N=6
-APPVERSION_P=3
+APPVERSION_P=4
 APPVERSION=$(APPVERSION_M).$(APPVERSION_N).$(APPVERSION_P)
 
 ICON_NANOS=icons/icon_security_key_nanos.gif
@@ -125,16 +125,23 @@ ENABLE_NOCRC_APP_LOAD_PARAMS = 1
 # then reinstall flow), and this reset was causing even more issues
 DEFINES += HAVE_NO_RESET_GENERATION_INCREMENT
 
-# Adds the Resident Key support and exposes a setting to enable/disable it.
-# Currently the settings default value is `False`, which means that by default
-# users will need to activate the setting before being able to create "Resident
-# Keys", also named "Discoverable Credentials".
-# This has been implemented to protect user from the NVRAM wipe mostly happening
-# during an app update which will erase their RK credentials with no possibility
-# to restore them.
-# Advanced users can still choose to completely disable this setting.
-# /!\ disabled for 1.6 stable release
-# DEFINES += HAVE_RK_SUPPORT_SETTING
+# These 2 flags allow to enable/disable the RK feature and expose an app setting for it.
+# This has been implemented to protect user from the NVRAM wipe mostly happening during
+# an app update which will erase their RK credentials with no possibility  to restore them.
+#
+# ENABLE_RK_CONFIG activates the internal code which allows to activate and deactivate
+# the feature. It sets the feature as deactivated by default.
+# ENABLE_RK_CONFIG_UI_SETTING activates the UI settings which allows a user to enable or
+# disable the feature.
+#
+# So the expected behaviors are the following:
+# - No flags -> RK are enabled by default and a user can not deactivate the feature,
+# - ENABLE_RK_CONFIG only -> RK are disabled by default and a user can not activate the feature,
+# - ENABLE_RK_CONFIG_UI_SETTING only -> compilation fails at link,
+# - ENABLE_RK_CONFIG & ENABLE_RK_CONFIG_UI_SETTING -> RK are disabled by default but a user can
+#   enable or disable the feature through the app's settings.
+DEFINES += ENABLE_RK_CONFIG
+# DEFINES += ENABLE_RK_CONFIG_UI_SETTING
 
 DEFINES += HAVE_FIDO2_RPID_FILTER
 

include/config.h

@@ -34,7 +34,7 @@ typedef struct config_t {
     uint8_t pin[16];
     uint8_t pinSet;
     uint8_t pinRetries;
-#ifdef HAVE_RK_SUPPORT_SETTING
+#ifdef ENABLE_RK_CONFIG
     uint8_t rk_enabled;
 #endif
 } config_t;
@@ -52,7 +52,7 @@ void config_set_ctap2_pin(uint8_t *pin);
 void config_decrease_ctap2_pin_retry_counter(void);
 void config_reset_ctap2_pin_retry_counter(void);
 
-#ifdef HAVE_RK_SUPPORT_SETTING
+#ifdef ENABLE_RK_CONFIG
 void config_set_rk_enabled(bool enabled);
 bool config_get_rk_enabled(void);
 #endif

src/config.c

@@ -92,7 +92,7 @@ int config_init(void) {
         tmp8 = 0;
         nvm_write((void *) &N_u2f.pinSet, (void *) &tmp8, sizeof(uint8_t));
 
-#ifdef HAVE_RK_SUPPORT_SETTING
+#ifdef ENABLE_RK_CONFIG
         // Initialize rk_enable value: Disabled by default
         tmp8 = 0;
         nvm_write((void *) &N_u2f.rk_enabled, (void *) &tmp8, sizeof(uint8_t));
@@ -161,7 +161,7 @@ void config_reset_ctap2_pin_retry_counter(void) {
     nvm_write((void *) &N_u2f.pinRetries, (void *) &tmp, sizeof(uint8_t));
 }
 
-#ifdef HAVE_RK_SUPPORT_SETTING
+#ifdef ENABLE_RK_CONFIG
 void config_set_rk_enabled(bool enabled) {
     uint8_t tmp8 = enabled ? 1 : 0;
     nvm_write((void *) &N_u2f.rk_enabled, (void *) &tmp8, sizeof(uint8_t));

src/ctap2_get_info.c

@@ -73,7 +73,7 @@ void ctap2_get_info_handle(u2f_service_t *service, uint8_t *buffer, uint16_t len
     cbip_add_int(&encoder, TAG_OPTIONS);
     cbip_add_map_header(&encoder, 4);
     cbip_add_string(&encoder, OPTION_RESIDENT_KEY, sizeof(OPTION_RESIDENT_KEY) - 1);
-#ifdef HAVE_RK_SUPPORT_SETTING
+#ifdef ENABLE_RK_CONFIG
     cbip_add_boolean(&encoder, config_get_rk_enabled());
 #else
     cbip_add_boolean(&encoder, true);

src/ctap2_make_credential.c

@@ -349,7 +349,7 @@ static int process_makeCred_authnr_options(cbipDecoder_t *decoder, cbipItem_t *m
 
         status = cbiph_get_map_key_str_bool(decoder, &optionsItem, OPTION_RESIDENT_KEY, &boolValue);
         if (status == CBIPH_STATUS_FOUND) {
-#ifdef HAVE_RK_SUPPORT_SETTING
+#ifdef ENABLE_RK_CONFIG
             if (boolValue && !config_get_rk_enabled()) {
                 PRINTF("RK disabled\n");
                 return ERROR_UNSUPPORTED_OPTION;

src/ui_shared.c

@@ -28,7 +28,7 @@ static void app_quit(void) {
 #include "ui_shared.h"
 
 #if defined(HAVE_BAGL)
-#ifdef HAVE_RK_SUPPORT_SETTING
+#ifdef ENABLE_RK_CONFIG_UI_SETTING
 
 static void display_warning();
 static void display_settings();
@@ -108,7 +108,7 @@ static void display_settings() {
         ux_flow_init(0, ux_settings_disabled_flow, NULL);
     }
 }
-#endif  // HAVE_RK_SUPPORT_SETTING
+#endif  // ENABLE_RK_CONFIG_UI_SETTING
 
 UX_STEP_NOCB(ux_idle_flow_1_step, pn, {&C_icon_security_key, "Security Key"});
 
@@ -132,15 +132,15 @@ UX_STEP_NOCB(ux_idle_flow_3_step,
                  APPVERSION,
              });
 
-#ifdef HAVE_RK_SUPPORT_SETTING
+#ifdef ENABLE_RK_CONFIG_UI_SETTING
 UX_STEP_VALID(ux_idle_flow_4_step,
               pb,
               display_settings(),
               {
                   &C_icon_coggle,
                   "Settings",
               });
-#endif  // HAVE_RK_SUPPORT_SETTING
+#endif  // ENABLE_RK_CONFIG_UI_SETTING
 
 UX_STEP_CB(ux_idle_flow_5_step,
            pb,
@@ -153,9 +153,9 @@ UX_FLOW(ux_idle_flow,
         &ux_idle_flow_1_step,
         &ux_idle_flow_2_step,
         &ux_idle_flow_3_step,
-#ifdef HAVE_RK_SUPPORT_SETTING
+#ifdef ENABLE_RK_CONFIG_UI_SETTING
         &ux_idle_flow_4_step,
-#endif  // HAVE_RK_SUPPORT_SETTING
+#endif  // ENABLE_RK_CONFIG_UI_SETTING
         &ux_idle_flow_5_step);
 
 void ui_idle(void) {
@@ -176,10 +176,10 @@ void ui_idle(void) {
  * 'Info' / 'Settings' menu
  */
 
-#ifdef HAVE_RK_SUPPORT_SETTING
+#ifdef ENABLE_RK_CONFIG_UI_SETTING
 static uint8_t initSettingPage;
 static nbgl_layoutSwitch_t switches[1] = {0};
-#endif  // HAVE_RK_SUPPORT_SETTING
+#endif  // ENABLE_RK_CONFIG_UI_SETTING
 static const char *const INFO_TYPES[] = {"Version", "Developer", "Copyright"};
 static const char *const INFO_CONTENTS[] = {APPVERSION, "Ledger", "(c) 2022-2024 Ledger"};
 
@@ -189,7 +189,7 @@ static const nbgl_contentInfoList_t infoList = {
     .infoContents = INFO_CONTENTS,
 };
 
-#ifdef HAVE_RK_SUPPORT_SETTING
+#ifdef ENABLE_RK_CONFIG_UI_SETTING
 
 static void controls_callback(int token, uint8_t index, int page);
 
@@ -244,7 +244,7 @@ static void controls_callback(int token, uint8_t index, int page) {
     }
     switches[0].initState = config_get_rk_enabled();
 }
-#endif  // HAVE_RK_SUPPORT_SETTING
+#endif  // ENABLE_RK_CONFIG_UI_SETTING
 
 /*
  * When no NFC, warning status page
@@ -294,7 +294,7 @@ void ui_idle(void) {
     }
 #endif  // ENABLE_NFC
 
-#ifdef HAVE_RK_SUPPORT_SETTING
+#ifdef ENABLE_RK_CONFIG_UI_SETTING
     switches[0].text = "Resident keys";
     switches[0].subText =
         "Stores login info on this\n"
@@ -308,17 +308,17 @@ void ui_idle(void) {
     switches[0].token = FIRST_USER_TOKEN;
     switches[0].tuneId = TUNE_TAP_CASUAL;
     switches[0].initState = config_get_rk_enabled();
-#endif  //  HAVE_RK_SUPPORT_SETTING
+#endif  //  ENABLE_RK_CONFIG_UI_SETTING
     nbgl_useCaseHomeAndSettings(
         APPNAME,
         &C_icon_security_key_64px,
         "Use this app for two-factor\nauthentication and\npassword-less log ins.",
         INIT_HOME_PAGE,
-#ifdef HAVE_RK_SUPPORT_SETTING
+#ifdef ENABLE_RK_CONFIG_UI_SETTING
         &settingContents,
 #else
         NULL,
-#endif  // HAVE_RK_SUPPORT_SETTING
+#endif  // ENABLE_RK_CONFIG_UI_SETTING
         &infoList,
         home_button,
         app_quit);

tests/speculos/fido2/test_fido2_screens.py

@@ -6,12 +6,12 @@
 from client import TESTS_SPECULOS_DIR
 from utils import generate_random_bytes
 from utils import generate_make_credentials_params
-from utils import HAVE_RK_SUPPORT_SETTING
+from utils import ENABLE_RK_CONFIG_UI_SETTING
 
 from ragger.navigator import NavInsID, NavIns
 
 
-@pytest.mark.skipif(not HAVE_RK_SUPPORT_SETTING, reason="settings not enable")
+@pytest.mark.skipif(not ENABLE_RK_CONFIG_UI_SETTING, reason="settings not enable")
 def test_fido_screens_settings(client, test_name):
 
     if client.model.startswith("nano"):

tests/speculos/fido2/test_get_info.py

@@ -1,6 +1,6 @@
 from hashlib import sha256
 from fido2.ctap2.base import Ctap2, Info
-from utils import HAVE_RK_SUPPORT_SETTING
+from utils import ENABLE_RK_CONFIG
 
 
 def test_get_info(client):
@@ -57,7 +57,7 @@ def test_get_info_options(client):
     info = client.ctap2.info
 
     # Specified options with fix value
-    if HAVE_RK_SUPPORT_SETTING:
+    if ENABLE_RK_CONFIG:
         assert not info.options["rk"]
     else:
         assert info.options["rk"]

tests/speculos/fido2/test_option_rk.py

@@ -7,12 +7,12 @@
 from client import TESTS_SPECULOS_DIR
 from utils import generate_random_bytes, generate_make_credentials_params
 from utils import generate_get_assertion_params
-from utils import HAVE_RK_SUPPORT_SETTING
+from utils import ENABLE_RK_CONFIG_UI_SETTING
 
 from ragger.navigator import NavInsID, NavIns
 
 
-@pytest.mark.skipif(not HAVE_RK_SUPPORT_SETTING,
+@pytest.mark.skipif(not ENABLE_RK_CONFIG_UI_SETTING,
                     reason="settings not enable")
 def test_option_rk_disabled(client):
     info = client.ctap2.info
@@ -36,7 +36,7 @@ def enable_rk_option(client):
     if info.options["rk"]:
         return
 
-    if not HAVE_RK_SUPPORT_SETTING:
+    if not ENABLE_RK_CONFIG_UI_SETTING:
         raise ValueError("rk and setting not enabled")
 
     if client.model.startswith("nano"):
@@ -85,13 +85,17 @@ def enable_rk_option(client):
     client.ctap2._info = client.ctap2.get_info()
 
 
+@pytest.mark.skipif(not ENABLE_RK_CONFIG_UI_SETTING,
+                    reason="settings not enable")
 def test_option_rk_enabled(client):
     enable_rk_option(client)
 
     info = client.ctap2.info
     assert info.options["rk"]
 
 
+@pytest.mark.skipif(not ENABLE_RK_CONFIG_UI_SETTING,
+                    reason="settings not enable")
 def test_option_rk_make_cred_exclude_refused(client, test_name):
     enable_rk_option(client)
 
@@ -141,6 +145,8 @@ def test_option_rk_make_cred_exclude_refused(client, test_name):
     client.ctap2.reset()
 
 
+@pytest.mark.skipif(not ENABLE_RK_CONFIG_UI_SETTING,
+                    reason="settings not enable")
 def test_option_rk_get_assertion(client, test_name):
     enable_rk_option(client)
 
@@ -205,6 +211,8 @@ def test_option_rk_get_assertion(client, test_name):
     "--fast" in sys.argv,
     reason="running in fast mode",
 )
+@pytest.mark.skipif(not ENABLE_RK_CONFIG_UI_SETTING,
+                    reason="settings not enable")
 def test_option_rk_key_store_full(client):
     enable_rk_option(client)
 
@@ -231,6 +239,8 @@ def test_option_rk_key_store_full(client):
     "--fast" in sys.argv,
     reason="running in fast mode",
 )
+@pytest.mark.skipif(not ENABLE_RK_CONFIG_UI_SETTING,
+                    reason="settings not enable")
 def test_option_rk_overwrite_get_assertion(client, test_name):
     enable_rk_option(client)
 

tests/speculos/u2f/test_u2f_screens.py

@@ -6,7 +6,7 @@
 
 from client import TESTS_SPECULOS_DIR
 from utils import generate_random_bytes, fido_known_appid
-from utils import HAVE_RK_SUPPORT_SETTING
+from utils import ENABLE_RK_CONFIG_UI_SETTING
 
 
 def test_u2f_screens_idle(client, test_name, firmware):
@@ -23,7 +23,7 @@ def test_u2f_screens_idle(client, test_name, firmware):
         # Screen 2 -> 3
         instructions.append(NavInsID.RIGHT_CLICK)
 
-        if HAVE_RK_SUPPORT_SETTING:
+        if ENABLE_RK_CONFIG_UI_SETTING:
             # Screen 3 -> 4
             instructions.append(NavInsID.RIGHT_CLICK)
     else:

tests/speculos/utils.py

@@ -11,7 +11,8 @@
 
 # Application build configuration
 HAVE_NO_RESET_GENERATION_INCREMENT = True
-HAVE_RK_SUPPORT_SETTING = False
+ENABLE_RK_CONFIG = True
+ENABLE_RK_CONFIG_UI_SETTING = False
 
 
 FIDO_RP_ID_HASH_1 = bytes.fromhex("000102030405060708090a0b0c0d0e0f"