Skip to content

Commit

Permalink
Bring certificate validity logic into spec compliance (#19119)
Browse files Browse the repository at this point in the history 
* remove use of hard-coded time
* add configuration manager support for firmware build time
* implement spec-mandated Last Known Good UTC Time support
* enable certificate validity NotBefore checks
* add application-injectable certificate validity policies
* add ESP32 platform workaround for #19081

Fixes: #14202, #15584

Co-authored-by: Tennessee Carmel-Veilleux <[email protected]>
Co-authored-by: Evgeny Margolis <[email protected]>
Co-authored-by: Boris Zbarsky <[email protected]>
  • Loading branch information
@msandstedt @tcarmelveilleux
@emargolis @bzbarsky-apple
4 people authored Jun 7, 2022
1 parent 58b3fe5 commit 8cc291d
Show file tree
Hide file tree
Showing 44 changed files with 2,635 additions and 512 deletions.
6 changes: 3 additions & 3 deletions src/app/CASEClient.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,9 @@ CHIP_ERROR CASEClient::EstablishSession(PeerId peer, const Transport::PeerAddres
VerifyOrReturnError(exchange != nullptr, CHIP_ERROR_INTERNAL);

mCASESession.SetGroupDataProvider(mInitParams.groupDataProvider);
ReturnErrorOnFailure(mCASESession.EstablishSession(*mInitParams.sessionManager, mInitParams.fabricInfo, peer.GetNodeId(),
exchange, mInitParams.sessionResumptionStorage, delegate,
mInitParams.mrpLocalConfig));
ReturnErrorOnFailure(mCASESession.EstablishSession(
*mInitParams.sessionManager, mInitParams.fabricTable, mInitParams.fabricIndex, peer.GetNodeId(), exchange,
mInitParams.sessionResumptionStorage, mInitParams.certificateValidityPolicy, delegate, mInitParams.mrpLocalConfig));

return CHIP_NO_ERROR;
}
Expand Down
12 changes: 7 additions & 5 deletions src/app/CASEClient.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,13 @@ class CASEClient;

struct CASEClientInitParams
{
SessionManager * sessionManager = nullptr;
SessionResumptionStorage * sessionResumptionStorage = nullptr;
Messaging::ExchangeManager * exchangeMgr = nullptr;
FabricInfo * fabricInfo = nullptr;
Credentials::GroupDataProvider * groupDataProvider = nullptr;
SessionManager * sessionManager = nullptr;
SessionResumptionStorage * sessionResumptionStorage = nullptr;
Credentials::CertificateValidityPolicy * certificateValidityPolicy = nullptr;
Messaging::ExchangeManager * exchangeMgr = nullptr;
FabricTable * fabricTable = nullptr;
FabricIndex fabricIndex = kUndefinedFabricIndex;
Credentials::GroupDataProvider * groupDataProvider = nullptr;

Optional<ReliableMessageProtocolConfig> mrpLocalConfig = Optional<ReliableMessageProtocolConfig>::Missing();
};
Expand Down
10 changes: 5 additions & 5 deletions src/app/OperationalDeviceProxy.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ bool OperationalDeviceProxy::AttachToExistingSecureSession()
{
VerifyOrReturnError(mState == State::NeedsAddress || mState == State::ResolvingAddress || mState == State::HasAddress, false);

ScopedNodeId peerNodeId(mPeerId.GetNodeId(), mFabricInfo->GetFabricIndex());
ScopedNodeId peerNodeId(mPeerId.GetNodeId(), mFabricIndex);
auto sessionHandle =
mInitParams.sessionManager->FindSecureSessionForNode(peerNodeId, MakeOptional(Transport::SecureSession::Type::kCASE));
if (!sessionHandle.HasValue())
Expand Down Expand Up @@ -211,9 +211,9 @@ void OperationalDeviceProxy::UpdateDeviceData(const Transport::PeerAddress & add

CHIP_ERROR OperationalDeviceProxy::EstablishConnection()
{
mCASEClient = mInitParams.clientPool->Allocate(
CASEClientInitParams{ mInitParams.sessionManager, mInitParams.sessionResumptionStorage, mInitParams.exchangeMgr,
mFabricInfo, mInitParams.groupDataProvider, mInitParams.mrpLocalConfig });
mCASEClient = mInitParams.clientPool->Allocate(CASEClientInitParams{
mInitParams.sessionManager, mInitParams.sessionResumptionStorage, mInitParams.certificateValidityPolicy,
mInitParams.exchangeMgr, mFabricTable, mFabricIndex, mInitParams.groupDataProvider, mInitParams.mrpLocalConfig });
ReturnErrorCodeIf(mCASEClient == nullptr, CHIP_ERROR_NO_MEMORY);

CHIP_ERROR err = mCASEClient->EstablishSession(mPeerId, mDeviceAddress, mRemoteMRPConfig, this);
Expand Down Expand Up @@ -352,7 +352,7 @@ void OperationalDeviceProxy::OnSessionHang()

CHIP_ERROR OperationalDeviceProxy::ShutdownSubscriptions()
{
return app::InteractionModelEngine::GetInstance()->ShutdownSubscriptions(mFabricInfo->GetFabricIndex(), GetDeviceId());
return app::InteractionModelEngine::GetInstance()->ShutdownSubscriptions(mFabricIndex, GetDeviceId());
}

OperationalDeviceProxy::~OperationalDeviceProxy()
Expand Down
37 changes: 20 additions & 17 deletions src/app/OperationalDeviceProxy.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,12 +47,13 @@ namespace chip {

struct DeviceProxyInitParams
{
SessionManager * sessionManager = nullptr;
SessionResumptionStorage * sessionResumptionStorage = nullptr;
Messaging::ExchangeManager * exchangeMgr = nullptr;
FabricTable * fabricTable = nullptr;
CASEClientPoolDelegate * clientPool = nullptr;
Credentials::GroupDataProvider * groupDataProvider = nullptr;
SessionManager * sessionManager = nullptr;
SessionResumptionStorage * sessionResumptionStorage = nullptr;
Credentials::CertificateValidityPolicy * certificateValidityPolicy = nullptr;
Messaging::ExchangeManager * exchangeMgr = nullptr;
FabricTable * fabricTable = nullptr;
CASEClientPoolDelegate * clientPool = nullptr;
Credentials::GroupDataProvider * groupDataProvider = nullptr;

Optional<ReliableMessageProtocolConfig> mrpLocalConfig = Optional<ReliableMessageProtocolConfig>::Missing();

Expand Down Expand Up @@ -105,8 +106,16 @@ class DLL_EXPORT OperationalDeviceProxy : public DeviceProxy,

mSystemLayer = params.exchangeMgr->GetSessionManager()->SystemLayer();
mPeerId = peerId;
mFabricInfo = params.fabricTable->FindFabricWithCompressedId(peerId.GetCompressedFabricId());
mState = State::NeedsAddress;
mFabricTable = params.fabricTable;
if (mFabricTable != nullptr)
{
auto fabricInfo = params.fabricTable->FindFabricWithCompressedId(peerId.GetCompressedFabricId());
if (fabricInfo != nullptr)
{
mFabricIndex = fabricInfo->GetFabricIndex();
}
}
mState = State::NeedsAddress;
mAddressLookupHandle.SetListener(this);
}

Expand Down Expand Up @@ -192,14 +201,7 @@ class DLL_EXPORT OperationalDeviceProxy : public DeviceProxy,
/**
* @brief Get the raw Fabric ID assigned to the device.
*/
FabricIndex GetFabricIndex() const
{
if (mFabricInfo != nullptr)
{
return mFabricInfo->GetFabricIndex();
}
return kUndefinedFabricIndex;
}
FabricIndex GetFabricIndex() const { return mFabricIndex; }

/**
* Triggers a DNSSD lookup to find a usable peer address for this operational device.
Expand All @@ -222,7 +224,8 @@ class DLL_EXPORT OperationalDeviceProxy : public DeviceProxy,
};

DeviceProxyInitParams mInitParams;
FabricInfo * mFabricInfo;
FabricTable * mFabricTable = nullptr;
FabricIndex mFabricIndex = kUndefinedFabricIndex;
System::Layer * mSystemLayer;

// mCASEClient is only non-null if we are in State::Connecting or just
Expand Down
37 changes: 32 additions & 5 deletions src/app/clusters/operational-credentials-server/operational-credentials-server.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -316,7 +316,17 @@ void FailSafeCleanup(const chip::DeviceLayer::ChipDeviceEvent * event)
// command.
if (event->FailSafeTimerExpired.addNocCommandHasBeenInvoked)
{
DeleteFabricFromTable(fabricIndex);
CHIP_ERROR err;
err = DeleteFabricFromTable(fabricIndex);
if (err != CHIP_NO_ERROR)
{
ChipLogError(Zcl, "OpCreds: failed to delete fabric at index %u: %" CHIP_ERROR_FORMAT, fabricIndex, err.Format());
}
err = Server::GetInstance().GetFabricTable().RevertLastKnownGoodChipEpochTime();
if (err != CHIP_NO_ERROR)
{
ChipLogError(Zcl, "OpCreds: failed to revert Last Known Good Time: %" CHIP_ERROR_FORMAT, err.Format());
}
}

// If an UpdateNOC command had been successfully invoked, revert the state of operational key pair, NOC and ICAC for that
Expand All @@ -325,6 +335,22 @@ void FailSafeCleanup(const chip::DeviceLayer::ChipDeviceEvent * event)
if (event->FailSafeTimerExpired.updateNocCommandHasBeenInvoked)
{
// TODO: Revert the state of operational key pair, NOC and ICAC
CHIP_ERROR err = Server::GetInstance().GetFabricTable().RevertLastKnownGoodChipEpochTime();
if (err != CHIP_NO_ERROR)
{
ChipLogError(Zcl, "OpCreds: failed to revert Last Known Good Time: %" CHIP_ERROR_FORMAT, err.Format());
}
}
}

void CommissioningComplete(const chip::DeviceLayer::ChipDeviceEvent * event)
{
ChipLogProgress(Zcl, "OpCreds: Commissioning Complete");

CHIP_ERROR err = Server::GetInstance().GetFabricTable().CommitLastKnownGoodChipEpochTime();
if (err != CHIP_NO_ERROR)
{
ChipLogError(Zcl, "OpCreds: failed to commit Last Known Good Time: %" CHIP_ERROR_FORMAT, err.Format());
}
}

Expand All @@ -334,6 +360,10 @@ void OnPlatformEventHandler(const chip::DeviceLayer::ChipDeviceEvent * event, in
{
FailSafeCleanup(event);
}
if (event->Type == DeviceLayer::DeviceEventType::kCommissioningComplete)
{
CommissioningComplete(event);
}
}

} // anonymous namespace
Expand Down Expand Up @@ -840,10 +870,7 @@ bool emberAfOperationalCredentialsClusterUpdateNOCCallback(app::CommandHandler *
VerifyOrExit(err == CHIP_NO_ERROR, nocResponse = ConvertToNOCResponseStatus(err));
}

err = fabric->SetFabricInfo(gFabricBeingCommissioned);
VerifyOrExit(err == CHIP_NO_ERROR, nocResponse = ConvertToNOCResponseStatus(err));

err = Server::GetInstance().GetFabricTable().Store(fabricIndex);
err = Server::GetInstance().GetFabricTable().UpdateFabric(fabricIndex, gFabricBeingCommissioned);
VerifyOrExit(err == CHIP_NO_ERROR, nocResponse = ConvertToNOCResponseStatus(err));

// Flag on the fail-safe context that the UpdateNOC command was invoked.
Expand Down
7 changes: 5 additions & 2 deletions src/app/server/Server.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,8 @@ CHIP_ERROR Server::Init(const ServerInitParams & initParams)
mDeviceStorage = initParams.persistentStorageDelegate;
mSessionResumptionStorage = initParams.sessionResumptionStorage;

mCertificateValidityPolicy = initParams.certificateValidityPolicy;

// Set up attribute persistence before we try to bring up the data model
// handler.
SuccessOrExit(mAttributePersister.Init(mDeviceStorage));
Expand Down Expand Up @@ -255,6 +257,7 @@ CHIP_ERROR Server::Init(const ServerInitParams & initParams)
.sessionInitParams = {
.sessionManager = &mSessions,
.sessionResumptionStorage = mSessionResumptionStorage,
.certificateValidityPolicy = mCertificateValidityPolicy,
.exchangeMgr = &mExchangeMgr,
.fabricTable = &mFabrics,
.clientPool = &mCASEClientPool,
Expand All @@ -266,8 +269,8 @@ CHIP_ERROR Server::Init(const ServerInitParams & initParams)
err = mCASESessionManager.Init(&DeviceLayer::SystemLayer(), caseSessionManagerConfig);
SuccessOrExit(err);

err =
mCASEServer.ListenForSessionEstablishment(&mExchangeMgr, &mSessions, &mFabrics, mSessionResumptionStorage, mGroupsProvider);
err = mCASEServer.ListenForSessionEstablishment(&mExchangeMgr, &mSessions, &mFabrics, mSessionResumptionStorage,
mCertificateValidityPolicy, mGroupsProvider);
SuccessOrExit(err);

// This code is necessary to restart listening to existing groups after a reboot
Expand Down
4 changes: 4 additions & 0 deletions src/app/server/Server.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,9 @@ struct ServerInitParams
// Session resumption storage: Optional. Support session resumption when provided.
// Must be initialized before being provided.
SessionResumptionStorage * sessionResumptionStorage = nullptr;
// Certificate validity policy: Optional. If none is injected, CHIPCert
// enforces a default policy.
Credentials::CertificateValidityPolicy * certificateValidityPolicy = nullptr;
// Group data provider: MUST be injected. Used to maintain critical keys such as the Identity
// Protection Key (IPK) for CASE. Must be initialized before being provided.
Credentials::GroupDataProvider * groupDataProvider = nullptr;
Expand Down Expand Up @@ -394,6 +397,7 @@ class Server

PersistentStorageDelegate * mDeviceStorage;
SessionResumptionStorage * mSessionResumptionStorage;
Credentials::CertificateValidityPolicy * mCertificateValidityPolicy;
Credentials::GroupDataProvider * mGroupsProvider;
app::DefaultAttributePersistenceProvider mAttributePersister;
GroupDataProviderListener mListener;
Expand Down
5 changes: 1 addition & 4 deletions src/controller/CHIPDeviceController.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -191,10 +191,7 @@ CHIP_ERROR DeviceController::InitControllerNOCChain(const ControllerInitParams &
mFabricInfo = params.systemState->Fabrics()->FindFabric(rootPublicKey, fabricId);
if (mFabricInfo != nullptr)
{
ReturnErrorOnFailure(mFabricInfo->SetFabricInfo(newFabric));
// Store the new fabric info, since we might now have new certificates
// and whatnot.
ReturnErrorOnFailure(params.systemState->Fabrics()->Store(mFabricInfo->GetFabricIndex()));
ReturnErrorOnFailure(params.systemState->Fabrics()->UpdateFabric(mFabricInfo->GetFabricIndex(), newFabric));
}
else
{
Expand Down
3 changes: 2 additions & 1 deletion src/controller/CHIPDeviceControllerFactory.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,7 @@ CHIP_ERROR DeviceControllerFactory::InitSystemState(FactoryInitParams params)
stateParams.sessionMgr = chip::Platform::New<SessionManager>();
SimpleSessionResumptionStorage * sessionResumptionStorage = chip::Platform::New<SimpleSessionResumptionStorage>();
stateParams.sessionResumptionStorage = sessionResumptionStorage;
stateParams.certificateValidityPolicy = params.certificateValidityPolicy;
stateParams.exchangeMgr = chip::Platform::New<Messaging::ExchangeManager>();
stateParams.messageCounterManager = chip::Platform::New<secure_channel::MessageCounterManager>();
stateParams.groupDataProvider = params.groupDataProvider;
Expand Down Expand Up @@ -190,7 +191,7 @@ CHIP_ERROR DeviceControllerFactory::InitSystemState(FactoryInitParams params)
// Enable listening for session establishment messages.
ReturnErrorOnFailure(stateParams.caseServer->ListenForSessionEstablishment(
stateParams.exchangeMgr, stateParams.sessionMgr, stateParams.fabricTable, stateParams.sessionResumptionStorage,
stateParams.groupDataProvider));
stateParams.certificateValidityPolicy, stateParams.groupDataProvider));

//
// We need to advertise the port that we're listening to for unsolicited messages over UDP. However, we have both a IPv4
Expand Down
13 changes: 7 additions & 6 deletions src/controller/CHIPDeviceControllerFactory.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -81,12 +81,13 @@ struct SetupParams
// We're blocked because of the need to support !CHIP_DEVICE_LAYER
struct FactoryInitParams
{
System::Layer * systemLayer = nullptr;
PersistentStorageDelegate * fabricIndependentStorage = nullptr;
Credentials::GroupDataProvider * groupDataProvider = nullptr;
Inet::EndPointManager<Inet::TCPEndPoint> * tcpEndPointManager = nullptr;
Inet::EndPointManager<Inet::UDPEndPoint> * udpEndPointManager = nullptr;
FabricTable * fabricTable = nullptr;
System::Layer * systemLayer = nullptr;
PersistentStorageDelegate * fabricIndependentStorage = nullptr;
Credentials::CertificateValidityPolicy * certificateValidityPolicy = nullptr;
Credentials::GroupDataProvider * groupDataProvider = nullptr;
Inet::EndPointManager<Inet::TCPEndPoint> * tcpEndPointManager = nullptr;
Inet::EndPointManager<Inet::UDPEndPoint> * udpEndPointManager = nullptr;
FabricTable * fabricTable = nullptr;
#if CONFIG_NETWORK_LAYER_BLE
Ble::BleLayer * bleLayer = nullptr;
#endif
Expand Down
21 changes: 11 additions & 10 deletions src/controller/CHIPDeviceControllerSystemState.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,16 +82,17 @@ struct DeviceControllerSystemStateParams

// Params that will be deallocated via Platform::Delete in
// DeviceControllerSystemState::Shutdown.
DeviceTransportMgr * transportMgr = nullptr;
SessionResumptionStorage * sessionResumptionStorage = nullptr;
SessionManager * sessionMgr = nullptr;
Messaging::ExchangeManager * exchangeMgr = nullptr;
secure_channel::MessageCounterManager * messageCounterManager = nullptr;
CASEServer * caseServer = nullptr;
CASESessionManager * caseSessionManager = nullptr;
OperationalDevicePool * operationalDevicePool = nullptr;
CASEClientPool * caseClientPool = nullptr;
FabricTable::Delegate * fabricTableDelegate = nullptr;
DeviceTransportMgr * transportMgr = nullptr;
SessionResumptionStorage * sessionResumptionStorage = nullptr;
Credentials::CertificateValidityPolicy * certificateValidityPolicy = nullptr;
SessionManager * sessionMgr = nullptr;
Messaging::ExchangeManager * exchangeMgr = nullptr;
secure_channel::MessageCounterManager * messageCounterManager = nullptr;
CASEServer * caseServer = nullptr;
CASESessionManager * caseSessionManager = nullptr;
OperationalDevicePool * operationalDevicePool = nullptr;
CASEClientPool * caseClientPool = nullptr;
FabricTable::Delegate * fabricTableDelegate = nullptr;
};

// A representation of the internal state maintained by the DeviceControllerFactory
Expand Down
5 changes: 5 additions & 0 deletions src/credentials/BUILD.gn
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ static_library("credentials") {
"CHIPCert.h",
"CHIPCertFromX509.cpp",
"CHIPCertToX509.cpp",
"CHIPCertificateSet.h",
"CertificationDeclaration.cpp",
"CertificationDeclaration.h",
"DeviceAttestationConstructor.cpp",
Expand All @@ -37,6 +38,8 @@ static_library("credentials") {
"GenerateChipX509Cert.cpp",
"GroupDataProvider.h",
"GroupDataProviderImpl.cpp",
"LastKnownGoodTime.cpp",
"LastKnownGoodTime.h",
"attestation_verifier/DeviceAttestationDelegate.h",
"attestation_verifier/DeviceAttestationVerifier.cpp",
"attestation_verifier/DeviceAttestationVerifier.h",
Expand All @@ -46,6 +49,8 @@ static_library("credentials") {
"examples/ExampleDACs.h",
"examples/ExamplePAI.cpp",
"examples/ExamplePAI.h",
"examples/LastKnownGoodTimeCertificateValidityPolicyExample.h",
"examples/StrictCertificateValidityPolicyExample.h",
]

# TODO: These tests files should be removed after the DeviceAttestationCredsExample implementation
Expand Down
Loading

0 comments on commit 8cc291d

Please sign in to comment.