Merge pull request #48 from All-Your-Locks-Are-Belong-To-Us/feature/f…

…unction-comments

Add function comments where missing

include/cbor.h

@@ -28,7 +28,7 @@ typedef int cbor_parse_map_item(const cb0r_t key, const cb0r_t value, void *data
  * @param cbor_map The map to iterate over.
  * @param cb The callback to call for each entry.
  * @param data User-supplied additional context data passed to the callback.
- * @return int FIDO_OK if map could be iterated completely.
+ * @return int FIDO_OK if the map could be iterated completely.
  */
 int cbor_iter_map(cb0r_t cbor_map, cbor_parse_map_item *cb, void *data);
 
@@ -38,7 +38,7 @@ int cbor_iter_map(cb0r_t cbor_map, cbor_parse_map_item *cb, void *data);
  * @param cbor_array The array to iterate over.
  * @param cb The callback to call for each entry.
  * @param data User-supplied additional context data passed to the callback.
- * @return int FIDO_OK if array could be iterated completely.
+ * @return int FIDO_OK if the array could be iterated completely.
  */
 int cbor_iter_array(cb0r_t cbor_array, cbor_parse_array_item *cb, void* data);
 
@@ -88,7 +88,7 @@ void cbor_writer_reset(cbor_writer_t writer, uint8_t* buffer, const size_t buffe
  * @brief Check a writer's status.
  *
  * @param writer The writer to check.
- * @return true iff no error occurred and writer->buffer can be used.
+ * @return true if no error occurred and writer->buffer can be used.
  */
 bool cbor_writer_is_ok(cbor_writer_t writer);
 

include/crypto.h

@@ -1,3 +1,10 @@
+/*
+ * Copyright (c) 2022 Felix Gohla, Konrad Hanff, Tobias Kantusch,
+ *                    Quentin Kuth, Felix Roth. All rights reserved.
+ *
+ * Use of this source code is governed by a BSD-style
+ * license that can be found in the LICENSE file.
+ */
 #pragma once
 
 #include <stdint.h>

include/dev.h

@@ -80,7 +80,7 @@ void fido_dev_set_transport(fido_dev_t *dev, const fido_dev_transport_t *transpo
  * Initializes the connection and makes it ready for communication.
  *
  * @param dev A pointer to the FIDO device to be opened.
- * @return intFIDO_OK when the operation was successful.
+ * @return intFIDO_OK if the operation was successful.
  */
 int fido_dev_open(fido_dev_t *dev);
 
@@ -90,7 +90,7 @@ int fido_dev_open(fido_dev_t *dev);
  * Closes the connection to the device.
  *
  * @param dev A pointer to the FIDO device to be closed.
- * @return int FIDO_OK when the operation was successful.
+ * @return int FIDO_OK if the operation was successful.
  */
 int fido_dev_close(fido_dev_t *dev);
 

include/internal.h

@@ -13,7 +13,28 @@
 
 #include "dev.h"
 
+/**
+ * @brief Ensure that the device is correctly set up to receive data and
+ *        call the device's transport receive function.
+ * 
+ * @param d A pointer to the FIDO device.
+ * @param cmd The CTAP command to receive data from.
+ * @param buf A pointer to the destination buffer.
+ * @param len The size of the destination buffer.
+ * @return int FIDO_OK if the read operation was successful.
+ */
 int fido_rx(fido_dev_t *d, const uint8_t cmd, void *buf, const size_t len);
+
+/**
+ * @brief Ensure that the device is correctly set up to transmit data and
+ *        call the device's transport transmit function.
+ * 
+ * @param d A pointer to the FIDO device.
+ * @param cmd The CTAP command to transmit.
+ * @param buf A pointer to the source buffer.
+ * @param len The size of the source buffer.
+ * @return int FIDO_OK if the write operation was successful.
+ */
 int fido_tx(fido_dev_t *d, const uint8_t cmd, const void *buf, const size_t len);
 int fido_get_random(void *buf, size_t len);
 
@@ -25,7 +46,7 @@ int fido_get_random(void *buf, size_t len);
  * @param len The length of the memory area pointed to by buf.
  * @param dst A pointer to the destination buffer.
  * @param count The number of bytes to read.
- * @return int FIDO_OK when the operation was successful.
+ * @return int FIDO_OK if the operation was successful.
  */
 int fido_buf_read(const unsigned char **buf, size_t *len, void *dst, size_t count);
 
@@ -37,6 +58,6 @@ int fido_buf_read(const unsigned char **buf, size_t *len, void *dst, size_t coun
  * @param len The length of the memory area pointed to by buf.
  * @param src The source buffer.
  * @param count The number of bytes to write.
- * @return int FIDO_OK when the operation was successful.
+ * @return int FIDO_OK if the operation was successful.
  */
 int fido_buf_write(unsigned char **buf, size_t *len, const void *src, size_t count);

src/assertion.c

@@ -112,6 +112,15 @@ static const uint8_t KEY_TYPE[] PROGMEM_MARKER = "type";
 static const uint8_t KEY_TYPE_PUBLIC_KEY[] PROGMEM_MARKER = "public-key";
 static const uint8_t KEY_ID[] PROGMEM_MARKER = "id";
 
+/**
+ * @brief CBOR decode the credential's data such as its type and ID.
+ *        See https://w3c.github.io/webauthn/#dictdef-publickeycredentialdescriptor
+ *
+ * @param key The CBOR key.
+ * @param assert The CBOR value.
+ * @param arg The assert reply argument to store the parsed data to.
+ * @return int FIDO_OK if the operation was successful.
+ */
 static int cbor_assert_decode_credential(const cb0r_t key, const cb0r_t value, void *arg) {
     if (!cbor_utf8string_is_definite(key)) {
         // Just ignore the entry according to https://fidoalliance.org/specs/fido-v2.1-ps-20210615/fido-client-to-authenticator-protocol-v2.1-ps-20210615.html#message-encoding.
@@ -142,7 +151,14 @@ static int cbor_assert_decode_credential(const cb0r_t key, const cb0r_t value, v
     return FIDO_OK;
 }
 
-// See https://www.w3.org/TR/webauthn-2/#authenticator-data
+/**
+ * @brief CBOR decode the auth data such as the RP ID hash and signature count.
+ *        See https://www.w3.org/TR/webauthn-2/#authenticator-data
+ *
+ * @param auth_data_raw The raw auth data.
+ * @param ca The reply entry to store the parsed data to.
+ * @return int FIDO_OK if the operation was successful.
+ */
 static int cbor_assert_decode_auth_data_inner(void* auth_data_raw, fido_assert_reply_t *ca) {
     uint8_t* auth_data_bytes = (uint8_t*) auth_data_raw;
 
@@ -163,6 +179,13 @@ static int cbor_assert_decode_auth_data_inner(void* auth_data_raw, fido_assert_r
     return FIDO_OK;
 }
 
+/**
+ * @brief Wrapper to decode the CBOR encoded authentication data.
+ *
+ * @param auth_data The CBOR encoded authentication data.
+ * @param ca The reply entry to store the parsed data to.
+ * @return int FIDO_OK if the operation was successful.
+ */
 static int cbor_assert_decode_auth_data(const cb0r_t auth_data, fido_assert_reply_t *ca) {
     if (!cbor_bytestring_is_definite(auth_data)) {
         return FIDO_ERR_CBOR_UNEXPECTED_TYPE;
@@ -177,6 +200,13 @@ static int cbor_assert_decode_auth_data(const cb0r_t auth_data, fido_assert_repl
     return cbor_assert_decode_auth_data_inner(ca->auth_data_raw, ca);
 }
 
+/**
+ * @brief Decode the assertion signature from the CBOR entry.
+ *
+ * @param signature The CBOR encoded signature data.
+ * @param ca The reply entry to store the parsed data to.
+ * @return int FIDO_OK if the operation was successful.
+ */
 static int cbor_assert_decode_signature(const cb0r_t signature, fido_assert_reply_t *ca) {
     if (!cbor_bytestring_is_definite(signature)) {
         return FIDO_ERR_CBOR_UNEXPECTED_TYPE;
@@ -189,6 +219,13 @@ static int cbor_assert_decode_signature(const cb0r_t signature, fido_assert_repl
     return FIDO_OK;
 }
 
+/**
+ * @brief Decode the large blob key from the CBOR entry.
+ *
+ * @param large_blob_key The CBOR encoded signature data.
+ * @param ca The reply entry to store the parsed data to.
+ * @return int FIDO_OK if the operation was successful.
+ */
 static int cbor_assert_decode_large_blob_key(const cb0r_t large_blob_key, fido_assert_reply_t *ca) {
     if (!cbor_bytestring_is_definite(large_blob_key)) {
         return FIDO_ERR_CBOR_UNEXPECTED_TYPE;
@@ -208,7 +245,7 @@ static int cbor_assert_decode_large_blob_key(const cb0r_t large_blob_key, fido_a
  * @param key The cb0r element representing the map key
  * @param value The cb0r element representing the map value
  * @param arg User-passed argument (here: assertion reply).
- * @return int FIDO_OK if entry could be parsed.
+ * @return int FIDO_OK if the entry could be parsed.
  */
 static int parse_get_assert_reply_entry(const cb0r_t key, const cb0r_t value, void *arg) {
     if (key->type != CB0R_INT || key->value > UINT8_MAX) {
@@ -239,13 +276,16 @@ static int parse_get_assert_reply_entry(const cb0r_t key, const cb0r_t value, vo
     }
 }
 
-// TODO: function to set client data (and compute hash)
-
+/**
+ * @brief Transmit the request data to the authenticator.
+ *
+ * @param dev The device to communicate to.
+ * @param assert The assertion request data.
+ * @return int FIDO_OK if the operation was successful.
+ */
 static int fido_dev_get_assert_tx(
     fido_dev_t *dev,
-    fido_assert_t *assert,
-    const es256_pk_t *pk,
-    const fido_blob_t *ecdh
+    fido_assert_t *assert
 ) {
     // We do not know the size of the command buffer, yet, as extensions can have different length.
     // So we start with a sane value and try our luck until the buffer is large enough.
@@ -287,6 +327,14 @@ static int fido_dev_get_assert_tx(
     return ret;
 }
 
+/**
+ * @brief Receive the response data from the authenticator and parse the authenticator's response into the reply.
+ *
+ * @param dev The device to communicate to.
+ * @param assert The assertion request data.
+ * @param reply A pointer to the structure to store the parsed data to.
+ * @return int FIDO_OK if the operation was successful.
+ */
 static int fido_dev_get_assert_rx(
     fido_dev_t *dev,
     fido_assert_t *assert,
@@ -319,16 +367,22 @@ static int fido_dev_get_assert_rx(
     return ret;
 }
 
+/**
+ * @brief Perform the assertion transmission and receival and wait for their completion.
+ *
+ * @param dev The device to communicate to.
+ * @param assert The assertion request data.
+ * @param reply A pointer to the structure to store the parsed data to.
+ * @return int 0, if the check is successful
+ */
 static int fido_dev_get_assert_wait(
     fido_dev_t *dev,
     fido_assert_t *assert,
-    const es256_pk_t *pk,
-    const fido_blob_t *ecdh,
     fido_assert_reply_t *reply
 ) {
     int r;
 
-    if ((r = fido_dev_get_assert_tx(dev, assert, pk, ecdh)) != FIDO_OK ||
+    if ((r = fido_dev_get_assert_tx(dev, assert)) != FIDO_OK ||
         (r = fido_dev_get_assert_rx(dev, assert, reply)) != FIDO_OK)
         return (r);
 
@@ -349,8 +403,6 @@ void fido_assert_reset(fido_assert_t *assert) {
 }
 
 int fido_dev_get_assert(fido_dev_t *dev, fido_assert_t *assert) {
-    fido_blob_t    *ecdh    = NULL;
-    es256_pk_t     *pk      = NULL;
     int             r;
 
     if (assert->rp_id.ptr == NULL) {
@@ -367,7 +419,7 @@ int fido_dev_get_assert(fido_dev_t *dev, fido_assert_t *assert) {
     }
 
     fido_assert_reply_reset(&assert->reply);
-    r = fido_dev_get_assert_wait(dev, assert, pk, ecdh, &assert->reply);
+    r = fido_dev_get_assert_wait(dev, assert, &assert->reply);
 
     return r;
 }
@@ -395,6 +447,13 @@ void fido_assert_set_extensions(fido_assert_t *assert, const fido_assert_ext_t e
     assert->ext = extensions;
 }
 
+/**
+ * @brief Check, that user presence or verification are performed successfully, when desired.
+ *
+ * @param auth_data_flags The flags retrieved in the auth_data from the authenticator.
+ * @param assert_opt The user defined options.
+ * @return int 0, if the check is successful
+ */
 static int fido_check_flags(fido_assert_auth_data_flags_t auth_data_flags, fido_assert_opt_t assert_opt) {
     int up = assert_opt & FIDO_ASSERT_OPTION_UP;
     int uv = assert_opt & FIDO_ASSERT_OPTION_UV;
@@ -413,6 +472,13 @@ static int fido_check_flags(fido_assert_auth_data_flags_t auth_data_flags, fido_
     return (0);
 }
 
+/**
+ * @brief Ensure that the hash of the relying party concurs with the expected one.
+ *
+ * @param rp_id The expected relying party id.
+ * @param obtained_hash The hash of the relying party id obtained from the authenticator.
+ * @return int 0, if the hash is correct
+ */
 static int fido_check_rp_id(const fido_assert_blob_t *rp_id, const uint8_t *obtained_hash) {
     uint8_t expected_hash[ASSERTION_AUTH_DATA_RPID_HASH_LEN] = {0};
     if(fido_sha256 == NULL) {

src/cbor.c

@@ -96,13 +96,27 @@ bool cbor_writer_is_ok(cbor_writer_t writer) {
     return writer->status == CBOR_WRITER_OK;
 }
 
+/**
+ * @brief Return whether the CBOR writer can advance.
+ *
+ * @param writer The CBOR writer object.
+ * @param count The amount of bytes to advance the writer
+ * @return bool whether the writer can advance
+ */
 static bool cbor_writer_can_advance(cbor_writer_t writer, const size_t count) {
     if(count > writer->buffer_len - writer->length || writer->status != CBOR_WRITER_OK) {
         return false;
     }
     return true;
 }
 
+/**
+ * @brief Ensure that the writer can advance and advance if possible. Otherwise set the writer status.
+ *
+ * @param writer The CBOR writer object.
+ * @param count The amount of bytes to advance the writer
+ * @return bool whether the writer can advance
+ */
 static void cbor_writer_advance(cbor_writer_t writer, const size_t count) {
     if(cbor_writer_can_advance(writer, count)) {
         writer->length += count;
@@ -112,6 +126,14 @@ static void cbor_writer_advance(cbor_writer_t writer, const size_t count) {
     }
 }
 
+/**
+ * @brief Write a CBOR value using the writer.
+ *
+ * @param writer The CBOR writer object.
+ * @param type The CBOR type of the data to write.
+ * @param value The value to write.
+ * @return size_t the amount of bytes written.
+ */
 static size_t cbor_write(cbor_writer_t writer, cb0r_e type, const uint64_t value) {
     size_t encoded_len = cbor_encoded_len(value);
     if(!cbor_writer_can_advance(writer, encoded_len)) {