Skip to content

Commit

Permalink
Make minmdns advertiser send a TTL=0 record broadcast when services a…
Browse files Browse the repository at this point in the history
…re removed. (#19692)

* Prepare to have response configuration, specifically allow replies to contain an TTL override

* Support the remove all type of advertisement (in theory)

* Make minmdns advertiser send 0 ttl advertisements

* make minmdns server compile and be able to shutdown more cleanly. Will NOT send removal broadcasts though (it does not send add either)

* Make unit tests compile

* Add a unit test showing TTL overrides

* Code review comments
  • Loading branch information
andy31415 authored and pull[bot] committed Aug 8, 2023
1 parent 715fab6 commit 8182993
Show file tree
Hide file tree
Showing 18 changed files with 206 additions and 69 deletions.
22 changes: 17 additions & 5 deletions examples/minimal-mdns/server.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ class ReplyDelegate : public mdns::Minimal::ServerDelegate, public mdns::Minimal

void OnQuery(const mdns::Minimal::QueryData & data) override
{
if (mResponder->Respond(mMessageId, data, mCurrentSource) != CHIP_NO_ERROR)
if (mResponder->Respond(mMessageId, data, mCurrentSource, mdns::Minimal::ResponseConfiguration()) != CHIP_NO_ERROR)
{
printf("FAILED to respond!\n");
}
Expand All @@ -167,6 +167,16 @@ class ReplyDelegate : public mdns::Minimal::ServerDelegate, public mdns::Minimal
uint32_t mMessageId = 0;
};

mdns::Minimal::Server<10 /* endpoints */> gMdnsServer;

void StopSignalHandler(int signal)
{
gMdnsServer.Shutdown();

DeviceLayer::PlatformMgr().StopEventLoopTask();
DeviceLayer::PlatformMgr().Shutdown();
}

} // namespace

int main(int argc, char ** args)
Expand All @@ -190,7 +200,6 @@ int main(int argc, char ** args)

printf("Running on port %d using %s...\n", gOptions.listenPort, gOptions.enableIpV4 ? "IPv4 AND IPv6" : "IPv6 ONLY");

mdns::Minimal::Server<10 /* endpoints */> mdnsServer;
mdns::Minimal::QueryResponder<16 /* maxRecords */> queryResponder;

mdns::Minimal::QNamePart tcpServiceName[] = { Dnssd::kOperationalServiceName, Dnssd::kOperationalProtocol,
Expand Down Expand Up @@ -249,22 +258,25 @@ int main(int argc, char ** args)
queryResponder.AddResponder(&ipv4Responder);
}

mdns::Minimal::ResponseSender responseSender(&mdnsServer);
mdns::Minimal::ResponseSender responseSender(&gMdnsServer);
responseSender.AddQueryResponder(&queryResponder);

ReplyDelegate delegate(&responseSender);
mdnsServer.SetDelegate(&delegate);
gMdnsServer.SetDelegate(&delegate);

{
MdnsExample::AllInterfaces allInterfaces(gOptions.enableIpV4);

if (mdnsServer.Listen(DeviceLayer::UDPEndPointManager(), &allInterfaces, gOptions.listenPort) != CHIP_NO_ERROR)
if (gMdnsServer.Listen(DeviceLayer::UDPEndPointManager(), &allInterfaces, gOptions.listenPort) != CHIP_NO_ERROR)
{
printf("Server failed to listen on all interfaces\n");
return 1;
}
}

signal(SIGTERM, StopSignalHandler);
signal(SIGINT, StopSignalHandler);

DeviceLayer::PlatformMgr().RunEventLoop();

printf("Done...\n");
Expand Down
70 changes: 55 additions & 15 deletions src/lib/dnssd/Advertiser_ImplMinimalMdns.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,12 @@ class OperationalQueryAllocator : public chip::IntrusiveListNodeBase<>
Allocator * mAllocator = nullptr;
};

