From c0b7c3d5962ce2d05c0babfe74b692e244bda170 Mon Sep 17 00:00:00 2001 From: Tobias Kantusch Date: Wed, 31 Aug 2022 15:42:32 +0200 Subject: [PATCH 1/4] Add function comments where missing This adds a lot of function comments whereever they were missing, mostly on static functions. --- include/internal.h | 21 ++++++++++ src/assertion.c | 89 ++++++++++++++++++++++++++++++++++++------ src/cbor.c | 22 +++++++++++ src/dev.c | 33 ++++++++++++++++ src/largeblob.c | 25 ++++++++++++ src/nfc.c | 96 +++++++++++++++++++++++++++++++++++++++++----- 6 files changed, 265 insertions(+), 21 deletions(-) diff --git a/include/internal.h b/include/internal.h index 178ef28..ec27e74 100644 --- a/include/internal.h +++ b/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 when the 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 when the 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); diff --git a/src/assertion.c b/src/assertion.c index 99c5d90..19a4475 100644 --- a/src/assertion.c +++ b/src/assertion.c @@ -112,6 +112,14 @@ 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. + * + * @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 performed successfully + */ 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 +150,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 performed successfully + */ 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 +178,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 performed successfully + */ 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 +199,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 performed successfully + */ 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 +218,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 performed successfully + */ 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; @@ -239,13 +275,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 performed successfully + */ 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 +326,14 @@ static int fido_dev_get_assert_tx( return ret; } +/** + * @brief Receive the response data from the authenticator and parse the CBOR 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 performed successfully + */ static int fido_dev_get_assert_rx( fido_dev_t *dev, fido_assert_t *assert, @@ -319,16 +366,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 +402,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 +418,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 +446,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 +471,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) { diff --git a/src/cbor.c b/src/cbor.c index e6e8dc7..a17776b 100644 --- a/src/cbor.c +++ b/src/cbor.c @@ -96,6 +96,13 @@ 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; @@ -103,6 +110,13 @@ static bool cbor_writer_can_advance(cbor_writer_t writer, const size_t count) { 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)) { diff --git a/src/dev.c b/src/dev.c index 53a1f29..a38aa38 100644 --- a/src/dev.c +++ b/src/dev.c @@ -14,6 +14,12 @@ static int nonce = 1234; // The only cryptographically secure nonce +/** + * @brief Store extensions received from the authenticator in the device object. + * + * @param dev The FIDO device to store the attributes in. + * @param info The parsed info received from the authenticator. + */ static void fido_dev_set_extension_flags(fido_dev_t *dev, const fido_cbor_info_t *info) { if (info->extensions & FIDO_EXTENSION_CRED_PROTECT) { dev->flags |= FIDO_DEV_CRED_PROT; @@ -23,6 +29,12 @@ static void fido_dev_set_extension_flags(fido_dev_t *dev, const fido_cbor_info_t } } +/** + * @brief Store options received from the authenticator in the device object. + * + * @param dev The FIDO device to store the attributes in. + * @param info The parsed info received from the authenticator. + */ static void fido_dev_set_option_flags(fido_dev_t *dev, const fido_cbor_info_t *info) { if(info->options & FIDO_OPTION_CLIENT_PIN) { dev->flags |= FIDO_DEV_PIN_SET; @@ -41,6 +53,12 @@ static void fido_dev_set_option_flags(fido_dev_t *dev, const fido_cbor_info_t *i } } +/** + * @brief Store PIN protocols received from the authenticator in the device object. + * + * @param dev The FIDO device to store the attributes in. + * @param info The parsed info received from the authenticator. + */ static void fido_dev_set_protocol_flags(fido_dev_t *dev, const fido_cbor_info_t *info) { if(info->protocols & FIDO_PIN_PROTOCOL_1){ dev->flags |= FIDO_DEV_PIN_PROTOCOL_1; @@ -50,6 +68,15 @@ static void fido_dev_set_protocol_flags(fido_dev_t *dev, const fido_cbor_info_t } } +/** + * @brief Store several flags such as extensions received in the info object in the device for later use. + * + * This allows that we don't have to keep the whole info object and only keep the data necessary to identify + * the authenticators features later on. + * + * @param dev The FIDO device to store the attributes in. + * @param info The parsed info received from the authenticator. + */ static void fido_dev_set_flags(fido_dev_t *dev, const fido_cbor_info_t *info) { fido_dev_set_extension_flags(dev, info); fido_dev_set_option_flags(dev, info); @@ -128,6 +155,12 @@ static int fido_dev_open_tx(fido_dev_t *dev) { return (r); } +/** + * @brief Open a device and ensure that is a FIDO one by receiving an initialization response. + * + * @param dev + * @return int A FIDO_ERR + */ static int fido_dev_open_rx(fido_dev_t *dev) { fido_cbor_info_t info = { 0 }; int reply_len; diff --git a/src/largeblob.c b/src/largeblob.c index e236287..432da85 100644 --- a/src/largeblob.c +++ b/src/largeblob.c @@ -278,6 +278,15 @@ typedef struct largeblob_array_entry { uint64_t origSize; } largeblob_array_entry_t; +/** + * @brief Uncompress the compressed data using tinf and store it in the provided buffer. + * + * @param out Pointer to a buffer to store the uncompressed data to. + * @param compressed Pointer to the compressed data. + * @param compressed_len Length of the compressed data. + * @param uncompressed_len_expected Expected length of the uncompressed data, to ensure that we got everything. + * @return int FIDO_OK when the operation was successful. + */ static int fido_uncompress(fido_blob_t* out, uint8_t *compressed, size_t compressed_len, size_t uncompressed_len_expected) { uint32_t uncompressed_len_actual = out->max_length; if(out->max_length < uncompressed_len_expected) { @@ -291,6 +300,14 @@ static int fido_uncompress(fido_blob_t* out, uint8_t *compressed, size_t compres return FIDO_OK; } +/** + * @brief Parse an entry in the largeblob array. + * + * @param key The CBOR key value of the entry. + * @param value The CBOR encoded value. + * @param data The largeblob array entry object to store the parsed data to. + * @return int FIDO_OK when the operation was successful. + */ static int largeblob_parse_array_entry(cb0r_t key, cb0r_t value, void *data) { largeblob_array_entry_t *entry = (largeblob_array_entry_t*) data; @@ -339,6 +356,14 @@ static int largeblob_parse_array_entry(cb0r_t key, cb0r_t value, void *data) { } } +/** + * @brief Iterate the largeblob array and check if we find an entry that matches the expected key, + * uncompress the data if we find an entry. + * + * @param value The CBOR encoded value. + * @param data The largeblob array lookup parameters. This also contains the buffer where we write the uncompressed data to. + * @return int FIDO_OK when the operation was successful. + */ static int largeblob_array_lookup(cb0r_t value, void* data) { largeblob_array_lookup_param_t *param = (largeblob_array_lookup_param_t*) data; largeblob_array_entry_t entry; diff --git a/src/nfc.c b/src/nfc.c index 19318eb..36383e4 100644 --- a/src/nfc.c +++ b/src/nfc.c @@ -19,6 +19,14 @@ static const uint8_t aid[] = { 0xa0, 0x00, 0x00 static const uint8_t fido_version_u2f[] PROGMEM_MARKER = "U2F_V2"; static const uint8_t fido_version_fido2[] PROGMEM_MARKER = "FIDO_2_0"; +/** + * @brief Receive the data from the CTAP init command. + * + * @param dev The device to receive data from. + * @param buf The buffer to write the received data to. + * @param len The length of the buffer. + * @return int FIDO_OK when the operation was successful. + */ static int rx_init(fido_dev_t *dev, unsigned char *buf, const size_t len) { fido_ctap_info_t *attr = (fido_ctap_info_t *)buf; @@ -54,11 +62,20 @@ static int rx_init(fido_dev_t *dev, unsigned char *buf, const size_t len) return (int)len; } -static int rx_apdu(fido_dev_t *d, uint8_t sw[2], unsigned char **buf, size_t *count) { +/** + * @brief Receive an NFC APDU. + * + * @param dev The device to receive data from. + * @param sw The buffer to write the status word to. + * @param buf A pointer to a pointer to a buffer where the received data is located. The pointer to the buffer will be advanced. + * @param count The remaining length of the buffer, will be reduced. + * @return int FIDO_OK when the operation was successful. + */ +static int rx_apdu(fido_dev_t *dev, uint8_t sw[2], unsigned char **buf, size_t *count) { uint8_t f[256 + 2]; int n, ok = -1; - if ((n = d->io.read(d->io_handle, f, sizeof(f))) < 2) { + if ((n = dev->io.read(dev->io_handle, f, sizeof(f))) < 2) { fido_log_debug("%s: read", __func__); goto fail; } @@ -77,7 +94,14 @@ static int rx_apdu(fido_dev_t *d, uint8_t sw[2], unsigned char **buf, size_t *co return ok; } -static int tx_get_response(fido_dev_t *d, uint8_t count) +/** + * @brief Transmit a GET_RESPONSE APDU. + * + * @param dev The device to transmit data to. + * @param count The amount of data expected to receive. + * @return int FIDO_OK when the operation was successful. + */ +static int tx_get_response(fido_dev_t *dev, uint8_t count) { uint8_t apdu[5]; @@ -85,7 +109,7 @@ static int tx_get_response(fido_dev_t *d, uint8_t count) apdu[1] = 0xc0; /* GET_RESPONSE */ apdu[4] = count; - if (d->io.write(d->io_handle, apdu, sizeof(apdu)) < 0) { + if (dev->io.write(dev->io_handle, apdu, sizeof(apdu)) < 0) { fido_log_debug("%s: write", __func__); return FIDO_ERR_TX; } @@ -93,9 +117,18 @@ static int tx_get_response(fido_dev_t *d, uint8_t count) return FIDO_OK; } -static int rx_msg(fido_dev_t *dev, unsigned char *buf, const size_t bufsiz) { +/** + * @brief Receive a complete message from the authenticator. + * This includes logic for receiving NFC frames until no more data is available. + * + * @param dev The device to receive data from. + * @param buf The buffer to write the received data to. + * @param len The length of the buffer. + * @return int The amount of bytes received. + */ +static int rx_msg(fido_dev_t *dev, unsigned char *buf, const size_t len) { uint8_t sw[2]; - size_t count = bufsiz; + size_t count = len; if (rx_apdu(dev, sw, &buf, &count) < 0) { fido_log_debug("%s: preamble", __func__); @@ -116,14 +149,24 @@ static int rx_msg(fido_dev_t *dev, unsigned char *buf, const size_t bufsiz) { } // TODO bufsiz - count > INT_MAX - if (bufsiz < count) { - fido_log_debug("%s: bufsiz", __func__); + if (len < count) { + fido_log_debug("%s: len", __func__); return FIDO_ERR_RX; } - return (int)(bufsiz - count); + return (int)(len - count); } +/** + * @brief Receive the CBOR message from the authenticator. + * This removes the status word (2 bytes) from the received bytes such that only the CBOR + * encoded message remains. + * + * @param dev The device to receive data from. + * @param buf The buffer to write the received data to. + * @param len The length of the buffer. + * @return int FIDO_OK when the operation was successful. + */ static int rx_cbor(fido_dev_t *dev, unsigned char *buf, const size_t count) { int r; @@ -133,6 +176,15 @@ static int rx_cbor(fido_dev_t *dev, unsigned char *buf, const size_t count) { return r - 2; } +/** + * @brief Receive data from an NFC device according to the executed CTAP command. + * + * @param dev The device to receive data from. + * @param cmd The CTAP command that was executed. + * @param buf The buffer to write the response to. + * @param len The length of the buffer. + * @return int FIDO_OK when the operation was successful. + */ static int nfc_rx(struct fido_dev *dev, const uint8_t cmd, unsigned char *buf, const size_t len) { switch (cmd) { case CTAP_CMD_INIT: @@ -147,6 +199,16 @@ static int nfc_rx(struct fido_dev *dev, const uint8_t cmd, unsigned char *buf, c } } +/** + * @brief Transmit a short ISO7816 APDU. + * + * @param dev The device to receive data from. + * @param h The ISO7816 header to send. + * @param payload The payload to send. + * @param payload_len The length of the payload. + * @param cla_flags The ISO7816 class flags to use. + * @return int FIDO_OK when the operation was successful. + */ static int tx_short_apdu( fido_dev_t *dev, const iso7816_header_t *h, @@ -193,6 +255,13 @@ static int tx_short_apdu( return ok; } +/** + * @brief Transmit a complete ISO7816 APDU. Currently this is implemented through repeated short APDUs. + * + * @param dev The device to receive data from. + * @param apdu The ISO7816 APDU to send. + * @return int FIDO_OK when the operation was successful. + */ static int nfc_do_tx(fido_dev_t *dev, const iso7816_apdu_t *apdu) { uint16_t apdu_len = apdu->payload_len; @@ -215,6 +284,15 @@ static int nfc_do_tx(fido_dev_t *dev, const iso7816_apdu_t *apdu) { return FIDO_OK; } +/** + * @brief Transmit an ISO7816 frame according to the desired CTAP command. + * + * @param dev The device to transmit data to. + * @param cmd The CTAP command. + * @param buf The payload to send. + * @param len The length of the payload. + * @return int FIDO_OK when the operation was successful. + */ static int nfc_tx(struct fido_dev *dev, const uint8_t cmd, const unsigned char *buf, const size_t len) { iso7816_apdu_t apdu; int status = FIDO_ERR_TX; From cbac0299e899bd238b7d9e44fa79f21237af545a Mon Sep 17 00:00:00 2001 From: frcroth Date: Wed, 31 Aug 2022 16:02:31 +0200 Subject: [PATCH 2/4] Unify return FIDO_OK in function comments --- include/cbor.h | 6 +++--- include/dev.h | 4 ++-- include/internal.h | 8 ++++---- src/assertion.c | 16 ++++++++-------- src/dev.c | 8 ++++---- src/info.c | 18 +++++++++--------- src/largeblob.c | 10 +++++----- src/nfc.c | 16 ++++++++-------- 8 files changed, 43 insertions(+), 43 deletions(-) diff --git a/include/cbor.h b/include/cbor.h index f1e5e4a..e5623e5 100644 --- a/include/cbor.h +++ b/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); diff --git a/include/dev.h b/include/dev.h index 3c654e3..adae84f 100644 --- a/include/dev.h +++ b/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); diff --git a/include/internal.h b/include/internal.h index ec27e74..c3ffe9f 100644 --- a/include/internal.h +++ b/include/internal.h @@ -21,7 +21,7 @@ * @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 when the operation was successful. + * @return int FIDO_OK if the operation was successful. */ int fido_rx(fido_dev_t *d, const uint8_t cmd, void *buf, const size_t len); @@ -33,7 +33,7 @@ int fido_rx(fido_dev_t *d, const uint8_t cmd, void *buf, const size_t len); * @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 when the operation was successful. + * @return int FIDO_OK if the 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); @@ -46,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); @@ -58,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); diff --git a/src/assertion.c b/src/assertion.c index 19a4475..f091ea9 100644 --- a/src/assertion.c +++ b/src/assertion.c @@ -118,7 +118,7 @@ static const uint8_t KEY_ID[] PROGMEM_MARKER = "id"; * @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 performed successfully + * @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)) { @@ -156,7 +156,7 @@ static int cbor_assert_decode_credential(const cb0r_t key, const cb0r_t value, v * * @param auth_data_raw The raw auth data. * @param ca The reply entry to store the parsed data to. - * @return int FIDO_OK if performed successfully + * @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; @@ -183,7 +183,7 @@ static int cbor_assert_decode_auth_data_inner(void* auth_data_raw, fido_assert_r * * @param auth_data The CBOR encoded authentication data. * @param ca The reply entry to store the parsed data to. - * @return int FIDO_OK if performed successfully + * @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)) { @@ -204,7 +204,7 @@ static int cbor_assert_decode_auth_data(const cb0r_t auth_data, fido_assert_repl * * @param signature The CBOR encoded signature data. * @param ca The reply entry to store the parsed data to. - * @return int FIDO_OK if performed successfully + * @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)) { @@ -223,7 +223,7 @@ static int cbor_assert_decode_signature(const cb0r_t signature, fido_assert_repl * * @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 performed successfully + * @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)) { @@ -244,7 +244,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) { @@ -280,7 +280,7 @@ static int parse_get_assert_reply_entry(const cb0r_t key, const cb0r_t value, vo * * @param dev The device to communicate to. * @param assert The assertion request data. - * @return int FIDO_OK if performed successfully + * @return int FIDO_OK if the operation was successful. */ static int fido_dev_get_assert_tx( fido_dev_t *dev, @@ -332,7 +332,7 @@ static int fido_dev_get_assert_tx( * @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 performed successfully + * @return int FIDO_OK if the operation was successful. */ static int fido_dev_get_assert_rx( fido_dev_t *dev, diff --git a/src/dev.c b/src/dev.c index a38aa38..571a24d 100644 --- a/src/dev.c +++ b/src/dev.c @@ -114,8 +114,8 @@ bool fido_dev_is_fido(fido_dev_t *dev) { /** * @brief Open a FIDO device sending an initialization command. * - * @param dev - * @return int A FIDO_ERR + * @param dev The FIDO device to open. + * @return int FIDO_OK if the operation was successful. */ static int fido_dev_open_tx(fido_dev_t *dev) { int r; @@ -158,8 +158,8 @@ static int fido_dev_open_tx(fido_dev_t *dev) { /** * @brief Open a device and ensure that is a FIDO one by receiving an initialization response. * - * @param dev - * @return int A FIDO_ERR + * @param dev The FIDO device to open. + * @return int FIDO_OK if the operation was successful. */ static int fido_dev_open_rx(fido_dev_t *dev) { fido_cbor_info_t info = { 0 }; diff --git a/src/info.c b/src/info.c index b7c1310..f839fd8 100644 --- a/src/info.c +++ b/src/info.c @@ -66,7 +66,7 @@ void fido_cbor_info_reset(fido_cbor_info_t *ci) { * * @param value The value containing the AAGUID. * @param ci The info to copy the ID to. - * @return int FIDO_OK when the operation was successful. + * @return int FIDO_OK if the operation was successful. */ static int copy_aaguid(const cb0r_t value, fido_cbor_info_t *ci) { if (!cbor_bytestring_is_definite(value) || value->length != sizeof(ci->aaguid)) { @@ -81,7 +81,7 @@ static int copy_aaguid(const cb0r_t value, fido_cbor_info_t *ci) { * * @param value The value to decode. * @param target A pointer to a location to store the decoded value at. - * @return int FIDO_OK when the operation was successful. + * @return int FIDO_OK if the operation was successful. */ static int decode_uint64(const cb0r_t value, uint64_t *target) { if (value->type != CB0R_INT) { @@ -226,7 +226,7 @@ static int cbor_info_decode_options(const cb0r_t key, const cb0r_t value, void * * * @param element The cb0r element containing the byte representing the protocol * @param arg User-passed argument (here: CBOR info). - * @return int FIDO_OK if protocol could be parsed. + * @return int FIDO_OK if the protocol could be parsed. */ static int cbor_info_decode_protocol(const cb0r_t element, void *arg) { fido_cbor_info_t *ci = (fido_cbor_info_t*)arg; @@ -253,7 +253,7 @@ static int cbor_info_decode_protocol(const cb0r_t element, void *arg) { * * @param element The cb0r element representing the transport * @param arg User-passed argument (here: CBOR info). - * @return int FIDO_OK if transport could be parsed. + * @return int FIDO_OK if the transport could be parsed. */ static int cbor_info_decode_transport(const cb0r_t element, void *arg) { fido_cbor_info_t *ci = (fido_cbor_info_t*)arg; @@ -281,7 +281,7 @@ static int cbor_info_decode_transport(const cb0r_t element, void *arg) { * @param key The cb0r element representing the algorithm key * @param value The cb0r element representing the algorithm identifier * @param arg User-passed argument (here: CBOR info). - * @return int FIDO_OK if algorithm could be parsed. + * @return int FIDO_OK if the algorithm could be parsed. */ static int cbor_info_decode_algorithm_entry(const cb0r_t key, const cb0r_t value, void *arg) { fido_cbor_info_t *ci = (fido_cbor_info_t*)arg; @@ -327,7 +327,7 @@ static int cbor_info_decode_algorithm_entry(const cb0r_t key, const cb0r_t value * * @param element The cb0r element representing the algorithm map * @param arg User-passed argument (here: CBOR info). - * @return int FIDO_OK if map could be parsed. + * @return int FIDO_OK if the map could be parsed. */ static int cbor_info_decode_algorithm(const cb0r_t element, void *arg) { if (element->type != CB0R_MAP) { @@ -343,7 +343,7 @@ static int cbor_info_decode_algorithm(const cb0r_t element, void *arg) { * @param key The cb0r element representing the map key * @param value The cb0r element representing the map value * @param arg User-passed argument (here: CBOR info). - * @return int FIDO_OK if entry could be parsed. + * @return int FIDO_OK if the entry could be parsed. */ static int parse_info_reply_entry(const cb0r_t key, const cb0r_t value, void *arg) { if (key->type != CB0R_INT || key->value > UINT8_MAX) { @@ -390,7 +390,7 @@ static int parse_info_reply_entry(const cb0r_t key, const cb0r_t value, void *ar * @brief Send a CTAP authenticatorGetInfo command. * * @param dev The device to communicate to. - * @return int FIDO_OK if transmission succeeded. + * @return int FIDO_OK if the transmission succeeded. */ static int fido_dev_get_cbor_info_tx(fido_dev_t *dev) { const unsigned char cbor[] = { CTAP_CBOR_GETINFO }; @@ -410,7 +410,7 @@ static int fido_dev_get_cbor_info_tx(fido_dev_t *dev) { * * @param dev The device to communicate to. * @param ci The fido_cbor_info_t to write the parsed reply to. - * @return int FIDO_OK if transmission succeeded. + * @return int FIDO_OK if the transmission succeeded. */ static int fido_dev_get_cbor_info_rx(fido_dev_t *dev, fido_cbor_info_t *ci) { unsigned char msg[dev->maxmsgsize]; diff --git a/src/largeblob.c b/src/largeblob.c index 432da85..4fa52b9 100644 --- a/src/largeblob.c +++ b/src/largeblob.c @@ -134,7 +134,7 @@ static bool largeblob_array_check(fido_blob_t *array) { * @param dev The device to read the large blob from. * @param offset The offset in the large blob to start reading from. * @param count The amount of bytes to read. - * @return int FIDO_OK when the operation was successful. + * @return int FIDO_OK if the operation was successful. */ static int largeblob_get_tx(fido_dev_t *dev, size_t offset, size_t count) { // 32 > 1 byte command + 1 byte map header + 1 byte get key + max. 9 byte get value + 1 byte offset key + max. 9 byte offset value @@ -192,7 +192,7 @@ static int parse_largeblob_reply(const cb0r_t key, const cb0r_t value, void *arg * * @param dev The device to read the answer from. * @param chunk The chunk to store the returned large blob chunk in. Must already be allocated. - * @return int FIDO_OK when the operation was successful. + * @return int FIDO_OK if the operation was successful. */ static int largeblob_get_rx(fido_dev_t *dev, fido_blob_t *chunk) { uint8_t msg[dev->maxmsgsize]; @@ -285,7 +285,7 @@ typedef struct largeblob_array_entry { * @param compressed Pointer to the compressed data. * @param compressed_len Length of the compressed data. * @param uncompressed_len_expected Expected length of the uncompressed data, to ensure that we got everything. - * @return int FIDO_OK when the operation was successful. + * @return int FIDO_OK if the operation was successful. */ static int fido_uncompress(fido_blob_t* out, uint8_t *compressed, size_t compressed_len, size_t uncompressed_len_expected) { uint32_t uncompressed_len_actual = out->max_length; @@ -306,7 +306,7 @@ static int fido_uncompress(fido_blob_t* out, uint8_t *compressed, size_t compres * @param key The CBOR key value of the entry. * @param value The CBOR encoded value. * @param data The largeblob array entry object to store the parsed data to. - * @return int FIDO_OK when the operation was successful. + * @return int FIDO_OK if the operation was successful. */ static int largeblob_parse_array_entry(cb0r_t key, cb0r_t value, void *data) { largeblob_array_entry_t *entry = (largeblob_array_entry_t*) data; @@ -362,7 +362,7 @@ static int largeblob_parse_array_entry(cb0r_t key, cb0r_t value, void *data) { * * @param value The CBOR encoded value. * @param data The largeblob array lookup parameters. This also contains the buffer where we write the uncompressed data to. - * @return int FIDO_OK when the operation was successful. + * @return int FIDO_OK if the operation was successful. */ static int largeblob_array_lookup(cb0r_t value, void* data) { largeblob_array_lookup_param_t *param = (largeblob_array_lookup_param_t*) data; diff --git a/src/nfc.c b/src/nfc.c index 36383e4..bdabf50 100644 --- a/src/nfc.c +++ b/src/nfc.c @@ -25,7 +25,7 @@ static const uint8_t fido_version_fido2[] PROGMEM_MARKER = "FIDO_2_0"; * @param dev The device to receive data from. * @param buf The buffer to write the received data to. * @param len The length of the buffer. - * @return int FIDO_OK when the operation was successful. + * @return int FIDO_OK if the operation was successful. */ static int rx_init(fido_dev_t *dev, unsigned char *buf, const size_t len) { @@ -69,7 +69,7 @@ static int rx_init(fido_dev_t *dev, unsigned char *buf, const size_t len) * @param sw The buffer to write the status word to. * @param buf A pointer to a pointer to a buffer where the received data is located. The pointer to the buffer will be advanced. * @param count The remaining length of the buffer, will be reduced. - * @return int FIDO_OK when the operation was successful. + * @return int FIDO_OK if the operation was successful. */ static int rx_apdu(fido_dev_t *dev, uint8_t sw[2], unsigned char **buf, size_t *count) { uint8_t f[256 + 2]; @@ -99,7 +99,7 @@ static int rx_apdu(fido_dev_t *dev, uint8_t sw[2], unsigned char **buf, size_t * * * @param dev The device to transmit data to. * @param count The amount of data expected to receive. - * @return int FIDO_OK when the operation was successful. + * @return int FIDO_OK if the operation was successful. */ static int tx_get_response(fido_dev_t *dev, uint8_t count) { @@ -165,7 +165,7 @@ static int rx_msg(fido_dev_t *dev, unsigned char *buf, const size_t len) { * @param dev The device to receive data from. * @param buf The buffer to write the received data to. * @param len The length of the buffer. - * @return int FIDO_OK when the operation was successful. + * @return int FIDO_OK if the operation was successful. */ static int rx_cbor(fido_dev_t *dev, unsigned char *buf, const size_t count) { int r; @@ -183,7 +183,7 @@ static int rx_cbor(fido_dev_t *dev, unsigned char *buf, const size_t count) { * @param cmd The CTAP command that was executed. * @param buf The buffer to write the response to. * @param len The length of the buffer. - * @return int FIDO_OK when the operation was successful. + * @return int FIDO_OK if the operation was successful. */ static int nfc_rx(struct fido_dev *dev, const uint8_t cmd, unsigned char *buf, const size_t len) { switch (cmd) { @@ -207,7 +207,7 @@ static int nfc_rx(struct fido_dev *dev, const uint8_t cmd, unsigned char *buf, c * @param payload The payload to send. * @param payload_len The length of the payload. * @param cla_flags The ISO7816 class flags to use. - * @return int FIDO_OK when the operation was successful. + * @return int FIDO_OK if the operation was successful. */ static int tx_short_apdu( fido_dev_t *dev, @@ -260,7 +260,7 @@ static int tx_short_apdu( * * @param dev The device to receive data from. * @param apdu The ISO7816 APDU to send. - * @return int FIDO_OK when the operation was successful. + * @return int FIDO_OK if the operation was successful. */ static int nfc_do_tx(fido_dev_t *dev, const iso7816_apdu_t *apdu) { uint16_t apdu_len = apdu->payload_len; @@ -291,7 +291,7 @@ static int nfc_do_tx(fido_dev_t *dev, const iso7816_apdu_t *apdu) { * @param cmd The CTAP command. * @param buf The payload to send. * @param len The length of the payload. - * @return int FIDO_OK when the operation was successful. + * @return int FIDO_OK if the operation was successful. */ static int nfc_tx(struct fido_dev *dev, const uint8_t cmd, const unsigned char *buf, const size_t len) { iso7816_apdu_t apdu; From 98c52e0f67c828646fec5edce0673fb4b416e53f Mon Sep 17 00:00:00 2001 From: frcroth Date: Wed, 31 Aug 2022 18:18:11 +0200 Subject: [PATCH 3/4] Improve function comments Co-authored-by: Felix Gohla <37421906+felix-gohla@users.noreply.github.com> --- include/internal.h | 2 +- src/assertion.c | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/include/internal.h b/include/internal.h index c3ffe9f..18b4f2e 100644 --- a/include/internal.h +++ b/include/internal.h @@ -21,7 +21,7 @@ * @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 operation was successful. + * @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); diff --git a/src/assertion.c b/src/assertion.c index f091ea9..4488513 100644 --- a/src/assertion.c +++ b/src/assertion.c @@ -113,7 +113,7 @@ 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. + * @brief CBOR decode the credential's data such as its type and ID. * * @param key The CBOR key. * @param assert The CBOR value. @@ -327,7 +327,7 @@ static int fido_dev_get_assert_tx( } /** - * @brief Receive the response data from the authenticator and parse the CBOR into the reply. + * @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. From 1bc32f7679fa42a356c5bcc86e6f8d95c1dedc97 Mon Sep 17 00:00:00 2001 From: frcroth Date: Wed, 31 Aug 2022 18:24:00 +0200 Subject: [PATCH 4/4] Add link to standard and license header --- include/crypto.h | 7 +++++++ include/internal.h | 2 +- src/assertion.c | 1 + 3 files changed, 9 insertions(+), 1 deletion(-) diff --git a/include/crypto.h b/include/crypto.h index 7a6e0c4..5b23c6c 100644 --- a/include/crypto.h +++ b/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 diff --git a/include/internal.h b/include/internal.h index 18b4f2e..7df315e 100644 --- a/include/internal.h +++ b/include/internal.h @@ -33,7 +33,7 @@ int fido_rx(fido_dev_t *d, const uint8_t cmd, void *buf, const size_t len); * @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 operation was successful. + * @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); diff --git a/src/assertion.c b/src/assertion.c index 4488513..39e4a41 100644 --- a/src/assertion.c +++ b/src/assertion.c @@ -114,6 +114,7 @@ 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.