Due to some changes to the minimal forwarding draft and in preparation
for Selective Fragment Recovery some changes to the VRB API were
needed. Now the index of a VRB entry is only (L2 src, tag) not as
before (L2 src, L2 dst, length, tag).
I know that the current `rbuf_base` causes waste, as all the fields not
used by the new index are effectively not used by the VRB. I'd like to
fix that however in a later change, since that also requires some
modifications of the classic reassembly buffer, and thus would
complicate the review and testing of the change.
Sources for the index change:
- https://tools.ietf.org/html/draft-ietf-6lo-minimal-fragment-04#section-1
- https://mailarchive.ietf.org/arch/browse/6lo/?gbt=1&index=DLCTxC2X4bRNtYPHhtEkavMWlz4
This fixes a denial of service where an attacker would be able to cause
a NULL pointer dereference by sending a spoofed packet. This attack only
requires knowledge about pending message ids.
TCP options have up to three fields (kind, length, value). The
current code only checks for the presence of the first field. Before
accessing the second field (length) the code must ensure that a length
field is even present.
A received packet is outputted in DEBUG _after_ it was already parsed,
but with a reference to the already parsed header. The result is that
there can be some garbage in the output and the packet is not dumped in
total. As without parsing we do not have access to the header yet, we
use the `gnrc_netif_addr_to_str()` helper function instead of parsing
the destination address by hand.
before this commit the src address was checked for multicast, but the dst address should be checked. Therefore udp multicast packets would be flooded back to the src as ICMPv6 error, as not all nodes had a UDP receiver registered.
When a keepalive timeout occurs keepalive_retry_cnt remains zero,
so when the connection is re-established _on_keepalive_evt will
immediately disconnect instead of actually sending a keepalive ping.
The sequence looks like:
1. _on_connack: start con->keepalive_timer
2. Server does not respond to keepalive pings
3. _on_keepalive_evt: con->keepalive_retry_cnt reaches zero
4. Connection torn down and ASYMCUTE_DISCONNECTED sent to application
5. Application starts reconnection
6. _on_connack: start con->keepalive_timer again
7. First _on_keepalive_evt: con->keepalive_retry_cnt is still zero
8. Repeat from 4.
So this simply resets keepalive_retry_cnt in _on_connack when
the keepalive timer is restarted. It's a new connection, so
resetting the keepalive retry counter make senses regardless.
Signed-off-by: Derek Hageman <hageman@inthat.cloud>
From the gnrc_pktbuf_mark documentation:
It's not guaranteed that `result->data` points to the
same address as the original `pkt->data.
Thus it should be necessary to update the `hdr` pointer.
If an address was pre-configured by the upper layer its validity is
currently ignored. It is neither checked if the address is on the
interface at all nor is it checked if it is valid.
This change provides a fix for that by checking both facts.
The length field in an MQTT packet carries the _total_ length of the
packet. If it is below 256 (i.e. fits in one byte) only one byte is
used for the length field. If it is larger than that 3 bytes are used,
with the first byte having the value `0x01` and the remaining bytes
representing the length in as a 2 byte unsigned integer in network byte
order. Resulting from that it can be assessed that the check in
`emcutes`'s `set_len()` function is wrong as it needs to be checked if
`len` is lesser or equal to `0xff - 1`. `len <= (0xff - 1)` can be
simplified to `len < 0xff`. For some larger packages this safes 2 bytes
of wasted packet space.
`len` is used with the `memcpy()` to copy the payload to `tbuf`. With a
payload provided that is just long enough to fill `tbuf`, `len += 6`
leads to the `memcpy()` overriding data after `tbuf` (e.g. the
`mutex` that is unlocked right after) and thus resulting in potential
segmentation faults.
Additionally `+ 6` can only be applied if the total packet length is
below 256 (see spec), so `len + pos` is what needs to be provided to the
corresponding send functions instead (`pos` adapts to the header length
of the PUBLISH message).
