Skip to content

Commit 70ea14c

Browse files
Donnie-IceDonnie-Ice
committed
[#375] Added edge checks to Crypto_Key_Update
1 parent 7f941bf commit 70ea14c

File tree

4 files changed

+108
-52
lines changed

4 files changed

+108
-52
lines changed

‎include/crypto_config.h

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -210,12 +210,14 @@
210210
#define AOS_FILL_SIZE 1145 /* bytes */
211211

212212
// SDLS Behavior Defines
213-
#define SDLS_OTAR_IV_OFFSET 2
214-
#define SDLS_KEYV_MAX_KEYS 21 /* keys */
215-
#define SDLS_IV_LEN 12 /* bytes */
216-
#define SDLS_KEYV_KEY_ID_LEN 2 /* bytes */
217-
#define SDLS_KEY_LEN 32 /* bytes */
218-
#define SDLS_KEYID_LEN 2 /* bytes */
213+
#define SDLS_OTAR_IV_OFFSET 2
214+
#define SDLS_MAX_KEY_UPDATES 16 /* keys */
215+
#define SDLS_KEYV_MAX_KEYS 21 /* keys */
216+
#define SDLS_MAX_KEY_UPDATE_LEN 35 /* bytes */
217+
#define SDLS_IV_LEN 12 /* bytes */
218+
#define SDLS_KEYV_KEY_ID_LEN 2 /* bytes */
219+
#define SDLS_KEY_LEN 32 /* bytes */
220+
#define SDLS_KEYID_LEN 2 /* bytes */
219221

220222
// TC Behavior Defines
221223
#define TC_SDLS_EP_VCID \

‎src/core/crypto.c

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -837,13 +837,21 @@ int32_t Crypto_Process_Extended_Procedure_Pdu(TC_t *tc_sdls_processed_frame, uin
837837

838838
// Subtract headers from total frame length
839839
// uint16_t max_tlv = tc_sdls_processed_frame->tc_header.fl - CCSDS_HDR_SIZE - CCSDS_PUS_SIZE - SDLS_TLV_HDR_SIZE;
840+
uint16_t max_tlv = (tc_sdls_processed_frame->tc_header.fl - CCSDS_HDR_SIZE - CCSDS_PUS_SIZE- SDLS_TLV_HDR_SIZE);
841+
len_ingest = len_ingest; // suppress error for now
840842
#ifdef CCSDS_DEBUG
841843
printf("Printing lengths for sanity check:\n");
842844
printf("\t Telecommand PDU Length: %d \n", tc_sdls_processed_frame->tc_pdu_len);
843-
printf("\t Received TLV Length: %d \n", 1);
844-
printf("\t Max possible TLV Length: %d \n", 1);
845-
printf("\t Calculated TLV length based on ");
845+
printf("\t Received TLV Length: %d \n", sdls_frame.tlv_pdu.hdr.pdu_len/8);
846+
printf("\t Max possible TLV Length: %d \n", max_tlv);
846847
#endif
848+
if ((sdls_frame.tlv_pdu.hdr.pdu_len/8) > max_tlv)
849+
{
850+
#ifdef PDU_DEBUG
851+
printf(KRED "PDU_LEN GT MAX_TLV\n" RESET);
852+
#endif
853+
return CRYPTO_LIB_ERR_BAD_TLV_LENGTH;
854+
}
847855
if (sdls_frame.hdr.pkt_length <= TLV_DATA_SIZE) // && (sdls_frame.hdr.pkt_length < max_tlv))
848856
{
849857
for (int x = 13; x < (13 + sdls_frame.hdr.pkt_length); x++)
@@ -853,6 +861,9 @@ int32_t Crypto_Process_Extended_Procedure_Pdu(TC_t *tc_sdls_processed_frame, uin
853861
}
854862
else
855863
{
864+
#ifdef PDU_DEBUG
865+
printf(KRED "Packet Header Length GT TLV_DATA_SIZE\n" RESET);
866+
#endif
856867
status = CRYPTO_LIB_ERR_BAD_TLV_LENGTH;
857868
return status;
858869
}
@@ -871,6 +882,16 @@ int32_t Crypto_Process_Extended_Procedure_Pdu(TC_t *tc_sdls_processed_frame, uin
871882
// Make sure TLV isn't larger than we have allocated, and it is sane given total frame length
872883
uint16_t max_tlv = tc_sdls_processed_frame->tc_header.fl - CCSDS_HDR_SIZE - SDLS_TLV_HDR_SIZE;
873884
len_ingest = len_ingest; // suppress error for now
885+
#ifdef PDU_DEBUG
886+
printf("PDU_LEN: %d\n",sdls_frame.tlv_pdu.hdr.pdu_len);
887+
#endif
888+
if ((sdls_frame.tlv_pdu.hdr.pdu_len / 8) > max_tlv)
889+
{
890+
#ifdef PDU_DEBUG
891+
printf(KRED "PDU_LEN GT MAX_TLV\n" RESET);
892+
#endif
893+
return CRYPTO_LIB_ERR_BAD_TLV_LENGTH;
894+
}
874895
if ((sdls_frame.hdr.pkt_length < TLV_DATA_SIZE) && (sdls_frame.hdr.pkt_length < max_tlv))
875896
{
876897
for (int x = 9; x < (9 + sdls_frame.hdr.pkt_length); x++)

‎src/core/crypto_key_mgmt.c

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -212,23 +212,43 @@ int32_t Crypto_Key_update(uint8_t state)
212212
{ // Local variables
213213
SDLS_KEY_BLK_t packet;
214214
int count = 0;
215-
int pdu_keys = sdls_frame.tlv_pdu.hdr.pdu_len / 2;
215+
int pdu_keys = (sdls_frame.tlv_pdu.hdr.pdu_len / 8) / 2;
216216
int32_t status;
217217
crypto_key_t *ekp = NULL;
218218
int x;
219+
int pdu_length = sdls_frame.tlv_pdu.hdr.pdu_len / 8;
220+
int frame_length = sdls_frame.hdr.pkt_length;
219221

220222
if (key_if == NULL)
221223
{
222224
status = CRYPTOGRAPHY_UNSUPPORTED_OPERATION_FOR_KEY_RING;
223225
return status;
224226
}
225-
227+
if (pdu_keys == 0)
228+
{
229+
#ifdef PDU_DEBUG
230+
printf(KYEL "PDU Length not long enough to hold key values\n" RESET);
231+
#endif
232+
}
233+
if ((state == KEY_DEACTIVATED || state == KEY_ACTIVE) && (pdu_length > SDLS_MAX_KEY_UPDATE_LEN || pdu_length > frame_length))
234+
{
235+
#ifdef PDU_DEBUG
236+
printf(KRED "PDU Length Exceded!\n" RESET);
237+
#endif
238+
return CRYPTO_LIB_ERROR;
239+
}
226240
#ifdef PDU_DEBUG
227241
printf("Key(s) ");
228242
#endif
229243
// Read in PDU
230244
for (x = 0; x < pdu_keys; x++)
231245
{
246+
if (x == SDLS_MAX_KEY_UPDATES)
247+
{
248+
printf(KRED "\nMax key updates exceded, exiting...\n" RESET);
249+
return CRYPTO_LIB_ERROR;
250+
}
251+
232252
packet.kblk[x].kid = (sdls_frame.tlv_pdu.data[count] << BYTE_LEN) | (sdls_frame.tlv_pdu.data[count + 1]);
233253
count = count + 2;
234254
#ifdef PDU_DEBUG

‎test/unit/ut_ep_key_mgmt.c

Lines changed: 54 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -899,27 +899,24 @@ UTEST(EP_KEY_MGMT, TLV_TESTS)
899899
crypto_key_t *ekp = NULL;
900900
int status = CRYPTO_LIB_SUCCESS;
901901

902-
// NOTE: Added Transfer Frame header to the plaintext
903-
char *buffer_nist_key_h = "000102030405060708090A0B0C0D0E0F000102030405060708090A0B0C0D0E0F";
904-
char *buffer_nist_iv_h = "b6ac8e4963f49207ffd6374b"; // The last valid IV that was seen by the SA
905-
906902
// These assume a max TLV of 494 as defined by TLV_DATA_SIZE
907903
// 2003001c00ff000000001880d039FFFF197f0b00030002008e1f6d21c4555555555555
908904
// 197f0b00 - pus
909905
// 03 - tag
910906
// 0002 - length
911907
// 008e - value
912908

913-
char *buffer_TLV_OVERRUN_h = "2003001c00ff000000001880d039FFFF197f0b0003FFFF008e1f6d21c4555555555555"; // FFFF = 65535
914-
char *buffer_TLV_MAX_MINUS_h ="2003001c00ff000000001880d03901EE197f0b000301ED008e1f6d21c4555555555555"; // 01ED = 493
915-
char *buffer_TLV_MAX_h = "2003001c00ff000000001880d03901EE197f0b000301EE008e1f6d21c4555555555555"; // 01EE = 494
916-
char *buffer_TLV_MAX_PLUS_h = "2003001c00ff000000001880d03901EF197f0b000301EF008e1f6d21c4555555555555"; // 01EF = 495
917-
char *buffer_TLV_ONE_h = "2003001c00ff000000001880d0390001197f0b0003000100811f6d21c4555555555555"; // 0001 = 1
918-
char *buffer_TLV_ZERO_h = "2003001c00ff000000001880d0390000197f0b00030000008e1f6d21c4555555555555"; // 0000 = 0
919-
920-
uint8_t *buffer_nist_iv_b, *buffer_nist_key_b, *buffer_TLV_OVERRUN_b, *buffer_TLV_MAX_MINUS_b, *buffer_TLV_MAX_b,
921-
*buffer_TLV_MAX_PLUS_b, *buffer_TLV_ONE_b, *buffer_TLV_ZERO_b = NULL;
922-
int buffer_nist_iv_len, buffer_nist_key_len, buffer_TLV_OVERRUN_len, buffer_TLV_MAX_MINUS_len, buffer_TLV_MAX_len,
909+
char *buffer_TLV_OVERRUN_h = "2003001c00ff000000001880d039FFFF197f0b0003FFFF008e1f6d21c4555555555555"; // FFFF = 65535
910+
char *buffer_TLV_ONE_KEY_h = "2003001c00ff000000001880d0390012197f0b00030010008e1f6d21c4555555555555"; // 0010 = 16
911+
char *buffer_TLV_MAX_h = "2003003a00ff000000001880d03901ee197f0b000301000080008100820083008400850086008700880089008a008b008c008d008e008f1f6d21c4"; // 01EE = 494
912+
char *buffer_TLV_MAX_BAD_LEN_h = "2003001a00ff000000001880d03901ee197f0b000301000080008100821f6d21c45555"; // 01EE = 494
913+
char *buffer_TLV_TEN_KEYS_h = "2003003400ff000000001880d0390024197f0b000300A000800081008200830084008500860087008800891f6d21c4555555555555"; // 00A0 = 160
914+
char *buffer_TLV_ONE_BIT_LEN_h = "2003001c00ff000000001880d0390019197f0b00030001008e1f6d21c4555555555555"; // 0001 = 1
915+
char *buffer_TLV_ZERO_h = "2003001a00ff000000001880d0390017197f0b000300001f6d21c4555555555555"; // 0000 = 0
916+
917+
uint8_t *buffer_TLV_OVERRUN_b, *buffer_TLV_ONE_KEY_b, *buffer_TLV_MAX_b, *buffer_TLV_MAX_BAD_LEN_b,
918+
*buffer_TLV_TEN_KEYS_b, *buffer_TLV_ONE_BIT_LEN_b, *buffer_TLV_ZERO_b = NULL;
919+
int buffer_TLV_OVERRUN_len, buffer_TLV_MAX_MINUS_len, buffer_TLV_MAX_len, buffer_TLV_MAX_BAD_LEN_len,
923920
buffer_TLV_MAX_PLUS_len, buffer_TLV_ONE_len, buffer_TLV_ZERO_len = 0;
924921

925922
// Setup Processed Frame For Decryption
@@ -943,57 +940,73 @@ UTEST(EP_KEY_MGMT, TLV_TESTS)
943940
test_association->shsnf_len = 2;
944941
test_association->arsn_len = 2;
945942
test_association->arsnw = 5;
946-
947-
// Insert key into keyring of SA 9
948-
hex_conversion(buffer_nist_key_h, (char **)&buffer_nist_key_b, &buffer_nist_key_len);
949-
ekp = key_if->get_key(142);
950-
memcpy(ekp->value, buffer_nist_key_b, buffer_nist_key_len);
951-
ekp->key_state = KEY_ACTIVE;
943+
944+
// set all keys to active
945+
for (int x = 128; x <= 143; x++)
946+
{
947+
ekp = key_if->get_key(x);
948+
ekp->key_state = KEY_ACTIVE;
949+
}
952950

953951
// Convert frames that will be processed
954952
hex_conversion(buffer_TLV_OVERRUN_h, (char **)&buffer_TLV_OVERRUN_b, &buffer_TLV_OVERRUN_len);
955-
hex_conversion(buffer_TLV_MAX_MINUS_h, (char **)&buffer_TLV_MAX_MINUS_b, &buffer_TLV_MAX_MINUS_len);
953+
hex_conversion(buffer_TLV_ONE_KEY_h, (char **)&buffer_TLV_ONE_KEY_b, &buffer_TLV_MAX_MINUS_len);
956954
hex_conversion(buffer_TLV_MAX_h, (char **)&buffer_TLV_MAX_b, &buffer_TLV_MAX_len);
957-
hex_conversion(buffer_TLV_MAX_PLUS_h, (char **)&buffer_TLV_MAX_PLUS_b, &buffer_TLV_MAX_PLUS_len);
955+
hex_conversion(buffer_TLV_MAX_BAD_LEN_h, (char **)&buffer_TLV_MAX_BAD_LEN_b, &buffer_TLV_MAX_BAD_LEN_len);
956+
hex_conversion(buffer_TLV_TEN_KEYS_h, (char **)&buffer_TLV_TEN_KEYS_b, &buffer_TLV_MAX_PLUS_len);
958957
hex_conversion(buffer_TLV_ZERO_h, (char **)&buffer_TLV_ZERO_b, &buffer_TLV_ZERO_len);
959-
hex_conversion(buffer_TLV_ONE_h, (char **)&buffer_TLV_ONE_b, &buffer_TLV_ONE_len);
960-
// Convert/Set input IV
961-
hex_conversion(buffer_nist_iv_h, (char **)&buffer_nist_iv_b, &buffer_nist_iv_len);
962-
memcpy(test_association->iv, buffer_nist_iv_b, buffer_nist_iv_len);
958+
hex_conversion(buffer_TLV_ONE_BIT_LEN_h, (char **)&buffer_TLV_ONE_BIT_LEN_b, &buffer_TLV_ONE_len);
963959

964960
printf(KGRN "Checking for TLV overrun, should fail... \n" RESET);
965961
status = Crypto_TC_ProcessSecurity(buffer_TLV_OVERRUN_b, &buffer_TLV_OVERRUN_len, &tc_nist_processed_frame);
966962
ASSERT_EQ(CRYPTO_LIB_ERR_BAD_TLV_LENGTH, status);
967963

968-
printf(KGRN "Checking for TLV MAX - 1, should pass... \n" RESET);
969-
status = Crypto_TC_ProcessSecurity(buffer_TLV_MAX_MINUS_b, &buffer_TLV_MAX_MINUS_len, &tc_nist_processed_frame);
964+
printf(KGRN "Checking for 1 key (16 bits), should pass... \n" RESET);
965+
status = Crypto_TC_ProcessSecurity(buffer_TLV_ONE_KEY_b, &buffer_TLV_MAX_MINUS_len, &tc_nist_processed_frame);
970966
ASSERT_EQ(CRYPTO_LIB_SUCCESS, status);
971967

972-
printf(KGRN "Checking for TLV MAX, should pass... \n" RESET);
968+
// set all keys to active
969+
for (int x = 128; x <= 143; x++)
970+
{
971+
ekp = key_if->get_key(x);
972+
ekp->key_state = KEY_ACTIVE;
973+
}
974+
975+
printf(KGRN "Checking for 32 keys (494 bits), should pass... \n" RESET);
973976
status = Crypto_TC_ProcessSecurity(buffer_TLV_MAX_b, &buffer_TLV_MAX_len, &tc_nist_processed_frame);
974977
ASSERT_EQ(CRYPTO_LIB_SUCCESS, status);
975978

976-
printf(KGRN "Checking for TLV MAX + 1, should fail... \n" RESET);
977-
status = Crypto_TC_ProcessSecurity(buffer_TLV_MAX_PLUS_b, &buffer_TLV_MAX_PLUS_len, &tc_nist_processed_frame);
979+
// set all keys to active
980+
for (int x = 128; x <= 137; x++)
981+
{
982+
ekp = key_if->get_key(x);
983+
ekp->key_state = KEY_ACTIVE;
984+
}
985+
986+
printf(KGRN "Checking for 10 keys (160 bits), should pass... \n" RESET);
987+
status = Crypto_TC_ProcessSecurity(buffer_TLV_TEN_KEYS_b, &buffer_TLV_MAX_PLUS_len, &tc_nist_processed_frame);
988+
ASSERT_EQ(CRYPTO_LIB_SUCCESS, status);
989+
990+
printf(KGRN "Checking for 3 keys with bad pdu len (512 bits), should fail... \n" RESET);
991+
status = Crypto_TC_ProcessSecurity(buffer_TLV_MAX_BAD_LEN_b, &buffer_TLV_MAX_BAD_LEN_len, &tc_nist_processed_frame);
978992
ASSERT_EQ(CRYPTO_LIB_ERR_BAD_TLV_LENGTH, status);
979993

980-
printf(KGRN "Checking for TLV length of 1, should pass... \n" RESET);
981-
status = Crypto_TC_ProcessSecurity(buffer_TLV_ONE_b, &buffer_TLV_ONE_len, &tc_nist_processed_frame);
994+
printf(KGRN "Checking for TLV length of 1 bit, should pass... \n" RESET);
995+
status = Crypto_TC_ProcessSecurity(buffer_TLV_ONE_BIT_LEN_b, &buffer_TLV_ONE_len, &tc_nist_processed_frame);
982996
ASSERT_EQ(CRYPTO_LIB_SUCCESS, status);
983997

984-
printf(KGRN "Checking for TLV length of 0, should ????... \n" RESET);
985-
status = Crypto_TC_ProcessSecurity(buffer_TLV_ONE_b, &buffer_TLV_ONE_len, &tc_nist_processed_frame);
986-
ASSERT_EQ(-110000, status);
998+
printf(KGRN "Checking for TLV length of 0 bits, should pass... \n" RESET);
999+
status = Crypto_TC_ProcessSecurity(buffer_TLV_ZERO_b, &buffer_TLV_ZERO_len, &tc_nist_processed_frame);
1000+
ASSERT_EQ(CRYPTO_LIB_SUCCESS, status);
9871001

9881002
printf("\n");
9891003
Crypto_Shutdown();
9901004
free(ptr_enc_frame);
991-
free(buffer_nist_iv_b);
992-
free(buffer_nist_key_b);
993-
free(buffer_TLV_MAX_MINUS_b);
1005+
free(buffer_TLV_ONE_KEY_b);
9941006
free(buffer_TLV_MAX_b);
995-
free(buffer_TLV_MAX_PLUS_b);
996-
free(buffer_TLV_ONE_b);
1007+
free(buffer_TLV_MAX_BAD_LEN_b);
1008+
free(buffer_TLV_TEN_KEYS_b);
1009+
free(buffer_TLV_ONE_BIT_LEN_b);
9971010
free(buffer_TLV_ZERO_b);
9981011
free(buffer_TLV_OVERRUN_b);
9991012
}

0 commit comments

Comments
 (0)