From 5a4cb3de6fe849551b541f0873d1026200cb47c9 Mon Sep 17 00:00:00 2001 From: Martin Turon Date: Tue, 2 Aug 2022 15:36:59 -0700 Subject: [PATCH 1/5] [privacy] Add AES_CTR_encrypt/decrypt with tests. --- src/credentials/GroupDataProviderImpl.cpp | 6 +- src/crypto/CHIPCryptoPAL.cpp | 10 +++ src/crypto/CHIPCryptoPAL.h | 21 ++++- src/crypto/tests/CHIPCryptoPALTest.cpp | 98 +++++++++++++++++++++++ 4 files changed, 132 insertions(+), 3 deletions(-) diff --git a/src/credentials/GroupDataProviderImpl.cpp b/src/credentials/GroupDataProviderImpl.cpp index 0626a8e73baa97..d286d406714ee6 100644 --- a/src/credentials/GroupDataProviderImpl.cpp +++ b/src/credentials/GroupDataProviderImpl.cpp @@ -1856,13 +1856,15 @@ CHIP_ERROR GroupDataProviderImpl::GroupKeyContext::MessageDecrypt(const ByteSpan CHIP_ERROR GroupDataProviderImpl::GroupKeyContext::PrivacyEncrypt(const ByteSpan & input, const ByteSpan & nonce, MutableByteSpan & output) const { - return CHIP_ERROR_NOT_IMPLEMENTED; + return Crypto::AES_CTR_crypt(input.data(), input.size(), mPrivacyKey, Crypto::kAES_CCM128_Key_Length, nonce.data(), + nonce.size(), output.data()); } CHIP_ERROR GroupDataProviderImpl::GroupKeyContext::PrivacyDecrypt(const ByteSpan & input, const ByteSpan & nonce, MutableByteSpan & output) const { - return CHIP_ERROR_NOT_IMPLEMENTED; + return Crypto::AES_CTR_crypt(input.data(), input.size(), mPrivacyKey, Crypto::kAES_CCM128_Key_Length, nonce.data(), + nonce.size(), output.data()); } GroupDataProviderImpl::GroupSessionIterator * GroupDataProviderImpl::IterateGroupSessions(uint16_t session_id) diff --git a/src/crypto/CHIPCryptoPAL.cpp b/src/crypto/CHIPCryptoPAL.cpp index 859f5809e4b742..4fa157c0ef7ae1 100644 --- a/src/crypto/CHIPCryptoPAL.cpp +++ b/src/crypto/CHIPCryptoPAL.cpp @@ -706,6 +706,16 @@ CHIP_ERROR EcdsaAsn1SignatureToRaw(size_t fe_length_bytes, const ByteSpan & asn1 return CHIP_NO_ERROR; } +CHIP_ERROR AES_CTR_crypt(const uint8_t * input, size_t input_length, const uint8_t * key, size_t key_length, const uint8_t * nonce, + size_t nonce_length, uint8_t * output) +{ + // Discard tag portion of CCM to apply only CTR mode encryption/decryption. + constexpr size_t kTagLen = 16; + uint8_t tag[kTagLen]; + + return AES_CCM_encrypt(input, input_length, nullptr, 0, key, key_length, nonce, nonce_length, output, tag, kTagLen); +} + CHIP_ERROR GenerateCompressedFabricId(const Crypto::P256PublicKey & root_public_key, uint64_t fabric_id, MutableByteSpan & out_compressed_fabric_id) { diff --git a/src/crypto/CHIPCryptoPAL.h b/src/crypto/CHIPCryptoPAL.h index 544a554dcb31eb..66aa904d9b68d9 100644 --- a/src/crypto/CHIPCryptoPAL.h +++ b/src/crypto/CHIPCryptoPAL.h @@ -614,11 +614,30 @@ CHIP_ERROR AES_CCM_encrypt(const uint8_t * plaintext, size_t plaintext_length, c * @param plaintext Buffer to write plaintext into * @return Returns a CHIP_ERROR on error, CHIP_NO_ERROR otherwise **/ - CHIP_ERROR AES_CCM_decrypt(const uint8_t * ciphertext, size_t ciphertext_length, const uint8_t * aad, size_t aad_length, const uint8_t * tag, size_t tag_length, const uint8_t * key, size_t key_length, const uint8_t * nonce, size_t nonce_length, uint8_t * plaintext); +/** + * @brief A function that implements AES-CCM decryption + * + * This implements the AES-CTR-Encrypt/Decrypt() cryptographic primitives per sections + * 3.7.1 and 3.7.2 of the specification. For an empty ciphertext, the user of the API + * can provide an empty string, or a nullptr, and provide ciphertext_length as 0. + * The output buffer, plaintext can also be an empty string, or a nullptr for this case. + * + * @param input Input text to encrypt/decrypt + * @param input_length Length of ciphertext + * @param key Decryption key + * @param key_length Length of Decryption key (in bytes) + * @param nonce Encryption nonce + * @param nonce_length Length of encryption nonce + * @param output Buffer to write output into + * @return Returns a CHIP_ERROR on error, CHIP_NO_ERROR otherwise + **/ +CHIP_ERROR AES_CTR_crypt(const uint8_t * input, size_t input_length, const uint8_t * key, size_t key_length, const uint8_t * nonce, + size_t nonce_length, uint8_t * output); + /** * @brief Generate a PKCS#10 CSR, usable for Matter, from a P256Keypair. * diff --git a/src/crypto/tests/CHIPCryptoPALTest.cpp b/src/crypto/tests/CHIPCryptoPALTest.cpp index 485ead2cf99322..a0106761aa79da 100644 --- a/src/crypto/tests/CHIPCryptoPALTest.cpp +++ b/src/crypto/tests/CHIPCryptoPALTest.cpp @@ -161,6 +161,103 @@ static int test_entropy_source(void * data, uint8_t * output, size_t len, size_t return 0; } +struct AesCtrTestEntry +{ + const uint8_t * key; ///< Key to use for AES-CTR-128 encryption/decryption -- 16 byte length + const uint8_t * nonce; ///< Nonce to use for AES-CTR-128 encryption/decryption -- 13 byte length + const uint8_t * plaintext; + size_t plaintextLen; + const uint8_t * ciphertext; + size_t ciphertextLen; +}; + +/** + * Test vectors for AES-CTR-128 encryption/decryption. + * + * Sourced from: https://www.ietf.org/rfc/rfc3686.txt (Section 6) + * Modified to use `IV = flags byte | 13 byte nonce | u16 counter` as defined in NIST SP 800-38A. + * + * All AES-CCM test vectors can be used as well, but those are already called to validate underlying AES-CCM functionality. + */ +const AesCtrTestEntry theAesCtrTestVector[] = { + { + .key = (const uint8_t *) "\xae\x68\x52\xf8\x12\x10\x67\xcc\x4b\xf7\xa5\x76\x55\x77\xf3\x9e", + .nonce = (const uint8_t *) "\x00\x00\x00\x30\x00\x00\x00\x00\x00\x00\x00\x00\x00", + .plaintext = (const uint8_t *) "\x53\x69\x6e\x67\x6c\x65\x20\x62\x6c\x6f\x63\x6b\x20\x6d\x73\x67", + .plaintextLen = 16, + .ciphertext = (const uint8_t *) "\x0d\x0a\x6b\x6d\xc1\xf6\x9b\x4d\x14\xca\x4c\x15\x42\x22\x42\xc4", + .ciphertextLen = 16, + }, + { + .key = (const uint8_t *) "\x7e\x24\x06\x78\x17\xfa\xe0\xd7\x43\xd6\xce\x1f\x32\x53\x91\x63", + .nonce = (const uint8_t *) "\x00\x6c\xb6\xdb\xc0\x54\x3b\x59\xda\x48\xd9\x0b\x00", + .plaintext = (const uint8_t *) "\x00\x01\x02\x03\x04\x05\x06\x07\x08\x09\x0a\x0b\x0c\x0d\x0e\x0f" + "\x10\x11\x12\x13\x14\x15\x16\x17\x18\x19\x1a\x1b\x1c\x1d\x1e\x1f", + .plaintextLen = 32, + .ciphertext = (const uint8_t *) "\x4f\x3d\xf9\x49\x15\x88\x4d\xe0\xdc\x0e\x30\x95\x0d\xe7\xa6\xe9" + "\x5a\x91\x7e\x1d\x06\x42\x22\xdb\x2f\x6e\xc7\x3d\x99\x4a\xd9\x5f", + .ciphertextLen = 32, + } +}; + +constexpr size_t kAesCtrTestVectorSize = sizeof(theAesCtrTestVector) / sizeof(theAesCtrTestVector[0]); + +constexpr size_t KEY_LENGTH = 16; +constexpr size_t NONCE_LENGTH = 13; + +static void TestAES_CTR_128_Encrypt(nlTestSuite * inSuite, const AesCtrTestEntry * vector) +{ + chip::Platform::ScopedMemoryBuffer outBuffer; + outBuffer.Alloc(vector->ciphertextLen); + NL_TEST_ASSERT(inSuite, outBuffer); + + CHIP_ERROR err = AES_CTR_crypt(vector->plaintext, vector->plaintextLen, vector->key, KEY_LENGTH, vector->nonce, NONCE_LENGTH, + outBuffer.Get()); + NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR); + + bool outputMatches = memcmp(outBuffer.Get(), vector->ciphertext, vector->ciphertextLen) == 0; + NL_TEST_ASSERT(inSuite, outputMatches); + if (!outputMatches) + { + printf("\n Test failed due to mismatching ciphertext\n"); + } +} + +static void TestAES_CTR_128_Decrypt(nlTestSuite * inSuite, const AesCtrTestEntry * vector) +{ + chip::Platform::ScopedMemoryBuffer outBuffer; + outBuffer.Alloc(vector->plaintextLen); + NL_TEST_ASSERT(inSuite, outBuffer); + + CHIP_ERROR err = AES_CTR_crypt(vector->ciphertext, vector->ciphertextLen, vector->key, KEY_LENGTH, vector->nonce, NONCE_LENGTH, + outBuffer.Get()); + NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR); + + bool outputMatches = memcmp(outBuffer.Get(), vector->plaintext, vector->plaintextLen) == 0; + NL_TEST_ASSERT(inSuite, outputMatches); + if (!outputMatches) + { + printf("\n Test failed due to mismatching ciphertext\n"); + } +} + +static void TestAES_CTR_128CryptTestVectors(nlTestSuite * inSuite, void * inContext) +{ + HeapChecker heapChecker(inSuite); + int numOfTestsRan = 0; + for (size_t vectorIndex = 0; vectorIndex < kAesCtrTestVectorSize; vectorIndex++) + { + const AesCtrTestEntry * vector = &theAesCtrTestVector[vectorIndex]; + if (vector->plaintextLen > 0) + { + numOfTestsRan++; + TestAES_CTR_128_Encrypt(inSuite, vector); + TestAES_CTR_128_Decrypt(inSuite, vector); + } + } + NL_TEST_ASSERT(inSuite, numOfTestsRan > 0); +} + static void TestAES_CCM_128EncryptTestVectors(nlTestSuite * inSuite, void * inContext) { HeapChecker heapChecker(inSuite); @@ -2279,6 +2376,7 @@ static const nlTest sTests[] = { NL_TEST_DEF("Test decrypting AES-CCM-128 invalid key", TestAES_CCM_128DecryptInvalidKey), NL_TEST_DEF("Test decrypting AES-CCM-128 invalid nonce", TestAES_CCM_128DecryptInvalidNonceLen), NL_TEST_DEF("Test decrypting AES-CCM-128 Containers", TestAES_CCM_128Containers), + NL_TEST_DEF("Test encrypt/decrypt AES-CTR-128 test vectors", TestAES_CTR_128CryptTestVectors), NL_TEST_DEF("Test ASN.1 signature conversion routines", TestAsn1Conversions), NL_TEST_DEF("Test Integer to ASN.1 DER conversion", TestRawIntegerToDerValidCases), NL_TEST_DEF("Test Integer to ASN.1 DER conversion error cases", TestRawIntegerToDerInvalidCases), From b7512ce17f5a632e99a164b473534f8378a9bfd2 Mon Sep 17 00:00:00 2001 From: Martin Turon Date: Tue, 23 Aug 2022 17:02:46 +0000 Subject: [PATCH 2/5] [style] Remove magic words. --- src/crypto/CHIPCryptoPAL.cpp | 2 +- src/crypto/CHIPCryptoPAL.h | 8 +++++--- src/crypto/tests/CHIPCryptoPALTest.cpp | 4 ++-- 3 files changed, 8 insertions(+), 6 deletions(-) diff --git a/src/crypto/CHIPCryptoPAL.cpp b/src/crypto/CHIPCryptoPAL.cpp index 4fa157c0ef7ae1..4a878eecfc5eeb 100644 --- a/src/crypto/CHIPCryptoPAL.cpp +++ b/src/crypto/CHIPCryptoPAL.cpp @@ -710,7 +710,7 @@ CHIP_ERROR AES_CTR_crypt(const uint8_t * input, size_t input_length, const uint8 size_t nonce_length, uint8_t * output) { // Discard tag portion of CCM to apply only CTR mode encryption/decryption. - constexpr size_t kTagLen = 16; + constexpr size_t kTagLen = Crypto::kAES_CCM128_Tag_Length; uint8_t tag[kTagLen]; return AES_CCM_encrypt(input, input_length, nullptr, 0, key, key_length, nonce, nonce_length, output, tag, kTagLen); diff --git a/src/crypto/CHIPCryptoPAL.h b/src/crypto/CHIPCryptoPAL.h index 66aa904d9b68d9..3882e1f64b07d7 100644 --- a/src/crypto/CHIPCryptoPAL.h +++ b/src/crypto/CHIPCryptoPAL.h @@ -78,6 +78,8 @@ constexpr size_t kP256_PublicKey_Length = CHIP_CRYPTO_PUBLIC_KEY_SIZE_BYTES; constexpr size_t kAES_CCM128_Key_Length = 128u / 8u; constexpr size_t kAES_CCM128_Block_Length = kAES_CCM128_Key_Length; +constexpr size_t kAES_CCM128_Nonce_Length = 13; +constexpr size_t kAES_CCM128_Tag_Length = 16; /* These sizes are hardcoded here to remove header dependency on underlying crypto library * in a public interface file. The validity of these sizes is verified by static_assert in @@ -622,9 +624,9 @@ CHIP_ERROR AES_CCM_decrypt(const uint8_t * ciphertext, size_t ciphertext_length, * @brief A function that implements AES-CCM decryption * * This implements the AES-CTR-Encrypt/Decrypt() cryptographic primitives per sections - * 3.7.1 and 3.7.2 of the specification. For an empty ciphertext, the user of the API - * can provide an empty string, or a nullptr, and provide ciphertext_length as 0. - * The output buffer, plaintext can also be an empty string, or a nullptr for this case. + * 3.7.1 and 3.7.2 of the specification. For an empty input, the user of the API + * can provide an empty string, or a nullptr, and provide input as 0. + * The output buffer can also be an empty string, or a nullptr for this case. * * @param input Input text to encrypt/decrypt * @param input_length Length of ciphertext diff --git a/src/crypto/tests/CHIPCryptoPALTest.cpp b/src/crypto/tests/CHIPCryptoPALTest.cpp index a0106761aa79da..c261d5c943b903 100644 --- a/src/crypto/tests/CHIPCryptoPALTest.cpp +++ b/src/crypto/tests/CHIPCryptoPALTest.cpp @@ -202,8 +202,8 @@ const AesCtrTestEntry theAesCtrTestVector[] = { constexpr size_t kAesCtrTestVectorSize = sizeof(theAesCtrTestVector) / sizeof(theAesCtrTestVector[0]); -constexpr size_t KEY_LENGTH = 16; -constexpr size_t NONCE_LENGTH = 13; +constexpr size_t KEY_LENGTH = Crypto::kAES_CCM128_Key_Length; +constexpr size_t NONCE_LENGTH = Crypto::kAES_CCM128_Nonce_Length; static void TestAES_CTR_128_Encrypt(nlTestSuite * inSuite, const AesCtrTestEntry * vector) { From 6ff20e952faadaa5b31aba1bc0d30833255712a9 Mon Sep 17 00:00:00 2001 From: Martin Turon Date: Thu, 25 Aug 2022 13:06:29 -0700 Subject: [PATCH 3/5] Apply suggestions from code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Jonathan Mégevand <77852424+jmeg-sfy@users.noreply.github.com> --- src/crypto/tests/CHIPCryptoPALTest.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/crypto/tests/CHIPCryptoPALTest.cpp b/src/crypto/tests/CHIPCryptoPALTest.cpp index c261d5c943b903..861db7f8b3bb7b 100644 --- a/src/crypto/tests/CHIPCryptoPALTest.cpp +++ b/src/crypto/tests/CHIPCryptoPALTest.cpp @@ -237,7 +237,7 @@ static void TestAES_CTR_128_Decrypt(nlTestSuite * inSuite, const AesCtrTestEntry NL_TEST_ASSERT(inSuite, outputMatches); if (!outputMatches) { - printf("\n Test failed due to mismatching ciphertext\n"); + printf("\n Test failed due to mismatching plaintext\n"); } } From 486de9019e6dba4e77907e81b11118f14da50912 Mon Sep 17 00:00:00 2001 From: Martin Turon Date: Thu, 25 Aug 2022 13:07:41 -0700 Subject: [PATCH 4/5] Update src/crypto/CHIPCryptoPAL.h Co-authored-by: Evgeny Margolis --- src/crypto/CHIPCryptoPAL.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/crypto/CHIPCryptoPAL.h b/src/crypto/CHIPCryptoPAL.h index 3882e1f64b07d7..9c5de092867ee3 100644 --- a/src/crypto/CHIPCryptoPAL.h +++ b/src/crypto/CHIPCryptoPAL.h @@ -621,7 +621,7 @@ CHIP_ERROR AES_CCM_decrypt(const uint8_t * ciphertext, size_t ciphertext_length, size_t nonce_length, uint8_t * plaintext); /** - * @brief A function that implements AES-CCM decryption + * @brief A function that implements AES-CTR encryption/decryption * * This implements the AES-CTR-Encrypt/Decrypt() cryptographic primitives per sections * 3.7.1 and 3.7.2 of the specification. For an empty input, the user of the API From e0f0c50abce6056c5f65dae8d5057192f988efb5 Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Mon, 29 Aug 2022 13:22:51 -0400 Subject: [PATCH 5/5] Use stdbuf to try to disable stdout buffering on darwin --- scripts/tests/chiptest/runner.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/scripts/tests/chiptest/runner.py b/scripts/tests/chiptest/runner.py index 3cea9b2c3fe9e0..71cf0c7d5e433f 100644 --- a/scripts/tests/chiptest/runner.py +++ b/scripts/tests/chiptest/runner.py @@ -18,6 +18,7 @@ import queue import re import subprocess +import sys import threading import typing @@ -130,6 +131,10 @@ def RunSubprocess(self, cmd, name, wait=True, dependencies=[], timeout_seconds: logging.INFO, capture_delegate=self.capture_delegate, name=name + ' ERR') + if sys.platform == 'darwin': + # Try harder to avoid any stdout buffering in our tests + cmd = ['stdbuf', '-o0'] + cmd + if self.capture_delegate: self.capture_delegate.Log(name, 'EXECUTING %r' % cmd)