Merge pull request #23 from LedgerHQ/develop

Align master with P1

.github/PULL_REQUEST_TEMPLATE.md

@@ -0,0 +1,8 @@
+# Checklist
+<!-- Put an `x` in each box when you have completed the items. -->
+- [ ] App update process has been followed <!-- See comment below -->
+- [ ] Target branch is `develop` <!-- unless you have a very good reason -->
+- [ ] Application version has been bumped <!-- required if your changes are to be deployed -->
+
+<!-- Make sure you followed the process described in https://developers.ledger.com/docs/device-app/deliver/maintenance before opening your Pull Request.
+Don't hesitate to contact us directly on Discord if you have any questions ! https://developers.ledger.com/discord -->

Makefile

@@ -34,7 +34,7 @@ APP_LOAD_PARAMS += $(COMMON_LOAD_PARAMS)
 
 APPVERSION_M=1
 APPVERSION_N=3
-APPVERSION_P=1
+APPVERSION_P=2
 APPVERSION=$(APPVERSION_M).$(APPVERSION_N).$(APPVERSION_P)
 
 ifeq ($(TARGET_NAME),TARGET_NANOS)

ledger_app.toml

@@ -0,0 +1,8 @@
+[app]
+build_directory = "./"
+sdk = "C"
+devices = ["nanos", "nanox", "nanos+", "stax"]
+
+[tests]
+unit_directory = "./tests/unit-tests/"
+pytest_directory = "./tests/speculos/"

src/config.c

@@ -22,8 +22,6 @@
 #include "config.h"
 #include "globals.h"
 
