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.

Sign up for GitHub

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

Jump to bottom

Invalid free in from_pem_multiple when loading invalid certificates #1

Closed
jethrogb opened this issue Apr 27, 2017 · 0 comments · Fixed by #128
Closed

Invalid free in from_pem_multiple when loading invalid certificates #1

jethrogb opened this issue Apr 27, 2017 · 0 comments · Fixed by #128
Labels
bug

Comments

@jethrogb
Copy link
Member

When mbedtls_x509_crt_parse parses multiple certificates, and it finds an invalid certificate (e.g. with unrecognized critical extensions), it will free the current certificate pointer. This means our strategy of calling once to determine the linked list size and then again to fill a pre-allocated linked list fails with an invalid cert in the input because mbedTLS will try to free the pre-allocated certificate.
@jethrogb jethrogb added the bug label Oct 30, 2018
@AdrianCX AdrianCX mentioned this issue Nov 30, 2020
Merged
bors bot added a commit that referenced this issue Dec 17, 2020
@bors
Merge #128
3cf759a 
128: MbedTLS Reference counted instead of lifetimes r=jethrogb a=AdrianCX

Moving from referene counting allows simpler move to native-tls / hyper.

Arc Changes:
- Each Config/Context/... will hold Arcs towards items it holds pointers to.
- This forces objects to live as long as needed, once no longer used they get destroyed by reference counting.

This allows passing the objects to multiple threads without worrying about lifetime.
I've also added notes why classes are Sync where used. Let me know if I missed any classes.

Usage example of an intermediate mbed-hyper integration is at: 
- https://github.com/fortanix/rust-mbedtls/tree/acruceru/wip-mbed-hyper-v2/mbedtls-hyper/examples/integrations

There I added a crate to wrap hyper - similar to native-tls. (that will be moved to native-tls layer soon)
That crate can be considered an integration test that I will raise a separate PR for.


Edit:

Changes after initial review:
-    Added forward_mbedtls_calloc / forward_mbedtls_free functions so we can pass certificates to and from mbedtls without allocator mismatches/corruptions.
-    Switched to MbedtlsList<Certificate> and Certificate. A MbedtlsBox is pending for this PR as well.
-    Fixed most comments.

Still pending:
-    Update define! macros
-    Add MbedtlsBox<Certificate>


Fixes #1
Partial progress on #3
Fixes #4
Fixes #8
Partially addresses #9

Co-authored-by: Adrian Cruceru <adrian.cruceru@fortanix.com>
@bors bors bot closed this as completed in ca38f0f Dec 17, 2020
mcr pushed a commit to mcr/rust-mbedtls that referenced this issue Aug 10, 2023
@algesten
fix building without tls. close fortanix#1
bf97aa9
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

MbedTLS Reference counted instead of lifetimes fortanix/rust-mbedtls
1 participant
@jethrogb