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
[interop] ASN1 NULL parameter for AlgorithmIdentifier MUST be omit for Elliptic Curve #2924
Comments
Oh joy, a bug where the sensible fix is to change the API :( Thanks for the report! Memo: when fixing this, add tests to make sure that Mbed TLS can read back both the correct encoding and the encoding it used to write. |
ARM Internal Ref: IOTSSL-2979 |
Note: this was reported again on the mailing list: https://lists.trustedfirmware.org/pipermail/mbed-tls/2020-December/000273.html |
Note: OpenSSL and GnuTLS (certtool) both generate the correct encoding, so clearly we can read the proper encoding, and we have test certificates generated using Mbed TLS, so we can also read the current incorrect encoding, but I agree that when fixing we need to have an explicit test for that. Also, I'm removing the label "component: crypto" as I believe this only affects X.509. The only place in the crypto library that calls |
The person who reported this by email confirmed that this bug is not just about compliance, but is causing interoperability issues in practice: for example, it causes Chrome to reject all ECDSA-signed certificates created with Mbed TLS. |
I tried to fix this problem (ran into it when trying to connect to a device with a self-signed certificate using ECC using Chrome after it has switched to its own Certificate Verifier). Could someone check if that looks ok? Thanks https://github.com/marekjansta/mbedtls/tree/fix-asn1-algorithm-identifier |
@marekjansta At first glance, the code looks correct. However the patch would not be acceptable in Mbed TLS because it changes the API. We can't change the signature of an existing function that's declared and documented in a public header. I think the best solution is to deprecate |
Hi All, good to see that the fix of the issue is ongoing. It will solve many interoperability issue. Do you know when it will be released? And in which version? Thanks |
@Fassino I'm afraid that so far we do not have a pending fix and I'm not aware of anybody working on it (unless @marekjansta plans to make a backward-compatible version of their fix?). |
I updated my solution so that it does not break the rules. |
@marekjansta Yes, that change looks fine in terms of not breaking the API. I haven't reviewed the implementation. And we would need unit tests, both for the new function and for the bug fix. Please make a pull request. Preferably with the tests, but if you don't have time for those, maybe someone else can contribute the tests. |
Fixed by #7788 |
Description
Bug
OS
Linux
mbed TLS build:
Version: Version: 2.16.3
Expected behavior
To be conform with RFC, mbedtls_asn1_write_algorithm_identifier MUSTN'T add a NULL parameter for Elliptic Curve Cryptography key pair.
RFC 5280 says NULL parameter is optional (see 4.1.1.2)
But, RFC 5754 (ECDSA for X509) says encoding MUST omit the parameters field (see 3.2).
Please note, that the RFC for RSA x509, specify that the NULL MUST be present… Cryptography is beautiful ;-)
Actual behavior
Implementation is not conform to RFC. So when using mbedtls to issue certificate based on ECC key pairs, the certificate is rejected by some cryptography implementations.
The text was updated successfully, but these errors were encountered: