Skip to content

Fix race condition when decrypting notifications during lock disconnection #368

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

Merged
merged 3 commits into from
Jul 7, 2025

Conversation

bdraco
Copy link
Member

@bdraco bdraco commented Jul 3, 2025

Fix race condition when decrypting notifications during lock disconnection

Fixes #353

Problem

A race condition occurs when encrypted notifications arrive during device disconnection:

  1. Encrypted command sent to lock
  2. Device starts disconnecting (clears IV)
  3. Notification arrives with encrypted response
  4. _decrypt fails with RuntimeError: Cannot decrypt: IV is None

This timing-dependent issue is hard to reproduce and may vary across Bluetooth stacks.

Solution

Modified _notification_handler in switchbot/devices/lock.py to check the _expected_disconnect flag and skip processing lock notifications during expected disconnection. This prevents the race condition where notifications with encrypted data arrive after the IV has been cleared.

Changes

  • Added check in _notification_handler to return early if _expected_disconnect is True
  • Logs debug message when ignoring notifications during disconnect
  • Prevents _decrypt from being called with None IV during disconnection

Testing

  • All existing lock tests pass without modification
  • Fix prevents the RuntimeError from occurring during disconnection

@bdraco bdraco mentioned this pull request Jul 3, 2025
Copy link

codecov bot commented Jul 3, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Files with missing lines Coverage Δ
switchbot/devices/device.py 60.07% <100.00%> (+0.23%) ⬆️
switchbot/devices/lock.py 100.00% <100.00%> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

zerzhang

This comment was marked as duplicate.

@bdraco
Copy link
Member Author

bdraco commented Jul 4, 2025

As this return b'', We should add a check in _parse_lock_data to prevent indexError


    def _update_lock_status(self, data: bytearray) -> None:

        lock_data = self._parse_lock_data(self._decrypt(data[4:]), self._model)

        if self._update_parsed_data(lock_data):

            # We leave notifications enabled in case

            # the lock is operated manually before we

            # disconnect.

            self._reset_disconnect_timer()

            self._fire_callbacks()



    @staticmethod

    def _parse_lock_data(data: bytes, model: SwitchbotModel) -> dict[str, Any]:

        if not data:

            return {}

        if model == SwitchbotModel.LOCK:

            return {

                "calibration": bool(data[0] & 0b10000000),

                "status": LockStatus((data[0] & 0b01110000) >> 4),

                "door_open": bool(data[0] & 0b00000100),

                "unclosed_alarm": bool(data[1] & 0b00100000),

                "unlocked_alarm": bool(data[1] & 0b00010000),

            }

You should have access to push to this PR. Feel free to adjust. I won't have time to look soon given it's a holiday in the US

SwitchbotModel.LOCK_ULTRA,
],
)
def test_notification_handler_during_disconnect(
Copy link
Member Author

Choose a reason for hiding this comment

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

We could really use a good end to end test framework, but since we don't have one, this will have to do

@zerzhang zerzhang merged commit 544dd5a into master Jul 7, 2025
5 checks passed
@zerzhang zerzhang deleted the lock_iv_race branch July 7, 2025 12:38
@bdraco
Copy link
Member Author

bdraco commented Jul 7, 2025

I'll do a release

@zerzhang
Copy link
Collaborator

zerzhang commented Jul 7, 2025

I'll do a release

ok, Thanks @bdraco !

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.

Lock status abnormal in HA
2 participants