diff --git a/Makefile b/Makefile index ebdaa35c6..aa468ac2d 100644 --- a/Makefile +++ b/Makefile @@ -14,8 +14,15 @@ # This makefile is used as a command runner and not for tracking dependencies between recipies +UNAME_S := $(shell uname -s) + .DEFAULT_GOAL := firmware -SANITIZE ?= ON +# asan/ubsan is not supported on darwin, default to off +ifeq ($(UNAME_S),Darwin) + SANITIZE ?= OFF +else + SANITIZE ?= ON +endif simulator: SANITIZE = OFF bootstrap: diff --git a/src/firmware.c b/src/firmware.c index 6552d0754..79a85d0d8 100644 --- a/src/firmware.c +++ b/src/firmware.c @@ -23,6 +23,10 @@ #include "screen.h" #include "ui/screen_stack.h" +#if APP_U2F == 1 +#include +#endif + uint32_t __stack_chk_guard = 0; int main(void) @@ -36,6 +40,9 @@ int main(void) qtouch_init(); common_main(); bitbox02_smarteeprom_init(); +#if APP_U2F == 1 + u2f_device_setup(); +#endif firmware_main_loop(); return 0; } diff --git a/src/firmware_main_loop.c b/src/firmware_main_loop.c index 4b0afc3c3..3e7f47780 100644 --- a/src/firmware_main_loop.c +++ b/src/firmware_main_loop.c @@ -59,6 +59,12 @@ void firmware_main_loop(void) } if (!u2f_data) { u2f_data = queue_pull(queue_u2f_queue()); + // If USB stack was locked and there is no more messages to send out, time to + // unlock it. + if (!u2f_data && usb_processing_locked(usb_processing_u2f())) { + util_log("u2f unlock"); + usb_processing_unlock(); + } } #endif // Only read new messages if we have nothing to send @@ -67,6 +73,7 @@ void firmware_main_loop(void) } #if APP_U2F == 1 if (!u2f_data && hid_u2f_read(&u2f_frame[0])) { + util_log("u2f data %s", util_dbg_hex((void*)u2f_frame, 16)); u2f_packet_process((const USB_FRAME*)u2f_frame); } #endif diff --git a/src/u2f.c b/src/u2f.c index 8490efbec..ed94123d8 100644 --- a/src/u2f.c +++ b/src/u2f.c @@ -32,6 +32,7 @@ #include #include #include +#include #include #include @@ -68,19 +69,14 @@ typedef enum { } u2f_auth_state_t; typedef struct { - uint32_t cid; /** - * Command that is currently executing - * (blocking) on the U2F stack. + * CID and last command are used to check that we eventually respond to the right command after + * the user has confirmed on screen. U2F is normally stateless between transactions, but since + * we have a display and Yes/No input we can ask the user on the first request and later respond + * that the user is present only if the user confirmed specifically that app-id. */ + uint32_t cid; uint8_t last_cmd; - /** - * Keeps track of whether there is an outstanding - * U2F operation going on in the background. - * This is not strictly necessary, but it's useful - * to have as a sanity checking mechanism. - */ - bool locked; /** * Keeps track of which part of a registration we're currently in. */ @@ -131,30 +127,6 @@ static void _clear_state(void) { _state.reg = U2F_REGISTER_IDLE; _state.auth = U2F_AUTHENTICATE_IDLE; - _state.locked = false; -} - -/** - * Unlocks the USB stack. Resets the U2F state. - */ -static void _unlock(void) -{ - usb_processing_unlock(); - _clear_state(); -} - -/** - * Locks the USB stack to process the given - * U2F request. - * - * @param apdu U2F packet that will own the lock on the USB stack. No other request - * will be processed by the USB stack until this request completes. - */ -static void _lock(const USB_APDU* apdu) -{ - usb_processing_lock(usb_processing_u2f()); - _state.locked = true; - _state.last_cmd = apdu->ins; } static component_t* _nudge_label = NULL; @@ -183,7 +155,7 @@ static void _start_refresh_webpage_screen(void) // Unfortunately unlocking takes more time than U2F is allowed to take. With the current // architecture of the firmware we cannot run it concurrently to other requests. Therefore // we will call it here, make the user unlock the device and then ask the user to refresh - // the webpage. (refreshing is only needed for some browsers) + // the webpage. (refreshing is only needed for some FIDO Clients) _state.refresh_webpage = _create_refresh_webpage(); ui_screen_stack_push(_state.refresh_webpage); _state.refresh_webpage_timeout = 0; @@ -421,7 +393,7 @@ static void _register_start(const USB_APDU* apdu, Packet* out_packet) const U2F_REGISTER_REQ* reg_request = (const U2F_REGISTER_REQ*)apdu->data; uint16_t req_error = _register_sanity_check_req(apdu); if (req_error) { - _unlock(); + _clear_state(); _error(req_error, out_packet); return; } @@ -462,7 +434,7 @@ static void _register_continue(const USB_APDU* apdu, Packet* out_packet) } /* No more pending operations for U2F register */ - _unlock(); + _clear_state(); if (async_result == ASYNC_OP_FALSE) { _error(U2F_SW_CONDITIONS_NOT_SATISFIED, out_packet); @@ -582,7 +554,7 @@ static void _authenticate_start(const USB_APDU* apdu, Packet* out_packet) { uint16_t req_error = _authenticate_sanity_check_req(apdu); if (req_error) { - _unlock(); + _clear_state(); _error(req_error, out_packet); return; } @@ -596,7 +568,7 @@ static void _authenticate_start(const USB_APDU* apdu, Packet* out_packet) } uint16_t key_error = _authenticate_start_confirm(apdu); if (key_error) { - _unlock(); + _clear_state(); _error(key_error, out_packet); return; } @@ -608,7 +580,7 @@ static void _authenticate_wait_refresh(const USB_APDU* apdu, Packet* out_packet) _assert_unlocked(); uint16_t key_error = _authenticate_start_confirm(apdu); if (key_error) { - _unlock(); + _clear_state(); _error(key_error, out_packet); return; } @@ -624,10 +596,18 @@ static void _authenticate_continue(const USB_APDU* apdu, Packet* out_packet) U2F_AUTHENTICATE_SIG_STR sig_base; uint16_t req_error = _authenticate_sanity_check_req(apdu); if (req_error) { + _clear_state(); _error(req_error, out_packet); return; } + uint16_t key_error = _authenticate_verify_key_valid(apdu); + if (key_error) { + _clear_state(); + _error(key_error, out_packet); + return; + } + const U2F_AUTHENTICATE_REQ* auth_request = (const U2F_AUTHENTICATE_REQ*)apdu->data; async_op_result_t async_result = u2f_app_confirm_retry(U2F_APP_AUTHENTICATE, auth_request->appId); @@ -637,7 +617,7 @@ static void _authenticate_continue(const USB_APDU* apdu, Packet* out_packet) } /* No more blocking operations pending for authentication. */ - _unlock(); + _clear_state(); if (async_result == ASYNC_OP_FALSE) { _error(U2F_SW_CONDITIONS_NOT_SATISFIED, out_packet); @@ -761,6 +741,8 @@ static void _cmd_init(const Packet* in_packet, Packet* out_packet, const size_t return; } + util_log("u2f init"); + const U2FHID_INIT_REQ* init_req = (const U2FHID_INIT_REQ*)&in_packet->data_addr; U2FHID_INIT_RESP response; @@ -787,14 +769,15 @@ static void _cmd_register(const Packet* in_packet, Packet* out_packet) { const USB_APDU* apdu = (const USB_APDU*)in_packet->data_addr; /* Sanity-check our state. */ - if (_state.reg != U2F_REGISTER_IDLE && - (_state.last_cmd != U2F_REGISTER || _state.cid != in_packet->cid)) { - Abort("U2F reg arbitration failed."); + if (_state.reg != U2F_REGISTER_IDLE && _state.last_cmd != U2F_REGISTER) { + util_log("u2f: ERROR register invalid state"); + _clear_state(); + return; } switch (_state.reg) { case U2F_REGISTER_IDLE: - _lock(apdu); + util_log("reg idle"); _register_start(apdu, out_packet); break; case U2F_REGISTER_UNLOCKING: @@ -804,6 +787,7 @@ static void _cmd_register(const Packet* in_packet, Packet* out_packet) _register_wait_refresh(apdu, out_packet); break; case U2F_REGISTER_CONFIRMING: + util_log("reg confirmgin"); _register_continue(apdu, out_packet); break; default: @@ -811,29 +795,6 @@ static void _cmd_register(const Packet* in_packet, Packet* out_packet) } } -/** - * Abort an existing registration request. - */ -static void _abort_register(void) -{ - switch (_state.reg) { - case U2F_REGISTER_UNLOCKING: - rust_workflow_abort_current(); - _clear_state(); - break; - case U2F_REGISTER_CONFIRMING: - u2f_app_confirm_abort(); - _clear_state(); - break; - case U2F_REGISTER_WAIT_REFRESH: - _stop_refresh_webpage_screen(); - _clear_state(); - break; - default: - Abort("Bad U2F register abort status"); - } -} - /** * Processes an incoming registration request. */ @@ -843,12 +804,13 @@ static void _cmd_authenticate(const Packet* in_packet, Packet* out_packet) /* Sanity-check our state. */ if (_state.auth != U2F_AUTHENTICATE_IDLE && (_state.last_cmd != U2F_AUTHENTICATE || _state.cid != in_packet->cid)) { - Abort("U2F auth arbitration failed."); + util_log("u2f: ERROR authenticate invalid state"); + _clear_state(); + return; } switch (_state.auth) { case U2F_AUTHENTICATE_IDLE: - _lock(apdu); _authenticate_start(apdu, out_packet); break; case U2F_AUTHENTICATE_UNLOCKING: @@ -865,35 +827,15 @@ static void _cmd_authenticate(const Packet* in_packet, Packet* out_packet) } } -/** - * Abort an existing authentication request. - */ -static void _abort_authenticate(void) -{ - switch (_state.auth) { - case U2F_AUTHENTICATE_UNLOCKING: - rust_workflow_abort_current(); - _clear_state(); - break; - case U2F_AUTHENTICATE_CONFIRMING: - u2f_app_confirm_abort(); - _clear_state(); - break; - case U2F_AUTHENTICATE_WAIT_REFRESH: - _stop_refresh_webpage_screen(); - _clear_state(); - break; - default: - Abort("Bad U2F register abort status"); - } -} - /** * Process a U2F message */ static void _cmd_msg(const Packet* in_packet, Packet* out_packet, const size_t max_out_len) { (void)max_out_len; + + util_log("u2f msg"); + // By default always use the recieved cid _state.cid = in_packet->cid; @@ -911,33 +853,27 @@ static void _cmd_msg(const Packet* in_packet, Packet* out_packet, const size_t m switch (apdu->ins) { case U2F_REGISTER: _cmd_register(in_packet, out_packet); + _state.last_cmd = apdu->ins; break; case U2F_AUTHENTICATE: _cmd_authenticate(in_packet, out_packet); + _state.last_cmd = apdu->ins; break; case U2F_VERSION: _version(apdu, out_packet); break; default: _error(U2F_SW_INS_NOT_SUPPORTED, out_packet); - return; + break; } } +// U2F only locks USB stack between a request and a response. A request is always immediately +// followed by a response. New requests must wait until the current one is finished. bool u2f_blocking_request_can_go_through(const Packet* in_packet) { - if (!_state.locked) { - Abort("USB stack thinks we're busy, but we're not."); - } - /* - * Check if this request is the same one we're currently operating on. - * For now, this checks the request type and channel ID only. - * FUTURE: Maybe check that the key handle is maintained between requests? - * so that when we're asking for confirmation, we ask to confirm - * "a particular key handle" instead of "a particular tab". - */ - const USB_APDU* apdu = (const USB_APDU*)in_packet->data_addr; - return apdu->ins == _state.last_cmd && in_packet->cid == _state.cid; + (void)in_packet; + return false; } void u2f_blocked_req_error(Packet* out_packet, const Packet* in_packet) @@ -980,11 +916,11 @@ static void _process_wait_refresh(void) { if (_state.refresh_webpage_timeout == 25000) { _stop_refresh_webpage_screen(); - _unlock(); + _clear_state(); } else { _state.refresh_webpage_timeout++; /* Prevent the USB watchdog from killing this workflow. */ - usb_processing_timeout_reset(); + usb_processing_timeout_reset(0); } } @@ -1024,9 +960,6 @@ static void _process_authenticate(void) void u2f_process(void) { - if (!_state.locked) { - return; - } switch (_state.last_cmd) { case U2F_REGISTER: _process_register(); @@ -1034,6 +967,9 @@ void u2f_process(void) case U2F_AUTHENTICATE: _process_authenticate(); break; + case 0: + // initial value + break; default: Abort("Bad U2F process state."); } @@ -1044,6 +980,8 @@ void u2f_process(void) */ void u2f_device_setup(void) { + _clear_state(); + _state.last_cmd = 0; const CMD_Callback u2f_cmd_callbacks[] = { {U2FHID_PING, _cmd_ping}, {U2FHID_WINK, _cmd_wink}, @@ -1056,17 +994,5 @@ void u2f_device_setup(void) void u2f_abort_outstanding_op(void) { - if (!_state.locked) { - Abort("USB stack thinks U2F is busy, but it's not."); - } - switch (_state.last_cmd) { - case U2F_REGISTER: - _abort_register(); - break; - case U2F_AUTHENTICATE: - _abort_authenticate(); - break; - default: - Abort("Invalid U2F status on abort."); - } + util_log("u2f_abort_outstanding_op"); } diff --git a/src/u2f/u2f_app.c b/src/u2f/u2f_app.c index 269d3a532..4bdb0be5e 100644 --- a/src/u2f/u2f_app.c +++ b/src/u2f/u2f_app.c @@ -219,7 +219,9 @@ void u2f_app_confirm_start(enum u2f_app_confirm_t type, const uint8_t* app_id) async_op_result_t u2f_app_confirm_retry(enum u2f_app_confirm_t type, const uint8_t* app_id) { if (_state.outstanding_confirm != type || !MEMEQ(app_id, _state.app_id, 32)) { - Abort("Arbitration failed for U2F confirmation."); + // CID used a new app_id, clearly invalid + // TODO: Clear u2f state + return ASYNC_OP_NOT_READY; } bool result = false; if (!rust_workflow_confirm_poll(&result)) { diff --git a/src/u2f/u2f_packet.c b/src/u2f/u2f_packet.c index 1b4468965..64669c82f 100644 --- a/src/u2f/u2f_packet.c +++ b/src/u2f/u2f_packet.c @@ -19,6 +19,7 @@ #include #include #include +#include #define ERR_NONE 0 @@ -126,6 +127,7 @@ void u2f_packet_timeout_tick(void) void u2f_packet_timeout(uint32_t cid) { + util_log("u2f_packet_timeout"); _timeout_disable(cid); if (cid == _in_state.cid) { _reset_state(); @@ -138,29 +140,41 @@ bool u2f_packet_process(const USB_FRAME* frame) struct usb_processing* ctx = usb_processing_u2f(); switch (usb_frame_process(frame, &_in_state)) { case FRAME_ERR_IGNORE: + util_log("u2f_packet ignore"); // Ignore this frame, i.e. no response. break; case FRAME_ERR_INVALID_SEQ: + util_log("u2f_packet seq"); // Reset the state becuase this error indicates that there is a host application bug _reset_state(); _queue_err(FRAME_ERR_INVALID_SEQ, frame->cid); break; case FRAME_ERR_CHANNEL_BUSY: + util_log("u2f_packet busy"); // We don't reset the state because this error doesn't indicate something wrong with the // "current" connection. _queue_err(FRAME_ERR_CHANNEL_BUSY, frame->cid); break; case FRAME_ERR_INVALID_LEN: + util_log("u2f_packet invalid len"); // Reset the state becuase this error indicates that there is a host application bug _reset_state(); _queue_err(FRAME_ERR_INVALID_LEN, frame->cid); break; case ERR_NONE: + // The usb stack must be locked until we have responded to this request. + // If init frame, lock usb stack: + if (frame->type & FRAME_TYPE_INIT) { + util_log("usb_processing lock"); + usb_processing_lock(usb_processing_u2f()); + } + util_log("u2f_packet none"); _reset_timeout(frame->cid); if (_need_more_data()) { // Do not send a message yet return true; } + util_log("u2f_packet none2"); /* We have received a complete frame. Buffer it for processing. */ if (usb_processing_enqueue( ctx, _in_state.data, _in_state.len, _in_state.cmd, _in_state.cid)) { diff --git a/src/usb/usb.c b/src/usb/usb.c index be3ad3766..67a7cec0a 100644 --- a/src/usb/usb.c +++ b/src/usb/usb.c @@ -72,7 +72,6 @@ static void _u2f_endpoint_available(void) if (!hid_u2f_is_enabled()) { return; }; - u2f_device_setup(); hid_u2f_setup(); } #endif diff --git a/src/usb/usb_processing.c b/src/usb/usb_processing.c index 57695dde1..807205b48 100644 --- a/src/usb/usb_processing.c +++ b/src/usb/usb_processing.c @@ -121,7 +121,7 @@ typedef struct { * USB_OUTSTANDING_OP_TIMEOUT_TICKS, any outstanding operation is aborted * and the USB stack is forcefully unlocked. */ - uint16_t timeout_counter; + int16_t timeout_counter; #endif } usb_processing_state_t; @@ -287,7 +287,7 @@ static void _usb_arbitrate_packet(struct usb_processing* ctx, const Packet* in_p } else { _usb_execute_packet(ctx, in_packet); /* New packet processed: reset the watchdog timeout. */ - _usb_state.timeout_counter = 0; + usb_processing_timeout_reset(0); } } #endif @@ -373,7 +373,7 @@ struct usb_processing* usb_processing_hww(void) static void _timer_cb(const struct timer_task* const timer_task) { (void)timer_task; - if (_usb_state.timeout_counter != (uint16_t)-1) { + if (_usb_state.timeout_counter < INT16_MAX) { _usb_state.timeout_counter++; } } @@ -426,9 +426,9 @@ void usb_processing_lock(struct usb_processing* ctx) _usb_state.blocking_ctx = ctx; } -void usb_processing_timeout_reset(void) +void usb_processing_timeout_reset(int16_t val) { - _usb_state.timeout_counter = 0; + _usb_state.timeout_counter = val; } void usb_processing_unlock(void) @@ -438,4 +438,8 @@ void usb_processing_unlock(void) } _usb_state.blocking_ctx = NULL; } +bool usb_processing_locked(struct usb_processing* ctx) +{ + return _usb_state.blocking_ctx == ctx; +} #endif diff --git a/src/usb/usb_processing.h b/src/usb/usb_processing.h index 6ec812d88..d8106d561 100644 --- a/src/usb/usb_processing.h +++ b/src/usb/usb_processing.h @@ -73,9 +73,11 @@ void usb_processing_lock(struct usb_processing* ctx); * expected behaviour, one can call this function to prevent * the USB watchdog from aborting the outstanding operation. */ -void usb_processing_timeout_reset(void); +void usb_processing_timeout_reset(int16_t val); void usb_processing_unlock(void); + +bool usb_processing_locked(struct usb_processing* ctx); #endif #endif diff --git a/test/unit-test/CMakeLists.txt b/test/unit-test/CMakeLists.txt index c6c2a4c11..efa9b6aad 100644 --- a/test/unit-test/CMakeLists.txt +++ b/test/unit-test/CMakeLists.txt @@ -14,10 +14,6 @@ # No linker for Mach-O supports the linker argument `--wrap`. Since we use # that, unit tests will never work on macos. Use linux/arm64 in docker instead. -if(${CMAKE_SYSTEM_NAME} MATCHES "Darwin") - message("No linker for Mach-O supports --wrap, will not generate unit-tests") - return() -endif() # We use FindPkgConfig instead of FindPackage because it finds libraries in # both linux and macos @@ -26,7 +22,11 @@ find_package(PkgConfig REQUIRED) # Unit testing uses CMocka pkg_check_modules(CMOCKA REQUIRED cmocka) # u2f tests with hardware uses hidapi-hidraw -pkg_check_modules(HIDAPI REQUIRED hidapi-hidraw) +if(${CMAKE_SYSTEM_NAME} MATCHES "Darwin") + pkg_check_modules(HIDAPI REQUIRED hidapi) +else() + pkg_check_modules(HIDAPI REQUIRED hidapi-hidraw) +endif() message("hidapi-hidraw " ${HIDAPI_INCLUDE_DIRS}) @@ -175,7 +175,7 @@ target_compile_definitions(bitbox PUBLIC TESTING _UNIT_TEST_) target_link_libraries(bitbox PUBLIC secp256k1 - ${CMOCKA_LIBRARIES} + ${CMOCKA_LDFLAGS} PRIVATE wallycore fatfs @@ -218,7 +218,7 @@ target_include_directories( target_compile_definitions(u2f-util PUBLIC "TESTING" _UNIT_TEST_ PRODUCT_BITBOX_MULTI "APP_U2F=1" "APP_BTC=1" "APP_LTC=1" "APP_ETH=1") target_compile_definitions(u2f-util PUBLIC "USE_KECCAK") -target_link_libraries(u2f-util PUBLIC ${HIDAPI_LIBRARIES}) +target_link_libraries(u2f-util PUBLIC ${HIDAPI_LDFLAGS}) #----------------------------------------------------------------------------- @@ -257,28 +257,32 @@ set(TEST_LIST "" ) -list(LENGTH TEST_LIST TEST_LIST_LEN) -math(EXPR TEST_LIST_LEN ${TEST_LIST_LEN}-1) -foreach(I RANGE 0 ${TEST_LIST_LEN} 2) - math(EXPR I2 ${I}+1) - list(GET TEST_LIST ${I} TEST_NAME) - list(GET TEST_LIST ${I2} TEST_LINK_ARGS) - set(EXE test_${TEST_NAME}) - add_executable(${EXE} test_${TEST_NAME}.c framework/eh_personality.c) - # asan must be first library in linking order - target_link_libraries(${EXE} PRIVATE - $<$:asan> - $<$:-fsanitize=undefined> - -Wl,--start-group - c-unit-tests_rust_c - bitbox - -Wl,--end-group - ${TEST_LINK_ARGS} - ) - if(NOT ${TEST_NAME} STREQUAL "simulator") - add_test(NAME test_${TEST_NAME} COMMAND ${EXE}) - endif() -endforeach() +if(${CMAKE_SYSTEM_NAME} MATCHES "Darwin") + message("No linker for Mach-O supports --wrap, will not generate unit-tests") +else() + list(LENGTH TEST_LIST TEST_LIST_LEN) + math(EXPR TEST_LIST_LEN ${TEST_LIST_LEN}-1) + foreach(I RANGE 0 ${TEST_LIST_LEN} 2) + math(EXPR I2 ${I}+1) + list(GET TEST_LIST ${I} TEST_NAME) + list(GET TEST_LIST ${I2} TEST_LINK_ARGS) + set(EXE test_${TEST_NAME}) + add_executable(${EXE} test_${TEST_NAME}.c framework/eh_personality.c) + # asan must be first library in linking order + target_link_libraries(${EXE} PRIVATE + $<$:asan> + $<$:-fsanitize=undefined> + -Wl,--start-group + c-unit-tests_rust_c + bitbox + -Wl,--end-group + ${TEST_LINK_ARGS} + ) + if(NOT ${TEST_NAME} STREQUAL "simulator") + add_test(NAME test_${TEST_NAME} COMMAND ${EXE}) + endif() + endforeach() +endif() # These unit tests for U2F are special because they don't call any bitbox functions directly, instead they go through hid_read/write. @@ -296,10 +300,10 @@ foreach(TEST_NAME ${U2F_TESTS}) target_link_libraries(${EXE} PRIVATE $<$:asan> $<$:-fsanitize=undefined> - -Wl,--start-group + $<$>:-Wl,--start-group> c-unit-tests_rust_c bitbox - -Wl,--end-group + $<$>:-Wl,--end-group> u2f-util ) target_compile_definitions(${EXE} PRIVATE "TESTING") diff --git a/test/unit-test/framework/mock_hidapi.c b/test/unit-test/framework/mock_hidapi.c index bb47b97b7..48412355e 100644 --- a/test/unit-test/framework/mock_hidapi.c +++ b/test/unit-test/framework/mock_hidapi.c @@ -8,7 +8,7 @@ // POSIX #include -#include +#include #include "queue.h" #include "u2f.h" @@ -136,6 +136,9 @@ int hid_read_timeout(hid_device* dev, unsigned char* data, size_t length, int mi return -127; } if (queue_peek(queue_u2f_queue()) == NULL) { + if (usb_processing_locked(usb_processing_u2f())) { + usb_processing_unlock(); + } _have_data = false; } _expect_more = false; diff --git a/test/unit-test/test_u2f_standard.c b/test/unit-test/test_u2f_standard.c index 5c665a586..27e0196ea 100644 --- a/test/unit-test/test_u2f_standard.c +++ b/test/unit-test/test_u2f_standard.c @@ -34,6 +34,7 @@ struct U2Fob* device; U2F_REGISTER_REQ regReq; U2F_REGISTER_RESP regRsp; +U2F_AUTHENTICATE_REQ authReq; static void util_uint8_to_hex(const uint8_t* in_bin, const size_t in_len, char* out) { @@ -126,14 +127,6 @@ static void test_WrongLength_U2F_REGISTER(void) static void test_Enroll(int expectedSW12, int printinfo) { - // pick random origin and challenge. - for (size_t i = 0; i < sizeof(regReq.challenge); ++i) { - regReq.challenge[i] = rand(); - } - for (size_t i = 0; i < sizeof(regReq.appId); ++i) { - regReq.appId[i] = rand(); - } - uint64_t t = 0; U2Fob_deltaTime(&t); @@ -218,12 +211,6 @@ static void test_Enroll(int expectedSW12, int printinfo) #if defined(WITH_HARDWARE) static uint32_t test_Sign(int expectedSW12, bool checkOnly) { - U2F_AUTHENTICATE_REQ authReq; - - // pick random challenge and use registered appId. - for (size_t i = 0; i < sizeof(authReq.challenge); ++i) { - authReq.challenge[i] = rand(); - } memcpy(authReq.appId, regReq.appId, sizeof(authReq.appId)); authReq.keyHandleLength = regRsp.keyHandleLen; memcpy(authReq.keyHandle, regRsp.keyHandleCertSig, authReq.keyHandleLength); @@ -316,6 +303,14 @@ static void run_tests(void) PASS(test_WrongLength_U2F_REGISTER()); PASS(test_BadCLA()); + // pick random origin and challenge. + for (size_t i = 0; i < sizeof(regReq.challenge); ++i) { + regReq.challenge[i] = rand(); + } + for (size_t i = 0; i < sizeof(regReq.appId); ++i) { + regReq.appId[i] = rand(); + } + // Fob with button should need touch. if (arg_hasButton) { // Timeout @@ -328,6 +323,11 @@ static void run_tests(void) WaitForUserPresence(device, arg_hasButton); PASS(test_Enroll(0x9000, 1)); + // pick random challenge and use registered appId. + for (size_t i = 0; i < sizeof(authReq.challenge); ++i) { + authReq.challenge[i] = rand(); + } + // Fob with button should have consumed touch. if (arg_hasButton) { // Timeout @@ -352,6 +352,12 @@ static void run_tests(void) WaitForUserPresence(device, arg_hasButton); PASS(test_Sign(0x6985, true)); + // This triggers the confirmation screen + WaitForUserPresence(device, arg_hasButton); + PASS(test_Sign(0x6985, false)); + + WaitForUserPresence(device, arg_hasButton); + uint32_t ctr1; PASS(ctr1 = test_Sign(0x9000, false)); // < fails // Timeout diff --git a/test/unit-test/u2f/u2f_util_t.c b/test/unit-test/u2f/u2f_util_t.c index 10ef6f76d..a5d853786 100644 --- a/test/unit-test/u2f/u2f_util_t.c +++ b/test/unit-test/u2f/u2f_util_t.c @@ -239,6 +239,7 @@ int U2Fob_init(struct U2Fob* device) res = U2Fob_receiveHidFrame(device, &response, 2.0); if (res == -U2FHID_ERR_MSG_TIMEOUT) { + printf("err timeout\n"); return res; } if (res == -U2FHID_ERR_OTHER) {