Skip to content

Commit

Permalink
[dns-client] handle multiple CNAME record in response (#9339)
Browse files Browse the repository at this point in the history
This commit updates the `CheckForHostNameAlias()` method to handle the
case where the response may include multiple CNAME records. For
example, host name mapped to another name which is itself mapped
again. In order to detect CNAME record loops, we limit the number
of CNAME record name changes to `kMaxCnameAliasNameChanges = 40`.
  • Loading branch information
abtink committed Aug 4, 2023
1 parent 499f668 commit 70adcab
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 14 deletions.
36 changes: 22 additions & 14 deletions src/core/net/dns_client.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,10 @@ Error Client::Response::CheckForHostNameAlias(Section aSection, Name &aHostName)
{
// If the response includes a CNAME record mapping the query host
// name to a canonical name, we update `aHostName` to the new alias
// name. Otherwise `aHostName` remains as before.
// name. Otherwise `aHostName` remains as before. This method handles
// when there are multiple CNAME records mapping the host name multiple
// times. We limit number of changes to `kMaxCnameAliasNameChanges`
// to detect and handle if the response contains CNAME record loops.

Error error;
uint16_t offset;
Expand All @@ -177,27 +180,32 @@ Error Client::Response::CheckForHostNameAlias(Section aSection, Name &aHostName)

VerifyOrExit(mMessage != nullptr, error = kErrorNotFound);

SelectSection(aSection, offset, numRecords);
error = ResourceRecord::FindRecord(*mMessage, offset, numRecords, /* aIndex */ 0, aHostName, cnameRecord);

switch (error)
for (uint16_t counter = 0; counter < kMaxCnameAliasNameChanges; counter++)
{
case kErrorNone:
SelectSection(aSection, offset, numRecords);
error = ResourceRecord::FindRecord(*mMessage, offset, numRecords, /* aIndex */ 0, aHostName, cnameRecord);

if (error == kErrorNotFound)
{
error = kErrorNone;
ExitNow();
}

SuccessOrExit(error);

// A CNAME record was found. `offset` now points to after the
// last read byte within the `mMessage` into the `cnameRecord`
// (which is the start of the new canonical name).
aHostName.SetFromMessage(*mMessage, offset);
error = Name::ParseName(*mMessage, offset);
break;

case kErrorNotFound:
error = kErrorNone;
break;
aHostName.SetFromMessage(*mMessage, offset);
SuccessOrExit(error = Name::ParseName(*mMessage, offset));

default:
break;
// Loop back to check if there may be a CNAME record for the
// new `aHostName`.
}

error = kErrorParse;

exit:
return error;
}
Expand Down
2 changes: 2 additions & 0 deletions src/core/net/dns_client.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -770,6 +770,8 @@ class Client : public InstanceLocator, private NonCopyable
#endif // OPENTHREAD_CONFIG_DNS_CLIENT_SERVICE_DISCOVERY_ENABLE

private:
static constexpr uint16_t kMaxCnameAliasNameChanges = 40;

enum QueryType : uint8_t
{
kIp6AddressQuery, // IPv6 Address resolution.
Expand Down

0 comments on commit 70adcab

Please sign in to comment.