[riot-notifications] [RIOT-OS/RIOT] FIDO2 support in RIOT (#16489)

Peter Kietzmann notifications at github.com
Mon Jul 26 16:47:13 CEST 2021


@PeterKietzmann commented on this pull request.



> +    fido2_ctap_transport_hid_check_timeouts();
+}
+
+void fido2_ctap_transport_init(void)
+{
+    g_pid = thread_create(g_stack, sizeof(g_stack), THREAD_PRIORITY_MAIN,
+                          THREAD_CREATE_STACKTEST, pkt_loop, NULL,
+                          "fido2_ctap_transport_loop");
+}
+
+int fido2_ctap_transport_read_timeout(ctap_transport_type_t type, void *buffer,
+                                      size_t size, uint32_t timeout)
+{
+    switch (type) {
+    case USB:
+        return usb_hid_io_read_timeout(buffer, size, timeout);

This function is protected by `#ifdef MODULE_FIDO2_CTAP_HID` above but here it is not.

> +
+/**
+ * @brief USB stack
+ */
+static char g_usb_stack[USBUS_STACKSIZE];
+
+/**
+ * @brief USBUS context
+ */
+static usbus_t g_usbus;
+
+/**
+ * @brief hid_io.c function declarations
+ */
+size_t usb_hid_io_write(const void *buffer, size_t size);
+int usb_hid_io_read_timeout(void *buffer, size_t size, uint32_t timeout);

I am a bit confused. `usb_hid_io_read_timeout` isn't even used in *ctap_hid.c*

>      bool pin_is_set;                            /**< PIN is set or not */
     int rem_pin_att;                            /**< remaining PIN tries */
     uint8_t pin_hash[SHA256_DIGEST_LENGTH / 2]; /**< LEFT(SHA-256(pin), 16) */
     ctap_config_t config;                       /**< configuration of authenticator */
     uint16_t rk_amount_stored;                  /**< total number of resident keys stored on device */
     uint8_t cred_key[CTAP_CRED_KEY_LEN];        /**< AES CCM encryption key for cred */
+    bool cred_key_is_initialized;               /**< AES CCM key initialized flag */
 } ctap_state_t;

Just realized: is it on purpose to define this struct in the c file and not the header?

> +        else {
+            send_error_response(cid, CTAP_HID_ERR_CHANNEL_BUSY);
+            return;
+        }
+    }
+    else {
+        /* first init packet received starts a transaction */
+        if (is_init_type_pkt(pkt)) {
+            g_is_busy = true;
+            status = buffer_pkt(pkt);
+        }
+        /* ignore rest */
+    }
+
+    if (status == CTAP_HID_BUFFER_STATUS_ERROR) {
+        send_error_response(cid, g_state.err);

Referring to [this comment](https://github.com/RIOT-OS/RIOT/pull/16489#discussion_r676511702): This one does not miss a `return`?

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/RIOT-OS/RIOT/pull/16489#pullrequestreview-714921167
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.riot-os.org/pipermail/notifications/attachments/20210726/d4664e53/attachment-0001.htm>


More information about the notifications mailing list