Skip to content

Commit

Permalink
Fix bridge-app init (#17694)
Browse files Browse the repository at this point in the history
* Fix bridge-app init

- Bridge app did not have proper CommissionableDataProvider
  init and nobody caught it until some testing weeks later.
- This PR adds the necessary init calls and also refactors
  elements of the bridge app structure to prevent this from
  happening again.

Fixes #15134

Testing done:
- All unit tests pass
- Cert tests pass
- chip-bridge-app works and is commissionablen by chip-tool and chip-repl

* Fix CI
  • Loading branch information
tcarmelveilleux authored and pull[bot] committed Apr 28, 2022
1 parent 463e995 commit 4726803
Show file tree
Hide file tree
Showing 8 changed files with 211 additions and 222 deletions.
2 changes: 1 addition & 1 deletion examples/bridge-app/linux/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,13 @@ executable("chip-bridge-app") {
sources = [
"${chip_root}/examples/tv-app/tv-common/include/CHIPProjectAppConfig.h",
"Device.cpp",
"Options.cpp",
"include/Device.h",
"main.cpp",
]

deps = [
"${chip_root}/examples/bridge-app/bridge-common",
"${chip_root}/examples/platform/linux:app-main",
"${chip_root}/src/lib",
]

Expand Down
83 changes: 0 additions & 83 deletions examples/bridge-app/linux/Options.cpp

This file was deleted.

40 changes: 0 additions & 40 deletions examples/bridge-app/linux/include/Options.h

This file was deleted.

47 changes: 35 additions & 12 deletions examples/bridge-app/linux/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
#include <setup_payload/QRCodeSetupPayloadGenerator.h>
#include <setup_payload/SetupPayload.h>

#include "CommissionableInit.h"
#include "Device.h"
#include "Options.h"
#include <app/server/Server.h>
Expand All @@ -51,16 +52,21 @@ using namespace chip::Inet;
using namespace chip::Transport;
using namespace chip::DeviceLayer;

static const int kNodeLabelSize = 32;
namespace {

const int kNodeLabelSize = 32;
// Current ZCL implementation of Struct uses a max-size array of 254 bytes
static const int kDescriptorAttributeArraySize = 254;
static const int kFixedLabelAttributeArraySize = 254;
const int kDescriptorAttributeArraySize = 254;
const int kFixedLabelAttributeArraySize = 254;
// Four attributes in descriptor cluster: DeviceTypeList, ServerList, ClientList, PartsList
static const int kFixedLabelElementsOctetStringSize = 16;
const int kFixedLabelElementsOctetStringSize = 16;

EndpointId gCurrentEndpointId;
EndpointId gFirstDynamicEndpointId;
Device * gDevices[CHIP_DEVICE_CONFIG_DYNAMIC_ENDPOINT_COUNT];

static EndpointId gCurrentEndpointId;
static EndpointId gFirstDynamicEndpointId;
static Device * gDevices[CHIP_DEVICE_CONFIG_DYNAMIC_ENDPOINT_COUNT];
// To hold SPAKE2+ verifier, discriminator, passcode
LinuxCommissionableDataProvider gCommissionableDataProvider;

// ENDPOINT DEFINITIONS:
// =================================================================================
Expand Down Expand Up @@ -194,6 +200,8 @@ DECLARE_DYNAMIC_ENDPOINT(bridgedSwitchEndpoint, bridgedSwitchClusters);
DataVersion gSwitch1DataVersions[ArraySize(bridgedSwitchClusters)];
DataVersion gSwitch2DataVersions[ArraySize(bridgedSwitchClusters)];

} // namespace

// REVISION DEFINITIONS:
// =================================================================================

Expand Down Expand Up @@ -653,22 +661,37 @@ int main(int argc, char * argv[])
err = chip::DeviceLayer::PlatformMgr().InitChipStack();
SuccessOrExit(err);

// Init the commissionable data provider based on command line options
// to handle custom verifiers, discriminators, etc.
err = chip::examples::InitCommissionableDataProvider(gCommissionableDataProvider, LinuxDeviceOptions::GetInstance());
SuccessOrExit(err);
DeviceLayer::SetCommissionableDataProvider(&gCommissionableDataProvider);

err = chip::examples::InitConfigurationManager(reinterpret_cast<ConfigurationManagerImpl &>(ConfigurationMgr()),
LinuxDeviceOptions::GetInstance());
SuccessOrExit(err);

err = PrintQRCodeContent();
SuccessOrExit(err);

chip::DeviceLayer::PlatformMgrImpl().AddEventHandler(EventHandler, 0);

chip::DeviceLayer::ConnectivityMgr().SetBLEDeviceName(nullptr); // Use default device name (CHIP-XXXX)

#if CONFIG_NETWORK_LAYER_BLE
chip::DeviceLayer::ConnectivityMgr().SetBLEDeviceName(nullptr); // Use default device name (CHIP-XXXX)
chip::DeviceLayer::Internal::BLEMgrImpl().ConfigureBle(LinuxDeviceOptions::GetInstance().mBleDevice, false);
#endif

chip::DeviceLayer::ConnectivityMgr().SetBLEAdvertisingEnabled(true);
#endif

// Init ZCL Data Model and CHIP App Server
// Init Data Model and CHIP App Server
static chip::CommonCaseDeviceServerInitParams initParams;
(void) initParams.InitializeStaticResourcesBeforeServerInit();

#if CHIP_DEVICE_ENABLE_PORT_PARAMS
// use a different service port to make testing possible with other sample devices running on same host
initParams.operationalServicePort = LinuxDeviceOptions::GetInstance().securedDevicePort;
#endif

initParams.interfaceId = LinuxDeviceOptions::GetInstance().interfaceId;
chip::Server::GetInstance().Init(initParams);

// Initialize device attestation config
Expand Down
102 changes: 16 additions & 86 deletions examples/platform/linux/AppMain.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,6 @@

#include <platform/CommissionableDataProvider.h>
#include <platform/DiagnosticDataProvider.h>
#include <platform/KvsPersistentStorageDelegate.h>
#include <platform/TestOnlyCommissionableDataProvider.h>

#if CHIP_DEVICE_CONFIG_ENABLE_BOTH_COMMISSIONER_AND_COMMISSIONEE
#include "CommissionerMain.h"
Expand Down Expand Up @@ -69,7 +67,7 @@
#include <signal.h>

#include "AppMain.h"
#include "LinuxCommissionableDataProvider.h"
#include "CommissionableInit.h"

using namespace chip;
using namespace chip::ArgParser;
Expand Down Expand Up @@ -162,80 +160,6 @@ void SetupSignalHandlers()
signal(SIGINT, OnSignalHandler);
}

CHIP_ERROR InitCommissionableDataProvider(LinuxCommissionableDataProvider & provider, LinuxDeviceOptions & options)
{
chip::Optional<uint32_t> setupPasscode;

if (options.payload.setUpPINCode != 0)
{
setupPasscode.SetValue(options.payload.setUpPINCode);
}
else if (!options.spake2pVerifier.HasValue())
{
uint32_t defaultTestPasscode = 0;
chip::DeviceLayer::TestOnlyCommissionableDataProvider TestOnlyCommissionableDataProvider;
VerifyOrDie(TestOnlyCommissionableDataProvider.GetSetupPasscode(defaultTestPasscode) == CHIP_NO_ERROR);

ChipLogError(Support,
"*** WARNING: Using temporary passcode %u due to no neither --passcode or --spake2p-verifier-base64 "
"given on command line. This is temporary and will disappear. Please update your scripts "
"to explicitly configure onboarding credentials. ***",
static_cast<unsigned>(defaultTestPasscode));
setupPasscode.SetValue(defaultTestPasscode);
options.payload.setUpPINCode = defaultTestPasscode;
}
else
{
// Passcode is 0, so will be ignored, and verifier will take over. Onboarding payload
// printed for debug will be invalid, but if the onboarding payload had been given
// properly to the commissioner later, PASE will succeed.
}

if (options.discriminator.HasValue())
{
options.payload.discriminator = options.discriminator.Value();
}
else
{
uint16_t defaultTestDiscriminator = 0;
chip::DeviceLayer::TestOnlyCommissionableDataProvider TestOnlyCommissionableDataProvider;
VerifyOrDie(TestOnlyCommissionableDataProvider.GetSetupDiscriminator(defaultTestDiscriminator) == CHIP_NO_ERROR);

ChipLogError(Support,
"*** WARNING: Using temporary test discriminator %u due to --discriminator not "
"given on command line. This is temporary and will disappear. Please update your scripts "
"to explicitly configure discriminator. ***",
static_cast<unsigned>(defaultTestDiscriminator));
options.payload.discriminator = defaultTestDiscriminator;
}

// Default to minimum PBKDF iterations
uint32_t spake2pIterationCount = chip::Crypto::kSpake2p_Min_PBKDF_Iterations;
if (options.spake2pIterations != 0)
{
spake2pIterationCount = options.spake2pIterations;
}
ChipLogError(Support, "PASE PBKDF iterations set to %u", static_cast<unsigned>(spake2pIterationCount));

return provider.Init(options.spake2pVerifier, options.spake2pSalt, spake2pIterationCount, setupPasscode,
options.payload.discriminator);
}

CHIP_ERROR InitConfigurationManager(ConfigurationManagerImpl & configManager, LinuxDeviceOptions & options)
{
if (options.payload.vendorID != 0)
{
configManager.StoreVendorId(options.payload.vendorID);
}

if (options.payload.productID != 0)
{
configManager.StoreProductId(options.payload.productID);
}

return CHIP_NO_ERROR;
}

void Cleanup()
{
#if CHIP_CONFIG_TRANSPORT_TRACE_ENABLED
Expand Down Expand Up @@ -305,28 +229,34 @@ int ChipLinuxAppInit(int argc, char ** argv, OptionSet * customOptions)

// Init the commissionable data provider based on command line options
// to handle custom verifiers, discriminators, etc.
err = InitCommissionableDataProvider(gCommissionableDataProvider, LinuxDeviceOptions::GetInstance());
err = chip::examples::InitCommissionableDataProvider(gCommissionableDataProvider, LinuxDeviceOptions::GetInstance());
SuccessOrExit(err);
DeviceLayer::SetCommissionableDataProvider(&gCommissionableDataProvider);

err = InitConfigurationManager(reinterpret_cast<ConfigurationManagerImpl &>(ConfigurationMgr()),
LinuxDeviceOptions::GetInstance());
err = chip::examples::InitConfigurationManager(reinterpret_cast<ConfigurationManagerImpl &>(ConfigurationMgr()),
LinuxDeviceOptions::GetInstance());
SuccessOrExit(err);

err = GetSetupPayload(LinuxDeviceOptions::GetInstance().payload, rendezvousFlags);
SuccessOrExit(err);

ConfigurationMgr().LogDeviceConfig();

PrintOnboardingCodes(LinuxDeviceOptions::GetInstance().payload);
{
ChipLogProgress(NotSpecified, "==== Onboarding payload for Standard Commissioning Flow ====");
PrintOnboardingCodes(LinuxDeviceOptions::GetInstance().payload);
}

// For testing of manual pairing code with custom commissioning flow
err = GetSetupPayload(LinuxDeviceOptions::GetInstance().payload, rendezvousFlags);
SuccessOrExit(err);
{
// For testing of manual pairing code with custom commissioning flow
ChipLogProgress(NotSpecified, "==== Onboarding payload for Custom Commissioning Flows ====");
err = GetSetupPayload(LinuxDeviceOptions::GetInstance().payload, rendezvousFlags);
SuccessOrExit(err);

LinuxDeviceOptions::GetInstance().payload.commissioningFlow = chip::CommissioningFlow::kCustom;
LinuxDeviceOptions::GetInstance().payload.commissioningFlow = chip::CommissioningFlow::kCustom;

PrintOnboardingCodes(LinuxDeviceOptions::GetInstance().payload);
PrintOnboardingCodes(LinuxDeviceOptions::GetInstance().payload);
}

#if defined(PW_RPC_ENABLED)
rpc::Init();
Expand Down
2 changes: 2 additions & 0 deletions examples/platform/linux/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ source_set("app-main") {
sources = [
"AppMain.cpp",
"AppMain.h",
"CommissionableInit.cpp",
"CommissionableInit.h",
"CommissioneeShellCommands.cpp",
"CommissioneeShellCommands.h",
"CommissionerMain.cpp",
Expand Down
Loading

0 comments on commit 4726803

Please sign in to comment.