gnrc, nanocoap: add optional work-arounds for buggy CoAP servers

[benpicco]

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

[riot-ci]

Murdock results

✔️ PASSED

13fd806 nanocoap_sock: add option to include token for block-wise

Success	Failures	Total	Runtime
10044	0	10045	14m:32s

Artifacts

• Documentation preview

[miri64]

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?

[benpicco]

At a glance it appears as if LWIP does not enforce this in the first place, I didn't test it though.

[miri64]

At a glance it appears as if LWIP does not enforce this in the first place, I didn't test it though.

And what about OpenWSN and OpenThread?

[fabian18]

I don´t find where or how it is called but seems like Openthread wants it to match if the socket is connected.
udp6.c

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;
}

[fabian18]

I don´t find where or how it is called

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;
    }

[fabian18]

For openwsn it is implemented by RIOT.

pkg/openwsn/sock/openwsn_sock_udp.c::484

    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;
    }

[fabian18]

For a workaround I would say it is ok as is, unless @miri64 objects.