gnrc, nanocoap: add optional work-arounds for buggy CoAP servers#20564
Merged
benpicco merged 2 commits intoRIOT-OS:masterfrom Apr 25, 2024
Merged
gnrc, nanocoap: add optional work-arounds for buggy CoAP servers#20564benpicco merged 2 commits intoRIOT-OS:masterfrom
benpicco merged 2 commits intoRIOT-OS:masterfrom
Conversation
If a server has e.g. multiple global IPv6 addresses, it might respond with a different address that the one it received the request on. This is against the CoAP spec, but that doesn't stop existing implementations from doing it.
miri64
reviewed
Apr 15, 2024
Member
miri64
left a comment
There was a problem hiding this comment.
Having a cursory look at the code, I see that mainly gnrc_sock is touched in the GNRC code. Do other sock implementations also need fixes then?
Contributor
Author
|
At a glance it appears as if LWIP does not enforce this in the first place, I didn't test it though. |
Member
And what about OpenWSN and OpenThread? |
Contributor
|
I don´t find where or how it is called but seems like Openthread wants it to match if the socket is connected. bool Udp::SocketHandle::Matches(const MessageInfo &aMessageInfo) const
{
bool matches = false;
VerifyOrExit(GetSockName().mPort == aMessageInfo.GetSockPort(), OT_NOOP);
VerifyOrExit(aMessageInfo.GetSockAddr().IsMulticast() || GetSockName().GetAddress().IsUnspecified() ||
GetSockName().GetAddress() == aMessageInfo.GetSockAddr(),
OT_NOOP);
// Verify source if connected socket
if (GetPeerName().mPort != 0)
{
VerifyOrExit(GetPeerName().mPort == aMessageInfo.GetPeerPort(), OT_NOOP);
VerifyOrExit(GetPeerName().GetAddress().IsUnspecified() ||
GetPeerName().GetAddress() == aMessageInfo.GetPeerAddr(),
OT_NOOP);
}
matches = true;
exit:
return matches;
} |
Contributor
Within the List implementation it is used. /**
* This template method searches within the linked list to find an entry matching a given indicator.
*
* The template type `Indicator` specifies the type of @p aIndicator object which is used to match against entries
* in the list. To check that an entry matches the given indicator, the `Matches()` method is invoked on each
* `Type` entry in the list. The `Matches()` method should be provided by `Type` class accordingly:
*
* bool Type::Matches(const Indicator &aIndicator) const
*
* @param[in] aIndicator An indicator to match with entries in the list..
* @param[out] aPrevEntry A pointer to output the previous entry on success (when a match is found in the list).
* @p aPrevEntry is set to nullptr if the matching entry is the head of the list. Otherwise
* it is updated to point to the previous entry before the matching entry in the list.
*
* @returns A pointer to the matching entry if one is found, or nullptr if no matching entry was found.
*
*/
template <typename Inidcator> const Type *FindMatching(const Inidcator &aIndicator, const Type *&aPrevEntry) const
{
const Type *entry;
aPrevEntry = nullptr;
for (entry = mHead; entry != nullptr; aPrevEntry = entry, entry = entry->GetNext())
{
if (entry->Matches(aIndicator))
{
break;
}
}
return entry;
} |
Contributor
|
For openwsn it is implemented by RIOT.
if ((sock->gen_sock.remote.family != AF_UNSPEC) &&
/* check remote end-point if set */
((sock->gen_sock.remote.port != pkt->l4_sourcePortORicmpv6Type) ||
/* We only have IPv6 for now, so just comparing the whole end point
* should suffice */
((memcmp(&sock->gen_sock.remote.addr, &ipv6_addr_unspecified,
sizeof(ipv6_addr_t)) != 0) &&
(memcmp(&sock->gen_sock.remote.addr, &ep.addr,
sizeof(ipv6_addr_t)) != 0)))) {
openqueue_freePacketBuffer(pkt);
return -EPROTO;
} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Contribution description
When working with go-coap we need to add several work-arounds:
If the server has multiple IPv6 addresses, it might respond with a different one that the one where it received the request on. We fixed this in our implementations (nanocoap_sock: ensure response address is the same as request address #19361, gcoap: ensure response address is the same as request address #18026) but obviously there is third-party software that does not have such fixes and we need to work with it. So add an option to disable the check.
go-coap expects to identify block-wise requests by token. This is against the spec, but since block-wise transfers don't work if there is no token, add an option to generate a random, per-transfer token to make the go-coap server happy.
Testing procedure
Issues/PRs references
plgd-dev/go-coap#512