Skip to content

Downlink replay protection rework - #335

Open
trond-snekvik wants to merge 2 commits into
mainfrom
replay_check
Open

Downlink replay protection rework#335
trond-snekvik wants to merge 2 commits into
mainfrom
replay_check

Conversation

@trond-snekvik

Copy link
Copy Markdown
Collaborator

As reported in #314, the downlink replay protection can be disarmed by
pushing an invalid downlink session, as the SESSION_VALID flag gets cleared
at the start of each session, which disables the is_valid_downlink check.

The fix #314 only addresses the invalid sequence number decrement. This bug
also disables replay protection for sessions that follow a session with
only corrupted pouches, though which has to be addressed separately. We
also reinitialize the session for every received pouch, which resets the
session pouch ID, preventing the pouch ID replay check from working
correctly.

This patch addresses the core issue that causes the bug addressed in #314,
and is intended to supersede #314. It additionally adds a check for the
block size log parameter, and adds a server.has_seqnum flag that replaces
the validation check for server.seqnum the SESSION_VALID flag
previously covered.

Note

As replay protection is not implemented on the server side, the
session replay protection defect does not affect any active deployments,
but all the while this code exists in this repo, it needs to be correct.

Adds a quick test that verifies the ruleset.

As reported in #314, the downlink replay protection can be disarmed by
pushing an invalid downlink session, as the SESSION_VALID flag gets cleared
at the start of each session, which disables the `is_valid_downlink` check.

The fix #314 only addresses the invalid sequence number decrement. This bug
also disables replay protection for sessions that follow a session with
only corrupted pouches, though which has to be addressed separately. We
also reinitialize the session for every received pouch, which resets the
session pouch ID, preventing the pouch ID replay check from working
correctly.

This patch addresses the core issue that causes the bug addressed in #314,
and is intended to supersede #314. It additionally adds a check for the
block size log parameter, and adds a `server.has_seqnum` flag that replaces
the validation check for `server.seqnum` the `SESSION_VALID` flag
previously covered.

Note: As replay protection is not implemented on the server side, the
session replay protection defect does not affect any active deployments,
but all the while this code exists in this repo, it needs to be correct.

Signed-off-by: Trond Snekvik <trond.snekvik@canonical.com>
Adds a downlink_session test that runs through the rules in the downlink
session creation and validation.

Signed-off-by: Trond Snekvik <trond.snekvik@canonical.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant