From 48ae7b6c8b48b43f31c87b3249303d8eeb038838 Mon Sep 17 00:00:00 2001 From: Tobias Kantusch Date: Wed, 31 Aug 2022 15:42:32 +0200 Subject: [PATCH] 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, 266 insertions(+), 20 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..e3a21e0 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,18 @@ 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) +// TODO: function to set client data (and compute hash) /fg isn't that done? /tk +/** + * @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 +328,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 +368,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 +404,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 +420,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 +448,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 +473,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;