From 87e6cf59913a9b247086a81f3d4c58205bc34725 Mon Sep 17 00:00:00 2001 From: whd <7058128+superwhd@users.noreply.github.com> Date: Mon, 7 Jun 2021 13:11:11 +0800 Subject: [PATCH] [srp-server] choose different UDP port from previous on reboot (#6702) This commit implements a mechanism to let an SRP server listen to a different UDP port on a reboot. - When an SRP server starts, it reads the UDP port used in the last time from Settings. Then the SRP server listens to the next port in the reserved range. - When SRP server successfully starts, store its UDP port in Settings. --- include/openthread/instance.h | 2 +- include/openthread/platform/settings.h | 1 + src/core/common/settings.cpp | 17 +- src/core/common/settings.hpp | 45 +++- .../config/openthread-core-config-check.h | 5 + src/core/config/srp_server.h | 32 ++- src/core/net/srp_server.cpp | 32 ++- src/core/net/srp_server.hpp | 7 +- src/core/net/udp6.cpp | 7 +- src/core/net/udp6.hpp | 6 + .../test_srp_server_reboot_port.py | 193 ++++++++++++++++++ 11 files changed, 334 insertions(+), 13 deletions(-) create mode 100755 tests/scripts/thread-cert/test_srp_server_reboot_port.py diff --git a/include/openthread/instance.h b/include/openthread/instance.h index 46e74a76f..e3acb7c6b 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 (122) +#define OPENTHREAD_API_VERSION (123) /** * @addtogroup api-instance diff --git a/include/openthread/platform/settings.h b/include/openthread/platform/settings.h index 56a062341..17a68a022 100644 --- a/include/openthread/platform/settings.h +++ b/include/openthread/platform/settings.h @@ -72,6 +72,7 @@ enum OT_SETTINGS_KEY_ON_LINK_PREFIX = 0x000a, ///< On-link prefix for infrastructure link. OT_SETTINGS_KEY_SRP_ECDSA_KEY = 0x000b, ///< SRP client ECDSA public/private key pair. OT_SETTINGS_KEY_SRP_CLIENT_INFO = 0x000c, ///< The SRP client info (selected SRP server address). + OT_SETTINGS_KEY_SRP_SERVER_INFO = 0x000d, ///< The SRP server info (UDP port). }; /** diff --git a/src/core/common/settings.cpp b/src/core/common/settings.cpp index 9c9047e93..82e43c3b7 100644 --- a/src/core/common/settings.cpp +++ b/src/core/common/settings.cpp @@ -98,6 +98,13 @@ void SettingsBase::SrpClientInfo::Log(Action aAction) const } #endif +#if OPENTHREAD_CONFIG_SRP_SERVER_ENABLE && OPENTHREAD_CONFIG_SRP_SERVER_PORT_SWITCH_ENABLE +void SettingsBase::SrpServerInfo::Log(Action aAction) const +{ + otLogInfoCore("[settings] %s SrpServerInfo {port:%u}", ActionToString(aAction), GetPort()); +} +#endif + #endif // OPENTHREAD_CONFIG_LOG_UTIL && (OPENTHREAD_CONFIG_LOG_LEVEL >= OT_LOG_LEVEL_INFO) #if OPENTHREAD_CONFIG_LOG_UTIL && (OPENTHREAD_CONFIG_LOG_LEVEL >= OT_LOG_LEVEL_WARN) @@ -146,6 +153,7 @@ const char *SettingsBase::KeyToString(Key aKey) "OnLinkPrefix", // (10) kKeyOnLinkPrefix "SrpEcdsaKey", // (11) kKeySrpEcdsaKey "SrpClientInfo", // (12) kKeySrpClientInfo + "SrpServerInfo", // (13) kKeySrpServerInfo }; static_assert(1 == kKeyActiveDataset, "kKeyActiveDataset value is incorrect"); @@ -160,8 +168,9 @@ const char *SettingsBase::KeyToString(Key aKey) static_assert(10 == kKeyOnLinkPrefix, "kKeyOnLinkPrefix value is incorrect"); static_assert(11 == kKeySrpEcdsaKey, "kKeySrpEcdsaKey value is incorrect"); static_assert(12 == kKeySrpClientInfo, "kKeySrpClientInfo value is incorrect"); + static_assert(13 == kKeySrpServerInfo, "kKeySrpServerInfo value is incorrect"); - static_assert(kLastKey == kKeySrpClientInfo, "kLastKey is not valid"); + static_assert(kLastKey == kKeySrpServerInfo, "kLastKey is not valid"); OT_ASSERT(aKey <= kLastKey); @@ -432,6 +441,12 @@ void Settings::Log(Action aAction, Error aError, Key aKey, const void *aValue) break; #endif +#if OPENTHREAD_CONFIG_SRP_SERVER_ENABLE && OPENTHREAD_CONFIG_SRP_SERVER_PORT_SWITCH_ENABLE + case kKeySrpServerInfo: + reinterpret_cast(aValue)->Log(aAction); + break; +#endif + default: // For any other keys, we do not want to include the value // in the log, so even if it is given we set `aValue` to diff --git a/src/core/common/settings.hpp b/src/core/common/settings.hpp index c6addd670..c288868bc 100644 --- a/src/core/common/settings.hpp +++ b/src/core/common/settings.hpp @@ -125,9 +125,10 @@ public: kKeyOnLinkPrefix = OT_SETTINGS_KEY_ON_LINK_PREFIX, kKeySrpEcdsaKey = OT_SETTINGS_KEY_SRP_ECDSA_KEY, kKeySrpClientInfo = OT_SETTINGS_KEY_SRP_CLIENT_INFO, + kKeySrpServerInfo = OT_SETTINGS_KEY_SRP_SERVER_INFO, }; - static constexpr Key kLastKey = kKeySrpClientInfo; ///< The last (numerically) enumerator value in `Key`. + static constexpr Key kLastKey = kKeySrpServerInfo; ///< The last (numerically) enumerator value in `Key`. /** * This structure represents the device's own network information for settings storage. @@ -677,6 +678,48 @@ public: #endif // OPENTHREAD_CONFIG_SRP_CLIENT_SAVE_SELECTED_SERVER_ENABLE #endif // OPENTHREAD_CONFIG_SRP_CLIENT_ENABLE +#if OPENTHREAD_CONFIG_SRP_SERVER_ENABLE && OPENTHREAD_CONFIG_SRP_SERVER_PORT_SWITCH_ENABLE + /** + * This structure represents the SRP server info. + * + */ + OT_TOOL_PACKED_BEGIN + class SrpServerInfo : private Clearable + { + friend class Settings; + + public: + static constexpr Key kKey = kKeySrpServerInfo; ///< The associated key. + + /** + * This method initializes the `SrpServerInfo` object. + * + */ + void Init(void) { Clear(); } + + /** + * This method returns the server port number. + * + * @returns The server port number. + * + */ + uint16_t GetPort(void) const { return Encoding::LittleEndian::HostSwap16(mPort); } + + /** + * This method sets the server port number. + * + * @param[in] aPort The server port number. + * + */ + void SetPort(uint16_t aPort) { mPort = Encoding::LittleEndian::HostSwap16(aPort); } + + private: + void Log(Action aAction) const; + + uint16_t mPort; // (in little-endian encoding) + } OT_TOOL_PACKED_END; +#endif // OPENTHREAD_CONFIG_SRP_SERVER_ENABLE && OPENTHREAD_CONFIG_SRP_SERVER_PORT_SWITCH_ENABLE + protected: explicit SettingsBase(Instance &aInstance) : InstanceLocator(aInstance) diff --git a/src/core/config/openthread-core-config-check.h b/src/core/config/openthread-core-config-check.h index f0ede5a56..5e259a1bc 100644 --- a/src/core/config/openthread-core-config-check.h +++ b/src/core/config/openthread-core-config-check.h @@ -509,4 +509,9 @@ "Service numbers are defined in `network_data_servcie.hpp` per spec" #endif +#ifdef OPENTHREAD_CONFIG_SRP_SERVER_UDP_PORT +#error "OPENTHREAD_CONFIG_SRP_SERVER_UDP_PORT was removed. "\ + "You can make OPENTHREAD_CONFIG_SRP_SERVER_UDP_PORT_MIN = OPENTHREAD_CONFIG_SRP_SERVER_UDP_PORT_MAX to specify a static UDP port. " +#endif + #endif // OPENTHREAD_CORE_CONFIG_CHECK_H_ diff --git a/src/core/config/srp_server.h b/src/core/config/srp_server.h index fd72a868c..124a8ec6f 100644 --- a/src/core/config/srp_server.h +++ b/src/core/config/srp_server.h @@ -46,13 +46,37 @@ #endif /** - * @def OPENTHREAD_CONFIG_SRP_SERVER_UDP_PORT + * @def OPENTHREAD_CONFIG_SRP_SERVER_UDP_PORT_MIN * - * Specifies the SRP Server UDP port, and use 0 for ephemeral port. + * Specifies the min port number in the port range reserved for SRP server. * */ -#ifndef OPENTHREAD_CONFIG_SRP_SERVER_UDP_PORT -#define OPENTHREAD_CONFIG_SRP_SERVER_UDP_PORT 0 +#ifndef OPENTHREAD_CONFIG_SRP_SERVER_UDP_PORT_MIN +#define OPENTHREAD_CONFIG_SRP_SERVER_UDP_PORT_MIN 53535 +#endif + +/** + * @def OPENTHREAD_CONFIG_SRP_SERVER_UDP_PORT_MAX + * + * Specifies the max port number in the port range reserved for SRP server. + * + */ +#ifndef OPENTHREAD_CONFIG_SRP_SERVER_UDP_PORT_MAX +#define OPENTHREAD_CONFIG_SRP_SERVER_UDP_PORT_MAX 53554 +#endif + +/** + * @def OPENTHREAD_CONFIG_SRP_SERVER_PORT_SWITCH_ENABLE + * + * Define to 1 to enable SRP server feature to save its own port in non-volatile settings. + * + * When enabled, the SRP server will save its port in the non-volatile settings. On a server + * restart (e.g., due to a device reset) it will restore the port and change to a different one. The info is written to + * Settings after the first service has been registered due to receiving an SRP Update. + * + */ +#ifndef OPENTHREAD_CONFIG_SRP_SERVER_PORT_SWITCH_ENABLE +#define OPENTHREAD_CONFIG_SRP_SERVER_PORT_SWITCH_ENABLE 1 #endif /** diff --git a/src/core/net/srp_server.cpp b/src/core/net/srp_server.cpp index 2cf8a1d45..01b96087e 100644 --- a/src/core/net/srp_server.cpp +++ b/src/core/net/srp_server.cpp @@ -85,6 +85,7 @@ Server::Server(Instance &aInstance) , mOutstandingUpdatesTimer(aInstance, HandleOutstandingUpdatesTimer) , mServiceUpdateId(Random::NonCrypto::GetUint32()) , mEnabled(false) + , mHasRegisteredAnyService(false) { IgnoreError(SetDomain(kDefaultDomain)); } @@ -420,6 +421,16 @@ void Server::CommitSrpUpdate(Error aError, { otLogInfoSrp("[server] add new host %s", aHost.GetFullName()); AddHost(&aHost); +#if OPENTHREAD_CONFIG_SRP_SERVER_PORT_SWITCH_ENABLE + if (!mHasRegisteredAnyService) + { + Settings::SrpServerInfo info; + + mHasRegisteredAnyService = true; + info.SetPort(mSocket.mSockName.mPort); + IgnoreError(Get().Save(info)); + } +#endif } // Re-schedule the lease timer. @@ -438,13 +449,28 @@ exit: void Server::Start(void) { - Error error = kErrorNone; + Error error = kErrorNone; + uint16_t port = kUdpPortMin; VerifyOrExit(!IsRunning()); - SuccessOrExit(error = mSocket.Open(HandleUdpReceive, this)); - SuccessOrExit(error = mSocket.Bind(kUdpPort, OT_NETIF_THREAD)); +#if OPENTHREAD_CONFIG_SRP_SERVER_PORT_SWITCH_ENABLE + { + Settings::SrpServerInfo info; + if (Get().Read(info) == kErrorNone) + { + port = info.GetPort() + 1; + if (port < kUdpPortMin || port > kUdpPortMax) + { + port = kUdpPortMin; + } + } + } +#endif + + SuccessOrExit(error = mSocket.Open(HandleUdpReceive, this)); + SuccessOrExit(error = mSocket.Bind(port, OT_NETIF_THREAD)); SuccessOrExit(error = PublishServerData()); otLogInfoSrp("[server] start listening on port %hu", mSocket.GetSockName().mPort); diff --git a/src/core/net/srp_server.hpp b/src/core/net/srp_server.hpp index 3918d52c0..e062d3a9a 100644 --- a/src/core/net/srp_server.hpp +++ b/src/core/net/srp_server.hpp @@ -78,8 +78,10 @@ class Server : public InstanceLocator, private NonCopyable public: enum : uint16_t { - kUdpPort = OPENTHREAD_CONFIG_SRP_SERVER_UDP_PORT, ///< The SRP Server UDP listening port. + kUdpPortMin = OPENTHREAD_CONFIG_SRP_SERVER_UDP_PORT_MIN, ///< The reserved min SRP Server UDP listening port. + kUdpPortMax = OPENTHREAD_CONFIG_SRP_SERVER_UDP_PORT_MAX, ///< The reserved max SRP Server UDP listening port. }; + static_assert(kUdpPortMin <= kUdpPortMax, "invalid port range"); /** * The ID of SRP service update transaction. @@ -671,7 +673,8 @@ private: LinkedList mOutstandingUpdates; ServiceUpdateId mServiceUpdateId; - bool mEnabled; + bool mEnabled : 1; + bool mHasRegisteredAnyService : 1; }; } // namespace Srp diff --git a/src/core/net/udp6.cpp b/src/core/net/udp6.cpp index 74bebd4b2..a99ace197 100644 --- a/src/core/net/udp6.cpp +++ b/src/core/net/udp6.cpp @@ -371,6 +371,11 @@ exit: return error; } +bool Udp::IsPortReserved(uint16_t aPort) +{ + return aPort == Tmf::kUdpPort || (kSrpServerPortMin <= aPort && aPort <= kSrpServerPortMax); +} + void Udp::AddSocket(SocketHandle &aSocket) { SuccessOrExit(mSockets.Add(aSocket)); @@ -417,7 +422,7 @@ uint16_t Udp::GetEphemeralPort(void) { mEphemeralPort = kDynamicPortMin; } - } while (mEphemeralPort == Tmf::kUdpPort); + } while (IsPortReserved(mEphemeralPort)); return mEphemeralPort; } diff --git a/src/core/net/udp6.hpp b/src/core/net/udp6.hpp index 1992cb415..178537b60 100644 --- a/src/core/net/udp6.hpp +++ b/src/core/net/udp6.hpp @@ -589,8 +589,14 @@ private: { kDynamicPortMin = 49152, ///< Service Name and Transport Protocol Port Number Registry kDynamicPortMax = 65535, ///< Service Name and Transport Protocol Port Number Registry + kSrpServerPortMin = + OPENTHREAD_CONFIG_SRP_SERVER_UDP_PORT_MIN, // The min port in the port range reserved for SRP server. + kSrpServerPortMax = + OPENTHREAD_CONFIG_SRP_SERVER_UDP_PORT_MAX, // The max port in the port range reserved for SRP server. }; + static bool IsPortReserved(uint16_t aPort); + void AddSocket(SocketHandle &aSocket); void RemoveSocket(SocketHandle &aSocket); #if OPENTHREAD_CONFIG_PLATFORM_UDP_ENABLE diff --git a/tests/scripts/thread-cert/test_srp_server_reboot_port.py b/tests/scripts/thread-cert/test_srp_server_reboot_port.py new file mode 100755 index 000000000..dd0d7f883 --- /dev/null +++ b/tests/scripts/thread-cert/test_srp_server_reboot_port.py @@ -0,0 +1,193 @@ +#!/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 ipaddress +import logging +import unittest + +import command +import thread_cert + +# Test description: +# This test verifies when an SRP server reboots, it will listen to a UDP port +# that wasn't used in the last time. +# +# Topology: +# +# LEADER (SRP client) -- ROUTER (SRP server) +# + +CLIENT = 1 +SERVER = 2 + +REBOOT_TIMES = 25 + + +class SrpAutoStartMode(thread_cert.TestCase): + USE_MESSAGE_FACTORY = False + SUPPORT_NCP = False + + TOPOLOGY = { + CLIENT: { + 'name': 'SRP_CLIENT', + 'mode': 'rdn', + }, + SERVER: { + 'name': 'SRP_SERVER', + 'mode': 'rdn', + }, + } + + def test(self): + client = self.nodes[CLIENT] + server = self.nodes[SERVER] + + # + # 0. Start the server & client devices. + # + + client.srp_server_set_enabled(False) + client.start() + self.simulator.go(5) + self.assertEqual(client.get_state(), 'leader') + + server.srp_server_set_enabled(True) + server.start() + self.simulator.go(5) + self.assertEqual(server.get_state(), 'router') + + # + # 1. Enable auto start mode on client and check that server is used. + # + + self.assertEqual(client.srp_client_get_state(), 'Disabled') + client.srp_client_enable_auto_start_mode() + self.assertEqual(client.srp_client_get_auto_start_mode(), 'Enabled') + self.simulator.go(2) + self.assertEqual(client.srp_client_get_state(), 'Enabled') + self.assertTrue(server.has_ipaddr(client.srp_client_get_server_address())) + + # + # 2. Reboot the server without any service registered. The server should + # listen to the same port after the reboot. + # + old_port = server.get_srp_server_port() + server.srp_server_set_enabled(False) + server.reset() + server.start() + self.simulator.go(5) + server.srp_server_set_enabled(True) + self.simulator.go(5) + self.assertEqual(old_port, server.get_srp_server_port()) + + # + # 3. Register a service + # + client.srp_client_set_host_name('my-host') + client.srp_client_set_host_address('2001::1') + client.srp_client_add_service('my-service', '_ipps._tcp', 12345, 0, 0, ['abc', 'def=', 'xyz=XYZ']) + self.simulator.go(5) + self.check_host_and_service(server, client, '2001::1') + + ports = [server.get_srp_server_port()] + + # Reboot the SRP server several times + for i in range(REBOOT_TIMES): + # + # 4. Disable server and check client is stopped/disabled. + # + old_port = server.get_srp_server_port() + server.srp_server_set_enabled(False) + server.reset() + server.start() + self.simulator.go(5) + + # + # 5. Enable server and check client starts again. Verify that the + # server is using a different port, and the service have been + # re-registered. + # + server.srp_server_set_enabled(True) + self.simulator.go(5) + self.assertEqual(client.srp_client_get_state(), 'Enabled') + self.assertEqual(client.srp_client_get_server_address(), server.get_mleid()) + self.assertNotEqual(old_port, server.get_srp_server_port()) + self.check_host_and_service(server, client, '2001::1') + ports.append(server.get_srp_server_port()) + logging.info(f'ports = {ports}') + + def check_host_and_service(self, server, client, host_addr): + # Check that we have properly registered host and service instance. + # Originally used in test_srp_register_single_service.py. + + client_services = client.srp_client_get_services() + print(client_services) + self.assertEqual(len(client_services), 1) + client_service = client_services[0] + + # Verify that the client possesses correct service resources. + self.assertEqual(client_service['instance'], 'my-service') + self.assertEqual(client_service['name'], '_ipps._tcp') + self.assertEqual(int(client_service['port']), 12345) + self.assertEqual(int(client_service['priority']), 0) + self.assertEqual(int(client_service['weight']), 0) + + # Verify that the client received a SUCCESS response for the server. + self.assertEqual(client_service['state'], 'Registered') + + server_services = server.srp_server_get_services() + self.assertEqual(len(server_services), 1) + server_service = server_services[0] + + # Verify that the server accepted the SRP registration and stores + # the same service resources. + self.assertEqual(server_service['deleted'], 'false') + self.assertEqual(server_service['instance'], client_service['instance']) + self.assertEqual(server_service['name'], client_service['name']) + self.assertEqual(int(server_service['port']), int(client_service['port'])) + self.assertEqual(int(server_service['priority']), int(client_service['priority'])) + self.assertEqual(int(server_service['weight']), int(client_service['weight'])) + # We output value of TXT entry as HEX string. + print(server_service['TXT']) + self.assertEqual(server_service['TXT'], ['abc', 'def=', 'xyz=58595a']) + self.assertEqual(server_service['host'], 'my-host') + + server_hosts = server.srp_server_get_hosts() + print(server_hosts) + self.assertEqual(len(server_hosts), 1) + server_host = server_hosts[0] + + self.assertEqual(server_host['deleted'], 'false') + self.assertEqual(server_host['fullname'], server_service['host_fullname']) + self.assertEqual(len(server_host['addresses']), 1) + self.assertEqual(ipaddress.ip_address(server_host['addresses'][0]), ipaddress.ip_address(host_addr)) + + +if __name__ == '__main__': + unittest.main()