search

Mbed-TLS / mbedtls

An open source, portable, easy to use, readable and flexible TLS library, and reference implementation of the PSA Cryptography API. Releases are on a varying cadence, typically around 3 - 6 months between releases.
https://www.trustedfirmware.org/projects/mbed-tls/
Other
5.55k stars 2.6k forks source link

Asn1 encoding and mbedtls_asn1_get_mpi issue #2099

Open RonEld opened 6 years ago

RonEld commented 6 years ago

Description

Reported by Jongho Lee in the support forum:

Hi. I’m developer using mbedtls. I found asn1 encoding issue with mbedtls. Here is an excerpt from X690 8.3.2 Encoding of an integer rule. 8.3.2 If the contents octets of an integer value encoding consist of more than one octet, then the bits of the first octet and bit 8 of the second octet: a) shall not all be ones; and b) shall not all be zero. NOTE – These rules ensure that an integer value is always encoded in the smallest possible number of octets. That is, Integer encoding 02020001 is invalid, 020101 is valid. (which means 1) Likewise Integer encoding 0202FFFF is invalid, 0201FF is valid (which means -1)

The ‘mbedtls_ecdsa_read_signature’ function allows the above invalid encoding.

I’ve attached simple test programs with a small variation of mbedtls/programs/pkey/ecdsa.c: image The result of two verifying are both pass. image On the other hand, in the similar test with openssl, the second result is failed. I’ve also attached my primitive patch for this issue. image Could you review this issue? Thanks. Jongho Lee.

hanno-becker commented 6 years ago

Internal Reference: IOTSSL-2116.

daverodgman commented 2 years ago

Fixed in development and 2.28 LTS.

gilles-peskine-arm commented 2 years ago

@daverodgman I'm not sure what you're referring to when you say that something is fixed, but

Integer encoding 02020001 is invalid

is indeed the case according to the specification of DER, but Mbed TLS still accepts it when parsing an INTEGER today.

I'm not completely sure this is an issue we want to fix: canonicity is important, but so is interoperability, and there's a history of non-compliant non-canonical encodings floating around. Non-compliance isn't as common as it used to be, so it's possible that we might not have wanted to fix this a few years ago but we'd want to fix it now, or maybe we'd want to avoid changing the LTS branch but we should fix it in development.