Add more generic functions for calculating/checking ICV. #30
Conversation
There was a problem hiding this comment.
Much of this PR overlaps with #28 - please remove those portions.
The implementation of FILE_get_file_content_if_existing_icv_is_valid should not have a (non-negligible) overlap with protect_or_check_icv().
And a couple of minor points.
| @@ -370,6 +371,7 @@ size_t UTIL_url_encode(const char *source, | |||
| # define HEX_BITS 4 | |||
| # define HEX_MASK 0x0f | |||
| # define MAX_DIGIT 9 | |||
| # define ICV_LEN16 16 | |||
There was a problem hiding this comment.
Why the strange name ICV_LEN16?
Better move #define ICVLEN 16 here from src/storage/files_icv.c
and rename the macro to, e.g., SECUTILS_ICV_LEN.
| /*! | ||
| * @brief implementation of the function UTIL_calculate_icv. | ||
| * @note this function was created to avoid code repetition (the same computation is needed in files_icv.c). | ||
| * | ||
| * @param ctx pointer to uta context object | ||
| * @param data pointer to data from which the ICV will be calculated | ||
| * @param data_len size of data from which the ICV will be calculated | ||
| * @param key_dv The derivation value for key for which the ICV is calculated | ||
| * @param mac Pointer to a buffer where the resulting ICV will be stored. This buffer must be at least | ||
| * ICV_LEN16 in size. | ||
| * @return true if calculating the ICV is successful, false otherwise | ||
| */ | ||
| bool UTIL_calculate_icv_impl(uta_ctx* ctx, const unsigned char* data, const size_t data_len, const char* key_dv, | ||
| unsigned char* mac); | ||
|
|
There was a problem hiding this comment.
This internal function should not be exported.
Better keep it as static in files_icv.c (at the point where it was originally part of calculate_icv_hex(), which will also ease reviewing the changes) because it is primarily used there.
Then of course also the new function UTIL_calculate_icv() needs to be implemented there, but this is no problem.
| @@ -435,4 +437,33 @@ int UTIL_base64_encode_to_buf(const unsigned char *data, int len, | |||
| unsigned char *UTIL_base64_decode(const char *b64_data, int b64_len, | |||
| int *decoded_len); | |||
|
|
|||
| /*! | |||
| * @brief derive integrity protection hash for data with given len, using key as DV. | |||
There was a problem hiding this comment.
hash -> HMAC
using key as DV -> using given derivation value (DV)
| * @param ctx pointer to uta context object | ||
| * @param data pointer to data from which the ICV will be calculated | ||
| * @param data_len size of data from which the ICV will be calculated | ||
| * @param key_dv The derivation value for key for which the ICV is calculated |
There was a problem hiding this comment.
-> * @param name_dv The name (as a \0-terminated string) of the derivation value to use for calculating the ICV
| * ICV_LEN16 in size. | ||
| * @return true if calculating the ICV is successful, false otherwise | ||
| */ | ||
| bool UTIL_calculate_icv(uta_ctx* ctx, const unsigned char* data, const size_t data_len, const char* key_dv, |
|
|
||
| if(0 is_eq path) | ||
| { | ||
| LOG(FL_ERR, "No path to ICV file"); |
| return 0; | ||
| } | ||
|
|
||
| // open file |
There was a problem hiding this comment.
Better not add such useless comments.
| @@ -135,13 +103,13 @@ static bool protect_or_check_icv(OPTIONAL uta_ctx* ctx, const char* file, const | |||
| } | |||
There was a problem hiding this comment.
Please use a little further up the new get_file_size() instead of fseek() etc.
Better check for size < ICV_LINE_LEN and complain "File '%s' is too short"
| else if(0 is_eq file_size) | ||
| { | ||
| LOG(FL_ERR, "File '%s' is empty", absolute_path); |
There was a problem hiding this comment.
(Better check for file_size < ICV_LINE_LEN and complain "File '%s' is too short" -
but this should anyway be done in protect_or_check_icv().)
| LOG(FL_ERR, "Could not resolve absolute path from: %s", path); | ||
| return 0; | ||
| } | ||
|
|
There was a problem hiding this comment.
Do not copy (most of) protect_or_check_icv(), but generalize it and use here, e.g.,
OPENSSL_STRING content = protect_or_check_icv(ctx, path, 0, false);
No description provided.