mbedtls_asn1_get_mpi() silently misparses negative values

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.48k stars 2.59k forks source link

mbedtls_asn1_get_mpi() silently misparses negative values #3329

Open mpg opened 4 years ago

mpg commented 4 years ago

Description

Type: Bug
Priority: Minor

Bug

OS
Any.

mbed TLS build:
All versions.

Expected behavior

mbedtls_asn1_get_mpi() should either:

accept negative values and parse them properly according to the ASN.1 standard
OR return an error code on negative values and document that restriction properly.

Actual behavior

mbedtls_asn1_get_mpi() silently returns a positive value when parsing what ASN.1 defines as a negative value, and the documentations gives no hint than negative values are not supported.

Notes

See internal tickets https://jira.arm.com/browse/IOTSSL-1813 (on this issue) and https://jira.arm.com/browse/IOTSSL-2216 (on signed-ness issues in ASN.1 int/mpi parsing/writing in general) which have some analysis on possible resolutions.

gilles-peskine-arm commented 4 years ago

The current behavior may be necessary for interoperability with software that generates INTEGER representations without a leading 0 bit but expects them to be parsed as positive numbers. This used to be fairly common, so the current behavior may well have been on purpose. In 2020, I expect such broken software to be less common, but we do need to investigate interoperability and backward compatibility before changing the behavior.

mu578 commented 4 years ago

(lldb) p *n
(mbedtls_mpi) $1 = {
  s = -1
  n = 8
  p = 0x0000000100509590
}

ok a simple set of 4 transform functions between one's complement and two's complement is what is needed; instead of doing all those back and forth inlined work; 0 sign meaning absolute.

There is a deep confusion between number as an mpi and its one's complement and two's complement representation; the structural issue is deeper than just a bug in asn.1; the absolute thingy is just a bad excuse, lucky charm; you can't write two's complement once and then one's complement for another number; then restore it as one's complement; the first author of mpi interface was fully aware of what he was doing; the later authors not really, mostly living in lalaland. Any absolute value should always be represented as positive two's complement in case of asn.1