From 42d86ed832511a361c00c04473b005426270a732 Mon Sep 17 00:00:00 2001 From: Abtin Keshavarzian Date: Mon, 12 Jul 2021 17:25:02 -0700 Subject: [PATCH] [dns-client] feature to auto set the default server address (#6787) This commit adds a new feature in DNS client to automatically set and update the server's IPv6 address in the default config. This is done only when user does not explicitly set or specify the server address. This behavior requires SRP client and its auto-start feature to be enabled. SRP client will then monitor the Thread Network Data for DNS/SRP Service entries to select an SRP server. The selected SRP server address is also set as the DNS server address in the default config. This commit also adds `test_dns_client_config_auto_start.py` which verifies the behavior of the new feature. In particular, that the DNS client's default config gets changed and mirrors the SRP client selected server address (when not set by user) and that it remains unchanged if user explicitly sets the address in the default config. --- include/openthread/dns_client.h | 6 + include/openthread/instance.h | 2 +- src/core/config/dns_client.h | 18 +++ src/core/net/dns_client.cpp | 26 +++ src/core/net/dns_client.hpp | 25 +++ src/core/net/srp_client.cpp | 13 +- src/core/net/srp_client.hpp | 8 + tests/scripts/thread-cert/Makefile.am | 2 + tests/scripts/thread-cert/node.py | 26 +++ .../test_dns_client_config_auto_start.py | 149 ++++++++++++++++++ 10 files changed, 272 insertions(+), 3 deletions(-) create mode 100755 tests/scripts/thread-cert/test_dns_client_config_auto_start.py diff --git a/include/openthread/dns_client.h b/include/openthread/dns_client.h index b24317d0d..e212062ad 100644 --- a/include/openthread/dns_client.h +++ b/include/openthread/dns_client.h @@ -124,6 +124,12 @@ const otDnsQueryConfig *otDnsClientGetDefaultConfig(otInstance *aInstance); * (value zero). The unspecified fields are replaced by the corresponding OT config option definitions * `OPENTHREAD_CONFIG_DNS_CLIENT_DEFAULT_{}` to form the default query config. * + * When `OPENTHREAD_CONFIG_DNS_CLIENT_DEFAULT_SERVER_ADDRESS_AUTO_SET_ENABLE` is enabled, the server's IPv6 address in + * the default config is automatically set and updated by DNS client. This is done only when user does not explicitly + * set or specify it. This behavior requires SRP client and its auto-start feature to be enabled. SRP client will then + * monitor the Thread Network Data for DNS/SRP Service entries to select an SRP server. The selected SRP server address + * is also set as the DNS server address in the default config. + * * @param[in] aInstance A pointer to an OpenThread instance. * @param[in] aConfig A pointer to the new query config to use as default. * diff --git a/include/openthread/instance.h b/include/openthread/instance.h index 621bd6cda..5b6a2cae6 100644 --- a/include/openthread/instance.h +++ b/include/openthread/instance.h @@ -53,7 +53,7 @@ extern "C" { * @note This number versions both OpenThread platform and user APIs. * */ -#define OPENTHREAD_API_VERSION (136) +#define OPENTHREAD_API_VERSION (137) /** * @addtogroup api-instance diff --git a/src/core/config/dns_client.h b/src/core/config/dns_client.h index 817d7ab25..e0a9ec894 100644 --- a/src/core/config/dns_client.h +++ b/src/core/config/dns_client.h @@ -35,6 +35,8 @@ #ifndef CONFIG_DNS_CLIENT_H_ #define CONFIG_DNS_CLIENT_H_ +#include "config/srp_client.h" + /** * @def OPENTHREAD_CONFIG_DNS_CLIENT_ENABLE * @@ -77,6 +79,22 @@ #define OPENTHREAD_CONFIG_DNS_CLIENT_SERVICE_DISCOVERY_ENABLE 1 #endif +/** + * @def OPENTHREAD_CONFIG_DNS_CLIENT_DEFAULT_SERVER_ADDRESS_AUTO_SET_ENABLE + * + * Set to 1 for DNS client to automatically set and update the server IPv6 address in the default config (when it is + * not explicitly set by user). + * + * This feature requires SRP client and its auto-start feature to be also enabled. SRP client will then monitor the + * Thread Network Data for DNS/SRP Service entries to select an SRP server. The selected SRP server address is also set + * as the DNS server address in the default config. + * + */ +#ifndef OPENTHREAD_CONFIG_DNS_CLIENT_DEFAULT_SERVER_ADDRESS_AUTO_SET_ENABLE +#define OPENTHREAD_CONFIG_DNS_CLIENT_DEFAULT_SERVER_ADDRESS_AUTO_SET_ENABLE \ + (OPENTHREAD_CONFIG_SRP_CLIENT_ENABLE && OPENTHREAD_CONFIG_SRP_CLIENT_AUTO_START_API_ENABLE) +#endif + /** * @def OPENTHREAD_CONFIG_DNS_CLIENT_DEFAULT_SERVER_IP6_ADDRESS * diff --git a/src/core/net/dns_client.cpp b/src/core/net/dns_client.cpp index c7d68808b..94980c0b5 100644 --- a/src/core/net/dns_client.cpp +++ b/src/core/net/dns_client.cpp @@ -557,6 +557,9 @@ Client::Client(Instance &aInstance) , mSocket(aInstance) , mTimer(aInstance, Client::HandleTimer) , mDefaultConfig(QueryConfig::kInitFromDefaults) +#if OPENTHREAD_CONFIG_DNS_CLIENT_DEFAULT_SERVER_ADDRESS_AUTO_SET_ENABLE + , mUserDidSetDefaultAddress(false) +#endif { static_assert(kIp6AddressQuery == 0, "kIp6AddressQuery value is not correct"); #if OPENTHREAD_CONFIG_DNS_CLIENT_NAT64_ENABLE @@ -599,13 +602,36 @@ void Client::SetDefaultConfig(const QueryConfig &aQueryConfig) QueryConfig startingDefault(QueryConfig::kInitFromDefaults); mDefaultConfig.SetFrom(aQueryConfig, startingDefault); + +#if OPENTHREAD_CONFIG_DNS_CLIENT_DEFAULT_SERVER_ADDRESS_AUTO_SET_ENABLE + mUserDidSetDefaultAddress = !aQueryConfig.GetServerSockAddr().GetAddress().IsUnspecified(); + UpdateDefaultConfigAddress(); +#endif } void Client::ResetDefaultConfig(void) { mDefaultConfig = QueryConfig(QueryConfig::kInitFromDefaults); + +#if OPENTHREAD_CONFIG_DNS_CLIENT_DEFAULT_SERVER_ADDRESS_AUTO_SET_ENABLE + mUserDidSetDefaultAddress = false; + UpdateDefaultConfigAddress(); +#endif } +#if OPENTHREAD_CONFIG_DNS_CLIENT_DEFAULT_SERVER_ADDRESS_AUTO_SET_ENABLE +void Client::UpdateDefaultConfigAddress(void) +{ + const Ip6::Address &srpServerAddr = Get().GetServerAddress().GetAddress(); + + if (!mUserDidSetDefaultAddress && Get().IsServerSelectedByAutoStart() && + !srpServerAddr.IsUnspecified()) + { + mDefaultConfig.GetServerSockAddr().SetAddress(srpServerAddr); + } +} +#endif + Error Client::ResolveAddress(const char * aHostName, AddressCallback aCallback, void * aContext, diff --git a/src/core/net/dns_client.hpp b/src/core/net/dns_client.hpp index b0a9b7c29..bcce1ba80 100644 --- a/src/core/net/dns_client.hpp +++ b/src/core/net/dns_client.hpp @@ -48,6 +48,18 @@ * This file includes definitions for the DNS client. */ +#if OPENTHREAD_CONFIG_DNS_CLIENT_DEFAULT_SERVER_ADDRESS_AUTO_SET_ENABLE + +#if !OPENTHREAD_CONFIG_SRP_CLIENT_ENABLE +#error "DNS_CLIENT_DEFAULT_SERVER_ADDRESS_AUTO_SET_ENABLE requires OPENTHREAD_CONFIG_SRP_CLIENT_ENABLE" +#endif + +#if !OPENTHREAD_CONFIG_SRP_CLIENT_AUTO_START_API_ENABLE +#error "DNS_CLIENT_DEFAULT_SERVER_ADDRESS_AUTO_SET_ENABLE requires OPENTHREAD_CONFIG_SRP_CLIENT_AUTO_START_API_ENABLE" +#endif + +#endif + /** * This struct represents an opaque (and empty) type for a response to an address resolution DNS query. * @@ -77,6 +89,11 @@ struct otDnsServiceResponse #endif // OPENTHREAD_CONFIG_DNS_CLIENT_SERVICE_DISCOVERY_ENABLE namespace ot { + +namespace Srp { +class Client; +} + namespace Dns { /** @@ -85,6 +102,8 @@ namespace Dns { */ class Client : public InstanceLocator, private NonCopyable { + friend class ot::Srp::Client; + typedef Message Query; // `Message` is used to save `Query` related info. public: @@ -722,6 +741,9 @@ private: #if OPENTHREAD_CONFIG_DNS_CLIENT_NAT64_ENABLE Error CheckAddressResponse(Response &aResponse, Error aResponseError) const; #endif +#if OPENTHREAD_CONFIG_DNS_CLIENT_DEFAULT_SERVER_ADDRESS_AUTO_SET_ENABLE + void UpdateDefaultConfigAddress(void); +#endif static const uint8_t kQuestionCount[]; static const uint16_t *kQuestionRecordTypes[]; @@ -739,6 +761,9 @@ private: QueryList mQueries; TimerMilli mTimer; QueryConfig mDefaultConfig; +#if OPENTHREAD_CONFIG_DNS_CLIENT_DEFAULT_SERVER_ADDRESS_AUTO_SET_ENABLE + bool mUserDidSetDefaultAddress; +#endif }; } // namespace Dns diff --git a/src/core/net/srp_client.cpp b/src/core/net/srp_client.cpp index f28fcc6ba..c5472c3c5 100644 --- a/src/core/net/srp_client.cpp +++ b/src/core/net/srp_client.cpp @@ -225,8 +225,17 @@ Error Client::Start(const Ip6::SockAddr &aServerSockAddr, Requester aRequester) #if OPENTHREAD_CONFIG_SRP_CLIENT_AUTO_START_API_ENABLE mAutoStartDidSelectServer = (aRequester == kRequesterAuto); - VerifyOrExit((aRequester == kRequesterAuto) && (mAutoStartCallback != nullptr)); - mAutoStartCallback(&aServerSockAddr, mAutoStartContext); + if (mAutoStartDidSelectServer) + { +#if OPENTHREAD_CONFIG_DNS_CLIENT_ENABLE && OPENTHREAD_CONFIG_DNS_CLIENT_DEFAULT_SERVER_ADDRESS_AUTO_SET_ENABLE + Get().UpdateDefaultConfigAddress(); +#endif + + if (mAutoStartCallback != nullptr) + { + mAutoStartCallback(&aServerSockAddr, mAutoStartContext); + } + } #endif exit: diff --git a/src/core/net/srp_client.hpp b/src/core/net/srp_client.hpp index d6f428522..e965b4321 100644 --- a/src/core/net/srp_client.hpp +++ b/src/core/net/srp_client.hpp @@ -364,6 +364,14 @@ public: * */ bool IsAutoStartModeEnabled(void) const { return mAutoStartModeEnabled; } + + /** + * This method indicates whether or not the current SRP server's address is selected by auto-start. + * + * @returns TRUE if the SRP server's address is selected by auto-start, FALSE otherwise. + * + */ + bool IsServerSelectedByAutoStart(void) const { return mAutoStartDidSelectServer; } #endif // OPENTHREAD_CONFIG_SRP_CLIENT_AUTO_START_API_ENABLE /** diff --git a/tests/scripts/thread-cert/Makefile.am b/tests/scripts/thread-cert/Makefile.am index 66881c697..1e71841de 100644 --- a/tests/scripts/thread-cert/Makefile.am +++ b/tests/scripts/thread-cert/Makefile.am @@ -160,6 +160,7 @@ EXTRA_DIST = \ test_crypto.py \ test_dataset_updater.py \ test_diag.py \ + test_dns_client_config_auto_start.py \ test_dnssd.py \ test_ipv6.py \ test_ipv6_fragmentation.py \ @@ -219,6 +220,7 @@ check_SCRIPTS = \ test_crypto.py \ test_dataset_updater.py \ test_diag.py \ + test_dns_client_config_auto_start.py \ test_dnssd.py \ test_ipv6.py \ test_ipv6_fragmentation.py \ diff --git a/tests/scripts/thread-cert/node.py b/tests/scripts/thread-cert/node.py index d45bc12ed..e5aac2aad 100755 --- a/tests/scripts/thread-cert/node.py +++ b/tests/scripts/thread-cert/node.py @@ -2487,6 +2487,32 @@ class NodeImpl: self.send_command(cmd) self._expect_done() + def dns_get_config(self): + """ + Returns the DNS config as a list of property dictionary (string key and string value). + + Example output: + { + 'Server': '[fd00:0:0:0:0:0:0:1]:1234' + 'ResponseTimeout': '5000 ms' + 'MaxTxAttempts': '2' + 'RecursionDesired': 'no' + } + """ + cmd = f'dns config' + self.send_command(cmd) + output = self._expect_command_output(cmd) + config = {} + for line in output: + k, v = line.split(': ') + config[k] = v + return config + + def dns_set_config(self, config): + cmd = f'dns config {config}' + self.send_command(cmd) + self._expect_done() + def dns_resolve(self, hostname, server=None, port=53): cmd = f'dns resolve {hostname}' if server is not None: diff --git a/tests/scripts/thread-cert/test_dns_client_config_auto_start.py b/tests/scripts/thread-cert/test_dns_client_config_auto_start.py new file mode 100755 index 000000000..c6b38071a --- /dev/null +++ b/tests/scripts/thread-cert/test_dns_client_config_auto_start.py @@ -0,0 +1,149 @@ +#!/usr/bin/env python3 +# +# Copyright (c) 2021, The OpenThread Authors. +# All rights reserved. +# +# Redistribution and use in source and binary forms, with or without +# modification, are permitted provided that the following conditions are met: +# 1. Redistributions of source code must retain the above copyright +# notice, this list of conditions and the following disclaimer. +# 2. Redistributions in binary form must reproduce the above copyright +# notice, this list of conditions and the following disclaimer in the +# documentation and/or other materials provided with the distribution. +# 3. Neither the name of the copyright holder nor the +# names of its contributors may be used to endorse or promote products +# derived from this software without specific prior written permission. +# +# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" +# AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE +# IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE +# ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE +# LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR +# CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF +# SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS +# INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN +# CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) +# ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE +# POSSIBILITY OF SUCH DAMAGE. +# + +import unittest + +import command +import thread_cert + +# Test description: +# +# This test verifies the behavior of DNS client feature where the SRP +# server address (from SRP client auto-start feature) is used as the +# default IPv6 address for DNS resolver config when user does not +# explicitly set a default address +# +# Topology: two nodes +# +# leader (SRP server & client & DNS client) --- router (SRP client & DNS client) +# + +LEADER = 1 +ROUTER = 2 + +DEFAULT_ADDRESS = 'fd00:1:2:3:4:5:6:7' + + +class DnsClientConfigAutoStart(thread_cert.TestCase): + USE_MESSAGE_FACTORY = False + SUPPORT_NCP = False + + TOPOLOGY = { + LEADER: { + 'name': 'LEADER', + 'mode': 'rdn', + }, + ROUTER: { + 'name': 'ROUTER', + 'mode': 'rdn', + }, + } + + def test(self): + leader = self.nodes[LEADER] + router = self.nodes[ROUTER] + + # Form network. + + leader.start() + self.simulator.go(5) + self.assertEqual(leader.get_state(), 'leader') + + router.start() + self.simulator.go(5) + self.assertEqual(router.get_state(), 'router') + + # On leader set DNS config explicitly. + + leader.dns_set_config(DEFAULT_ADDRESS) + config = leader.dns_get_config() + self.assertEqual(config['Server'], '[{}]:53'.format(DEFAULT_ADDRESS)) + + # Start leader to act as SRP server. + + leader.srp_server_set_enabled(True) + + # Enable SRP client auto start on both nodes. + + leader.srp_client_enable_auto_start_mode() + router.srp_client_enable_auto_start_mode() + self.simulator.go(5) + + # Verify that on router the default DNS config is changed + # to the same address being used by SRP client. + + srp_server_address = router.srp_client_get_server_address() + self.assertEqual(leader.srp_client_get_server_address(), srp_server_address) + + config = router.dns_get_config() + self.assertEqual(config['Server'], '[{}]:53'.format(srp_server_address)) + + # Verify that on leader the default DNS config remains + # as before (the address explicitly set earlier). + + config = leader.dns_get_config() + self.assertEqual(config['Server'], '[{}]:53'.format(DEFAULT_ADDRESS)) + + # On leader clear DNS config (the explicitly set address) + # and verify that it adopts the SRP server address. + + leader.dns_set_config("::") + config = leader.dns_get_config() + self.assertEqual(config['Server'], '[{}]:53'.format(srp_server_address)) + + # On leader set DNS config explicitly again. + + leader.dns_set_config(DEFAULT_ADDRESS) + config = leader.dns_get_config() + self.assertEqual(config['Server'], '[{}]:53'.format(DEFAULT_ADDRESS)) + + # Stop SRP server on leader and start it on router. + + leader.srp_server_set_enabled(False) + router.srp_server_set_enabled(True) + self.simulator.go(5) + + # Verify that SRP client on router switched to new SRP server. + + self.assertNotEqual(srp_server_address, router.srp_client_get_server_address()) + self.assertEqual(router.srp_client_get_server_address(), leader.srp_client_get_server_address()) + + # Verify that config on router gets changed while on leader + # it remains unchanged. + + config = router.dns_get_config() + srp_server_address = router.srp_client_get_server_address() + self.assertEqual(config['Server'], '[{}]:53'.format(srp_server_address)) + + config = leader.dns_get_config() + self.assertEqual(config['Server'], '[{}]:53'.format(DEFAULT_ADDRESS)) + + +if __name__ == '__main__': + unittest.main()