Skip to content

Commit

Permalink
Fix a startup crash when a node wants to advertise as a commissioner …
Browse files Browse the repository at this point in the history 
…but not as commissionable. (project-chip#24565)

In DnssdServer::Advertise we were trying to get things like a discriminator and
other commissionee information even if we were just advertising as a
commissioner (commissionableNode was false).  This could lead to crashes due to
calling things like GetCommissionableDataProvider() when we are not in fact
commissionable and don't have one.

The fix is to reorder things a bit so we set up all the advertiseParameters
state that both commissioners and commissionable share, and then condition all
the commissionable-only bits on commissionableNode.
  • Loading branch information
@bzbarsky-apple
bzbarsky-apple authored and David Lechner committed Mar 22, 2023
1 parent 56acf43 commit d0b485c
Showing 1 changed file with 54 additions and 51 deletions.
105 changes: 54 additions & 51 deletions src/app/server/Dnssd.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -219,22 +219,6 @@ CHIP_ERROR DnssdServer::Advertise(bool commissionableNode, chip::Dnssd::Commissi
advertiseParameters.SetProductId(chip::Optional<uint16_t>::Value(value));
}

uint16_t discriminator = 0;
CHIP_ERROR error = DeviceLayer::GetCommissionableDataProvider()->GetSetupDiscriminator(discriminator);
if (error != CHIP_NO_ERROR)
{
ChipLogError(Discovery,
"Setup discriminator read error (%" CHIP_ERROR_FORMAT ")! Critical error, will not be commissionable.",
error.Format());
return error;
}

// Override discriminator with temporary one if one is set
discriminator = mEphemeralDiscriminator.ValueOr(discriminator);

advertiseParameters.SetShortDiscriminator(static_cast<uint8_t>((discriminator >> 8) & 0x0F))
.SetLongDiscriminator(discriminator);

if (DeviceLayer::ConfigurationMgr().IsCommissionableDeviceTypeEnabled() &&
DeviceLayer::ConfigurationMgr().GetDeviceTypeId(val32) == CHIP_NO_ERROR)
{
Expand All @@ -248,52 +232,71 @@ CHIP_ERROR DnssdServer::Advertise(bool commissionableNode, chip::Dnssd::Commissi
advertiseParameters.SetDeviceName(chip::Optional<const char *>::Value(deviceName));
}

#if CHIP_ENABLE_ROTATING_DEVICE_ID && defined(CHIP_DEVICE_CONFIG_ROTATING_DEVICE_ID_UNIQUE_ID)
char rotatingDeviceIdHexBuffer[RotatingDeviceId::kHexMaxLength];
ReturnErrorOnFailure(GenerateRotatingDeviceId(rotatingDeviceIdHexBuffer, ArraySize(rotatingDeviceIdHexBuffer)));
advertiseParameters.SetRotatingDeviceId(chip::Optional<const char *>::Value(rotatingDeviceIdHexBuffer));
#endif

advertiseParameters.SetLocalMRPConfig(GetLocalMRPConfig()).SetTcpSupported(Optional<bool>(INET_CONFIG_ENABLE_TCP_ENDPOINT));

if (!HaveOperationalCredentials())
if (commissionableNode)
{
if (DeviceLayer::ConfigurationMgr().GetInitialPairingHint(value) != CHIP_NO_ERROR)
{
ChipLogDetail(Discovery, "DNS-SD Pairing Hint not set");
}
else
uint16_t discriminator = 0;
CHIP_ERROR error = DeviceLayer::GetCommissionableDataProvider()->GetSetupDiscriminator(discriminator);
if (error != CHIP_NO_ERROR)
{
advertiseParameters.SetPairingHint(chip::Optional<uint16_t>::Value(value));
ChipLogError(Discovery,
"Setup discriminator read error (%" CHIP_ERROR_FORMAT ")! Critical error, will not be commissionable.",
error.Format());
return error;
}

if (DeviceLayer::ConfigurationMgr().GetInitialPairingInstruction(pairingInst, sizeof(pairingInst)) != CHIP_NO_ERROR)
{
ChipLogDetail(Discovery, "DNS-SD Pairing Instruction not set");
}
else
{
advertiseParameters.SetPairingInstruction(chip::Optional<const char *>::Value(pairingInst));
}
}
else
{
if (DeviceLayer::ConfigurationMgr().GetSecondaryPairingHint(value) != CHIP_NO_ERROR)
{
ChipLogDetail(Discovery, "DNS-SD Pairing Hint not set");
}
else
{
advertiseParameters.SetPairingHint(chip::Optional<uint16_t>::Value(value));
}
// Override discriminator with temporary one if one is set
discriminator = mEphemeralDiscriminator.ValueOr(discriminator);

advertiseParameters.SetShortDiscriminator(static_cast<uint8_t>((discriminator >> 8) & 0x0F))
.SetLongDiscriminator(discriminator);

#if CHIP_ENABLE_ROTATING_DEVICE_ID && defined(CHIP_DEVICE_CONFIG_ROTATING_DEVICE_ID_UNIQUE_ID)
char rotatingDeviceIdHexBuffer[RotatingDeviceId::kHexMaxLength];
ReturnErrorOnFailure(GenerateRotatingDeviceId(rotatingDeviceIdHexBuffer, ArraySize(rotatingDeviceIdHexBuffer)));
advertiseParameters.SetRotatingDeviceId(chip::Optional<const char *>::Value(rotatingDeviceIdHexBuffer));
#endif

if (DeviceLayer::ConfigurationMgr().GetSecondaryPairingInstruction(pairingInst, sizeof(pairingInst)) != CHIP_NO_ERROR)
if (!HaveOperationalCredentials())
{
ChipLogDetail(Discovery, "DNS-SD Pairing Instruction not set");
if (DeviceLayer::ConfigurationMgr().GetInitialPairingHint(value) != CHIP_NO_ERROR)
{
ChipLogDetail(Discovery, "DNS-SD Pairing Hint not set");
}
else
{
advertiseParameters.SetPairingHint(chip::Optional<uint16_t>::Value(value));
}

if (DeviceLayer::ConfigurationMgr().GetInitialPairingInstruction(pairingInst, sizeof(pairingInst)) != CHIP_NO_ERROR)
{
ChipLogDetail(Discovery, "DNS-SD Pairing Instruction not set");
}
else
{
advertiseParameters.SetPairingInstruction(chip::Optional<const char *>::Value(pairingInst));
}
}
else
{
advertiseParameters.SetPairingInstruction(chip::Optional<const char *>::Value(pairingInst));
if (DeviceLayer::ConfigurationMgr().GetSecondaryPairingHint(value) != CHIP_NO_ERROR)
{
ChipLogDetail(Discovery, "DNS-SD Pairing Hint not set");
}
else
{
advertiseParameters.SetPairingHint(chip::Optional<uint16_t>::Value(value));
}

if (DeviceLayer::ConfigurationMgr().GetSecondaryPairingInstruction(pairingInst, sizeof(pairingInst)) != CHIP_NO_ERROR)
{
ChipLogDetail(Discovery, "DNS-SD Pairing Instruction not set");
}
else
{
advertiseParameters.SetPairingInstruction(chip::Optional<const char *>::Value(pairingInst));
}
}
}

Expand Down

0 comments on commit d0b485c

Please sign in to comment.