All other subsystems are disabled during unlock

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:
                         '''

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])

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('        },')
 
 

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;
 }

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 <stdint.h>
 #include <stdbool.h>
 
+/** \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);

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 <xap_generated.inl>
 
+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) {