[srp-client] add aSendUnregToServer param to RemoveHostAndServices() (#6927)

This commit adds a new parameter `aSendUnregToServer` to SRP client's
`RemoveHostAndServices()` API. This parameter determines the behavior
when the host info is not yet registered with the server. If it is
set to `false` (which is the default/expected value) then the SRP
client will immediately remove the host info and services without
sending an update message to server (no need to update the server if
nothing is yet registered with it). If it is set to `true` then the
SRP client will send an update message to the server. Note that if
the host info is registered then the value of `aSendUnregToServer`
does not matter and the SRP client will always send an update message
to the server requesting removal of all info.

One situation where this parameter can be useful is on a device
reset/reboot where the caller may want to remove any previously
registered services with the server. In this case, caller can
`SetHostName()` and then request `RemoveHostAndServices()` with
`aSendUnregToServer` as `true`.

This commit also adds `test_srp_client_remove_host.py` which
verifies the behavior the newly added mechanism between client and
server.
This commit is contained in:
Abtin Keshavarzian
2021-08-23 23:07:57 -07:00
committed by Jonathan Hui
parent da629d63ff
commit 5851524d7a
12 changed files with 244 additions and 29 deletions
+1 -1
View File
@@ -53,7 +53,7 @@ extern "C" {
* @note This number versions both OpenThread platform and user APIs.
*
*/
#define OPENTHREAD_API_VERSION (155)
#define OPENTHREAD_API_VERSION (156)
/**
* @addtogroup api-instance
+17 -5
View File
@@ -180,8 +180,8 @@ typedef void (*otSrpClientCallback)(otError aError,
* This callback is invoked when auto-start mode is enabled and the SRP client is either automatically started or
* stopped.
*
* @param[in] aSeverSockAddress A non-NULL pointer indicates SRP sever was started and pointer will give the
* selected server socket address. A NULL pointer indicates SRP sever was stopped.
* @param[in] aServerSockAddress A non-NULL pointer indicates SRP server was started and pointer will give the
* selected server socket address. A NULL pointer indicates SRP server was stopped.
* @param[in] aContext A pointer to an arbitrary context (provided when callback was registered).
*
*/
@@ -515,15 +515,27 @@ const otSrpClientService *otSrpClientGetServices(otInstance *aInstance);
* that the server holds the host name in reserve for when the client is once again able to provide and register its
* service(s).
*
* @param[in] aInstance A pointer to the OpenThread instance.
* @param[in] aRemoveKeyLease A boolean indicating whether or not the host key lease should also be removed.
* The @p aSendUnregToServer determines the behavior when the host info is not yet registered with the server. If
* @p aSendUnregToServer is set to `false` (which is the default/expected value) then the SRP client will immediately
* remove the host info and services without sending an update message to server (no need to update the server if
* nothing is yet registered with it). If @p aSendUnregToServer is set to `true` then the SRP client will send an
* update message to the server. Note that if the host info is registered then the value of @p aSendUnregToServer does
* not matter and the SRP client will always send an update message to server requesting removal of all info.
*
* One situation where @p aSendUnregToServer can be useful is on a device reset/reboot, caller may want to remove any
* previously registered services with the server. In this case, caller can `otSrpClientSetHostName()` and then request
* `otSrpClientRemoveHostAndServices()` with `aSendUnregToServer` as `true`.
*
* @param[in] aInstance A pointer to the OpenThread instance.
* @param[in] aRemoveKeyLease A boolean indicating whether or not the host key lease should also be removed.
* @param[in] aSendUnregToServer A boolean indicating whether to send update to server when host info is not registered.
*
* @retval OT_ERROR_NONE The removal of host info and services started successfully. The `otSrpClientCallback`
* will be called to report the status.
* @retval OT_ERROR_ALREADY The host info is already removed.
*
*/
otError otSrpClientRemoveHostAndServices(otInstance *aInstance, bool aRemoveKeyLease);
otError otSrpClientRemoveHostAndServices(otInstance *aInstance, bool aRemoveKeyLease, bool aSendUnregToServer);
/**
* This function clears all host info and all the services.
+5 -2
View File
@@ -201,9 +201,12 @@ The possible states are (same value for service state):
### host remove
Usage: `srp client host remove [removekeylease]`
Usage: `srp client host remove [removekeylease] [sendunregtoserver]`
Remove host info and all services from server. `removekeylease` is boolean value indicating whether or not the host key lease should also be removed
Remove host info and all services from server.
- `removekeylease` is an optional boolean value indicating whether or not the host key lease should also be removed (default is false).
- `sendunregtoserver` is a another optional boolean value indicating whether or not to send an update message to the server when host info is not yet registered (default is false).
```bash
> srp client host remove 1
+9 -3
View File
@@ -238,15 +238,21 @@ otError SrpClient::ProcessHost(Arg aArgs[])
}
else if (aArgs[0] == "remove")
{
bool removeKeyLease = false;
bool removeKeyLease = false;
bool sendUnregToServer = false;
if (!aArgs[1].IsEmpty())
{
SuccessOrExit(error = aArgs[1].ParseAsBool(removeKeyLease));
VerifyOrExit(aArgs[2].IsEmpty(), error = OT_ERROR_INVALID_ARGS);
if (!aArgs[2].IsEmpty())
{
SuccessOrExit(error = aArgs[2].ParseAsBool(sendUnregToServer));
VerifyOrExit(aArgs[3].IsEmpty(), error = OT_ERROR_INVALID_ARGS);
}
}
error = otSrpClientRemoveHostAndServices(mInterpreter.mInstance, removeKeyLease);
error = otSrpClientRemoveHostAndServices(mInterpreter.mInstance, removeKeyLease, sendUnregToServer);
}
else if (aArgs[0] == "clear")
{
+2 -2
View File
@@ -179,11 +179,11 @@ const otSrpClientService *otSrpClientGetServices(otInstance *aInstance)
return instance.Get<Srp::Client>().GetServices().GetHead();
}
otError otSrpClientRemoveHostAndServices(otInstance *aInstance, bool aRemoveKeyLease)
otError otSrpClientRemoveHostAndServices(otInstance *aInstance, bool aRemoveKeyLease, bool aSendUnregToServer)
{
Instance &instance = *static_cast<Instance *>(aInstance);
return instance.Get<Srp::Client>().RemoveHostAndServices(aRemoveKeyLease);
return instance.Get<Srp::Client>().RemoveHostAndServices(aRemoveKeyLease, aSendUnregToServer);
}
void otSrpClientClearHostAndServices(otInstance *aInstance)
+9 -9
View File
@@ -501,7 +501,7 @@ exit:
return error;
}
Error Client::RemoveHostAndServices(bool aShouldRemoveKeyLease)
Error Client::RemoveHostAndServices(bool aShouldRemoveKeyLease, bool aSendUnregToServer)
{
Error error = kErrorNone;
@@ -523,7 +523,7 @@ Error Client::RemoveHostAndServices(bool aShouldRemoveKeyLease)
UpdateServiceStateToRemove(service);
}
if (mHostInfo.GetState() == kToAdd)
if ((mHostInfo.GetState() == kToAdd) && !aSendUnregToServer)
{
// Host info is not added yet (not yet registered with
// server), so we can remove it and all services immediately.
@@ -1260,7 +1260,7 @@ void Client::ProcessResponse(Message &aMessage)
offset += sizeof(header);
// Skip over all sections till Additional Data section
// SPEC ENHANCEMENT: Sever can echo the request back or not
// SPEC ENHANCEMENT: Server can echo the request back or not
// include any of RRs. Would be good to explicitly require SRP server
// to not echo back RRs.
@@ -1446,7 +1446,7 @@ void Client::UpdateState(void)
bool shouldUpdate = false;
VerifyOrExit((GetState() != kStateStopped) && (GetState() != kStatePaused));
VerifyOrExit((mHostInfo.GetName() != nullptr) && (mHostInfo.GetNumAddresses() > 0));
VerifyOrExit(mHostInfo.GetName() != nullptr);
// Go through the host info and all the services to check if there
// are any new changes (i.e., anything new to add or remove). This
@@ -1474,11 +1474,11 @@ void Client::UpdateState(void)
case kToAdd:
case kToRefresh:
// Make sure we have at least one service otherwise no need to
// send SRP update message with host info only. The exception
// is when removing host info where we allow for empty
// service list.
VerifyOrExit(!mServices.IsEmpty());
// Make sure we have at least one service and at least one
// host address, otherwise no need to send SRP update message.
// The exception is when removing host info where we allow
// for empty service list.
VerifyOrExit(!mServices.IsEmpty() && (mHostInfo.GetNumAddresses() > 0));
// Fall through
+17 -3
View File
@@ -556,7 +556,7 @@ public:
/**
* This method starts the remove process of the host info and all services.
*
* After retuning from this method, `Callback` will be called to report the status of remove request with
* After returning from this method, `Callback` will be called to report the status of remove request with
* SRP server.
*
* If the host info is to be permanently removed from server, @p aRemoveKeyLease should be set to `true` which
@@ -564,14 +564,28 @@ public:
* ensures that the server holds the host name in reserve for when the client once again able to provide and
* register its service(s).
*
* @param[in] aRemoveKeyLease A boolean indicating whether or not the host key lease should also be removed.
* The @p aSendUnregToServer determines the behavior when the host info is not yet registered with the server. If
* @p aSendUnregToServer is set to `false` (which is the default/expected value) then the SRP client will
* immediately remove the host info and services without sending an update message to server (no need to update the
* server if nothing is yet registered with it). If @p aSendUnregToServer is set to `true` then the SRP client will
* send an update message to the server. Note that if the host info is registered then the value of
* @p aSendUnregToServer does not matter and the SRP client will always send an update message to server requesting
* removal of all info.
*
* One situation where @p aSendUnregToServer can be useful is on a device reset/reboot, caller may want to remove
* any previously registered services with the server. In this case, caller can `SetHostName()` and then request
* `RemoveHostAndServices()` with `aSendUnregToServer` as `true`.
*
* @param[in] aRemoveKeyLease A boolean indicating whether or not the host key lease should also be removed.
* @param[in] aSendUnregToServer A boolean indicating whether to send update to server when host info is not
* registered.
*
* @retval kErrorNone The removal of host and services started successfully. The `Callback` will be called
* to report the status.
* @retval kErrorAlready The host is already removed.
*
*/
Error RemoveHostAndServices(bool aShouldRemoveKeyLease);
Error RemoveHostAndServices(bool aShouldRemoveKeyLease, bool aSendUnregToServer = false);
/**
* This method clears all host info and all the services.
+2 -1
View File
@@ -4137,7 +4137,7 @@ enum
SPINEL_PROP_SRP_CLIENT_SERVICES = SPINEL_PROP_OPENTHREAD__BEGIN + 23,
/// SRP Client Host And Services Remove
/** Format: `b` : Write only
/** Format: `bb` : Write only
* Required capability: `SPINEL_CAP_SRP_CLIENT`.
*
* Writing to this property with starts the remove process of the host info and all services.
@@ -4146,6 +4146,7 @@ enum
* Format is:
*
* `b` : A boolean indicating whether or not the host key lease should also be cleared.
* `b` : A boolean indicating whether or not to send update to server when host info is not registered.
*
*/
SPINEL_PROP_SRP_CLIENT_HOST_SERVICES_REMOVE = SPINEL_PROP_OPENTHREAD__BEGIN + 24,
+3 -1
View File
@@ -3932,10 +3932,12 @@ template <> otError NcpBase::HandlePropertySet<SPINEL_PROP_SRP_CLIENT_HOST_SERVI
{
otError error = OT_ERROR_NONE;
bool removeKeyLease;
bool sendUnregToServer;
SuccessOrExit(error = mDecoder.ReadBool(removeKeyLease));
SuccessOrExit(error = mDecoder.ReadBool(sendUnregToServer));
error = otSrpClientRemoveHostAndServices(mInstance, removeKeyLease);
error = otSrpClientRemoveHostAndServices(mInstance, removeKeyLease, sendUnregToServer);
exit:
return error;
+2
View File
@@ -182,6 +182,7 @@ EXTRA_DIST = \
test_service.py \
test_set_mliid.py \
test_srp_auto_start_mode.py \
test_srp_client_remove_host.py \
test_srp_client_save_server_info.py \
test_srp_lease.py \
test_srp_name_conflicts.py \
@@ -248,6 +249,7 @@ check_SCRIPTS = \
test_router_reattach.py \
test_service.py \
test_srp_auto_start_mode.py \
test_srp_client_remove_host.py \
test_srp_client_save_server_info.py \
test_srp_lease.py \
test_srp_name_conflicts.py \
+2 -2
View File
@@ -989,8 +989,8 @@ class NodeImpl:
self.send_command(f'srp client host name')
self._expect_done()
def srp_client_remove_host(self, remove_key=False):
self.send_command(f'srp client host remove {"1" if remove_key else "0"}')
def srp_client_remove_host(self, remove_key=False, send_unreg_to_server=False):
self.send_command(f'srp client host remove {int(remove_key)} {int(send_unreg_to_server)}')
self._expect_done()
def srp_client_clear_host(self):
+175
View File
@@ -0,0 +1,175 @@
#!/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 unittest
import command
import thread_cert
# Test description:
# This test verifies SRP client `RemoveHostAndServices()` behavior when host info is not yet registered.
#
# Topology:
#
# LEADER (SRP server)
# |
# |
# ROUTER (SRP client)
#
SERVER = 1
CLIENT = 2
class SrpRemoveHost(thread_cert.TestCase):
USE_MESSAGE_FACTORY = False
SUPPORT_NCP = False
TOPOLOGY = {
SERVER: {
'name': 'SRP_SERVER',
'mode': 'rdn',
},
CLIENT: {
'name': 'SRP_CLIENT',
'mode': 'rdn',
},
}
def test(self):
server = self.nodes[SERVER]
client = self.nodes[CLIENT]
#-------------------------------------------------------------------------------------
# Start the server & client devices.
server.start()
self.simulator.go(5)
self.assertEqual(server.get_state(), 'leader')
client.start()
self.simulator.go(5)
self.assertEqual(client.get_state(), 'router')
server.srp_server_set_enabled(True)
client.srp_client_enable_auto_start_mode()
self.simulator.go(5)
#-------------------------------------------------------------------------------------
# Register a single service and verify that it worked.
client.srp_client_set_host_name('host')
client.srp_client_set_host_address('2001::1')
client.srp_client_add_service('ins', '_srv._udp', 1977)
self.simulator.go(2)
client_services = client.srp_client_get_services()
self.assertEqual(len(client_services), 1)
client_service = client_services[0]
self.assertEqual(client_service['instance'], 'ins')
self.assertEqual(client_service['name'], '_srv._udp')
self.assertEqual(int(client_service['port']), 1977)
self.assertEqual(int(client_service['priority']), 0)
self.assertEqual(int(client_service['weight']), 0)
self.assertEqual(client_service['state'], 'Registered')
server_hosts = server.srp_server_get_hosts()
self.assertEqual(len(server_hosts), 1)
server_host = server_hosts[0]
self.assertEqual(server_host['fullname'], 'host.default.service.arpa.')
self.assertEqual(server_host['deleted'], 'false')
self.assertEqual(len(server_host['addresses']), 1)
self.assertIn('2001:0:0:0:0:0:0:1', server_host['addresses'])
server_services = server.srp_server_get_services()
self.assertEqual(len(server_services), 1)
server_service = server_services[0]
self.assertEqual(server_service['fullname'], 'ins._srv._udp.default.service.arpa.')
self.assertEqual(server_service['instance'], 'ins')
self.assertEqual(server_service['name'], '_srv._udp')
self.assertEqual(server_service['deleted'], 'false')
self.assertEqual(int(server_service['port']), 1977)
self.assertEqual(int(server_service['priority']), 0)
self.assertEqual(int(server_service['weight']), 0)
self.assertEqual(server_service['host'], 'host')
#-------------------------------------------------------------------------------------
# Clear the info on client, and verify that it is still present on server.
client.srp_client_clear_host()
self.simulator.go(2)
client_services = client.srp_client_get_services()
self.assertEqual(len(client_services), 0)
self.assertIsNone(client.srp_client_get_host_name())
self.assertIsNone(client.srp_client_get_host_address())
server_services = server.srp_server_get_services()
self.assertEqual(len(server_services), 1)
#-------------------------------------------------------------------------------------
# Now set the host name and request "host remove" requiring that client updates server
# even if host is not yet registered. Verify that client updates the server by checking
# that the host and service entries are marked as "deleted" on the server (i.e. deleted
# by the name (and associated key) are retained).
client.srp_client_set_host_name('host')
client.srp_client_remove_host(send_unreg_to_server=True)
self.simulator.go(2)
self.assertIsNone(client.srp_client_get_host_name())
server_hosts = server.srp_server_get_hosts()
self.assertEqual(len(server_hosts), 1)
server_host = server_hosts[0]
self.assertEqual(server_host['fullname'], 'host.default.service.arpa.')
self.assertEqual(server_host['deleted'], 'true')
server_services = server.srp_server_get_services()
self.assertEqual(len(server_services), 1)
server_service = server_services[0]
self.assertEqual(server_service['fullname'], 'ins._srv._udp.default.service.arpa.')
self.assertEqual(server_service['deleted'], 'true')
#-------------------------------------------------------------------------------------
# Again request "host remove" but this time request `remove_key`. Verify that entries
# on the server are fully removed .
client.srp_client_set_host_name('host')
client.srp_client_remove_host(remove_key=True, send_unreg_to_server=True)
self.simulator.go(2)
self.assertIsNone(client.srp_client_get_host_name())
self.assertEqual(len(server.srp_server_get_services()), 0)
self.assertEqual(len(server.srp_server_get_hosts()), 0)
if __name__ == '__main__':
unittest.main()