net: tcp: Send RST reply for unexpected TCP packets #64029
Merged
Conversation
This file contains 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
rlubos
force-pushed
the
net/tcp-fix-missing-rst-on-unexpected-packets
branch
from
86bd790
to
7d8cd44
Compare
Add a helper function which allows to send a RST packet in response to an unexpected TCP packet, w/o associated connection or net context. Signed-off-by: Robert Lubos <robert.lubos@nordicsemi.no>
rlubos
force-pushed
the
net/tcp-fix-missing-rst-on-unexpected-packets
branch
from
7d8cd44
to
a4128c1
Compare
jukkar
reviewed
Oct 18, 2023
| if (IS_ENABLED(CONFIG_NET_TCP) && proto == IPPROTO_TCP) { | ||
| net_tcp_reply_rst(pkt); |
There was a problem hiding this comment.
In Linux one can use netfilter to prevent RST to be sent out if there is no socket bind to the port, and in Zephyr that might be possible at some point using net_pkt_filter (currently not possible). So I was thinking that should we have a Kconfig option to allow earlier behavior, so that RST would not be sent if option is set, by default the option could be off so RST would be sent out. WDYT?
There was a problem hiding this comment.
Sure, I can add such an option.
There was a problem hiding this comment.
@jukkar I've added a Kconfig option to configure whether to send RST packet on closed port. If disabled, ICMP is sent instead as it used to.
Send RST as a reply for unexpected TCP packets in the following scenarios: 1) Unexpected ACK value received during handshake (connection still open on the peer side), 2) Unexpected data packet on a listening port (accepted connection closed), 3) SYN received on a closed port. This allows the other end to detect that the connection is no longer valid (for example due to reboot) and release the resources. Signed-off-by: Robert Lubos <robert.lubos@nordicsemi.no>
Add tests which verify that Zephyr's TCP stack replies with RST packet as expected. Signed-off-by: Robert Lubos <robert.lubos@nordicsemi.no>
Add Kconfig option to control TCP RST behavior on connection attempts on unbound ports. If enabled, TCP stack will reply with RST packet (enabled by default). Signed-off-by: Robert Lubos <robert.lubos@nordicsemi.no>
The test delegates putting the server socket into listening (i. e. calling listen() on the socket) to a separate thread, which did have a chance to run before client attempted to establish TCP connection. This was not visible before, as we did not reply with RST to a connection attempt on a closed port, so the connection was eventually establish after SYN retransmission. But as we do reject such a connection now with RST, the connection attempt failed. Therefore, a small delay was added after spawning the server thread, to give it a chance to configure the server socket. Additionally, lower the CONFIG_NET_TCP_TIME_WAIT_DELAY value so that TCP contexts are released earlier, and add a respective delay in the test teardown function. Not doing so also triggered unneeded SYN retransmissions, as there were no enough TCP context to accept the incoming connection, before freeing the resources allocated for the previous one. Signed-off-by: Robert Lubos <robert.lubos@nordicsemi.no>
rlubos
force-pushed
the
net/tcp-fix-missing-rst-on-unexpected-packets
branch
from
a4128c1
to
177984f
Compare
|
@cfriedt Thrift tests needed small adjustments due to timing issues, please check the commit message for details. |
jukkar
approved these changes
Oct 18, 2023
cfriedt
approved these changes
Oct 18, 2023
fabiobaltieri
added
backport v3.5-branch
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.
This PR improves the handling of unexpected packets within Zephyr's TCP stack. The TCP stack will now send RST packet in a response to unexpected TCP packets, to address issues mostly related to ungraceful TCP connection shutdown (for example due to reboot):
on the peer side),
closed),
For this, a new helper function was introduced which allows to send an RST packet to any destination (current functions allow only to send TCP packets to destinations associated with a TCP connection context, which is not always the case in this scenario).
Finally, add a few new tests to cover this functionality.
Fixes #63966