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

nimble/ll: Fix encryption pause/restart procedure #1796

Merged

Conversation

andrzej-kaczmarek
Copy link
Contributor

This fixes 2 issues in encrpytion pause/restart procedure (i.e. key refresh) initiated in central role:

  1. We transition to "paused" encryption state when LL_PAUSE_ENC_RSP is txd, but that is done after we try to enqueue LL_ENC_REQ. This means LL_ENC_REQ is put at the end of tx queue. By conicidence this makes order of PDUs correct when there's only LL_PAUSE_ENC_RSP on tx queue, but it fails if there was an ACL packet already queued. In such case LL_ENC_REQ is queued after that ACL packet which means neither will be sent - ACL packet cannot be sent because we have no encryption, LL_ENC_REQ cannot be sent because there's ACL packet in front.
  2. We do not check if LL_ENC_REQ was properly queued (i.e. if mbuf was allocated for PDU) so it may happen that LL_ENC_REQ will never be queued.

In both scenarios the connection will timeout eventually since encryption restart procedure cannot be completed.

This fix ensures that LL_ENC_REQ is queued properly by using a "pending" flag that is evaluated when checking for pending procedures.

This fixes 2 issues in encrpytion pause/restart procedure (i.e. key
refresh) initiated in central role:

1. We transition to "paused" encryption state when LL_PAUSE_ENC_RSP is
   txd, but that is done after we try to enqueue LL_ENC_REQ. This means
   LL_ENC_REQ is put at the end of tx queue. By conicidence this makes
   order of PDUs correct when there's only LL_PAUSE_ENC_RSP on tx queue,
   but it fails if there was an ACL packet already queued. In such case
   LL_ENC_REQ is queued after that ACL packet which means neither will
   be sent - ACL packet cannot be sent because we have no encryption,
   LL_ENC_REQ cannot be sent because there's ACL packet in front.
2. We do not check if LL_ENC_REQ was properly queued (i.e. if mbuf was
   allocated for PDU) so it may happen that LL_ENC_REQ will never be
   queued.

In both scenarios the connection will timeout eventually since
encryption restart procedure cannot be completed.

This fix ensures that LL_ENC_REQ is queued properly by using a "pending"
flag that is evaluated when checking for pending procedures.
Copy link
Contributor

@rymanluk rymanluk left a comment

Choose a reason for hiding this comment

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

LGTM

@andrzej-kaczmarek andrzej-kaczmarek merged commit 554e063 into apache:master Jun 14, 2024
18 checks passed
@andrzej-kaczmarek andrzej-kaczmarek deleted the ll-enc-restart-fix branch June 14, 2024 09:11
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.

2 participants