Skip to content

Commit 567a316

Browse files
committed
fix(ble): use False, not None, to disable command signing
An explicit `private_key=None` used to fall through to the parent's key and raise `ValueError("No private key.")` when there was none. The sentinel design in this PR changed that: `None` became the opt-out, so a caller who wrote `private_key=None` meaning "I haven't got one" silently got a vehicle that could not sign instead of the error that told them so. That is a behaviour change on a published, security-adjacent library, not an additive one. Drop `KeyOmitted`/`KEY_OMITTED` and restore `None` as the default. `False` is now the explicit "signing is disabled" value - a value no current caller can already be passing, so opting out has to be deliberate, and `None` keeps its existing meaning for both omitted and explicit-`None` callers. Because `False` and `None` are both falsy, the constructor branches on identity (`is False` / `is not None`); a truthiness check would collapse the two states and reintroduce the bug in a new form. `Vehicles.createBluetooth` had no `key` parameter at all, so the opt-out was unreachable from the Fleet-parented factory; it gains one as keyword-only (no positional shift), with the matching `NotImplementedError` overrides in Teslemetry/Tessie kept in step with their "parameters match the Fleet API Bluetooth factory" contract. The `SigningDisabled` guards at `_handshake` and in `pair()`'s fast path are unchanged.
1 parent 5fae915 commit 567a316

9 files changed

Lines changed: 104 additions & 66 deletions

File tree

