Closed str4d closed 9 months ago
However, CTAP1 (which is what U2F has been relabeled to) also calls it with a
pinUvAuthToken
that can be either 16 or 32 bytes; the 16-byte case will trigger the non-determinism bug.
Note that a 16-byte pinUvAuthToken
can still be used with the current API (as can any key with length shorter than 32 bytes), but the caller needs to be aware that they need to pad with trailing zero bytes. An API that was aware of the caller's key length would be nicer.
Yes, this version of HMAC is used by U2F app only. Our U2F implementation uses fixed-size 32-byte key, so there was no need in support for other key lengths. We'll remove it from public API on the next API version update
Also we have mbedtls library with full HMAC support.
Slight issue migrating to Mbed TLS's HMAC implementation is it includes a ton of hashing functions by default, and they're all referenced by md.c
. A custom config should be created to enable a small subset of features, but where to put that and who's deciding what goes in?
fixed by migrating to mbedtls, its configuration updated to included more options by default
Describe the bug.
The
hmac_sha256_init
function exposed in the SDK is undocumented, but internally calls this helper function:https://github.com/flipperdevices/flipperzero-firmware/blob/b698126c36a6b491420b673683333102a242455d/lib/toolbox/hmac_sha256.c#L49-L55
The internal documentation and code make clear that this is assuming a key length of 32 bytes, matching the output size of SHA-256.
The reference to RFC 6979 is somewhat misleading here; the more pertinent reference is RFC 2104 which specifies HMAC, and states:
Put in RFC 2104 terms,
hmac_sha256_init
is internally assuming that the provided key is always exactlyL
bytes. However, applications may use a key of any length, andhmac_sha256_init
only takes a pointer to the key bytes, not an array indicating the length required for the internal implementation assumptions. This leads to two bugs:It appears that this HMAC-SHA-256 implementation was introduced as part of the U2F implementation (#879). And indeed, both CTAP1 and CTAP2 only ever call HMAC-SHA-256 with a
sharedSecret
that is exactly 32 bytes. However, CTAP1 (which is what U2F has been relabeled to) also calls it with apinUvAuthToken
that can be either 16 or 32 bytes; the 16-byte case will trigger the non-determinism bug. (In CTAP2,pinUvAuthToken
is always 32 bytes.)If the HMAC-SHA-256 implementation is intended to be exposed as a general part of the Flipper Zero SDK, then it should be fixed to be compatible with RFC 2104. If it is only intended to be used as part of the U2F implementation, then it should either be removed from the SDK API, or have its 32-byte key requirement clearly documented or enforced.
Reproduction
hmac_sha256_init
with a key that is either shorter or longer than 32 bytes.Target
No response
Logs
No response
Anything else?
No response