Skip to content

Keep keys through transient Keystore errors and drop stale paired state - #138

Merged
dzid26 merged 4 commits into
mainfrom
work/key-store-followups
Oct 8, 2026
Merged

dzid26 merged 4 commits into
mainfrom
work/key-store-followups

Conversation

@dzid26

@dzid26 dzid26 commented Oct 7, 2026

Copy link
Copy Markdown
Collaborator

Summary

Follow-up to #135. Part of #116.

This implements the two follow-ups from #135's review notes:

1. PairingKeyStore: a transient Keystore lookup failure no longer deletes the key (Android 13+)

  • keystore2 reports a failed key lookup (busy, system error) as UnrecoverableKeyException, which Keep pairing keys through transient Keystore errors #135 counts as a definite loss.
  • On API 33+, if that exception's cause is an android.security.KeyStoreException with isTransientFailure() == true, load() now returns null and keeps the key.
  • Below API 33 nothing changes. The check is guarded by Build.VERSION.SDK_INT >= TIRAMISU, so the API 33 class and method are only reached there.
  • A missing Keystore key (material from another install) still counts as definite, since it has no transient cause.

2. TeslaBleController: a complete whitelist without our key drops the stale paired state

  • Before, a cached key slot or a pairing OK kept a car shown as paired after its key was removed in the car, or after a restore brought back a key the car no longer has. The only way out was Settings → Clear pairing cache.
  • Now a complete listing that doesn't contain our key ID drops the cached slot (persisted and per connection) and a stale OK. The car then shows as unpaired and offers pairing, with one log line that has no VIN and no key bytes.
  • A listing counts as complete only if the car reports at least one key, lists exactly that many key IDs, its slot mask has the same count, and every ID is long enough to compare. A partial, inconsistent or failed read changes nothing.
  • A pairing in progress is left alone, and so is the pair check, which still enrolls a fresh key.
  • The cleanup from Keep pairing keys through transient Keystore errors #135 in handleSessionInfo now uses the same helper. Its behavior is unchanged.
  • Scan code is untouched.

Type

  • Bug fix
  • Feature
  • Protocol / crypto change
  • Docs / CI / tooling
  • Refactor (no behavior change)

Testing

The app module can't be built where this was written (Google Maven is blocked). Verified there:

  • ktlint and detekt, using the repo config, on the changed files: clean.
  • The changed PairingKeyStore and TeslaBleController compile against main's core plus minimal Android stubs.
  • A throwaway JVM harness (not committed) ran the real classes with a fake AndroidKeyStore provider (real AES-GCM), in-memory prefs and a fake GATT transport fed with real Wire-encoded responses. 32 checks pass.
    • The 4 new key-store cases cover a transient cause on 33 (kept), a non-transient cause (dropped), a transient cause on 32 (dropped), and a missing Keystore key on 33 (dropped).
    • The 11 controller cases cover:
      • a complete listing without our key: slot dropped, persisted, logged once;
      • partial, mask-mismatched, short-ID and empty listings: slot kept;
      • listings with our key, as a full SHA-1 or a 4-byte ID: slot kept;
      • a pairing in progress: untouched;
      • the pair check: still enrolls a fresh key;
      • a stale OK: reset;
      • the Keep pairing keys through transient Keystore errors #135 handshake path through the shared helper.
    • Removing the completeness check, the cleanup call or the API-level guard makes the matching cases fail.
  • No new tests in the repo (no Robolectric).

Needs a real-car check:

  • With the phone key removed in the car (Controls > Locks), reconnect: the car shows "Not paired" and offers pairing.

  • With the key still enrolled, it keeps "Paired · slot N" after a reconnect.

  • ./gradlew :core:test (core untouched)

  • ./gradlew :app:testDebugUnitTest (left to CI)

  • ./gradlew :app:lintDebug (left to CI; also checks the NewApi guard)

  • Real car (see above)

Checklist

  • Protocol changes ship test vectors; expected.txt regenerated, not hand-edited (n/a: no protocol change)
  • No VINs, keys, or decrypted payloads in logs
  • Docs/master-plan updated where relevant (n/a)
  • New source files carry SPDX-License-Identifier: AGPL-3.0-only (n/a: no new files)

Notes for review

  • The "complete listing" test is deliberately strict. If some firmware reports a slot mask whose count differs from the entry count, the rule never fires there, and the car keeps today's behavior.
  • A pairing OK followed by a complete listing without the new key would be reset to unpaired. The car sends the add-key OK only after it has added the key, so this shouldn't happen in practice.

🤖 Generated with Claude Code

https://claude.ai/code/session_0129RzhbEE2cboMcYyZMuxqX


Generated by Claude Code

claude added 2 commits October 7, 2026 22:02
keystore2 reports a failed key lookup (busy, system error) as an
UnrecoverableKeyException, which counted as a definite key loss and
deleted the key. On Android 13+ its cause, android.security
.KeyStoreException, says whether the failure is transient. If it is,
load() now returns null and keeps the key. Below Android 13 nothing
changes.

Part of #116.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0129RzhbEE2cboMcYyZMuxqX
A cached key slot, or a pairing OK, kept a car shown as paired after
its key was removed in the car, or after a restore brought back a key
the car no longer has. The only way out was Settings, Clear pairing
cache.

When a whitelist listing is complete and our key ID is not in it,
forget the cached slot and a stale OK, so the car shows as unpaired and
offers pairing. Complete means the car reports at least one key, lists
an ID for each, its slot mask has the same count, and every ID is long
enough to compare. A partial or failed read changes nothing, and a
pairing in progress is left alone. handleSessionInfo now uses the same
helper for keys the store dropped.

Part of #116.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0129RzhbEE2cboMcYyZMuxqX
@github-actions

github-actions Bot commented Oct 7, 2026

Copy link
Copy Markdown
Contributor

Screenshots

App screenshots: light and dark

Captured from the demo build at 6aa3a72.

@github-actions

github-actions Bot commented Oct 7, 2026

Copy link
Copy Markdown
Contributor

Screenshots

App screenshots: light and dark

Captured from the demo build at 8f8fb14.

Debug APK: zip from Android CI (GitHub sign-in needed)

A value pasted on Windows ends in CRLF, and base64 -d rejects the
carriage return ("invalid input"), which failed the build job before
it produced an APK. Strip all whitespace before decoding; this also
accepts line-wrapped encoder output such as certutil's.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0129RzhbEE2cboMcYyZMuxqX
@github-actions

github-actions Bot commented Oct 8, 2026

Copy link
Copy Markdown
Contributor

The build re-run failed at "Set up the shared CI debug key": the new CI_DEBUG_KEYSTORE_BASE64 repository variable ends in a Windows line break, which base64 -d rejects. Nothing in this PR's code caused it. 6ac4527 ports the one-line fix from #141 (strip whitespace before decoding), so this build now uses the shared key. The change is identical to #141's and merges cleanly whichever lands first.


Generated by Claude Code

@github-actions

github-actions Bot commented Oct 8, 2026

Copy link
Copy Markdown
Contributor

Screenshots

App screenshots: light and dark

Captured from the demo build at 6ac4527.

Debug APK: zip from Android CI (GitHub sign-in needed)

@dzid26
dzid26 merged commit 352bd80 into main Oct 8, 2026
6 checks passed
@dzid26
dzid26 deleted the work/key-store-followups branch October 8, 2026 06:45
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