From 2dab08a0a11f4713df8bfc81b61bea4b0aa4cdbb Mon Sep 17 00:00:00 2001 From: Vivien Nicolas Date: Mon, 24 Oct 2022 17:01:36 +0200 Subject: [PATCH] [SetUpCodePairer] Ask Dnssd to reconfirm discovered addresses if connecting to them ends with a CHIP_ERROR_TIMEOUT --- src/controller/SetUpCodePairer.cpp | 74 ++++++++++++++++++++---------- src/controller/SetUpCodePairer.h | 18 +++++++- 2 files changed, 68 insertions(+), 24 deletions(-) diff --git a/src/controller/SetUpCodePairer.cpp b/src/controller/SetUpCodePairer.cpp index 64b2459ce5faeb..09ee3a012a4021 100644 --- a/src/controller/SetUpCodePairer.cpp +++ b/src/controller/SetUpCodePairer.cpp @@ -223,7 +223,7 @@ bool SetUpCodePairer::ConnectToDiscoveredDevice() // Remove it from the queue before we try to connect, in case the // connection attempt fails and calls right back into us to try the next // thing. - RendezvousParameters params(mDiscoveredParameters.front()); + SetUpCodePairerParameters params(mDiscoveredParameters.front()); mDiscoveredParameters.pop(); params.SetSetupPINCode(mSetUpPINCode); @@ -237,6 +237,11 @@ bool SetUpCodePairer::ConnectToDiscoveredDevice() // Handle possibly-sync call backs from attempts to establish PASE. ExpectPASEEstablishment(); + if (params.GetPeerAddress().GetTransportType() == Transport::Type::kUdp) + { + mCurrentPASEParameters.SetValue(params); + } + CHIP_ERROR err; if (mConnectionType == SetupCodePairerBehaviour::kCommission) { @@ -267,9 +272,7 @@ void SetUpCodePairer::OnDiscoveredDeviceOverBle(BLE_CONNECTION_OBJECT connObj) mWaitingForDiscovery[kBLETransport] = false; - Transport::PeerAddress peerAddress = Transport::PeerAddress::BLE(); - mDiscoveredParameters.emplace(); - mDiscoveredParameters.back().SetPeerAddress(peerAddress).SetConnectionObject(connObj); + mDiscoveredParameters.emplace(connObj); ConnectToDiscoveredDevice(); } @@ -338,25 +341,7 @@ void SetUpCodePairer::NotifyCommissionableDeviceDiscovered(const Dnssd::Discover ChipLogProgress(Controller, "Discovered device to be commissioned over DNS-SD"); - Inet::InterfaceId interfaceId = - nodeData.resolutionData.ipAddress[0].IsIPv6LinkLocal() ? nodeData.resolutionData.interfaceId : Inet::InterfaceId::Null(); - Transport::PeerAddress peerAddress = - Transport::PeerAddress::UDP(nodeData.resolutionData.ipAddress[0], nodeData.resolutionData.port, interfaceId); - mDiscoveredParameters.emplace(); - mDiscoveredParameters.back().SetPeerAddress(peerAddress); - - if (nodeData.resolutionData.mrpRetryIntervalIdle.HasValue()) - { - auto interval = nodeData.resolutionData.mrpRetryIntervalIdle.Value(); - mDiscoveredParameters.back().SetIdleInterval(interval); - } - - if (nodeData.resolutionData.mrpRetryIntervalActive.HasValue()) - { - auto interval = nodeData.resolutionData.mrpRetryIntervalActive.Value(); - mDiscoveredParameters.back().SetActiveInterval(interval); - } - + mDiscoveredParameters.emplace(nodeData.resolutionData); ConnectToDiscoveredDevice(); } @@ -412,6 +397,7 @@ void SetUpCodePairer::ResetDiscoveryState() mDiscoveredParameters.pop(); } + mCurrentPASEParameters.ClearValue(); mLastPASEError = CHIP_NO_ERROR; } @@ -488,6 +474,21 @@ void SetUpCodePairer::OnPairingComplete(CHIP_ERROR error) return; } + // It may happen that there is a stale DNS entry. If so, ReconfirmRecord will flush + // the record from the daemon cache once it determines that it is invalid. + // It may not help for this particular resolve, but may help subsequent resolves. + if (CHIP_ERROR_TIMEOUT == error && mCurrentPASEParameters.HasValue()) + { + const auto & params = mCurrentPASEParameters.Value(); + auto & ip = params.GetPeerAddress().GetIPAddress(); + auto err = Dnssd::Resolver::Instance().ReconfirmRecord(params.mHostName, ip, params.mInterfaceId); + if (CHIP_NO_ERROR != err && CHIP_ERROR_NOT_IMPLEMENTED != err) + { + ChipLogError(Controller, "Error when verifying the validity of an address: %" CHIP_ERROR_FORMAT, err.Format()); + } + } + mCurrentPASEParameters.ClearValue(); + // We failed to establish PASE. Try the next thing we have discovered, if // any. if (TryNextRendezvousParameters()) @@ -541,5 +542,32 @@ void SetUpCodePairer::OnDeviceDiscoveredTimeoutCallback(System::Layer * layer, v } } +SetUpCodePairerParameters::SetUpCodePairerParameters(const Dnssd::CommonResolutionData & data) +{ + mInterfaceId = data.interfaceId; + Platform::CopyString(mHostName, data.hostName); + + auto & ip = data.ipAddress[0]; + SetPeerAddress(Transport::PeerAddress::UDP(ip, data.port, ip.IsIPv6LinkLocal() ? data.interfaceId : Inet::InterfaceId::Null())); + + if (data.mrpRetryIntervalIdle.HasValue()) + { + SetIdleInterval(data.mrpRetryIntervalIdle.Value()); + } + + if (data.mrpRetryIntervalActive.HasValue()) + { + SetActiveInterval(data.mrpRetryIntervalActive.Value()); + } +} + +#if CONFIG_NETWORK_LAYER_BLE +SetUpCodePairerParameters::SetUpCodePairerParameters(BLE_CONNECTION_OBJECT connObj) +{ + Transport::PeerAddress peerAddress = Transport::PeerAddress::BLE(); + SetPeerAddress(peerAddress).SetConnectionObject(connObj); +} +#endif // CONFIG_NETWORK_LAYER_BLE + } // namespace Controller } // namespace chip diff --git a/src/controller/SetUpCodePairer.h b/src/controller/SetUpCodePairer.h index ed1c6f0c2ad5ac..eae15065d75966 100644 --- a/src/controller/SetUpCodePairer.h +++ b/src/controller/SetUpCodePairer.h @@ -48,6 +48,17 @@ namespace Controller { class DeviceCommissioner; +class SetUpCodePairerParameters : public RendezvousParameters +{ +public: + SetUpCodePairerParameters(const Dnssd::CommonResolutionData & data); +#if CONFIG_NETWORK_LAYER_BLE + SetUpCodePairerParameters(BLE_CONNECTION_OBJECT connObj); +#endif // CONFIG_NETWORK_LAYER_BLE + char mHostName[Dnssd::kHostNameMaxLength + 1] = {}; + Inet::InterfaceId mInterfaceId; +}; + enum class SetupCodePairerBehaviour : uint8_t { kCommission, @@ -180,7 +191,12 @@ class DLL_EXPORT SetUpCodePairer : public DevicePairingDelegate // Queue of things we have discovered but not tried connecting to yet. The // general discovery/pairing process will terminate once this queue is empty // and all the booleans in mWaitingForDiscovery are false. - std::queue mDiscoveredParameters; + std::queue mDiscoveredParameters; + + // Current thing we are trying to connect to over UDP. If a PASE connection fails with + // a CHIP_ERROR_TIMEOUT, the discovered parameters will be used to ask the + // mdns daemon to invalidate the + Optional mCurrentPASEParameters; // mWaitingForPASE is true if we have called either // EstablishPASEConnection or PairDevice on mCommissioner and are now just