Skip to content

Commit

Permalink
Merge pull request #86 from nasa/84-cleartext-bug-reported-by-jpl
Browse files Browse the repository at this point in the history 
84 cleartext bug reported by jpl
  • Loading branch information
@dccutrig
dccutrig committed Mar 9, 2022
2 parents 492c3d0 + 9fec569 commit b7d23e2
Show file tree
Hide file tree
Showing 5 changed files with 106 additions and 38 deletions.
1 change: 1 addition & 0 deletions include/crypto_error.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -78,5 +78,6 @@
#define CRYPTO_LIB_ERR_NULL_SA (-25)
#define CRYPTO_LIB_ERR_UNSUPPORTED_ACS (-26)
#define CRYPTO_LIB_ERR_ENCRYPTION_ERROR (-27)
#define CRYPTO_LIB_ERR_INVALID_SA_CONFIGURATION (-28)

#endif //_crypto_error_h_
59 changes: 24 additions & 35 deletions src/src_main/crypto_tc.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -318,18 +318,23 @@ int32_t Crypto_TC_ApplySecurity(const uint8_t* p_in_frame, const uint16_t in_fra
index += 2;

// Set initialization vector if specified
if ((sa_service_type == SA_AUTHENTICATION) || (sa_service_type == SA_AUTHENTICATED_ENCRYPTION) ||
(sa_service_type == SA_ENCRYPTION))
{
#ifdef SA_DEBUG
printf(KYEL "Using IV value:\n\t");
for (i = 0; i < sa_ptr->shivf_len; i++)
if (sa_ptr->shivf_len > 0 && sa_ptr->iv != NULL)
{
printf("%02x", *(sa_ptr->iv + i));
printf(KYEL "Using IV value:\n\t");
for (i = 0; i < sa_ptr->shivf_len; i++)
{
printf("%02x", *(sa_ptr->iv + i));
}
printf("\n" RESET);
}
printf("\n" RESET);
#endif

if (sa_ptr->shivf_len > 0 && sa_ptr->iv == NULL)
{
return CRYPTO_LIB_ERR_INVALID_SA_CONFIGURATION;
}
else
{
for (i = 0; i < sa_ptr->shivf_len; i++)
{
// Copy in IV from SA
Expand All @@ -345,13 +350,18 @@ int32_t Crypto_TC_ApplySecurity(const uint8_t* p_in_frame, const uint16_t in_fra
** for an SA, the Sequence Number field shall be zero octets in length.
** Reference CCSDS 3550b1
*/
// TODO: Workout ARSN vs SN and when they may
// or may not be the same or different field
for (i = 0; i < sa_ptr->shsnf_len; i++)
if ((sa_ptr->shsnf_len > 0 || sa_ptr->arsn_len > 0) && sa_ptr->arsn == NULL)
{
// Copy in ARSN from SA
*(p_new_enc_frame + index) = *(sa_ptr->arsn + i);
index++;
return CRYPTO_LIB_ERR_INVALID_SA_CONFIGURATION;
}
else
{
for (i = 0; i < sa_ptr->shsnf_len; i++)
{
// Copy in ARSN from SA
*(p_new_enc_frame + index) = *(sa_ptr->arsn + i);
index++;
}
}

// Set security header padding if specified
Expand Down Expand Up @@ -388,27 +398,6 @@ int32_t Crypto_TC_ApplySecurity(const uint8_t* p_in_frame, const uint16_t in_fra
tf_payload_len = temp_tc_header.fl - TC_FRAME_HEADER_SIZE - segment_hdr_len - fecf_len + 1;
memcpy((p_new_enc_frame + index), (p_in_frame + TC_FRAME_HEADER_SIZE + segment_hdr_len), tf_payload_len);

/*
** Begin Security Trailer Fields
*/

// Set MAC Field if present
/*
** May be present and unused if switching between clear and authenticated
** CCSDS 3550b1 4.1.2.3
*/
// By leaving MAC as zeros, can use index for encryption output
// for (i=0; i < temp_SA.stmacf_len; i++)
// {
// // Temp fill MAC
// *(p_new_enc_frame + index) = 0x00;
// index++;
// }

/*
** End Security Trailer Fields
*/

/*
** Begin Authentication / Encryption
*/
Expand Down
7 changes: 4 additions & 3 deletions src/src_main/sadb_routine_inmemory.template.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -79,9 +79,10 @@ int32_t sadb_config(void)
sa[1].sa_state = SA_OPERATIONAL;
sa[1].est = 0;
sa[1].ast = 0;
sa[1].shivf_len = 2;
sa[1].iv = (uint8_t*) calloc(1, sa[1].shivf_len * sizeof(uint8_t));
sa[1].arsn_len = 1;
sa[1].shivf_len = 0;
sa[1].shsnf_len = 2;
sa[1].arsn_len = 2;
sa[1].arsn = (uint8_t*) calloc(1, sa[1].arsn_len * sizeof(uint8_t));
sa[1].arsnw_len = 1;
sa[1].arsnw = 5;
sa[1].gvcid_tc_blk.tfvn = 0;
Expand Down
35 changes: 35 additions & 0 deletions util/src_util/et_dt_validation.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1913,4 +1913,39 @@ UTEST(NIST_DEC_CMAC_VALIDATION, AES_CMAC_256_PT_128_TEST_1)
// sadb_routine->sadb_close();
}

/**
* @brief Unit Test: Encrypts a frame, then decrypts the output to ensure the reverse doesn't error
**/
UTEST(PLAINTEXT, ENCRYPT_DECRYPT)
{
int32_t status = CRYPTO_LIB_ERROR;
uint8_t* ptr_enc_frame = NULL;
uint16_t enc_frame_len = 0;
// Setup & Initialize CryptoLib
Crypto_Config_CryptoLib(SADB_TYPE_INMEMORY, CRYPTOGRAPHY_TYPE_LIBGCRYPT, CRYPTO_TC_CREATE_FECF_TRUE, TC_PROCESS_SDLS_PDUS_TRUE, TC_HAS_PUS_HDR,
TC_IGNORE_SA_STATE_TRUE, TC_IGNORE_ANTI_REPLAY_TRUE, TC_UNIQUE_SA_PER_MAP_ID_FALSE,
TC_CHECK_FECF_TRUE, 0x3F);
Crypto_Config_Add_Gvcid_Managed_Parameter(0, 0x0003, 0, TC_HAS_FECF, TC_NO_SEGMENT_HDRS);
Crypto_Config_Add_Gvcid_Managed_Parameter(0, 0x0003, 1, TC_HAS_FECF, TC_NO_SEGMENT_HDRS);
Crypto_Init();

char* jpl_frame_pt_h = "2003001c00ff000100001880d03e000a197f0b000300020093d4ba21c4555555555555";
uint8_t* jpl_frame_pt_b = NULL;
int jpl_frame_pt_len = 0;
TC_t* tc_sdls_processed_frame;
tc_sdls_processed_frame = calloc(1, sizeof(uint8_t) * TC_SIZE);

// Convert input jpl frame
hex_conversion(jpl_frame_pt_h, (char**) &jpl_frame_pt_b, &jpl_frame_pt_len);

// Apply, save the generated frame
status = Crypto_TC_ApplySecurity(jpl_frame_pt_b, jpl_frame_pt_len, &ptr_enc_frame, &enc_frame_len);
ASSERT_EQ(status, CRYPTO_LIB_SUCCESS);

// // Process the generated frame
int len = (int)enc_frame_len;
status = Crypto_TC_ProcessSecurity(ptr_enc_frame, &len, tc_sdls_processed_frame);
ASSERT_EQ(status, CRYPTO_LIB_SUCCESS);
}

UTEST_MAIN();
42 changes: 42 additions & 0 deletions util/src_util/ut_crypto.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -280,4 +280,46 @@ UTEST(CRYPTO_C, GET_ACS_ALGO)
ASSERT_EQ(libgcrypt_algo, CRYPTO_LIB_ERR_UNSUPPORTED_ACS);
}

/*
* @brief Unit Test: Test that an SA set to use IV/ARSN without mallocing doesn't segfault and returns an error
**/
UTEST(INVALID_SA_CONFIGS, INVALID_IV_ARSN)
{
int32_t status = CRYPTO_LIB_ERROR;
uint8_t* ptr_enc_frame = NULL;
uint16_t enc_frame_len = 0;
// Setup & Initialize CryptoLib
Crypto_Config_CryptoLib(SADB_TYPE_INMEMORY, CRYPTOGRAPHY_TYPE_LIBGCRYPT, CRYPTO_TC_CREATE_FECF_TRUE, TC_PROCESS_SDLS_PDUS_TRUE, TC_HAS_PUS_HDR,
TC_IGNORE_SA_STATE_TRUE, TC_IGNORE_ANTI_REPLAY_TRUE, TC_UNIQUE_SA_PER_MAP_ID_FALSE,
TC_CHECK_FECF_TRUE, 0x3F);
Crypto_Config_Add_Gvcid_Managed_Parameter(0, 0x0003, 0, TC_HAS_FECF, TC_NO_SEGMENT_HDRS);
Crypto_Config_Add_Gvcid_Managed_Parameter(0, 0x0003, 1, TC_HAS_FECF, TC_NO_SEGMENT_HDRS);
Crypto_Init();

char* jpl_frame_pt_h = "2003001c00ff000100001880d03e000a197f0b000300020093d4ba21c4555555555555";
uint8_t* jpl_frame_pt_b = NULL;
int jpl_frame_pt_len = 0;

// Expose/setup SAs for testing
SecurityAssociation_t* test_association = NULL;
test_association = malloc(sizeof(SecurityAssociation_t) * sizeof(uint8_t));
sadb_routine->sadb_get_sa_from_spi(1, &test_association);

// Convert input jpl frame
hex_conversion(jpl_frame_pt_h, (char**) &jpl_frame_pt_b, &jpl_frame_pt_len);

// Should fail, as SA will be set to use ARSN, but ARSN pointer is NULL
free(test_association->arsn);
test_association->arsn = NULL;
status = Crypto_TC_ApplySecurity(jpl_frame_pt_b, jpl_frame_pt_len, &ptr_enc_frame, &enc_frame_len);
ASSERT_EQ(CRYPTO_LIB_ERR_INVALID_SA_CONFIGURATION, status);

// Should fail, as SA will be set to use IV, but IV pointer is NULL
free(test_association->iv);
test_association->iv = NULL;
test_association->shivf_len = 12;
status = Crypto_TC_ApplySecurity(jpl_frame_pt_b, jpl_frame_pt_len, &ptr_enc_frame, &enc_frame_len);
ASSERT_EQ(CRYPTO_LIB_ERR_INVALID_SA_CONFIGURATION, status);
}

UTEST_MAIN();

0 comments on commit b7d23e2

Please sign in to comment.