From d19285019d7f9a868f3bd603aa3d7b05c87c9eb3 Mon Sep 17 00:00:00 2001 From: zvecr Date: Tue, 12 Apr 2022 01:37:25 +0100 Subject: [PATCH] All other subsystems are disabled during unlock --- data/xap/xap_0.1.0.hjson | 4 +- lib/python/qmk/cli/xap/xap.py | 2 +- .../qmk/xap/gen_firmware/inline_generator.py | 11 ++++- quantum/secure.c | 4 -- quantum/secure.h | 40 ++++++++++++++++- quantum/xap/xap.c | 45 ++++++++++++------- 6 files changed, 81 insertions(+), 25 deletions(-) diff --git a/data/xap/xap_0.1.0.hjson b/data/xap/xap_0.1.0.hjson index 3d094eb375..6835ec05bd 100755 --- a/data/xap/xap_0.1.0.hjson +++ b/data/xap/xap_0.1.0.hjson @@ -171,6 +171,7 @@ * 2 means secure routes are allowed * any other value should be interpreted as disabled ''' + permissions: ignore return_type: u8 return_execute: secure_status } @@ -185,6 +186,7 @@ type: command name: Secure Lock define: SECURE_LOCK + permissions: ignore description: Disable secure routes return_execute: secure_lock } @@ -301,7 +303,7 @@ type: command name: Jump to bootloader define: BOOTLOADER_JUMP - secure: true + permissions: secure enable_if_preprocessor: defined(BOOTLOADER_JUMP_SUPPORTED) description: ''' diff --git a/lib/python/qmk/cli/xap/xap.py b/lib/python/qmk/cli/xap/xap.py index 9ab41e3ee3..57f93afaf1 100644 --- a/lib/python/qmk/cli/xap/xap.py +++ b/lib/python/qmk/cli/xap/xap.py @@ -93,7 +93,7 @@ def _xap_transaction(device, sub, route, *args): def _query_device(device): ver_data = _xap_transaction(device, 0x00, 0x00) if not ver_data: - return {'xap': 'UNKNOWN'} + return {'xap': 'UNKNOWN', 'secure': 'UNKNOWN'} # to u32 to BCD string a = (ver_data[3] << 24) + (ver_data[2] << 16) + (ver_data[1] << 8) + (ver_data[0]) diff --git a/lib/python/qmk/xap/gen_firmware/inline_generator.py b/lib/python/qmk/xap/gen_firmware/inline_generator.py index 074a37729d..8aeb356047 100755 --- a/lib/python/qmk/xap/gen_firmware/inline_generator.py +++ b/lib/python/qmk/xap/gen_firmware/inline_generator.py @@ -125,10 +125,17 @@ def _append_routing_table_declaration(lines, container, container_id, route_stac def _append_routing_table_entry_flags(lines, container, container_id, route_stack): - is_secure = 1 if ('secure' in container and container['secure'] is True) else 0 + pem_map = { + None: 'ROUTE_PERMISSIONS_INSECURE', + 'secure': 'ROUTE_PERMISSIONS_SECURE', + 'ignore': 'ROUTE_PERMISSIONS_IGNORE', + } + + is_secure = pem_map[container.get('permissions', None)] + lines.append(' .flags = {') lines.append(f' .type = {_get_route_type(container)},') - lines.append(f' .is_secure = {is_secure},') + lines.append(f' .secure = {is_secure},') lines.append(' },') diff --git a/quantum/secure.c b/quantum/secure.c index 375f75fb00..7eab865b9f 100644 --- a/quantum/secure.c +++ b/quantum/secure.c @@ -27,10 +27,6 @@ secure_status_t secure_get_status(void) { return secure_status; } -bool secure_is_unlocking(void) { - return secure_status == SECURE_PENDING; -} - void secure_lock(void) { secure_status = SECURE_LOCKED; } diff --git a/quantum/secure.h b/quantum/secure.h index e1e3067c55..04507fd5b1 100644 --- a/quantum/secure.h +++ b/quantum/secure.h @@ -3,27 +3,65 @@ #pragma once +/** \file + * + * Exposes a set of functionality to act as a virtual padlock for your device + * ... As long as that padlock is made of paper and its currently raining. + */ + #include #include +/** \brief Available secure states + */ typedef enum { SECURE_LOCKED, SECURE_PENDING, SECURE_UNLOCKED, } secure_status_t; +/** \brief Query current secure state + */ secure_status_t secure_get_status(void); -bool secure_is_unlocking(void); +/** \brief Helper to check if unlocking is currently locked + */ +#define secure_is_locked() (secure_get_status() == SECURE_LOCKED) +/** \brief Helper to check if unlocking is currently in progress + */ +#define secure_is_unlocking() (secure_get_status() == SECURE_PENDING) + +/** \brief Helper to check if unlocking is currently unlocked + */ +#define secure_is_unlocked() (secure_get_status() == SECURE_UNLOCKED) + +/** \brief Lock down the device + */ void secure_lock(void); +/** \brief Force unlock the device + * + * \warning bypasses user unlock sequence + */ void secure_unlock(void); +/** \brief Begin listening for an unlock sequence + */ void secure_request_unlock(void); +/** \brief Flag to the secure subsystem that user activity has happened + * + * Call when some user activity has happened and the device should remain unlocked + */ void secure_activity_event(void); +/** \brief Flag to the secure subsystem that user has triggered a keypress + * + * Call to trigger processing of the unlock sequence + */ void secure_keypress_event(uint8_t row, uint8_t col); +/** \brief Handle various secure subsystem background tasks + */ void secure_task(void); diff --git a/quantum/xap/xap.c b/quantum/xap/xap.c index 61e0d4de30..d4b6eb80fa 100644 --- a/quantum/xap/xap.c +++ b/quantum/xap/xap.c @@ -43,9 +43,17 @@ typedef enum xap_route_type_t { #define XAP_ROUTE_TYPE_BIT_COUNT 3 +typedef enum xap_route_secure_t { + ROUTE_PERMISSIONS_INSECURE, + ROUTE_PERMISSIONS_SECURE, + ROUTE_PERMISSIONS_IGNORE, +} xap_route_secure_t; + +#define XAP_ROUTE_SECURE_BIT_COUNT 2 + typedef struct __attribute__((packed)) xap_route_flags_t { - xap_route_type_t type : XAP_ROUTE_TYPE_BIT_COUNT; - uint8_t is_secure : 1; + xap_route_type_t type : XAP_ROUTE_TYPE_BIT_COUNT; + xap_route_secure_t secure : XAP_ROUTE_SECURE_BIT_COUNT; } xap_route_flags_t; _Static_assert(TOTAL_XAP_ROUTE_TYPES <= (1 << (XAP_ROUTE_TYPE_BIT_COUNT)), "Number of XAP route types is too large for XAP_ROUTE_TYPE_BITS."); @@ -77,6 +85,24 @@ struct __attribute__((packed)) xap_route_t { #include +bool xap_pre_execute_route(xap_token_t token, const xap_route_t *route) { +#ifdef SECURE_ENABLE + if (!secure_is_unlocked() && (route->flags.secure == ROUTE_PERMISSIONS_SECURE)) { + xap_respond_failure(token, XAP_RESPONSE_FLAG_SECURE_FAILURE); + return true; + } + + if (secure_is_unlocking() && (route->flags.type != XAP_ROUTE) && (route->flags.secure != ROUTE_PERMISSIONS_IGNORE)) { + xap_respond_failure(token, XAP_RESPONSE_FLAG_UNLOCK_IN_PROGRESS); + return true; + } + + // TODO: XAP messages extend unlocked timeout? + secure_activity_event(); +#endif + return false; +} + void xap_execute_route(xap_token_t token, const xap_route_t *routes, size_t max_routes, const uint8_t *data, size_t data_len) { if (data_len == 0) return; xap_identifier_t id = data[0]; @@ -85,23 +111,10 @@ void xap_execute_route(xap_token_t token, const xap_route_t *routes, size_t max_ xap_route_t route; memcpy_P(&route, &routes[id], sizeof(xap_route_t)); -#ifdef SECURE_ENABLE - if (route.flags.is_secure && secure_get_status() != SECURE_UNLOCKED) { - xap_respond_failure(token, XAP_RESPONSE_FLAG_SECURE_FAILURE); + if (xap_pre_execute_route(token, &route)) { return; } - // TODO: All other subsystems are disabled during unlock. - // how to flag status route as still allowed? - // if (!route.flags.is_secure && secure_get_status() == SECURE_PENDING) { - // xap_respond_failure(token, XAP_RESPONSE_FLAG_UNLOCK_IN_PROGRESS); - // return; - // } - - // TODO: XAP messages extend timeout? - secure_activity_event(); -#endif - switch (route.flags.type) { case XAP_ROUTE: if (route.child_routes != NULL && route.child_routes_len > 0 && data_len > 0) {