From 8692a597cb19b7f337924dbc09fde0db1016daf3 Mon Sep 17 00:00:00 2001 From: Pankaj Garg Date: Fri, 11 Jun 2021 08:55:11 -0700 Subject: [PATCH 01/12] Enable CASE session establishment --- src/app/server/RendezvousServer.cpp | 12 +- src/app/server/RendezvousServer.h | 13 +- src/app/server/Server.cpp | 16 +- src/controller/CHIPDevice.cpp | 92 ++++++--- src/controller/CHIPDevice.h | 27 ++- src/controller/CHIPDeviceController.cpp | 180 +++++++++++------- src/controller/CHIPDeviceController.h | 16 +- .../ExampleOperationalCredentialsIssuer.cpp | 11 ++ src/protocols/secure_channel/BUILD.gn | 2 + src/protocols/secure_channel/CASEServer.cpp | 30 +-- src/protocols/secure_channel/CASEServer.h | 8 +- src/protocols/secure_channel/CASESession.cpp | 19 +- .../secure_channel/SessionIDAllocator.cpp | 64 +++++++ .../secure_channel/SessionIDAllocator.h | 41 ++++ src/transport/SecureSessionMgr.cpp | 10 + src/transport/SecureSessionMgr.h | 2 + 16 files changed, 391 insertions(+), 152 deletions(-) create mode 100644 src/protocols/secure_channel/SessionIDAllocator.cpp create mode 100644 src/protocols/secure_channel/SessionIDAllocator.h diff --git a/src/app/server/RendezvousServer.cpp b/src/app/server/RendezvousServer.cpp index 3055f2478e86b8..344075ca039ec7 100644 --- a/src/app/server/RendezvousServer.cpp +++ b/src/app/server/RendezvousServer.cpp @@ -100,15 +100,18 @@ CHIP_ERROR RendezvousServer::WaitForPairing(const RendezvousParameters & params, ReturnErrorOnFailure(mExchangeManager->RegisterUnsolicitedMessageHandlerForType( Protocols::SecureChannel::MsgType::PBKDFParamRequest, &mPairingSession)); + uint16_t keyID = 0; + ReturnErrorOnFailure(mIDAllocator->Allocate(keyID)); + if (params.HasPASEVerifier()) { - ReturnErrorOnFailure(mPairingSession.WaitForPairing(params.GetPASEVerifier(), mNextKeyId++, this)); + ReturnErrorOnFailure(mPairingSession.WaitForPairing(params.GetPASEVerifier(), keyID, this)); } else { ReturnErrorOnFailure(mPairingSession.WaitForPairing(params.GetSetupPINCode(), kSpake2p_Iteration_Count, reinterpret_cast(kSpake2pKeyExchangeSalt), - strlen(kSpake2pKeyExchangeSalt), mNextKeyId++, this)); + strlen(kSpake2pKeyExchangeSalt), keyID, this)); } ReturnErrorOnFailure(mPairingSession.MessageDispatch().Init(transportMgr)); @@ -181,6 +184,9 @@ void RendezvousServer::OnSessionEstablished() VerifyOrReturn(connection.StoreIntoKVS(*mStorage) == CHIP_NO_ERROR, ChipLogError(AppServer, "Failed to store the connection state")); - mStorage->SyncSetKeyValue(kStorablePeerConnectionCountKey, &mNextKeyId, sizeof(mNextKeyId)); + uint16_t keyID = 0; + mIDAllocator->Peek(keyID); + + mStorage->SyncSetKeyValue(kStorablePeerConnectionCountKey, &keyID, sizeof(keyID)); } } // namespace chip diff --git a/src/app/server/RendezvousServer.h b/src/app/server/RendezvousServer.h index 4ab2c21ce731fc..c9920976157e9e 100644 --- a/src/app/server/RendezvousServer.h +++ b/src/app/server/RendezvousServer.h @@ -22,6 +22,7 @@ #include #include #include +#include namespace chip { @@ -31,11 +32,12 @@ class RendezvousServer : public SessionEstablishmentDelegate CHIP_ERROR WaitForPairing(const RendezvousParameters & params, Messaging::ExchangeManager * exchangeManager, TransportMgrBase * transportMgr, SecureSessionMgr * sessionMgr, Transport::AdminPairingInfo * admin); - CHIP_ERROR Init(AppDelegate * delegate, PersistentStorageDelegate * storage) + CHIP_ERROR Init(AppDelegate * delegate, PersistentStorageDelegate * storage, SessionIDAllocator * idAllocator) { VerifyOrReturnError(storage != nullptr, CHIP_ERROR_INVALID_ARGUMENT); - mDelegate = delegate; - mStorage = storage; + mDelegate = delegate; + mStorage = storage; + mIDAllocator = idAllocator; return CHIP_NO_ERROR; } @@ -45,8 +47,6 @@ class RendezvousServer : public SessionEstablishmentDelegate void Cleanup(); - uint16_t GetNextKeyId() const { return mNextKeyId; } - void SetNextKeyId(uint16_t id) { mNextKeyId = id; } void OnPlatformEvent(const DeviceLayer::ChipDeviceEvent * event); private: @@ -55,11 +55,12 @@ class RendezvousServer : public SessionEstablishmentDelegate Messaging::ExchangeManager * mExchangeManager = nullptr; PASESession mPairingSession; - uint16_t mNextKeyId = 0; SecureSessionMgr * mSessionMgr = nullptr; Transport::AdminPairingInfo * mAdmin = nullptr; + SessionIDAllocator * mIDAllocator = nullptr; + const RendezvousAdvertisementDelegate * mAdvDelegate; bool HasAdvertisementDelegate() const { return mAdvDelegate != nullptr; } diff --git a/src/app/server/Server.cpp b/src/app/server/Server.cpp index 4198882d67486b..2eddf3c633dcbe 100644 --- a/src/app/server/Server.cpp +++ b/src/app/server/Server.cpp @@ -98,6 +98,7 @@ class ServerStorageDelegate : public PersistentStorageDelegate }; ServerStorageDelegate gServerStorage; +SessionIDAllocator gSessionIDAllocator; CHIP_ERROR PersistAdminPairingToKVS(AdminPairingInfo * admin, AdminId nextAvailableId) { @@ -149,7 +150,7 @@ void EraseAllAdminPairingsUpTo(AdminId nextAvailableId) } } -static CHIP_ERROR RestoreAllSessionsFromKVS(SecureSessionMgr & sessionMgr, RendezvousServer & server) +static CHIP_ERROR RestoreAllSessionsFromKVS(SecureSessionMgr & sessionMgr) { uint16_t nextSessionKeyId = 0; // It's not an error if the key doesn't exist. Just return right away. @@ -173,13 +174,13 @@ static CHIP_ERROR RestoreAllSessionsFromKVS(SecureSessionMgr & sessionMgr, Rende sessionMgr.NewPairing(Optional::Value(session->PeerConnection().GetPeerAddress()), session->PeerConnection().GetPeerNodeId(), session, SecureSession::SessionRole::kResponder, connection.GetAdminId(), nullptr); + gSessionIDAllocator.Reserve(keyId); session->Clear(); } } chip::Platform::Delete(session); - server.SetNextKeyId(nextSessionKeyId); return CHIP_NO_ERROR; } @@ -189,6 +190,7 @@ void EraseAllSessionsUpTo(uint16_t nextSessionKeyId) for (uint16_t keyId = 0; keyId < nextSessionKeyId; keyId++) { + gSessionIDAllocator.Free(keyId); StorablePeerConnection::DeleteFromKVS(gServerStorage, keyId); } } @@ -427,7 +429,8 @@ CHIP_ERROR OpenDefaultPairingWindow(ResetAdmins resetAdmins, chip::PairingWindow if (resetAdmins == ResetAdmins::kYes) { - uint16_t nextKeyId = gRendezvousServer.GetNextKeyId(); + uint16_t nextKeyId; + ReturnErrorOnFailure(gSessionIDAllocator.Peek(nextKeyId)); EraseAllAdminPairingsUpTo(gNextAvailableAdminId); EraseAllSessionsUpTo(nextKeyId); // Only resetting gNextAvailableAdminId at reboot otherwise previously paired device with adminID 0 @@ -462,7 +465,7 @@ void InitServer(AppDelegate * delegate) PersistedStorage::KeyValueStoreMgrImpl().Init("/tmp/chip_server_kvs"); #endif - err = gRendezvousServer.Init(delegate, &gServerStorage); + err = gRendezvousServer.Init(delegate, &gServerStorage, &gSessionIDAllocator); SuccessOrExit(err); gAdvDelegate.SetDelegate(delegate); @@ -517,7 +520,7 @@ void InitServer(AppDelegate * delegate) VerifyOrExit(CHIP_NO_ERROR == RestoreAllAdminPairingsFromKVS(gAdminPairings, gNextAvailableAdminId), ChipLogError(AppServer, "Could not restore admin table")); - VerifyOrExit(CHIP_NO_ERROR == RestoreAllSessionsFromKVS(gSessions, gRendezvousServer), + VerifyOrExit(CHIP_NO_ERROR == RestoreAllSessionsFromKVS(gSessions), ChipLogError(AppServer, "Could not restore previous sessions")); } else @@ -542,7 +545,8 @@ void InitServer(AppDelegate * delegate) err = gExchangeMgr.RegisterUnsolicitedMessageHandlerForProtocol(Protocols::ServiceProvisioning::Id, &gCallbacks); VerifyOrExit(err == CHIP_NO_ERROR, err = CHIP_ERROR_NO_UNSOLICITED_MESSAGE_HANDLER); - err = gCASEServer.ListenForSessionEstablishment(&gExchangeMgr, &gTransports, &gSessions, &GetGlobalAdminPairingTable()); + err = gCASEServer.ListenForSessionEstablishment(&gExchangeMgr, &gTransports, &gSessions, &GetGlobalAdminPairingTable(), + &gSessionIDAllocator); SuccessOrExit(err); exit: diff --git a/src/controller/CHIPDevice.cpp b/src/controller/CHIPDevice.cpp index e4f5e28fa4221c..c95776d0b49ad8 100644 --- a/src/controller/CHIPDevice.cpp +++ b/src/controller/CHIPDevice.cpp @@ -125,8 +125,7 @@ CHIP_ERROR Device::LoadSecureSessionParametersIfNeeded(bool & didLoad) } else { - Transport::PeerConnectionState * connectionState = nullptr; - connectionState = mSessionManager->GetPeerConnectionState(mSecureSession); + Transport::PeerConnectionState * connectionState = mSessionManager->GetPeerConnectionState(mSecureSession); // Check if the connection state has the correct transport information if (connectionState == nullptr || connectionState->GetPeerAddress().GetTransportType() == Transport::Type::kUndefined || @@ -172,8 +171,7 @@ CHIP_ERROR Device::Serialize(SerializedDevice & output) serializable.mLocalMessageCounter = Encoding::LittleEndian::HostSwap32(localMessageCounter); serializable.mPeerMessageCounter = Encoding::LittleEndian::HostSwap32(peerMessageCounter); - serializable.mCASESessionKeyId = Encoding::LittleEndian::HostSwap16(mCASESessionKeyId); - serializable.mDeviceProvisioningComplete = (mDeviceProvisioningComplete) ? 1 : 0; + serializable.mDeviceOperationalCertProvisioned = (mDeviceOperationalCertProvisioned) ? 1 : 0; static_assert(std::is_same::type, uint8_t>::value, "The underlying type of Transport::Type is not uint8_t."); @@ -232,8 +230,7 @@ CHIP_ERROR Device::Deserialize(const SerializedDevice & input) // the old counter value (which is 1 less than the updated counter). mLocalMessageCounter++; - mCASESessionKeyId = Encoding::LittleEndian::HostSwap16(serializable.mCASESessionKeyId); - mDeviceProvisioningComplete = (serializable.mDeviceProvisioningComplete != 0); + mDeviceOperationalCertProvisioned = (serializable.mDeviceOperationalCertProvisioned != 0); // The InterfaceNameToId() API requires initialization of mInterface, and lock/unlock of // LwIP stack. @@ -309,6 +306,8 @@ void Device::OnNewConnection(SecureSessionHandle session) void Device::OnConnectionExpired(SecureSessionHandle session) { + VerifyOrReturn(session == mSecureSession, + ChipLogDetail(Controller, "Connection expired, but it doesn't match the current session")); mState = ConnectionState::NotConnected; mSecureSession = SecureSessionHandle{}; } @@ -426,9 +425,6 @@ CHIP_ERROR Device::LoadSecureSessionParameters(ResetTransport resetNeeded) ExitNow(err = CHIP_ERROR_INCORRECT_STATE); } - err = pairingSession.FromSerializable(mPairing); - SuccessOrExit(err); - if (resetNeeded == ResetTransport::kYes) { err = mTransportMgr->ResetTransport( @@ -445,16 +441,20 @@ CHIP_ERROR Device::LoadSecureSessionParameters(ResetTransport resetNeeded) SuccessOrExit(err); } - err = mSessionManager->NewPairing(Optional::Value(mDeviceAddress), mDeviceId, &pairingSession, - SecureSession::SessionRole::kInitiator, mAdminId); - SuccessOrExit(err); + if (IsOperationalCertProvisioned()) + { + err = WarmupCASESession(); + SuccessOrExit(err); + } + else + { + err = pairingSession.FromSerializable(mPairing); + SuccessOrExit(err); - // TODO - Enable CASE Session setup before message is sent to a fully provisioned device - // if (IsProvisioningComplete()) - // { - // err = EstablishCASESession(); - // SuccessOrExit(err); - // } + err = mSessionManager->NewPairing(Optional::Value(mDeviceAddress), mDeviceId, &pairingSession, + SecureSession::SessionRole::kInitiator, mAdminId); + SuccessOrExit(err); + } exit: @@ -475,35 +475,65 @@ bool Device::GetAddress(Inet::IPAddress & addr, uint16_t & port) const return true; } -CHIP_ERROR Device::EstablishCASESession() +void Device::OperationalCertProvisioned() { + VerifyOrReturn(!mDeviceOperationalCertProvisioned, + ChipLogDetail(Controller, "Operational certificates already provisioned for this device")); + + ChipLogDetail(Controller, "Enabling CASE session establishment for the device"); + mDeviceOperationalCertProvisioned = true; + + if (mState == ConnectionState::SecureConnected) + { + mSessionManager->ExpirePairing(mSecureSession); + mState = ConnectionState::NotConnected; + } + + WarmupCASESession(); +} + +CHIP_ERROR Device::WarmupCASESession() +{ + VerifyOrReturnError(mDeviceOperationalCertProvisioned, CHIP_ERROR_INCORRECT_STATE); + VerifyOrReturnError(mState == ConnectionState::NotConnected, CHIP_NO_ERROR); + Messaging::ExchangeContext * exchange = mExchangeMgr->NewContext(SecureSessionHandle(), &mCASESession); VerifyOrReturnError(exchange != nullptr, CHIP_ERROR_INTERNAL); ReturnErrorOnFailure(mCASESession.MessageDispatch().Init(mSessionManager->GetTransportManager())); mCASESession.MessageDispatch().SetPeerAddress(mDeviceAddress); - ReturnErrorOnFailure(mCASESession.EstablishSession(mDeviceAddress, mCredentials, mDeviceId, 0, exchange, this)); + uint16_t keyID = 0; + ReturnErrorOnFailure(mIDAllocator->Allocate(keyID)); + + mLocalMessageCounter = 0; + mPeerMessageCounter = 0; + + ReturnErrorOnFailure(mCASESession.EstablishSession(mDeviceAddress, mCredentials, mDeviceId, keyID, exchange, this)); + + mState = ConnectionState::Connecting; return CHIP_NO_ERROR; } -void Device::OnSessionEstablishmentError(CHIP_ERROR error) {} +void Device::OnSessionEstablishmentError(CHIP_ERROR error) +{ + mState = ConnectionState::NotConnected; + mIDAllocator->Free(mCASESession.GetLocalKeyId()); +} void Device::OnSessionEstablished() { mCASESession.PeerConnection().SetPeerNodeId(mDeviceId); - // TODO - Enable keys derived from CASE Session - // CHIP_ERROR err = mSessionManager->NewPairing(Optional::Value(mDeviceAddress), mDeviceId, - // &mCASESession, - // SecureSession::SessionRole::kInitiator, mAdminId, nullptr); - // if (err != CHIP_NO_ERROR) - // { - // ChipLogError(Controller, "Failed in setting up CASE secure channel: err %s", ErrorStr(err)); - // OnSessionEstablishmentError(err); - // return; - // } + CHIP_ERROR err = mSessionManager->NewPairing(Optional::Value(mDeviceAddress), mDeviceId, &mCASESession, + SecureSession::SessionRole::kInitiator, mAdminId, nullptr); + if (err != CHIP_NO_ERROR) + { + ChipLogError(Controller, "Failed in setting up CASE secure channel: err %s", ErrorStr(err)); + OnSessionEstablishmentError(err); + return; + } } void Device::AddResponseHandler(uint8_t seqNum, Callback::Cancelable * onSuccessCallback, Callback::Cancelable * onFailureCallback) diff --git a/src/controller/CHIPDevice.h b/src/controller/CHIPDevice.h index 9d683a71bda340..10d2ac46c736e3 100644 --- a/src/controller/CHIPDevice.h +++ b/src/controller/CHIPDevice.h @@ -38,6 +38,7 @@ #include #include #include +#include #include #include #include @@ -80,6 +81,7 @@ struct ControllerDeviceInitParams Inet::InetLayer * inetLayer = nullptr; PersistentStorageDelegate * storageDelegate = nullptr; Credentials::OperationalCredentialSet * credentials = nullptr; + SessionIDAllocator * idAllocator = nullptr; #if CONFIG_NETWORK_LAYER_BLE Ble::BleLayer * bleLayer = nullptr; #endif @@ -175,6 +177,7 @@ class DLL_EXPORT Device : public Messaging::ExchangeDelegate, public SessionEsta mAdminId = admin; mStorageDelegate = params.storageDelegate; mCredentials = params.credentials; + mIDAllocator = params.idAllocator; #if CONFIG_NETWORK_LAYER_BLE mBleLayer = params.bleLayer; #endif @@ -334,12 +337,8 @@ class DLL_EXPORT Device : public Messaging::ExchangeDelegate, public SessionEsta Callback::Cancelable * onFailureCallback); void CancelIMResponseHandler(app::Command * commandObj); - void ProvisioningComplete(uint16_t caseKeyId) - { - mDeviceProvisioningComplete = true; - mCASESessionKeyId = caseKeyId; - } - bool IsProvisioningComplete() const { return mDeviceProvisioningComplete; } + void OperationalCertProvisioned(); + bool IsOperationalCertProvisioned() const { return mDeviceOperationalCertProvisioned; } //////////// SessionEstablishmentDelegate Implementation /////////////// void OnSessionEstablishmentError(CHIP_ERROR error) override; @@ -351,6 +350,8 @@ class DLL_EXPORT Device : public Messaging::ExchangeDelegate, public SessionEsta ByteSpan GetCSRNonce() const { return ByteSpan(mCSRNonce, sizeof(mCSRNonce)); } + CHIP_ERROR WarmupCASESession(); + private: enum class ConnectionState { @@ -419,22 +420,21 @@ class DLL_EXPORT Device : public Messaging::ExchangeDelegate, public SessionEsta */ CHIP_ERROR LoadSecureSessionParametersIfNeeded(bool & didLoad); - CHIP_ERROR EstablishCASESession(); - uint16_t mListenPort; Transport::AdminId mAdminId = Transport::kUndefinedAdminId; - bool mDeviceProvisioningComplete = false; + bool mDeviceOperationalCertProvisioned = false; CASESession mCASESession; - uint16_t mCASESessionKeyId = 0; Credentials::OperationalCredentialSet * mCredentials = nullptr; PersistentStorageDelegate * mStorageDelegate = nullptr; uint8_t mCSRNonce[kOpCSRNonceLength]; + + SessionIDAllocator * mIDAllocator = nullptr; }; /** @@ -483,11 +483,10 @@ typedef struct SerializableDevice PASESessionSerializable mOpsCreds; uint64_t mDeviceId; /* This field is serialized in LittleEndian byte order */ uint8_t mDeviceAddr[INET6_ADDRSTRLEN]; - uint16_t mDevicePort; /* This field is serialized in LittleEndian byte order */ - uint16_t mAdminId; /* This field is serialized in LittleEndian byte order */ - uint16_t mCASESessionKeyId; /* This field is serialized in LittleEndian byte order */ + uint16_t mDevicePort; /* This field is serialized in LittleEndian byte order */ + uint16_t mAdminId; /* This field is serialized in LittleEndian byte order */ uint8_t mDeviceTransport; - uint8_t mDeviceProvisioningComplete; + uint8_t mDeviceOperationalCertProvisioned; uint8_t mInterfaceName[kMaxInterfaceName]; uint32_t mLocalMessageCounter; /* This field is serialized in LittleEndian byte order */ uint32_t mPeerMessageCounter; /* This field is serialized in LittleEndian byte order */ diff --git a/src/controller/CHIPDeviceController.cpp b/src/controller/CHIPDeviceController.cpp index 64b4a7fb214663..68924dbb16497a 100644 --- a/src/controller/CHIPDeviceController.cpp +++ b/src/controller/CHIPDeviceController.cpp @@ -211,6 +211,42 @@ CHIP_ERROR DeviceController::Init(NodeId localDeviceId, ControllerInitParams par return CHIP_NO_ERROR; } +CHIP_ERROR DeviceController::GenerateOperationalCertificates(const ByteSpan & CSR, NodeId deviceId, uint8_t * certBuf, + uint32_t certBufSize, uint32_t & outCertLen) +{ + chip::Platform::ScopedMemoryBuffer noc; + ReturnErrorCodeIf(!noc.Alloc(kMaxCHIPOpCertLength), CHIP_ERROR_NO_MEMORY); + + uint32_t nocLen = 0; + ChipLogProgress(Controller, "Generating operational certificate for device " ChipLogFormatX64, ChipLogValueX64(deviceId)); + ReturnErrorOnFailure(mOperationalCredentialsDelegate->GenerateNodeOperationalCertificate( + PeerId().SetNodeId(deviceId), CSR, 1, noc.Get(), kMaxCHIPOpCertLength, nocLen)); + + ReturnErrorCodeIf(nocLen == 0, CHIP_ERROR_CERT_NOT_FOUND); + + chip::Platform::ScopedMemoryBuffer ica; + ReturnErrorCodeIf(!ica.Alloc(kMaxCHIPOpCertLength), CHIP_ERROR_NO_MEMORY); + + uint32_t icaLen = 0; + ChipLogProgress(Controller, "Getting intermediate CA certificate from the issuer"); + CHIP_ERROR err = mOperationalCredentialsDelegate->GetIntermediateCACertificate(0, ica.Get(), kMaxCHIPOpCertLength, icaLen); + ChipLogProgress(Controller, "GetIntermediateCACertificate returned %" PRId32, err); + if (err == CHIP_ERROR_INTERMEDIATE_CA_NOT_REQUIRED) + { + // This implies that the commissioner application uses root CA to sign the operational + // certificates, and an intermediate CA is not needed. It's not an error condition, so + // let's just send operational certificate and root CA certificate to the device. + err = CHIP_NO_ERROR; + icaLen = 0; + ChipLogProgress(Controller, "Intermediate CA is not needed"); + } + ReturnErrorOnFailure(err); + ReturnErrorOnFailure(ConvertX509CertsToChipCertArray(ByteSpan(noc.Get(), nocLen), ByteSpan(ica.Get(), icaLen), certBuf, + certBufSize, outCertLen)); + + return CHIP_NO_ERROR; +} + CHIP_ERROR DeviceController::LoadLocalCredentials(Transport::AdminPairingInfo * admin) { ChipLogProgress(Controller, "Getting operational keys"); @@ -220,38 +256,42 @@ CHIP_ERROR DeviceController::LoadLocalCredentials(Transport::AdminPairingInfo * if (!admin->AreCredentialsAvailable()) { - chip::Platform::ScopedMemoryBuffer buffer1; - ReturnErrorCodeIf(!buffer1.Alloc(kMaxCHIPCSRLength), CHIP_ERROR_NO_MEMORY); - - chip::Platform::ScopedMemoryBuffer buffer2; - ReturnErrorCodeIf(!buffer2.Alloc(kMaxCHIPOpCertLength), CHIP_ERROR_NO_MEMORY); - - uint8_t * CSR = buffer1.Get(); - size_t csrLength = kMaxCHIPCSRLength; - ReturnErrorOnFailure(keypair->NewCertificateSigningRequest(CSR, csrLength)); - - uint8_t * cert = buffer2.Get(); - uint32_t certLen = 0; - - // TODO - Match the generated cert against CSR and operational keypair - // Make sure it chains back to the trusted root. - ChipLogProgress(Controller, "Generating operational certificate for the controller"); - ReturnErrorOnFailure(mOperationalCredentialsDelegate->GenerateNodeOperationalCertificate( - PeerId().SetNodeId(mLocalDeviceId), ByteSpan(CSR, csrLength), 1, cert, kMaxCHIPOpCertLength, certLen)); - - uint8_t * chipCert = buffer1.Get(); + chip::Platform::ScopedMemoryBuffer chipCert; + uint32_t chipCertAllocatedLen = kMaxCHIPOpCertLength * 2; + ReturnErrorCodeIf(!chipCert.Alloc(chipCertAllocatedLen), CHIP_ERROR_NO_MEMORY); uint32_t chipCertLen = 0; - ReturnErrorOnFailure(ConvertX509CertToChipCert(cert, certLen, chipCert, kMaxCHIPOpCertLength, chipCertLen)); - ReturnErrorOnFailure(admin->SetOperationalCert(ByteSpan(chipCert, chipCertLen))); + // Get root CA certificate + { + ChipLogProgress(Controller, "Getting root certificate for the controller from the issuer"); + chip::Platform::ScopedMemoryBuffer rootCert; + ReturnErrorCodeIf(!rootCert.Alloc(kMaxCHIPOpCertLength), CHIP_ERROR_NO_MEMORY); + uint32_t rootCertLen = 0; - ChipLogProgress(Controller, "Getting root certificate for the controller from the issuer"); - ReturnErrorOnFailure(mOperationalCredentialsDelegate->GetRootCACertificate(0, cert, kMaxCHIPOpCertLength, certLen)); + ReturnErrorOnFailure( + mOperationalCredentialsDelegate->GetRootCACertificate(0, rootCert.Get(), kMaxCHIPOpCertLength, rootCertLen)); + ReturnErrorOnFailure( + ConvertX509CertToChipCert(rootCert.Get(), rootCertLen, chipCert.Get(), chipCertAllocatedLen, chipCertLen)); - chipCertLen = 0; - ReturnErrorOnFailure(ConvertX509CertToChipCert(cert, certLen, chipCert, kMaxCHIPOpCertLength, chipCertLen)); + ReturnErrorOnFailure(admin->SetRootCert(ByteSpan(chipCert.Get(), chipCertLen))); + } - ReturnErrorOnFailure(admin->SetRootCert(ByteSpan(chipCert, chipCertLen))); + // Generate Operational Certificates (NOC and ICAC) + { + chip::Platform::ScopedMemoryBuffer CSR; + size_t csrLength = kMaxCHIPCSRLength; + ReturnErrorCodeIf(!CSR.Alloc(csrLength), CHIP_ERROR_NO_MEMORY); + + ReturnErrorOnFailure(keypair->NewCertificateSigningRequest(CSR.Get(), csrLength)); + + // TODO - Match the generated cert against CSR and operational keypair + // Make sure it chains back to the trusted root. + ChipLogProgress(Controller, "Generating operational certificate for the controller"); + chipCertLen = 0; + ReturnErrorOnFailure(GenerateOperationalCertificates(ByteSpan(CSR.Get(), csrLength), mLocalDeviceId, chipCert.Get(), + chipCertAllocatedLen, chipCertLen)); + ReturnErrorOnFailure(admin->SetOperationalCert(ByteSpan(chipCert.Get(), chipCertLen))); + } ReturnErrorOnFailure(mAdmins.Store(admin->GetAdminId())); } @@ -431,6 +471,12 @@ CHIP_ERROR DeviceController::GetDevice(NodeId deviceId, Device ** out_device) VerifyOrExit(err == CHIP_NO_ERROR, ReleaseDevice(device)); device->Init(GetControllerDeviceInitParams(), mListenPort, mAdminId); + + if (device->IsOperationalCertProvisioned()) + { + err = device->WarmupCASESession(); + SuccessOrExit(err); + } } } @@ -446,10 +492,6 @@ CHIP_ERROR DeviceController::GetDevice(NodeId deviceId, Device ** out_device) CHIP_ERROR DeviceController::UpdateDevice(Device * device, uint64_t fabricId) { - // TODO - Detect when the device is fully provisioned, instead of relying on UpdateDevice() - device->ProvisioningComplete(mNextKeyId++); - PersistDevice(device); - PersistNextKeyId(); #if CHIP_DEVICE_CONFIG_ENABLE_MDNS return Mdns::Resolver::Instance().ResolveNodeId(chip::PeerId().SetNodeId(device->GetDeviceId()).SetFabricId(fabricId), chip::Inet::kIPAddressType_Any); @@ -690,7 +732,9 @@ void DeviceController::PersistNextKeyId() { if (mStorageDelegate != nullptr && mState == State::Initialized) { - mStorageDelegate->SyncSetKeyValue(kNextAvailableKeyID, &mNextKeyId, sizeof(mNextKeyId)); + uint16_t nextKeyID = 0; + mIDAllocator.Peek(nextKeyID); + mStorageDelegate->SyncSetKeyValue(kNextAvailableKeyID, &nextKeyID, sizeof(nextKeyID)); } } @@ -738,6 +782,7 @@ ControllerDeviceInitParams DeviceController::GetControllerDeviceInitParams() .inetLayer = mInetLayer, .storageDelegate = mStorageDelegate, .credentials = &mCredentials, + .idAllocator = &mIDAllocator, }; } @@ -757,11 +802,16 @@ CHIP_ERROR DeviceCommissioner::Init(NodeId localDeviceId, CommissionerInitParams { ReturnErrorOnFailure(DeviceController::Init(localDeviceId, params)); - uint16_t size = sizeof(mNextKeyId); - CHIP_ERROR error = mStorageDelegate->SyncGetKeyValue(kNextAvailableKeyID, &mNextKeyId, size); - if (error || (size != sizeof(mNextKeyId))) + uint16_t nextKeyID = 0; + uint16_t size = sizeof(nextKeyID); + CHIP_ERROR error = mStorageDelegate->SyncGetKeyValue(kNextAvailableKeyID, &nextKeyID, size); + if (error || (size != sizeof(nextKeyID))) { - mNextKeyId = 0; + nextKeyID = 0; + } + for (uint16_t i = 0; i < nextKeyID; i++) + { + mIDAllocator.Reserve(i); } mPairingDelegate = params.pairingDelegate; @@ -790,6 +840,8 @@ CHIP_ERROR DeviceCommissioner::PairDevice(NodeId remoteDeviceId, RendezvousParam Messaging::ExchangeContext * exchangeCtxt = nullptr; + uint16_t keyID = 0; + Transport::AdminPairingInfo * admin = mAdmins.FindAdminWithId(mAdminId); VerifyOrExit(remoteDeviceId != kAnyNodeId && remoteDeviceId != kUndefinedNodeId, err = CHIP_ERROR_INVALID_ARGUMENT); @@ -858,7 +910,10 @@ CHIP_ERROR DeviceCommissioner::PairDevice(NodeId remoteDeviceId, RendezvousParam exchangeCtxt = mExchangeMgr->NewContext(SecureSessionHandle(), &mPairingSession); VerifyOrExit(exchangeCtxt != nullptr, err = CHIP_ERROR_INTERNAL); - err = mPairingSession.Pair(params.GetPeerAddress(), params.GetSetupPINCode(), mNextKeyId++, exchangeCtxt, this); + err = mIDAllocator.Allocate(keyID); + SuccessOrExit(err); + + err = mPairingSession.Pair(params.GetPeerAddress(), params.GetSetupPINCode(), keyID, exchangeCtxt, this); // Immediately persist the updted mNextKeyID value // TODO maybe remove FreeRendezvousSession() since mNextKeyID is always persisted immediately PersistNextKeyId(); @@ -996,6 +1051,20 @@ CHIP_ERROR DeviceCommissioner::UnpairDevice(NodeId remoteDeviceId) return CHIP_NO_ERROR; } +CHIP_ERROR DeviceCommissioner::OperationalDiscoveryComplete(NodeId remoteDeviceId) +{ + ChipLogProgress(Controller, "OperationalDiscoveryComplete for device ID %" PRIu64, remoteDeviceId); + VerifyOrReturnError(mState == State::Initialized, CHIP_ERROR_INCORRECT_STATE); + + Device * device = nullptr; + ReturnErrorOnFailure(GetDevice(remoteDeviceId, &device)); + device->OperationalCertProvisioned(); + PersistDevice(device); + PersistNextKeyId(); + + return CHIP_NO_ERROR; +} + void DeviceCommissioner::FreeRendezvousSession() { PersistNextKeyId(); @@ -1058,7 +1127,7 @@ void DeviceCommissioner::OnSessionEstablished() return; } - ChipLogDetail(Controller, "Remote device completed SPAKE2+ handshake\n"); + ChipLogDetail(Controller, "Remote device completed SPAKE2+ handshake"); // TODO: Add code to receive OpCSR from the device, and process the signing request // For IP rendezvous, this is sent as part of the state machine. @@ -1146,40 +1215,12 @@ CHIP_ERROR DeviceCommissioner::ProcessOpCSR(const ByteSpan & CSR, const ByteSpan VerifyOrReturnError(CSRNonce.size() == nonce.size(), CHIP_ERROR_INVALID_ARGUMENT); VerifyOrReturnError(memcmp(CSRNonce.data(), nonce.data(), CSRNonce.size()) == 0, CHIP_ERROR_INVALID_ARGUMENT); - chip::Platform::ScopedMemoryBuffer noc; - ReturnErrorCodeIf(!noc.Alloc(kMaxCHIPOpCertLength), CHIP_ERROR_NO_MEMORY); - - uint32_t nocLen = 0; - ChipLogProgress(Controller, "Generating operational certificate for device " ChipLogFormatX64, - ChipLogValueX64(device->GetDeviceId())); - ReturnErrorOnFailure(mOperationalCredentialsDelegate->GenerateNodeOperationalCertificate( - PeerId().SetNodeId(device->GetDeviceId()), CSR, 1, noc.Get(), kMaxCHIPOpCertLength, nocLen)); - - ReturnErrorCodeIf(nocLen == 0, CHIP_ERROR_CERT_NOT_FOUND); - - chip::Platform::ScopedMemoryBuffer ica; - ReturnErrorCodeIf(!ica.Alloc(kMaxCHIPOpCertLength), CHIP_ERROR_NO_MEMORY); - - uint32_t icaLen = 0; - ChipLogProgress(Controller, "Getting intermediate CA certificate from the issuer"); - CHIP_ERROR err = mOperationalCredentialsDelegate->GetIntermediateCACertificate(0, ica.Get(), kMaxCHIPOpCertLength, icaLen); - ChipLogProgress(Controller, "GetIntermediateCACertificate returned %" PRId32, err); - if (err == CHIP_ERROR_INTERMEDIATE_CA_NOT_REQUIRED) - { - // This implies that the commissioner application uses root CA to sign the operational - // certificates, and an intermediate CA is not needed. It's not an error condition, so - // let's just send operational certificate and root CA certificate to the device. - err = CHIP_NO_ERROR; - icaLen = 0; - ChipLogProgress(Controller, "Intermediate CA is not needed"); - } - ReturnErrorOnFailure(err); - chip::Platform::ScopedMemoryBuffer chipOpCert; ReturnErrorCodeIf(!chipOpCert.Alloc(kMaxCHIPOpCertLength * 2), CHIP_ERROR_NO_MEMORY); uint32_t chipOpCertLen = 0; - ReturnErrorOnFailure(ConvertX509CertsToChipCertArray(ByteSpan(noc.Get(), nocLen), ByteSpan(ica.Get(), icaLen), chipOpCert.Get(), - kMaxCHIPOpCertLength * 2, chipOpCertLen)); + + ReturnErrorOnFailure( + GenerateOperationalCertificates(CSR, device->GetDeviceId(), chipOpCert.Get(), kMaxCHIPOpCertLength * 2, chipOpCertLen)); ChipLogProgress(Controller, "Sending operational certificate to the device. Op Cert Len %" PRIu32, chipOpCertLen); ReturnErrorOnFailure(SendOperationalCertificate(device, ByteSpan(chipOpCert.Get(), chipOpCertLen))); @@ -1489,6 +1530,7 @@ void DeviceCommissioner::OnNodeIdResolved(const chip::Mdns::ResolvedNodeData & n } } DeviceController::OnNodeIdResolved(nodeData); + OperationalDiscoveryComplete(nodeData.mPeerId.GetNodeId()); } void DeviceCommissioner::OnNodeIdResolutionFailed(const chip::PeerId & peer, CHIP_ERROR error) diff --git a/src/controller/CHIPDeviceController.h b/src/controller/CHIPDeviceController.h index c57caa7defa3b1..725a6b871c3af7 100644 --- a/src/controller/CHIPDeviceController.h +++ b/src/controller/CHIPDeviceController.h @@ -334,7 +334,7 @@ class DLL_EXPORT DeviceController : public Messaging::ExchangeDelegate, Credentials::OperationalCredentialSet mCredentials; Credentials::CertificateKeyId mRootKeyId; - uint16_t mNextKeyId = 0; + SessionIDAllocator mIDAllocator; #if CHIP_DEVICE_CONFIG_ENABLE_MDNS //////////// ResolverDelegate Implementation /////////////// @@ -343,6 +343,9 @@ class DLL_EXPORT DeviceController : public Messaging::ExchangeDelegate, Mdns::DiscoveredNodeData * GetDiscoveredNodes() override { return mCommissionableNodes; } #endif // CHIP_DEVICE_CONFIG_ENABLE_MDNS + CHIP_ERROR GenerateOperationalCertificates(const ByteSpan & CSR, NodeId deviceId, uint8_t * certBuf, uint32_t certBufSize, + uint32_t & outCertLen); + private: //////////// ExchangeDelegate Implementation /////////////// void OnMessageReceived(Messaging::ExchangeContext * ec, const PacketHeader & packetHeader, const PayloadHeader & payloadHeader, @@ -445,6 +448,17 @@ class DLL_EXPORT DeviceCommissioner : public DeviceController, public SessionEst */ CHIP_ERROR UnpairDevice(NodeId remoteDeviceId); + /** + * @brief + * This function is called by the commissioner application when a device (being paired) is + * discovered on the operational network. + * + * @param[in] remoteDeviceId The remote device Id. + * + * @return CHIP_ERROR CHIP_NO_ERROR on success, or corresponding error + */ + CHIP_ERROR OperationalDiscoveryComplete(NodeId remoteDeviceId); + //////////// SessionEstablishmentDelegate Implementation /////////////// void OnSessionEstablishmentError(CHIP_ERROR error) override; void OnSessionEstablished() override; diff --git a/src/controller/ExampleOperationalCredentialsIssuer.cpp b/src/controller/ExampleOperationalCredentialsIssuer.cpp index 84bf3b6322ffc5..c007e1ad55f151 100644 --- a/src/controller/ExampleOperationalCredentialsIssuer.cpp +++ b/src/controller/ExampleOperationalCredentialsIssuer.cpp @@ -18,6 +18,7 @@ #include #include +#include namespace chip { namespace Controller { @@ -29,6 +30,16 @@ using namespace Crypto; CHIP_ERROR ExampleOperationalCredentialsIssuer::Initialize(PersistentStorageDelegate & storage) { + using namespace ASN1; + ASN1UniversalTime effectiveTime; + + // Initializing the default start validity to start of 2021. The default validity duration is 10 years. + CHIP_ZERO_AT(effectiveTime); + effectiveTime.Year = 2021; + effectiveTime.Month = 1; + effectiveTime.Day = 1; + ReturnErrorOnFailure(ASN1ToChipEpochTime(effectiveTime, mNow)); + Crypto::P256SerializedKeypair serializedKey; uint16_t keySize = static_cast(sizeof(serializedKey)); diff --git a/src/protocols/secure_channel/BUILD.gn b/src/protocols/secure_channel/BUILD.gn index e499545c98294e..525406a1c1acdc 100644 --- a/src/protocols/secure_channel/BUILD.gn +++ b/src/protocols/secure_channel/BUILD.gn @@ -14,6 +14,8 @@ static_library("secure_channel") { "SessionEstablishmentDelegate.h", "SessionEstablishmentExchangeDispatch.cpp", "SessionEstablishmentExchangeDispatch.h", + "SessionIDAllocator.cpp", + "SessionIDAllocator.h", "StatusReport.cpp", "StatusReport.h", ] diff --git a/src/protocols/secure_channel/CASEServer.cpp b/src/protocols/secure_channel/CASEServer.cpp index 25adca9cbf1794..00c3a01a7960db 100644 --- a/src/protocols/secure_channel/CASEServer.cpp +++ b/src/protocols/secure_channel/CASEServer.cpp @@ -30,7 +30,8 @@ using namespace ::chip::Credentials; namespace chip { CHIP_ERROR CASEServer::ListenForSessionEstablishment(Messaging::ExchangeManager * exchangeManager, TransportMgrBase * transportMgr, - SecureSessionMgr * sessionMgr, Transport::AdminPairingTable * admins) + SecureSessionMgr * sessionMgr, Transport::AdminPairingTable * admins, + SessionIDAllocator * idAllocator) { VerifyOrReturnError(transportMgr != nullptr, CHIP_ERROR_INVALID_ARGUMENT); VerifyOrReturnError(exchangeManager != nullptr, CHIP_ERROR_INVALID_ARGUMENT); @@ -40,6 +41,7 @@ CHIP_ERROR CASEServer::ListenForSessionEstablishment(Messaging::ExchangeManager mSessionMgr = sessionMgr; mAdmins = admins; mExchangeManager = exchangeManager; + mIDAllocator = idAllocator; ReturnErrorOnFailure(mPairingSession.MessageDispatch().Init(transportMgr)); @@ -67,8 +69,10 @@ CHIP_ERROR CASEServer::InitCASEHandshake(Messaging::ExchangeContext * ec) ReturnErrorOnFailure(admin->GetCredentials(mCredentials, mCertificates, mRootKeyId)); + ReturnErrorOnFailure(mIDAllocator->Allocate(mSessionKeyId)); + // Setup CASE state machine using the credentials for the current admin. - ReturnErrorOnFailure(mPairingSession.ListenForSessionEstablishment(&mCredentials, mNextKeyId++, this)); + ReturnErrorOnFailure(mPairingSession.ListenForSessionEstablishment(&mCredentials, mSessionKeyId, this)); // Hand over the exchange context to the CASE session. ec->SetDelegate(&mPairingSession); @@ -101,23 +105,23 @@ void CASEServer::Cleanup() void CASEServer::OnSessionEstablishmentError(CHIP_ERROR err) { ChipLogProgress(Inet, "CASE Session establishment failed: %s", ErrorStr(err)); + mIDAllocator->Free(mSessionKeyId); Cleanup(); } void CASEServer::OnSessionEstablished() { ChipLogProgress(Inet, "CASE Session established. Setting up the secure channel."); - // TODO - enable use of secure session established via CASE - // CHIP_ERROR err = - // mSessionMgr->NewPairing(Optional::Value(mPairingSession.PeerConnection().GetPeerAddress()), - // mPairingSession.PeerConnection().GetPeerNodeId(), &mPairingSession, - // SecureSession::SessionRole::kResponder, mAdminId, nullptr); - // if (err != CHIP_NO_ERROR) - // { - // ChipLogError(Inet, "Failed in setting up secure channel: err %s", ErrorStr(err)); - // OnSessionEstablishmentError(err); - // return; - // } + CHIP_ERROR err = + mSessionMgr->NewPairing(Optional::Value(mPairingSession.PeerConnection().GetPeerAddress()), + mPairingSession.PeerConnection().GetPeerNodeId(), &mPairingSession, + SecureSession::SessionRole::kResponder, mAdminId, nullptr); + if (err != CHIP_NO_ERROR) + { + ChipLogError(Inet, "Failed in setting up secure channel: err %s", ErrorStr(err)); + OnSessionEstablishmentError(err); + return; + } ChipLogProgress(Inet, "CASE secure channel is available now."); Cleanup(); diff --git a/src/protocols/secure_channel/CASEServer.h b/src/protocols/secure_channel/CASEServer.h index 5a976b79558525..54c1f5db2f7b61 100644 --- a/src/protocols/secure_channel/CASEServer.h +++ b/src/protocols/secure_channel/CASEServer.h @@ -20,6 +20,7 @@ #include #include #include +#include namespace chip { @@ -38,7 +39,8 @@ class CASEServer : public SessionEstablishmentDelegate, public Messaging::Exchan } CHIP_ERROR ListenForSessionEstablishment(Messaging::ExchangeManager * exchangeManager, TransportMgrBase * transportMgr, - SecureSessionMgr * sessionMgr, Transport::AdminPairingTable * admins); + SecureSessionMgr * sessionMgr, Transport::AdminPairingTable * admins, + SessionIDAllocator * idAllocator); //////////// SessionEstablishmentDelegate Implementation /////////////// void OnSessionEstablishmentError(CHIP_ERROR error) override; @@ -58,7 +60,7 @@ class CASEServer : public SessionEstablishmentDelegate, public Messaging::Exchan Messaging::ExchangeManager * mExchangeManager = nullptr; CASESession mPairingSession; - uint16_t mNextKeyId = 0; + uint16_t mSessionKeyId = 0; SecureSessionMgr * mSessionMgr = nullptr; Transport::AdminId mAdminId = Transport::kUndefinedAdminId; @@ -70,6 +72,8 @@ class CASEServer : public SessionEstablishmentDelegate, public Messaging::Exchan CHIP_ERROR InitCASEHandshake(Messaging::ExchangeContext * ec); + SessionIDAllocator * mIDAllocator = nullptr; + void Cleanup(); }; diff --git a/src/protocols/secure_channel/CASESession.cpp b/src/protocols/secure_channel/CASESession.cpp index 99c38f19418dde..ccb478b7a58aa1 100644 --- a/src/protocols/secure_channel/CASESession.cpp +++ b/src/protocols/secure_channel/CASESession.cpp @@ -55,7 +55,8 @@ constexpr uint8_t kIVSR2[] = { 0x4e, 0x43, 0x41, 0x53, 0x45, 0x5f, 0x53, 0x69, 0 constexpr uint8_t kIVSR3[] = { 0x4e, 0x43, 0x41, 0x53, 0x45, 0x5f, 0x53, 0x69, 0x67, 0x6d, 0x61, 0x52, 0x33 }; constexpr size_t kIVLength = sizeof(kIVSR2); -constexpr size_t kTAGSize = 16; +constexpr size_t kTAGSize = 16; +constexpr uint32_t kMaxCHIPOpCertLength = 1024; using namespace Crypto; using namespace Credentials; @@ -1021,30 +1022,34 @@ CHIP_ERROR CASESession::ConstructSaltSigmaR3(const uint8_t * ipk, size_t ipkLen, CHIP_ERROR CASESession::Validate_and_RetrieveResponderID(const uint8_t ** msgIterator, P256PublicKey & responderID, const uint8_t ** responderOpCert, uint16_t & responderOpCertLen) { - ChipCertificateData chipCertData; ChipCertificateData * resultCert = nullptr; + ChipCertificateSet certSet; + // Certificate set can contain up to 3 certs (NOC, ICA cert, and Root CA cert) + ReturnErrorOnFailure(certSet.Init(3, kMaxCHIPOpCertLength * 3)); + responderOpCertLen = chip::Encoding::LittleEndian::Read16(*msgIterator); *responderOpCert = *msgIterator; *msgIterator += responderOpCertLen; Encoding::LittleEndian::BufferWriter bbuf(responderID, responderID.Length()); - ReturnErrorOnFailure(DecodeChipCert(*responderOpCert, responderOpCertLen, chipCertData)); + ReturnErrorOnFailure( + certSet.LoadCerts(*responderOpCert, responderOpCertLen, BitFlags(CertDecodeFlags::kGenerateTBSHash))); - bbuf.Put(chipCertData.mPublicKey, chipCertData.mPublicKeyLen); + bbuf.Put(certSet.GetCertSet()[0].mPublicKey, certSet.GetCertSet()[0].mPublicKeyLen); VerifyOrReturnError(bbuf.Fit(), CHIP_ERROR_NO_MEMORY); // Validate responder identity located in msg_r2_encrypted ReturnErrorOnFailure( mOpCredSet->FindCertSet(mTrustedRootId) - ->LoadCert(*responderOpCert, responderOpCertLen, BitFlags(CertDecodeFlags::kGenerateTBSHash))); + ->LoadCerts(*responderOpCert, responderOpCertLen, BitFlags(CertDecodeFlags::kGenerateTBSHash))); ReturnErrorOnFailure(SetEffectiveTime()); // Locate the subject DN and key id that will be used as input the FindValidCert() method. { - const ChipDN & subjectDN = chipCertData.mSubjectDN; - const CertificateKeyId & subjectKeyId = chipCertData.mSubjectKeyId; + const ChipDN & subjectDN = certSet.GetCertSet()[0].mSubjectDN; + const CertificateKeyId & subjectKeyId = certSet.GetCertSet()[0].mSubjectKeyId; ReturnErrorOnFailure(mOpCredSet->FindValidCert(mTrustedRootId, subjectDN, subjectKeyId, mValidContext, resultCert)); } diff --git a/src/protocols/secure_channel/SessionIDAllocator.cpp b/src/protocols/secure_channel/SessionIDAllocator.cpp new file mode 100644 index 00000000000000..d3d5a10969baa4 --- /dev/null +++ b/src/protocols/secure_channel/SessionIDAllocator.cpp @@ -0,0 +1,64 @@ +/* + * + * Copyright (c) 2021 Project CHIP Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include + +#include + +namespace chip { + +CHIP_ERROR SessionIDAllocator::Allocate(uint16_t & id) +{ + VerifyOrReturnError(mNextAvailable < kMaxSessionID, CHIP_ERROR_NO_MEMORY); + id = mNextAvailable; + + // TODO - Update SessionID allocator to use freed session IDs + mNextAvailable++; + + return CHIP_NO_ERROR; +} + +CHIP_ERROR SessionIDAllocator::Free(uint16_t id) +{ + if (mNextAvailable == id && mNextAvailable > 0) + { + mNextAvailable--; + } + return CHIP_NO_ERROR; +} + +CHIP_ERROR SessionIDAllocator::Reserve(uint16_t id) +{ + VerifyOrReturnError(id < kMaxSessionID, CHIP_ERROR_NO_MEMORY); + if (id >= mNextAvailable) + { + mNextAvailable = id; + mNextAvailable++; + } + + return CHIP_NO_ERROR; +} + +CHIP_ERROR SessionIDAllocator::Peek(uint16_t & id) +{ + VerifyOrReturnError(mNextAvailable < kMaxSessionID, CHIP_ERROR_NO_MEMORY); + id = mNextAvailable; + + return CHIP_NO_ERROR; +} + +} // namespace chip diff --git a/src/protocols/secure_channel/SessionIDAllocator.h b/src/protocols/secure_channel/SessionIDAllocator.h new file mode 100644 index 00000000000000..9515282362920a --- /dev/null +++ b/src/protocols/secure_channel/SessionIDAllocator.h @@ -0,0 +1,41 @@ +/* + * + * Copyright (c) 2021 Project CHIP Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#pragma once + +#include + +namespace chip { + +class SessionIDAllocator +{ +public: + SessionIDAllocator() {} + ~SessionIDAllocator() {} + + CHIP_ERROR Allocate(uint16_t & id); + CHIP_ERROR Free(uint16_t id); + CHIP_ERROR Reserve(uint16_t id); + CHIP_ERROR Peek(uint16_t & id); + +private: + // Session ID is a 15 bit value (16th bit indicates unicast/group key) + static constexpr uint16_t kMaxSessionID = (1 << 15) - 1; + uint16_t mNextAvailable = 0; +}; + +} // namespace chip diff --git a/src/transport/SecureSessionMgr.cpp b/src/transport/SecureSessionMgr.cpp index 162efd3cd5974c..be2ded77a25811 100644 --- a/src/transport/SecureSessionMgr.cpp +++ b/src/transport/SecureSessionMgr.cpp @@ -214,6 +214,16 @@ CHIP_ERROR SecureSessionMgr::SendPreparedMessage(SecureSessionHandle session, co return err; } +void SecureSessionMgr::ExpirePairing(SecureSessionHandle session) +{ + PeerConnectionState * state = GetPeerConnectionState(session); + if (state != nullptr) + { + mPeerConnections.MarkConnectionExpired( + state, [this](const Transport::PeerConnectionState & state1) { HandleConnectionExpired(state1); }); + } +} + CHIP_ERROR SecureSessionMgr::NewPairing(const Optional & peerAddr, NodeId peerNodeId, PairingSession * pairing, SecureSession::SessionRole direction, Transport::AdminId admin, Transport::Base * transport) diff --git a/src/transport/SecureSessionMgr.h b/src/transport/SecureSessionMgr.h index acdb01c1c7e2fa..cf065b8c66b0e5 100644 --- a/src/transport/SecureSessionMgr.h +++ b/src/transport/SecureSessionMgr.h @@ -220,6 +220,8 @@ class DLL_EXPORT SecureSessionMgr : public TransportMgrDelegate CHIP_ERROR NewPairing(const Optional & peerAddr, NodeId peerNodeId, PairingSession * pairing, SecureSession::SessionRole direction, Transport::AdminId admin, Transport::Base * transport = nullptr); + void ExpirePairing(SecureSessionHandle session); + /** * @brief * Return the System Layer pointer used by current SecureSessionMgr. From 31ad603ded7d027384f31ab504fa1c7276e2566b Mon Sep 17 00:00:00 2001 From: Pankaj Garg Date: Tue, 15 Jun 2021 18:01:10 -0700 Subject: [PATCH 02/12] persist counter if device is connected --- src/controller/CHIPDevice.cpp | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/controller/CHIPDevice.cpp b/src/controller/CHIPDevice.cpp index c95776d0b49ad8..4eadad4bdf265e 100644 --- a/src/controller/CHIPDevice.cpp +++ b/src/controller/CHIPDevice.cpp @@ -164,9 +164,14 @@ CHIP_ERROR Device::Serialize(SerializedDevice & output) serializable.mAdminId = Encoding::LittleEndian::HostSwap16(mAdminId); Transport::PeerConnectionState * connectionState = mSessionManager->GetPeerConnectionState(mSecureSession); - VerifyOrReturnError(connectionState != nullptr, CHIP_ERROR_INCORRECT_STATE); - const uint32_t localMessageCounter = connectionState->GetSessionMessageCounter().GetLocalMessageCounter().Value(); - const uint32_t peerMessageCounter = connectionState->GetSessionMessageCounter().GetPeerMessageCounter().GetCounter(); + + uint32_t localMessageCounter = 0; + uint32_t peerMessageCounter = 0; + if (connectionState != nullptr) + { + localMessageCounter = connectionState->GetSessionMessageCounter().GetLocalMessageCounter().Value(); + peerMessageCounter = connectionState->GetSessionMessageCounter().GetPeerMessageCounter().GetCounter(); + } serializable.mLocalMessageCounter = Encoding::LittleEndian::HostSwap32(localMessageCounter); serializable.mPeerMessageCounter = Encoding::LittleEndian::HostSwap32(peerMessageCounter); From 7e4b7925674c8d10e9a15ec1fe5bfb1258ed3f78 Mon Sep 17 00:00:00 2001 From: Pankaj Garg Date: Wed, 16 Jun 2021 08:56:44 -0700 Subject: [PATCH 03/12] enable CASE sessions for chip-tool --- .../commands/clusters/ModelCommand.cpp | 26 +++++++++++++++++++ src/controller/CHIPDevice.cpp | 2 ++ src/controller/CHIPDevice.h | 2 ++ src/protocols/secure_channel/CASEServer.cpp | 2 ++ src/transport/SecureSessionMgr.cpp | 18 +++++++++++++ src/transport/SecureSessionMgr.h | 1 + 6 files changed, 51 insertions(+) diff --git a/examples/chip-tool/commands/clusters/ModelCommand.cpp b/examples/chip-tool/commands/clusters/ModelCommand.cpp index 632304580563aa..0655b79192ef51 100644 --- a/examples/chip-tool/commands/clusters/ModelCommand.cpp +++ b/examples/chip-tool/commands/clusters/ModelCommand.cpp @@ -37,6 +37,25 @@ void DispatchSingleClusterCommand(chip::ClusterId aClusterId, chip::CommandId aC "Default DispatchSingleClusterCommand is called, this should be replaced by actual dispatched for cluster commands"); } +CHIP_ERROR WaitForSessionSetup(chip::Controller::Device * device) +{ + constexpr time_t kWaitPerIteration = 1; + constexpr uint16_t kIterationCount = 5; + + struct timespec sleep_time; + sleep_time.tv_sec = kWaitPerIteration; + sleep_time.tv_nsec = 0; + + for (uint32_t i = 0; i < kIterationCount && device->IsSessionSetupInProgress(); i++) + { + nanosleep(&sleep_time, nullptr); + } + + ReturnErrorCodeIf(!device->IsSecureConnected(), CHIP_ERROR_TIMEOUT); + + return CHIP_NO_ERROR; +} + CHIP_ERROR ModelCommand::Run(NodeId localId, NodeId remoteId) { CHIP_ERROR err = CHIP_NO_ERROR; @@ -54,6 +73,13 @@ CHIP_ERROR ModelCommand::Run(NodeId localId, NodeId remoteId) err = GetExecContext()->commissioner->GetDevice(remoteId, &mDevice); VerifyOrExit(err == CHIP_NO_ERROR, ChipLogError(chipTool, "Init failure! No pairing for device: %" PRIu64, localId)); + if (mDevice->IsSessionSetupInProgress()) + { + err = WaitForSessionSetup(mDevice); + VerifyOrExit(err == CHIP_NO_ERROR, + ChipLogError(chipTool, "Timed out while waiting for session setup for device: %" PRIu64, localId)); + } + err = SendCommand(mDevice, mEndPointId); VerifyOrExit(err == CHIP_NO_ERROR, ChipLogError(chipTool, "Failed to send message: %s", ErrorStr(err))); } diff --git a/src/controller/CHIPDevice.cpp b/src/controller/CHIPDevice.cpp index 4eadad4bdf265e..ab9dfd4f1dda73 100644 --- a/src/controller/CHIPDevice.cpp +++ b/src/controller/CHIPDevice.cpp @@ -488,6 +488,8 @@ void Device::OperationalCertProvisioned() ChipLogDetail(Controller, "Enabling CASE session establishment for the device"); mDeviceOperationalCertProvisioned = true; + Persist(); + if (mState == ConnectionState::SecureConnected) { mSessionManager->ExpirePairing(mSecureSession); diff --git a/src/controller/CHIPDevice.h b/src/controller/CHIPDevice.h index 10d2ac46c736e3..a25b28b20961d1 100644 --- a/src/controller/CHIPDevice.h +++ b/src/controller/CHIPDevice.h @@ -314,6 +314,8 @@ class DLL_EXPORT Device : public Messaging::ExchangeDelegate, public SessionEsta bool IsSecureConnected() const { return IsActive() && mState == ConnectionState::SecureConnected; } + bool IsSessionSetupInProgress() const { return IsActive() && mState == ConnectionState::Connecting; } + void Reset(); NodeId GetDeviceId() const { return mDeviceId; } diff --git a/src/protocols/secure_channel/CASEServer.cpp b/src/protocols/secure_channel/CASEServer.cpp index 00c3a01a7960db..e3368e4f5a58e6 100644 --- a/src/protocols/secure_channel/CASEServer.cpp +++ b/src/protocols/secure_channel/CASEServer.cpp @@ -112,6 +112,8 @@ void CASEServer::OnSessionEstablishmentError(CHIP_ERROR err) void CASEServer::OnSessionEstablished() { ChipLogProgress(Inet, "CASE Session established. Setting up the secure channel."); + mSessionMgr->ExpireAllPairings(mPairingSession.PeerConnection().GetPeerNodeId(), mAdminId); + CHIP_ERROR err = mSessionMgr->NewPairing(Optional::Value(mPairingSession.PeerConnection().GetPeerAddress()), mPairingSession.PeerConnection().GetPeerNodeId(), &mPairingSession, diff --git a/src/transport/SecureSessionMgr.cpp b/src/transport/SecureSessionMgr.cpp index be2ded77a25811..b13e23670ce4a5 100644 --- a/src/transport/SecureSessionMgr.cpp +++ b/src/transport/SecureSessionMgr.cpp @@ -224,6 +224,24 @@ void SecureSessionMgr::ExpirePairing(SecureSessionHandle session) } } +void SecureSessionMgr::ExpireAllPairings(NodeId peerNodeId, Transport::AdminId admin) +{ + PeerConnectionState * state = mPeerConnections.FindPeerConnectionState(peerNodeId, nullptr); + while (state != nullptr) + { + if (admin == state->GetAdminId()) + { + mPeerConnections.MarkConnectionExpired( + state, [this](const Transport::PeerConnectionState & state1) { HandleConnectionExpired(state1); }); + state = mPeerConnections.FindPeerConnectionState(peerNodeId, nullptr); + } + else + { + state = mPeerConnections.FindPeerConnectionState(peerNodeId, state); + } + } +} + CHIP_ERROR SecureSessionMgr::NewPairing(const Optional & peerAddr, NodeId peerNodeId, PairingSession * pairing, SecureSession::SessionRole direction, Transport::AdminId admin, Transport::Base * transport) diff --git a/src/transport/SecureSessionMgr.h b/src/transport/SecureSessionMgr.h index cf065b8c66b0e5..ac29c1b30614ed 100644 --- a/src/transport/SecureSessionMgr.h +++ b/src/transport/SecureSessionMgr.h @@ -221,6 +221,7 @@ class DLL_EXPORT SecureSessionMgr : public TransportMgrDelegate SecureSession::SessionRole direction, Transport::AdminId admin, Transport::Base * transport = nullptr); void ExpirePairing(SecureSessionHandle session); + void ExpireAllPairings(NodeId peerNodeId, Transport::AdminId admin); /** * @brief From 8fb02e04bd567abcb967da78491648e313d87d4a Mon Sep 17 00:00:00 2001 From: Pankaj Garg Date: Wed, 16 Jun 2021 13:45:00 -0700 Subject: [PATCH 04/12] address review comments --- .../commands/clusters/ModelCommand.cpp | 18 ++++++++-------- src/app/server/RendezvousServer.cpp | 9 +++++--- src/app/server/RendezvousServer.h | 9 ++++++-- src/app/server/Server.cpp | 18 +++++++++------- src/controller/CHIPDevice.cpp | 21 ++++++++++++------- src/controller/CHIPDeviceController.cpp | 12 ++++++----- .../secure_channel/SessionIDAllocator.cpp | 10 +++------ .../secure_channel/SessionIDAllocator.h | 4 ++-- 8 files changed, 59 insertions(+), 42 deletions(-) diff --git a/examples/chip-tool/commands/clusters/ModelCommand.cpp b/examples/chip-tool/commands/clusters/ModelCommand.cpp index 0655b79192ef51..f04b7e275c9268 100644 --- a/examples/chip-tool/commands/clusters/ModelCommand.cpp +++ b/examples/chip-tool/commands/clusters/ModelCommand.cpp @@ -39,11 +39,11 @@ void DispatchSingleClusterCommand(chip::ClusterId aClusterId, chip::CommandId aC CHIP_ERROR WaitForSessionSetup(chip::Controller::Device * device) { - constexpr time_t kWaitPerIteration = 1; - constexpr uint16_t kIterationCount = 5; + constexpr time_t kWaitPerIterationSec = 1; + constexpr uint16_t kIterationCount = 5; struct timespec sleep_time; - sleep_time.tv_sec = kWaitPerIteration; + sleep_time.tv_sec = kWaitPerIterationSec; sleep_time.tv_nsec = 0; for (uint32_t i = 0; i < kIterationCount && device->IsSessionSetupInProgress(); i++) @@ -73,12 +73,12 @@ CHIP_ERROR ModelCommand::Run(NodeId localId, NodeId remoteId) err = GetExecContext()->commissioner->GetDevice(remoteId, &mDevice); VerifyOrExit(err == CHIP_NO_ERROR, ChipLogError(chipTool, "Init failure! No pairing for device: %" PRIu64, localId)); - if (mDevice->IsSessionSetupInProgress()) - { - err = WaitForSessionSetup(mDevice); - VerifyOrExit(err == CHIP_NO_ERROR, - ChipLogError(chipTool, "Timed out while waiting for session setup for device: %" PRIu64, localId)); - } + // TODO - Implement notification from device object when the secure session is available. + // Current code is polling the device object to check for secure session availability. This should + // be updated to a notification/callback mechanism. + err = WaitForSessionSetup(mDevice); + VerifyOrExit(err == CHIP_NO_ERROR, + ChipLogError(chipTool, "Timed out while waiting for session setup for device: %" PRIu64, localId)); err = SendCommand(mDevice, mEndPointId); VerifyOrExit(err == CHIP_NO_ERROR, ChipLogError(chipTool, "Failed to send message: %s", ErrorStr(err))); diff --git a/src/app/server/RendezvousServer.cpp b/src/app/server/RendezvousServer.cpp index 344075ca039ec7..f595c10ab57a54 100644 --- a/src/app/server/RendezvousServer.cpp +++ b/src/app/server/RendezvousServer.cpp @@ -184,9 +184,12 @@ void RendezvousServer::OnSessionEstablished() VerifyOrReturn(connection.StoreIntoKVS(*mStorage) == CHIP_NO_ERROR, ChipLogError(AppServer, "Failed to store the connection state")); - uint16_t keyID = 0; - mIDAllocator->Peek(keyID); - + // The Peek() is used to find the highest key ID that's been assigned to any session. + // This value is persisted, and on reboot, it is used to revive any previously + // active secure sessions. + // We support one active PASE session at any time. So the key ID should not be updated + // in another thread, while we retrieve it here. + uint16_t keyID = mIDAllocator->Peek(); mStorage->SyncSetKeyValue(kStorablePeerConnectionCountKey, &keyID, sizeof(keyID)); } } // namespace chip diff --git a/src/app/server/RendezvousServer.h b/src/app/server/RendezvousServer.h index c9920976157e9e..e470534603b1a2 100644 --- a/src/app/server/RendezvousServer.h +++ b/src/app/server/RendezvousServer.h @@ -35,9 +35,14 @@ class RendezvousServer : public SessionEstablishmentDelegate CHIP_ERROR Init(AppDelegate * delegate, PersistentStorageDelegate * storage, SessionIDAllocator * idAllocator) { VerifyOrReturnError(storage != nullptr, CHIP_ERROR_INVALID_ARGUMENT); - mDelegate = delegate; - mStorage = storage; + mStorage = storage; + + VerifyOrReturnError(idAllocator != nullptr, CHIP_ERROR_INVALID_ARGUMENT); mIDAllocator = idAllocator; + + // The caller may chose to not provide a delegate object. The RendezvousServer checks for null delegate before calling + // it's methods. + mDelegate = delegate; return CHIP_NO_ERROR; } diff --git a/src/app/server/Server.cpp b/src/app/server/Server.cpp index 2eddf3c633dcbe..335ce725db5891 100644 --- a/src/app/server/Server.cpp +++ b/src/app/server/Server.cpp @@ -171,10 +171,16 @@ static CHIP_ERROR RestoreAllSessionsFromKVS(SecureSessionMgr & sessionMgr) ChipLogProgress(AppServer, "Fetched the session information: from 0x" ChipLogFormatX64, ChipLogValueX64(session->PeerConnection().GetPeerNodeId())); - sessionMgr.NewPairing(Optional::Value(session->PeerConnection().GetPeerAddress()), - session->PeerConnection().GetPeerNodeId(), session, SecureSession::SessionRole::kResponder, - connection.GetAdminId(), nullptr); - gSessionIDAllocator.Reserve(keyId); + if (gSessionIDAllocator.Reserve(keyId) == CHIP_NO_ERROR) + { + sessionMgr.NewPairing(Optional::Value(session->PeerConnection().GetPeerAddress()), + session->PeerConnection().GetPeerNodeId(), session, SecureSession::SessionRole::kResponder, + connection.GetAdminId(), nullptr); + } + else + { + ChipLogProgress(AppServer, "Session Key ID %d cannot be used. Skipping over this session", keyId); + } session->Clear(); } } @@ -429,10 +435,8 @@ CHIP_ERROR OpenDefaultPairingWindow(ResetAdmins resetAdmins, chip::PairingWindow if (resetAdmins == ResetAdmins::kYes) { - uint16_t nextKeyId; - ReturnErrorOnFailure(gSessionIDAllocator.Peek(nextKeyId)); EraseAllAdminPairingsUpTo(gNextAvailableAdminId); - EraseAllSessionsUpTo(nextKeyId); + EraseAllSessionsUpTo(gSessionIDAllocator.Peek()); // Only resetting gNextAvailableAdminId at reboot otherwise previously paired device with adminID 0 // can continue sending messages to accessory as next available admin will also be 0. // This logic is not up to spec, will be implemented up to spec once AddOptCert is implemented. diff --git a/src/controller/CHIPDevice.cpp b/src/controller/CHIPDevice.cpp index ab9dfd4f1dda73..43352e28fe1848 100644 --- a/src/controller/CHIPDevice.cpp +++ b/src/controller/CHIPDevice.cpp @@ -165,16 +165,23 @@ CHIP_ERROR Device::Serialize(SerializedDevice & output) Transport::PeerConnectionState * connectionState = mSessionManager->GetPeerConnectionState(mSecureSession); - uint32_t localMessageCounter = 0; - uint32_t peerMessageCounter = 0; + // The connection state could be null if the device is moving from PASE connection to CASE connection. + // The device parameters (e.g. mDeviceOperationalCertProvisioned) are updated during this transition. + // The state during this transistion is being persisted so that the next access of the device will + // trigger the CASE based secure session. if (connectionState != nullptr) { - localMessageCounter = connectionState->GetSessionMessageCounter().GetLocalMessageCounter().Value(); - peerMessageCounter = connectionState->GetSessionMessageCounter().GetPeerMessageCounter().GetCounter(); - } + const uint32_t localMessageCounter = connectionState->GetSessionMessageCounter().GetLocalMessageCounter().Value(); + const uint32_t peerMessageCounter = connectionState->GetSessionMessageCounter().GetPeerMessageCounter().GetCounter(); - serializable.mLocalMessageCounter = Encoding::LittleEndian::HostSwap32(localMessageCounter); - serializable.mPeerMessageCounter = Encoding::LittleEndian::HostSwap32(peerMessageCounter); + serializable.mLocalMessageCounter = Encoding::LittleEndian::HostSwap32(localMessageCounter); + serializable.mPeerMessageCounter = Encoding::LittleEndian::HostSwap32(peerMessageCounter); + } + else + { + serializable.mLocalMessageCounter = 0; + serializable.mPeerMessageCounter = 0; + } serializable.mDeviceOperationalCertProvisioned = (mDeviceOperationalCertProvisioned) ? 1 : 0; diff --git a/src/controller/CHIPDeviceController.cpp b/src/controller/CHIPDeviceController.cpp index 68924dbb16497a..ed95f5d5862d53 100644 --- a/src/controller/CHIPDeviceController.cpp +++ b/src/controller/CHIPDeviceController.cpp @@ -236,11 +236,14 @@ CHIP_ERROR DeviceController::GenerateOperationalCertificates(const ByteSpan & CS // This implies that the commissioner application uses root CA to sign the operational // certificates, and an intermediate CA is not needed. It's not an error condition, so // let's just send operational certificate and root CA certificate to the device. - err = CHIP_NO_ERROR; icaLen = 0; ChipLogProgress(Controller, "Intermediate CA is not needed"); } - ReturnErrorOnFailure(err); + else if (err != CHIP_NO_ERROR) + { + return err; + } + ReturnErrorOnFailure(ConvertX509CertsToChipCertArray(ByteSpan(noc.Get(), nocLen), ByteSpan(ica.Get(), icaLen), certBuf, certBufSize, outCertLen)); @@ -732,8 +735,7 @@ void DeviceController::PersistNextKeyId() { if (mStorageDelegate != nullptr && mState == State::Initialized) { - uint16_t nextKeyID = 0; - mIDAllocator.Peek(nextKeyID); + uint16_t nextKeyID = mIDAllocator.Peek(); mStorageDelegate->SyncSetKeyValue(kNextAvailableKeyID, &nextKeyID, sizeof(nextKeyID)); } } @@ -811,7 +813,7 @@ CHIP_ERROR DeviceCommissioner::Init(NodeId localDeviceId, CommissionerInitParams } for (uint16_t i = 0; i < nextKeyID; i++) { - mIDAllocator.Reserve(i); + ReturnErrorOnFailure(mIDAllocator.Reserve(i)); } mPairingDelegate = params.pairingDelegate; diff --git a/src/protocols/secure_channel/SessionIDAllocator.cpp b/src/protocols/secure_channel/SessionIDAllocator.cpp index d3d5a10969baa4..f2a81c5d6c3cfc 100644 --- a/src/protocols/secure_channel/SessionIDAllocator.cpp +++ b/src/protocols/secure_channel/SessionIDAllocator.cpp @@ -32,13 +32,12 @@ CHIP_ERROR SessionIDAllocator::Allocate(uint16_t & id) return CHIP_NO_ERROR; } -CHIP_ERROR SessionIDAllocator::Free(uint16_t id) +void SessionIDAllocator::Free(uint16_t id) { if (mNextAvailable == id && mNextAvailable > 0) { mNextAvailable--; } - return CHIP_NO_ERROR; } CHIP_ERROR SessionIDAllocator::Reserve(uint16_t id) @@ -53,12 +52,9 @@ CHIP_ERROR SessionIDAllocator::Reserve(uint16_t id) return CHIP_NO_ERROR; } -CHIP_ERROR SessionIDAllocator::Peek(uint16_t & id) +uint16_t SessionIDAllocator::Peek() { - VerifyOrReturnError(mNextAvailable < kMaxSessionID, CHIP_ERROR_NO_MEMORY); - id = mNextAvailable; - - return CHIP_NO_ERROR; + return mNextAvailable; } } // namespace chip diff --git a/src/protocols/secure_channel/SessionIDAllocator.h b/src/protocols/secure_channel/SessionIDAllocator.h index 9515282362920a..88c705c7af4a4b 100644 --- a/src/protocols/secure_channel/SessionIDAllocator.h +++ b/src/protocols/secure_channel/SessionIDAllocator.h @@ -28,9 +28,9 @@ class SessionIDAllocator ~SessionIDAllocator() {} CHIP_ERROR Allocate(uint16_t & id); - CHIP_ERROR Free(uint16_t id); + void Free(uint16_t id); CHIP_ERROR Reserve(uint16_t id); - CHIP_ERROR Peek(uint16_t & id); + uint16_t Peek(); private: // Session ID is a 15 bit value (16th bit indicates unicast/group key) From 481ea8fda482599cb44a58a412f0e76f393d571e Mon Sep 17 00:00:00 2001 From: Pankaj Garg Date: Wed, 16 Jun 2021 13:58:36 -0700 Subject: [PATCH 05/12] add tests for SessionIDAllocator --- .../secure_channel/SessionIDAllocator.cpp | 4 +- src/protocols/secure_channel/tests/BUILD.gn | 1 + .../tests/TestSessionIDAllocator.cpp | 148 ++++++++++++++++++ 3 files changed, 152 insertions(+), 1 deletion(-) create mode 100644 src/protocols/secure_channel/tests/TestSessionIDAllocator.cpp diff --git a/src/protocols/secure_channel/SessionIDAllocator.cpp b/src/protocols/secure_channel/SessionIDAllocator.cpp index f2a81c5d6c3cfc..ae097ce089664f 100644 --- a/src/protocols/secure_channel/SessionIDAllocator.cpp +++ b/src/protocols/secure_channel/SessionIDAllocator.cpp @@ -34,7 +34,7 @@ CHIP_ERROR SessionIDAllocator::Allocate(uint16_t & id) void SessionIDAllocator::Free(uint16_t id) { - if (mNextAvailable == id && mNextAvailable > 0) + if (mNextAvailable > 0 && (mNextAvailable - 1) == id) { mNextAvailable--; } @@ -49,6 +49,8 @@ CHIP_ERROR SessionIDAllocator::Reserve(uint16_t id) mNextAvailable++; } + // TODO - Check if ID is already allocated in SessionIDAllocator::Reserve() + return CHIP_NO_ERROR; } diff --git a/src/protocols/secure_channel/tests/BUILD.gn b/src/protocols/secure_channel/tests/BUILD.gn index 6bbcfb328b9def..a48aa9a65dacf1 100644 --- a/src/protocols/secure_channel/tests/BUILD.gn +++ b/src/protocols/secure_channel/tests/BUILD.gn @@ -12,6 +12,7 @@ chip_test_suite("tests") { "TestCASESession.cpp", "TestMessageCounterManager.cpp", "TestPASESession.cpp", + "TestSessionIDAllocator.cpp", "TestStatusReport.cpp", ] diff --git a/src/protocols/secure_channel/tests/TestSessionIDAllocator.cpp b/src/protocols/secure_channel/tests/TestSessionIDAllocator.cpp new file mode 100644 index 00000000000000..3d0cd14cb6f066 --- /dev/null +++ b/src/protocols/secure_channel/tests/TestSessionIDAllocator.cpp @@ -0,0 +1,148 @@ +/* + * + * Copyright (c) 2021 Project CHIP Authors + * All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include +#include +#include + +#include + +using namespace chip; + +void TestStatusReport_Allocate(nlTestSuite * inSuite, void * inContext) +{ + SessionIDAllocator allocator; + + NL_TEST_ASSERT(inSuite, allocator.Peek() == 0); + + uint16_t id; + + for (uint16_t i = 0; i < 16; i++) + { + CHIP_ERROR err = allocator.Allocate(id); + NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR); + NL_TEST_ASSERT(inSuite, id == i); + NL_TEST_ASSERT(inSuite, allocator.Peek() == i + 1); + } +} + +void TestStatusReport_Free(nlTestSuite * inSuite, void * inContext) +{ + SessionIDAllocator allocator; + + NL_TEST_ASSERT(inSuite, allocator.Peek() == 0); + + uint16_t id; + + for (uint16_t i = 0; i < 16; i++) + { + CHIP_ERROR err = allocator.Allocate(id); + NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR); + NL_TEST_ASSERT(inSuite, id == i); + NL_TEST_ASSERT(inSuite, allocator.Peek() == i + 1); + } + + // Free an intermediate ID + allocator.Free(10); + NL_TEST_ASSERT(inSuite, allocator.Peek() == 16); + + // Free the last allocated ID + allocator.Free(15); + NL_TEST_ASSERT(inSuite, allocator.Peek() == 15); + + // Free some random unallocated ID + allocator.Free(100); + NL_TEST_ASSERT(inSuite, allocator.Peek() == 15); +} + +void TestStatusReport_Reserve(nlTestSuite * inSuite, void * inContext) +{ + SessionIDAllocator allocator; + + uint16_t id; + + for (uint16_t i = 0; i < 16; i++) + { + CHIP_ERROR err = allocator.Allocate(id); + NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR); + NL_TEST_ASSERT(inSuite, id == i); + NL_TEST_ASSERT(inSuite, allocator.Peek() == i + 1); + } + + allocator.Reserve(100); + NL_TEST_ASSERT(inSuite, allocator.Peek() == 101); +} + +// Test Suite + +/** + * Test Suite that lists all the test functions. + */ +// clang-format off +static const nlTest sTests[] = +{ + NL_TEST_DEF("SessionIDAllocator_Allocate", TestStatusReport_Allocate), + NL_TEST_DEF("SessionIDAllocator_Free", TestStatusReport_Free), + NL_TEST_DEF("SessionIDAllocator_Reserve", TestStatusReport_Reserve), + + NL_TEST_SENTINEL() +}; +// clang-format on + +/** + * Set up the test suite. + */ +static int TestSetup(void * inContext) +{ + CHIP_ERROR error = chip::Platform::MemoryInit(); + if (error != CHIP_NO_ERROR) + return FAILURE; + return SUCCESS; +} + +/** + * Tear down the test suite. + */ +static int TestTeardown(void * inContext) +{ + chip::Platform::MemoryShutdown(); + return SUCCESS; +} + +// clang-format off +static nlTestSuite sSuite = +{ + "Test-CHIP-SessionIDAllocator", + &sTests[0], + TestSetup, + TestTeardown, +}; +// clang-format on + +/** + * Main + */ +int TestStatusReport() +{ + // Run test suit against one context + nlTestRunner(&sSuite, nullptr); + + return (nlTestRunnerStats(&sSuite)); +} + +CHIP_REGISTER_TEST_SUITE(TestStatusReport) From c15904e3160f0da2ed4f5e16d70c2fcaca7393ee Mon Sep 17 00:00:00 2001 From: Pankaj Garg Date: Thu, 17 Jun 2021 09:08:53 -0700 Subject: [PATCH 06/12] address review comments --- src/app/server/RendezvousServer.cpp | 2 +- src/app/server/RendezvousServer.h | 2 +- src/app/server/Server.cpp | 2 +- src/controller/CHIPDevice.h | 8 +++++ src/controller/CHIPDeviceController.cpp | 31 ++++++++++++------- src/controller/CHIPDeviceController.h | 5 +++ src/credentials/CHIPCert.h | 3 ++ src/protocols/secure_channel/CASESession.cpp | 5 ++- .../tests/TestSessionIDAllocator.cpp | 16 +++++----- 9 files changed, 49 insertions(+), 25 deletions(-) diff --git a/src/app/server/RendezvousServer.cpp b/src/app/server/RendezvousServer.cpp index f595c10ab57a54..25aa9b2c4e42be 100644 --- a/src/app/server/RendezvousServer.cpp +++ b/src/app/server/RendezvousServer.cpp @@ -184,7 +184,7 @@ void RendezvousServer::OnSessionEstablished() VerifyOrReturn(connection.StoreIntoKVS(*mStorage) == CHIP_NO_ERROR, ChipLogError(AppServer, "Failed to store the connection state")); - // The Peek() is used to find the highest key ID that's been assigned to any session. + // The Peek() is used to find the smallest key ID that's not been assigned to any session. // This value is persisted, and on reboot, it is used to revive any previously // active secure sessions. // We support one active PASE session at any time. So the key ID should not be updated diff --git a/src/app/server/RendezvousServer.h b/src/app/server/RendezvousServer.h index e470534603b1a2..845dcd5e4f5cc2 100644 --- a/src/app/server/RendezvousServer.h +++ b/src/app/server/RendezvousServer.h @@ -41,7 +41,7 @@ class RendezvousServer : public SessionEstablishmentDelegate mIDAllocator = idAllocator; // The caller may chose to not provide a delegate object. The RendezvousServer checks for null delegate before calling - // it's methods. + // its methods. mDelegate = delegate; return CHIP_NO_ERROR; } diff --git a/src/app/server/Server.cpp b/src/app/server/Server.cpp index 335ce725db5891..b1beb043c45773 100644 --- a/src/app/server/Server.cpp +++ b/src/app/server/Server.cpp @@ -179,7 +179,7 @@ static CHIP_ERROR RestoreAllSessionsFromKVS(SecureSessionMgr & sessionMgr) } else { - ChipLogProgress(AppServer, "Session Key ID %d cannot be used. Skipping over this session", keyId); + ChipLogProgress(AppServer, "Session Key ID %" PRIu16 " cannot be used. Skipping over this session", keyId); } session->Clear(); } diff --git a/src/controller/CHIPDevice.h b/src/controller/CHIPDevice.h index a25b28b20961d1..4644b0289bcfe1 100644 --- a/src/controller/CHIPDevice.h +++ b/src/controller/CHIPDevice.h @@ -352,6 +352,14 @@ class DLL_EXPORT Device : public Messaging::ExchangeDelegate, public SessionEsta ByteSpan GetCSRNonce() const { return ByteSpan(mCSRNonce, sizeof(mCSRNonce)); } + /** + * @brief + * This function triggers CASE session setup if the device has been provisioned with + * operational credentials, and there is no currently active session. + * + * @return CHIP_NO_ERROR if the session setup was triggered or a session is already available. + */ + CHIP_ERROR WarmupCASESession(); private: diff --git a/src/controller/CHIPDeviceController.cpp b/src/controller/CHIPDeviceController.cpp index ed95f5d5862d53..1534143b86e42d 100644 --- a/src/controller/CHIPDeviceController.cpp +++ b/src/controller/CHIPDeviceController.cpp @@ -96,8 +96,12 @@ using namespace chip::Encoding; constexpr uint32_t kSessionEstablishmentTimeout = 30 * kMillisecondPerSecond; -constexpr uint32_t kMaxCHIPOpCertLength = 1024; -constexpr uint32_t kMaxCHIPCSRLength = 1024; +// TODO - Reduce memory requirement for generating x509 certificates +// As per specifications (section 6.3.7. Trusted Root CA Certificates), DER certs +// should require 600 bytes. Currently, due to TLV writer overheads, a larger buffer +// is needed, even though the generated certificate fits in 600 bytes limit. +constexpr uint32_t kMaxCHIPDERCertLength = 1024; +constexpr uint32_t kMaxCHIPCSRLength = 1024; DeviceController::DeviceController() { @@ -214,22 +218,25 @@ CHIP_ERROR DeviceController::Init(NodeId localDeviceId, ControllerInitParams par CHIP_ERROR DeviceController::GenerateOperationalCertificates(const ByteSpan & CSR, NodeId deviceId, uint8_t * certBuf, uint32_t certBufSize, uint32_t & outCertLen) { + // This code requires about 2K RAM to generate the certificates. + // The code would run as part of commissioner applications, so RAM requirements should be fine. + // Need to analyze if this requirement could be better managed by using static memory pools. chip::Platform::ScopedMemoryBuffer noc; - ReturnErrorCodeIf(!noc.Alloc(kMaxCHIPOpCertLength), CHIP_ERROR_NO_MEMORY); + ReturnErrorCodeIf(!noc.Alloc(kMaxCHIPDERCertLength), CHIP_ERROR_NO_MEMORY); uint32_t nocLen = 0; ChipLogProgress(Controller, "Generating operational certificate for device " ChipLogFormatX64, ChipLogValueX64(deviceId)); ReturnErrorOnFailure(mOperationalCredentialsDelegate->GenerateNodeOperationalCertificate( - PeerId().SetNodeId(deviceId), CSR, 1, noc.Get(), kMaxCHIPOpCertLength, nocLen)); + PeerId().SetNodeId(deviceId), CSR, 1, noc.Get(), kMaxCHIPDERCertLength, nocLen)); ReturnErrorCodeIf(nocLen == 0, CHIP_ERROR_CERT_NOT_FOUND); chip::Platform::ScopedMemoryBuffer ica; - ReturnErrorCodeIf(!ica.Alloc(kMaxCHIPOpCertLength), CHIP_ERROR_NO_MEMORY); + ReturnErrorCodeIf(!ica.Alloc(kMaxCHIPDERCertLength), CHIP_ERROR_NO_MEMORY); uint32_t icaLen = 0; ChipLogProgress(Controller, "Getting intermediate CA certificate from the issuer"); - CHIP_ERROR err = mOperationalCredentialsDelegate->GetIntermediateCACertificate(0, ica.Get(), kMaxCHIPOpCertLength, icaLen); + CHIP_ERROR err = mOperationalCredentialsDelegate->GetIntermediateCACertificate(0, ica.Get(), kMaxCHIPDERCertLength, icaLen); ChipLogProgress(Controller, "GetIntermediateCACertificate returned %" PRId32, err); if (err == CHIP_ERROR_INTERMEDIATE_CA_NOT_REQUIRED) { @@ -260,7 +267,7 @@ CHIP_ERROR DeviceController::LoadLocalCredentials(Transport::AdminPairingInfo * if (!admin->AreCredentialsAvailable()) { chip::Platform::ScopedMemoryBuffer chipCert; - uint32_t chipCertAllocatedLen = kMaxCHIPOpCertLength * 2; + uint32_t chipCertAllocatedLen = kMaxCHIPCertLength * 2; ReturnErrorCodeIf(!chipCert.Alloc(chipCertAllocatedLen), CHIP_ERROR_NO_MEMORY); uint32_t chipCertLen = 0; @@ -268,14 +275,16 @@ CHIP_ERROR DeviceController::LoadLocalCredentials(Transport::AdminPairingInfo * { ChipLogProgress(Controller, "Getting root certificate for the controller from the issuer"); chip::Platform::ScopedMemoryBuffer rootCert; - ReturnErrorCodeIf(!rootCert.Alloc(kMaxCHIPOpCertLength), CHIP_ERROR_NO_MEMORY); + ReturnErrorCodeIf(!rootCert.Alloc(kMaxCHIPDERCertLength), CHIP_ERROR_NO_MEMORY); uint32_t rootCertLen = 0; ReturnErrorOnFailure( - mOperationalCredentialsDelegate->GetRootCACertificate(0, rootCert.Get(), kMaxCHIPOpCertLength, rootCertLen)); + mOperationalCredentialsDelegate->GetRootCACertificate(0, rootCert.Get(), kMaxCHIPDERCertLength, rootCertLen)); + ChipLogProgress(Controller, "Got x509 root cert (len %d). Converting to CHIP Cert", rootCertLen); ReturnErrorOnFailure( ConvertX509CertToChipCert(rootCert.Get(), rootCertLen, chipCert.Get(), chipCertAllocatedLen, chipCertLen)); + ChipLogProgress(Controller, "Got root cert (len %d). Saving it in table", chipCertLen); ReturnErrorOnFailure(admin->SetRootCert(ByteSpan(chipCert.Get(), chipCertLen))); } @@ -1218,11 +1227,11 @@ CHIP_ERROR DeviceCommissioner::ProcessOpCSR(const ByteSpan & CSR, const ByteSpan VerifyOrReturnError(memcmp(CSRNonce.data(), nonce.data(), CSRNonce.size()) == 0, CHIP_ERROR_INVALID_ARGUMENT); chip::Platform::ScopedMemoryBuffer chipOpCert; - ReturnErrorCodeIf(!chipOpCert.Alloc(kMaxCHIPOpCertLength * 2), CHIP_ERROR_NO_MEMORY); + ReturnErrorCodeIf(!chipOpCert.Alloc(kMaxCHIPCertLength * 2), CHIP_ERROR_NO_MEMORY); uint32_t chipOpCertLen = 0; ReturnErrorOnFailure( - GenerateOperationalCertificates(CSR, device->GetDeviceId(), chipOpCert.Get(), kMaxCHIPOpCertLength * 2, chipOpCertLen)); + GenerateOperationalCertificates(CSR, device->GetDeviceId(), chipOpCert.Get(), kMaxCHIPCertLength * 2, chipOpCertLen)); ChipLogProgress(Controller, "Sending operational certificate to the device. Op Cert Len %" PRIu32, chipOpCertLen); ReturnErrorOnFailure(SendOperationalCertificate(device, ByteSpan(chipOpCert.Get(), chipOpCertLen))); diff --git a/src/controller/CHIPDeviceController.h b/src/controller/CHIPDeviceController.h index 725a6b871c3af7..b9ee0c6011e1c5 100644 --- a/src/controller/CHIPDeviceController.h +++ b/src/controller/CHIPDeviceController.h @@ -343,6 +343,11 @@ class DLL_EXPORT DeviceController : public Messaging::ExchangeDelegate, Mdns::DiscoveredNodeData * GetDiscoveredNodes() override { return mCommissionableNodes; } #endif // CHIP_DEVICE_CONFIG_ENABLE_MDNS + // This function uses `OperationalCredentialsDelegate` to generate the operational certificates + // for the given device. The output is a TLV encoded array of compressed CHIP certificates. The + // array can contain up to two certificates (node operational certificate, and ICA certificate). + // If the certificate issuer doesn't require an ICA (i.e. NOC is signed by the root CA), the array + // will have only one certificate (node operational certificate). CHIP_ERROR GenerateOperationalCertificates(const ByteSpan & CSR, NodeId deviceId, uint8_t * certBuf, uint32_t certBufSize, uint32_t & outCertLen); diff --git a/src/credentials/CHIPCert.h b/src/credentials/CHIPCert.h index 0dfaa4e3d6d73f..473886af59462c 100755 --- a/src/credentials/CHIPCert.h +++ b/src/credentials/CHIPCert.h @@ -44,6 +44,9 @@ static constexpr uint32_t kChip32bitAttrUTF8Length = 8; static constexpr uint32_t kChip64bitAttrUTF8Length = 16; static constexpr uint16_t kX509NoWellDefinedExpirationDateYear = 9999; +// As per specifications (section 6.3.7. Trusted Root CA Certificates) +static constexpr uint32_t kMaxCHIPCertLength = 400; + /** Data Element Tags for the CHIP Certificate */ enum diff --git a/src/protocols/secure_channel/CASESession.cpp b/src/protocols/secure_channel/CASESession.cpp index ccb478b7a58aa1..d3e42315774bb5 100644 --- a/src/protocols/secure_channel/CASESession.cpp +++ b/src/protocols/secure_channel/CASESession.cpp @@ -55,8 +55,7 @@ constexpr uint8_t kIVSR2[] = { 0x4e, 0x43, 0x41, 0x53, 0x45, 0x5f, 0x53, 0x69, 0 constexpr uint8_t kIVSR3[] = { 0x4e, 0x43, 0x41, 0x53, 0x45, 0x5f, 0x53, 0x69, 0x67, 0x6d, 0x61, 0x52, 0x33 }; constexpr size_t kIVLength = sizeof(kIVSR2); -constexpr size_t kTAGSize = 16; -constexpr uint32_t kMaxCHIPOpCertLength = 1024; +constexpr size_t kTAGSize = 16; using namespace Crypto; using namespace Credentials; @@ -1026,7 +1025,7 @@ CHIP_ERROR CASESession::Validate_and_RetrieveResponderID(const uint8_t ** msgIte ChipCertificateSet certSet; // Certificate set can contain up to 3 certs (NOC, ICA cert, and Root CA cert) - ReturnErrorOnFailure(certSet.Init(3, kMaxCHIPOpCertLength * 3)); + ReturnErrorOnFailure(certSet.Init(3, kMaxCHIPCertLength * 3)); responderOpCertLen = chip::Encoding::LittleEndian::Read16(*msgIterator); *responderOpCert = *msgIterator; diff --git a/src/protocols/secure_channel/tests/TestSessionIDAllocator.cpp b/src/protocols/secure_channel/tests/TestSessionIDAllocator.cpp index 3d0cd14cb6f066..2e09d537bc2822 100644 --- a/src/protocols/secure_channel/tests/TestSessionIDAllocator.cpp +++ b/src/protocols/secure_channel/tests/TestSessionIDAllocator.cpp @@ -24,7 +24,7 @@ using namespace chip; -void TestStatusReport_Allocate(nlTestSuite * inSuite, void * inContext) +void TestSessionIDAllocator_Allocate(nlTestSuite * inSuite, void * inContext) { SessionIDAllocator allocator; @@ -41,7 +41,7 @@ void TestStatusReport_Allocate(nlTestSuite * inSuite, void * inContext) } } -void TestStatusReport_Free(nlTestSuite * inSuite, void * inContext) +void TestSessionIDAllocator_Free(nlTestSuite * inSuite, void * inContext) { SessionIDAllocator allocator; @@ -70,7 +70,7 @@ void TestStatusReport_Free(nlTestSuite * inSuite, void * inContext) NL_TEST_ASSERT(inSuite, allocator.Peek() == 15); } -void TestStatusReport_Reserve(nlTestSuite * inSuite, void * inContext) +void TestSessionIDAllocator_Reserve(nlTestSuite * inSuite, void * inContext) { SessionIDAllocator allocator; @@ -96,9 +96,9 @@ void TestStatusReport_Reserve(nlTestSuite * inSuite, void * inContext) // clang-format off static const nlTest sTests[] = { - NL_TEST_DEF("SessionIDAllocator_Allocate", TestStatusReport_Allocate), - NL_TEST_DEF("SessionIDAllocator_Free", TestStatusReport_Free), - NL_TEST_DEF("SessionIDAllocator_Reserve", TestStatusReport_Reserve), + NL_TEST_DEF("SessionIDAllocator_Allocate", TestSessionIDAllocator_Allocate), + NL_TEST_DEF("SessionIDAllocator_Free", TestSessionIDAllocator_Free), + NL_TEST_DEF("SessionIDAllocator_Reserve", TestSessionIDAllocator_Reserve), NL_TEST_SENTINEL() }; @@ -137,7 +137,7 @@ static nlTestSuite sSuite = /** * Main */ -int TestStatusReport() +int TestSessionIDAllocator() { // Run test suit against one context nlTestRunner(&sSuite, nullptr); @@ -145,4 +145,4 @@ int TestStatusReport() return (nlTestRunnerStats(&sSuite)); } -CHIP_REGISTER_TEST_SUITE(TestStatusReport) +CHIP_REGISTER_TEST_SUITE(TestSessionIDAllocator) From c1f513a6d0f48395c01beb317e103e3c747e9a3c Mon Sep 17 00:00:00 2001 From: Pankaj Garg Date: Thu, 17 Jun 2021 11:31:03 -0700 Subject: [PATCH 07/12] address review comments --- src/controller/CHIPDeviceController.cpp | 27 ++++++++----------- src/controller/CHIPDeviceController.h | 14 +++++----- src/credentials/CHIPCert.h | 11 +++----- src/credentials/CHIPCertFromX509.cpp | 12 ++++++--- src/credentials/tests/TestChipCert.cpp | 35 ++++++++++++++----------- src/transport/AdminPairingTable.cpp | 6 ++--- src/transport/AdminPairingTable.h | 6 ++--- 7 files changed, 53 insertions(+), 58 deletions(-) diff --git a/src/controller/CHIPDeviceController.cpp b/src/controller/CHIPDeviceController.cpp index 1534143b86e42d..e80422f9fc0edd 100644 --- a/src/controller/CHIPDeviceController.cpp +++ b/src/controller/CHIPDeviceController.cpp @@ -98,7 +98,7 @@ constexpr uint32_t kSessionEstablishmentTimeout = 30 * kMillisecondPerSecond; // TODO - Reduce memory requirement for generating x509 certificates // As per specifications (section 6.3.7. Trusted Root CA Certificates), DER certs -// should require 600 bytes. Currently, due to TLV writer overheads, a larger buffer +// should require 600 bytes. Currently, due to ASN writer overheads, a larger buffer // is needed, even though the generated certificate fits in 600 bytes limit. constexpr uint32_t kMaxCHIPDERCertLength = 1024; constexpr uint32_t kMaxCHIPCSRLength = 1024; @@ -215,8 +215,7 @@ CHIP_ERROR DeviceController::Init(NodeId localDeviceId, ControllerInitParams par return CHIP_NO_ERROR; } -CHIP_ERROR DeviceController::GenerateOperationalCertificates(const ByteSpan & CSR, NodeId deviceId, uint8_t * certBuf, - uint32_t certBufSize, uint32_t & outCertLen) +CHIP_ERROR DeviceController::GenerateOperationalCertificates(const ByteSpan & CSR, NodeId deviceId, MutableByteSpan & cert) { // This code requires about 2K RAM to generate the certificates. // The code would run as part of commissioner applications, so RAM requirements should be fine. @@ -251,8 +250,7 @@ CHIP_ERROR DeviceController::GenerateOperationalCertificates(const ByteSpan & CS return err; } - ReturnErrorOnFailure(ConvertX509CertsToChipCertArray(ByteSpan(noc.Get(), nocLen), ByteSpan(ica.Get(), icaLen), certBuf, - certBufSize, outCertLen)); + ReturnErrorOnFailure(ConvertX509CertsToChipCertArray(ByteSpan(noc.Get(), nocLen), ByteSpan(ica.Get(), icaLen), cert)); return CHIP_NO_ERROR; } @@ -280,11 +278,9 @@ CHIP_ERROR DeviceController::LoadLocalCredentials(Transport::AdminPairingInfo * ReturnErrorOnFailure( mOperationalCredentialsDelegate->GetRootCACertificate(0, rootCert.Get(), kMaxCHIPDERCertLength, rootCertLen)); - ChipLogProgress(Controller, "Got x509 root cert (len %d). Converting to CHIP Cert", rootCertLen); ReturnErrorOnFailure( ConvertX509CertToChipCert(rootCert.Get(), rootCertLen, chipCert.Get(), chipCertAllocatedLen, chipCertLen)); - ChipLogProgress(Controller, "Got root cert (len %d). Saving it in table", chipCertLen); ReturnErrorOnFailure(admin->SetRootCert(ByteSpan(chipCert.Get(), chipCertLen))); } @@ -299,10 +295,9 @@ CHIP_ERROR DeviceController::LoadLocalCredentials(Transport::AdminPairingInfo * // TODO - Match the generated cert against CSR and operational keypair // Make sure it chains back to the trusted root. ChipLogProgress(Controller, "Generating operational certificate for the controller"); - chipCertLen = 0; - ReturnErrorOnFailure(GenerateOperationalCertificates(ByteSpan(CSR.Get(), csrLength), mLocalDeviceId, chipCert.Get(), - chipCertAllocatedLen, chipCertLen)); - ReturnErrorOnFailure(admin->SetOperationalCert(ByteSpan(chipCert.Get(), chipCertLen))); + MutableByteSpan chipCertSpan(chipCert.Get(), chipCertAllocatedLen); + ReturnErrorOnFailure(GenerateOperationalCertificates(ByteSpan(CSR.Get(), csrLength), mLocalDeviceId, chipCertSpan)); + ReturnErrorOnFailure(admin->SetOperationalCert(chipCertSpan)); } ReturnErrorOnFailure(mAdmins.Store(admin->GetAdminId())); @@ -1228,13 +1223,13 @@ CHIP_ERROR DeviceCommissioner::ProcessOpCSR(const ByteSpan & CSR, const ByteSpan chip::Platform::ScopedMemoryBuffer chipOpCert; ReturnErrorCodeIf(!chipOpCert.Alloc(kMaxCHIPCertLength * 2), CHIP_ERROR_NO_MEMORY); - uint32_t chipOpCertLen = 0; - ReturnErrorOnFailure( - GenerateOperationalCertificates(CSR, device->GetDeviceId(), chipOpCert.Get(), kMaxCHIPCertLength * 2, chipOpCertLen)); + MutableByteSpan chipCertSpan(chipOpCert.Get(), kMaxCHIPCertLength * 2); - ChipLogProgress(Controller, "Sending operational certificate to the device. Op Cert Len %" PRIu32, chipOpCertLen); - ReturnErrorOnFailure(SendOperationalCertificate(device, ByteSpan(chipOpCert.Get(), chipOpCertLen))); + ReturnErrorOnFailure(GenerateOperationalCertificates(CSR, device->GetDeviceId(), chipCertSpan)); + + ChipLogProgress(Controller, "Sending operational certificate to the device"); + ReturnErrorOnFailure(SendOperationalCertificate(device, chipCertSpan)); return CHIP_NO_ERROR; } diff --git a/src/controller/CHIPDeviceController.h b/src/controller/CHIPDeviceController.h index b9ee0c6011e1c5..134ae3279600e5 100644 --- a/src/controller/CHIPDeviceController.h +++ b/src/controller/CHIPDeviceController.h @@ -348,8 +348,7 @@ class DLL_EXPORT DeviceController : public Messaging::ExchangeDelegate, // array can contain up to two certificates (node operational certificate, and ICA certificate). // If the certificate issuer doesn't require an ICA (i.e. NOC is signed by the root CA), the array // will have only one certificate (node operational certificate). - CHIP_ERROR GenerateOperationalCertificates(const ByteSpan & CSR, NodeId deviceId, uint8_t * certBuf, uint32_t certBufSize, - uint32_t & outCertLen); + CHIP_ERROR GenerateOperationalCertificates(const ByteSpan & CSR, NodeId deviceId, MutableByteSpan & cert); private: //////////// ExchangeDelegate Implementation /////////////// @@ -454,13 +453,12 @@ class DLL_EXPORT DeviceCommissioner : public DeviceController, public SessionEst CHIP_ERROR UnpairDevice(NodeId remoteDeviceId); /** - * @brief - * This function is called by the commissioner application when a device (being paired) is - * discovered on the operational network. - * - * @param[in] remoteDeviceId The remote device Id. + * This function call indicates that the device has been provisioned with operational + * credentials, and is reachable on operational network. At this point, the device is + * available for CASE session establishment. * - * @return CHIP_ERROR CHIP_NO_ERROR on success, or corresponding error + * The function updates the state of device proxy object such that all subsequent messages + * will use secure session established via CASE handshake. */ CHIP_ERROR OperationalDiscoveryComplete(NodeId remoteDeviceId); diff --git a/src/credentials/CHIPCert.h b/src/credentials/CHIPCert.h index 473886af59462c..abf7a17c1a9097 100755 --- a/src/credentials/CHIPCert.h +++ b/src/credentials/CHIPCert.h @@ -645,16 +645,13 @@ CHIP_ERROR ConvertX509CertToChipCert(const uint8_t * x509Cert, uint32_t x509Cert * * The API enforces that the NOC is issued by ICA (if ICA is provided). * - * @param x509NOC Node operational credentials certificate in X.509 DER encoding. - * @param x509ICAC Intermediate CA certificate in X.509 DER encoding. - * @param chipCertArrayBuf Buffer to store converted certificates in CHIP format. - * @param chipCertArrayBufSize The size of the buffer to store converted certificates. - * @param chipCertBufLen The length of the converted certificates. + * @param x509NOC Node operational credentials certificate in X.509 DER encoding. + * @param x509ICAC Intermediate CA certificate in X.509 DER encoding. + * @param chipCertArray Buffer to store converted certificates in CHIP format. * * @return Returns a CHIP_ERROR on error, CHIP_NO_ERROR otherwise **/ -CHIP_ERROR ConvertX509CertsToChipCertArray(const ByteSpan & x509NOC, const ByteSpan & x509ICAC, uint8_t * chipCertArrayBuf, - uint32_t chipCertArrayBufSize, uint32_t & chipCertBufLen); +CHIP_ERROR ConvertX509CertsToChipCertArray(const ByteSpan & x509NOC, const ByteSpan & x509ICAC, MutableByteSpan & chipCertArray); /** * @brief Convert CHIP certificate to the standard X.509 DER encoded certificate. diff --git a/src/credentials/CHIPCertFromX509.cpp b/src/credentials/CHIPCertFromX509.cpp index 6cd0e6e3b7dc49..f2f9fa142bb27b 100644 --- a/src/credentials/CHIPCertFromX509.cpp +++ b/src/credentials/CHIPCertFromX509.cpp @@ -728,14 +728,17 @@ DLL_EXPORT CHIP_ERROR ConvertX509CertToChipCert(const uint8_t * x509Cert, uint32 return err; } -CHIP_ERROR ConvertX509CertsToChipCertArray(const ByteSpan & x509NOC, const ByteSpan & x509ICAC, uint8_t * chipCertArrayBuf, - uint32_t chipCertArrayBufSize, uint32_t & chipCertBufLen) +CHIP_ERROR ConvertX509CertsToChipCertArray(const ByteSpan & x509NOC, const ByteSpan & x509ICAC, MutableByteSpan & chipCertArray) { // NOC is mandatory VerifyOrReturnError(x509NOC.size() > 0, CHIP_ERROR_INVALID_ARGUMENT); TLVWriter writer; - writer.Init(chipCertArrayBuf, chipCertArrayBufSize); + + // We can still generate the certificate if the output chip cert buffer is bigger than UINT32_MAX, + // since generated cert needs less space than UINT32_MAX. + uint32_t chipCertBufLen = (chipCertArray.size() > UINT32_MAX) ? UINT32_MAX : static_cast(chipCertArray.size()); + writer.Init(chipCertArray.data(), chipCertBufLen); TLVType outerContainer; ReturnErrorOnFailure(writer.StartContainer(AnonymousTag, kTLVType_Array, outerContainer)); @@ -760,7 +763,8 @@ CHIP_ERROR ConvertX509CertsToChipCertArray(const ByteSpan & x509NOC, const ByteS ReturnErrorOnFailure(writer.EndContainer(outerContainer)); ReturnErrorOnFailure(writer.Finalize()); - chipCertBufLen = writer.GetLengthWritten(); + ReturnErrorCodeIf(writer.GetLengthWritten() > chipCertBufLen, CHIP_ERROR_INTERNAL); + chipCertArray.reduce_size(writer.GetLengthWritten()); return CHIP_NO_ERROR; } diff --git a/src/credentials/tests/TestChipCert.cpp b/src/credentials/tests/TestChipCert.cpp index a404afafffc8ab..544936fbb20aaa 100644 --- a/src/credentials/tests/TestChipCert.cpp +++ b/src/credentials/tests/TestChipCert.cpp @@ -1009,20 +1009,22 @@ static void TestChipCert_X509ToChipArray(nlTestSuite * inSuite, void * inContext sizeof(noc_cert), noc_len) == CHIP_NO_ERROR); uint8_t outCertBuf[kTestCertBufSize * 2]; - uint32_t outCertLen; + MutableByteSpan outCert(outCertBuf, sizeof(outCertBuf)); NL_TEST_ASSERT(inSuite, - ConvertX509CertsToChipCertArray(ByteSpan(noc_cert, noc_len), ByteSpan(ica_cert, ica_len), outCertBuf, - sizeof(outCertBuf), outCertLen) == CHIP_NO_ERROR); + ConvertX509CertsToChipCertArray(ByteSpan(noc_cert, noc_len), ByteSpan(ica_cert, ica_len), outCert) == + CHIP_NO_ERROR); + NL_TEST_ASSERT(inSuite, outCert.size() <= sizeof(outCertBuf)); ChipCertificateSet certSet; NL_TEST_ASSERT(inSuite, certSet.Init(3, kTestCertBufSize * 3) == CHIP_NO_ERROR); NL_TEST_ASSERT(inSuite, - certSet.LoadCerts(outCertBuf, outCertLen, BitFlags(CertDecodeFlags::kGenerateTBSHash)) == - CHIP_NO_ERROR); + certSet.LoadCerts(outCert.data(), static_cast(outCert.size()), + BitFlags(CertDecodeFlags::kGenerateTBSHash)) == CHIP_NO_ERROR); static uint8_t rootCertBuf[kTestCertBufSize]; + uint32_t outCertLen; NL_TEST_ASSERT(inSuite, ConvertX509CertToChipCert(root_cert, root_len, rootCertBuf, sizeof(rootCertBuf), outCertLen) == CHIP_NO_ERROR); NL_TEST_ASSERT( @@ -1071,16 +1073,17 @@ static void TestChipCert_X509ToChipArrayNoICA(nlTestSuite * inSuite, void * inCo uint8_t outCertBuf[kTestCertBufSize * 2]; uint32_t outCertLen; + MutableByteSpan outCert(outCertBuf, sizeof(outCertBuf)); NL_TEST_ASSERT(inSuite, - ConvertX509CertsToChipCertArray(ByteSpan(noc_cert, noc_len), ByteSpan(nullptr, 0), outCertBuf, - sizeof(outCertBuf), outCertLen) == CHIP_NO_ERROR); + ConvertX509CertsToChipCertArray(ByteSpan(noc_cert, noc_len), ByteSpan(nullptr, 0), outCert) == CHIP_NO_ERROR); + NL_TEST_ASSERT(inSuite, outCert.size() <= sizeof(outCertBuf)); ChipCertificateSet certSet; NL_TEST_ASSERT(inSuite, certSet.Init(3, kTestCertBufSize * 3) == CHIP_NO_ERROR); NL_TEST_ASSERT(inSuite, - certSet.LoadCerts(outCertBuf, outCertLen, BitFlags(CertDecodeFlags::kGenerateTBSHash)) == - CHIP_NO_ERROR); + certSet.LoadCerts(outCert.data(), static_cast(outCert.size()), + BitFlags(CertDecodeFlags::kGenerateTBSHash)) == CHIP_NO_ERROR); static uint8_t rootCertBuf[kTestCertBufSize]; @@ -1142,16 +1145,16 @@ static void TestChipCert_X509ToChipArrayErrorScenarios(nlTestSuite * inSuite, vo sizeof(noc_cert), noc_len) == CHIP_NO_ERROR); uint8_t outCertBuf[kTestCertBufSize * 2]; - uint32_t outCertLen; + MutableByteSpan outCert(outCertBuf, sizeof(outCertBuf)); // Test that NOC is mandatory NL_TEST_ASSERT(inSuite, - ConvertX509CertsToChipCertArray(ByteSpan(nullptr, 0), ByteSpan(ica_cert, ica_len), outCertBuf, - sizeof(outCertBuf), outCertLen) == CHIP_ERROR_INVALID_ARGUMENT); + ConvertX509CertsToChipCertArray(ByteSpan(nullptr, 0), ByteSpan(ica_cert, ica_len), outCert) == + CHIP_ERROR_INVALID_ARGUMENT); // Test that NOC issuer must match ICA NL_TEST_ASSERT(inSuite, - ConvertX509CertsToChipCertArray(ByteSpan(noc_cert, noc_len), ByteSpan(root_cert, root_len), outCertBuf, - sizeof(outCertBuf), outCertLen) == CHIP_ERROR_INVALID_ARGUMENT); + ConvertX509CertsToChipCertArray(ByteSpan(noc_cert, noc_len), ByteSpan(root_cert, root_len), outCert) == + CHIP_ERROR_INVALID_ARGUMENT); X509CertRequestParams ica_params_wrong_fabric = { 1234, 0xabcdabcd, 631161876, 729942000, true, 0x9999, false, 0 }; @@ -1160,8 +1163,8 @@ static void TestChipCert_X509ToChipArrayErrorScenarios(nlTestSuite * inSuite, vo ica_len) == CHIP_NO_ERROR); // Test that NOC fabric must match ICA fabric NL_TEST_ASSERT(inSuite, - ConvertX509CertsToChipCertArray(ByteSpan(noc_cert, noc_len), ByteSpan(ica_cert, ica_len), outCertBuf, - sizeof(outCertBuf), outCertLen) == CHIP_ERROR_INVALID_ARGUMENT); + ConvertX509CertsToChipCertArray(ByteSpan(noc_cert, noc_len), ByteSpan(ica_cert, ica_len), outCert) == + CHIP_ERROR_INVALID_ARGUMENT); } /** diff --git a/src/transport/AdminPairingTable.cpp b/src/transport/AdminPairingTable.cpp index 3ff3946c536bdf..ceb554f473e4a3 100644 --- a/src/transport/AdminPairingTable.cpp +++ b/src/transport/AdminPairingTable.cpp @@ -213,7 +213,7 @@ CHIP_ERROR AdminPairingInfo::SetRootCert(const ByteSpan & cert) return CHIP_NO_ERROR; } - VerifyOrReturnError(cert.size() <= kMaxChipCertSize, CHIP_ERROR_INVALID_ARGUMENT); + VerifyOrReturnError(cert.size() <= kMaxCHIPCertLength, CHIP_ERROR_INVALID_ARGUMENT); if (mRootCertLen != 0 && mRootCertAllocatedLen < cert.size()) { ReleaseRootCert(); @@ -252,7 +252,7 @@ CHIP_ERROR AdminPairingInfo::SetOperationalCert(const ByteSpan & cert) } // There could be two certs in the set -> ICA and NOC - VerifyOrReturnError(cert.size() <= kMaxChipCertSize * 2, CHIP_ERROR_INVALID_ARGUMENT); + VerifyOrReturnError(cert.size() <= kMaxCHIPCertLength * 2, CHIP_ERROR_INVALID_ARGUMENT); if (mOpCertLen != 0 && mOpCertAllocatedLen < cert.size()) { ReleaseOperationalCert(); @@ -275,7 +275,7 @@ CHIP_ERROR AdminPairingInfo::GetCredentials(OperationalCredentialSet & credentia CertificateKeyId & rootKeyId) { constexpr uint8_t kMaxNumCertsInOpCreds = 3; - ReturnErrorOnFailure(certificates.Init(kMaxNumCertsInOpCreds, kMaxChipCertSize * kMaxNumCertsInOpCreds)); + ReturnErrorOnFailure(certificates.Init(kMaxNumCertsInOpCreds, kMaxCHIPCertLength * kMaxNumCertsInOpCreds)); ReturnErrorOnFailure( certificates.LoadCert(mRootCert, mRootCertLen, diff --git a/src/transport/AdminPairingTable.h b/src/transport/AdminPairingTable.h index b25079da1090ef..1b2126c762c29f 100644 --- a/src/transport/AdminPairingTable.h +++ b/src/transport/AdminPairingTable.h @@ -43,8 +43,6 @@ static constexpr uint8_t kFabricLabelMaxLengthInBytes = 32; constexpr char kAdminTableKeyPrefix[] = "CHIPAdmin"; constexpr char kAdminTableCountKey[] = "CHIPAdminNextId"; -constexpr uint16_t kMaxChipCertSize = 600; - struct AccessControlList { uint32_t placeholder; @@ -193,9 +191,9 @@ class DLL_EXPORT AdminPairingInfo uint16_t mOpCertLen; /* This field is serialized in LittleEndian byte order */ Crypto::P256SerializedKeypair mOperationalKey; - uint8_t mRootCert[kMaxChipCertSize]; + uint8_t mRootCert[Credentials::kMaxCHIPCertLength]; // The operationa credentials set can have up to two certs -> ICAC and NOC - uint8_t mOperationalCert[kMaxChipCertSize * 2]; + uint8_t mOperationalCert[Credentials::kMaxCHIPCertLength * 2]; char mFabricLabel[kFabricLabelMaxLengthInBytes + 1] = { '\0' }; }; }; From 797d24d77b9ad24bafdb4b49c508db7539a6d936 Mon Sep 17 00:00:00 2001 From: Pankaj Garg Date: Thu, 17 Jun 2021 11:47:38 -0700 Subject: [PATCH 08/12] add ReserveUpTo --- src/controller/CHIPDeviceController.cpp | 5 +---- .../secure_channel/SessionIDAllocator.cpp | 17 +++++++++++++++++ .../secure_channel/SessionIDAllocator.h | 1 + .../tests/TestSessionIDAllocator.cpp | 9 +++++++++ 4 files changed, 28 insertions(+), 4 deletions(-) diff --git a/src/controller/CHIPDeviceController.cpp b/src/controller/CHIPDeviceController.cpp index e80422f9fc0edd..d83734d7f39f3b 100644 --- a/src/controller/CHIPDeviceController.cpp +++ b/src/controller/CHIPDeviceController.cpp @@ -815,10 +815,7 @@ CHIP_ERROR DeviceCommissioner::Init(NodeId localDeviceId, CommissionerInitParams { nextKeyID = 0; } - for (uint16_t i = 0; i < nextKeyID; i++) - { - ReturnErrorOnFailure(mIDAllocator.Reserve(i)); - } + ReturnErrorOnFailure(mIDAllocator.ReserveUpTo(nextKeyID)); mPairingDelegate = params.pairingDelegate; return CHIP_NO_ERROR; diff --git a/src/protocols/secure_channel/SessionIDAllocator.cpp b/src/protocols/secure_channel/SessionIDAllocator.cpp index ae097ce089664f..eadb736cde59eb 100644 --- a/src/protocols/secure_channel/SessionIDAllocator.cpp +++ b/src/protocols/secure_channel/SessionIDAllocator.cpp @@ -54,6 +54,23 @@ CHIP_ERROR SessionIDAllocator::Reserve(uint16_t id) return CHIP_NO_ERROR; } +CHIP_ERROR SessionIDAllocator::ReserveUpTo(uint16_t id) +{ + VerifyOrReturnError(id < kMaxSessionID, CHIP_ERROR_NO_MEMORY); + if (id >= mNextAvailable) + { + mNextAvailable = id; + mNextAvailable++; + } + + // TODO - Update ReserveUpTo to mark all IDs in use + // Current SessionIDAllocator only tracks the smallest unused session ID. + // If/when we change it to track all in use IDs, we should also update ReserveUpTo + // to reserve all individual session IDs, instead of just setting the mNextAvailable. + + return CHIP_NO_ERROR; +} + uint16_t SessionIDAllocator::Peek() { return mNextAvailable; diff --git a/src/protocols/secure_channel/SessionIDAllocator.h b/src/protocols/secure_channel/SessionIDAllocator.h index 88c705c7af4a4b..577d6d3f657695 100644 --- a/src/protocols/secure_channel/SessionIDAllocator.h +++ b/src/protocols/secure_channel/SessionIDAllocator.h @@ -30,6 +30,7 @@ class SessionIDAllocator CHIP_ERROR Allocate(uint16_t & id); void Free(uint16_t id); CHIP_ERROR Reserve(uint16_t id); + CHIP_ERROR ReserveUpTo(uint16_t id); uint16_t Peek(); private: diff --git a/src/protocols/secure_channel/tests/TestSessionIDAllocator.cpp b/src/protocols/secure_channel/tests/TestSessionIDAllocator.cpp index 2e09d537bc2822..be12be977e9f26 100644 --- a/src/protocols/secure_channel/tests/TestSessionIDAllocator.cpp +++ b/src/protocols/secure_channel/tests/TestSessionIDAllocator.cpp @@ -88,6 +88,14 @@ void TestSessionIDAllocator_Reserve(nlTestSuite * inSuite, void * inContext) NL_TEST_ASSERT(inSuite, allocator.Peek() == 101); } +void TestSessionIDAllocator_ReserveUpTo(nlTestSuite * inSuite, void * inContext) +{ + SessionIDAllocator allocator; + + allocator.ReserveUpTo(100); + NL_TEST_ASSERT(inSuite, allocator.Peek() == 101); +} + // Test Suite /** @@ -99,6 +107,7 @@ static const nlTest sTests[] = NL_TEST_DEF("SessionIDAllocator_Allocate", TestSessionIDAllocator_Allocate), NL_TEST_DEF("SessionIDAllocator_Free", TestSessionIDAllocator_Free), NL_TEST_DEF("SessionIDAllocator_Reserve", TestSessionIDAllocator_Reserve), + NL_TEST_DEF("SessionIDAllocator_ReserveUpTo", TestSessionIDAllocator_ReserveUpTo), NL_TEST_SENTINEL() }; From 13de1078aaae06826c000213effda7a8f94a2d07 Mon Sep 17 00:00:00 2001 From: Pankaj Garg Date: Thu, 17 Jun 2021 16:58:21 -0700 Subject: [PATCH 09/12] release resouces on cleanup, and reduce message timeout --- src/protocols/secure_channel/CASEServer.cpp | 9 +++++++-- src/protocols/secure_channel/CASESession.cpp | 4 ++-- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/src/protocols/secure_channel/CASEServer.cpp b/src/protocols/secure_channel/CASEServer.cpp index e3368e4f5a58e6..1b04df61ee02aa 100644 --- a/src/protocols/secure_channel/CASEServer.cpp +++ b/src/protocols/secure_channel/CASEServer.cpp @@ -62,9 +62,13 @@ CHIP_ERROR CASEServer::InitCASEHandshake(Messaging::ExchangeContext * ec) // ReturnErrorCodeIf(mAdminId == Transport::kUndefinedAdminId, CHIP_ERROR_INVALID_ARGUMENT); mAdminId = 0; - mAdmins->LoadFromStorage(mAdminId); - Transport::AdminPairingInfo * admin = mAdmins->FindAdminWithId(mAdminId); + + if (admin == nullptr) + { + ReturnErrorOnFailure(mAdmins->LoadFromStorage(mAdminId)); + admin = mAdmins->FindAdminWithId(mAdminId); + } ReturnErrorCodeIf(admin == nullptr, CHIP_ERROR_INVALID_ARGUMENT); ReturnErrorOnFailure(admin->GetCredentials(mCredentials, mCertificates, mRootKeyId)); @@ -100,6 +104,7 @@ void CASEServer::Cleanup() mExchangeManager->RegisterUnsolicitedMessageHandlerForType(Protocols::SecureChannel::MsgType::CASE_SigmaR1, this); mAdminId = Transport::kUndefinedAdminId; mCredentials.Release(); + mCertificates.Release(); } void CASEServer::OnSessionEstablishmentError(CHIP_ERROR err) diff --git a/src/protocols/secure_channel/CASESession.cpp b/src/protocols/secure_channel/CASESession.cpp index d3e42315774bb5..100e4d6cb6152b 100644 --- a/src/protocols/secure_channel/CASESession.cpp +++ b/src/protocols/secure_channel/CASESession.cpp @@ -67,10 +67,10 @@ using HKDF_sha_crypto = HKDF_shaHSM; using HKDF_sha_crypto = HKDF_sha; #endif -// Wait at most 30 seconds for the response from the peer. +// Wait at most 10 seconds for the response from the peer. // This timeout value assumes the underlying transport is reliable. // The session establishment fails if the response is not received within timeout window. -static constexpr ExchangeContext::Timeout kSigma_Response_Timeout = 30000; +static constexpr ExchangeContext::Timeout kSigma_Response_Timeout = 10000; CASESession::CASESession() { From bf54cb4b8d097a46266d722d4f2f4041c626bea9 Mon Sep 17 00:00:00 2001 From: Pankaj Garg Date: Fri, 18 Jun 2021 06:26:21 -0700 Subject: [PATCH 10/12] release resources on cleanup and init. --- src/protocols/secure_channel/CASEServer.cpp | 7 +++---- src/protocols/secure_channel/CASESession.cpp | 5 +++++ src/protocols/secure_channel/CASESession.h | 6 ++++-- 3 files changed, 12 insertions(+), 6 deletions(-) diff --git a/src/protocols/secure_channel/CASEServer.cpp b/src/protocols/secure_channel/CASEServer.cpp index 1b04df61ee02aa..74c6d09e6bb770 100644 --- a/src/protocols/secure_channel/CASEServer.cpp +++ b/src/protocols/secure_channel/CASEServer.cpp @@ -55,6 +55,8 @@ CHIP_ERROR CASEServer::InitCASEHandshake(Messaging::ExchangeContext * ec) { ReturnErrorCodeIf(ec == nullptr, CHIP_ERROR_INVALID_ARGUMENT); + Cleanup(); + // TODO - Use PK of the root CA for the initiator to figure out the admin. mAdminId = ec->GetSecureSession().GetAdminId(); @@ -93,18 +95,15 @@ void CASEServer::OnMessageReceived(Messaging::ExchangeContext * ec, const Packet mPairingSession.OnMessageReceived(ec, packetHeader, payloadHeader, std::move(payload)); // TODO - Enable multiple concurrent CASE session establishment - // This will prevent CASEServer to process another CASE session establishment request until the current - // one completes (successfully or failed) - mExchangeManager->UnregisterUnsolicitedMessageHandlerForType(Protocols::SecureChannel::MsgType::CASE_SigmaR1); } void CASEServer::Cleanup() { // Let's re-register for CASE SigmaR1 message, so that the next CASE session setup request can be processed. - mExchangeManager->RegisterUnsolicitedMessageHandlerForType(Protocols::SecureChannel::MsgType::CASE_SigmaR1, this); mAdminId = Transport::kUndefinedAdminId; mCredentials.Release(); mCertificates.Release(); + mPairingSession.Clear(); } void CASEServer::OnSessionEstablishmentError(CHIP_ERROR err) diff --git a/src/protocols/secure_channel/CASESession.cpp b/src/protocols/secure_channel/CASESession.cpp index 100e4d6cb6152b..be3bf1664bf374 100644 --- a/src/protocols/secure_channel/CASESession.cpp +++ b/src/protocols/secure_channel/CASESession.cpp @@ -969,6 +969,11 @@ CHIP_ERROR CASESession::FindValidTrustedRoot(const uint8_t ** msgIterator, uint3 if (mOpCredSet->IsTrustedRootIn(trustedRoot[i])) { + if (mTrustedRootId.mId != nullptr) + { + chip::Platform::MemoryFree(const_cast(mTrustedRootId.mId)); + mTrustedRootId.mId = nullptr; + } mTrustedRootId.mId = reinterpret_cast(chip::Platform::MemoryAlloc(kTrustedRootIdSize)); VerifyOrReturnError(mTrustedRootId.mId != nullptr, CHIP_ERROR_NO_MEMORY); diff --git a/src/protocols/secure_channel/CASESession.h b/src/protocols/secure_channel/CASESession.h index 41b38e6d9ab190..67e9bf7d86a8b5 100644 --- a/src/protocols/secure_channel/CASESession.h +++ b/src/protocols/secure_channel/CASESession.h @@ -188,6 +188,10 @@ class DLL_EXPORT CASESession : public Messaging::ExchangeDelegate, public Pairin return &mMessageDispatch; } + /** @brief This function zeroes out and resets the memory used by the object. + **/ + void Clear(); + private: enum SigmaErrorType : uint8_t { @@ -235,8 +239,6 @@ class DLL_EXPORT CASESession : public Messaging::ExchangeDelegate, public Pairin // TODO: Remove this and replace with system method to retrieve current time CHIP_ERROR SetEffectiveTime(void); - void Clear(); - CHIP_ERROR ValidateReceivedMessage(Messaging::ExchangeContext * ec, const PacketHeader & packetHeader, const PayloadHeader & payloadHeader, System::PacketBufferHandle & msg); From 5373ba593e0ae0796ba83f1c00fbded1e1f6a44f Mon Sep 17 00:00:00 2001 From: Pankaj Garg Date: Tue, 22 Jun 2021 08:22:06 -0700 Subject: [PATCH 11/12] Remove sleep from ModelCommand --- .../commands/clusters/ModelCommand.cpp | 49 ++++------ .../commands/clusters/ModelCommand.h | 12 ++- .../chip-tool/commands/discover/Commands.h | 4 +- .../commands/pairing/PairingCommand.cpp | 24 ++++- .../commands/pairing/PairingCommand.h | 1 + src/controller/CHIPDevice.cpp | 60 +++++++++++- src/controller/CHIPDevice.h | 36 +++++-- src/controller/CHIPDeviceController.cpp | 98 +++++++++++-------- src/controller/CHIPDeviceController.h | 44 +++++---- .../Framework/CHIP/CHIPDeviceController.mm | 11 +-- 10 files changed, 223 insertions(+), 116 deletions(-) diff --git a/examples/chip-tool/commands/clusters/ModelCommand.cpp b/examples/chip-tool/commands/clusters/ModelCommand.cpp index f04b7e275c9268..f6ecc96811816d 100644 --- a/examples/chip-tool/commands/clusters/ModelCommand.cpp +++ b/examples/chip-tool/commands/clusters/ModelCommand.cpp @@ -37,25 +37,6 @@ void DispatchSingleClusterCommand(chip::ClusterId aClusterId, chip::CommandId aC "Default DispatchSingleClusterCommand is called, this should be replaced by actual dispatched for cluster commands"); } -CHIP_ERROR WaitForSessionSetup(chip::Controller::Device * device) -{ - constexpr time_t kWaitPerIterationSec = 1; - constexpr uint16_t kIterationCount = 5; - - struct timespec sleep_time; - sleep_time.tv_sec = kWaitPerIterationSec; - sleep_time.tv_nsec = 0; - - for (uint32_t i = 0; i < kIterationCount && device->IsSessionSetupInProgress(); i++) - { - nanosleep(&sleep_time, nullptr); - } - - ReturnErrorCodeIf(!device->IsSecureConnected(), CHIP_ERROR_TIMEOUT); - - return CHIP_NO_ERROR; -} - CHIP_ERROR ModelCommand::Run(NodeId localId, NodeId remoteId) { CHIP_ERROR err = CHIP_NO_ERROR; @@ -70,18 +51,10 @@ CHIP_ERROR ModelCommand::Run(NodeId localId, NodeId remoteId) { chip::DeviceLayer::StackLock lock; - err = GetExecContext()->commissioner->GetDevice(remoteId, &mDevice); - VerifyOrExit(err == CHIP_NO_ERROR, ChipLogError(chipTool, "Init failure! No pairing for device: %" PRIu64, localId)); - - // TODO - Implement notification from device object when the secure session is available. - // Current code is polling the device object to check for secure session availability. This should - // be updated to a notification/callback mechanism. - err = WaitForSessionSetup(mDevice); + err = GetExecContext()->commissioner->GetConnectedDevice(remoteId, &mOnDeviceConnectedCallback, + &mOnDeviceConnectionFailureCallback); VerifyOrExit(err == CHIP_NO_ERROR, - ChipLogError(chipTool, "Timed out while waiting for session setup for device: %" PRIu64, localId)); - - err = SendCommand(mDevice, mEndPointId); - VerifyOrExit(err == CHIP_NO_ERROR, ChipLogError(chipTool, "Failed to send message: %s", ErrorStr(err))); + ChipLogError(chipTool, "Failed in initiating connection to the device: %" PRIu64 ", error %d", remoteId, err)); } WaitForResponse(kWaitDurationInSeconds); @@ -91,3 +64,19 @@ CHIP_ERROR ModelCommand::Run(NodeId localId, NodeId remoteId) exit: return err; } + +void ModelCommand::OnDeviceConnectedFn(void * context, chip::Controller::Device * device) +{ + ModelCommand * command = reinterpret_cast(context); + VerifyOrReturn(command != nullptr, + ChipLogError(chipTool, "Device connected, but cannot send the command, as the context is null")); + command->SendCommand(device, command->mEndPointId); +} + +void ModelCommand::OnDeviceConnectionFailureFn(void * context, NodeId deviceId, CHIP_ERROR error) +{ + ModelCommand * command = reinterpret_cast(context); + ChipLogError(chipTool, "Failed in connecting to the device %" PRIu64 ". Error %d", deviceId, error); + VerifyOrReturn(command != nullptr, ChipLogError(chipTool, "ModelCommand context is null")); + command->SetCommandExitStatus(false); +} diff --git a/examples/chip-tool/commands/clusters/ModelCommand.h b/examples/chip-tool/commands/clusters/ModelCommand.h index f35f162468f430..3583a3fbe591e4 100644 --- a/examples/chip-tool/commands/clusters/ModelCommand.h +++ b/examples/chip-tool/commands/clusters/ModelCommand.h @@ -31,7 +31,10 @@ class ModelCommand : public Command { public: - ModelCommand(const char * commandName) : Command(commandName) {} + ModelCommand(const char * commandName) : + Command(commandName), mOnDeviceConnectedCallback(OnDeviceConnectedFn, this), + mOnDeviceConnectionFailureCallback(OnDeviceConnectionFailureFn, this) + {} void AddArguments() { AddArgument("endpoint-id", CHIP_ZCL_ENDPOINT_MIN, CHIP_ZCL_ENDPOINT_MAX, &mEndPointId); } @@ -41,6 +44,11 @@ class ModelCommand : public Command virtual CHIP_ERROR SendCommand(ChipDevice * device, uint8_t endPointId) = 0; private: - ChipDevice * mDevice; uint8_t mEndPointId; + + static void OnDeviceConnectedFn(void * context, chip::Controller::Device * device); + static void OnDeviceConnectionFailureFn(void * context, NodeId deviceId, CHIP_ERROR error); + + chip::Callback::Callback mOnDeviceConnectedCallback; + chip::Callback::Callback mOnDeviceConnectionFailureCallback; }; diff --git a/examples/chip-tool/commands/discover/Commands.h b/examples/chip-tool/commands/discover/Commands.h index 9b81043e30c7c6..ed743f96a1c8b2 100644 --- a/examples/chip-tool/commands/discover/Commands.h +++ b/examples/chip-tool/commands/discover/Commands.h @@ -66,10 +66,8 @@ class Update : public DiscoverCommand /////////// DiscoverCommand Interface ///////// CHIP_ERROR RunCommand(NodeId remoteId, uint64_t fabricId) override { - ChipDevice * device; - ReturnErrorOnFailure(GetExecContext()->commissioner->GetDevice(remoteId, &device)); ChipLogProgress(chipTool, "Mdns: Updating NodeId: %" PRIx64 " FabricId: %" PRIx64 " ...", remoteId, fabricId); - return GetExecContext()->commissioner->UpdateDevice(device, fabricId); + return GetExecContext()->commissioner->UpdateDevice(remoteId, fabricId); } /////////// DeviceAddressUpdateDelegate Interface ///////// diff --git a/examples/chip-tool/commands/pairing/PairingCommand.cpp b/examples/chip-tool/commands/pairing/PairingCommand.cpp index 21fc0cc3d033b9..cc2c6640337422 100644 --- a/examples/chip-tool/commands/pairing/PairingCommand.cpp +++ b/examples/chip-tool/commands/pairing/PairingCommand.cpp @@ -162,6 +162,20 @@ void PairingCommand::OnPairingDeleted(CHIP_ERROR err) SetCommandExitStatus(err == CHIP_NO_ERROR); } +void PairingCommand::OnCommissioningComplete(NodeId nodeId, CHIP_ERROR err) +{ + if (err == CHIP_NO_ERROR) + { + ChipLogProgress(chipTool, "Device commissioning completed with success"); + } + else + { + ChipLogProgress(chipTool, "Device commissioning Failure: %s", ErrorStr(err)); + } + + SetCommandExitStatus(err == CHIP_NO_ERROR); +} + CHIP_ERROR PairingCommand::SetupNetwork() { @@ -316,13 +330,17 @@ void PairingCommand::OnEnableNetworkResponse(void * context, uint8_t errorCode, CHIP_ERROR PairingCommand::UpdateNetworkAddress() { - ReturnErrorOnFailure(GetExecContext()->commissioner->GetDevice(mRemoteId, &mDevice)); ChipLogProgress(chipTool, "Mdns: Updating NodeId: %" PRIx64 " FabricId: %" PRIx64 " ...", mRemoteId, mFabricId); - return GetExecContext()->commissioner->UpdateDevice(mDevice, mFabricId); + return GetExecContext()->commissioner->UpdateDevice(mRemoteId, mFabricId); } void PairingCommand::OnAddressUpdateComplete(NodeId nodeId, CHIP_ERROR err) { ChipLogProgress(chipTool, "OnAddressUpdateComplete: %s", ErrorStr(err)); - SetCommandExitStatus(CHIP_NO_ERROR == err); + if (err != CHIP_NO_ERROR) + { + // Set exit status only if the address update failed. + // Otherwise wait for OnCommissioningComplete() callback. + SetCommandExitStatus(false); + } } diff --git a/examples/chip-tool/commands/pairing/PairingCommand.h b/examples/chip-tool/commands/pairing/PairingCommand.h index 7fdc1375fb45d9..abb33af9d0c4e0 100644 --- a/examples/chip-tool/commands/pairing/PairingCommand.h +++ b/examples/chip-tool/commands/pairing/PairingCommand.h @@ -103,6 +103,7 @@ class PairingCommand : public Command, void OnStatusUpdate(chip::Controller::DevicePairingDelegate::Status status) override; void OnPairingComplete(CHIP_ERROR error) override; void OnPairingDeleted(CHIP_ERROR error) override; + void OnCommissioningComplete(NodeId deviceId, CHIP_ERROR error) override; /////////// DeviceAddressUpdateDelegate Interface ///////// void OnAddressUpdateComplete(NodeId nodeId, CHIP_ERROR error) override; diff --git a/src/controller/CHIPDevice.cpp b/src/controller/CHIPDevice.cpp index 43352e28fe1848..b08d9d1cb46524 100644 --- a/src/controller/CHIPDevice.cpp +++ b/src/controller/CHIPDevice.cpp @@ -437,6 +437,11 @@ CHIP_ERROR Device::LoadSecureSessionParameters(ResetTransport resetNeeded) ExitNow(err = CHIP_ERROR_INCORRECT_STATE); } + if (mState == ConnectionState::Connecting) + { + ExitNow(err = CHIP_NO_ERROR); + } + if (resetNeeded == ResetTransport::kYes) { err = mTransportMgr->ResetTransport( @@ -502,8 +507,6 @@ void Device::OperationalCertProvisioned() mSessionManager->ExpirePairing(mSecureSession); mState = ConnectionState::NotConnected; } - - WarmupCASESession(); } CHIP_ERROR Device::WarmupCASESession() @@ -534,6 +537,17 @@ void Device::OnSessionEstablishmentError(CHIP_ERROR error) { mState = ConnectionState::NotConnected; mIDAllocator->Free(mCASESession.GetLocalKeyId()); + + Cancelable ready; + mConnectionFailure.DequeueAll(ready); + while (ready.mNext != &ready) + { + Callback::Callback * cb = + Callback::Callback::FromCancelable(ready.mNext); + + cb->Cancel(); + cb->mCall(cb->mContext, GetDeviceId(), error); + } } void Device::OnSessionEstablished() @@ -548,6 +562,48 @@ void Device::OnSessionEstablished() OnSessionEstablishmentError(err); return; } + + Cancelable ready; + mConnectionSuccess.DequeueAll(ready); + while (ready.mNext != &ready) + { + Callback::Callback * cb = Callback::Callback::FromCancelable(ready.mNext); + + cb->Cancel(); + cb->mCall(cb->mContext, this); + } +} + +CHIP_ERROR Device::EstablishConnectivity(Callback::Callback * onConnection, + Callback::Callback * onFailure) +{ + bool loadedSecureSession = false; + ReturnErrorOnFailure(LoadSecureSessionParametersIfNeeded(loadedSecureSession)); + + if (loadedSecureSession) + { + if (IsOperationalCertProvisioned()) + { + if (onConnection != nullptr) + { + mConnectionSuccess.Enqueue(onConnection->Cancel()); + } + + if (onFailure != nullptr) + { + mConnectionFailure.Enqueue(onFailure->Cancel()); + } + } + else + { + if (onConnection != nullptr) + { + onConnection->mCall(onConnection->mContext, this); + } + } + } + + return CHIP_NO_ERROR; } void Device::AddResponseHandler(uint8_t seqNum, Callback::Cancelable * onSuccessCallback, Callback::Cancelable * onFailureCallback) diff --git a/src/controller/CHIPDevice.h b/src/controller/CHIPDevice.h index 4644b0289bcfe1..e5f8eda3ed6471 100644 --- a/src/controller/CHIPDevice.h +++ b/src/controller/CHIPDevice.h @@ -87,6 +87,11 @@ struct ControllerDeviceInitParams #endif }; +class Device; + +typedef void (*OnDeviceConnected)(void * context, Device * device); +typedef void (*OnDeviceConnectionFailure)(void * context, NodeId deviceId, CHIP_ERROR error); + class DLL_EXPORT Device : public Messaging::ExchangeDelegate, public SessionEstablishmentDelegate { public: @@ -352,15 +357,22 @@ class DLL_EXPORT Device : public Messaging::ExchangeDelegate, public SessionEsta ByteSpan GetCSRNonce() const { return ByteSpan(mCSRNonce, sizeof(mCSRNonce)); } - /** - * @brief - * This function triggers CASE session setup if the device has been provisioned with - * operational credentials, and there is no currently active session. + /* + * This function can be called to establish a secure session with the device. + * + * If the device doesn't have operational credentials, and is under commissioning process, + * PASE keys will be used for secure session. + * + * If the device has been commissioned and has operational credentials, CASE session + * setup will be triggered. * - * @return CHIP_NO_ERROR if the session setup was triggered or a session is already available. + * On establishing the session, the callback function `onConnection` will be called. If the + * session setup fails, `onFailure` will be called. + * + * If the session already exists, `onConnection` will be called immediately. */ - - CHIP_ERROR WarmupCASESession(); + CHIP_ERROR EstablishConnectivity(Callback::Callback * onConnection, + Callback::Callback * onFailure); private: enum class ConnectionState @@ -430,6 +442,13 @@ class DLL_EXPORT Device : public Messaging::ExchangeDelegate, public SessionEsta */ CHIP_ERROR LoadSecureSessionParametersIfNeeded(bool & didLoad); + /** + * This function triggers CASE session setup if the device has been provisioned with + * operational credentials, and there is no currently active session. + */ + + CHIP_ERROR WarmupCASESession(); + uint16_t mListenPort; Transport::AdminId mAdminId = Transport::kUndefinedAdminId; @@ -445,6 +464,9 @@ class DLL_EXPORT Device : public Messaging::ExchangeDelegate, public SessionEsta uint8_t mCSRNonce[kOpCSRNonceLength]; SessionIDAllocator * mIDAllocator = nullptr; + + Callback::CallbackDeque mConnectionSuccess; + Callback::CallbackDeque mConnectionFailure; }; /** diff --git a/src/controller/CHIPDeviceController.cpp b/src/controller/CHIPDeviceController.cpp index d83734d7f39f3b..6e28c74943d545 100644 --- a/src/controller/CHIPDeviceController.cpp +++ b/src/controller/CHIPDeviceController.cpp @@ -408,39 +408,6 @@ CHIP_ERROR DeviceController::SetUdpListenPort(uint16_t listenPort) return CHIP_NO_ERROR; } -CHIP_ERROR DeviceController::GetDevice(NodeId deviceId, const SerializedDevice & deviceInfo, Device ** out_device) -{ - Device * device = nullptr; - - VerifyOrReturnError(out_device != nullptr, CHIP_ERROR_INVALID_ARGUMENT); - uint16_t index = FindDeviceIndex(deviceId); - - if (index < kNumMaxActiveDevices) - { - device = &mActiveDevices[index]; - } - else - { - VerifyOrReturnError(mPairedDevices.Contains(deviceId), CHIP_ERROR_NOT_CONNECTED); - - index = GetInactiveDeviceIndex(); - VerifyOrReturnError(index < kNumMaxActiveDevices, CHIP_ERROR_NO_MEMORY); - device = &mActiveDevices[index]; - - CHIP_ERROR err = device->Deserialize(deviceInfo); - if (err != CHIP_NO_ERROR) - { - ReleaseDevice(device); - ReturnErrorOnFailure(err); - } - - device->Init(GetControllerDeviceInitParams(), mListenPort, mAdminId); - } - - *out_device = device; - return CHIP_NO_ERROR; -} - CHIP_ERROR DeviceController::GetDevice(NodeId deviceId, Device ** out_device) { CHIP_ERROR err = CHIP_NO_ERROR; @@ -478,12 +445,6 @@ CHIP_ERROR DeviceController::GetDevice(NodeId deviceId, Device ** out_device) VerifyOrExit(err == CHIP_NO_ERROR, ReleaseDevice(device)); device->Init(GetControllerDeviceInitParams(), mListenPort, mAdminId); - - if (device->IsOperationalCertProvisioned()) - { - err = device->WarmupCASESession(); - SuccessOrExit(err); - } } } @@ -497,10 +458,48 @@ CHIP_ERROR DeviceController::GetDevice(NodeId deviceId, Device ** out_device) return err; } -CHIP_ERROR DeviceController::UpdateDevice(Device * device, uint64_t fabricId) +bool DeviceController::DoesDevicePairingExist(const PeerId & deviceId) +{ + if (InitializePairedDeviceList() == CHIP_NO_ERROR) + { + return mPairedDevices.Contains(deviceId.GetNodeId()); + } + + return false; +} + +CHIP_ERROR DeviceController::GetConnectedDevice(NodeId deviceId, Callback::Callback * onConnection, + Callback::Callback * onFailure) +{ + CHIP_ERROR err = CHIP_NO_ERROR; + Device * device = nullptr; + + err = GetDevice(deviceId, &device); + SuccessOrExit(err); + + if (device->IsSecureConnected()) + { + // Call onConnectedDevice + onConnection->mCall(onConnection->mContext, device); + return CHIP_NO_ERROR; + } + + err = device->EstablishConnectivity(onConnection, onFailure); + SuccessOrExit(err); + +exit: + if (err != CHIP_NO_ERROR) + { + onFailure->mCall(onFailure->mContext, deviceId, err); + } + + return err; +} + +CHIP_ERROR DeviceController::UpdateDevice(NodeId deviceId, uint64_t fabricId) { #if CHIP_DEVICE_CONFIG_ENABLE_MDNS - return Mdns::Resolver::Instance().ResolveNodeId(chip::PeerId().SetNodeId(device->GetDeviceId()).SetFabricId(fabricId), + return Mdns::Resolver::Instance().ResolveNodeId(chip::PeerId().SetNodeId(deviceId).SetFabricId(fabricId), chip::Inet::kIPAddressType_Any); #else return CHIP_ERROR_UNSUPPORTED_CHIP_FEATURE; @@ -797,7 +796,8 @@ DeviceCommissioner::DeviceCommissioner() : mOpCSRResponseCallback(OnOperationalCertificateSigningRequest, this), mOpCertResponseCallback(OnOperationalCertificateAddResponse, this), mRootCertResponseCallback(OnRootCertSuccessResponse, this), mOnCSRFailureCallback(OnCSRFailureResponse, this), mOnCertFailureCallback(OnAddOpCertFailureResponse, this), - mOnRootCertFailureCallback(OnRootCertFailureResponse, this) + mOnRootCertFailureCallback(OnRootCertFailureResponse, this), mOnDeviceConnectedCallback(OnDeviceConnectedFn, this), + mOnDeviceConnectionFailureCallback(OnDeviceConnectionFailureFn, this) { mPairingDelegate = nullptr; mDeviceBeingPaired = kNumMaxActiveDevices; @@ -1065,7 +1065,7 @@ CHIP_ERROR DeviceCommissioner::OperationalDiscoveryComplete(NodeId remoteDeviceI PersistDevice(device); PersistNextKeyId(); - return CHIP_NO_ERROR; + return GetConnectedDevice(remoteDeviceId, &mOnDeviceConnectedCallback, &mOnDeviceConnectionFailureCallback); } void DeviceCommissioner::FreeRendezvousSession() @@ -1551,6 +1551,18 @@ void DeviceCommissioner::OnNodeIdResolutionFailed(const chip::PeerId & peer, CHI #endif +void DeviceCommissioner::OnDeviceConnectedFn(void * context, Device * device) +{ + DeviceCommissioner * commissioner = reinterpret_cast(context); + commissioner->mPairingDelegate->OnCommissioningComplete(device->GetDeviceId(), CHIP_NO_ERROR); +} + +void DeviceCommissioner::OnDeviceConnectionFailureFn(void * context, NodeId deviceId, CHIP_ERROR error) +{ + DeviceCommissioner * commissioner = reinterpret_cast(context); + commissioner->mPairingDelegate->OnCommissioningComplete(deviceId, error); +} + CommissioningStage DeviceCommissioner::GetNextCommissioningStage() { switch (mCommissioningStage) diff --git a/src/controller/CHIPDeviceController.h b/src/controller/CHIPDeviceController.h index 134ae3279600e5..30273a5c6d7602 100644 --- a/src/controller/CHIPDeviceController.h +++ b/src/controller/CHIPDeviceController.h @@ -143,6 +143,11 @@ class DLL_EXPORT DevicePairingDelegate * @param error Error cause, if any */ virtual void OnPairingDeleted(CHIP_ERROR error) {} + + /** + * Called when the commissioning process is complete (with success or error) + */ + virtual void OnCommissioningComplete(NodeId deviceId, CHIP_ERROR error) {} }; struct CommissionerInitParams : public ControllerInitParams @@ -202,21 +207,6 @@ class DLL_EXPORT DeviceController : public Messaging::ExchangeDelegate, */ virtual CHIP_ERROR Shutdown(); - /** - * @brief - * This function deserializes the provided deviceInfo object, and initializes and outputs the - * corresponding Device object. The lifetime of the output object is tied to that of the DeviceController - * object. The caller must not use the Device object If they free the DeviceController object, or - * after they call ReleaseDevice() on the returned device object. - * - * @param[in] deviceId Node ID for the CHIP device - * @param[in] deviceInfo Serialized device info for the device - * @param[out] device The output device object - * - * @return CHIP_ERROR CHIP_NO_ERROR on success, or corresponding error code. - */ - CHIP_ERROR GetDevice(NodeId deviceId, const SerializedDevice & deviceInfo, Device ** device); - /** * @brief * This function is similar to the other GetDevice object, except it reads the serialized object from @@ -229,17 +219,31 @@ class DLL_EXPORT DeviceController : public Messaging::ExchangeDelegate, */ CHIP_ERROR GetDevice(NodeId deviceId, Device ** device); + /** + * This function returns true if the device corresponding to `deviceId` has previously been commissioned + * on the fabric. + */ + bool DoesDevicePairingExist(const PeerId & deviceId); + + /** + * This function finds the device corresponding to deviceId, and establishes a secure connection with it. + * Once the connection is successfully establishes (or if it's already connected), it calls `onConnectedDevice` + * callback. If it fails to establish the connection, it calls `onError` callback. + */ + CHIP_ERROR GetConnectedDevice(NodeId deviceId, Callback::Callback * onConnection, + Callback::Callback * onFailure); + /** * @brief * This function update the device informations asynchronously using mdns. * If new device informations has been found, it will be persisted. * - * @param[in] device The input device object to update + * @param[in] deviceId Node ID for the CHIP devicex * @param[in] fabricId The fabricId used for mdns resolution * * @return CHIP_ERROR CHIP_NO_ERROR on success, or corresponding error code. */ - CHIP_ERROR UpdateDevice(Device * device, uint64_t fabricId); + CHIP_ERROR UpdateDevice(NodeId deviceId, uint64_t fabricId); void PersistDevice(Device * device); @@ -601,6 +605,9 @@ class DLL_EXPORT DeviceCommissioner : public DeviceController, public SessionEst /* Callback called when adding root cert to device results in failure */ static void OnRootCertFailureResponse(void * context, uint8_t status); + static void OnDeviceConnectedFn(void * context, Device * device); + static void OnDeviceConnectionFailureFn(void * context, NodeId deviceId, CHIP_ERROR error); + /** * @brief * This function processes the CSR sent by the device. @@ -629,6 +636,9 @@ class DLL_EXPORT DeviceCommissioner : public DeviceController, public SessionEst Callback::Callback mOnCertFailureCallback; Callback::Callback mOnRootCertFailureCallback; + Callback::Callback mOnDeviceConnectedCallback; + Callback::Callback mOnDeviceConnectionFailureCallback; + PASESession mPairingSession; }; diff --git a/src/darwin/Framework/CHIP/CHIPDeviceController.mm b/src/darwin/Framework/CHIP/CHIPDeviceController.mm index 47d96c7a9c202a..0d92975f80d143 100644 --- a/src/darwin/Framework/CHIP/CHIPDeviceController.mm +++ b/src/darwin/Framework/CHIP/CHIPDeviceController.mm @@ -383,17 +383,10 @@ - (void)updateDevice:(uint64_t)deviceID fabricId:(uint64_t)fabricId return; } dispatch_sync(_chipWorkQueue, ^{ - chip::Controller::Device * device = nullptr; - if ([self isRunning]) { - errorCode = self.cppCommissioner->GetDevice(deviceID, &device); + errorCode = self.cppCommissioner->UpdateDevice(deviceID, fabricId); + CHIP_LOG_ERROR("Update device address returned: %d", errorCode); } - - if (errorCode != CHIP_NO_ERROR) { - return; - } - - errorCode = self.cppCommissioner->UpdateDevice(device, fabricId); }); } From 6c1f43ac3b90ea014fd499e0817ae3be34025a7e Mon Sep 17 00:00:00 2001 From: Pankaj Garg Date: Tue, 22 Jun 2021 09:28:47 -0700 Subject: [PATCH 12/12] some cleanup --- src/controller/CHIPDeviceController.cpp | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/controller/CHIPDeviceController.cpp b/src/controller/CHIPDeviceController.cpp index 6e28c74943d545..8fb1b768cd4e40 100644 --- a/src/controller/CHIPDeviceController.cpp +++ b/src/controller/CHIPDeviceController.cpp @@ -479,7 +479,6 @@ CHIP_ERROR DeviceController::GetConnectedDevice(NodeId deviceId, Callback::Callb if (device->IsSecureConnected()) { - // Call onConnectedDevice onConnection->mCall(onConnection->mContext, device); return CHIP_NO_ERROR; } @@ -1554,12 +1553,20 @@ void DeviceCommissioner::OnNodeIdResolutionFailed(const chip::PeerId & peer, CHI void DeviceCommissioner::OnDeviceConnectedFn(void * context, Device * device) { DeviceCommissioner * commissioner = reinterpret_cast(context); + VerifyOrReturn(commissioner != nullptr, ChipLogProgress(Controller, "Device connected callback with null context. Ignoring")); + VerifyOrReturn(commissioner->mPairingDelegate != nullptr, + ChipLogProgress(Controller, "Device connected callback with null pairing delegate. Ignoring")); commissioner->mPairingDelegate->OnCommissioningComplete(device->GetDeviceId(), CHIP_NO_ERROR); } void DeviceCommissioner::OnDeviceConnectionFailureFn(void * context, NodeId deviceId, CHIP_ERROR error) { DeviceCommissioner * commissioner = reinterpret_cast(context); + ChipLogProgress(Controller, "Device connection failed. Error %s", ErrorStr(error)); + VerifyOrReturn(commissioner != nullptr, + ChipLogProgress(Controller, "Device connection failure callback with null context. Ignoring")); + VerifyOrReturn(commissioner->mPairingDelegate != nullptr, + ChipLogProgress(Controller, "Device connection failure callback with null pairing delegate. Ignoring")); commissioner->mPairingDelegate->OnCommissioningComplete(deviceId, error); }