Skip to content

Commit

Permalink
Add support for controllers not advertising their operational identit…
Browse files Browse the repository at this point in the history 
…ies. (#28537)

If multiple controllers are running, and some want to enable server interactions
while others do not, the ones not enabling server interactions should not
advertise.

Fixes #28279
  • Loading branch information
@bzbarsky-apple
bzbarsky-apple authored Aug 8, 2023
1 parent 4d7932e commit 35998cd
Show file tree
Hide file tree
Showing 7 changed files with 121 additions and 61 deletions.
5 changes: 5 additions & 0 deletions src/app/server/Dnssd.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,11 @@ CHIP_ERROR DnssdServer::AdvertiseOperational()

for (const FabricInfo & fabricInfo : *mFabricTable)
{
if (!fabricInfo.ShouldAdvertiseIdentity())
{
continue;
}

uint8_t macBuffer[DeviceLayer::ConfigurationManager::kPrimaryMACAddressLength];
MutableByteSpan mac(macBuffer);
if (chip::DeviceLayer::ConfigurationMgr().GetPrimaryMACAddress(mac) != CHIP_NO_ERROR)
Expand Down
24 changes: 10 additions & 14 deletions src/controller/CHIPDeviceController.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -129,15 +129,6 @@ CHIP_ERROR DeviceController::Init(ControllerInitParams params)
if (params.operationalKeypair != nullptr || !params.controllerNOC.empty() || !params.controllerRCAC.empty())
{
ReturnErrorOnFailure(InitControllerNOCChain(params));

if (params.enableServerInteractions)
{
//
// Advertise our operational identity on the network to facilitate discovery by clients that look to
// establish CASE with a controller that is also offering server-side capabilities (e.g an OTA provider).
//
app::DnssdServer::Instance().AdvertiseOperational();
}
}

mSystemState = params.systemState->Retain();
Expand Down Expand Up @@ -239,6 +230,9 @@ CHIP_ERROR DeviceController::InitControllerNOCChain(const ControllerInitParams &

CHIP_ERROR err = CHIP_NO_ERROR;

auto advertiseOperational =
params.enableServerInteractions ? FabricTable::AdvertiseIdentity::Yes : FabricTable::AdvertiseIdentity::No;

//
// We permit colliding fabrics when multiple controllers are present on the same logical fabric
// since each controller is associated with a unique FabricInfo 'identity' object and consequently,
Expand All @@ -261,16 +255,17 @@ CHIP_ERROR DeviceController::InitControllerNOCChain(const ControllerInitParams &
if (fabricFoundInTable)
{
err = fabricTable->UpdatePendingFabricWithProvidedOpKey(fabricIndex, nocSpan, icacSpan, externalOperationalKeypair,
hasExternallyOwnedKeypair);
hasExternallyOwnedKeypair, advertiseOperational);
}
else
// CASE 2: New fabric with injected key
{
err = fabricTable->AddNewPendingTrustedRootCert(rcacSpan);
if (err == CHIP_NO_ERROR)
{
err = fabricTable->AddNewPendingFabricWithProvidedOpKey(
nocSpan, icacSpan, newFabricVendorId, externalOperationalKeypair, hasExternallyOwnedKeypair, &fabricIndex);
err = fabricTable->AddNewPendingFabricWithProvidedOpKey(nocSpan, icacSpan, newFabricVendorId,
externalOperationalKeypair, hasExternallyOwnedKeypair,
&fabricIndex, advertiseOperational);
}
}
}
Expand All @@ -283,15 +278,16 @@ CHIP_ERROR DeviceController::InitControllerNOCChain(const ControllerInitParams &
{
VerifyOrReturnError(fabricTable->HasOperationalKeyForFabric(fabricIndex), CHIP_ERROR_KEY_NOT_FOUND);

err = fabricTable->UpdatePendingFabricWithOperationalKeystore(fabricIndex, nocSpan, icacSpan);
err = fabricTable->UpdatePendingFabricWithOperationalKeystore(fabricIndex, nocSpan, icacSpan, advertiseOperational);
}
else
// CASE 4: New fabric with operational keystore
{
err = fabricTable->AddNewPendingTrustedRootCert(rcacSpan);
if (err == CHIP_NO_ERROR)
{
err = fabricTable->AddNewPendingFabricWithOperationalKeystore(nocSpan, icacSpan, newFabricVendorId, &fabricIndex);
err = fabricTable->AddNewPendingFabricWithOperationalKeystore(nocSpan, icacSpan, newFabricVendorId, &fabricIndex,
advertiseOperational);
}

if (err == CHIP_NO_ERROR)
Expand Down
31 changes: 24 additions & 7 deletions src/controller/CHIPDeviceControllerFactory.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -256,13 +256,6 @@ CHIP_ERROR DeviceControllerFactory::InitSystemState(FactoryInitParams params)
// Consequently, reach in set the fabric table pointer to point to the right version.
//
app::DnssdServer::Instance().SetFabricTable(stateParams.fabricTable);

//
// Start up the DNS-SD server. We are not giving it a
// CommissioningModeProvider, so it will not claim we are in
// commissioning mode.
//
chip::app::DnssdServer::Instance().StartServer();
}

stateParams.sessionSetupPool = Platform::New<DeviceControllerSystemStateParams::SessionSetupPool>();
Expand Down Expand Up @@ -315,6 +308,18 @@ void DeviceControllerFactory::PopulateInitParams(ControllerInitParams & controll
controllerParams.enableServerInteractions = params.enableServerInteractions;
}

void DeviceControllerFactory::ControllerInitialized(const DeviceController & controller)
{
if (mEnableServerInteractions && controller.GetFabricIndex() != kUndefinedFabricIndex)
{
// Restart DNS-SD advertising, because initialization of this controller could
// have modified whether a particular fabric identity should be
// advertised. Just calling AdvertiseOperational() is not good enough
// here, since we might be removing advertising.
app::DnssdServer::Instance().StartServer();
}
}

CHIP_ERROR DeviceControllerFactory::SetupController(SetupParams params, DeviceController & controller)
{
VerifyOrReturnError(mSystemState != nullptr, CHIP_ERROR_INCORRECT_STATE);
Expand All @@ -326,6 +331,12 @@ CHIP_ERROR DeviceControllerFactory::SetupController(SetupParams params, DeviceCo
PopulateInitParams(controllerParams, params);

CHIP_ERROR err = controller.Init(controllerParams);

if (err == CHIP_NO_ERROR)
{
ControllerInitialized(controller);
}

return err;
}

Expand All @@ -347,6 +358,12 @@ CHIP_ERROR DeviceControllerFactory::SetupCommissioner(SetupParams params, Device
commissionerParams.deviceAttestationVerifier = params.deviceAttestationVerifier;

CHIP_ERROR err = commissioner.Init(commissionerParams);

if (err == CHIP_NO_ERROR)
{
ControllerInitialized(commissioner);
}

return err;
}

Expand Down
1 change: 1 addition & 0 deletions src/controller/CHIPDeviceControllerFactory.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -252,6 +252,7 @@ class DeviceControllerFactory
void PopulateInitParams(ControllerInitParams & controllerParams, const SetupParams & params);
CHIP_ERROR InitSystemState(FactoryInitParams params);
CHIP_ERROR InitSystemState();
void ControllerInitialized(const DeviceController & controller);

uint16_t mListenPort;
DeviceControllerSystemState * mSystemState = nullptr;
Expand Down
41 changes: 23 additions & 18 deletions src/credentials/FabricTable.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -78,12 +78,13 @@ CHIP_ERROR FabricInfo::Init(const FabricInfo::InitParams & initParams)

Reset();

mNodeId = initParams.nodeId;
mFabricId = initParams.fabricId;
mFabricIndex = initParams.fabricIndex;
mCompressedFabricId = initParams.compressedFabricId;
mRootPublicKey = initParams.rootPublicKey;
mVendorId = static_cast<VendorId>(initParams.vendorId);
mNodeId = initParams.nodeId;
mFabricId = initParams.fabricId;
mFabricIndex = initParams.fabricIndex;
mCompressedFabricId = initParams.compressedFabricId;
mRootPublicKey = initParams.rootPublicKey;
mVendorId = static_cast<VendorId>(initParams.vendorId);
mShouldAdvertiseIdentity = initParams.advertiseIdentity;

// Deal with externally injected keys
if (initParams.operationalKeypair != nullptr)
Expand All @@ -105,12 +106,13 @@ void FabricInfo::operator=(FabricInfo && other)
{
Reset();

mNodeId = other.mNodeId;
mFabricId = other.mFabricId;
mFabricIndex = other.mFabricIndex;
mCompressedFabricId = other.mCompressedFabricId;
mRootPublicKey = other.mRootPublicKey;
mVendorId = other.mVendorId;
mNodeId = other.mNodeId;
mFabricId = other.mFabricId;
mFabricIndex = other.mFabricIndex;
mCompressedFabricId = other.mCompressedFabricId;
mRootPublicKey = other.mRootPublicKey;
mVendorId = other.mVendorId;
mShouldAdvertiseIdentity = other.mShouldAdvertiseIdentity;

SetFabricLabel(other.GetFabricLabel());

Expand Down Expand Up @@ -768,7 +770,7 @@ CHIP_ERROR FabricTable::NotifyFabricCommitted(FabricIndex fabricIndex)

CHIP_ERROR
FabricTable::AddOrUpdateInner(FabricIndex fabricIndex, bool isAddition, Crypto::P256Keypair * existingOpKey,
bool isExistingOpKeyExternallyOwned, uint16_t vendorId)
bool isExistingOpKeyExternallyOwned, uint16_t vendorId, AdvertiseIdentity advertiseIdentity)
{
// All parameters pre-validated before we get here

Expand Down Expand Up @@ -867,6 +869,8 @@ FabricTable::AddOrUpdateInner(FabricIndex fabricIndex, bool isAddition, Crypto::
return CHIP_ERROR_INCORRECT_STATE;
}

newFabricInfo.advertiseIdentity = (advertiseIdentity == AdvertiseIdentity::Yes);

// Update local copy of fabric data. For add it's a new entry, for update, it's `mPendingFabric` shadow entry.
ReturnErrorOnFailure(fabricEntry->Init(newFabricInfo));

Expand Down Expand Up @@ -1642,7 +1646,7 @@ CHIP_ERROR FabricTable::FindExistingFabricByNocChaining(FabricIndex pendingFabri

CHIP_ERROR FabricTable::AddNewPendingFabricCommon(const ByteSpan & noc, const ByteSpan & icac, uint16_t vendorId,
Crypto::P256Keypair * existingOpKey, bool isExistingOpKeyExternallyOwned,
FabricIndex * outNewFabricIndex)
AdvertiseIdentity advertiseIdentity, FabricIndex * outNewFabricIndex)
{
VerifyOrReturnError(mOpCertStore != nullptr, CHIP_ERROR_INCORRECT_STATE);
VerifyOrReturnError(outNewFabricIndex != nullptr, CHIP_ERROR_INVALID_ARGUMENT);
Expand Down Expand Up @@ -1692,8 +1696,8 @@ CHIP_ERROR FabricTable::AddNewPendingFabricCommon(const ByteSpan & noc, const By
ReturnErrorOnFailure(mOpCertStore->AddNewOpCertsForFabric(fabricIndexToUse, noc, icac));
VerifyOrReturnError(SetPendingDataFabricIndex(fabricIndexToUse), CHIP_ERROR_INCORRECT_STATE);

CHIP_ERROR err =
AddOrUpdateInner(fabricIndexToUse, /* isAddition = */ true, existingOpKey, isExistingOpKeyExternallyOwned, vendorId);
CHIP_ERROR err = AddOrUpdateInner(fabricIndexToUse, /* isAddition = */ true, existingOpKey, isExistingOpKeyExternallyOwned,
vendorId, advertiseIdentity);
if (err != CHIP_NO_ERROR)
{
// Revert partial state added on error
Expand All @@ -1712,7 +1716,8 @@ CHIP_ERROR FabricTable::AddNewPendingFabricCommon(const ByteSpan & noc, const By
}

CHIP_ERROR FabricTable::UpdatePendingFabricCommon(FabricIndex fabricIndex, const ByteSpan & noc, const ByteSpan & icac,
Crypto::P256Keypair * existingOpKey, bool isExistingOpKeyExternallyOwned)
Crypto::P256Keypair * existingOpKey, bool isExistingOpKeyExternallyOwned,
AdvertiseIdentity advertiseIdentity)
{
VerifyOrReturnError(mOpCertStore != nullptr, CHIP_ERROR_INCORRECT_STATE);
VerifyOrReturnError(IsValidFabricIndex(fabricIndex), CHIP_ERROR_INVALID_ARGUMENT);
Expand Down Expand Up @@ -1751,7 +1756,7 @@ CHIP_ERROR FabricTable::UpdatePendingFabricCommon(FabricIndex fabricIndex, const
VerifyOrReturnError(SetPendingDataFabricIndex(fabricIndex), CHIP_ERROR_INCORRECT_STATE);

CHIP_ERROR err = AddOrUpdateInner(fabricIndex, /* isAddition = */ false, existingOpKey, isExistingOpKeyExternallyOwned,
fabricInfo->GetVendorId());
fabricInfo->GetVendorId(), advertiseIdentity);
if (err != CHIP_NO_ERROR)
{
// Revert partial state added on error
Expand Down
51 changes: 35 additions & 16 deletions src/credentials/FabricTable.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,8 @@ class DLL_EXPORT FabricInfo

bool HasOperationalKey() const { return mOperationalKey != nullptr; }

bool ShouldAdvertiseIdentity() const { return mShouldAdvertiseIdentity; }

friend class FabricTable;

private:
Expand All @@ -125,6 +127,7 @@ class DLL_EXPORT FabricInfo
VendorId vendorId = VendorId::NotSpecified; /**< Vendor ID for commissioner of fabric */
Crypto::P256Keypair * operationalKeypair = nullptr;
bool hasExternallyOwnedKeypair = false;
bool advertiseIdentity = false;

CHIP_ERROR AreValid() const
{
Expand Down Expand Up @@ -204,7 +207,9 @@ class DLL_EXPORT FabricInfo
{
chip::Platform::Delete(mOperationalKey);
}
mOperationalKey = nullptr;
mOperationalKey = nullptr;
mHasExternallyOwnedOperationalKey = false;
mShouldAdvertiseIdentity = true;

mFabricIndex = kUndefinedFabricIndex;
mNodeId = kUndefinedNodeId;
Expand All @@ -230,14 +235,16 @@ class DLL_EXPORT FabricInfo
// mFabricLabel is 33 bytes, so ends on a 1 mod 4 byte boundary.
char mFabricLabel[kFabricLabelMaxLengthInBytes + 1] = { '\0' };

// mFabricIndex, mVendorId, mHasExternallyOwnedOperationalKey are 4 bytes
// and do not end up with any padding if they come after the 33-byte
// mFabricLabel, so end on a 1 mod 4 byte boundary.
// mFabricIndex, mVendorId, mHasExternallyOwnedOperationalKey,
// mShouldAdvertiseIdentity are 5 bytes and do not include any padding if
// they come after the 33-byte mFabricLabel, so end on a 2 mod 4 byte
// boundary.
FabricIndex mFabricIndex = kUndefinedFabricIndex;
VendorId mVendorId = VendorId::NotSpecified;
bool mHasExternallyOwnedOperationalKey = false;
bool mShouldAdvertiseIdentity = true;

// 3 bytes of padding here, since mOperationalKey needs to be void*-aligned,
// 2 bytes of padding here, since mOperationalKey needs to be void*-aligned,
// so has to be at a 0 mod 4 byte location.

mutable Crypto::P256Keypair * mOperationalKey = nullptr;
Expand Down Expand Up @@ -400,6 +407,12 @@ class DLL_EXPORT FabricTable
FabricTable(FabricTable const &) = delete;
void operator=(FabricTable const &) = delete;

enum class AdvertiseIdentity : uint8_t
{
Yes,
No
};

// Returns CHIP_ERROR_NOT_FOUND if there is no fabric for that index.
CHIP_ERROR Delete(FabricIndex fabricIndex);
void DeleteAllFabrics();
Expand Down Expand Up @@ -783,9 +796,10 @@ class DLL_EXPORT FabricTable
* @retval other CHIP_ERROR_* on internal errors or certificate validation errors.
*/
CHIP_ERROR AddNewPendingFabricWithOperationalKeystore(const ByteSpan & noc, const ByteSpan & icac, uint16_t vendorId,
FabricIndex * outNewFabricIndex)
FabricIndex * outNewFabricIndex,
AdvertiseIdentity advertiseIdentity = AdvertiseIdentity::Yes)
{
return AddNewPendingFabricCommon(noc, icac, vendorId, nullptr, false, outNewFabricIndex);
return AddNewPendingFabricCommon(noc, icac, vendorId, nullptr, false, advertiseIdentity, outNewFabricIndex);
};

/**
Expand Down Expand Up @@ -818,9 +832,11 @@ class DLL_EXPORT FabricTable
*/
CHIP_ERROR AddNewPendingFabricWithProvidedOpKey(const ByteSpan & noc, const ByteSpan & icac, uint16_t vendorId,
Crypto::P256Keypair * existingOpKey, bool isExistingOpKeyExternallyOwned,
FabricIndex * outNewFabricIndex)
FabricIndex * outNewFabricIndex,
AdvertiseIdentity advertiseIdentity = AdvertiseIdentity::Yes)
{
return AddNewPendingFabricCommon(noc, icac, vendorId, existingOpKey, isExistingOpKeyExternallyOwned, outNewFabricIndex);
return AddNewPendingFabricCommon(noc, icac, vendorId, existingOpKey, isExistingOpKeyExternallyOwned, advertiseIdentity,
outNewFabricIndex);
};

/**
Expand Down Expand Up @@ -852,9 +868,10 @@ class DLL_EXPORT FabricTable
* @retval CHIP_ERROR_INVALID_ARGUMENT if any of the arguments are invalid such as too large or out of bounds.
* @retval other CHIP_ERROR_* on internal errors or certificate validation errors.
*/
CHIP_ERROR UpdatePendingFabricWithOperationalKeystore(FabricIndex fabricIndex, const ByteSpan & noc, const ByteSpan & icac)
CHIP_ERROR UpdatePendingFabricWithOperationalKeystore(FabricIndex fabricIndex, const ByteSpan & noc, const ByteSpan & icac,
AdvertiseIdentity advertiseIdentity = AdvertiseIdentity::Yes)
{
return UpdatePendingFabricCommon(fabricIndex, noc, icac, nullptr, false);
return UpdatePendingFabricCommon(fabricIndex, noc, icac, nullptr, false, advertiseIdentity);
}

/**
Expand Down Expand Up @@ -886,9 +903,10 @@ class DLL_EXPORT FabricTable
*/

CHIP_ERROR UpdatePendingFabricWithProvidedOpKey(FabricIndex fabricIndex, const ByteSpan & noc, const ByteSpan & icac,
Crypto::P256Keypair * existingOpKey, bool isExistingOpKeyExternallyOwned)
Crypto::P256Keypair * existingOpKey, bool isExistingOpKeyExternallyOwned,
AdvertiseIdentity advertiseIdentity = AdvertiseIdentity::Yes)
{
return UpdatePendingFabricCommon(fabricIndex, noc, icac, existingOpKey, isExistingOpKeyExternallyOwned);
return UpdatePendingFabricCommon(fabricIndex, noc, icac, existingOpKey, isExistingOpKeyExternallyOwned, advertiseIdentity);
}

/**
Expand Down Expand Up @@ -1050,16 +1068,17 @@ class DLL_EXPORT FabricTable

// Core validation logic for fabric additions/updates
CHIP_ERROR AddOrUpdateInner(FabricIndex fabricIndex, bool isAddition, Crypto::P256Keypair * existingOpKey,
bool isExistingOpKeyExternallyOwned, uint16_t vendorId);
bool isExistingOpKeyExternallyOwned, uint16_t vendorId, AdvertiseIdentity advertiseIdentity);

// Common code for fabric addition, for either OperationalKeystore or injected key scenarios.
CHIP_ERROR AddNewPendingFabricCommon(const ByteSpan & noc, const ByteSpan & icac, uint16_t vendorId,
Crypto::P256Keypair * existingOpKey, bool isExistingOpKeyExternallyOwned,
FabricIndex * outNewFabricIndex);
AdvertiseIdentity advertiseIdentity, FabricIndex * outNewFabricIndex);

// Common code for fabric updates, for either OperationalKeystore or injected key scenarios.
CHIP_ERROR UpdatePendingFabricCommon(FabricIndex fabricIndex, const ByteSpan & noc, const ByteSpan & icac,
Crypto::P256Keypair * existingOpKey, bool isExistingOpKeyExternallyOwned);
Crypto::P256Keypair * existingOpKey, bool isExistingOpKeyExternallyOwned,
AdvertiseIdentity advertiseIdentity);

// Common code for looking up a fabric given a root public key, a fabric ID and an optional node id scoped to that fabric.
const FabricInfo * FindFabricCommon(const Crypto::P256PublicKey & rootPubKey, FabricId fabricId,
Expand Down
Loading

0 comments on commit 35998cd

Please sign in to comment.