← Watch feed

Moderate 59 Bitcoin Committed May 14, 2026 · 09:53 UTC

chore(core): add consteq check directly into chachapoly.finish when decrypting

Trezor/Trezor firmware

f4fa0d1d View source ↗

Public commit record

What the developer wrote

Authored by M1nd3r

62/100 · Adequate

chore(core): add consteq check directly into chachapoly.finish when decrypting

[no changelog]

✓ Specific, descriptive subject✓ Names a concrete action or component✓ Uses a recognizable type or scope! No meaningful explanatory body

The short version

What changed, and why it matters

This commit hardens how a Trezor hardware wallet checks the authentication tag when decrypting data protected by the ChaCha20-Poly1305 cipher. Previously, the Python caller compared the expected tag with the computed tag using a helper called utils.consteq. Now the comparison is done inside the low-level C module using a constant-time equality function, and decryption callers must pass the expected tag directly to finish(). This reduces the risk of a developer forgetting to verify the tag or using a non-constant-time comparison, which could let an attacker tamper with encrypted secrets.

Recommended action

Review all other Python callers of chacha20poly1305.finish() to ensure they now supply the expected MAC during decryption and no longer rely on external tag comparisons. Verify that consteq.h provides a truly constant-time implementation on the target architecture. Consider adding unit tests for missing/invalid MAC and for state-machine misuse.

Security signals we found

Constant-time MAC comparison added inside cryptographic module

Mandatory expected MAC argument when decrypting

State machine added to ChaCha20Poly1305 object to prevent misuse

Removal of external Python-side tag comparison in WebAuthn credential decryption

Potential prior vulnerability: missing or incorrect tag verification could allow ciphertext forgery

Technical analysis

Evidence from the diff

The patch modifies modtrezorcrypto-chacha20poly1305.h to track ChaCha20Poly1305 object state (INIT/ENCRYPTING/DECRYPTING/FINISHED) and to require an expected_mac argument to finish() when the object is in DECRYPTING state. It performs a consteq constant-time comparison between the internally computed Poly1305 tag and the supplied expected tag, raising RuntimeError on mismatch. The Python mock type stub is updated accordingly, and the WebAuthn credential.py caller is changed from ctx.finish() followed by utils.consteq() to ctx.finish(tag) with exception handling. The change is defensive: it moves the authentication check into the cryptographic boundary and makes it mandatory.

Changed components

core/embed/upymod/modtrezorcrypto/modtrezorcrypto-chacha20poly1305.hcore/mocks/generated/trezorcrypto/__init__.pyicore/src/apps/webauthn/credential.py

Inspect captured patch +57 / −9

diff --git a/core/embed/upymod/modtrezorcrypto/modtrezorcrypto-chacha20poly1305.h b/core/embed/upymod/modtrezorcrypto/modtrezorcrypto-chacha20poly1305.h
index 090accd8..ddb0e91a 100644
--- a/core/embed/upymod/modtrezorcrypto/modtrezorcrypto-chacha20poly1305.h
+++ b/core/embed/upymod/modtrezorcrypto/modtrezorcrypto-chacha20poly1305.h
@@ -20,6 +20,7 @@
 #include "py/objstr.h"
 
 #include "chacha20poly1305/rfc7539.h"
+#include "consteq.h"
 #include "memzero.h"
 
 /// package: trezorcrypto.__init__
@@ -32,6 +33,12 @@ typedef struct _mp_obj_ChaCha20Poly1305_t {
   mp_obj_base_t base;
   chacha20poly1305_ctx ctx;
   int64_t alen, plen;
+  enum {
+    INIT,
+    ENCRYPTING,
+    DECRYPTING,
+    FINISHED,
+  } state;
 } mp_obj_ChaCha20Poly1305_t;
 
 /// def __init__(self, key: AnyBytes, nonce: AnyBytes) -> None:
@@ -58,6 +65,7 @@ STATIC mp_obj_t mod_trezorcrypto_ChaCha20Poly1305_make_new(
   o->base.type = type;
   o->alen = 0;
   o->plen = 0;
+  o->state = INIT;
   return MP_OBJ_FROM_PTR(o);
 }
 
