Skip to content

fix(signed): re-request session info when a reply fails authentication - #172

Merged
Bre77 merged 1 commit into
mainfrom
fm/scout-tfa-session-info-auth
Oct 2, 2026
Merged

Bre77 merged 1 commit into
mainfrom
fm/scout-tfa-session-info-auth

Conversation

@Bre77

@Bre77 Bre77 commented Oct 1, 2026

Copy link
Copy Markdown
Member

Summary

A session_info reply whose session_info_tag fails authentication made the handshake fail on the first bad reply. Tesla's vehicle-command discards such a reply instead ("Discarding unauthenticated session info", internal/dispatcher/dispatcher.go). Its tryStartSession then re-sends the unsigned, idempotent SessionInfoRequest after RetryInterval(), which is 1 s on BLE.

Commands._handshake now does the same. On SessionInfoAuthenticationFault it re-sends the request, up to 3 attempts 1 s apart (_handshake_attempts / _handshake_retry_interval). It raises the fault after the last attempt.

  • An unauthenticated reply is still never trusted.
  • Each attempt uses a fresh request uuid, so each reply must authenticate against its own challenge.

Why

Seen live during BLE pairing, on Home Assistant 2026.10.0b0 with 1.17.2. About 0.3 s after the vehicle acknowledged the key-card-approved whitelist add, VCSEC's first handshake reply had a session_info_tag with a zero-length tag (signature_data = 6a 02 32 00). The rest of the SessionInfo was sane: the vehicle's public key, status OK, counter 0 and the new key handle 14. The key was enrolled, but HA reported "Bluetooth security handshake failed: Session info reply failed authentication and was discarded."

Ruled out:

  • Logging artifact: re-serializing the logged message gives exactly the 141-byte wire frame.
  • Protobuf decode: field numbers are identical in tesla-protocol 2.2.0, 3.0.0 and upstream Go. Go decodes the same frame as tag == nil.
  • HMAC derivation: it matches vehicle-command byte for byte, checked against a fresh Go-generated vector.

The difference is purely what happens after the reply is rejected.

Out of scope: the separate establish_connection timeouts seen on the same evening are not addressed here. They predate the pairing, and the link was closed cleanly after the fault.

Tests

tests/test_session_info_authentication.py::EmptyTagHandshakeRetryTests, using the captured 141-byte frame as the fixture:

  • test_captured_reply_has_present_but_empty_tag: the frame round-trips, the tag is present but empty, and validate_msg rejects it (session not ready).
  • test_handshake_retries_past_one_empty_tag_reply: runs through the real BLE _send/_await_response with only GATT faked. The first reply is the captured frame and the second is validly tagged; the session becomes ready after exactly 2 writes. Fails on main.
  • test_persistent_empty_tag_still_raises: still raises after exactly 3 writes, and the session is not ready. Fails on main (main stops after 1 write).

The existing forged-reply test still passes; its retry interval is shortened so it stays fast.

Gate: uv run pytest tests: 876 passed. uv run pyright tesla_fleet_api: 0 errors. uv run ruff check tesla_fleet_api: clean.

