[srp-server] ensure Host is freed when committing with zero key-lease (#9309)

This commit updates `CommitSrpUpdate()` to ensure that `aHost` is
freed when granted key lease is zero and host is fully removed.

This commit also updates unit test `test_srp_server` (which checks the
heap allocations by server) to cover the situations where client sends
an update to remove host and all its services with or without
clearing key-lease.
This commit is contained in:
Abtin Keshavarzian
2023-07-22 08:18:27 -07:00
committed by GitHub
parent 955b07416b
commit 8ea2031c74
2 changed files with 122 additions and 2 deletions
+4 -2
View File
@@ -438,6 +438,7 @@ void Server::CommitSrpUpdate(Error aError,
uint32_t hostKeyLease = 0;
uint32_t grantedLease = 0;
uint32_t grantedKeyLease = 0;
bool useShortLease = aHost.ShouldUseShortLeaseOption();
if (aError != kErrorNone)
{
@@ -448,7 +449,7 @@ void Server::CommitSrpUpdate(Error aError,
hostLease = aHost.GetLease();
hostKeyLease = aHost.GetKeyLease();
grantedLease = aLeaseConfig.GrantLease(hostLease);
grantedKeyLease = aHost.ShouldUseShortLeaseOption() ? grantedLease : aLeaseConfig.GrantKeyLease(hostKeyLease);
grantedKeyLease = useShortLease ? grantedLease : aLeaseConfig.GrantKeyLease(hostKeyLease);
grantedTtl = aTtlConfig.GrantTtl(grantedLease, aHost.GetTtl());
existingHost = mHosts.RemoveMatching(aHost.GetFullName());
@@ -465,6 +466,7 @@ void Server::CommitSrpUpdate(Error aError,
{
VerifyOrExit(existingHost != nullptr);
LogInfo("Fully remove host %s", aHost.GetFullName());
aHost.Free();
ExitNow();
}
@@ -537,7 +539,7 @@ exit:
{
if (aError == kErrorNone && !(grantedLease == hostLease && grantedKeyLease == hostKeyLease))
{
SendResponse(aDnsHeader, grantedLease, grantedKeyLease, aHost.ShouldUseShortLeaseOption(), *aMessageInfo);
SendResponse(aDnsHeader, grantedLease, grantedKeyLease, useShortLease, *aMessageInfo);
}
else
{
+118
View File
@@ -714,6 +714,122 @@ void TestSrpServerIgnore(void)
Log("End of TestSrpServerIgnore");
}
void TestSrpServerClientRemove(bool aShouldRemoveKeyLease)
{
Srp::Server *srpServer;
Srp::Client *srpClient;
Srp::Client::Service service1;
Srp::Client::Service service2;
uint16_t heapAllocations;
Log("--------------------------------------------------------------------------------------------");
Log("TestSrpServerClientRemove(aShouldRemoveKeyLease:%u)", aShouldRemoveKeyLease);
InitTest();
srpServer = &sInstance->Get<Srp::Server>();
srpClient = &sInstance->Get<Srp::Client>();
heapAllocations = sHeapAllocatedPtrs.GetLength();
PrepareService1(service1);
PrepareService2(service2);
//- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
// Start SRP server.
SuccessOrQuit(srpServer->SetAddressMode(Srp::Server::kAddressModeUnicast));
VerifyOrQuit(srpServer->GetAddressMode() == Srp::Server::kAddressModeUnicast);
VerifyOrQuit(srpServer->GetState() == Srp::Server::kStateDisabled);
srpServer->SetServiceHandler(HandleSrpServerUpdate, sInstance);
srpServer->SetEnabled(true);
VerifyOrQuit(srpServer->GetState() != Srp::Server::kStateDisabled);
AdvanceTime(10000);
VerifyOrQuit(srpServer->GetState() == Srp::Server::kStateRunning);
//- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
// Start SRP client.
srpClient->SetCallback(HandleSrpClientCallback, sInstance);
srpClient->EnableAutoStartMode(nullptr, nullptr);
VerifyOrQuit(srpClient->IsAutoStartModeEnabled());
AdvanceTime(2000);
VerifyOrQuit(srpClient->IsRunning());
SuccessOrQuit(srpClient->SetHostName(kHostName));
SuccessOrQuit(srpClient->EnableAutoHostAddress());
//- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
// Register two services, validate that update handler is called.
SuccessOrQuit(srpClient->AddService(service1));
SuccessOrQuit(srpClient->AddService(service2));
sUpdateHandlerMode = kAccept;
sProcessedUpdateCallback = false;
sProcessedClientCallback = false;
AdvanceTime(2 * 1000);
VerifyOrQuit(sProcessedUpdateCallback);
VerifyOrQuit(sProcessedClientCallback);
VerifyOrQuit(sLastClientCallbackError == kErrorNone);
VerifyOrQuit(service1.GetState() == Srp::Client::kRegistered);
VerifyOrQuit(service2.GetState() == Srp::Client::kRegistered);
ValidateHost(*srpServer, kHostName);
//- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
// Remove two services and clear key-lease, validate that update handler is called.
SuccessOrQuit(srpClient->RemoveHostAndServices(aShouldRemoveKeyLease));
AdvanceTime(2 * 1000);
VerifyOrQuit(sProcessedUpdateCallback);
VerifyOrQuit(sProcessedClientCallback);
VerifyOrQuit(sLastClientCallbackError == kErrorNone);
VerifyOrQuit(service1.GetState() == Srp::Client::kRemoved);
VerifyOrQuit(service2.GetState() == Srp::Client::kRemoved);
if (aShouldRemoveKeyLease)
{
VerifyOrQuit(srpServer->GetNextHost(nullptr) == nullptr);
}
else
{
ValidateHost(*srpServer, kHostName);
}
//- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
// Disable SRP server, verify that all heap allocations by SRP server
// are freed.
Log("Disabling SRP server");
srpServer->SetEnabled(false);
AdvanceTime(100);
VerifyOrQuit(heapAllocations == sHeapAllocatedPtrs.GetLength());
//- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
// Finalize OT instance and validate all heap allocations are freed.
Log("Finalizing OT instance");
FinalizeTest();
VerifyOrQuit(sHeapAllocatedPtrs.IsEmpty());
Log("End of TestSrpServerClientRemove");
}
#if OPENTHREAD_CONFIG_REFERENCE_DEVICE_ENABLE
void TestUpdateLeaseShortVariant(void)
{
@@ -918,6 +1034,8 @@ int main(void)
TestSrpServerBase();
TestSrpServerReject();
TestSrpServerIgnore();
TestSrpServerClientRemove(/* aShouldRemoveKeyLease */ true);
TestSrpServerClientRemove(/* aShouldRemoveKeyLease */ false);
#if OPENTHREAD_CONFIG_REFERENCE_DEVICE_ENABLE
TestUpdateLeaseShortVariant();
#endif