Skip to content

Commit c0d60ff

Browse files
bridge-dragonxmlop0porfanid
bridge-dragonxmlop0
and
porfanid
committed
Enhancement - Standardize Error Returns for Failure Paths (#483)
* Initial plan * Standardize error returns: Update crypto_config.c and sa_interface functions Co-authored-by: porfanid <19299306+porfanid@users.noreply.github.com> * Improve error propagation: Check return values of key_init and mc_initialize Co-authored-by: porfanid <19299306+porfanid@users.noreply.github.com>
1 parent f473a8d commit c0d60ff

3 files changed

Lines changed: 230 additions & 71 deletions

File tree

‎include/crypto.h‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -237,12 +237,12 @@ void Crypto_TM_updateOCF(Telemetry_Frame_Ocf_Fsr_t *report, TM_t *tm_f
237237
uint8_t *Crypto_Prepare_TC_AAD(const uint8_t *buffer, uint16_t len_aad, const uint8_t *abm_buffer);
238238
uint32_t Crypto_Prepare_TM_AAD(const uint8_t *buffer, uint16_t len_aad, const uint8_t *abm_buffer, uint8_t *aad);
239239
uint32_t Crypto_Prepare_AOS_AAD(const uint8_t *buffer, uint16_t len_aad, const uint8_t *abm_buffer, uint8_t *aad);
240-
void Crypto_Local_Config(void);
241-
void Crypto_Local_Init(void);
240+
int32_t Crypto_Local_Config(void);
241+
int32_t Crypto_Local_Init(void);
242242
int32_t Crypto_window(uint8_t *actual, uint8_t *expected, int length, int window);
243243
uint16_t Crypto_Calc_FECF(const uint8_t *ingest, int len_ingest);
244244
uint16_t Crypto_Calc_FHECF(uint8_t *data);
245-
void Crypto_Calc_CRC_Init_Table(void);
245+
int32_t Crypto_Calc_CRC_Init_Table(void);
246246
uint16_t Crypto_Calc_CRC16(uint8_t *data, int size);
247247
int32_t Crypto_Check_Anti_Replay(SecurityAssociation_t *sa_ptr, uint8_t *arsn, uint8_t *iv);
248248
int32_t Crypto_Get_ECS_Algo_Keylen(uint8_t algo);
@@ -303,7 +303,7 @@ int32_t Crypto_Get_Managed_Parameters_For_Gvcid(uint8_t tfvn, uint16_t scid, uin
303303
GvcidManagedParameters_t *managed_parameters_in,
304304
GvcidManagedParameters_t *managed_parameters_out);
305305
// Project-wide support functions
306-
extern char *crypto_deep_copy_string(char *src_string);
306+
extern int32_t crypto_deep_copy_string(char *src_string, char **dst_string);
307307

308308
/*
309309
** Extern Global Variables

‎src/core/crypto_config.c‎

Lines changed: 193 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -279,8 +279,12 @@ int32_t Crypto_Init(void)
279279
key_if = get_key_interface_kmc();
280280
}
281281
}
282-
key_if->key_init();
283-
// TODO: Check and return status on error
282+
status = key_if->key_init();
283+
if (status != CRYPTO_LIB_SUCCESS)
284+
{
285+
return status;
286+
}
287+
284288
/* MC Interface */
285289
if (mc_if == NULL)
286290
{
@@ -297,8 +301,12 @@ int32_t Crypto_Init(void)
297301
mc_if = get_mc_interface_internal();
298302
}
299303
}
300-
mc_if->mc_initialize();
301-
// TODO: Check and return status on error
304+
status = mc_if->mc_initialize();
305+
if (status != CRYPTO_LIB_SUCCESS)
306+
{
307+
return status;
308+
}
309+
302310
/* SA Interface */
303311
if (sa_if == NULL)
304312
{
@@ -325,7 +333,7 @@ int32_t Crypto_Init(void)
325333
{
326334
status = SADB_INVALID_SADB_TYPE;
327335
return status;
328-
} // TODO: Error stack
336+
}
329337
}
330338