@@ -69,6 +77,10 @@ STATIC mp_obj_t mod_trezorcrypto_ChaCha20Poly1305_make_new(
 STATIC mp_obj_t mod_trezorcrypto_ChaCha20Poly1305_encrypt(mp_obj_t self,
                                                           mp_obj_t data) {
   mp_obj_ChaCha20Poly1305_t *o = MP_OBJ_TO_PTR(self);
+  if (o->state != INIT && o->state != ENCRYPTING) {
+    mp_raise_msg(&mp_type_RuntimeError, MP_ERROR_TEXT("Invalid state."));
+  }
+  o->state = ENCRYPTING;
   mp_buffer_info_t in = {0};
   mp_get_buffer_raise(data, &in, MP_BUFFER_READ);
   vstr_t vstr = {0};
@@ -88,6 +100,10 @@ STATIC MP_DEFINE_CONST_FUN_OBJ_2(mod_trezorcrypto_ChaCha20Poly1305_encrypt_obj,
 STATIC mp_obj_t mod_trezorcrypto_ChaCha20Poly1305_decrypt(mp_obj_t self,
                                                           mp_obj_t data) {
   mp_obj_ChaCha20Poly1305_t *o = MP_OBJ_TO_PTR(self);
+  if (o->state != INIT && o->state != DECRYPTING) {
+    mp_raise_msg(&mp_type_RuntimeError, MP_ERROR_TEXT("Invalid state."));
+  }
+  o->state = DECRYPTING;
   mp_buffer_info_t in = {0};
   mp_get_buffer_raise(data, &in, MP_BUFFER_READ);
   vstr_t vstr = {0};
@@ -108,6 +124,9 @@ STATIC MP_DEFINE_CONST_FUN_OBJ_2(mod_trezorcrypto_ChaCha20Poly1305_decrypt_obj,
 STATIC mp_obj_t mod_trezorcrypto_ChaCha20Poly1305_auth(mp_obj_t self,
                                                        mp_obj_t data) {
   mp_obj_ChaCha20Poly1305_t *o = MP_OBJ_TO_PTR(self);
+  if (o->state != INIT && o->state != ENCRYPTING && o->state != DECRYPTING) {
+    mp_raise_msg(&mp_type_RuntimeError, MP_ERROR_TEXT("Invalid state."));
+  }
   mp_buffer_info_t in = {0};
   mp_get_buffer_raise(data, &in, MP_BUFFER_READ);
   rfc7539_auth(&(o->ctx), in.buf, in.len);
@@ -117,19 +136,45 @@ STATIC mp_obj_t mod_trezorcrypto_ChaCha20Poly1305_auth(mp_obj_t self,
 STATIC MP_DEFINE_CONST_FUN_OBJ_2(mod_trezorcrypto_ChaCha20Poly1305_auth_obj,
                                  mod_trezorcrypto_ChaCha20Poly1305_auth);
 
-/// def finish(self) -> bytes:
+/// def finish(self, expected_mac: AnyBytes | None = None) -> bytes:
 ///     """
-///     Compute RFC 7539-style Poly1305 MAC.
+///     Compute RFC 7539-style Poly1305 MAC. The `expected_mac` is required when
+///     decrypting.
 ///     """
-STATIC mp_obj_t mod_trezorcrypto_ChaCha20Poly1305_finish(mp_obj_t self) {
-  mp_obj_ChaCha20Poly1305_t *o = MP_OBJ_TO_PTR(self);
+STATIC mp_obj_t mod_trezorcrypto_ChaCha20Poly1305_finish(size_t n_args,
+                                                         const mp_obj_t *args) {
+  mp_obj_ChaCha20Poly1305_t *o = MP_OBJ_TO_PTR(args[0]);
+  if (o->state != INIT && o->state != ENCRYPTING && o->state != DECRYPTING) {
+    mp_raise_msg(&mp_type_RuntimeError, MP_ERROR_TEXT("Invalid state."));
+  }
+  if (n_args == 1 && o->state == DECRYPTING) {
+    mp_raise_msg(
+        &mp_type_RuntimeError,
+        MP_ERROR_TEXT("Argument `expected_mac` is required when decrypting."));
+  }
+
+  o->state = FINISHED;
   vstr_t mac = {0};
   vstr_init_len(&mac, 16);
   rfc7539_finish(&(o->ctx), o->alen, o->plen, (uint8_t *)mac.buf);
+  if (n_args == 2) {
+    mp_buffer_info_t expected_mac = {0};
+    mp_get_buffer_raise(args[1], &expected_mac, MP_BUFFER_READ);
+    if (expected_mac.len != 16) {
+      mp_raise_ValueError(MP_ERROR_TEXT(
+          "Invalid length of the expected mac. It has to be 16 bytes."));
+    }
+    if (!consteq((uint8_t *)mac.buf, mac.len, (uint8_t *)expected_mac.buf,
+                 expected_mac.len)) {
+      mp_raise_msg(&mp_type_RuntimeError,
+                   MP_ERROR_TEXT("Authentication failed."));
+    }
+  }
   return mp_obj_new_str_from_vstr(&mp_type_bytes, &mac);
 }
-STATIC MP_DEFINE_CONST_FUN_OBJ_1(mod_trezorcrypto_ChaCha20Poly1305_finish_obj,
-                                 mod_trezorcrypto_ChaCha20Poly1305_finish);
+STATIC MP_DEFINE_CONST_FUN_OBJ_VAR_BETWEEN(
+    mod_trezorcrypto_ChaCha20Poly1305_finish_obj, 1, 2,
+    mod_trezorcrypto_ChaCha20Poly1305_finish);
 
 STATIC mp_obj_t mod_trezorcrypto_ChaCha20Poly1305___del__(mp_obj_t self) {
   mp_obj_ChaCha20Poly1305_t *o = MP_OBJ_TO_PTR(self);
diff --git a/core/mocks/generated/trezorcrypto/__init__.pyi b/core/mocks/generated/trezorcrypto/__init__.pyi
index a3ebdc2c..7afe91e1 100644
--- a/core/mocks/generated/trezorcrypto/__init__.pyi
+++ b/core/mocks/generated/trezorcrypto/__init__.pyi
@@ -199,9 +199,10 @@ class chacha20poly1305:
         to encryption or decryption.
         """
 
-    def finish(self) -> bytes:
+    def finish(self, expected_mac: AnyBytes | None = None) -> bytes:
         """
-        Compute RFC 7539-style Poly1305 MAC.
+        Compute RFC 7539-style Poly1305 MAC. The `expected_mac` is required when
+        decrypting.
         """
 
 
diff --git a/core/src/apps/webauthn/credential.py b/core/src/apps/webauthn/credential.py
index 5dc941a0..720d5120 100644
--- a/core/src/apps/webauthn/credential.py
+++ b/core/src/apps/webauthn/credential.py
@@ -204,7 +204,9 @@ class Fido2Credential(Credential):
         ctx = chacha20poly1305(key, iv)
         ctx.auth(rp_id_hash)
         data = ctx.decrypt(ciphertext)
-        if not utils.consteq(ctx.finish(), tag):
+        try:
+            ctx.finish(tag)
+        except RuntimeError:
             raise ValueError  # inauthentic ciphertext
 
         try:

Risk score

Why this scored 59/100

Our methodology →

Potential impact 18/30

Exploitability 12/25

Stealth signal 8/15

Affected reach 10/15

Confidence 7/10

Evidence quality 4/5

Human-validated context

Community notes

Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.

No validated notes yet.

The AI analysis stands alone for now. Submit a note if you can add evidence or important context.

What the developer wrote

What changed, and why it matters

Security signals we found

Evidence from the diff

Changed components

Why this scored 59/100

Community notes

Related watches

chore(core): remove sha1 from regular FW

refactor(crypto): pass static public key to `noise_xxpsk3_*_init()`

refactor(crypto): change order of parameters