-#define RNG_MODULO 5
-
 config_t const N_u2f_real;
 
 static void derive_and_store_keys(void) {
@@ -33,7 +31,6 @@ static void derive_and_store_keys(void) {
     keyPath[0] = PRIVATE_KEY_PATH;
 
     // privateHmacKey
-    keyPath[0] = PRIVATE_KEY_PATH;
     os_perso_derive_node_bip32(CX_CURVE_SECP256R1, keyPath, 1, key, key + 32);
     if (memcmp(key, (uint8_t *) N_u2f.privateHmacKey, sizeof(N_u2f.privateHmacKey)) == 0) {
         // Keys are already initialized with the proper seed and resetGeneration

src/ui_shared.c

@@ -83,7 +83,7 @@ static void ui_menu_about() {
 void ui_idle(void) {
     nbgl_useCaseHome(APPNAME,
                      &C_stax_id_64px,
-                     "Ready to authenticate",
+                     "This app enables using\nyour Ledger device for\nTwo Factor Authentication.",
                      false,
                      ui_menu_about,
                      app_quit);

tests/speculos/client.py

@@ -49,15 +49,12 @@ class LedgerCtapHidConnection(CtapHidConnection):
     to speculos raw socket.
     """
     def __init__(self, transport, debug=False):
-        self.sock = socket.create_connection(('127.0.0.1', 9999))
+        self.sock = socket.create_connection(('127.0.0.1', 5001))
         self.u2f_hid_endpoint = (transport.upper() == "U2F")
         self.debug = debug
 
-        if self.u2f_hid_endpoint:
-            # Device answers should be fast
-            self.sock.settimeout(1)
-        else:
-            self.sock.settimeout(10)
+        # Set a timeout to allow tests to raise on socket rx failure
+        self.sock.settimeout(5)
 
     def write_packet(self, packet):
         packet = bytes(packet)

tests/speculos/conftest.py

@@ -50,7 +50,7 @@ def prepare_speculos_args(root_pytest_dir: Path, firmware: Firmware, display: bo
     if display:
         speculos_args += ["--display", "qt"]
 
-    device = firmware.device
+    device = firmware.name
     if device == "nanosp":
         device = "nanos2"
 

tests/speculos/ctap1_client.py

@@ -58,8 +58,8 @@ def confirm(self):
         self.navigator.navigate(instructions,
                                 screen_change_after_last_instruction=False)
 
-    def wait_for_return_on_dashboard(self):
-        if self.model == "stax":
+    def wait_for_return_on_dashboard(self, dismiss=False):
+        if dismiss and self.model == "stax":
             # On Stax tap on the center to dismiss the status message faster
             self.navigator.navigate([NavInsID.USE_CASE_STATUS_DISMISS],
                                     screen_change_before_first_instruction=True)
@@ -161,11 +161,11 @@ def register(self, client_param, app_param, user_accept=True,
                 response = self.parse_response(response)
             else:
                 if user_accept is not None:
-                    self.wait_for_return_on_dashboard()
+                    self.wait_for_return_on_dashboard(dismiss=True)
                 raise e
 
         if user_accept is not None:
-            self.wait_for_return_on_dashboard()
+            self.wait_for_return_on_dashboard(dismiss=True)
 
         return RegistrationData(response)
 
@@ -243,10 +243,10 @@ def authenticate(self, client_param, app_param, key_handle,
                 response = self.parse_response(response)
             else:
                 if user_accept is not None:
-                    self.wait_for_return_on_dashboard()
+                    self.wait_for_return_on_dashboard(dismiss=True)
                 raise e
 
         if user_accept is not None:
-            self.wait_for_return_on_dashboard()
+            self.wait_for_return_on_dashboard(dismiss=True)
 
         return SignatureData(response)

tests/speculos/requirements.txt

@@ -1,3 +1,4 @@
 ragger[tests,speculos]>=1.7.0
 pytest>=6.1.1,<7.0.0
+cryptography>=3.3.1,<4.0.0
 fido2==1.0.0

tests/speculos/setup.cfg

@@ -1,8 +1,15 @@
 [flake8]
-ignore = C0114,  # missing-module-docstring
-         C0115,  # missing-class-docstring
-         C0116,  # missing-function-docstring
-         C0103,  # invalid-name
-         R0801,  # duplicate-code
-         R0913,  # too-many-arguments
+ignore =
+         # missing-module-docstring
+         C114,
+         # missing-class-docstring
+         C115,
+         # missing-function-docstring
+         C116,
+         # invalid-name
+         C103,
+         # duplicate-code
+         R801,
+         # too-many-arguments
+         R913,
 max-line-length = 100

tests/speculos/u2f/test_authenticate_cmd.py

@@ -1,7 +1,6 @@
 import cryptography
 import pytest
 import struct
-import time
 
 from fido2.ctap1 import Ctap1, ApduError, SignatureData
 from fido2.hid import CTAPHID
@@ -127,7 +126,7 @@ def test_authenticate_with_reboot_ok(client):
 
 def test_authenticate_multiple_ok(client):
     registrations = []
-    for _ in range(10):
+    for _ in range(5):
         app_param, registration_data = register(client)
         registrations.append((app_param, registration_data))
 
@@ -145,7 +144,7 @@ def test_authenticate_counter_increment(client):
     app_param, registration_data = register(client)
 
     prev = 0
-    for _ in range(10):
+    for _ in range(5):
         challenge = generate_random_bytes(32)
 
         authentication_data = client.ctap1.authenticate(challenge,
@@ -324,11 +323,6 @@ def test_authenticate_raw(client):
             # On U2F endpoint, the device should return APDU.SW_CONDITIONS_NOT_SATISFIED
             # until user validate.
             for i in range(5):
-
-                if client.model == "stax":
-                    # Patch issue with more time needed on Stax on CI or slow computers
-                    time.sleep(0.5)
-
                 client.ctap1.send_apdu_nowait(ins=Ctap1.INS.AUTHENTICATE,
                                               p1=p1, data=data)
 

tests/speculos/u2f/test_register_cmd.py

@@ -1,6 +1,5 @@
 import pytest
 import socket
-import time
 
 from cryptography.x509 import load_der_x509_certificate
 
@@ -85,7 +84,7 @@ def test_register_duplicate(client):
 
 
 def test_register_multiple_ok(client):
-    for i in range(10):
+    for _ in range(5):
         challenge = generate_random_bytes(32)
         app_param = generate_random_bytes(32)
 
@@ -153,11 +152,6 @@ def test_register_raw(client):
     # On U2F endpoint, the device should return APDU.SW_CONDITIONS_NOT_SATISFIED
     # until user validate.
     for i in range(5):
-
-        if client.model == "stax":
-            # Patch issue with more time needed on Stax on CI or slow computers
-            time.sleep(0.5)
-
         client.ctap1.send_apdu_nowait(cla=0x00,
                                       ins=Ctap1.INS.REGISTER,
                                       p1=0x00,
@@ -244,6 +238,10 @@ def test_register_raw_u2f_fake_channel_security_crc(client):
                                       screen_change_after_last_instruction=False)
 
         # App should then recover and allow new requests
+        client.ctap1.wait_for_return_on_dashboard()
+
+        challenge = bytearray(generate_random_bytes(32))
+        app_param = generate_random_bytes(32)
         registration_data = client.ctap1.register(challenge, app_param)
         registration_data.verify(app_param, challenge)
 
@@ -294,6 +292,10 @@ def test_register_raw_u2f_fake_channel_security_length(client):
                                   screen_change_after_last_instruction=False)
 
     # App should then recover and allow new requests
+    client.ctap1.wait_for_return_on_dashboard()
+
+    challenge = generate_random_bytes(32)
+    app_param = generate_random_bytes(32)
     registration_data = client.ctap1.register(challenge, app_param)
     registration_data.verify(app_param, challenge)