331339
/* Crypto Interface */
@@ -397,19 +405,30 @@ int32_t Crypto_Init(void)
397405
{
398406
status = sa_if->sa_config();
399407

400-
Crypto_Local_Init();
401-
Crypto_Local_Config();
402-
403-
// TODO - Add error checking
404-
405-
// Init table for CRC calculations
406-
Crypto_Calc_CRC_Init_Table();
407-
408-
// cFS Standard Initialized Message
408+
if (status == CRYPTO_LIB_SUCCESS)
409+
{
410+
status = Crypto_Local_Init();
411+
}
412+
413+
if (status == CRYPTO_LIB_SUCCESS)
414+
{
415+
status = Crypto_Local_Config();
416+
}
417+
418+
if (status == CRYPTO_LIB_SUCCESS)
419+
{
420+
// Init table for CRC calculations
421+
status = Crypto_Calc_CRC_Init_Table();
422+
}
423+
424+
if (status == CRYPTO_LIB_SUCCESS)
425+
{
426+
// cFS Standard Initialized Message
409427
#ifdef DEBUG
410-
printf(KBLU "Crypto Lib Intialized. Version %d.%d.%d.%d\n" RESET, CRYPTO_LIB_MAJOR_VERSION,
411-
CRYPTO_LIB_MINOR_VERSION, CRYPTO_LIB_REVISION, CRYPTO_LIB_MISSION_REV);
428+
printf(KBLU "Crypto Lib Intialized. Version %d.%d.%d.%d\n" RESET, CRYPTO_LIB_MAJOR_VERSION,
429+
CRYPTO_LIB_MINOR_VERSION, CRYPTO_LIB_REVISION, CRYPTO_LIB_MISSION_REV);
412430
#endif
431+
}
413432
}
414433
else
415434
{
@@ -517,21 +536,53 @@ int32_t Crypto_Config_MariaDB(char *mysql_hostname, char *mysql_database, uint16
517536
sa_mariadb_config = (SadbMariaDBConfig_t *)calloc(1, SADB_MARIADB_CONFIG_SIZE);
518537
if (sa_mariadb_config != NULL)
519538
{
520-
sa_mariadb_config->mysql_username = crypto_deep_copy_string(mysql_username);
521-
sa_mariadb_config->mysql_password = crypto_deep_copy_string(mysql_password);
522-
sa_mariadb_config->mysql_hostname = crypto_deep_copy_string(mysql_hostname);
523-
sa_mariadb_config->mysql_database = crypto_deep_copy_string(mysql_database);
524-
sa_mariadb_config->mysql_port = mysql_port;
539+
status = CRYPTO_LIB_SUCCESS;
540+
541+
// Copy all string parameters, checking for errors
542+
if (status == CRYPTO_LIB_SUCCESS)
543+
status = crypto_deep_copy_string(mysql_username, &sa_mariadb_config->mysql_username);
544+
if (status == CRYPTO_LIB_SUCCESS)
545+
status = crypto_deep_copy_string(mysql_password, &sa_mariadb_config->mysql_password);
546+
if (status == CRYPTO_LIB_SUCCESS)
547+
status = crypto_deep_copy_string(mysql_hostname, &sa_mariadb_config->mysql_hostname);
548+
if (status == CRYPTO_LIB_SUCCESS)
549+
status = crypto_deep_copy_string(mysql_database, &sa_mariadb_config->mysql_database);
550+
551+
sa_mariadb_config->mysql_port = mysql_port;
552+
525553
/*start - encrypted connection related parameters*/
526-
sa_mariadb_config->mysql_mtls_cert = crypto_deep_copy_string(mysql_mtls_cert);
527-
sa_mariadb_config->mysql_mtls_key = crypto_deep_copy_string(mysql_mtls_key);
528-
sa_mariadb_config->mysql_mtls_ca = crypto_deep_copy_string(mysql_tls_ca);
529-
sa_mariadb_config->mysql_mtls_capath = crypto_deep_copy_string(mysql_tls_capath);
530-
sa_mariadb_config->mysql_tls_verify_server = mysql_tls_verify_server;
531-
sa_mariadb_config->mysql_mtls_client_key_password = crypto_deep_copy_string(mysql_mtls_client_key_password);
554+
if (status == CRYPTO_LIB_SUCCESS)
555+
status = crypto_deep_copy_string(mysql_mtls_cert, &sa_mariadb_config->mysql_mtls_cert);
556+
if (status == CRYPTO_LIB_SUCCESS)
557+
status = crypto_deep_copy_string(mysql_mtls_key, &sa_mariadb_config->mysql_mtls_key);
558+
if (status == CRYPTO_LIB_SUCCESS)
559+
status = crypto_deep_copy_string(mysql_tls_ca, &sa_mariadb_config->mysql_mtls_ca);
560+
if (status == CRYPTO_LIB_SUCCESS)
561+
status = crypto_deep_copy_string(mysql_tls_capath, &sa_mariadb_config->mysql_mtls_capath);
562+
563+
sa_mariadb_config->mysql_tls_verify_server = mysql_tls_verify_server;
564+
565+
if (status == CRYPTO_LIB_SUCCESS)
566+
status = crypto_deep_copy_string(mysql_mtls_client_key_password, &sa_mariadb_config->mysql_mtls_client_key_password);
567+
532568
sa_mariadb_config->mysql_require_secure_transport = mysql_require_secure_transport;
533569
/*end - encrypted connection related parameters*/
534-
status = CRYPTO_LIB_SUCCESS;
570+
571+
// If any string copying failed, clean up
572+
if (status != CRYPTO_LIB_SUCCESS)
573+
{
574+
if (sa_mariadb_config->mysql_username) free(sa_mariadb_config->mysql_username);
575+
if (sa_mariadb_config->mysql_password) free(sa_mariadb_config->mysql_password);
576+
if (sa_mariadb_config->mysql_hostname) free(sa_mariadb_config->mysql_hostname);
577+
if (sa_mariadb_config->mysql_database) free(sa_mariadb_config->mysql_database);
578+
if (sa_mariadb_config->mysql_mtls_cert) free(sa_mariadb_config->mysql_mtls_cert);
579+
if (sa_mariadb_config->mysql_mtls_key) free(sa_mariadb_config->mysql_mtls_key);
580+
if (sa_mariadb_config->mysql_mtls_ca) free(sa_mariadb_config->mysql_mtls_ca);
581+
if (sa_mariadb_config->mysql_mtls_capath) free(sa_mariadb_config->mysql_mtls_capath);
582+
if (sa_mariadb_config->mysql_mtls_client_key_password) free(sa_mariadb_config->mysql_mtls_client_key_password);
583+
free(sa_mariadb_config);
584+
sa_mariadb_config = NULL;
585+
}
535586
}
536587
return status;
537588
}
@@ -545,27 +596,66 @@ int32_t Crypto_Config_Kmc_Crypto_Service(char *protocol, char *kmc_crypto_hostna
545596
int32_t status = CRYPTO_LIB_SUCCESS;
546597
cryptography_kmc_crypto_config =
547598
(CryptographyKmcCryptoServiceConfig_t *)calloc(1, CRYPTOGRAPHY_KMC_CRYPTO_SERVICE_CONFIG_SIZE);
548-
cryptography_kmc_crypto_config->protocol = crypto_deep_copy_string(protocol);
549-
cryptography_kmc_crypto_config->kmc_crypto_hostname = crypto_deep_copy_string(kmc_crypto_hostname);
550-
cryptography_kmc_crypto_config->kmc_crypto_port = kmc_crypto_port;
599+
600+
if (cryptography_kmc_crypto_config == NULL)
601+
{
602+
return CRYPTO_LIB_ERROR;
603+
}
604+
605+
// Copy string parameters, checking for errors
606+
if (status == CRYPTO_LIB_SUCCESS)
607+
status = crypto_deep_copy_string(protocol, &cryptography_kmc_crypto_config->protocol);
608+
if (status == CRYPTO_LIB_SUCCESS)
609+
status = crypto_deep_copy_string(kmc_crypto_hostname, &cryptography_kmc_crypto_config->kmc_crypto_hostname);
610+
611+
cryptography_kmc_crypto_config->kmc_crypto_port = kmc_crypto_port;
612+
551613
if (kmc_crypto_app != NULL)
552614
{
553-
cryptography_kmc_crypto_config->kmc_crypto_app_uri = crypto_deep_copy_string(kmc_crypto_app);
615+
if (status == CRYPTO_LIB_SUCCESS)
616+
status = crypto_deep_copy_string(kmc_crypto_app, &cryptography_kmc_crypto_config->kmc_crypto_app_uri);
554617
}
555618
else
556619
{
557-
char *crypto_service_tmp = (char *)"crypto-service";
558-
cryptography_kmc_crypto_config->kmc_crypto_app_uri = crypto_deep_copy_string(crypto_service_tmp);
620+
char *crypto_service_tmp = (char *)"crypto-service";
621+
if (status == CRYPTO_LIB_SUCCESS)
622+
status = crypto_deep_copy_string(crypto_service_tmp, &cryptography_kmc_crypto_config->kmc_crypto_app_uri);
559623
}
560624

561-
cryptography_kmc_crypto_config->mtls_client_cert_path = crypto_deep_copy_string(mtls_client_cert_path);
562-
cryptography_kmc_crypto_config->mtls_client_cert_type = crypto_deep_copy_string(mtls_client_cert_type);
563-
cryptography_kmc_crypto_config->mtls_client_key_path = crypto_deep_copy_string(mtls_client_key_path);
564-
cryptography_kmc_crypto_config->mtls_client_key_pass = crypto_deep_copy_string(mtls_client_key_pass);
565-
cryptography_kmc_crypto_config->mtls_ca_bundle = crypto_deep_copy_string(kmc_tls_ca_bundle);
566-
cryptography_kmc_crypto_config->mtls_ca_path = crypto_deep_copy_string(kmc_tls_ca_path);
567-
cryptography_kmc_crypto_config->mtls_issuer_cert = crypto_deep_copy_string(mtls_issuer_cert);
625+
if (status == CRYPTO_LIB_SUCCESS)
626+
status = crypto_deep_copy_string(mtls_client_cert_path, &cryptography_kmc_crypto_config->mtls_client_cert_path);
627+
if (status == CRYPTO_LIB_SUCCESS)
628+
status = crypto_deep_copy_string(mtls_client_cert_type, &cryptography_kmc_crypto_config->mtls_client_cert_type);
629+
if (status == CRYPTO_LIB_SUCCESS)
630+
status = crypto_deep_copy_string(mtls_client_key_path, &cryptography_kmc_crypto_config->mtls_client_key_path);
631+
if (status == CRYPTO_LIB_SUCCESS)
632+
status = crypto_deep_copy_string(mtls_client_key_pass, &cryptography_kmc_crypto_config->mtls_client_key_pass);
633+
if (status == CRYPTO_LIB_SUCCESS)
634+
status = crypto_deep_copy_string(kmc_tls_ca_bundle, &cryptography_kmc_crypto_config->mtls_ca_bundle);
635+
if (status == CRYPTO_LIB_SUCCESS)
636+
status = crypto_deep_copy_string(kmc_tls_ca_path, &cryptography_kmc_crypto_config->mtls_ca_path);
637+
if (status == CRYPTO_LIB_SUCCESS)
638+
status = crypto_deep_copy_string(mtls_issuer_cert, &cryptography_kmc_crypto_config->mtls_issuer_cert);
639+
568640
cryptography_kmc_crypto_config->ignore_ssl_hostname_validation = kmc_ignore_ssl_hostname_validation;
641+
642+
// If any string copying failed, clean up
643+
if (status != CRYPTO_LIB_SUCCESS)
644+
{
645+
if (cryptography_kmc_crypto_config->protocol) free(cryptography_kmc_crypto_config->protocol);
646+
if (cryptography_kmc_crypto_config->kmc_crypto_hostname) free(cryptography_kmc_crypto_config->kmc_crypto_hostname);
647+
if (cryptography_kmc_crypto_config->kmc_crypto_app_uri) free(cryptography_kmc_crypto_config->kmc_crypto_app_uri);
648+
if (cryptography_kmc_crypto_config->mtls_client_cert_path) free(cryptography_kmc_crypto_config->mtls_client_cert_path);
649+
if (cryptography_kmc_crypto_config->mtls_client_cert_type) free(cryptography_kmc_crypto_config->mtls_client_cert_type);
650+
if (cryptography_kmc_crypto_config->mtls_client_key_path) free(cryptography_kmc_crypto_config->mtls_client_key_path);
651+
if (cryptography_kmc_crypto_config->mtls_client_key_pass) free(cryptography_kmc_crypto_config->mtls_client_key_pass);
652+
if (cryptography_kmc_crypto_config->mtls_ca_bundle) free(cryptography_kmc_crypto_config->mtls_ca_bundle);
653+
if (cryptography_kmc_crypto_config->mtls_ca_path) free(cryptography_kmc_crypto_config->mtls_ca_path);
654+
if (cryptography_kmc_crypto_config->mtls_issuer_cert) free(cryptography_kmc_crypto_config->mtls_issuer_cert);
655+
free(cryptography_kmc_crypto_config);
656+
cryptography_kmc_crypto_config = NULL;
657+
}
658+
569659
return status;
570660
}
571661

@@ -577,15 +667,40 @@ int32_t Crypto_Config_Kmc_Crypto_Service(char *protocol, char *kmc_crypto_hostna
577667
int32_t Crypto_Config_Cam(uint8_t cam_enabled, char *cookie_file_path, char *keytab_file_path, uint8_t login_method,
578668
char *access_manager_uri, char *username, char *cam_home)
579669
{
580-
int32_t status = CRYPTO_LIB_SUCCESS;
581-
cam_config = (CamConfig_t *)calloc(1, CAM_CONFIG_SIZE);
582-
cam_config->cam_enabled = cam_enabled;
583-
cam_config->cookie_file_path = crypto_deep_copy_string(cookie_file_path);
584-
cam_config->keytab_file_path = crypto_deep_copy_string(keytab_file_path);
585-
cam_config->login_method = login_method;
586-
cam_config->access_manager_uri = crypto_deep_copy_string(access_manager_uri);
587-
cam_config->username = crypto_deep_copy_string(username);
588-
cam_config->cam_home = crypto_deep_copy_string(cam_home);
670+
int32_t status = CRYPTO_LIB_SUCCESS;
671+
cam_config = (CamConfig_t *)calloc(1, CAM_CONFIG_SIZE);
672+
673+
if (cam_config == NULL)
674+
{
675+
return CRYPTO_LIB_ERROR;
676+
}
677+
678+
cam_config->cam_enabled = cam_enabled;
679+
cam_config->login_method = login_method;
680+
681+
// Copy string parameters, checking for errors
682+
if (status == CRYPTO_LIB_SUCCESS)
683+
status = crypto_deep_copy_string(cookie_file_path, &cam_config->cookie_file_path);
684+
if (status == CRYPTO_LIB_SUCCESS)
685+
status = crypto_deep_copy_string(keytab_file_path, &cam_config->keytab_file_path);
686+
if (status == CRYPTO_LIB_SUCCESS)
687+
status = crypto_deep_copy_string(access_manager_uri, &cam_config->access_manager_uri);
688+
if (status == CRYPTO_LIB_SUCCESS)
689+
status = crypto_deep_copy_string(username, &cam_config->username);
690+
if (status == CRYPTO_LIB_SUCCESS)
691+
status = crypto_deep_copy_string(cam_home, &cam_config->cam_home);
692+
693+
// If any string copying failed, clean up
694+
if (status != CRYPTO_LIB_SUCCESS)
695+
{
696+
if (cam_config->cookie_file_path) free(cam_config->cookie_file_path);
697+
if (cam_config->keytab_file_path) free(cam_config->keytab_file_path);
698+
if (cam_config->access_manager_uri) free(cam_config->access_manager_uri);
699+
if (cam_config->username) free(cam_config->username);
700+
if (cam_config->cam_home) free(cam_config->cam_home);
701+
free(cam_config);
702+
cam_config = NULL;
703+
}
589704

590705
return status;
591706
}
@@ -672,27 +787,40 @@ int32_t crypto_free_config_structs(void)
672787
* @brief Function: crypto_deep_copy_string
673788
* Used to malloc a local copy of an externally referenced string. The string MUST BE null-terminated.
674789
* @param src_string: Pointer to externally-memory-managed string.
675-
* @return char*: Pointer to locally-memory-managed string copy.
790+
* @param dst_string: Pointer to store the locally-memory-managed string copy.
791+
* @return int32_t: Success/Failure status.
676792
**/
677793

678-
char *crypto_deep_copy_string(char *src_string)
794+
int32_t crypto_deep_copy_string(char *src_string, char **dst_string)
679795
{
796+
if (dst_string == NULL)
797+
{
798+
return CRYPTO_LIB_ERR_NULL_BUFFER;
799+
}
800+
680801
if (src_string == NULL)
681802
{
682-
return NULL;
803+
*dst_string = NULL;
804+
return CRYPTO_LIB_SUCCESS;
683805
}
806+
684807
// Note that the strlen() function doesn't count the null character \0 while calculating the length.
685-
char *deep_copied_str = malloc((strlen(src_string) + 1) * sizeof(char));
686-
memcpy(deep_copied_str, src_string, strlen(src_string) + 1);
687-
return deep_copied_str;
808+
*dst_string = malloc((strlen(src_string) + 1) * sizeof(char));
809+
if (*dst_string == NULL)
810+
{
811+
return CRYPTO_LIB_ERROR;
812+
}
813+
814+
memcpy(*dst_string, src_string, strlen(src_string) + 1);
815+
return CRYPTO_LIB_SUCCESS;
688816
}
689817

690818
/**
691819
* @brief Function: Crypto_Local_Config
692820
*
693821
* CCSDS Compliance: CCSDS 355.0-B-2 Section 7 (Management)
694822
*/
695-
void Crypto_Local_Config(void)
823+
int32_t Crypto_Local_Config(void)
696824
{
697825
// Initial TM configuration
698826
// tm_frame.tm_sec_header.spi = 1;
@@ -718,14 +846,16 @@ void Crypto_Local_Config(void)
718846
mc_log.blk[log_count].emv[3] = 0x41;
719847
mc_log.blk[log_count++].em_len = 4;
720848
}
849+
850+
return CRYPTO_LIB_SUCCESS;
721851
}
722852

723853
/**
724854
* @brief Function: Crypto_Local_Init
725855
*
726856
* CCSDS Compliance: CCSDS 355.0-B-2 Section 7 (Management)
727857
*/
728-
void Crypto_Local_Init(void)
858+
int32_t Crypto_Local_Init(void)
729859
{
730860
// Initialize CLCW
731861
clcw.cwt = 0; // Control Word Type "0"
@@ -752,14 +882,16 @@ void Crypto_Local_Init(void)
752882
report.bsaf = 0; // Invalid SPI Flag
753883
report.lspi = 0; // Last SPI Used
754884
report.snval = 0; // SN Value (LSB)
885+
886+
return CRYPTO_LIB_SUCCESS;
755887
}
756888

757889
/**
758890
* @brief Function: Crypto_Calc_CRC_Init_Table
759891
*
760892
* CCSDS Compliance: CCSDS 355.0-B-2 Section 7 (Management)
761893
*/
762-
void Crypto_Calc_CRC_Init_Table(void)
894+
int32_t Crypto_Calc_CRC_Init_Table(void)
763895
{
764896
uint16_t val;
765897
uint32_t poly = 0xEDB88320;
@@ -800,4 +932,6 @@ void Crypto_Calc_CRC_Init_Table(void)
800932
val ^= 0x9188;
801933
crc16Table[i] = val;
802934
}
935+
936+
return CRYPTO_LIB_SUCCESS;
803937
}

0 commit comments

Comments
 (0)