From 6d72f6a793341910802741978fede0a2e8896b36 Mon Sep 17 00:00:00 2001 From: Donnie-Ice Date: Mon, 18 Nov 2024 18:16:22 +0000 Subject: [PATCH] [nasa/cryptolib#346] Resolved multiple SonarCloud results --- src/core/crypto.c | 14 ++- src/core/crypto_aos.c | 39 +++--- src/core/crypto_config.c | 7 +- src/core/crypto_error.c | 2 +- src/core/crypto_print.c | 112 +++++++++--------- ...hy_interface_kmc_crypto_service.template.c | 22 ++-- 6 files changed, 108 insertions(+), 88 deletions(-) diff --git a/src/core/crypto.c b/src/core/crypto.c index 15b40df8..6d7c84d3 100644 --- a/src/core/crypto.c +++ b/src/core/crypto.c @@ -265,7 +265,7 @@ uint8_t Crypto_Prep_Reply(uint8_t *reply, uint8_t appID) // Fill reply with reply header reply[count++] = (sdls_frame.hdr.pvn << 5) | (sdls_frame.hdr.type << 4) | (sdls_frame.hdr.shdr << 3) | - ((sdls_frame.hdr.appID & 0x700 >> 8)); + (sdls_frame.hdr.appID & 0x700 >> 8); reply[count++] = (sdls_frame.hdr.appID & 0x00FF); reply[count++] = (sdls_frame.hdr.seq << 6) | ((sdls_frame.hdr.pktid & 0x3F00) >> 8); reply[count++] = (sdls_frame.hdr.pktid & 0x00FF); @@ -442,12 +442,24 @@ int32_t Crypto_PDU(uint8_t *ingest, TC_t *tc_frame) break; } break; + + default: // ERROR +#ifdef PDU_DEBUG + printf(KRED "Error: Crypto_PDU failed interpreting User Flag! \n" RESET); +#endif + break; } break; case PDU_TYPE_REPLY: #ifdef PDU_DEBUG printf(KRED "Error: Crypto_PDU failed interpreting PDU Type! Received a Reply!?! \n" RESET); +#endif + break; + + default: +#ifdef PDU_DEBUG + printf(KRED "Error: Crypto_PDU failed interpreting PDU Type!\n" RESET); #endif break; } diff --git a/src/core/crypto_aos.c b/src/core/crypto_aos.c index d5bf0485..579d5ff3 100644 --- a/src/core/crypto_aos.c +++ b/src/core/crypto_aos.c @@ -86,7 +86,7 @@ int32_t Crypto_AOS_ApplySecurity(uint8_t *pTfBuffer) printf("\tVCID: 0x%04X", vcid); printf("\tMAP: %d\n", 0); printf("\tPriHdr as follows:\n\t\t"); - for (int i = 0; i < 6; i++) + for (i = 0; i < 6; i++) { printf("%02X", (uint8_t)pTfBuffer[i]); } @@ -120,7 +120,7 @@ int32_t Crypto_AOS_ApplySecurity(uint8_t *pTfBuffer) #ifdef AOS_DEBUG printf(KYEL "AOS BEFORE Apply Sec:\n\t" RESET); - for (int16_t i = 0; i < current_managed_parameters_struct.max_frame_size; i++) + for (i = 0; i < current_managed_parameters_struct.max_frame_size; i++) { printf("%02X", pTfBuffer[i]); } @@ -180,6 +180,9 @@ int32_t Crypto_AOS_ApplySecurity(uint8_t *pTfBuffer) case SA_AUTHENTICATED_ENCRYPTION: printf(KBLU "Creating a SDLS AOS - AUTHENTICATED ENCRYPTION!\n" RESET); break; + default: + printf(KRED "Failed interpreting SA Service Type\n" RESET); + break; } #endif @@ -241,20 +244,18 @@ int32_t Crypto_AOS_ApplySecurity(uint8_t *pTfBuffer) return status; } - if (sa_ptr->est == 0 && sa_ptr->ast == 1) + if (sa_ptr->est == 0 && sa_ptr->ast == 1 && sa_ptr->acs_len != 0) { - if (sa_ptr->acs_len != 0) + if ((sa_ptr->acs == CRYPTO_MAC_CMAC_AES256 || sa_ptr->acs == CRYPTO_MAC_HMAC_SHA256 || + sa_ptr->acs == CRYPTO_MAC_HMAC_SHA512) && + sa_ptr->iv_len > 0) { - if ((sa_ptr->acs == CRYPTO_MAC_CMAC_AES256 || sa_ptr->acs == CRYPTO_MAC_HMAC_SHA256 || - sa_ptr->acs == CRYPTO_MAC_HMAC_SHA512) && - sa_ptr->iv_len > 0) - { - status = CRYPTO_LIB_ERR_IV_NOT_SUPPORTED_FOR_ACS_ALGO; - mc_if->mc_log(status); - return status; - } + status = CRYPTO_LIB_ERR_IV_NOT_SUPPORTED_FOR_ACS_ALGO; + mc_if->mc_log(status); + return status; } } + // Start index from the transmitted portion for (i = sa_ptr->iv_len - sa_ptr->shivf_len; i < sa_ptr->iv_len; i++) { @@ -297,8 +298,8 @@ int32_t Crypto_AOS_ApplySecurity(uint8_t *pTfBuffer) // Byte Magic hex_padding[0] = (pkcs_padding >> 16) & 0xFF; - hex_padding[1] = (pkcs_padding >> 8) & 0xFF; - hex_padding[2] = (pkcs_padding)&0xFF; + hex_padding[1] = (pkcs_padding >> 8) & 0xFF; + hex_padding[2] = pkcs_padding & 0xFF; uint8_t padding_start = 0; padding_start = 3 - sa_ptr->shplf_len; @@ -619,7 +620,7 @@ int32_t Crypto_AOS_ApplySecurity(uint8_t *pTfBuffer) #ifdef AOS_DEBUG printf(KYEL "Printing new AOS frame:\n\t"); - for (int i = 0; i < current_managed_parameters_struct.max_frame_size; i++) + for (i = 0; i < current_managed_parameters_struct.max_frame_size; i++) { printf("%02X", pTfBuffer[i]); } @@ -1063,6 +1064,9 @@ int32_t Crypto_AOS_ProcessSecurity(uint8_t *p_ingest, uint16_t len_ingest, uint8 case SA_AUTHENTICATED_ENCRYPTION: printf(KBLU "Processing a AOS - AUTHENTICATED ENCRYPTION!\n" RESET); break; + default: + printf(KRED "Failed interpreting SA Service Type\n" RESET); + break; } #endif @@ -1113,7 +1117,7 @@ int32_t Crypto_AOS_ProcessSecurity(uint8_t *p_ingest, uint16_t len_ingest, uint8 } // Accio buffer - p_new_dec_frame = (uint8_t *)calloc(1, (len_ingest) * sizeof(uint8_t)); + p_new_dec_frame = (uint8_t *)calloc(1, len_ingest * sizeof(uint8_t)); if (!p_new_dec_frame) { #ifdef DEBUG @@ -1166,7 +1170,7 @@ int32_t Crypto_AOS_ProcessSecurity(uint8_t *p_ingest, uint16_t len_ingest, uint8 // Calculate size of the protocol data unit // NOTE: This size itself is not the length for authentication - pdu_len = current_managed_parameters_struct.max_frame_size - (byte_idx)-sa_ptr->stmacf_len; + pdu_len = current_managed_parameters_struct.max_frame_size - byte_idx - sa_ptr->stmacf_len; if (current_managed_parameters_struct.has_ocf == AOS_HAS_OCF) { pdu_len -= 4; @@ -1378,7 +1382,6 @@ int32_t Crypto_AOS_ProcessSecurity(uint8_t *p_ingest, uint16_t len_ingest, uint8 else if (sa_service_type == SA_PLAINTEXT) { memcpy(p_new_dec_frame + byte_idx, &(p_ingest[byte_idx]), pdu_len); - byte_idx += pdu_len; } #ifdef AOS_DEBUG diff --git a/src/core/crypto_config.c b/src/core/crypto_config.c index 2dff325f..ad336865 100644 --- a/src/core/crypto_config.c +++ b/src/core/crypto_config.c @@ -306,12 +306,9 @@ int32_t Crypto_Init(void) { cryptography_if = get_cryptography_interface_custom(); } - if (cryptography_if == NULL) + if (cryptography_if == NULL && cryptography_kmc_crypto_config != NULL) { // Note this needs to be the last option in the chain due to addition configuration required - if (cryptography_kmc_crypto_config != NULL) - { - cryptography_if = get_cryptography_interface_kmc_crypto_service(); - } + cryptography_if = get_cryptography_interface_kmc_crypto_service(); } if (cryptography_if == NULL) { diff --git a/src/core/crypto_error.c b/src/core/crypto_error.c index c5dff512..b01d94ab 100644 --- a/src/core/crypto_error.c +++ b/src/core/crypto_error.c @@ -213,7 +213,7 @@ char *Crypto_Get_Error_Code_Enum_String(int32_t crypto_error_code) { return_string = Crypto_Get_Crypto_Error_Code_String( crypto_error_code, CRYPTO_CORE_ERROR_CODES_MAX, - crypto_enum_errlist_core[(crypto_error_code * (-1))]); // Cryptolib uses negative error return codes. + crypto_enum_errlist_core[crypto_error_code * (-1)]); // Cryptolib uses negative error return codes. } return return_string; } \ No newline at end of file diff --git a/src/core/crypto_print.c b/src/core/crypto_print.c index 4fc5bb6e..791fece2 100644 --- a/src/core/crypto_print.c +++ b/src/core/crypto_print.c @@ -211,69 +211,75 @@ void Crypto_ccsdsPrint(CCSDS_t *sdls_frame) void Crypto_saPrint(SecurityAssociation_t *sa) { int i; - - printf("SA status: \n"); - printf("\t spi = %d \n", sa->spi); - printf("\t sa_state = 0x%01x \n", sa->sa_state); - // printf("\t gvcid[0] = 0x%02x \n", sa->gvcid_blk[spi].gvcid[0]); - // printf("\t gvcid[1] = 0x%02x \n", sa->gvcid_blk[spi].gvcid[1]); - // printf("\t gvcid[2] = 0x%02x \n", sa->gvcid_blk[spi].gvcid[2]); - // printf("\t gvcid[3] = 0x%02x \n", sa->gvcid_blk[spi].gvcid[3]); - printf("\t est = 0x%01x \n", sa->est); - printf("\t ast = 0x%01x \n", sa->ast); - printf("\t shivf_len = %d \n", sa->shivf_len); - printf("\t shsnf_len = %d \n", sa->shsnf_len); - printf("\t shplf_len = %d \n", sa->shplf_len); - printf("\t stmacf_len = %d \n", sa->stmacf_len); - printf("\t ecs_len = %d \n", sa->ecs_len); - if (sa->ecs_len > 0) + if (sa->spi < NUM_SA) { - for (i = 0; i < sa->ecs_len; i++) + printf("SA status: \n"); + printf("\t spi = %d \n", sa->spi); + printf("\t sa_state = 0x%01x \n", sa->sa_state); + // printf("\t gvcid[0] = 0x%02x \n", sa->gvcid_blk[spi].gvcid[0]); + // printf("\t gvcid[1] = 0x%02x \n", sa->gvcid_blk[spi].gvcid[1]); + // printf("\t gvcid[2] = 0x%02x \n", sa->gvcid_blk[spi].gvcid[2]); + // printf("\t gvcid[3] = 0x%02x \n", sa->gvcid_blk[spi].gvcid[3]); + printf("\t est = 0x%01x \n", sa->est); + printf("\t ast = 0x%01x \n", sa->ast); + printf("\t shivf_len = %d \n", sa->shivf_len); + printf("\t shsnf_len = %d \n", sa->shsnf_len); + printf("\t shplf_len = %d \n", sa->shplf_len); + printf("\t stmacf_len = %d \n", sa->stmacf_len); + printf("\t ecs_len = %d \n", sa->ecs_len); + if (sa->ecs_len > 0) { - printf("\t ecs[%d] = 0x%02x \n", i, (sa->ecs + i)); + for (i = 0; i < sa->ecs_len; i++) + { + printf("\t ecs[%d] = 0x%02x \n", i, (sa->ecs + i)); + } } - } - printf("\t ekid = %d \n", sa->ekid); - printf("\t ek_ref = %s \n", sa->ek_ref); - printf("\t akid = %d \n", sa->akid); - printf("\t ak_ref = %s \n", sa->ak_ref); - printf("\t iv_len = %d \n", sa->iv_len); - if (sa->iv_len > 0) - { - for (i = 0; i < sa->iv_len; i++) + printf("\t ekid = %d \n", sa->ekid); + printf("\t ek_ref = %s \n", sa->ek_ref); + printf("\t akid = %d \n", sa->akid); + printf("\t ak_ref = %s \n", sa->ak_ref); + printf("\t iv_len = %d \n", sa->iv_len); + if (sa->iv_len > 0) { - printf("\t iv[%d] = 0x%02x \n", i, *(sa->iv + i)); + for (i = 0; i < sa->iv_len; i++) + { + printf("\t iv[%d] = 0x%02x \n", i, *(sa->iv + i)); + } } - } - else - { - printf("\t iv = %s \n", sa->iv); - } - printf("\t acs_len = %d \n", sa->acs_len); - printf("\t acs = 0x%02x \n", sa->acs); - printf("\t abm_len = %d \n", sa->abm_len); - if (sa->abm_len > 0) - { - printf("\t abm = "); - for (i = 0; i < sa->abm_len; i++) + else { - printf("%02x", *(sa->abm + i)); + printf("\t iv = %s \n", sa->iv); } - printf("\n"); - } - printf("\t arsn_len = %d \n", sa->arsn_len); - if (sa->arsn_len > 0) - { - printf("\t arsn = "); - for (i = 0; i < sa->arsn_len; i++) + printf("\t acs_len = %d \n", sa->acs_len); + printf("\t acs = 0x%02x \n", sa->acs); + printf("\t abm_len = %d \n", sa->abm_len); + if (sa->abm_len > 0) { - printf("%02x", *(sa->arsn + i)); + printf("\t abm = "); + for (i = 0; i < sa->abm_len; i++) + { + printf("%02x", *(sa->abm + i)); + } + printf("\n"); + } + printf("\t arsn_len = %d \n", sa->arsn_len); + if (sa->arsn_len > 0) + { + printf("\t arsn = "); + for (i = 0; i < sa->arsn_len; i++) + { + printf("%02x", *(sa->arsn + i)); + } + printf("\n"); } - printf("\n"); - } - printf("\t arsnw_len = %d \n", sa->arsnw_len); - printf("\t arsnw = %d \n", sa->arsnw); + printf("\t arsnw_len = %d \n", sa->arsnw_len); + printf("\t arsnw = %d \n", sa->arsnw); + } + else + { + printf("SPI %d does not exist\n", sa->spi); + } } /** diff --git a/src/crypto/kmc/cryptography_interface_kmc_crypto_service.template.c b/src/crypto/kmc/cryptography_interface_kmc_crypto_service.template.c index 20833e2c..56d8f6a6 100644 --- a/src/crypto/kmc/cryptography_interface_kmc_crypto_service.template.c +++ b/src/crypto/kmc/cryptography_interface_kmc_crypto_service.template.c @@ -395,6 +395,7 @@ static int32_t cryptography_encrypt(uint8_t *data_out, size_t len_data_out, uint uint8_t ciphertext_found = CRYPTO_FALSE; char *ciphertext_base64 = NULL; char *ciphertext_IV_base64 = NULL; + uint8_t *save_ptr; for (json_idx = 1; json_idx < parse_result; json_idx++) { if (jsoneq(chunk_write->response, &t[json_idx], "metadata") == 0) @@ -407,15 +408,15 @@ static int32_t cryptography_encrypt(uint8_t *data_out, size_t len_data_out, uint char *line; char *token; char temp_buff[256]; - for (line = strtok(ciphertext_IV_base64, ","); line != NULL; line = strtok(line + strlen(line) + 1, ",")) + for (line = __strtok_r(ciphertext_IV_base64, ",", save_ptr); line != NULL; line = __strtok_r(line + strlen(line) + 1, ",", save_ptr)) { strncpy(temp_buff, line, sizeof(temp_buff)); - for (token = strtok(temp_buff, ":"); token != NULL; token = strtok(token + strlen(token) + 1, ":")) + for (token = __strtok_r(temp_buff, ":", save_ptr); token != NULL; token = strtok_r(token + strlen(token) + 1, ":", save_ptr)) { if (strcmp(token, "initialVector") == 0) { - token = strtok(token + strlen(token) + 1, ":"); + token = __strtok_r(token + strlen(token) + 1, ":", save_ptr); char *ciphertext_token_base64 = malloc(strlen(token)); size_t cipher_text_token_len = strlen(token); memcpy(ciphertext_token_base64, token, cipher_text_token_len); @@ -1356,10 +1357,11 @@ static int32_t cryptography_aead_encrypt(uint8_t *data_out, size_t len_data_out, return status; } - int json_idx = 0; - uint8_t ciphertext_found = CRYPTO_FALSE; - char *ciphertext_base64 = NULL; - char *ciphertext_IV_base64 = NULL; + int json_idx = 0; + uint8_t ciphertext_found = CRYPTO_FALSE; + char *ciphertext_base64 = NULL; + char *ciphertext_IV_base64 = NULL; + uint8_t *save_ptr; for (json_idx = 1; json_idx < parse_result; json_idx++) { if (jsoneq(chunk_write->response, &t[json_idx], "metadata") == 0) @@ -1373,15 +1375,15 @@ static int32_t cryptography_aead_encrypt(uint8_t *data_out, size_t len_data_out, char *line; char *token; char temp_buff[256]; - for (line = strtok(ciphertext_IV_base64, ","); line != NULL; line = strtok(line + strlen(line) + 1, ",")) + for (line = __strtok_r(ciphertext_IV_base64, ",", save_ptr); line != NULL; line = __strtok_r(line + strlen(line) + 1, ",", save_ptr)) { strncpy(temp_buff, line, sizeof(temp_buff)); - for (token = strtok(temp_buff, ":"); token != NULL; token = strtok(token + strlen(token) + 1, ":")) + for (token = __strtok_r(temp_buff, ":", save_ptr); token != NULL; token = __strtok_r(token + strlen(token) + 1, ":", save_ptr)) { if (strcmp(token, "initialVector") == 0) { - token = strtok(token + strlen(token) + 1, ":"); + token = __strtok_r(token + strlen(token) + 1, ":", save_ptr); char *ciphertext_token_base64 = malloc(strlen(token)); size_t cipher_text_token_len = strlen(token); memcpy(ciphertext_token_base64, token, cipher_text_token_len);