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

Initial changes for async support #182

Merged
merged 1 commit into from
Feb 10, 2022
Merged

Initial changes for async support #182

merged 1 commit into from
Feb 10, 2022

Conversation

AdrianCX
Copy link
Contributor

@AdrianCX AdrianCX commented Feb 8, 2022

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.

@jethrogb
Copy link
Member

jethrogb commented Feb 8, 2022

verify with sig len 0 makes mbedtls assume default siglen and potentially overflow instead of stopping.

This is fixed in MbedTLS 2.28, I think?

mbedtls/src/pk/mod.rs Show resolved Hide resolved
mbedtls/src/pk/mod.rs Outdated Show resolved Hide resolved
mbedtls/Cargo.toml Outdated Show resolved Hide resolved
mbedtls/src/rng/ctr_drbg.rs Outdated Show resolved Hide resolved
mbedtls/src/ssl/config.rs Outdated Show resolved Hide resolved
mbedtls/src/ssl/context.rs Show resolved Hide resolved
mbedtls/src/ssl/context.rs Outdated Show resolved Hide resolved
mbedtls/src/ssl/context.rs Outdated Show resolved Hide resolved
mbedtls/src/ssl/context.rs Show resolved Hide resolved
mbedtls/src/wrapper_macros.rs Outdated Show resolved Hide resolved
@AdrianCX AdrianCX force-pushed the acruceru/async-prepare branch 3 times, most recently from c5fc912 to 4086897 Compare February 8, 2022 16:32
assert_eq!(pk.verify(digest, &[], &signature[0..len]).unwrap_err(), Error::PkBadInputData);


let mut dummy_sig = vec![];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: doesn't need to be Vec.

mbedtls/src/ssl/config.rs Outdated Show resolved Hide resolved
mbedtls/src/ssl/config.rs Show resolved Hide resolved
@@ -89,7 +84,23 @@ define!(
impl<'a> UnsafeFrom<ptr> {}
);

impl Context {
define!(
#[c_custom_ty(ssl_context)]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We discussed not using a macro here

mbedtls/src/ssl/context.rs Show resolved Hide resolved
mbedtls/src/ssl/context.rs Outdated Show resolved Hide resolved
@jethrogb
Copy link
Member

bors r+

@bors
Copy link
Contributor

bors bot commented Feb 10, 2022

Build succeeded:

  • continuous-integration/travis-ci/push

@bors bors bot merged commit 51ad965 into master Feb 10, 2022
@Taowyoo Taowyoo deleted the acruceru/async-prepare branch March 21, 2023 22:59
@Taowyoo Taowyoo restored the acruceru/async-prepare branch March 21, 2023 23:00
@Taowyoo Taowyoo deleted the acruceru/async-prepare branch July 12, 2023 00:26
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.

2 participants