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

Cannot use mbedtls_x509_crt_ext_cb_t (certificate extension callback) with mbedtls_ssl_conf_verify() #7182

Open
dimakuv opened this issue Feb 27, 2023 · 8 comments · May be fixed by #7846
Open
Labels
bug component-tls component-tls13 size-s Estimated task size: small (~2d)

Comments

@dimakuv
Copy link

dimakuv commented Feb 27, 2023

Suggested enhancement

The callback mbedtls_x509_crt_ext_cb_t allows to examine unknown-to-mbedTLS x.509 certificate extensions, and allow the certificate if the callback returns non-negative value.

This allows to implement a callback required for the new OID being standardized in the TCG DICE specification. The OID is 2.23.133.5.4.9 and is called tcg-dice-conceptual-message-wrapper (previously called tcg-dice-tagged-evidence). This OID is marked a critical extension.

mbed TLS by default fails on unknown critical extensions. So to verify a certificate with this new OID, we must use the above callback.

Unfortunately, it seems that invoking this callback is only implemented for the mbedtls_x509_crt_parse_der_with_ext_cb() function; this function is found in the libmbedx509.so library.

However, for our implementation in the Interoperable RA-TLS and Gramine projects, we would need a similar function that works at the level of mbedTLS's TLS library libmbedtls.so.

There seems to be nothing of the sort in libmbedtls.so. E.g., here is the excerpt of the exported symbols:

$ nm --defined-only libmbedtls.so | grep verify
00000000000163e0 T mbedtls_ssl_conf_verify
00000000000177f0 T mbedtls_ssl_get_verify_result
000000000001a960 T mbedtls_ssl_set_calc_verify_md
0000000000016560 T mbedtls_ssl_set_verify
0000000000015760 t ssl_calc_verify_tls_sha256
00000000000154d0 t ssl_calc_verify_tls_sha384

Currently, Gramine uses mbedtls_ssl_conf_verify() with the callback, that allows to correctly verify non-critical certificate extensions. However, as soon as we enable the "criticality" flag in the extension, mbedTLS's TLS handshake fails even before jumping into the callback.

Summary: the callback to verify certificate extensions mbedtls_x509_crt_ext_cb_t is implemented for X.509 parsing code (part of libmbedx509.so), but is not implemented for TLS handshake code (part of libmbedtls.so).

  • Is there a plan to implement similar callbacks for the TLS handshake logic?
  • Is there any work around for this problem?

The tested mbed TLS version is v3.3.0.

Justification

Mbed TLS needs this because there seems to be no way of using libmbedtls.so library with currently-unknown-to-mbedTLS certificate extensions.

Additional materials

Please see discussions:

Related mbedTLS issues:

@gilles-peskine-arm
Copy link
Contributor

I'm labeling this as a bug because while it requires a new feature (a new ssl_conf function), it's technically a regression in Mbed TLS 3.0, since in 2.x there was the (clumsy) workaround of compiling with MBEDTLS_X509_ALLOW_UNSUPPORTED_CRITICAL_EXTENSION.

@askourtis
Copy link

This is breaking for us as well, is there any update on the status? Thank you!

@gilles-peskine-arm
Copy link
Contributor

Unfortunately it's unlikely that we'll have time to work on this in the short or medium term.

Would you be willing to submit a patch that works for you? I can't make a firm promise, but that would greatly increase the chances that this gets resolved.

The patch would need:

  • to not break the existing API (feel free to propose something for a design review);
  • to have unit tests.

@askourtis
Copy link

We are working on it!

@askourtis askourtis linked a pull request Jun 27, 2023 that will close this issue
3 tasks
@gilles-peskine-arm
Copy link
Contributor

What even are mbedtls's priories anymore?

Mbed TLS's priority is resource-constrained devices that often don't use “general” CAs. This has been the case ever since Mbed TLS was called Mbed TLS. This ecosystem doesn't depend critically on certificate pinning.

a TLS library intended for client devices, not really for serving major servers

I wouldn't exactly say that. A lot of embedded servers use Mbed TLS. Major servers aren't our priority target, but neither are desktop/laptop/smartphone clients.

peers are completely unverifiable past one hard-coded certificate

??? Mbed TLS does verify certificate chains. Do you mean that you want to check the revocation status of certificates on every connection? When your ecosystem doesn't have hundreds of root CAs and has a very tight communication budget, pervasive revocation checks are rarely an acceptable cost-benefit compromise.

any future proofing in the form of quic interfaces are open to question

Again, we're a crypto and TLS library, not a QUIC library. QUIC is relevant to the web but less so in IoT and so far it isn't in our priority list.

@mpg
Copy link
Contributor

mpg commented Jul 4, 2023

Unfortunately it's unlikely that we'll have time to work on this in the short or medium term.

Including this? #7079

@ReeceSX If you're unhappy that some issues aren't fixed yet, and want to let us know they're blocking you, please comment on the relevant issues (and create new ones if necessary) rather than derailing other discussions. I'd also suggest sticking to technical points relevant to the issue at hand; overly general rants seem less likely to be constructive. Thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug component-tls component-tls13 size-s Estimated task size: small (~2d)
Projects
Status: [3.6] TLS 1.3 misc for LTS
Development

Successfully merging a pull request may close this issue.

5 participants
@mpg @dimakuv @gilles-peskine-arm @askourtis and others