From b6297f3314f62bc0c8c8d6d61f9f4ef3ac3eb298 Mon Sep 17 00:00:00 2001 From: Vijay Selvaraj Date: Thu, 9 Dec 2021 12:55:53 -0500 Subject: [PATCH] Added additional AttestationVerificationResult values (#12655) * Added additional AttestationVerificationResult values * Initialized result to error at the beginning of ValidateCertificateChain and added failure scenarios to the unit tests. --- src/credentials/DeviceAttestationVerifier.h | 15 ++-- .../DefaultDeviceAttestationVerifier.cpp | 41 ++++++++++- src/crypto/CHIPCryptoPAL.h | 23 ++++++- src/crypto/CHIPCryptoPALOpenSSL.cpp | 44 +++++++----- src/crypto/CHIPCryptoPALmbedTLS.cpp | 68 ++++++++++++------- src/crypto/tests/CHIPCryptoPALTest.cpp | 36 +++++++--- 6 files changed, 169 insertions(+), 58 deletions(-) diff --git a/src/credentials/DeviceAttestationVerifier.h b/src/credentials/DeviceAttestationVerifier.h index 292597a1fb25eb..16e241de4d244a 100644 --- a/src/credentials/DeviceAttestationVerifier.h +++ b/src/credentials/DeviceAttestationVerifier.h @@ -34,21 +34,24 @@ enum class AttestationVerificationResult : uint16_t kPaaSignatureInvalid = 103, kPaaRevoked = 104, kPaaFormatInvalid = 105, + kPaaArgumentInvalid = 106, kPaiExpired = 200, kPaiSignatureInvalid = 201, kPaiRevoked = 202, kPaiFormatInvalid = 203, - kPaiVendorIdMismatch = 204, - kPaiAuthorityNotFound = 205, + kPaiArgumentInvalid = 204, + kPaiVendorIdMismatch = 205, + kPaiAuthorityNotFound = 206, kDacExpired = 300, kDacSignatureInvalid = 301, kDacRevoked = 302, kDacFormatInvalid = 303, - kDacVendorIdMismatch = 304, - kDacProductIdMismatch = 305, - kDacAuthorityNotFound = 306, + kDacArgumentInvalid = 304, + kDacVendorIdMismatch = 305, + kDacProductIdMismatch = 306, + kDacAuthorityNotFound = 307, kFirmwareInformationMismatch = 400, kFirmwareInformationMissing = 401, @@ -69,6 +72,8 @@ enum class AttestationVerificationResult : uint16_t kInvalidArgument = 800, + kInternalError = 900, + kNotImplemented = 0xFFFFU, // TODO: Add more attestation verification errors diff --git a/src/credentials/examples/DefaultDeviceAttestationVerifier.cpp b/src/credentials/examples/DefaultDeviceAttestationVerifier.cpp index bc26e558f945a3..7929b292c81e5f 100644 --- a/src/credentials/examples/DefaultDeviceAttestationVerifier.cpp +++ b/src/credentials/examples/DefaultDeviceAttestationVerifier.cpp @@ -115,6 +115,42 @@ const ByteSpan kTestPaaRoots[] = { const ArrayAttestationTrustStore kTestAttestationTrustStore{ &kTestPaaRoots[0], ArraySize(kTestPaaRoots) }; +AttestationVerificationResult MapError(CertificateChainValidationResult certificateChainValidationResult) +{ + switch (certificateChainValidationResult) + { + case CertificateChainValidationResult::kRootFormatInvalid: + return AttestationVerificationResult::kPaaFormatInvalid; + + case CertificateChainValidationResult::kRootArgumentInvalid: + return AttestationVerificationResult::kPaaArgumentInvalid; + + case CertificateChainValidationResult::kICAFormatInvalid: + return AttestationVerificationResult::kPaiFormatInvalid; + + case CertificateChainValidationResult::kICAArgumentInvalid: + return AttestationVerificationResult::kPaiArgumentInvalid; + + case CertificateChainValidationResult::kLeafFormatInvalid: + return AttestationVerificationResult::kDacFormatInvalid; + + case CertificateChainValidationResult::kLeafArgumentInvalid: + return AttestationVerificationResult::kDacArgumentInvalid; + + case CertificateChainValidationResult::kChainInvalid: + return AttestationVerificationResult::kDacSignatureInvalid; + + case CertificateChainValidationResult::kNoMemory: + return AttestationVerificationResult::kNoMemory; + + case CertificateChainValidationResult::kInternalFrameworkError: + return AttestationVerificationResult::kInternalError; + + default: + return AttestationVerificationResult::kInternalError; + } +} + /** * @brief Look-up of well-known keys used for CD signing by CSA. * @@ -258,9 +294,10 @@ AttestationVerificationResult DefaultDACVerifier::VerifyAttestationInformation(c VerifyOrReturnError(IsCertificateValidAtIssuance(dacDerBuffer, paaDerBuffer) == CHIP_NO_ERROR, AttestationVerificationResult::kPaaExpired); + CertificateChainValidationResult chainValidationResult; VerifyOrReturnError(ValidateCertificateChain(paaDerBuffer.data(), paaDerBuffer.size(), paiDerBuffer.data(), paiDerBuffer.size(), - dacDerBuffer.data(), dacDerBuffer.size()) == CHIP_NO_ERROR, - AttestationVerificationResult::kDacSignatureInvalid); + dacDerBuffer.data(), dacDerBuffer.size(), chainValidationResult) == CHIP_NO_ERROR, + MapError(chainValidationResult)); // if PAA contains VID, see if matches with DAC's VID. { diff --git a/src/crypto/CHIPCryptoPAL.h b/src/crypto/CHIPCryptoPAL.h index 8fce34addbc45e..3eaae07b470c5a 100644 --- a/src/crypto/CHIPCryptoPAL.h +++ b/src/crypto/CHIPCryptoPAL.h @@ -1208,8 +1208,29 @@ CHIP_ERROR LoadCertFromPKCS7(const char * pkcs7, X509DerCertificate * x509list, CHIP_ERROR GetNumberOfCertsFromPKCS7(const char * pkcs7, uint32_t * n_certs); +enum class CertificateChainValidationResult +{ + kSuccess = 0, + + kRootFormatInvalid = 100, + kRootArgumentInvalid = 101, + + kICAFormatInvalid = 200, + kICAArgumentInvalid = 201, + + kLeafFormatInvalid = 300, + kLeafArgumentInvalid = 301, + + kChainInvalid = 400, + + kNoMemory = 500, + + kInternalFrameworkError = 600, +}; + CHIP_ERROR ValidateCertificateChain(const uint8_t * rootCertificate, size_t rootCertificateLen, const uint8_t * caCertificate, - size_t caCertificateLen, const uint8_t * leafCertificate, size_t leafCertificateLen); + size_t caCertificateLen, const uint8_t * leafCertificate, size_t leafCertificateLen, + CertificateChainValidationResult & result); /** * @brief Validate timestamp of a certificate (toBeEvaluatedCertificate) in comparison with other certificate's diff --git a/src/crypto/CHIPCryptoPALOpenSSL.cpp b/src/crypto/CHIPCryptoPALOpenSSL.cpp index 44352ba369b9bc..dbe45f13b11f79 100644 --- a/src/crypto/CHIPCryptoPALOpenSSL.cpp +++ b/src/crypto/CHIPCryptoPALOpenSSL.cpp @@ -1650,7 +1650,8 @@ CHIP_ERROR GetNumberOfCertsFromPKCS7(const char * pkcs7, uint32_t * n_certs) } CHIP_ERROR ValidateCertificateChain(const uint8_t * rootCertificate, size_t rootCertificateLen, const uint8_t * caCertificate, - size_t caCertificateLen, const uint8_t * leafCertificate, size_t leafCertificateLen) + size_t caCertificateLen, const uint8_t * leafCertificate, size_t leafCertificateLen, + CertificateChainValidationResult & result) { CHIP_ERROR err = CHIP_NO_ERROR; int status = 0; @@ -1660,38 +1661,47 @@ CHIP_ERROR ValidateCertificateChain(const uint8_t * rootCertificate, size_t root X509 * x509CACertificate = nullptr; X509 * x509LeafCertificate = nullptr; + result = CertificateChainValidationResult::kInternalFrameworkError; + + VerifyOrReturnError(rootCertificate != nullptr && rootCertificateLen != 0, + (result = CertificateChainValidationResult::kRootArgumentInvalid, CHIP_ERROR_INVALID_ARGUMENT)); + VerifyOrReturnError(caCertificate != nullptr && caCertificateLen != 0, + (result = CertificateChainValidationResult::kICAArgumentInvalid, CHIP_ERROR_INVALID_ARGUMENT)); + VerifyOrReturnError(leafCertificate != nullptr && leafCertificateLen != 0, + (result = CertificateChainValidationResult::kLeafArgumentInvalid, CHIP_ERROR_INVALID_ARGUMENT)); + store = X509_STORE_new(); - VerifyOrExit(store != nullptr, err = CHIP_ERROR_NO_MEMORY); + VerifyOrExit(store != nullptr, (result = CertificateChainValidationResult::kNoMemory, err = CHIP_ERROR_NO_MEMORY)); verifyCtx = X509_STORE_CTX_new(); - VerifyOrExit(verifyCtx != nullptr, err = CHIP_ERROR_NO_MEMORY); + VerifyOrExit(verifyCtx != nullptr, (result = CertificateChainValidationResult::kNoMemory, err = CHIP_ERROR_NO_MEMORY)); x509RootCertificate = d2i_X509(NULL, &rootCertificate, static_cast(rootCertificateLen)); - VerifyOrExit(x509RootCertificate != nullptr, err = CHIP_ERROR_NO_MEMORY); + VerifyOrExit(x509RootCertificate != nullptr, + (result = CertificateChainValidationResult::kRootFormatInvalid, err = CHIP_ERROR_INTERNAL)); status = X509_STORE_add_cert(store, x509RootCertificate); - VerifyOrExit(status == 1, err = CHIP_ERROR_INTERNAL); + VerifyOrExit(status == 1, (result = CertificateChainValidationResult::kInternalFrameworkError, err = CHIP_ERROR_INTERNAL)); - if (caCertificate != nullptr && caCertificateLen != 0) - { - x509CACertificate = d2i_X509(NULL, &caCertificate, static_cast(caCertificateLen)); - VerifyOrExit(x509CACertificate != nullptr, err = CHIP_ERROR_NO_MEMORY); + x509CACertificate = d2i_X509(NULL, &caCertificate, static_cast(caCertificateLen)); + VerifyOrExit(x509CACertificate != nullptr, + (result = CertificateChainValidationResult::kICAFormatInvalid, err = CHIP_ERROR_INTERNAL)); - status = X509_STORE_add_cert(store, x509CACertificate); - VerifyOrExit(status == 1, err = CHIP_ERROR_INTERNAL); - } + status = X509_STORE_add_cert(store, x509CACertificate); + VerifyOrExit(status == 1, (result = CertificateChainValidationResult::kInternalFrameworkError, err = CHIP_ERROR_INTERNAL)); x509LeafCertificate = d2i_X509(NULL, &leafCertificate, static_cast(leafCertificateLen)); - VerifyOrExit(x509LeafCertificate != nullptr, err = CHIP_ERROR_NO_MEMORY); + VerifyOrExit(x509LeafCertificate != nullptr, + (result = CertificateChainValidationResult::kLeafFormatInvalid, err = CHIP_ERROR_INTERNAL)); status = X509_STORE_CTX_init(verifyCtx, store, x509LeafCertificate, NULL); - VerifyOrExit(status == 1, err = CHIP_ERROR_INTERNAL); + VerifyOrExit(status == 1, (result = CertificateChainValidationResult::kInternalFrameworkError, err = CHIP_ERROR_INTERNAL)); - // TODO: If any specific error occurs here, it should be flagged accordingly status = X509_verify_cert(verifyCtx); - VerifyOrExit(status == 1, err = CHIP_ERROR_CERT_NOT_TRUSTED); + VerifyOrExit(status == 1, (result = CertificateChainValidationResult::kChainInvalid, err = CHIP_ERROR_CERT_NOT_TRUSTED)); - err = CHIP_NO_ERROR; + err = CHIP_NO_ERROR; + result = CertificateChainValidationResult::kSuccess; exit: X509_free(x509LeafCertificate); diff --git a/src/crypto/CHIPCryptoPALmbedTLS.cpp b/src/crypto/CHIPCryptoPALmbedTLS.cpp index 012113ec069e4b..d48120e86b27b2 100644 --- a/src/crypto/CHIPCryptoPALmbedTLS.cpp +++ b/src/crypto/CHIPCryptoPALmbedTLS.cpp @@ -1217,46 +1217,65 @@ CHIP_ERROR GetNumberOfCertsFromPKCS7(const char * pkcs7, uint32_t * n_certs) } CHIP_ERROR ValidateCertificateChain(const uint8_t * rootCertificate, size_t rootCertificateLen, const uint8_t * caCertificate, - size_t caCertificateLen, const uint8_t * leafCertificate, size_t leafCertificateLen) + size_t caCertificateLen, const uint8_t * leafCertificate, size_t leafCertificateLen, + CertificateChainValidationResult & result) { #if defined(MBEDTLS_X509_CRT_PARSE_C) CHIP_ERROR error = CHIP_NO_ERROR; - mbedtls_x509_crt cert_chain; - mbedtls_x509_crt root_cert; - int result; + mbedtls_x509_crt certChain; + mbedtls_x509_crt rootCert; + int mbedResult; uint32_t flags; - mbedtls_x509_crt_init(&cert_chain); - mbedtls_x509_crt_init(&root_cert); + result = CertificateChainValidationResult::kInternalFrameworkError; + + VerifyOrReturnError(rootCertificate != nullptr && rootCertificateLen != 0, + (result = CertificateChainValidationResult::kRootArgumentInvalid, CHIP_ERROR_INVALID_ARGUMENT)); + VerifyOrReturnError(caCertificate != nullptr && caCertificateLen != 0, + (result = CertificateChainValidationResult::kICAArgumentInvalid, CHIP_ERROR_INVALID_ARGUMENT)); + VerifyOrReturnError(leafCertificate != nullptr && leafCertificateLen != 0, + (result = CertificateChainValidationResult::kLeafArgumentInvalid, CHIP_ERROR_INVALID_ARGUMENT)); + + mbedtls_x509_crt_init(&certChain); + mbedtls_x509_crt_init(&rootCert); /* Start of chain */ - result = mbedtls_x509_crt_parse(&cert_chain, Uint8::to_const_uchar(leafCertificate), leafCertificateLen); - VerifyOrExit(result == 0, error = CHIP_ERROR_INTERNAL); + mbedResult = mbedtls_x509_crt_parse(&certChain, Uint8::to_const_uchar(leafCertificate), leafCertificateLen); + VerifyOrExit(mbedResult == 0, (result = CertificateChainValidationResult::kLeafFormatInvalid, error = CHIP_ERROR_INTERNAL)); - if (caCertificate != nullptr && caCertificateLen != 0) - { - /* Add the intermediate to the chain */ - result = mbedtls_x509_crt_parse(&cert_chain, Uint8::to_const_uchar(caCertificate), caCertificateLen); - VerifyOrExit(result == 0, error = CHIP_ERROR_INTERNAL); - } + /* Add the intermediate to the chain */ + mbedResult = mbedtls_x509_crt_parse(&certChain, Uint8::to_const_uchar(caCertificate), caCertificateLen); + VerifyOrExit(mbedResult == 0, (result = CertificateChainValidationResult::kICAFormatInvalid, error = CHIP_ERROR_INTERNAL)); /* Add the root to the chain */ - result = mbedtls_x509_crt_parse(&cert_chain, Uint8::to_const_uchar(rootCertificate), rootCertificateLen); - VerifyOrExit(result == 0, error = CHIP_ERROR_INTERNAL); + mbedResult = mbedtls_x509_crt_parse(&certChain, Uint8::to_const_uchar(rootCertificate), rootCertificateLen); + VerifyOrExit(mbedResult == 0, (result = CertificateChainValidationResult::kRootFormatInvalid, error = CHIP_ERROR_INTERNAL)); /* Parse the root cert */ - result = mbedtls_x509_crt_parse(&root_cert, Uint8::to_const_uchar(rootCertificate), rootCertificateLen); - VerifyOrExit(result == 0, error = CHIP_ERROR_INTERNAL); + mbedResult = mbedtls_x509_crt_parse(&rootCert, Uint8::to_const_uchar(rootCertificate), rootCertificateLen); + VerifyOrExit(mbedResult == 0, (result = CertificateChainValidationResult::kRootFormatInvalid, error = CHIP_ERROR_INTERNAL)); - // TODO: If any specific error occurs here, it should be flagged accordingly, such as specific chain element errors /* Verify the chain against the root */ - result = mbedtls_x509_crt_verify(&cert_chain, &root_cert, NULL, NULL, &flags, NULL, NULL); - VerifyOrExit(result == 0 && flags == 0, error = CHIP_ERROR_CERT_NOT_TRUSTED); + mbedResult = mbedtls_x509_crt_verify(&certChain, &rootCert, NULL, NULL, &flags, NULL, NULL); + + switch (mbedResult) + { + case 0: + VerifyOrExit(flags == 0, (result = CertificateChainValidationResult::kInternalFrameworkError, error = CHIP_ERROR_INTERNAL)); + result = CertificateChainValidationResult::kSuccess; + break; + case MBEDTLS_ERR_X509_CERT_VERIFY_FAILED: + result = CertificateChainValidationResult::kChainInvalid; + error = CHIP_ERROR_CERT_NOT_TRUSTED; + break; + default: + SuccessOrExit((result = CertificateChainValidationResult::kInternalFrameworkError, error = CHIP_ERROR_INTERNAL)); + } exit: - _log_mbedTLS_error(result); - mbedtls_x509_crt_free(&cert_chain); - mbedtls_x509_crt_free(&root_cert); + _log_mbedTLS_error(mbedResult); + mbedtls_x509_crt_free(&certChain); + mbedtls_x509_crt_free(&rootCert); #else (void) rootCertificate; @@ -1265,6 +1284,7 @@ CHIP_ERROR ValidateCertificateChain(const uint8_t * rootCertificate, size_t root (void) caCertificateLen; (void) leafCertificate; (void) leafCertificateLen; + (void) result; CHIP_ERROR error = CHIP_ERROR_NOT_IMPLEMENTED; #endif // defined(MBEDTLS_X509_CRT_PARSE_C) diff --git a/src/crypto/tests/CHIPCryptoPALTest.cpp b/src/crypto/tests/CHIPCryptoPALTest.cpp index a65b314364e1da..7530140fba30e8 100644 --- a/src/crypto/tests/CHIPCryptoPALTest.cpp +++ b/src/crypto/tests/CHIPCryptoPALTest.cpp @@ -1853,19 +1853,37 @@ static void TestX509_CertChainValidation(nlTestSuite * inSuite, void * inContext err = GetTestCert(TestCert::kNode01_01, TestCertLoadFlags::kDERForm, leaf_cert); NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR); + CertificateChainValidationResult chainValidationResult; err = ValidateCertificateChain(root_cert.data(), root_cert.size(), ica_cert.data(), ica_cert.size(), leaf_cert.data(), - leaf_cert.size()); + leaf_cert.size(), chainValidationResult); NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR); - - // Now test chain without ICA certificate - err = GetTestCert(TestCert::kRoot01, TestCertLoadFlags::kDERForm, root_cert); + NL_TEST_ASSERT(inSuite, chainValidationResult == CertificateChainValidationResult::kSuccess); + + // Now test for invalid arguments. + err = ValidateCertificateChain(nullptr, 0, ica_cert.data(), ica_cert.size(), leaf_cert.data(), leaf_cert.size(), + chainValidationResult); + NL_TEST_ASSERT(inSuite, err == CHIP_ERROR_INVALID_ARGUMENT); + NL_TEST_ASSERT(inSuite, chainValidationResult == CertificateChainValidationResult::kRootArgumentInvalid); + + err = ValidateCertificateChain(root_cert.data(), root_cert.size(), nullptr, 0, leaf_cert.data(), leaf_cert.size(), + chainValidationResult); + NL_TEST_ASSERT(inSuite, err == CHIP_ERROR_INVALID_ARGUMENT); + NL_TEST_ASSERT(inSuite, chainValidationResult == CertificateChainValidationResult::kICAArgumentInvalid); + + err = ValidateCertificateChain(root_cert.data(), root_cert.size(), ica_cert.data(), ica_cert.size(), nullptr, 0, + chainValidationResult); + NL_TEST_ASSERT(inSuite, err == CHIP_ERROR_INVALID_ARGUMENT); + NL_TEST_ASSERT(inSuite, chainValidationResult == CertificateChainValidationResult::kLeafArgumentInvalid); + + // Now test with an ICA certificate that does not correspond to the chain + ByteSpan wrong_ica_cert; + err = GetTestCert(TestCert::kICA02, TestCertLoadFlags::kDERForm, wrong_ica_cert); NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR); - err = GetTestCert(TestCert::kNode01_02, TestCertLoadFlags::kDERForm, leaf_cert); - NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR); - - err = ValidateCertificateChain(root_cert.data(), root_cert.size(), nullptr, 0, leaf_cert.data(), leaf_cert.size()); - NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR); + err = ValidateCertificateChain(root_cert.data(), root_cert.size(), wrong_ica_cert.data(), wrong_ica_cert.size(), + leaf_cert.data(), leaf_cert.size(), chainValidationResult); + NL_TEST_ASSERT(inSuite, err == CHIP_ERROR_CERT_NOT_TRUSTED); + NL_TEST_ASSERT(inSuite, chainValidationResult == CertificateChainValidationResult::kChainInvalid); } static void TestX509_IssuingTimestampValidation(nlTestSuite * inSuite, void * inContext)