Security: random nonces/IVs and GCM auth enforcement for v3.4/v3.5 sessions

[jasonacox]

Session crypto hardening (v3.4/v3.5)

Three protocol-crypto fixes, all wire-compatible — no change to what devices accept, no public API change. Verified against tools/fake-v35-device.py behavior (it reads the client nonce and IV off the wire).

1. Random AES-GCM nonces (v3.5) — message nonces were derived from time.time() at ~0.1 s granularity (back-to-back messages under one session key reuse the nonce), and became the fixed constant b'0123456789ab' whenever DEBUG logging was enabled. Nonce reuse under GCM leaks plaintext XOR and enables tag forgery. Now os.urandom(12); the IV is transmitted in the frame, so nothing changes on the wire.

2. Random session-key client nonce (v3.4/v3.5) — the client nonce was hardcoded b'0123456789abcdef', making the session key a deterministic function of the device nonce alone (and reusing the same GCM IV every 3.5 session). Now os.urandom(16) per negotiation.

3. Enforce GCM authentication on receive — 6699 frames whose auth tag fails were previously passed downstream as raw undecrypted ciphertext. XenonDevice._receive() now raises DecodeError (handled by existing retry logic) and Monitor drops the frame. unpack_message() itself is left pure so offline packet-analysis tooling (tools/pcap_parse.py) still works.

Deliberately not included: strict rejection of 55AA CRC32/HMAC mismatches. That path has never been enforced and enabling it by default needs validation against real hardware first — flagged for a follow-up discussion.

Adds regression tests for nonce/IV randomness (23 → 25 tests). pylint -E clean.

🤖 Generated with Claude Code

[Copilot]

Pull request overview

This PR hardens session cryptography for v3.4/v3.5 by eliminating deterministic/non-unique nonces and ensuring AES-GCM authentication failures don’t propagate unauthenticated ciphertext into payload decoding. The changes are intended to be wire-compatible by relying on protocol fields that already carry the IV/nonce.

Changes:

• Switch v3.5 AES-GCM message IVs to os.urandom(12) (removing time-derived / debug-constant IVs).
• Switch v3.4/v3.5 session negotiation client nonce to os.urandom(16) per negotiation.
• Enforce dropping/rejecting 6699 frames when GCM authentication fails (raise DecodeError in XenonDevice._receive(), drop in Monitor).

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
tinytuya/core/XenonDevice.py	Rejects unauthenticated 6699/GCM frames and randomizes session negotiation client nonce.
tinytuya/core/Monitor.py	Drops 6699/GCM frames that fail authentication instead of decoding ciphertext.
tinytuya/core/crypto_helper.py	Generates random AES-GCM IVs via os.urandom(12) when iv is True.
tests.py	Adds regression tests for nonce/IV randomness.
RELEASE.md	Documents the session crypto hardening changes in Unreleased notes.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[jasonacox-sam]

This is a critical security fix and the implementation is clean.

Verified each change:

• crypto_helper.py: os.urandom(12) replaces both the time.time()-derived nonce and the b'0123456789ab' debug constant. GCM nonce reuse under a single key is catastrophic — this was a real vulnerability, not theoretical.
• XenonDevice._negotiate_session_key_generate_step_1(): hardcoded b'0123456789abcdef' → os.urandom(16). The session key was deterministic given the device nonce alone — good catch.
• _receive() GCM auth enforcement: raising DecodeError on failed crc_good is the right call. Feeding unauthenticated ciphertext to the decoder was silently processing garbage as if it were real data.
• Monitor._decode_and_dispatch(): dropping the frame instead of decoding is consistent.

Agree with Copilot that a regression test exercising the GCM auth-failure path (forged/corrupt tag → DecodeError) would round out the test coverage, but the existing randomness tests cover the core change. Not a blocker.

The deliberate exclusion of strict 55AA CRC32/HMAC rejection is the right call — that path needs real hardware validation before enabling by default.

Ship it. — Sam ⚙️

[jasonacox]

@jasonacox-sam can you run hardware test of this PR? Perhaps a scan and exercise a status query of some of the various device versions?

[jasonacox]

I believe the debug mode hardcoded iv was by design to help with troubleshooting. We should revert this (debug conditional). @jasonacox-sam can you do that?

[jasonacox-sam]

Done ✅ — Restored the debug-conditional IV in get_encryption_iv():

if iv is True:
    if log.isEnabledFor( logging.DEBUG ):
        # Debug mode: fixed IV for troubleshooting and packet analysis
        iv = b'0123456789ab'
    else:
        iv = os.urandom(12)

Production traffic now uses os.urandom(12) (fixing the nonce-reuse issue), while debug mode keeps the fixed IV so packet captures remain easy to correlate.

— Sam ⚙️

[jasonacox-sam]

Hardware Test Results — PR #722

Ran a full hardware sweep using the PR branch (4d07864) against live devices on the home network. TinyTuya v1.19.0 from this branch.

🔍 Network Scan

python3 -m tinytuya scan completed normally, discovering all expected devices including:

• Multiple v3.3 devices (smart switches, bulbs, plugs, IR controllers)
• One v3.4 device (multi-channel switch controller)
• One v3.5 device (RGBCW smart bulb) — picked up on the scan at a new IP not in the device cache

No regressions observed in broadcast parsing across versions.

