Merge pull request #8112 from cladmi/pr/crypto/input_len

crypto/ccm: fix input_len checking with length_encoding

sys/crypto/modes/ccm.c

@@ -129,6 +129,17 @@ int ccm_compute_adata_mac(cipher_t* cipher, uint8_t* auth_data,
 }
 
 
+/* Check if 'value' can be stored in 'num_bytes' */
+static inline int _fits_in_nbytes(size_t value, uint8_t num_bytes)
+{
+    /* Not allowed to shift more or equal than left operand width
+     * So we shift by maximum num bits of size_t -1 and compare to 1
+     */
+    unsigned shift = (8 * min(sizeof(size_t), num_bytes)) - 1;
+    return (value >> shift) <= 1;
+}
+
+
 int cipher_encrypt_ccm(cipher_t* cipher, uint8_t* auth_data, uint32_t auth_data_len,
                        uint8_t mac_length, uint8_t length_encoding,
                        uint8_t* nonce, size_t nonce_len,
@@ -136,7 +147,6 @@ int cipher_encrypt_ccm(cipher_t* cipher, uint8_t* auth_data, uint32_t auth_data_
                        uint8_t* output)
 {
     int len = -1;
-    uint32_t length_max;
     uint8_t nonce_counter[16] = {0}, mac_iv[16] = {0}, mac[16] = {0},
                                 stream_block[16] = {0}, zero_block[16] = {0}, block_size;
 
@@ -144,9 +154,8 @@ int cipher_encrypt_ccm(cipher_t* cipher, uint8_t* auth_data, uint32_t auth_data_
         return CCM_ERR_INVALID_MAC_LENGTH;
     }
 
-    length_max = 2 << (8 * length_encoding);
     if (length_encoding < 2 || length_encoding > 8 ||
-            input_len - auth_data_len > length_max) {
+            !_fits_in_nbytes(input_len, length_encoding)) {
         return CCM_ERR_INVALID_LENGTH_ENCODING;
     }
 
@@ -197,7 +206,6 @@ int cipher_decrypt_ccm(cipher_t* cipher, uint8_t* auth_data,
                        uint8_t* input, size_t input_len, uint8_t* plain)
 {
     int len = -1;
-    uint32_t length_max;
     uint8_t nonce_counter[16] = {0}, mac_iv[16] = {0}, mac[16] = {0},
                                 mac_recv[16] = {0}, stream_block[16] = {0}, zero_block[16] = {0},
                                         plain_len, block_size;
@@ -206,9 +214,8 @@ int cipher_decrypt_ccm(cipher_t* cipher, uint8_t* auth_data,
         return CCM_ERR_INVALID_MAC_LENGTH;
     }
 
-    length_max = 2 << (8 * length_encoding);
     if (length_encoding < 2 || length_encoding > 8 ||
-            input_len - auth_data_len > length_max) {
+            !_fits_in_nbytes(input_len, length_encoding)) {
         return CCM_ERR_INVALID_LENGTH_ENCODING;
     }
 

tests/unittests/tests-crypto/tests-crypto-modes-ccm.c

@@ -190,11 +190,71 @@ static void test_crypto_modes_ccm_decrypt(void)
 }
 
 
+typedef int (*func_ccm_t)(cipher_t*, uint8_t*, uint32_t, uint8_t, uint8_t,
+                          uint8_t*, size_t, uint8_t*, size_t, uint8_t*);
+
+static int _test_ccm_len(func_ccm_t func, uint8_t len_encoding,
+                         uint8_t *input, size_t input_len, size_t adata_len)
+{
+    int ret;
+    cipher_t cipher;
+    uint8_t mac_length = 8;
+    uint8_t nonce[15] = {0};
+    uint8_t key[16] = {0};
+
+    uint8_t nonce_len = nonce_and_len_encoding_size - len_encoding;
+    cipher_init(&cipher, CIPHER_AES_128, key, 16);
+
+    ret = func(&cipher, NULL, adata_len, mac_length, len_encoding,
+               nonce, nonce_len, input, input_len, data);
+    return ret;
+}
+
+
+/* Test length checking in ccm functions. */
+static void test_crypto_modes_ccm_check_len(void)
+{
+    int ret;
+    /* Just 1 to big to fit */
+    ret = _test_ccm_len(cipher_encrypt_ccm, 2, NULL, 1 << 16, 0);
+    TEST_ASSERT_EQUAL_INT(CCM_ERR_INVALID_LENGTH_ENCODING, ret);
+    ret = _test_ccm_len(cipher_decrypt_ccm, 2, NULL, 1 << 16, 0);
+    TEST_ASSERT_EQUAL_INT(CCM_ERR_INVALID_LENGTH_ENCODING, ret);
+
+    /* adata_len should not change the result (was wrong in previous implem) */
+    ret = _test_ccm_len(cipher_encrypt_ccm, 2, NULL, 1 << 16, 65535);
+    TEST_ASSERT_EQUAL_INT(CCM_ERR_INVALID_LENGTH_ENCODING, ret);
+    ret = _test_ccm_len(cipher_decrypt_ccm, 2, NULL, 1 << 16, 65535);
+    TEST_ASSERT_EQUAL_INT(CCM_ERR_INVALID_LENGTH_ENCODING, ret);
+
+    /* Valid length that were wrongly checked */
+    /* Check should work with len_encoding >= 4, test with 8 */
+    uint8_t input[8];
+    ret = _test_ccm_len(cipher_encrypt_ccm, 8, input, 8, 0);
+    TEST_ASSERT_MESSAGE(ret > 0, "Encryption : failed with valid input_len");
+
+    /* einput is encrypted value for
+     * - 8 * 0 input
+     * - All 0 nonce and key
+     * - adata_len == 0
+     * - mac_len == 8 and len_encoding = 8
+     */
+    uint8_t einput[16] = {
+        0xa2, 0x46, 0x75, 0xfc, 0x5f, 0x1b, 0x01, 0x37,
+        0x8a, 0x85, 0xd7, 0xf8, 0x42, 0x82, 0x6a, 0x63,
+    };
+
+    ret = _test_ccm_len(cipher_decrypt_ccm, 8, einput, 16, 0);
+    TEST_ASSERT_MESSAGE(ret > 0, "Decryption : failed with valid input_len");
+}
+
+
 Test* tests_crypto_modes_ccm_tests(void)
 {
     EMB_UNIT_TESTFIXTURES(fixtures) {
         new_TestFixture(test_crypto_modes_ccm_encrypt),
         new_TestFixture(test_crypto_modes_ccm_decrypt),
+        new_TestFixture(test_crypto_modes_ccm_check_len),
     };
 
     EMB_UNIT_TESTCALLER(crypto_modes_ccm_tests, NULL, NULL, fixtures);