From 2168416b292dfc3c3ec471c590fb2e64cfbbff7b Mon Sep 17 00:00:00 2001 From: Boris Zbarsky Date: Mon, 23 Jan 2023 10:47:19 -0500 Subject: [PATCH] Fix a startup crash when a node wants to advertise as a commissioner but not as commissionable. (#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. --- src/app/server/Dnssd.cpp | 105 ++++++++++++++++++++------------------- 1 file changed, 54 insertions(+), 51 deletions(-) diff --git a/src/app/server/Dnssd.cpp b/src/app/server/Dnssd.cpp index 6c7d6d908eddc3..29dd3d016daadf 100644 --- a/src/app/server/Dnssd.cpp +++ b/src/app/server/Dnssd.cpp @@ -219,22 +219,6 @@ CHIP_ERROR DnssdServer::Advertise(bool commissionableNode, chip::Dnssd::Commissi advertiseParameters.SetProductId(chip::Optional::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((discriminator >> 8) & 0x0F)) - .SetLongDiscriminator(discriminator); - if (DeviceLayer::ConfigurationMgr().IsCommissionableDeviceTypeEnabled() && DeviceLayer::ConfigurationMgr().GetDeviceTypeId(val32) == CHIP_NO_ERROR) { @@ -248,52 +232,71 @@ CHIP_ERROR DnssdServer::Advertise(bool commissionableNode, chip::Dnssd::Commissi advertiseParameters.SetDeviceName(chip::Optional::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::Value(rotatingDeviceIdHexBuffer)); -#endif - advertiseParameters.SetLocalMRPConfig(GetLocalMRPConfig()).SetTcpSupported(Optional(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::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::Value(pairingInst)); - } - } - else - { - if (DeviceLayer::ConfigurationMgr().GetSecondaryPairingHint(value) != CHIP_NO_ERROR) - { - ChipLogDetail(Discovery, "DNS-SD Pairing Hint not set"); - } - else - { - advertiseParameters.SetPairingHint(chip::Optional::Value(value)); - } + // Override discriminator with temporary one if one is set + discriminator = mEphemeralDiscriminator.ValueOr(discriminator); + + advertiseParameters.SetShortDiscriminator(static_cast((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::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::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::Value(pairingInst)); + } } else { - advertiseParameters.SetPairingInstruction(chip::Optional::Value(pairingInst)); + if (DeviceLayer::ConfigurationMgr().GetSecondaryPairingHint(value) != CHIP_NO_ERROR) + { + ChipLogDetail(Discovery, "DNS-SD Pairing Hint not set"); + } + else + { + advertiseParameters.SetPairingHint(chip::Optional::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::Value(pairingInst)); + } } }