From 4d9dfd80a132be7fe9b22675af57f1e42e1ef004 Mon Sep 17 00:00:00 2001 From: Pankaj Garg Date: Mon, 3 May 2021 08:31:33 -0700 Subject: [PATCH] Add Crypto API to verify the CSR and get pubkey (#6405) * Add Crypto API to verify the CSR and get pubkey * fix test build * Detect if CSR parsing is not supported * reduce stack usage in the test * disable CSR processing for embedded targets --- src/crypto/CHIPCryptoPAL.h | 9 ++ src/crypto/CHIPCryptoPALOpenSSL.cpp | 117 +++++++++++++++++++------ src/crypto/CHIPCryptoPALmbedTLS.cpp | 55 ++++++++++++ src/crypto/tests/CHIPCryptoPALTest.cpp | 23 ++++- 4 files changed, 177 insertions(+), 27 deletions(-) diff --git a/src/crypto/CHIPCryptoPAL.h b/src/crypto/CHIPCryptoPAL.h index 37014a5ca66cc7..9174a084a24636 100644 --- a/src/crypto/CHIPCryptoPAL.h +++ b/src/crypto/CHIPCryptoPAL.h @@ -341,6 +341,15 @@ CHIP_ERROR AES_CCM_decrypt(const uint8_t * ciphertext, size_t ciphertext_length, const uint8_t * tag, size_t tag_length, const uint8_t * key, size_t key_length, const uint8_t * iv, size_t iv_length, uint8_t * plaintext); +/** + * @brief Verify the Certificate Signing Request (CSR). If successfully verified, it outputs the public key from the CSR. + * @param csr CSR in DER format + * @param csr_length The length of the CSR + * @param pubkey The public key from the verified CSR + * @return Returns a CHIP_ERROR on error, CHIP_NO_ERROR otherwise + **/ +CHIP_ERROR VerifyCertificateSigningRequest(const uint8_t * csr, size_t csr_length, P256PublicKey & pubkey); + /** * @brief A function that implements SHA-256 hash * @param data The data to hash diff --git a/src/crypto/CHIPCryptoPALOpenSSL.cpp b/src/crypto/CHIPCryptoPALOpenSSL.cpp index ef848baacd0d2a..4ed545b8844fa1 100644 --- a/src/crypto/CHIPCryptoPALOpenSSL.cpp +++ b/src/crypto/CHIPCryptoPALOpenSSL.cpp @@ -890,41 +890,61 @@ void ClearSecretData(uint8_t * buf, uint32_t len) memset(buf, 0, len); } +static CHIP_ERROR P256PublicKeyFromECKey(EC_KEY * ec_key, P256PublicKey & pubkey) +{ + ERR_clear_error(); + CHIP_ERROR error = CHIP_NO_ERROR; + + int nid = NID_undef; + ECName curve = MapECName(pubkey.Type()); + EC_GROUP * group = nullptr; + size_t pubkey_size = 0; + + const EC_POINT * pubkey_ecp = EC_KEY_get0_public_key(ec_key); + VerifyOrExit(pubkey_ecp != nullptr, error = CHIP_ERROR_INVALID_ARGUMENT); + + nid = _nidForCurve(curve); + VerifyOrExit(nid != NID_undef, error = CHIP_ERROR_INVALID_ARGUMENT); + + group = EC_GROUP_new_by_curve_name(nid); + VerifyOrExit(group != nullptr, error = CHIP_ERROR_INTERNAL); + + pubkey_size = + EC_POINT_point2oct(group, pubkey_ecp, POINT_CONVERSION_UNCOMPRESSED, Uint8::to_uchar(pubkey), pubkey.Length(), nullptr); + pubkey_ecp = nullptr; + + VerifyOrExit(pubkey_size == pubkey.Length(), error = CHIP_ERROR_INVALID_ARGUMENT); + +exit: + if (group != nullptr) + { + EC_GROUP_free(group); + group = nullptr; + } + + _logSSLError(); + return error; +} + CHIP_ERROR P256Keypair::Initialize() { ERR_clear_error(); CHIP_ERROR error = CHIP_NO_ERROR; int result = 0; - int nid = NID_undef; EC_KEY * ec_key = nullptr; - EC_GROUP * group = nullptr; ECName curve = MapECName(mPublicKey.Type()); - VerifyOrExit(curve == MapECName(mPublicKey.Type()), error = CHIP_ERROR_INVALID_ARGUMENT); - - nid = _nidForCurve(curve); + int nid = _nidForCurve(curve); VerifyOrExit(nid != NID_undef, error = CHIP_ERROR_INVALID_ARGUMENT); ec_key = EC_KEY_new_by_curve_name(nid); VerifyOrExit(ec_key != nullptr, error = CHIP_ERROR_INTERNAL); - group = EC_GROUP_new_by_curve_name(nid); - VerifyOrExit(group != nullptr, error = CHIP_ERROR_INTERNAL); - result = EC_KEY_generate_key(ec_key); VerifyOrExit(result == 1, error = CHIP_ERROR_INTERNAL); - { - size_t pubkey_size = 0; - const EC_POINT * pubkey_ecp = EC_KEY_get0_public_key(ec_key); - VerifyOrExit(pubkey_ecp != nullptr, error = CHIP_ERROR_INTERNAL); - - pubkey_size = EC_POINT_point2oct(group, pubkey_ecp, POINT_CONVERSION_UNCOMPRESSED, Uint8::to_uchar(mPublicKey), - mPublicKey.Length(), nullptr); - pubkey_ecp = nullptr; - - VerifyOrExit(pubkey_size == mPublicKey.Length(), error = CHIP_ERROR_INTERNAL); - } + error = P256PublicKeyFromECKey(ec_key, mPublicKey); + SuccessOrExit(error); from_EC_KEY(ec_key, &mKeypair); mInitialized = true; @@ -937,12 +957,6 @@ CHIP_ERROR P256Keypair::Initialize() ec_key = nullptr; } - if (group != nullptr) - { - EC_GROUP_free(group); - group = nullptr; - } - _logSSLError(); return error; } @@ -1078,6 +1092,9 @@ CHIP_ERROR P256Keypair::NewCertificateSigningRequest(uint8_t * out_csr, size_t & EC_KEY * ec_key = to_EC_KEY(&mKeypair); + X509_NAME * subject = X509_NAME_new(); + VerifyOrExit(subject != nullptr, error = CHIP_ERROR_INTERNAL); + VerifyOrExit(mInitialized, error = CHIP_ERROR_INCORRECT_STATE); result = X509_REQ_set_version(x509_req, 0); @@ -1095,6 +1112,15 @@ CHIP_ERROR P256Keypair::NewCertificateSigningRequest(uint8_t * out_csr, size_t & result = X509_REQ_set_pubkey(x509_req, evp_pkey); VerifyOrExit(result == 1, error = CHIP_ERROR_INTERNAL); + // TODO: mbedTLS CSR parser fails if the subject name is not set (or if empty). + // CHIP Spec doesn't specify the subject name that can be used. + // Figure out the correct value and update this code. + result = X509_NAME_add_entry_by_txt(subject, "O", MBSTRING_ASC, Uint8::from_const_char("CSR"), -1, -1, 0); + VerifyOrExit(result == 1, error = CHIP_ERROR_INTERNAL); + + result = X509_REQ_set_subject_name(x509_req, subject); + VerifyOrExit(result == 1, error = CHIP_ERROR_INTERNAL); + result = X509_REQ_sign(x509_req, evp_pkey, EVP_sha256()); VerifyOrExit(result > 0, error = CHIP_ERROR_INTERNAL); @@ -1109,12 +1135,53 @@ CHIP_ERROR P256Keypair::NewCertificateSigningRequest(uint8_t * out_csr, size_t & evp_pkey = nullptr; } + X509_NAME_free(subject); + subject = nullptr; + X509_REQ_free(x509_req); _logSSLError(); return error; } +CHIP_ERROR VerifyCertificateSigningRequest(const uint8_t * csr, size_t csr_length, P256PublicKey & pubkey) +{ + ERR_clear_error(); + CHIP_ERROR error = CHIP_NO_ERROR; + int result = 0; + + EVP_PKEY * evp_pkey = nullptr; + EC_KEY * ec_key = nullptr; + + const unsigned char * csr_buf = Uint8::to_const_uchar(csr); + X509_REQ * x509_req = d2i_X509_REQ(nullptr, &csr_buf, (int) csr_length); + VerifyOrExit(x509_req != nullptr, error = CHIP_ERROR_INVALID_ARGUMENT); + + VerifyOrExit(X509_REQ_get_version(x509_req) == 0, error = CHIP_ERROR_INVALID_ARGUMENT); + + evp_pkey = X509_REQ_get0_pubkey(x509_req); + VerifyOrExit(evp_pkey != nullptr, error = CHIP_ERROR_INVALID_ARGUMENT); + + result = X509_REQ_verify(x509_req, evp_pkey); + VerifyOrExit(result == 1, error = CHIP_ERROR_INVALID_ARGUMENT); + + ec_key = EVP_PKEY_get1_EC_KEY(evp_pkey); + VerifyOrExit(ec_key != nullptr, error = CHIP_ERROR_INVALID_ARGUMENT); + + error = P256PublicKeyFromECKey(ec_key, pubkey); + SuccessOrExit(error); + +exit: + + if (x509_req != nullptr) + { + X509_REQ_free(x509_req); + } + + _logSSLError(); + return error; +} + #define init_point(_point_) \ do \ { \ diff --git a/src/crypto/CHIPCryptoPALmbedTLS.cpp b/src/crypto/CHIPCryptoPALmbedTLS.cpp index f9a61be0646ce7..6ba6f146644f97 100644 --- a/src/crypto/CHIPCryptoPALmbedTLS.cpp +++ b/src/crypto/CHIPCryptoPALmbedTLS.cpp @@ -734,6 +734,12 @@ CHIP_ERROR P256Keypair::NewCertificateSigningRequest(uint8_t * out_csr, size_t & mbedtls_x509write_csr_set_md_alg(&csr, MBEDTLS_MD_SHA256); + // TODO: mbedTLS CSR parser fails if the subject name is not set (or if empty). + // CHIP Spec doesn't specify the subject name that can be used. + // Figure out the correct value and update this code. + result = mbedtls_x509write_csr_set_subject_name(&csr, "O=CSR"); + VerifyOrExit(result == 0, error = CHIP_ERROR_INTERNAL); + result = mbedtls_x509write_csr_der(&csr, out_csr, csr_length, CryptoRNG, nullptr); VerifyOrExit(result > 0, error = CHIP_ERROR_INTERNAL); @@ -759,6 +765,55 @@ CHIP_ERROR P256Keypair::NewCertificateSigningRequest(uint8_t * out_csr, size_t & return error; } +CHIP_ERROR VerifyCertificateSigningRequest(const uint8_t * csr_buf, size_t csr_length, P256PublicKey & pubkey) +{ +#if !CHIP_TARGET_STYLE_EMBEDDED + // TODO: For some embedded targets, mbedTLS library doesn't have mbedtls_x509_csr_parse_der, and mbedtls_x509_csr_parse_free. + // Taking a step back, embedded targets likely will not process CSR requests. Adding this action item to reevaluate + // this if there's a need for this processing for embedded targets. + CHIP_ERROR error = CHIP_NO_ERROR; + size_t pubkey_size = 0; + + mbedtls_ecp_keypair * keypair = nullptr; + + P256ECDSASignature signature; + + mbedtls_x509_csr csr; + mbedtls_x509_csr_init(&csr); + + int result = mbedtls_x509_csr_parse_der(&csr, csr_buf, csr_length); + VerifyOrExit(result == 0, error = CHIP_ERROR_INTERNAL); + + // Verify the signature algorithm and public key type + VerifyOrExit(csr.sig_md == MBEDTLS_MD_SHA256, error = CHIP_ERROR_UNSUPPORTED_SIGNATURE_TYPE); + VerifyOrExit(csr.sig_pk == MBEDTLS_PK_ECDSA, error = CHIP_ERROR_WRONG_KEY_TYPE); + + keypair = mbedtls_pk_ec(csr.pk); + + // Copy the public key from the CSR + result = mbedtls_ecp_point_write_binary(&keypair->grp, &keypair->Q, MBEDTLS_ECP_PF_UNCOMPRESSED, &pubkey_size, + Uint8::to_uchar(pubkey), pubkey.Length()); + VerifyOrExit(result == 0, error = CHIP_ERROR_INTERNAL); + VerifyOrExit(pubkey_size == pubkey.Length(), error = CHIP_ERROR_INTERNAL); + + // Verify the signature using the public key + VerifyOrExit(kMax_ECDSA_Signature_Length >= csr.sig.len, error = CHIP_ERROR_INTERNAL); + memmove(Uint8::to_uchar(signature), csr.sig.p, csr.sig.len); + signature.SetLength(csr.sig.len); + + error = pubkey.ECDSA_validate_msg_signature(csr.cri.p, csr.cri.len, signature); + SuccessOrExit(error); + +exit: + mbedtls_x509_csr_free(&csr); + _log_mbedTLS_error(result); + return error; +#else + ChipLogError(Crypto, "MBEDTLS_X509_CSR_PARSE_C is not enabled. CSR cannot be parsed"); + return CHIP_ERROR_UNSUPPORTED_CHIP_FEATURE; +#endif +} + typedef struct Spake2p_Context { mbedtls_ecp_group curve; diff --git a/src/crypto/tests/CHIPCryptoPALTest.cpp b/src/crypto/tests/CHIPCryptoPALTest.cpp index 42d2953b80d83a..03657ea4f300c8 100644 --- a/src/crypto/tests/CHIPCryptoPALTest.cpp +++ b/src/crypto/tests/CHIPCryptoPALTest.cpp @@ -973,13 +973,32 @@ static void TestP256_Keygen(nlTestSuite * inSuite, void * inContext) static void TestCSR_Gen(nlTestSuite * inSuite, void * inContext) { - uint8_t csr[kMAX_CSR_Length]; + static uint8_t csr[kMAX_CSR_Length]; size_t length = sizeof(csr); - P256Keypair keypair; + static P256Keypair keypair; NL_TEST_ASSERT(inSuite, keypair.Initialize() == CHIP_NO_ERROR); NL_TEST_ASSERT(inSuite, keypair.NewCertificateSigningRequest(csr, length) == CHIP_NO_ERROR); NL_TEST_ASSERT(inSuite, length > 0); + + static P256PublicKey pubkey; + CHIP_ERROR err = VerifyCertificateSigningRequest(csr, length, pubkey); + if (err != CHIP_ERROR_UNSUPPORTED_CHIP_FEATURE) + { + NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR); + NL_TEST_ASSERT(inSuite, pubkey.Length() == kP256_PublicKey_Length); + NL_TEST_ASSERT(inSuite, memcmp(pubkey, keypair.Pubkey(), pubkey.Length()) == 0); + + // Let's corrupt the CSR buffer and make sure it fails to verify + csr[length - 2] = (uint8_t)(csr[length - 2] + 1); + csr[length - 1] = (uint8_t)(csr[length - 1] + 1); + + NL_TEST_ASSERT(inSuite, VerifyCertificateSigningRequest(csr, length, pubkey) != CHIP_NO_ERROR); + } + else + { + ChipLogError(Crypto, "The current platform does not support CSR parsing."); + } } static void TestKeypair_Serialize(nlTestSuite * inSuite, void * inContext)