From 47645793b501ebffd68328330aca0a0029a08a47 Mon Sep 17 00:00:00 2001 From: Boris Zbarsky Date: Fri, 5 May 2023 12:19:38 -0400 Subject: [PATCH] Ignore root cert expiration in Server. (#26372) Since there is no real way to rotate the root cert for a fabric, even just to update its validity period, enforcing the validity period for it just means making the fabric not work and runs the risk of making devices completely unreachable. Switch to not validating expiration time for a root certificate, while keeping existing behavior for the NotBefore and all other certificate types. --- src/app/server/Server.cpp | 6 +-- src/app/server/Server.h | 50 ++++++++++++++++++++- src/credentials/CHIPCert.cpp | 46 ++++++++++--------- src/credentials/CertificateValidityPolicy.h | 6 +++ 4 files changed, 84 insertions(+), 24 deletions(-) diff --git a/src/app/server/Server.cpp b/src/app/server/Server.cpp index b80c3ca8f45b11..344776c49ac93b 100644 --- a/src/app/server/Server.cpp +++ b/src/app/server/Server.cpp @@ -128,7 +128,7 @@ CHIP_ERROR Server::Init(const ServerInitParams & initParams) mOperationalKeystore = initParams.operationalKeystore; mOpCertStore = initParams.opCertStore; - mCertificateValidityPolicy = initParams.certificateValidityPolicy; + mCertificateValidityPolicy.Init(initParams.certificateValidityPolicy); #if defined(CHIP_SUPPORT_ENABLE_STORAGE_API_AUDIT) VerifyOrDie(chip::audit::ExecutePersistentStorageApiAudit(*mDeviceStorage)); @@ -286,7 +286,7 @@ CHIP_ERROR Server::Init(const ServerInitParams & initParams) .sessionInitParams = { .sessionManager = &mSessions, .sessionResumptionStorage = mSessionResumptionStorage, - .certificateValidityPolicy = mCertificateValidityPolicy, + .certificateValidityPolicy = &mCertificateValidityPolicy, .exchangeMgr = &mExchangeMgr, .fabricTable = &mFabrics, .groupDataProvider = mGroupsProvider, @@ -300,7 +300,7 @@ CHIP_ERROR Server::Init(const ServerInitParams & initParams) SuccessOrExit(err); err = mCASEServer.ListenForSessionEstablishment(&mExchangeMgr, &mSessions, &mFabrics, mSessionResumptionStorage, - mCertificateValidityPolicy, mGroupsProvider); + &mCertificateValidityPolicy, mGroupsProvider); SuccessOrExit(err); err = chip::app::InteractionModelEngine::GetInstance()->Init(&mExchangeMgr, &GetFabricTable(), &mCASESessionManager, diff --git a/src/app/server/Server.h b/src/app/server/Server.h index 45bcf0a7290515..380e8240abfed9 100644 --- a/src/app/server/Server.h +++ b/src/app/server/Server.h @@ -539,6 +539,53 @@ class Server Server * mServer = nullptr; }; + /** + * Since root certificates for Matter nodes cannot be updated in a reasonable + * way, it doesn't make sense to enforce expiration time on root certificates. + * This policy allows through root certificates, even if they're expired, and + * otherwise delegates to the provided policy, or to the default policy if no + * policy is provided. + */ + class IgnoreRootExpirationValidityPolicy : public Credentials::CertificateValidityPolicy + { + public: + IgnoreRootExpirationValidityPolicy() {} + + void Init(Credentials::CertificateValidityPolicy * providedPolicy) { mProvidedPolicy = providedPolicy; } + + CHIP_ERROR ApplyCertificateValidityPolicy(const Credentials::ChipCertificateData * cert, uint8_t depth, + Credentials::CertificateValidityResult result) override + { + switch (result) + { + case Credentials::CertificateValidityResult::kExpired: + case Credentials::CertificateValidityResult::kExpiredAtLastKnownGoodTime: + case Credentials::CertificateValidityResult::kTimeUnknown: { + uint8_t certType; + ReturnErrorOnFailure(cert->mSubjectDN.GetCertType(certType)); + if (certType == Credentials::kCertType_Root) + { + return CHIP_NO_ERROR; + } + + break; + } + default: + break; + } + + if (mProvidedPolicy) + { + return mProvidedPolicy->ApplyCertificateValidityPolicy(cert, depth, result); + } + + return Credentials::CertificateValidityPolicy::ApplyDefaultPolicy(cert, depth, result); + } + + private: + Credentials::CertificateValidityPolicy * mProvidedPolicy = nullptr; + }; + #if CONFIG_NETWORK_LAYER_BLE Ble::BleLayer * mBleLayer = nullptr; #endif @@ -560,10 +607,11 @@ class Server #endif // CHIP_DEVICE_CONFIG_ENABLE_COMMISSIONER_DISCOVERY_CLIENT CommissioningWindowManager mCommissioningWindowManager; + IgnoreRootExpirationValidityPolicy mCertificateValidityPolicy; + PersistentStorageDelegate * mDeviceStorage; SessionResumptionStorage * mSessionResumptionStorage; app::SubscriptionResumptionStorage * mSubscriptionResumptionStorage; - Credentials::CertificateValidityPolicy * mCertificateValidityPolicy; Credentials::GroupDataProvider * mGroupsProvider; Crypto::SessionKeystore * mSessionKeystore; app::DefaultAttributePersistenceProvider mAttributePersister; diff --git a/src/credentials/CHIPCert.cpp b/src/credentials/CHIPCert.cpp index 3d6af3db842e8e..6c1c2012d32d6d 100644 --- a/src/credentials/CHIPCert.cpp +++ b/src/credentials/CHIPCert.cpp @@ -431,26 +431,7 @@ CHIP_ERROR ChipCertificateSet::ValidateCert(const ChipCertificateData * cert, Va } else { - switch (validityResult) - { - case CertificateValidityResult::kValid: - case CertificateValidityResult::kNotExpiredAtLastKnownGoodTime: - // By default, we do not enforce certificate validity based upon a Last - // Known Good Time source. However, implementations may always inject a - // policy that does enforce based upon this. - case CertificateValidityResult::kExpiredAtLastKnownGoodTime: - case CertificateValidityResult::kTimeUnknown: - break; - case CertificateValidityResult::kNotYetValid: - ExitNow(err = CHIP_ERROR_CERT_NOT_VALID_YET); - break; - case CertificateValidityResult::kExpired: - ExitNow(err = CHIP_ERROR_CERT_EXPIRED); - break; - default: - ExitNow(err = CHIP_ERROR_INTERNAL); - break; - } + SuccessOrExit(err = CertificateValidityPolicy::ApplyDefaultPolicy(cert, depth, validityResult)); } // If the certificate itself is trusted, then it is implicitly valid. Record this certificate as the trust @@ -1505,5 +1486,30 @@ CHIP_ERROR ExtractSubjectDNFromX509Cert(const ByteSpan & x509Cert, ChipDN & dn) return err; } +CHIP_ERROR CertificateValidityPolicy::ApplyDefaultPolicy(const ChipCertificateData * cert, uint8_t depth, + CertificateValidityResult result) +{ + switch (result) + { + case CertificateValidityResult::kValid: + case CertificateValidityResult::kNotExpiredAtLastKnownGoodTime: + // By default, we do not enforce certificate validity based upon a Last + // Known Good Time source. However, implementations may always inject a + // policy that does enforce based upon this. + case CertificateValidityResult::kExpiredAtLastKnownGoodTime: + case CertificateValidityResult::kTimeUnknown: + return CHIP_NO_ERROR; + + case CertificateValidityResult::kNotYetValid: + return CHIP_ERROR_CERT_NOT_VALID_YET; + + case CertificateValidityResult::kExpired: + return CHIP_ERROR_CERT_EXPIRED; + + default: + return CHIP_ERROR_INTERNAL; + } +} + } // namespace Credentials } // namespace chip diff --git a/src/credentials/CertificateValidityPolicy.h b/src/credentials/CertificateValidityPolicy.h index 0df351dff8390c..121647d1b94226 100644 --- a/src/credentials/CertificateValidityPolicy.h +++ b/src/credentials/CertificateValidityPolicy.h @@ -52,6 +52,12 @@ class CertificateValidityPolicy */ virtual CHIP_ERROR ApplyCertificateValidityPolicy(const ChipCertificateData * cert, uint8_t depth, CertificateValidityResult result) = 0; + + /** + * Default policy that will be used if no other policy is defined. This is + * exposed to allow other policies to explicitly delegate to it as needed. + */ + static CHIP_ERROR ApplyDefaultPolicy(const ChipCertificateData * cert, uint8_t depth, CertificateValidityResult result); }; } // namespace Credentials