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.

Sign up for GitHub

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

Jump to bottom

mqtt: fix consumed bytes computation for truncated msg #7264

Closed
wants to merge 1 commit into from
Closed

mqtt: fix consumed bytes computation for truncated msg #7264

wants to merge 1 commit into from

Conversation

inashivb
Copy link
Member

@catenacyber @inashivb
mqtt: fix consumed bytes computation for truncated msg
ecd9586 
Ticket: 5268
(cherry picked from commit 3b13008)
@victorjulien victorjulien mentioned this pull request Apr 19, 2022
Merged
@victorjulien
Copy link
Member

Merged in #7271, thanks!

@inashivb inashivb deleted the next/605/20220419/v1 branch April 20, 2022 06:08
victorjulien added a commit to victorjulien/suricata that referenced this pull request Sep 20, 2024
@victorjulien
stream: improve 3whs completed by ACK with data
b088de8 
If the ACK completing the 3whs, the stream engine will transition
to "established". However, the packet itself will not be tagged as
"established". This will only happen for the next packet after the 3whs,
so that `flow:established` only matches after the 3whs.

It is also possible that the ACK completing the 3whs was lost. Since the
ACK packets themself are not acknoledged, there will be no
retransmission of them. Instead, the next packet can have the ACK flag
as well as data.

This case was mishandled in a suble way. The stream engine state
transition was done correctly, as well as the data handling and
app-layer updates. However, the packet itself was not tagged as
"established", which meant that `flow:established` would not yet match.

This patch detects this case and tags the packet as established if ACK
with data is received that completes the 3whs.

Bug: OISF#7264.
@victorjulien victorjulien mentioned this pull request Sep 20, 2024
Closed
victorjulien added a commit to victorjulien/suricata that referenced this pull request Sep 23, 2024
@victorjulien
stream: improve 3whs completed by ACK with data
ae044f3 
If the ACK completing the 3whs, the stream engine will transition
to "established". However, the packet itself will not be tagged as
"established". This will only happen for the next packet after the 3whs,
so that `flow:established` only matches after the 3whs.

It is also possible that the ACK packet completing the 3whs was lost. Since
the ACK packets themselves are not acknowledged, there will be no
retransmission of them. Instead, the next packet can have the ACK flag
as well as data.

This case was mishandled in a subtle way. The stream engine state
transition was done correctly, as well as the data handling and
app-layer updates. However, the packet itself was not tagged as
"established", which meant that `flow:established` would not yet match.

This patch detects this case and tags the packet as established if ACK
with data is received that completes the 3whs.

Bug: OISF#7264.
@victorjulien victorjulien mentioned this pull request Sep 23, 2024
Closed
victorjulien added a commit to victorjulien/suricata that referenced this pull request Sep 23, 2024
@victorjulien
stream: improve 3whs completed by ACK with data
9b27c18 
If the ACK packet completing the 3whs is received, the stream engine will
transition to "established". However, the packet itself will not be tagged
as "established". This will only happen for the next packet after the 3whs,
so that `flow:established` only matches after the 3whs.

It is possible that the ACK packet completing the 3whs was lost. Since the
ACK packets themselves are not acknowledged, there will be no retransmission
of them. Instead, the next packet can have the expected ACK flag as well as
data.

This case was mishandled in a subtle way. The stream engine state transition
was done correctly, as well as the data handling and app-layer updates.
However, the packet itself was not tagged as "established", which meant
that `flow:established` would not yet match.

This patch detects this case and tags the packet as established if ACK
with data is received that completes the 3whs.

Bug: OISF#7264.
@victorjulien victorjulien mentioned this pull request Sep 23, 2024
Closed
victorjulien added a commit to victorjulien/suricata that referenced this pull request Sep 24, 2024
@victorjulien
stream: improve 3whs completed by ACK with data
45eb7e4 
If the ACK packet completing the 3whs is received, the stream engine will
transition to "established". However, the packet itself will not be tagged
as "established". This will only happen for the next packet after the 3whs,
so that `flow:established` only matches after the 3whs.

It is possible that the ACK packet completing the 3whs was lost. Since the
ACK packets themselves are not acknowledged, there will be no retransmission
of them. Instead, the next packet can have the expected ACK flag as well as
data.

This case was mishandled in a subtle way. The stream engine state transition
was done correctly, as well as the data handling and app-layer updates.
However, the packet itself was not tagged as "established", which meant
that `flow:established` would not yet match.

This patch detects this case and tags the packet as established if ACK
with data is received that completes the 3whs.

Bug: OISF#7264.
victorjulien added a commit to victorjulien/suricata that referenced this pull request Sep 25, 2024
@victorjulien
stream: improve 3whs completed by ACK with data
374a78f 
If the ACK packet completing the 3whs is received, the stream engine will
transition to "established". However, the packet itself will not be tagged
as "established". This will only happen for the next packet after the 3whs,
so that `flow:established` only matches after the 3whs.

It is possible that the ACK packet completing the 3whs was lost. Since the
ACK packets themselves are not acknowledged, there will be no retransmission
of them. Instead, the next packet can have the expected ACK flag as well as
data.

This case was mishandled in a subtle way. The stream engine state transition
was done correctly, as well as the data handling and app-layer updates.
However, the packet itself was not tagged as "established", which meant
that `flow:established` would not yet match.

This patch detects this case and tags the packet as established if ACK
with data is received that completes the 3whs.

Bug: OISF#7264.
(cherry picked from commit 45eb7e4)
@victorjulien victorjulien mentioned this pull request Sep 25, 2024
Closed
victorjulien added a commit to victorjulien/suricata that referenced this pull request Sep 25, 2024
@victorjulien
stream: improve 3whs completed by ACK with data
4f59fd9 
If the ACK packet completing the 3whs is received, the stream engine will
transition to "established". However, the packet itself will not be tagged
as "established". This will only happen for the next packet after the 3whs,
so that `flow:established` only matches after the 3whs.

It is possible that the ACK packet completing the 3whs was lost. Since the
ACK packets themselves are not acknowledged, there will be no retransmission
of them. Instead, the next packet can have the expected ACK flag as well as
data.

This case was mishandled in a subtle way. The stream engine state transition
was done correctly, as well as the data handling and app-layer updates.
However, the packet itself was not tagged as "established", which meant
that `flow:established` would not yet match.

This patch detects this case and tags the packet as established if ACK
with data is received that completes the 3whs.

Bug: OISF#7264.
(cherry picked from commit 45eb7e4)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@victorjulien victorjulien Awaiting requested review from victorjulien victorjulien is a code owner

@jasonish jasonish Awaiting requested review from jasonish jasonish is a code owner

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@inashivb @victorjulien @catenacyber