-
Notifications
You must be signed in to change notification settings - Fork 84
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
Acruceru/mbedtls8 async support #163
Conversation
todo: pick up: #171 (comment) |
Going to split this off into multiple PRs. First is the preparation work: #182 Pending:
|
182: Initial changes for async support r=jethrogb a=AdrianCX Splitting #163 into separate parts. Also rebased since `@raoulstrackx` already helped with some pieces of the PR above. Code does the following: - Minor update to comments on Pk (revisited the latest code to check if any issues, none found) - verify with sig len 0 makes mbedtls assume default siglen and potentially overflow instead of stopping. - EntropyHolder can have different sources, either shared or not (request from another older PR) - Ported ALPN changes from `@mzohreva` branch - NullTerminatedList - Split off HandshakeContext and made it a sort of base class to Context so we can use it safely. (lots of ideas from `@jethrogb` here) - Made Context < T > as opposed to using Any, makes life easier for async and a lot of other use cases. Co-authored-by: Adrian Cruceru <[email protected]>
Hi @AdrianCX, |
@Taowyoo please feel free to pick it up, I can likely help if you run into issues or want to talk (email in profile) This was done quite some time ago and code needs to be revisited a bit, it's mostly adjusting async code from @mzohreva branch on 0.7 to 0.8. Unfortunately I kept adding things to the branch since PR was raised. Some notes on what might be needed:
I was changing C code to const everywhere to make sure it's true but I'm not sure if I finished that, so you either need to revert to '&mut self' or check that assumption is valid. Regarding this:
When this is merged then additional work on integrating with rust-nativetls might be useful. A old branch that had that: https://github.com/AdrianCX/rust-native-tls/tree/acruceru/rust-mbedtls |
@AdrianCX Thank you for providing these information. Helps a lot! |
226: Rebased Acruceru/mbedtls8 async support r=Taowyoo a=Taowyoo This is the successor of #163, except the commits in original PR. Following changes are added: - Add async tests to CI 2023-03-15 update: According to #163 (comment) Here are some to-dos in this PR: - [x] remove 'migration_mode' feature - [x] Since changing 'sign' from '&mut self' to '&self' was something that was in-progress, need to either revert to '&mut self' or check that assumption is valid. Regarding this: ```rust pub fn sign<F: Random>( &self, ``` Following two tasks could be put in another PR&issue - Performance tests (compare to 0.7 under same scenario, should give similar numbers) - Additional unit tests / run tests with valgrind. Co-authored-by: Adrian Cruceru <[email protected]> Co-authored-by: Yuxiang Cao <[email protected]>
Closed because this is done by #239 |
This is broken into 2 commits, please review per commit.
First one is the preparation needed to port the async support (plus a migration mode feature to allow both mbedtls7 and 8 to coexists)
Second is the actual async support ported from Mohsen's branch (with some changes due to reference counting)
Let me know if this should be merged (either first/both commits or none), I can also keep it as a separate branch for now similar to the 0.7 one.