AGENTS.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ Commands (vehicle/commands.py) - protobuf-based signed command implementation (A
7878
`Vehicles` (vehicle/vehicles.py) is a `dict[str, Vehicle]` with factory methods:
7979
- `createFleet(vin)``VehicleFleet`
8080
- `createSigned(vin)``VehicleSigned`
81-
- `createBluetooth(vin, confirmation="ack", keepalive_interval=..., raise_unconfirmed=False, *, verify_commands=None, optimistic=None)``VehicleBluetooth`
81+
- `createBluetooth(vin, confirmation="ack", keepalive_interval=..., raise_unconfirmed=False, *, verify_commands=None, optimistic=None, key=None)``VehicleBluetooth`
8282

8383
Teslemetry/Tessie override `Vehicles` with their own vehicle classes (`TeslemetryVehicle`, `TessieVehicle`) extending `VehicleFleet` with service-specific commands (e.g., `closure()`, `seat_heater()` for Teslemetry; `wake()`, `lock()` for Tessie).
8484

@@ -153,7 +153,7 @@ Keep the `tesla-protocol` floor at `>=0.5.0`; earlier releases have generated `.
153153
- **`VehicleAction`/`GetVehicleData` proto coverage is locked by test, not just by convention**: `tests/test_proto_coverage_lock.py` walks both descriptors and fails if any field has no wrapper (`commands.py`) or reader (`bluetooth.py`) and isn't on one of its two small, reasoned allowlists — keep that test in sync with any future `tesla-protocol` bump rather than special-casing new fields elsewhere. The only fields deliberately left unwrapped today are the 7-field push-style subscription/streaming family (`createStreamSession`/`streamMessage`/`vehicleDataSubscription`/`vehicleDataAck`/`vitalsSubscription`/`vitalsAck`/`cancelVehicleDataSubscription`, which need a public lifecycle/iterator API atop the private `_stream_sinks` routing above) and `getVehicleImageState` (needs chunked binary-transfer paging). CarServer's `GetVehicleState` sub-state is exposed as `legacy_vehicle_state()` (`bluetooth.py`), matching the `VehicleData.legacy_vehicle_state` reply field name, to avoid confusion with `vehicle_state()` (VCSEC `VehicleStatus`, a different message/domain). `set_rate_tariff`/`add_managed_charging_site` (`commands.py`) take `tesla_protocol` message types directly for their deeply-nested arguments rather than a parallel flattened dataclass API.
154154
- **Energy-gateway authorized-client pairing has security- and protocol-specific constraints**: use RSA for LAN TEDapi v1r, treat `PENDING_VERIFICATION_TIMEOUT` as terminal, and account for presence-free key removal. The authoritative pairing, retry, encoding, and removal guidance is in `docs/energy_local_control.md`; enum values and API contracts live in `const.py` and the relevant method docstrings.
155155
- **`register_client()` (`teslemetry/teslemetry.py`) is Teslemetry-only OAuth Dynamic Client Registration (RFC 7591)**: a module-level function, not a `Teslemetry` instance method, since registration precedes having a `client_id` or access token — callers pass a bare `aiohttp.ClientSession`. It always registers a new client (no dedup/caching) and raises `TeslemetryRegistrationError` (`exceptions.py`) on transport failure, a non-2xx response, a non-JSON body, or a response missing a usable `client_id`; a non-dict-but-valid-JSON body (list/scalar) is treated as the same malformed-response error rather than raising an uncaught `AttributeError`. Fleet API and Tessie have no equivalent — don't add one speculatively. See `docs/teslemetry.md`'s "OAuth Dynamic Client Registration" section and `tests/test_teslemetry_register_client.py`.
156-
- **`Commands.__init__`'s `private_key` (and `VehicleBluetooth`/`vehicles.create*`'s `key`) distinguishes omitted from explicit `None` via a sentinel default (`KEY_OMITTED`/`KeyOmitted` in `commands.py`)**: omitting the argument keeps falling back to the parent's key, raising `ValueError("No private key.")` if it has none; passing `None` explicitly disables signing for a passive BLE listener that only observes broadcasts. `self.private_key` is therefore `EllipticCurvePrivateKey | None` — `_handshake` (reached by `_command`, i.e. every signed command, and by `_ensure_handshake`, used by signed reads) raises `SigningDisabled` (`exceptions.py`) up front rather than failing deep in the signing/crypto path. `pair()`'s fast path never calls `_handshake` (it builds and sends its own whitelist request directly), so it carries its own identical guard at the top instead — `_handshake` is not a single choke point every signed-session entry point routes through; each entry point that doesn't call it needs its own `self.private_key is None` check. Tests: `tests/test_ble_null_key.py`.
156+
- **`False`, not `None`, is the "signing is disabled" value for `Commands.__init__`'s `private_key` (and `VehicleBluetooth.__init__`/`Vehicles.createBluetooth`/`VehiclesBluetooth.create`/`createBluetooth`'s `key`)**: `None` — the default and an explicit `None` — keeps its long-standing meaning of falling back to the parent's key, raising `ValueError("No private key.")` if it has none; `False` disables signing for a passive BLE listener that only observes broadcasts. `None` is deliberately *not* the opt-out: a caller already passing `private_key=None` to mean "I haven't got one" must keep getting that `ValueError`, not a silently unsignable vehicle. Because `False` and `None` are both falsy, every branch on this argument must test **identity** (`is False`/`is not None`) — a truthiness check (`if private_key:`) collapses the two states and reintroduces the bug. `self.private_key` is `EllipticCurvePrivateKey | None`, its `None` meaning signing-disabled — `_handshake` (reached by `_command`, i.e. every signed command, and by `_ensure_handshake`, used by signed reads) raises `SigningDisabled` (`exceptions.py`) up front rather than failing deep in the signing/crypto path. `pair()`'s fast path never calls `_handshake` (it builds and sends its own whitelist request directly), so it carries its own identical guard at the top instead — `_handshake` is not a single choke point every signed-session entry point routes through; each entry point that doesn't call it needs its own `self.private_key is None` check. Tests: `tests/test_ble_null_key.py`.
157157

158158
## Maintaining this file
159159

docs/bluetooth_vehicles.md

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -577,18 +577,22 @@ message routing; `KeyboardInterrupt` and `SystemExit` still propagate.
577577
### Passive listening without a private key
578578

579579
A vehicle that only decodes broadcasts and never sends a command has no
580-
reason to hold a signing key. Pass `key=None` explicitly to `vehicles.create`
581-
(or `VehicleBluetooth` directly) to construct without one:
580+
reason to hold a signing key. Pass `key=False` to `vehicles.create` (or
581+
`VehicleBluetooth` directly) to construct without one:
582582

583583
```python
584-
vehicle = tesla_bluetooth.vehicles.create("<vin>", key=None)
584+
vehicle = tesla_bluetooth.vehicles.create("<vin>", key=False)
585585
```
586586

587-
Omitting `key` still falls back to the parent's private key as before -
588-
`key=None` must be passed explicitly to disable signing. Reads and listeners
589-
that don't need a signed session still work; any command that does raises
590-
`SigningDisabled` naming that signing was explicitly disabled for this
591-
vehicle.
587+
`key=None` - the default, and an explicit `None` - still falls back to the
588+
parent's private key exactly as before, raising `ValueError("No private
589+
key.")` if the parent has none. Only `False` disables signing, so a caller
590+
already passing `key=None` to mean "I haven't got one" keeps getting that
591+
error rather than silently ending up with a vehicle that cannot sign.
592+
593+
Reads and listeners that don't need a signed session still work; any command
594+
that does raises `SigningDisabled` naming that signing was explicitly disabled
595+
for this vehicle.
592596

593597
### Connection-status events
594598

tesla_fleet_api/exceptions.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -411,14 +411,14 @@ class LibraryError(Exception):
411411
class SigningDisabled(LibraryError):
412412
"""A signed operation was attempted on a vehicle constructed with signing explicitly disabled.
413413
414-
Pass ``private_key=None`` explicitly only for a passive listener that
415-
never sends a command; construct with a real key (or omit the argument to
414+
Pass ``private_key=False`` only for a passive listener that never sends a
415+
command; construct with a real key (or leave the argument at ``None`` to
416416
inherit the parent's) to issue signed commands.
417417
"""
418418

419419
def __init__(self) -> None:
420420
super().__init__(
421-
"This vehicle was constructed with private_key=None, explicitly "
421+
"This vehicle was constructed with private_key=False, explicitly "
422422
"disabling command signing. It can only observe unsolicited "
423423
"broadcasts (the listen_* methods); any signed command or read "
424424
"needs a real private_key."

tesla_fleet_api/tesla/vehicle/bluetooth.py

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
import warnings
88
from collections import deque
99
from random import randbytes
10-
from typing import TYPE_CHECKING, Any, Callable, Generic, TypeVar
10+
from typing import TYPE_CHECKING, Any, Callable, Generic, Literal, TypeVar
1111

1212
import bleak
1313
from bleak.backends.characteristic import BleakGATTCharacteristic
@@ -30,9 +30,7 @@
3030
)
3131
from tesla_fleet_api.tesla.vehicle.broadcast import BroadcastListeners, Unsubscribe
3232
from tesla_fleet_api.tesla.vehicle.commands import (
33-
KEY_OMITTED,
3433
Commands,
35-
KeyOmitted,
3634
infotainment_command_name,
3735
vcsec_command_name,
3836
)
@@ -515,7 +513,7 @@ def __init__(
515513
self,
516514
parent: BluetoothParentT,
517515
vin: str,
518-
key: ec.EllipticCurvePrivateKey | None | KeyOmitted = KEY_OMITTED,
516+
key: ec.EllipticCurvePrivateKey | Literal[False] | None = None,
519517
device: BLEDevice | None = None,
520518
confirmation: BluetoothConfirmation | bool = "ack",
521519
keepalive_interval: float | None = DEFAULT_KEEPALIVE_INTERVAL,
@@ -533,10 +531,10 @@ def __init__(
533531
``optimistic`` wins over a ``True`` ``verify_commands`` if both are
534532
somehow passed, matching the old dominance order).
535533
536-
Passing ``key=None`` explicitly disables command signing, for a
534+
Passing ``key=False`` explicitly disables command signing, for a
537535
passive listener that only observes broadcasts via the ``listen_*``
538-
methods and never sends a command. Omitting ``key`` keeps the usual
539-
fallback to the parent's key.
536+
methods and never sends a command. ``key=None`` (the default, and an
537+
explicit ``None``) keeps the usual fallback to the parent's key.
540538
"""
541539
super().__init__(parent, vin, key)
542540
if isinstance(confirmation, bool):

tesla_fleet_api/tesla/vehicle/commands.py

Lines changed: 20 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -426,15 +426,6 @@ def aes_gcm_personalized(self) -> AES_GCM_Personalized_Signature_Data:
426426
)
427427

428428

429-
class KeyOmitted:
430-
"""Sentinel distinguishing an omitted ``private_key`` argument from an explicit ``None``."""
431-
432-
433-
# Distinct from ``None`` so a caller can explicitly pass ``private_key=None`` to
434-
# disable signing, rather than that meaning "use the parent's key".
435-
KEY_OMITTED = KeyOmitted()
436-
437-
438429
class Commands(ABC, Vehicle[CommandParentT], Generic[CommandParentT]):
439430
"""Class describing the Tesla Fleet API vehicle endpoints and commands for a specific vehicle with command signing."""
440431

@@ -450,15 +441,18 @@ def __init__(
450441
self,
451442
parent: CommandParentT,
452443
vin: str,
453-
private_key: ec.EllipticCurvePrivateKey | None | KeyOmitted = KEY_OMITTED,
444+
private_key: ec.EllipticCurvePrivateKey | Literal[False] | None = None,
454445
public_key: bytes | None = None,
455446
):
456-
"""Initialize with a signing key, or ``private_key=None`` to disable signing.
457-
458-
Omitting ``private_key`` falls back to the parent's key (raising if it
459-
has none, same as always). Passing ``private_key=None`` explicitly
460-
disables signing for this vehicle - for a passive BLE listener that
461-
only observes broadcasts and never sends a command.
447+
"""Initialize with a signing key, or ``private_key=False`` to disable signing.
448+
449+
``None`` (the default, and an explicit ``None``) keeps the long-standing
450+
behaviour: fall back to the parent's key, raising ``ValueError`` if it
451+
has none. Passing ``private_key=False`` explicitly disables signing for
452+
this vehicle - for a passive BLE listener that only observes broadcasts
453+
and never sends a command. ``False`` is used rather than ``None`` so
454+
that no caller who already passes ``private_key=None`` meaning "I
455+
haven't got one" silently gets a vehicle that cannot sign.
462456
"""
463457
super().__init__(parent, vin)
464458

@@ -470,13 +464,17 @@ def __init__(
470464
Domain.DOMAIN_INFOTAINMENT: Session(self, Domain.DOMAIN_INFOTAINMENT),
471465
}
472466

473-
if isinstance(private_key, KeyOmitted):
474-
if parent.private_key:
475-
self.private_key = parent.private_key
476-
else:
477-
raise ValueError("No private key.")
478-
else:
467+
# Identity checks, not truthiness: ``False`` and ``None`` are both
468+
# falsy, and collapsing them would make an explicit ``None`` silently
469+
# disable signing instead of falling back to the parent's key.
470+
if private_key is False:
471+
self.private_key = None
472+
elif private_key is not None:
479473
self.private_key = private_key
474+
elif parent.private_key is not None:
475+
self.private_key = parent.private_key
476+
else:
477+
raise ValueError("No private key.")
480478

481479
self._public_key = public_key or (
482480
self.private_key.public_key().public_bytes(

tesla_fleet_api/tesla/vehicle/vehicles.py

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,10 @@
11
from __future__ import annotations
2-
from typing import TYPE_CHECKING, Any, Generic, TypeVar
2+
from typing import TYPE_CHECKING, Any, Generic, Literal, TypeVar
33

44
from bleak.backends.device import BLEDevice
55
from cryptography.hazmat.primitives.asymmetric import ec
66

77
from tesla_fleet_api.const import BluetoothConfirmation
8-
from tesla_fleet_api.tesla.vehicle.commands import KEY_OMITTED, KeyOmitted
98
from tesla_fleet_api.tesla.vehicle.signed import VehicleSigned
109
from tesla_fleet_api.tesla.vehicle.bluetooth import (
1110
DEFAULT_KEEPALIVE_INTERVAL,
@@ -54,6 +53,7 @@ def createBluetooth(
5453
raise_unconfirmed: bool = False,
5554
*,
5655
verify_commands: bool | None = None,
56+
key: ec.EllipticCurvePrivateKey | Literal[False] | None = None,
5757
) -> VehicleBluetooth[FleetParentT]:
5858
"""Creates a bluetooth vehicle that uses command protocol.
5959
@@ -68,10 +68,13 @@ def createBluetooth(
6868
success. ``verify_commands``/``optimistic`` are deprecated aliases for
6969
``confirmation="verify"``/``confirmation="optimistic"``. See
7070
``VehicleBluetooth``'s docstring for the full ladder.
71+
``key=False`` explicitly disables signing, for a passive listener;
72+
``key=None`` (the default) keeps the usual parent-key fallback.
7173
"""
7274
vehicle = self.Bluetooth(
7375
self._parent,
7476
vin,
77+
key,
7578
confirmation=confirmation,
7679
keepalive_interval=keepalive_interval,
7780
optimistic=optimistic,
@@ -102,7 +105,7 @@ def __init__(self, parent: BluetoothClientT):
102105
def create(
103106
self,
104107
vin: str,
105-
key: ec.EllipticCurvePrivateKey | None | KeyOmitted = KEY_OMITTED,
108+
key: ec.EllipticCurvePrivateKey | Literal[False] | None = None,
106109
device: BLEDevice | None = None,
107110
confirmation: BluetoothConfirmation | bool = "ack",
108111
keepalive_interval: float | None = DEFAULT_KEEPALIVE_INTERVAL,
@@ -124,8 +127,8 @@ def create(
124127
success. ``verify_commands``/``optimistic`` are deprecated aliases for
125128
``confirmation="verify"``/``confirmation="optimistic"``. See
126129
``VehicleBluetooth``'s docstring for the full ladder.
127-
``key=None`` explicitly disables signing, for a passive listener;
128-
omitting ``key`` keeps the usual parent-key fallback.
130+
``key=False`` explicitly disables signing, for a passive listener;
131+
``key=None`` (the default) keeps the usual parent-key fallback.
129132
"""
130133
return self.createBluetooth(
131134
vin,
@@ -141,7 +144,7 @@ def create(
141144
def createBluetooth(
142145
self,
143146
vin: str,
144-
key: ec.EllipticCurvePrivateKey | None | KeyOmitted = KEY_OMITTED,
147+
key: ec.EllipticCurvePrivateKey | Literal[False] | None = None,
145148
device: BLEDevice | None = None,
146149
confirmation: BluetoothConfirmation | bool = "ack",
147150
keepalive_interval: float | None = DEFAULT_KEEPALIVE_INTERVAL,
@@ -163,8 +166,8 @@ def createBluetooth(
163166
success. ``verify_commands``/``optimistic`` are deprecated aliases for
164167
``confirmation="verify"``/``confirmation="optimistic"``. See
165168
``VehicleBluetooth``'s docstring for the full ladder.
166-
``key=None`` explicitly disables signing, for a passive listener;
167-
omitting ``key`` keeps the usual parent-key fallback.
169+
``key=False`` explicitly disables signing, for a passive listener;
170+
``key=None`` (the default) keeps the usual parent-key fallback.
168171
"""
169172
vehicle = self.Bluetooth(
170173
self._parent,

tesla_fleet_api/teslemetry/vehicle.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
from __future__ import annotations
22

3-
from typing import TYPE_CHECKING, Any
3+
from typing import TYPE_CHECKING, Any, Literal
4+
5+
from cryptography.hazmat.primitives.asymmetric import ec
46

57
from tesla_fleet_api.const import (
68
BluetoothConfirmation,
@@ -673,6 +675,7 @@ def createBluetooth(
673675
raise_unconfirmed: bool = False,
674676
*,
675677
verify_commands: bool | None = None,
678+
key: ec.EllipticCurvePrivateKey | Literal[False] | None = None,
676679
) -> Any:
677680
"""Not supported; parameters match the Fleet API Bluetooth factory."""
678681
raise NotImplementedError("Teslemetry cannot use local Bluetooth")

tesla_fleet_api/tessie/vehicle.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
from __future__ import annotations
2-
from typing import TYPE_CHECKING, Any
2+
from typing import TYPE_CHECKING, Any, Literal
3+
4+
from cryptography.hazmat.primitives.asymmetric import ec
35

46
from tesla_fleet_api.const import BluetoothConfirmation, Method
57
from tesla_fleet_api.tesla.vehicle.vehicles import Vehicles
@@ -1222,6 +1224,7 @@ def createBluetooth(
12221224
raise_unconfirmed: bool = False,
12231225
*,
12241226
verify_commands: bool | None = None,
1227+
key: ec.EllipticCurvePrivateKey | Literal[False] | None = None,
12251228
) -> Any:
12261229
"""Not supported; parameters match the Fleet API Bluetooth factory."""
12271230
raise NotImplementedError("Tessie cannot use local Bluetooth")

0 commit comments

Comments
 (0)