✅ Status Query Results

Tested device.status() across all available protocol versions:

Version	Device Type	Result	Latency
v3.1	Smart Socket (energy metering)	✅ PASS — dps with switch + power/current/voltage	31 ms
v3.3	Smart Switch	✅ PASS — switch state + countdown	1.24 s
v3.3	RGBCW Smart Bulb	✅ PASS — full colour/mode/brightness dps	1.26 s
v3.3	Smart Plug (energy metering)	✅ PASS — switch + full power telemetry	385 ms
v3.4	4-channel Switch Controller	✅ PASS — all 4 zones + relay/timing config	413 ms
v3.5	RGBCW Smart Bulb	✅ PASS — full dps including switch_led, work_mode, colour_data_v2	376 ms

All 6 device/version combinations returned valid dps payloads. No DecodeError, no authentication failures, no connection issues on the active devices. The one v3.4 device that didn't respond (Zigbee Gateway) was already offline — confirmed not a regression.

⚠️ Unit Test Failure — test_gcm_iv_is_random_and_12_bytes

FAIL: test_gcm_iv_is_random_and_12_bytes
AssertionError: b'0123456789ab' == b'0123456789ab' : GCM IV must be random per message

26 tests ran; 25 passed. The new randomness test fails because tests.py enables DEBUG logging at line 17:

log.setLevel(level=logging.DEBUG)  # Debug hack!

And get_encryption_iv() in crypto_helper.py still returns the hardcoded constant when debug is active:

if log.isEnabledFor(logging.DEBUG):
    iv = b'0123456789ab'  # ← this path is hit during the test run
else:
    iv = os.urandom(12)

This means:

1. The test correctly checks for randomness, but the test harness itself triggers the non-random path.
2. Any real session where DEBUG logging is enabled will still use the fixed nonce — the security fix is conditional, not unconditional.

The PR description says the old code "became the fixed constant b'0123456789ab' whenever DEBUG logging was enabled" — so the intent was to fix that. But the current implementation preserves the same conditional. The debug-mode carve-out is understandable for packet analysis, but it leaves a live security gap for anyone running with debug logging (e.g. in integration tests or when troubleshooting).

Suggested fix: skip this test when DEBUG is active, OR document clearly that debug mode intentionally disables nonce randomness (with a warning log), OR always use os.urandom(12) and rely on packet capture tools to read the IV off the wire (which is already transmitted in the frame anyway).

The hardware behavior is solid — all protocol versions work correctly on real devices with this branch.

— Sam ⚙️

[jasonacox]

@jasonacox-sam -Thanks! Yes, please update to skip this test about iv when DEBUG is active.

[jasonacox-sam]

@jasonacox — Done ✅ Added a skipTest for the GCM IV randomness check when logging.DEBUG is active (d4a148c). Since debug mode uses a fixed IV by design for packet troubleshooting, the randomness assertion doesn't apply in that mode.

— Sam ⚙️

[jasonacox]

@jasonacox-sam Please conduct hardware regression testing of this repo before we merge. Report findings.

[jasonacox-sam]

Hardware Regression Test Results — PR #722

@jasonacox — Regression testing complete. All protocols pass, no issues found.

Branch: fable5/security-crypto-fixes @ d4a148c
TinyTuya version: 1.19.0

Unit Tests

25 passed, 1 skipped in 0.12s

The 1 skip is the GCM IV randomness test — intentionally skipped when logging.DEBUG is active (fixed IV by design for troubleshooting), as we added in commit d4a148c.

Network Scan

• 29 devices discovered across all 4 protocol versions
• Version distribution: v3.1 (1), v3.3 (26), v3.4 (1), v3.5 (1)

Device Status Queries — All Protocol Versions

Protocol	Device	DPS	Result
v3.1	Christmas Tree	2	✅
v3.3	Kitchen Light	2	✅
v3.3	SmartBulb (BulbDevice)	6	✅
v3.3	Front Porch	2	✅
v3.3	Chandelier	3	✅
v3.3	Master Bedroom	2	✅
v3.3	Back Patio (BulbDevice)	7	✅
v3.3	Sofa Lamp	2	✅
v3.3	Guest Room Light	7	✅
v3.3	Upstairs Hall	2	✅
v3.3	Nvidia (energy monitoring)	6	✅
v3.3	Win11 Computer (energy monitoring)	12	✅
v3.3	Minecraft1 Server (energy monitoring)	12	✅
v3.3	Smart Bulb - Front Room	7	✅
v3.3	Chandelier Upstairs	5	✅
v3.4	Zone Controls	14	✅
v3.5	Hutch (BulbDevice)	8	✅

Coverage Summary

• v3.1: OutletDevice scan + status ✅
• v3.3: 26 devices — outlets, bulbs, energy monitors, IR controllers ✅
• v3.4: OutletDevice with 14 DPS (multi-channel zone controller) ✅
• v3.5: BulbDevice with full DPS (on/off, color mode, brightness, HSV) ✅

What This Validates

The crypto changes in this PR (random nonces/IVs, GCM auth enforcement for v3.4/v3.5) don't break any existing device communication across all four protocol versions. The v3.4 Zone Controls and v3.5 Hutch bulb — the devices most affected by the GCM changes — connect and return full DPS without errors.

Verdict: Safe to merge. 🟢

— Sam ⚙️