In previous patch fixing this issue, I've missed the fact that offloaded
drivers would not set the context->local address, which resulted in a
regression, where the previously introduced assert would hit in
offloaded cases. Not setting laddr is not a problem in case of
offloading, as it's only used in net_tcp_connect() which would not be
reached in this case.
Therefore I propose to remove previous patch to get rid of regression.
As an alternative fix, verify the laddr just before use, so that it is
only checked when native net stack is in use.
Signed-off-by: Robert Lubos <robert.lubos@nordicsemi.no>
Coverity reported, that laddr pointer used in net_context_connect()
could be passed as NULL to net_tcp_connect() where it could be
dereferenced. This is because the actual setting of laddr to a valid
address structure was only done after
net_sin/sin6_ptr(&context->local)->sin/sin6_addr verification.
In practice though, the aforementioned pointer verification would always
pass, as the bind_default() guarantee that the context->local address is
set to an unspecified address (if it hasn't been set earlier).
Therefore refactor the code a bit: replace the pointer verification
with NET_ASSERT - only to assure that we can catch regression in case
for any reason the behavior of bind_default() changes. This should also
ensure that Coverity no longer reports that laddr is NULL when reaching
net_tcp_connect().
Signed-off-by: Robert Lubos <robert.lubos@nordicsemi.no>
As since commit bff6a5c, params.dst.ext_addr is no longer assigned with
the ctx->coord_ext_addr pointer, but a endianness swap is included, it
should be assigned with a proper buffer before the memcpy.
Signed-off-by: Robert Lubos <robert.lubos@nordicsemi.no>
Coverity reported potential issues with implicit signed/unsigned type
conversions and potential problems with this. Fix this, by casing the
byte-shifted data variable to uint64_t type explicitly.
Signed-off-by: Robert Lubos <robert.lubos@nordicsemi.no>
Coverity reported that assigning ret = 0 is pointless, as in any
scenario (loop continues or ends) the ret variable is overwritten
anyway, w/o using the assigned value. Therefore remove the needless
assignment.
Signed-off-by: Robert Lubos <robert.lubos@nordicsemi.no>
obj_inst_id should not be used directly to index the instance array, as
the instance ID is not tightly bound to the maximum instance count and
can exceed this value, causing out-of-bound access.
Therefore, perform some extra validation when choosing the array index
for the object instance to make sure we stay in the array bounds, or
return an error if there's no more room for more object instances, in a
similar way it's done for Security object.
Signed-off-by: Robert Lubos <robert.lubos@nordicsemi.no>
In case TCP connection is being closed from the TCP stack, due to for
instance retransmission timeout, the stack should also switch the TCP
state to CLOSED. Otherwise, there was a risk of dereferencing the TCP
context twice, for example if the application was in active socket
send(), and tried to reschedule data transmission.
Additionally, make sure that the TCP_CLOSED state handling is a no-op
state - otherwise, there is a risk that if packets keep incoming before
the application dereferences the TCP context on its side, TCP stack
will incorrectly dereference the context for the second time from
within due to current TCP_CLOSED state logic.
Signed-off-by: Robert Lubos <robert.lubos@nordicsemi.no>
The port we are connecting to is stored in network byte order,
thus, we need to convert it to the CPU's byte order before logging
Signed-off-by: Mark Oude Elberink <mark@oude-elberink.de>
`local_addr` would only be initialized if `context->local->sin*_addr` was
non-null. However, since `context` is a fresh context object, `local_addr`
always remains at its initial value of `INADDR_ANY`, which is propagated to
the context by `net_context_bind()`.
By populating `local_addr` using the TCP endpoint, `getsockname()` now
returns the correct local address.
Signed-off-by: Armin Brauns <armin.brauns@embedded-solutions.at>
In order to support external IP stacks that might have Connection
ID support, the LwM2M engine should allow client to bypass default
behaviour.
New set_socketoptions() callback added into client context
that allows overriding all socket opetions. This is called
after a socket is opened, but before the connect() is
called. This cannot be combined with load_credentials() callback
on all platforms as for example nRF91 requires modem offline
when credentials are written. This would cause socket to be closed
as well.
Second change is that we allow fine tuning of what we do with
socket handle when QUEUE mode is enabled and engine enters idle
state.
First option would be to close the socket. That would cause
TLS Alert(Close Notify) to be send. This is a band choice if
LTE modem was already in PSM or eDRX power saving mode.
Second option would be to delay socket closing until we
are going to send LwM2M update. There TLS Alert is also send,
but most probably lost due to NAT mapping timed out. This
is a best choice for LTE modem with DTL session cache enabled.
Two new options are to keep socket open, and either stop listening
or just keep listening. Both of these options work fine when
we have DTLS Connection ID support.
Signed-off-by: Seppo Takalo <seppo.takalo@nordicsemi.no>
In case lwm2m_socket_start() internal error, it should only do cleanup
on the socket, i. e. call lwm2m_socket_close(), not lwm2m_engine_stop().
The latter resets the entire lwm2m_context, which results in removal of
active observations.
This should not be done, as it collides with the RD client logic, where
connection resumption may skip the full registration phase, in result
not notifying the server that it should restart the observations.
At the same time, the RD client should clean the lwm2m_context when it's
done trying to update the registration and proceeds with regular
registration/bootstrap in the network error handler. In that case, only
the socket was closed, so the lwm2m_context needs to be reset
separately.
Signed-off-by: Robert Lubos <robert.lubos@nordicsemi.no>
The number of buffer provided was verified in the final else block of a
long validation sequence. It would never be executed though, as one of
the conditions before would always evaluate to true.
As the number of buffers provided verification appears to be significant
in this case, as the buffers are referenced during other validations,
move this check at the beginning of the sequence instead. This also
eliminates the dead-code problem.
Signed-off-by: Robert Lubos <robert.lubos@nordicsemi.no>
For IEEE 802.15.4 compliance, the NET_L2_IEEE802154_ACK_REPLY option must
automatically be active if the radio driver does not AUTOACK and inactive
otherwise. No user interaction is required.
Future changes will deprecate this option and replace it by a standard
compliant automatic mechanism.
Signed-off-by: Florian Grandel <fgrandel@code-for-humans.de>
The IEEE 802.15.4 standard does not specify a "Sub-GHz" option.
Therefore this option will be deprecated in the foreseeable future.
Rationale: Selecting PHYs and frequency bands is abstracted by the
concept of channel pages (see IEEE 802.15.4-2020, 10.1.3).
Radio drivers may expose additional configuration options that
specify the low level frequency band, operating mode and other
PHY-specific parameters. Such parameters should be exposed on a driver
instance level, e.g. via devicetree or as runtime options of the driver.
PHY-specific attributes derived from driver settings (e.g. available
channels) can then be announced to L2 w/o the user having to be aware of
any implementation details of the specific PHY.
Future changes will introduce the necessary infrastructure to define
channel pages and other PHY-specific configuration options to
replace this option. New drivers should therefore not rely on this
option.
Signed-off-by: Florian Grandel <fgrandel@code-for-humans.de>
In theory, coap_header_get_token() should only be used on already parsed
packets, and coap_packet_parse() would detect an invalid token length in
a packet. Coverity however complains about possible out-of-bound access,
as in theory the function can return token length up to 15. Therefore
add an extra validation of the token length within the function, to
avoid out-of-bound access due to programming errors and to make Coverity
happy.
Signed-off-by: Robert Lubos <robert.lubos@nordicsemi.no>
This patch fixes commit b70f92e570
("net: buf: keep memory alignment provided by k_heap_alloc and k_malloc").
One-line was overlooked in the above patch and may result in a
cloned net_buf using a data block that has already been freed.
Signed-off-by: Johann Fischer <johann.fischer@nordicsemi.no>
Although websocket_recv_msg function accepts timeout parameter, the
functionality was rather limited, allowing only to either work in
non-blocking manner, or to block indefinitely. Any timeout value
other than -1 (forever) ended up in non-blocking operation.
This PR fixes this by implementing a basic timeout mechanism, built on
top of poll(). For now on, only timeout of 0 will result in non-blocking
operation, any other timeout will make the function block for the
specified amount of time.
Signed-off-by: Robert Lubos <robert.lubos@nordicsemi.no>
In order to allow the TLS accept() call to be interrupted, it should
release the top-level TLS socket mutex before blocking. As the
underlying TCP accept() makes no use of TLS resources, and has its own
mutex protection, it should be safe to do so.
Signed-off-by: Robert Lubos <robert.lubos@nordicsemi.no>
The accept() so far would block with mutex held, making it impossible to
interrupt it from another thread when the socket was closed.
Fix this, by reusing the condvar mechanism used for receiving. It's OK
to use the same routine, as underneath accept() is monitoring the same
FIFO as recv().
Additionally, simplify k_fifo_get() handling in accept() - as the
waiting now takes place on condvar, it can be used in a non-blocking
manner. Blocking accept() call should not reach this place if there's no
new incoming connection waiting on the FIFO.
Signed-off-by: Robert Lubos <robert.lubos@nordicsemi.no>
Installing recv callback with net_context_recv() after
net_context_connect() left an opening for a possible race - in case the
server send some data immediately after establishing TCP connection, and
Zephyr did not manage to install the callback on time, the data would be
lost, corrupting the stream.
This can be avoided, by installing the recv callback before the
connection is triggered. As net_context_recv() called w/o timeout only
registers the callback function, it should have no negative impact. The
only change on the TCP side is when the connection is closed - in case
TCP is in connect stage, do not call the recv callback (before this
change it'd be NULL at that point).
Signed-off-by: Robert Lubos <robert.lubos@nordicsemi.no>
The coap client takes requests and provides responses
asynchronously to callback given in a request.
Currently supports only 1 request at a time.
Signed-off-by: Jarno Lämsä <jarno.lamsa@nordicsemi.no>
In case recv() call was waiting for data, and the socket was closed from
another thread, the recv() call would not be interrupted, causing the
receiving thread to be blocked indefinitely.
Fix this, by signalling the condvar the recv() call is waiting on
close(). Additionally, close will now set the socket into error mode,
with EINTR as the error condition, allowing the blocked calls to
recognise that the call was interrupted, and return a proper error code
on the event.
Signed-off-by: Robert Lubos <robert.lubos@nordicsemi.no>
Variable declarations are moved to the beginning of the block in
which they are visible to ensure consistency with the remainder
of the code base.
Signed-off-by: Florian Grandel <fgrandel@code-for-humans.de>
The encryption-only security level has been removed from the
spec, see IEEE 802.15.4-2020, 9.4.2.2 Security Level field.
The standard provides the following explanation (ibid):
"This security level is deprecated and shall not be used in
implementation compliant with this standard. Devices that
receive frames with security level 4 shall discard them, as
described in 9.2.4. The counter mode encryption and cipher
block chaining message authentication code (CCM) used allows
trivial changes to the underlaying encrypted data unless
data authenticity is provided, thus using data confidentiality
only is not useful. In the case of TSCH mode, security level 4
allows higher security level frames to be downgraded to
security level 4 frames."
Signed-off-by: Florian Grandel <fgrandel@code-for-humans.de>
See IEEE 802.15.4-2020, 7.2.2.11 Source Addressing Mode field and
7.2.2.9 Destination Addressing Mode field, table 7-3: The
previously deprecated "Simple addressing mode" was removed
from the spec.
Signed-off-by: Florian Grandel <fgrandel@code-for-humans.de>
Currently the insertion of an authentication tag requires a memcpy() call
and breaks encapsulation.
This change removes the need for memcpy() and improves the encapsulation
by calculating and reserving the required headspace early on while
keeping insertion where it belongs in the outgoing security procedure.
This is also a preparation for improved standard compliance of the
outgoing security procedure which is scheduled for a later commit.
Signed-off-by: Florian Grandel <fgrandel@code-for-humans.de>
The naming of variables and arguments containing the authentication
tag length was inconsistent:
* Naming inconsistency between header "length" vs. authtag "size"
in the same API calls
* "Tag" rather than "Auth[orization ]Tag" in external API calls
which is too generic from a compliance and readability viewpoint.
This is in preparation to zero-copy authentication support.
Almost all call sites will be subject to required structural changes
later on so no relevant git blame noise/history loss will be introduced
by this naming change in the long run.
Signed-off-by: Florian Grandel <fgrandel@code-for-humans.de>
Over time, some non-standard concepts and extensions were introduced
into the stack (in KConfig, in drivers, in the internal API and into the
implementation) which makes introduction of additional standard-
compliant extensions like TSCH (and others) unnecessarily difficult.
To introduce extensions like TSCH it is required for the IEEE 802.15.4
stack to become more structurally aligned with the standard again which
will be the focus of some of the upcoming preparatory changes.
One way to check and prove standard compliance is to reference the
standard from within the source code. This change therefore introduces
inline references to the IEEE 802.15.4-2020 standard wherever possible.
Deviations from the standard are documented with TODO or deprecation
labels to be addressed in future changes.
In the future, new code introduced to the IEEE 802.15.4 stack should
be documented and reviewed for standard-compliance to avoid further
divergence. Most importantly:
* MAC/PHY configuration (via net mgmt, radio API, devicetree or
KConfig) should always be directly linked to well-defined MAC/PHY
PIB attributes if visible to the MAC API or the end user.
* Net management/shell/radio API commands should have a documented
reference to the corresponding MLME action from the standard.
Signed-off-by: Florian Grandel <fgrandel@code-for-humans.de>
According to RFC768 UDP packets with zero checksum are allowed with IPv4.
Enable this by default.
For example, some routers use zero checksum in DHCP packets.
Signed-off-by: Nils Ruf <nils.ruf@endress.com>
The current implicit MAC payload length algorithm (based on
an otherwise irrelevant footer pointer) produces invalid
(non-standard) values for beacon and command frames.
This change produces standard-conforming MAC payload length
values and simplifies access to payload length.
It would have been possible to fix the current footer pointer
based approach but there are arguments in favor of the new
approach:
- The footer pointer is used nowhere in the current code
base and makes length calculations rather non-obvious.
- The new approach does not use more memory and is easier
to understand and use.
- This change is a first step to support of IEEE 802.15.4
information element (IE) support. At a later stage the
distinction between MAC payload length and frame payload
length will be introduced and become relevant to
distinguish between header and payload IEs. At that point
the current implicit length calculation algorithm will
break down anyways.
Signed-off-by: Florian Grandel <fgrandel@code-for-humans.de>
The current IEEE 802.15.4 stack would not accept beacons
unless hardware filtering was active.
This change fixes the bug.
Signed-off-by: Florian Grandel <fgrandel@code-for-humans.de>
The shell command calling the IEEE 802.15.4 scanning procedure
did not properly release its net management event callback.
This change fixes the memory leak.
Signed-off-by: Florian Grandel <fgrandel@code-for-humans.de>
Active and passive scanning requires the channel and PAN to
be set temporarily.
This change ensures that the temporary configuration will
be reverted even when the scan is aborted due to an error.
Signed-off-by: Florian Grandel <fgrandel@code-for-humans.de>
Beacon and command frames are consumed by the IEEE 802.15.4
stack internally and should therefore be released before
returning control to the generic net stack.
This change fixes the resource leak.
Signed-off-by: Florian Grandel <fgrandel@code-for-humans.de>
In certain scenarios, it may be necessary to get values of additional
options from the application layer. With this patch, this can be
accomplished by registering a callback with the DHCP client.
This change has been tested using the posix build in qemu.
Signed-off-by: Matthias Breithaupt <m.breithaupt@vogl-electronic.com>
Until now iterable sections APIs have been part of the toolchain
(common) headers. They are not strictly related to a toolchain, they
just rely on linker providing support for sections. Most files relied on
indirect includes to access the API, now, it is included as needed.
Signed-off-by: Gerard Marull-Paretas <gerard.marull@nordicsemi.no>
This change splits the IEEE 802.15.4 test into two separate
test profiles, one with and one without sockets enabled to ensure
that both configurations work correctly.
Signed-off-by: Florian Grandel <fgrandel@code-for-humans.de>
The current IEEE 802.15.4 stack assumes that drivers will
not provide more than one fragment in incoming packages.
This change exposes and enforces the pre-existing assumption
made by the implementation.
So far this is not a limiting restriction as MPDUs with more
than 127 bytes are not supported yet. It will probably have to
be changed as soon as larger payloads (as allowed by PHYs
introduced more recently into the spec) might want to allocate
smaller fragments.
Signed-off-by: Florian Grandel <fgrandel@code-for-humans.de>
When changing IEEE 802.15.4 security settings or setting security
to 'none' then the previous session must be cleaned up to avoid
resource leaks.
This change introduces proper clean-up of the security session.
Signed-off-by: Florian Grandel <fgrandel@code-for-humans.de>
Shifting "timeout <<= conn->zwp_retries" by more then 63 bits results in
undefined behaviour.
Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
If a socket has DSCP set then the packets from the socket should also be
marked with appropriate priority in case traffic classes are used in
networking stack.
Signed-off-by: Krishna T <krishna.t@nordicsemi.no>
This is needed for applications that rely on WPA supplicant being
in disconnected state before issue subsequent commands (e.g., issue scan
immediately after disconnect fails, until disconnect is completed).
Signed-off-by: Krishna T <krishna.t@nordicsemi.no>
In the FIN_WAIT_1 any incoming data is dropped, but anknowledged. Add a
check to see if the data is already acknowleged to prevent double
acknowledging of the data and bringing the acknowlegde counter out of
sync. When the acknowledge counter gets out of sync, the connection will
never properly terminate any more.
Signed-off-by: Sjors Hettinga <s.a.hettinga@gmail.com>
