Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

lwm2m: Check CoAP header offset #84550

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

mlasch
Copy link
Contributor

@mlasch mlasch commented Jan 25, 2025

There is a possibility where the Registration message is silently dropped when the header, as well as, the payload get too large. This primarily affects applications using long endpoint names and large messages or block transfer.

The LwM2M client has a Kconfig option CONFIG_LWM2M_ENGINE_MESSAGE_HEADER_SIZE to configure the maximum CoAP header length. The header length defaults to 48 bytes in a range from 24 to 128. For Registration messages, the header size is dynamic and specific to an application. The endpoint name max length configured with CONFIG_RD_CLIENT_ENDPOINT_NAME_MAX_LENGTH defaults to 33 (32 characters and a zero termination byte). This may cause the header ending up larger than configured.

When constructing a header for a LwM2M message, the final size is not checked. If the header is larger than the configured maximum size, it is writing into the area meant for the payload in the network packet buffer. In most cases this is not an issue, because the Registration message is rather small. However, if the Registration message would exceed the buffer reserved for the whole packet (determined by the block size), the packet can't be assembled (-ENOMEM). This prevents the packet to be sent, as well as using a block transfer for even larger messages. In addition, since the result of async_message_send is not checked, the Registration message would be silently dropped without warning or error to the user.

The issue can be reproduced by extending the endpoint name and as a result increase the header size beyond the limit. The registration message is now lost, if the payload would be too larger that the space left in the buffer. Like when having a large number of objects registered with the client.

$ west build -b qemu_x86/atom samples/net/lwm2m_client -- -DCONFIG_LWM2M_APP_ID="\"AAAABBBBCCCCDDDDEEEEFFFFGGGGHHHHIIII\""
$ west build -t run
# all goes well even though the header is too large

A Bootstrap message would also exceed the header boundary, but since it has no payload it would no be dropped that easily.

This change adds a boundary check for every constructed CoAP packet, in order to ensure the headers always stay withing the configured limit. A check is also introduced for the result of async_message_send to propagate and log an error. This prevents issues with future modifications to the header which might have effects on the size independent of CONFIG_LWM2M_ENGINE_MESSAGE_HEADER_SIZE.

With the changes the output would look like this

[00:00:00.510,000] <inf> net_lwm2m_rd_client: RD Client started with endpoint 'AAAABBBBCCCCDDDDEEEEFFFFGGGGHHHH' with client lifetime 30 using server object 0
[00:00:00.510,000] <dbg> net_lwm2m_message_handling: lwm2m_parse_peerinfo: Parse url: coap://192.0.2.2:5683
[00:00:00.510,000] <inf> net_lwm2m_engine: Connected, sock id 0
[00:00:00.510,000] <dbg> net_lwm2m_registry: lwm2m_engine_get: path:1/0/1/0, level 3, buf:0x15b6c0, buflen:4
[00:00:00.510,000] <err> net_lwm2m_message_handling: CoAP header size exceeded, increase the value of CONFIG_LWM2M_ENGINE_MESSAGE_HEADER_SIZE to at least 71
[00:00:00.510,000] <err> net_lwm2m_rd_client: error -12 when sending registration message
[00:00:00.510,000] <err> net_lwm2m_rd_client: Registration err: -12
[00:00:00.510,000] <dbg> net_lwm2m_rd_client: lwm2m_rd_client_service: State: 18
[00:00:00.510,000] <err> net_lwm2m_rd_client: sm_do_network_error, retries 0

clearly showing the issue. Same for Bootstrap

[00:00:00.110,000] <inf> net_lwm2m_rd_client: Start LWM2M Client: AAAABBBBCCCCDDDDEEEEFFFFGGGGHHHH
[00:00:00.510,000] <dbg> net_lwm2m_rd_client: lwm2m_rd_client_service: State: 1
[00:00:00.510,000] <dbg> net_lwm2m_rd_client: lwm2m_rd_client_service: State: 2
[00:00:00.510,000] <dbg> net_lwm2m_registry: lwm2m_engine_get: path:0/0/1/0, level 3, buf:0x15bb8f, buflen:1
[00:00:00.510,000] <inf> net_lwm2m_rd_client: Bootstrap started with endpoint 'AAAABBBBCCCCDDDDEEEEFFFFGGGGHHHH' using security object 0
[00:00:00.510,000] <dbg> net_lwm2m_message_handling: lwm2m_parse_peerinfo: Parse url: coap://192.0.2.2:5683
[00:00:00.510,000] <inf> net_lwm2m_engine: Connected, sock id 0
[00:00:00.510,000] <err> net_lwm2m_message_handling: CoAP header size exceeded, increase the value of CONFIG_LWM2M_ENGINE_MESSAGE_HEADER_SIZE to at least 52
[00:00:00.510,000] <err> net_lwm2m_rd_client: Bootstrap registration err: -12
[00:00:00.510,000] <dbg> net_lwm2m_rd_client: lwm2m_rd_client_service: State: 18
[00:00:00.510,000] <err> net_lwm2m_rd_client: sm_do_network_error, retries 0

`lwm2m_send_message_async` returns critial errors which are not handled in
many cases. The function can return -ENOMEM, -EPERM or other error codes
from functions called.

Signed-off-by: Marc Lasch <[email protected]>
@mlasch mlasch force-pushed the gardena/ml/lwm2m-coap-header-offset branch from eaeb69d to b4c3289 Compare January 25, 2025 11:26
@mlasch
Copy link
Contributor Author

mlasch commented Jan 26, 2025

Some tests fail because they reuse the packet buffer and append CoAP option bytes somewhere in the middle of the packet. The offset of those options is now boundary checked to ensure they stay within the area of CONFIG_LWM2M_ENGINE_MESSAGE_HEADER_SIZE.
It looks like I have to adjust the tests such, that the packet buffer is reset before reusing it.

Check the CoAP header does not exceed the configured limits after all CoAP
options were addded.

Signed-off-by: Marc Lasch <[email protected]>
To ensure that CoAP options fit in the configured memory, the message
buffer has to be reset after every test.

Signed-off-by: Marc Lasch <[email protected]>
@mlasch mlasch force-pushed the gardena/ml/lwm2m-coap-header-offset branch from b4c3289 to 6a9154e Compare January 26, 2025 09:17
Create a stub function for `lwm2m_check_header_boundary` for fff.

Signed-off-by: Marc Lasch <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants