Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PKCS#12 PBE doesn't convert to UTF-16BE correctly #337

Open
magnumripper opened this issue Dec 22, 2019 · 4 comments
Open

PKCS#12 PBE doesn't convert to UTF-16BE correctly #337

magnumripper opened this issue Dec 22, 2019 · 4 comments

Comments

@magnumripper
Copy link

magnumripper commented Dec 22, 2019

See openwall/john#4179

The PKCS#12 PBE function cheats when "converting" to UTF-16BE. It simply casts char -> short, or "inserts every other zero". While this happens to be correct for ISO-8859-1, it's not for the remaining 99.8% of the Unicode charset or 99.5% of UCS-2.

Actually, as far as I can see, these functions can't ever take a "string" as input unless they also take a parameter describing the encoding of it (that, or requiring a certain encoding - which should probably be UTF-8 for guaranteeing full functionality) because otherwise you simply can't know how to convert it properly. Most other functions take binary data and length and then the caller can decide to convert [correctly] to UTF-16BE before calling them.

A super-quick "workaround" would be to clearly document that the function only work correctly for ASCII and ISO-8859-1. But then you wouldn't be compatible with libs that do it right.

This bug is particularly bad in that if you, say, encrypt a certificate on a system using mbedTLS and then try to decrypt it on a system without this bug, your password won't work. Or vice versa. We are seeing examples of this.

@magnumripper
Copy link
Author

magnumripper commented Dec 22, 2019

(See below) Please note that I haven't yet checked how OpenSSL handles this. I wouldn't be too surprised if they do the same, and in that case you are bug compatible with them.

The software we've seen so far that does the correct conversion happens to be running on Windows.

@ciarmcom
Copy link
Member

Internal Jira reference: https://jira.arm.com/browse/IOTCRYPT-1032

@magnumripper
Copy link
Author

magnumripper commented Dec 22, 2019

Apparently OpenSSL doesn't have this bug if used correctly: It has functions like OPENSSL_asc2uni() which works just like your code but it also has OPENSSL_utf82uni() and as long as the latter is used, caller is bug free in this aspect. The UTF-8 functions were added in November 2015.

Edit:
https://mta.openssl.org/pipermail/openssl-dev/2016-August/008371.html

@gilles-peskine-arm
Copy link
Collaborator

Mbed TLS mostly focuses on embedded devices where code size is at a premium and non-ASCII character encodings are rarely used, and we try to avoid breaking backward compatibility. So I favor:

  • Documenting that pkcs12_pbe_derive_key_iv expects its argument pwd to be encoded in latin1.
  • Adding a new function that supports Unicode. I don't know what the argument type should be: wchar_t? (Would create a dependency on that type, which freestanding C implementations might not have.) uint16_t? (Would this be useful in practice?) And should it be documented as taking a BMPString per RFC 7292 or UTF-16 (allowing characters outside the basic plane)?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants