Skip to content

Fix Victron CTR nonce length regression (#316) - #317

Merged
DigiH merged 1 commit into
developmentfrom
fix/victron-ctr-nonce-length
Jun 7, 2026
Merged

Fix Victron CTR nonce length regression (#316)#317
DigiH merged 1 commit into
developmentfrom
fix/victron-ctr-nonce-length

Conversation

@1technophile

Copy link
Copy Markdown
Member

Summary

Fixes #316.

  • Reverts VictronDecryptor.compute_nonce() back to padding the nonce to 8 bytes (was changed to 16 in Fully consistent 16 byte long nonce #308).
  • Adds a short comment explaining why 8 — and not 16 — is correct, so this regression doesn't get re-introduced under another "consistency" pass.

Why the 16-byte padding broke decryption

pycryptodome's AES.new(..., MODE_CTR, nonce=...) requires the nonce length to be strictly less than the 16-byte AES block size, because the remaining bytes are reserved for the internal counter that CTR mode increments per block. Padding to a full 16 bytes leaves no room for the counter and pycryptodome rejects it with ValueError: Nonce is too long, which is exactly the traceback the issue reporter captured:

File ".../TheengsGateway/decryption.py", line 156, in decrypt
    cipher = AES.new(bindkey, AES.MODE_CTR, nonce=nonce)
ValueError: Nonce is too long

The "consistency with OMG decryption" rationale in #308 does not transfer: the OMG decryptors in this same file use AES.MODE_CCM (lines 70 and 115), which has different nonce-length rules from CTR. CCM and CTR are not API-compatible on this parameter.

Net effect of the regression: every Victron encrypted advertisement has silently failed to decrypt since 1.7.0 (MQTT messages contain "model": "Victron encrypted" with no decoded fields). Confirmed by the reporter on both Pi 5 and Pi 3B against a Victron Blue Smart IP65 12/25 charger; reverting to 8-byte padding restores all expected fields (volt_batt_1, current_batt_1, device_state, error_code).

Test plan

The gateway has no Python test suite to add a regression test to, so verification is manual:

  • Build and install this branch on a host with a Victron Blue Smart device in range.
  • Confirm decoded fields appear in MQTT (volt_batt_1, device_state, etc.) instead of just "model": "Victron encrypted".
  • Confirm no Decryption failed / Nonce is too long errors in the log.

Standing up Python test infrastructure for the decryption module is worth doing in a follow-up so this can't silently regress again, but is out of scope for this one-line revert.

Reverts the 16-byte padding introduced in #308 back to 8 bytes.

pycryptodome's AES.MODE_CTR requires the nonce passed via the nonce=
parameter to be shorter than the 16-byte block size, because the
remaining bytes are reserved for the counter. A 16-byte nonce leaves
no room for the counter and pycryptodome rejects it with
"Nonce is too long", so every Victron encrypted advertisement has
failed to decrypt since 1.7.0.

The "consistency with OMG decryption" rationale of #308 does not apply:
the OMG decryptors use AES.MODE_CCM, which has different nonce-length
rules from CTR.

@cnause cnause left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Works as expected with my Victron charger

@1technophile

Copy link
Copy Markdown
Member Author

@DigiH should we merge this?

@DigiH
DigiH merged commit 4d58ca8 into development Jun 7, 2026
6 checks passed
@1technophile
1technophile deleted the fix/victron-ctr-nonce-length branch June 7, 2026 22:18
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.

Regression in TheengsGateway 1.7.0 that breaks Victron BLE advertisement decryption.

3 participants