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

Nils Ollrogge notifications at github.com
Wed Jun 16 18:31:52 CEST 2021

@Ollrogge commented on this pull request.

> +
+    /* sha256 of shared secret ((abG).x) to obtain shared key */
+    sha256(shared_secret, sizeof(shared_secret), shared_key);
+    hmac_sha256_init(&ctx, shared_key, sizeof(shared_key));
+    hmac_sha256_update(&ctx, req->new_pin_enc, req->new_pin_enc_size);
+    hmac_sha256_update(&ctx, req->pin_hash_enc, sizeof(req->pin_hash_enc));
+    hmac_sha256_final(&ctx, hmac);
+    if (memcmp(hmac, req->pin_auth, 16) != 0) {
+        DEBUG("fido2_ctap: pin hmac and pin_auth differ \n");
+        return CTAP2_ERR_PIN_AUTH_INVALID;
+    }
+    sz = sizeof(pin_hash_dec);
+    ret = fido2_ctap_crypto_aes_dec(pin_hash_dec, &sz, req->pin_hash_enc,

I used hashes directly because no crypto related variables need to be initialized. For e.g. `fido2_ctap_crypto_ecdh` I need to initialize the curve or for `fido2_ctap_crypto_aes_enc` the IV. Therefore these functions have an abstraction. On the other hand I currently also have an abstraction with `fido2_ctap_crypto_prng` that does not initialize any crypto related variables. I see your point that this has no real structure.

Generally the purpose of all the helpers (`ctap_cbor.c`, `ctap_mem.c`, `ctap_crypto.c`, `ctap_utils.c`) is to split pure CTAP logic from other logic. Therefore my goto solution would be to rather also add an abstraction function for hashes, than removing `ctap_crypto` entirely and doing everything inside `ctap.c`.

You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.riot-os.org/pipermail/notifications/attachments/20210616/c6f7b714/attachment.htm>

More information about the notifications mailing list