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

Support loading CA certs with unsupported extensions (IDFGH-2307) #4445

Closed

Conversation

ryankurte
Copy link
Contributor

@ryankurte ryankurte commented Dec 5, 2019

Hey hi,

This PR adds support for CA certificates with critical extensions unsupported by mbedtls (fixes esp-tls: mbedtls_x509_crt_parse returned -0x2562 when loading CA certificates with critical Name Constraints).

  • Added KConfig to mbedtls module
  • Added config option to mbedtls/esp_config.h

(It's be awesome if it was possible to land this in 4.0 but I wasn't sure what to open the PR against)

@claassistantio
Copy link

claassistantio commented Dec 5, 2019

CLA assistant check
All committers have signed the CLA.

@github-actions github-actions bot changed the title Support loading CA certs with unsupported extensions Support loading CA certs with unsupported extensions (IDFGH-2307) Dec 5, 2019
@mahavirj
Copy link
Member

mahavirj commented Dec 6, 2019

@ryankurte This is not recommended config option per https://github.com/ARMmbed/mbedtls/blob/development/include/mbedtls/config.h#L1875, I wonder if we should expose this through kconfig. Could you please help to elaborate on requirement? (CC @projectgus)

@ryankurte
Copy link
Contributor Author

Hey thanks for the reply, we use name constraints on our internal TLS infrastructure which are unsupported by mbed-tls, leaving us with the option to disable this or to re-issue our infrastructure without name constraints.

If y'all don't want it in the kconfig we can just define it globally in our projects, it was however pretty annoying to track down so might make a good addition to the TLS docs somewhere?

@espressif espressif deleted a comment Dec 8, 2019
@projectgus
Copy link
Contributor

@mahavirj @ryankurte What about adding a "Show configurations with potential security risks" option to the mbedTLS menu that defaults to n, and make any non-recommended items depend on this being set first?

@ryankurte
Copy link
Contributor Author

seems reasonable, something like this?

Screenshot from 2019-12-10 10-52-07

Screenshot from 2019-12-10 10-52-34

@projectgus
Copy link
Contributor

Hi @ryankurte,

Yes, something like that! There is a way to do it without moving all these config items under the parent, something like visible if instead of depends. But I think it's fine either way.

@mahavirj
Copy link
Member

@ryankurte Thanks for changes. I will put this up in our internal review queue.

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

Successfully merging this pull request may close these issues.

4 participants