enum BroadcastAdvertiseType
{
kStarted, // Advertise at startup of all records added, as required by RFC 6762.
kRemovingAll, // sent a TTL 0 for all records, as records are removed
};

class AdvertiserMinMdns : public ServiceAdvertiser,
public MdnsPacketDelegate, // receive query packets
public ParserDelegate // parses queries
Expand Down Expand Up @@ -190,10 +196,12 @@ class AdvertiserMinMdns : public ServiceAdvertiser,
void OnQuery(const QueryData & data) override;

private:
/// Advertise available records configured within the server
/// Advertise available records configured within the server.
///
/// Usable as boot-time advertisement of available SRV records.
void AdvertiseRecords();
/// Establishes a type of 'Advertise all currently configured items'
/// for a specific purpose (e.g. boot time advertises everything, shut-down
/// removes all records by advertising a 0 TTL)
void AdvertiseRecords(BroadcastAdvertiseType type);

/// Determine if advertisement on the specified interface/address is ok given the
/// interfaces on which the mDNS server is listening
Expand Down Expand Up @@ -311,7 +319,8 @@ void AdvertiserMinMdns::OnQuery(const QueryData & data)

LogQuery(data);

CHIP_ERROR err = mResponseSender.Respond(mMessageId, data, mCurrentSource);
const ResponseConfiguration defaultResponseConfiguration;
CHIP_ERROR err = mResponseSender.Respond(mMessageId, data, mCurrentSource, defaultResponseConfiguration);
if (err != CHIP_NO_ERROR)
{
ChipLogError(Discovery, "Failed to reply to query: %s", ErrorStr(err));
Expand Down Expand Up @@ -339,7 +348,7 @@ CHIP_ERROR AdvertiserMinMdns::Init(chip::Inet::EndPointManager<chip::Inet::UDPEn

ChipLogProgress(Discovery, "CHIP minimal mDNS started advertising.");

AdvertiseRecords();
AdvertiseRecords(BroadcastAdvertiseType::kStarted);

mIsInitialized = true;

Expand All @@ -348,6 +357,8 @@ CHIP_ERROR AdvertiserMinMdns::Init(chip::Inet::EndPointManager<chip::Inet::UDPEn

void AdvertiserMinMdns::Shutdown()
{
AdvertiseRecords(BroadcastAdvertiseType::kRemovingAll);

GlobalMinimalMdnsServer::Server().Shutdown();
mIsInitialized = false;
}
Expand Down Expand Up @@ -418,11 +429,17 @@ OperationalQueryAllocator::Allocator * AdvertiserMinMdns::FindEmptyOperationalAl

CHIP_ERROR AdvertiserMinMdns::Advertise(const OperationalAdvertisingParameters & params)
{

char nameBuffer[Operational::kInstanceNameMaxLength + 1] = "";

/// need to set server name
// need to set server name
ReturnErrorOnFailure(MakeInstanceName(nameBuffer, sizeof(nameBuffer), params.GetPeerId()));

// Advertising data changed. Send a TTL=0 for everything as a refresh,
// which will clear caches (including things we are about to remove). Once this is done
// we will re-advertise available records with a longer TTL again.
AdvertiseRecords(BroadcastAdvertiseType::kRemovingAll);

QNamePart nameCheckParts[] = { nameBuffer, kOperationalServiceName, kOperationalProtocol, kLocalDomain };
FullQName nameCheck = FullQName(nameCheckParts);
auto * operationalAllocator = FindOperationalAllocator(nameCheck);
Expand Down Expand Up @@ -510,9 +527,7 @@ CHIP_ERROR AdvertiserMinMdns::Advertise(const OperationalAdvertisingParameters &

ChipLogProgress(Discovery, "CHIP minimal mDNS configured as 'Operational device'.");

// Advertise the records we just added as required by RFC 6762.
// TODO - Don't announce records that haven't been updated.
AdvertiseRecords();
AdvertiseRecords(BroadcastAdvertiseType::kStarted);

ChipLogProgress(Discovery, "mDNS service published: %s.%s", instanceName.names[1], instanceName.names[2]);

Expand Down Expand Up @@ -540,6 +555,11 @@ CHIP_ERROR AdvertiserMinMdns::UpdateCommissionableInstanceName()

CHIP_ERROR AdvertiserMinMdns::Advertise(const CommissionAdvertisingParameters & params)
{
// Advertising data changed. Send a TTL=0 for everything as a refresh,
// which will clear caches (including things we are about to remove). Once this is done
// we will re-advertise available records with a longer TTL again.
AdvertiseRecords(BroadcastAdvertiseType::kRemovingAll);

if (params.GetCommissionAdvertiseMode() == CommssionAdvertiseMode::kCommissionableNode)
{
mQueryResponderAllocatorCommissionable.Clear();
Expand Down Expand Up @@ -709,9 +729,7 @@ CHIP_ERROR AdvertiserMinMdns::Advertise(const CommissionAdvertisingParameters &
ChipLogProgress(Discovery, "CHIP minimal mDNS configured as 'Commissioner device'.");
}

// Advertise the records we just added as required by RFC 6762.
// TODO - Don't announce records that haven't been updated.
AdvertiseRecords();
AdvertiseRecords(BroadcastAdvertiseType::kStarted);

ChipLogProgress(Discovery, "mDNS service published: %s.%s", instanceName.names[1], instanceName.names[2]);

Expand Down Expand Up @@ -841,7 +859,7 @@ bool AdvertiserMinMdns::ShouldAdvertiseOn(const chip::Inet::InterfaceId id, cons
return result;
}

void AdvertiserMinMdns::AdvertiseRecords()
void AdvertiserMinMdns::AdvertiseRecords(BroadcastAdvertiseType type)
{
chip::Inet::InterfaceAddressIterator interfaceAddress;

Expand All @@ -850,6 +868,13 @@ void AdvertiserMinMdns::AdvertiseRecords()
return;
}

ResponseConfiguration responseConfiguration;
if (type == BroadcastAdvertiseType::kRemovingAll)
{
// make a "remove all records now" broadcast
responseConfiguration.SetTtlSecondsOverride(0);
}

for (; interfaceAddress.HasCurrent(); interfaceAddress.Next())
{
if (!Internal::IsCurrentInterfaceUsable(interfaceAddress))
Expand Down Expand Up @@ -883,8 +908,23 @@ void AdvertiserMinMdns::AdvertiseRecords()
packetInfo.DestPort = kMdnsPort;
packetInfo.Interface = interfaceAddress.GetInterfaceId();

// Advertise all records
//
// TODO: Consider advertising delta changes.
//
// Current advertisement does not have a concept of "delta" to only
// advertise changes. Current implementation is to always
// 1. advertise TTL=0 (clear all caches)
// 2. advertise available records (with longer TTL)
//
// It would be nice if we could selectively advertise what changes, like
// send TTL=0 for anything removed/about to be removed (and only those),
// then only advertise new items added.
//
// This optimization likely will take more logic and state storage, so
// for now it is not done.
QueryData queryData(QType::PTR, QClass::IN, false /* unicast */);
queryData.SetIsBootAdvertising(true);
queryData.SetIsInternalBroadcast(true);

for (auto & it : mOperationalResponders)
{
Expand All @@ -893,7 +933,7 @@ void AdvertiserMinMdns::AdvertiseRecords()
mQueryResponderAllocatorCommissionable.GetQueryResponder()->ClearBroadcastThrottle();
mQueryResponderAllocatorCommissioner.GetQueryResponder()->ClearBroadcastThrottle();

CHIP_ERROR err = mResponseSender.Respond(0, queryData, &packetInfo);
CHIP_ERROR err = mResponseSender.Respond(0, queryData, &packetInfo, responseConfiguration);
if (err != CHIP_NO_ERROR)
{
ChipLogError(Discovery, "Failed to advertise records: %s", ErrorStr(err));
Expand Down
16 changes: 9 additions & 7 deletions src/lib/dnssd/minimal_mdns/Parser.h
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,12 @@ class QueryData
QClass GetClass() const { return mClass; }
bool RequestedUnicastAnswer() const { return mAnswerViaUnicast; }

/// Boot advertisement is an internal query meant to advertise all available
/// services at device startup time.
bool IsBootAdvertising() const { return mIsBootAdvertising; }
void SetIsBootAdvertising(bool isBootAdvertising) { mIsBootAdvertising = isBootAdvertising; }
/// Internal broadcasts will advertise all available data and will not apply
/// any broadcast filtering. Intent is for paths such as:
/// - boot time advertisement: advertise all services available
/// - stop-time advertisement: advertise a TTL of 0 as services are removed
bool IsInternalBroadcast() const { return mIsInternalBroadcast; }
void SetIsInternalBroadcast(bool isInternalBroadcast) { mIsInternalBroadcast = isInternalBroadcast; }

SerializedQNameIterator GetName() const { return mNameIterator; }

Expand All @@ -65,9 +67,9 @@ class QueryData
bool mAnswerViaUnicast = false;
SerializedQNameIterator mNameIterator;

/// Flag as a boot-time internal query. This allows query replies
/// to be built accordingly.
bool mIsBootAdvertising = false;
/// Flag as an internal broadcast, controls reply construction (e.g. no
/// filtering applied)
bool mIsInternalBroadcast = false;
};

class ResourceData
Expand Down
2 changes: 1 addition & 1 deletion src/lib/dnssd/minimal_mdns/QueryReplyFilter.h
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ class QueryReplyFilter : public ReplyFilter

bool AcceptablePath(FullQName qname)
{
if (mIgnoreNameMatch || mQueryData.IsBootAdvertising())
if (mIgnoreNameMatch || mQueryData.IsInternalBroadcast())
{
return true;
}
Expand Down
7 changes: 4 additions & 3 deletions src/lib/dnssd/minimal_mdns/ResponseSender.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,8 @@ bool ResponseSender::HasQueryResponders() const
return false;
}

CHIP_ERROR ResponseSender::Respond(uint32_t messageId, const QueryData & query, const chip::Inet::IPPacketInfo * querySource)
CHIP_ERROR ResponseSender::Respond(uint32_t messageId, const QueryData & query, const chip::Inet::IPPacketInfo * querySource,
const ResponseConfiguration & configuration)
{
mSendState.Reset(messageId, query, querySource);

Expand Down Expand Up @@ -142,7 +143,7 @@ CHIP_ERROR ResponseSender::Respond(uint32_t messageId, const QueryData & query,
}
for (auto it = (*responder)->begin(&responseFilter); it != (*responder)->end(); it++)
{
it->responder->AddAllResponses(querySource, this);
it->responder->AddAllResponses(querySource, this, configuration);
ReturnErrorOnFailure(mSendState.GetError());

(*responder)->MarkAdditionalRepliesFor(it);
Expand Down Expand Up @@ -175,7 +176,7 @@ CHIP_ERROR ResponseSender::Respond(uint32_t messageId, const QueryData & query,
}
for (auto it = (*responder)->begin(&responseFilter); it != (*responder)->end(); it++)
{
it->responder->AddAllResponses(querySource, this);
it->responder->AddAllResponses(querySource, this, configuration);
ReturnErrorOnFailure(mSendState.GetError());
}
}
Expand Down
3 changes: 2 additions & 1 deletion src/lib/dnssd/minimal_mdns/ResponseSender.h
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,8 @@ class ResponseSender : public ResponderDelegate
bool HasQueryResponders() const;

/// Send back the response to a particular query
CHIP_ERROR Respond(uint32_t messageId, const QueryData & query, const chip::Inet::IPPacketInfo * querySource);
CHIP_ERROR Respond(uint32_t messageId, const QueryData & query, const chip::Inet::IPPacketInfo * querySource,
const ResponseConfiguration & configuration);

// Implementation of ResponderDelegate
void AddResponse(const ResourceRecord & record) override;
Expand Down
14 changes: 10 additions & 4 deletions src/lib/dnssd/minimal_mdns/responders/IP.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -21,19 +21,23 @@
namespace mdns {
namespace Minimal {

void IPv4Responder::AddAllResponses(const chip::Inet::IPPacketInfo * source, ResponderDelegate * delegate)
void IPv4Responder::AddAllResponses(const chip::Inet::IPPacketInfo * source, ResponderDelegate * delegate,
const ResponseConfiguration & configuration)
{
chip::Inet::IPAddress addr;
for (chip::Inet::InterfaceAddressIterator it; it.HasCurrent(); it.Next())
{
if ((it.GetInterfaceId() == source->Interface) && (it.GetAddress(addr) == CHIP_NO_ERROR) && addr.IsIPv4())
{
delegate->AddResponse(IPResourceRecord(GetQName(), addr));
IPResourceRecord record(GetQName(), addr);
configuration.Adjust(record);
delegate->AddResponse(record);
}
}
}

void IPv6Responder::AddAllResponses(const chip::Inet::IPPacketInfo * source, ResponderDelegate * delegate)
void IPv6Responder::AddAllResponses(const chip::Inet::IPPacketInfo * source, ResponderDelegate * delegate,
const ResponseConfiguration & configuration)
{
for (chip::Inet::InterfaceAddressIterator it; it.HasCurrent(); it.Next())
{
Expand All @@ -45,7 +49,9 @@ void IPv6Responder::AddAllResponses(const chip::Inet::IPPacketInfo * source, Res
chip::Inet::IPAddress addr;
if ((it.GetInterfaceId() == source->Interface) && (it.GetAddress(addr) == CHIP_NO_ERROR) && addr.IsIPv6())
{
delegate->AddResponse(IPResourceRecord(GetQName(), addr));
IPResourceRecord record(GetQName(), addr);
configuration.Adjust(record);
delegate->AddResponse(record);
}
}
}
Expand Down
6 changes: 4 additions & 2 deletions src/lib/dnssd/minimal_mdns/responders/IP.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,15 +27,17 @@ class IPv4Responder : public RecordResponder
public:
IPv4Responder(const FullQName & qname) : RecordResponder(QType::A, qname) {}

void AddAllResponses(const chip::Inet::IPPacketInfo * source, ResponderDelegate * delegate) override;
void AddAllResponses(const chip::Inet::IPPacketInfo * source, ResponderDelegate * delegate,
const ResponseConfiguration & configuration) override;
};

class IPv6Responder : public RecordResponder
{
public:
IPv6Responder(const FullQName & qname) : RecordResponder(QType::AAAA, qname) {}

void AddAllResponses(const chip::Inet::IPPacketInfo * source, ResponderDelegate * delegate) override;
void AddAllResponses(const chip::Inet::IPPacketInfo * source, ResponderDelegate * delegate,
const ResponseConfiguration & configuration) override;
};

} // namespace Minimal
Expand Down
7 changes: 5 additions & 2 deletions src/lib/dnssd/minimal_mdns/responders/Ptr.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,12 @@ class PtrResponder : public RecordResponder
public:
PtrResponder(const FullQName & qname, const FullQName & target) : RecordResponder(QType::PTR, qname), mTarget(target) {}

void AddAllResponses(const chip::Inet::IPPacketInfo * source, ResponderDelegate * delegate) override
void AddAllResponses(const chip::Inet::IPPacketInfo * source, ResponderDelegate * delegate,
const ResponseConfiguration & configuration) override
{
delegate->AddResponse(PtrResourceRecord(GetQName(), mTarget));
PtrResourceRecord record(GetQName(), mTarget);
configuration.Adjust(record);
delegate->AddResponse(record);
}

private:
Expand Down

0 comments on commit 8182993

Please sign in to comment.