Skip to content

Commit

Permalink
Code review.
Browse files Browse the repository at this point in the history
  • Loading branch information
@rcasallas-silabs
rcasallas-silabs committed Nov 25, 2024
1 parent 6386f32 commit 4236469
Show file tree
Hide file tree
Showing 5 changed files with 37 additions and 31 deletions.
15 changes: 11 additions & 4 deletions src/crypto/BUILD.gn
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,25 +46,30 @@ if (chip_crypto_keystore == "") {
}
}

assert(chip_crypto_keystore == "psa" || chip_crypto_keystore == "raw",
"Please select a valid crypto keystore: psa, raw")
assert(chip_crypto_keystore == "psa" || chip_crypto_keystore == "raw" ||
chip_crypto_keystore == "app",
"Please select a valid crypto keystore: psa, raw, app")

buildconfig_header("crypto_buildconfig") {
header = "CryptoBuildConfig.h"
header_dir = "crypto"

chip_crypto_mbedtls = chip_crypto == "mbedtls"
chip_crypto_psa = chip_crypto == "psa"
chip_crypto_psa_keystore = chip_crypto_keystore == "psa"
chip_crypto_openssl = chip_crypto == "openssl"
chip_crypto_boringssl = chip_crypto == "boringssl"
chip_crypto_platform = chip_crypto == "platform"
chip_crypto_keystore_psa = chip_crypto_keystore == "psa"
chip_crypto_keystore_raw = chip_crypto_keystore == "raw"
chip_crypto_keystore_app = chip_crypto_keystore == "app"

defines = [
"CHIP_CRYPTO_MBEDTLS=${chip_crypto_mbedtls}",
"CHIP_CRYPTO_PSA=${chip_crypto_psa}",
"CHIP_CRYPTO_PSA_SPAKE2P=${chip_crypto_psa_spake2p}",
"CHIP_CRYPTO_PSA_KEYSTORE=${chip_crypto_psa_keystore}",
"CHIP_CRYPTO_KEYSTORE_PSA=${chip_crypto_keystore_psa}",
"CHIP_CRYPTO_KEYSTORE_RAW=${chip_crypto_keystore_raw}",
"CHIP_CRYPTO_KEYSTORE_APP=${chip_crypto_keystore_app}",
"CHIP_CRYPTO_OPENSSL=${chip_crypto_openssl}",
"CHIP_CRYPTO_BORINGSSL=${chip_crypto_boringssl}",
"CHIP_CRYPTO_PLATFORM=${chip_crypto_platform}",
Expand Down Expand Up @@ -169,6 +174,8 @@ static_library("crypto") {
"RawKeySessionKeystore.cpp",
"RawKeySessionKeystore.h",
]
} else {
# Keystore provided by app
}

if (chip_crypto_psa_spake2p) {
Expand Down
8 changes: 4 additions & 4 deletions src/crypto/DefaultSessionKeystore.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,9 @@
#include <crypto/CryptoBuildConfig.h>
#endif

#if CHIP_CRYPTO_PSA_KEYSTORE
#if CHIP_CRYPTO_KEYSTORE_PSA
#include <crypto/PSASessionKeystore.h>
#else
#elif CHIP_CRYPTO_KEYSTORE_RAW
#include <crypto/RawKeySessionKeystore.h>
#endif

Expand All @@ -34,9 +34,9 @@ namespace Crypto {
// when the PSA crypto backend is used, AES encryption/decryption function assume that the input
// key handle carries a key reference instead of raw key material, so PSASessionKeystore must be
// used instead of RawKeySessionKeystore to initialize the key handle.
#if CHIP_CRYPTO_PSA_KEYSTORE
#if CHIP_CRYPTO_KEYSTORE_PSA
using DefaultSessionKeystore = PSASessionKeystore;
#else
#elif CHIP_CRYPTO_KEYSTORE_RAW
using DefaultSessionKeystore = RawKeySessionKeystore;
#endif

Expand Down
3 changes: 2 additions & 1 deletion src/crypto/crypto.gni
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@ declare_args() {
# Use PSA Spake2+ implementation. Only used if chip_crypto == "psa"
chip_crypto_psa_spake2p = false

# Crypto storage: psa, raw.
# Crypto storage: psa, raw, app.
# app: includes zero new files and disables the unit tests for the keystore.
chip_crypto_keystore = ""
}

Expand Down
9 changes: 7 additions & 2 deletions src/crypto/tests/TestChipCryptoPAL.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,7 @@ const AesCtrTestEntry theAesCtrTestVector[] = {
}
};

#if !(CHIP_CRYPTO_KEYSTORE_APP)
struct TestAesKey
{
public:
Expand Down Expand Up @@ -245,6 +246,7 @@ struct TestHmacKey
DefaultSessionKeystore keystore;
Hmac128KeyHandle key;
};
#endif

static void TestAES_CTR_128_Encrypt(const AesCtrTestEntry * vector)
{
Expand Down Expand Up @@ -964,6 +966,7 @@ TEST_F(TestChipCryptoPAL, TestHMAC_SHA256_RawKey)
EXPECT_EQ(numOfTestsExecuted, numOfTestCases);
}

#if !(CHIP_CRYPTO_KEYSTORE_APP)
TEST_F(TestChipCryptoPAL, TestHMAC_SHA256_KeyHandle)
{
HeapChecker heapChecker;
Expand Down Expand Up @@ -994,6 +997,7 @@ TEST_F(TestChipCryptoPAL, TestHMAC_SHA256_KeyHandle)
}
EXPECT_EQ(numOfTestsExecuted, numOfTestCases);
}
#endif

TEST_F(TestChipCryptoPAL, TestHKDF_SHA256)
{
Expand Down Expand Up @@ -1870,7 +1874,6 @@ TEST_F(TestChipCryptoPAL, TestSPAKE2P_RFC)
size_t Pverifier_len = sizeof(Pverifier);
uint8_t Vverifier[kMAX_Hash_Length];
size_t Vverifier_len = sizeof(Vverifier);
DefaultSessionKeystore keystore;

int numOfTestVectors = ArraySize(rfc_tvs);
int numOfTestsRan = 0;
Expand Down Expand Up @@ -1967,7 +1970,9 @@ TEST_F(TestChipCryptoPAL, TestSPAKE2P_RFC)
error = Verifier.KeyConfirm(Pverifier, Pverifier_len);
EXPECT_EQ(error, CHIP_NO_ERROR);

#if !(CHIP_CRYPTO_KEYSTORE_APP)
// Import HKDF key from the test vector to the keystore
DefaultSessionKeystore keystore;
HkdfKeyHandle vectorKe;
error = keystore.CreateKey(ByteSpan(vector->Ke, vector->Ke_len), vectorKe);
EXPECT_EQ(error, CHIP_NO_ERROR);
Expand All @@ -1988,7 +1993,7 @@ TEST_F(TestChipCryptoPAL, TestSPAKE2P_RFC)
keystore.DestroyKey(vectorKe);
keystore.DestroyKey(PKe);
keystore.DestroyKey(VKe);

#endif
numOfTestsRan += 1;
}
EXPECT_GT(numOfTestsRan, 0);
Expand Down
33 changes: 13 additions & 20 deletions src/platform/silabs/efr32/CHIPCryptoPALPsaEfr32.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ void _log_PSA_error(psa_status_t status)
* @param t2 Second time to compare
* @return int 0 If both times are idential to the second, -1 if t1 < t2, 1 if t1 > t2.
*/
int timeCompare(mbedtls_x509_time * t1, mbedtls_x509_time * t2)
int TimeCompare(mbedtls_x509_time * t1, mbedtls_x509_time * t2)
{
VerifyOrReturnValue(t1->year >= t2->year, -1);
VerifyOrReturnValue(t1->year <= t2->year, 1);
Expand All @@ -155,12 +155,12 @@ int timeCompare(mbedtls_x509_time * t1, mbedtls_x509_time * t2)
return 0;
}

bool isBufferNonEmpty(const uint8_t * data, size_t data_length)
bool IsBufferNonEmpty(const uint8_t * data, size_t data_length)
{
return data != nullptr && data_length > 0;
}

bool isValidTag(const uint8_t * tag, size_t tag_length)
bool IsValidTag(const uint8_t * tag, size_t tag_length)
{
return tag != nullptr && (tag_length == 8 || tag_length == 12 || tag_length == 16);
}
Expand All @@ -171,8 +171,8 @@ CHIP_ERROR AES_CCM_encrypt(const uint8_t * plaintext, size_t plaintext_length, c
const Aes128KeyHandle & key, const uint8_t * nonce, size_t nonce_length, uint8_t * ciphertext,
uint8_t * tag, size_t tag_length)
{
VerifyOrReturnError(isBufferNonEmpty(nonce, nonce_length), CHIP_ERROR_INVALID_ARGUMENT);
VerifyOrReturnError(isValidTag(tag, tag_length), CHIP_ERROR_INVALID_ARGUMENT);
VerifyOrReturnError(IsBufferNonEmpty(nonce, nonce_length), CHIP_ERROR_INVALID_ARGUMENT);
VerifyOrReturnError(IsValidTag(tag, tag_length), CHIP_ERROR_INVALID_ARGUMENT);
VerifyOrReturnError((ciphertext != nullptr && plaintext != nullptr) || plaintext_length == 0, CHIP_ERROR_INVALID_ARGUMENT);
VerifyOrReturnError(aad != nullptr || aad_length == 0, CHIP_ERROR_INVALID_ARGUMENT);

Expand All @@ -191,15 +191,8 @@ CHIP_ERROR AES_CCM_encrypt(const uint8_t * plaintext, size_t plaintext_length, c
status = psa_aead_set_nonce(&operation, nonce, nonce_length);
VerifyOrReturnError(status == PSA_SUCCESS, CHIP_ERROR_INTERNAL);

if (aad_length != 0)
{
status = psa_aead_update_ad(&operation, aad, aad_length);
VerifyOrReturnError(status == PSA_SUCCESS, CHIP_ERROR_INTERNAL);
}
else
{
ChipLogDetail(Crypto, "AES_CCM_encrypt: Using aad == null path");
}
status = psa_aead_update_ad(&operation, aad, aad_length);
VerifyOrReturnError(status == PSA_SUCCESS, CHIP_ERROR_INTERNAL);

if (plaintext_length != 0)
{
Expand All @@ -225,8 +218,8 @@ CHIP_ERROR AES_CCM_decrypt(const uint8_t * ciphertext, size_t ciphertext_length,
const uint8_t * tag, size_t tag_length, const Aes128KeyHandle & key, const uint8_t * nonce,
size_t nonce_length, uint8_t * plaintext)
{
VerifyOrReturnError(isBufferNonEmpty(nonce, nonce_length), CHIP_ERROR_INVALID_ARGUMENT);
VerifyOrReturnError(isValidTag(tag, tag_length), CHIP_ERROR_INVALID_ARGUMENT);
VerifyOrReturnError(IsBufferNonEmpty(nonce, nonce_length), CHIP_ERROR_INVALID_ARGUMENT);
VerifyOrReturnError(IsValidTag(tag, tag_length), CHIP_ERROR_INVALID_ARGUMENT);
VerifyOrReturnError((ciphertext != nullptr && plaintext != nullptr) || ciphertext_length == 0, CHIP_ERROR_INVALID_ARGUMENT);
VerifyOrReturnError(aad != nullptr || aad_length == 0, CHIP_ERROR_INVALID_ARGUMENT);

Expand Down Expand Up @@ -1686,15 +1679,15 @@ CHIP_ERROR ValidateCertificateChain(const uint8_t * rootCertificate, size_t root
VerifyOrExit(mbedResult == 0, (result = CertificateChainValidationResult::kRootFormatInvalid, error = CHIP_ERROR_INTERNAL));

/* Validates that intermediate and root certificates are valid at the time of the leaf certificate's start time. */
compare_from = timeCompare(&leaf_valid_from, &rootCert.valid_from);
compare_until = timeCompare(&leaf_valid_from, &rootCert.valid_to);
compare_from = TimeCompare(&leaf_valid_from, &rootCert.valid_from);
compare_until = TimeCompare(&leaf_valid_from, &rootCert.valid_to);
VerifyOrExit((compare_from >= 0) && (compare_until <= 0),
(result = CertificateChainValidationResult::kChainInvalid, error = CHIP_ERROR_CERT_NOT_TRUSTED));
cert = certChain.next;
while (cert)
{
compare_from = timeCompare(&leaf_valid_from, &cert->valid_from);
compare_until = timeCompare(&leaf_valid_from, &cert->valid_to);
compare_from = TimeCompare(&leaf_valid_from, &cert->valid_from);
compare_until = TimeCompare(&leaf_valid_from, &cert->valid_to);
VerifyOrExit((compare_from >= 0) && (compare_until <= 0),
(result = CertificateChainValidationResult::kChainInvalid, error = CHIP_ERROR_CERT_NOT_TRUSTED));
cert = cert->next;
Expand Down

0 comments on commit 4236469

Please sign in to comment.