Changelog

  • Fix: a BLE/signed handshake no longer fails on a single unauthenticated session_info reply (for example VCSEC's empty tag right after pairing). The request is re-sent up to 3 times, 1 s apart, matching Tesla's vehicle-command.

No version bump or release in this PR.

A session_info reply whose session_info_tag fails authentication made the
handshake fail on the first bad reply. Tesla's vehicle-command instead
discards it and re-sends the unsigned, idempotent SessionInfoRequest after
RetryInterval (1s on BLE). Match that: Commands._handshake now retries up
to 3 attempts 1s apart on SessionInfoAuthenticationFault and still raises
after the last. The unauthenticated reply is never trusted.

Observed live: right after a key-card-approved whitelist add, VCSEC's first
handshake reply carried session_info_tag with a zero-length tag
(signature_data = 6a 02 32 00) alongside an otherwise sane SessionInfo
(status OK, new key handle), so pairing appeared to fail although the key
was enrolled. The captured 141-byte frame is the regression fixture.
@coderabbitai

coderabbitai Bot commented Oct 1, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: Teslemetry/coderabbit/.coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: 28050300-458b-4e2e-8705-2dac17e75ad3

📥 Commits

Reviewing files that changed from the base of the PR and between 0acdc5f and b7c90e5.

📒 Files selected for processing (3)
  • AGENTS.md
  • tesla_fleet_api/tesla/vehicle/commands.py
  • tests/test_session_info_authentication.py

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 8 remain after this review.


📝 Walkthrough

Walkthrough

The BLE handshake now retries session-info requests after authentication failures, up to three attempts with a one-second interval. Tests cover replies with an empty authentication tag, recovery after a retry, and failure after all attempts.

Changes

BLE handshake

Layer / File(s) Summary
Empty-tag reply validation
tests/test_session_info_authentication.py
A captured session-info reply with an empty authentication tag is preserved and rejected by direct validation.
Handshake retry behavior
tesla_fleet_api/tesla/vehicle/commands.py, tests/test_session_info_authentication.py, AGENTS.md
Commands retries the session-info request after authentication failures, up to three attempts with a one-second interval. Tests cover recovery after a retry and persistent failure. AGENTS.md documents the retry behavior and the observed empty-tag case.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~12 minutes

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant Commands
  participant BLETransport
  participant AuthenticationValidation
  Commands->>BLETransport: Send session-info request
  BLETransport-->>Commands: Return session-info reply
  Commands->>AuthenticationValidation: Validate reply authentication tag
  AuthenticationValidation-->>Commands: Return success or authentication fault
  Commands->>BLETransport: Retry request after authentication fault
Loading

Merge Risk: ⚪ Minimal · up to b7c90

The handshake can recover from an unauthenticated reply without trusting it, while persistent failures still raise after three attempts. No actionable merge-blocking risk is identified, subject to normal checks.

Security Architecture Review

Security architecture risk: 🔵 Low · up to b7c90

Retries remain bounded and preserve authentication checks before session state is accepted. The change also affects Fleet API handshakes, not only Bluetooth, so its timing and traffic effects extend beyond the reported pairing failure. No authentication bypass was identified in the examined paths.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The behavioral change follows existing vehicle session instances and both supported transport paths. It increases session-discovery attempts across vehicle-security and infotainment handshakes, without adding an operation payload or additional signing authority to the retry loop.

Security Findings and Attack Paths

  • inferred — An actor able to inject replies that fail session-info authentication can cause bounded additional discovery traffic and delay. The new loop does not accept the rejected reply or repeat vehicle actuation; persistent rejection still terminates with an authentication fault.

Trust Boundaries and Controls

  • observed — Incoming session information remains untrusted until request-bound authentication succeeds. An absent wire-level request UUID is permitted for hardware compatibility, but HMAC challenge binding remains required. The existing empty-public-key, not-on-whitelist exception raises a failure rather than establishing a session.

Resilience and Maintainability Implications

  • inferred — Existing per-domain locks serialize request/response ownership in both transports. Bluetooth drains queued replies before sending again, while fresh challenges prevent late replies from authenticating against a different attempt. Cancellation releases the asynchronous lock and is not converted into a retry. These controls support failure containment across repetition and concurrent handshakes.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 36.36% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 11 functions across 2 files. (1 skipped: … Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description check ✅ Passed The description clearly explains the handshake retry change, the BLE authentication failure scenario, the test coverage, and the validation results.
Title check ✅ Passed The title clearly and concisely identifies the main change: re-requesting session information when authentication fails.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

Docstring coverage is 36.36% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 11 functions across 2 files. (1 skipped: 1 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.


Comment @coderabbitai help to get the list of available commands.

@Bre77
Bre77 merged commit b849266 into main Oct 2, 2026
7 checks passed
@Bre77 Bre77 mentioned this pull request Oct 6, 2026
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