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

Peter Kietzmann notifications at github.com
Tue Jul 20 15:06:16 CEST 2021


@PeterKietzmann commented on this pull request.

> Probably need to test with UP turned on. This would be a real pain though.

Why, because you have to press the button repeatedly? In that case,  we might consider executing the tests on our HiL rack @MrKevinWeiss

> +
+#include "fido2/ctap/transport/ctap_transport.h"
+#include "fido2/ctap.h"
+#include "fido2/ctap/ctap_utils.h"
+#include "fido2/ctap/ctap_cbor.h"
+#include "fido2/ctap/ctap_mem.h"
+
+#define ENABLE_DEBUG    (0)
+#include "debug.h"
+
+/*** main CTAP2 functions ***/
+
+/**
+ * @brief MakeCredential method
+ *
+ * CTAP specification (version 20190130) section 5.1

Still not addressed. What about all the return codes in *ctap_status.h*?

> +    load_state(&g_state);
+
+    ret = fido2_ctap_crypto_init();
+
+    if (ret != CTAP2_OK) {
+        DEBUG("fido2_ctap: initialization failed \n");
+        return -1;
+    }
+
+    /* first startup up of the device */
+    if (g_state.initialized != CTAP_INITIALIZED_MARKER) {
+        reset();
+    }
+
+    /* initialize pin_token */
+    fido2_ctap_crypto_prng(g_pin_token, sizeof(g_pin_token));

You are providing an abstraction layer here. The abstraction layer shouldn't copy the mistakes from the layer below. I am asking to introduce proper return values for the API. For the current `random_bytes` function there is not much more that you can do (right now).

> +    DEBUG("fido2_ctap: initialization successful \n");
+
+    return 0;
+}
+
+static void reset(void)
+{
+    g_state.initialized = CTAP_INITIALIZED_MARKER;
+    g_state.rem_pin_att = CTAP_PIN_MAX_ATTS;
+    g_state.pin_is_set = false;
+    g_state.rk_amount_stored = 0;
+    g_state.sign_count = 0;
+
+    g_rem_pin_att_boot = CTAP_PIN_MAX_ATTS_BOOT;
+
+    fido2_ctap_crypto_prng(g_state.cred_key, sizeof(g_state.cred_key));

So that means it is expected that "factory default state" has a newly created key available, or **why** do you generate a new key in this `reset` function?

> + * General Public License v2.1. See the file LICENSE in the top level
+ * directory for more details.
+ */
+
+/**
+ * @defgroup    fido2 FIDO2 - Fast Identity Online 2
+ * @ingroup     sys
+ *
+ * FIDO2 is an authentication standard that seeks to solve the password problem
+ * by enabling passwordless authentication. Instead of using passwords to
+ * authenticate to web services, FIDO2 enables users to use common devices
+ * (authenticators) to create cryptographic credentials which are then used
+ * for authentication. FIDO2 consists of the W3C Web Authentication
+ * specification (WebAuthn) and the Client to Authenticator Protocol (CTAP).
+ *
+ * This code implements the FIDO2 CTAP protocol.

Just an overview of the "general entities"

-- 
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-710538606
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.riot-os.org/pipermail/notifications/attachments/20210720/4408d795/attachment-0001.htm>


More information about the notifications mailing list