Skip to content

gnrc_tcp: allow unknown options#12298

Merged
miri64 merged 1 commit intoRIOT-OS:masterfrom
brummer-simon:gnrc_tcp-allow_unknown_options
Sep 24, 2019
Merged

gnrc_tcp: allow unknown options#12298
miri64 merged 1 commit intoRIOT-OS:masterfrom
brummer-simon:gnrc_tcp-allow_unknown_options

Conversation

@brummer-simon
Copy link
Copy Markdown
Member

The latest hardening of the gnrc_tcp option parser was a bit too restrictive.

Packets with unknown options were dropped by gnrc_tcp. This leads to problems then communicating with feature complete tcp implementations, since valid options like SACK are often announced in first SYN Packet. Without this PR such a SYN would be dropped by gnrc_tcp.

This PR allows packets with unknown options to be valid as long as they respect the boundaries of the option field and the option minimal length.

@brummer-simon
Copy link
Copy Markdown
Member Author

@miri64 @nmeum : As non-maintainer I can't request a review from a specific person. Would you both do me a favour an review this PR?

@miri64 miri64 self-requested a review September 24, 2019 17:35
@miri64 miri64 added Area: network Area: Networking CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) labels Sep 24, 2019
Copy link
Copy Markdown
Member

@miri64 miri64 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK

@miri64 miri64 merged commit 58e3384 into RIOT-OS:master Sep 24, 2019
@brummer-simon brummer-simon deleted the gnrc_tcp-allow_unknown_options branch September 24, 2019 18:55
@kb2ma kb2ma added this to the Release 2019.10 milestone Sep 29, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area: network Area: Networking CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants